[Skiboot] [PATCH] npu2: Use unfiltered mode in XTS tables

Alistair Popple alistair at popple.id.au
Fri Mar 9 10:33:51 AEDT 2018


Thanks Reza!

I had one very minor thought below but it doesn't require a respin - everything
looks correct and seemed to work on our limited test setup. Note I was only able
to excersise the translation request path and not the shootdown path as we lack
an application stack to test ATSDs.

Acked-by: Alistair Popple <alistair at popple.id.au>

> -static int opal_npu_destroy_context(uint64_t phb_id, uint64_t pid, uint64_t bdf)
> +static int opal_npu_destroy_context(uint64_t phb_id, uint64_t pid __unused,
> +				    uint64_t bdf)
>  {
>  	struct phb *phb = pci_get_phb(phb_id);
>  	struct npu2 *p = phb_to_npu2(phb);
> -	uint64_t xts_bdf, xts_bdf_pid;
> -	uint64_t lparshort;
> -	int id, rc = 0;
> +	uint64_t xts_bdf;
> +	int rc = 0;
>  
>  	if (!phb || phb->phb_type != phb_type_npu_v2)
>  		return OPAL_PARAMETER;
> @@ -2137,26 +2136,13 @@ static int opal_npu_destroy_context(uint64_t phb_id, uint64_t pid, uint64_t bdf)
>  			     &xts_bdf, NPU2_XTS_BDF_MAP_BDF) < 0) {
>  		NPU2ERR(p, "LPARID not associated with any GPU\n");

I guess this check is still somewhat useful to catch a buggy device driver out
but I'm not convinced it provides much value - we could just as easily remove it
as well.

>  		rc = OPAL_PARAMETER;
> -		goto out;
>  	}
>  
> -	lparshort = GETFIELD(NPU2_XTS_BDF_MAP_LPARSHORT, xts_bdf);
> -	NPU2DBG(p, "Found LPARSHORT = 0x%llx destroy context for BDF = 0x%03llx PID = 0x%llx\n",
> -		lparshort, bdf, pid);
> -
> -	/* Now find the entry in the bdf/pid table */
> -	xts_bdf_pid = SETFIELD(NPU2_XTS_PID_MAP_LPARSHORT, 0ul, lparshort);
> -	xts_bdf_pid = SETFIELD(NPU2_XTS_PID_MAP_PASID, xts_bdf_pid, pid);
> -	id = npu_table_search(p, NPU2_XTS_PID_MAP, 0x20, NPU2_XTS_PID_MAP_SIZE, &xts_bdf_pid,
> -			      NPU2_XTS_PID_MAP_LPARSHORT | NPU2_XTS_PID_MAP_PASID);
> -	if (id < 0) {
> -		rc = OPAL_PARAMETER;
> -		goto out;
> -	}
> +	/*
> +	 * The bdf/pid table only contains wildcard entries, so we don't
> +	 * need to remove anything here.
> +	 */
>  
> -	/* And zero the entry */
> -	npu2_write(p, NPU2_XTS_PID_MAP + id*0x20, 0);
> -out:
>  	unlock(&p->lock);
>  	return rc;
>  }
> @@ -2208,6 +2194,7 @@ static int opal_npu_map_lpar(uint64_t phb_id, uint64_t bdf, uint64_t lparid,
>  	}
>  
>  	xts_bdf_lpar = SETFIELD(NPU2_XTS_BDF_MAP_VALID, 0UL, 1);
> +	xts_bdf_lpar = SETFIELD(NPU2_XTS_BDF_MAP_UNFILT, xts_bdf_lpar, 1);
>  	xts_bdf_lpar = SETFIELD(NPU2_XTS_BDF_MAP_BDF, xts_bdf_lpar, bdf);
>  
>  	/* We only support radix for the moment */
> diff --git a/include/npu2-regs.h b/include/npu2-regs.h
> index 73925f9..fd249f4 100644
> --- a/include/npu2-regs.h
> +++ b/include/npu2-regs.h
> @@ -425,6 +425,7 @@ void npu2_write_mask(struct npu2 *p, uint64_t reg, uint64_t val, uint64_t mask);
>  #define NPU2_XTS_MMIO_ATSD7_LPARID		NPU2_REG_OFFSET(NPU2_STACK_MISC, NPU2_BLOCK_XTS, 0x138)
>  #define NPU2_XTS_BDF_MAP			NPU2_REG_OFFSET(NPU2_STACK_MISC, NPU2_BLOCK_XTS, 0x4000)
>  #define   NPU2_XTS_BDF_MAP_VALID		PPC_BIT(0)
> +#define   NPU2_XTS_BDF_MAP_UNFILT		PPC_BIT(1)
>  #define   NPU2_XTS_BDF_MAP_STACK		PPC_BITMASK(4, 6)
>  #define   NPU2_XTS_BDF_MAP_BRICK		PPC_BIT(7)
>  #define   NPU2_XTS_BDF_MAP_BDF			PPC_BITMASK(16, 31)
> @@ -437,6 +438,7 @@ void npu2_write_mask(struct npu2 *p, uint64_t reg, uint64_t val, uint64_t mask);
>  #define   NPU2_XTS_PID_MAP_VALID_ATRGPA0	PPC_BIT(0)
>  #define   NPU2_XTS_PID_MAP_VALID_ATRGPA1	PPC_BIT(1)
>  #define   NPU2_XTS_PID_MAP_VALID_ATSD		PPC_BIT(2)
> +#define   NPU2_XTS_PID_MAP_MSR			PPC_BITMASK(25,31)
>  #define   NPU2_XTS_PID_MAP_MSR_DR		PPC_BIT(25)
>  #define   NPU2_XTS_PID_MAP_MSR_TA		PPC_BIT(26)
>  #define   NPU2_XTS_PID_MAP_MSR_HV		PPC_BIT(27)
> 




More information about the Skiboot mailing list