Re: [PATCH V2 1/5] ocxl: Assign a register set to a Logical Partition

[Frederic Barrat <fbarrat@linux.ibm.com>]

On 20/11/2020 18:32, Christophe Lombard wrote:
> Platform specific function to assign a register set to a Logical Partition.
> The "ibm,mmio-atsd" property, provided by the firmware, contains the 16
> base ATSD physical addresses (ATSD0 through ATSD15) of the set of MMIO
> registers (XTS MMIO ATSDx LPARID/AVA/launch/status register).
> 
> For the time being, the ATSD0 set of registers is used by default.
> 
> Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
> ---
>   arch/powerpc/include/asm/pnv-ocxl.h   |  3 ++
>   arch/powerpc/platforms/powernv/ocxl.c | 48 +++++++++++++++++++++++++++
>   2 files changed, 51 insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/pnv-ocxl.h b/arch/powerpc/include/asm/pnv-ocxl.h
> index d37ededca3ee..3f38aed7100c 100644
> --- a/arch/powerpc/include/asm/pnv-ocxl.h
> +++ b/arch/powerpc/include/asm/pnv-ocxl.h
> @@ -28,4 +28,7 @@ int pnv_ocxl_spa_setup(struct pci_dev *dev, void *spa_mem, int PE_mask, void **p
>   void pnv_ocxl_spa_release(void *platform_data);
>   int pnv_ocxl_spa_remove_pe_from_cache(void *platform_data, int pe_handle);
> 
> +int pnv_ocxl_map_lpar(struct pci_dev *dev, uint64_t lparid,
> +		      uint64_t lpcr, void __iomem **arva);
> +void pnv_ocxl_unmap_lpar(void __iomem **arva);
>   #endif /* _ASM_PNV_OCXL_H */
> diff --git a/arch/powerpc/platforms/powernv/ocxl.c b/arch/powerpc/platforms/powernv/ocxl.c
> index ecdad219d704..bc20cf867900 100644
> --- a/arch/powerpc/platforms/powernv/ocxl.c
> +++ b/arch/powerpc/platforms/powernv/ocxl.c
> @@ -483,3 +483,51 @@ int pnv_ocxl_spa_remove_pe_from_cache(void *platform_data, int pe_handle)
>   	return rc;
>   }
>   EXPORT_SYMBOL_GPL(pnv_ocxl_spa_remove_pe_from_cache);
> +
> +int pnv_ocxl_map_lpar(struct pci_dev *dev, uint64_t lparid,
> +		      uint64_t lpcr, void __iomem **arva)
> +{
> +	struct pci_controller *hose = pci_bus_to_host(dev->bus);
> +	struct pnv_phb *phb = hose->private_data;
> +	u64 mmio_atsd;
> +	int rc;
> +
> +	/* ATSD physical address.
> +	 * ATSD LAUNCH register: write access initiates a shoot down to
> +	 * initiate the TLB Invalidate command.
> +	 */
> +	rc = of_property_read_u64_index(hose->dn, "ibm,mmio-atsd",
> +					0, &mmio_atsd);
> +	if (rc) {
> +		dev_info(&dev->dev, "No available ATSD found\n");
> +		return rc;
> +	}
> +
> +	/* Assign a register set to a Logical Partition and MMIO ATSD
> +	 * LPARID register to the required value.
> +	 */
> +	if (mmio_atsd)


If we don't have the "ibm,mmio-atsd", i.e on P9, then we've already 
exited above. So why not consider mmio_atsd as an error?


> +		rc = opal_npu_map_lpar(phb->opal_id, pci_dev_id(dev),
> +				       lparid, lpcr);
> +	if (rc) {
> +		dev_err(&dev->dev, "Error mapping device to LPAR: %d\n", rc);
> +		return rc;
> +	}
> +
> +	if (mmio_atsd) {


Same here


> +		*arva = ioremap(mmio_atsd, 24);
> +		if (!(*arva)) {
> +			dev_warn(&dev->dev, "ioremap failed - mmio_atsd: %#llx\n", mmio_atsd);
> +			rc = -ENOMEM;
> +		}
> +	}
> +
> +	return rc;
> +}
> +EXPORT_SYMBOL_GPL(pnv_ocxl_map_lpar);
> +
> +void pnv_ocxl_unmap_lpar(void __iomem **arva)


The arva argument doesn't need to be a double pointer. Void * is enough.

   Fred


> +{
> +	iounmap(*arva);
> +}
> +EXPORT_SYMBOL_GPL(pnv_ocxl_unmap_lpar);
>