[PATCH 0/5] ocxl: Allow external drivers to access LPC memory

[PATCH 0/5] ocxl: Allow external drivers to access LPC memory

[Alastair D'Silva]

From: Alastair D'Silva <alastair@d-silva.org>

This series provides the prerequisite infrastructure to allow
external drivers to map & access OpenCAPI LPC memory.

Alastair D'Silva (5):
  powerpc: Add OPAL calls for LPC memory alloc/release
  powerpc: Map & release OpenCAPI LPC memory
  ocxl: Tally up the LPC memory on a link & allow it to be mapped
  ocxl: Add functions to map/unmap LPC memory
  ocxl: Provide additional metadata to userspace

 arch/powerpc/include/asm/opal-api.h        |  4 +-
 arch/powerpc/include/asm/opal.h            |  3 ++
 arch/powerpc/include/asm/pnv-ocxl.h        |  2 +
 arch/powerpc/platforms/powernv/ocxl.c      | 42 +++++++++++++++
 arch/powerpc/platforms/powernv/opal-call.c |  2 +
 drivers/misc/ocxl/config.c                 | 50 ++++++++++++++++++
 drivers/misc/ocxl/core.c                   | 59 +++++++++++++++++++++
 drivers/misc/ocxl/file.c                   |  3 +-
 drivers/misc/ocxl/link.c                   | 61 ++++++++++++++++++++++
 drivers/misc/ocxl/ocxl_internal.h          | 48 +++++++++++++++++
 include/misc/ocxl.h                        | 19 +++++++
 include/uapi/misc/ocxl.h                   |  9 +++-
 12 files changed, 299 insertions(+), 3 deletions(-)

-- 
2.21.0

[PATCH 1/5] powerpc: Add OPAL calls for LPC memory alloc/release

[Alastair D'Silva]

From: Alastair D'Silva <alastair@d-silva.org>

Add OPAL calls for LPC memory alloc/release

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 arch/powerpc/include/asm/opal-api.h        | 4 +++-
 arch/powerpc/include/asm/opal.h            | 3 +++
 arch/powerpc/platforms/powernv/opal-call.c | 2 ++
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
index 383242eb0dea..c58161cd7cfb 100644
--- a/arch/powerpc/include/asm/opal-api.h
+++ b/arch/powerpc/include/asm/opal-api.h
@@ -208,7 +208,9 @@
 #define OPAL_HANDLE_HMI2			166
 #define	OPAL_NX_COPROC_INIT			167
 #define OPAL_XIVE_GET_VP_STATE			170
-#define OPAL_LAST				170
+#define OPAL_NPU_MEM_ALLOC			171
+#define OPAL_NPU_MEM_RELEASE			172
+#define OPAL_LAST				172
 
 #define QUIESCE_HOLD			1 /* Spin all calls at entry */
 #define QUIESCE_REJECT			2 /* Fail all calls with OPAL_BUSY */
diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index 57bd029c715e..8c957a003be4 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -39,6 +39,9 @@ int64_t opal_npu_spa_clear_cache(uint64_t phb_id, uint32_t bdfn,
 				uint64_t PE_handle);
 int64_t opal_npu_tl_set(uint64_t phb_id, uint32_t bdfn, long cap,
 			uint64_t rate_phys, uint32_t size);
+int64_t opal_npu_mem_alloc(uint64_t phb_id, uint32_t bdfn,
+			uint64_t size, uint64_t *bar);
+int64_t opal_npu_mem_release(uint64_t phb_id, uint32_t bdfn);
 int64_t opal_console_write(int64_t term_number, __be64 *length,
 			   const uint8_t *buffer);
 int64_t opal_console_read(int64_t term_number, __be64 *length,
diff --git a/arch/powerpc/platforms/powernv/opal-call.c b/arch/powerpc/platforms/powernv/opal-call.c
index 29ca523c1c79..09a280446507 100644
--- a/arch/powerpc/platforms/powernv/opal-call.c
+++ b/arch/powerpc/platforms/powernv/opal-call.c
@@ -287,3 +287,5 @@ OPAL_CALL(opal_pci_set_pbcq_tunnel_bar,		OPAL_PCI_SET_PBCQ_TUNNEL_BAR);
 OPAL_CALL(opal_sensor_read_u64,			OPAL_SENSOR_READ_U64);
 OPAL_CALL(opal_sensor_group_enable,		OPAL_SENSOR_GROUP_ENABLE);
 OPAL_CALL(opal_nx_coproc_init,			OPAL_NX_COPROC_INIT);
+OPAL_CALL(opal_npu_mem_alloc,			OPAL_NPU_MEM_ALLOC);
+OPAL_CALL(opal_npu_mem_release,			OPAL_NPU_MEM_RELEASE);
-- 
2.21.0

Re: [PATCH 1/5] powerpc: Add OPAL calls for LPC memory alloc/release

[Andrew Donnellan]

On 17/9/19 3:42 am, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
> 
> Add OPAL calls for LPC memory alloc/release
> 
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>

This needs rebasing, but apart from that.

Acked-by: Andrew Donnellan <ajd@linux.ibm.com>

> ---
>   arch/powerpc/include/asm/opal-api.h        | 4 +++-
>   arch/powerpc/include/asm/opal.h            | 3 +++
>   arch/powerpc/platforms/powernv/opal-call.c | 2 ++
>   3 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
> index 383242eb0dea..c58161cd7cfb 100644
> --- a/arch/powerpc/include/asm/opal-api.h
> +++ b/arch/powerpc/include/asm/opal-api.h
> @@ -208,7 +208,9 @@
>   #define OPAL_HANDLE_HMI2			166
>   #define	OPAL_NX_COPROC_INIT			167
>   #define OPAL_XIVE_GET_VP_STATE			170
> -#define OPAL_LAST				170
> +#define OPAL_NPU_MEM_ALLOC			171
> +#define OPAL_NPU_MEM_RELEASE			172
> +#define OPAL_LAST				172
>   
>   #define QUIESCE_HOLD			1 /* Spin all calls at entry */
>   #define QUIESCE_REJECT			2 /* Fail all calls with OPAL_BUSY */
> diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
> index 57bd029c715e..8c957a003be4 100644
> --- a/arch/powerpc/include/asm/opal.h
> +++ b/arch/powerpc/include/asm/opal.h
> @@ -39,6 +39,9 @@ int64_t opal_npu_spa_clear_cache(uint64_t phb_id, uint32_t bdfn,
>   				uint64_t PE_handle);
>   int64_t opal_npu_tl_set(uint64_t phb_id, uint32_t bdfn, long cap,
>   			uint64_t rate_phys, uint32_t size);
> +int64_t opal_npu_mem_alloc(uint64_t phb_id, uint32_t bdfn,
> +			uint64_t size, uint64_t *bar);
> +int64_t opal_npu_mem_release(uint64_t phb_id, uint32_t bdfn);
>   int64_t opal_console_write(int64_t term_number, __be64 *length,
>   			   const uint8_t *buffer);
>   int64_t opal_console_read(int64_t term_number, __be64 *length,
> diff --git a/arch/powerpc/platforms/powernv/opal-call.c b/arch/powerpc/platforms/powernv/opal-call.c
> index 29ca523c1c79..09a280446507 100644
> --- a/arch/powerpc/platforms/powernv/opal-call.c
> +++ b/arch/powerpc/platforms/powernv/opal-call.c
> @@ -287,3 +287,5 @@ OPAL_CALL(opal_pci_set_pbcq_tunnel_bar,		OPAL_PCI_SET_PBCQ_TUNNEL_BAR);
>   OPAL_CALL(opal_sensor_read_u64,			OPAL_SENSOR_READ_U64);
>   OPAL_CALL(opal_sensor_group_enable,		OPAL_SENSOR_GROUP_ENABLE);
>   OPAL_CALL(opal_nx_coproc_init,			OPAL_NX_COPROC_INIT);
> +OPAL_CALL(opal_npu_mem_alloc,			OPAL_NPU_MEM_ALLOC);
> +OPAL_CALL(opal_npu_mem_release,			OPAL_NPU_MEM_RELEASE);
> 

-- 
Andrew Donnellan              OzLabs, ADL Canberra
ajd@linux.ibm.com             IBM Australia Limited

[PATCH 2/5] powerpc: Map & release OpenCAPI LPC memory

[Alastair D'Silva]

From: Alastair D'Silva <alastair@d-silva.org>

Map & release OpenCAPI LPC memory.

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 arch/powerpc/include/asm/pnv-ocxl.h   |  2 ++
 arch/powerpc/platforms/powernv/ocxl.c | 42 +++++++++++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/arch/powerpc/include/asm/pnv-ocxl.h b/arch/powerpc/include/asm/pnv-ocxl.h
index 7de82647e761..f8f8ffb48aa8 100644
--- a/arch/powerpc/include/asm/pnv-ocxl.h
+++ b/arch/powerpc/include/asm/pnv-ocxl.h
@@ -32,5 +32,7 @@ extern int pnv_ocxl_spa_remove_pe_from_cache(void *platform_data, int pe_handle)
 
 extern int pnv_ocxl_alloc_xive_irq(u32 *irq, u64 *trigger_addr);
 extern void pnv_ocxl_free_xive_irq(u32 irq);
+extern u64 pnv_ocxl_platform_lpc_setup(struct pci_dev *pdev, u64 size);
+extern void pnv_ocxl_platform_lpc_release(struct pci_dev *pdev);
 
 #endif /* _ASM_PNV_OCXL_H */
diff --git a/arch/powerpc/platforms/powernv/ocxl.c b/arch/powerpc/platforms/powernv/ocxl.c
index 8c65aacda9c8..81393728d6a3 100644
--- a/arch/powerpc/platforms/powernv/ocxl.c
+++ b/arch/powerpc/platforms/powernv/ocxl.c
@@ -475,6 +475,48 @@ void pnv_ocxl_spa_release(void *platform_data)
 }
 EXPORT_SYMBOL_GPL(pnv_ocxl_spa_release);
 
+u64 pnv_ocxl_platform_lpc_setup(struct pci_dev *pdev, u64 size)
+{
+	struct pci_controller *hose = pci_bus_to_host(pdev->bus);
+	struct pnv_phb *phb = hose->private_data;
+	struct pci_dn *pdn = pci_get_pdn(pdev);
+	u32 bdfn = (pdn->busno << 8) | pdn->devfn;
+	u64 base_addr = 0;
+
+	int rc = opal_npu_mem_alloc(phb->opal_id, bdfn, size, &base_addr);
+
+	WARN_ON(rc);
+
+	base_addr = be64_to_cpu(base_addr);
+
+	rc = check_hotplug_memory_addressable(base_addr, base_addr + size);
+	if (rc) {
+		dev_warn(&pdev->dev,
+			 "LPC memory range 0x%llx-0x%llx is not fully addressable",
+			 base_addr, base_addr + size - 1);
+		return 0;
+	}
+
+
+	return base_addr;
+}
+EXPORT_SYMBOL_GPL(pnv_ocxl_platform_lpc_setup);
+
+void pnv_ocxl_platform_lpc_release(struct pci_dev *pdev)
+{
+	struct pci_controller *hose = pci_bus_to_host(pdev->bus);
+	struct pnv_phb *phb = hose->private_data;
+	struct pci_dn *pdn = pci_get_pdn(pdev);
+	u32 bdfn;
+	int rc;
+
+	bdfn = (pdn->busno << 8) | pdn->devfn;
+	rc = opal_npu_mem_release(phb->opal_id, bdfn);
+	WARN_ON(rc);
+}
+EXPORT_SYMBOL_GPL(pnv_ocxl_platform_lpc_release);
+
+
 int pnv_ocxl_spa_remove_pe_from_cache(void *platform_data, int pe_handle)
 {
 	struct spa_data *data = (struct spa_data *) platform_data;
-- 
2.21.0

Re: [PATCH 2/5] powerpc: Map & release OpenCAPI LPC memory

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2399 bytes --]

Hi Alastair,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[cannot apply to v5.3 next-20190916]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Alastair-D-Silva/ocxl-Allow-external-drivers-to-access-LPC-memory/20190917-094857
config: powerpc-allmodconfig (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=powerpc 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   arch/powerpc/platforms/powernv/ocxl.c: In function 'pnv_ocxl_platform_lpc_setup':
>> arch/powerpc/platforms/powernv/ocxl.c:492:7: error: implicit declaration of function 'check_hotplug_memory_addressable' [-Werror=implicit-function-declaration]
     rc = check_hotplug_memory_addressable(base_addr, base_addr + size);
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/check_hotplug_memory_addressable +492 arch/powerpc/platforms/powernv/ocxl.c

   477	
   478	u64 pnv_ocxl_platform_lpc_setup(struct pci_dev *pdev, u64 size)
   479	{
   480		struct pci_controller *hose = pci_bus_to_host(pdev->bus);
   481		struct pnv_phb *phb = hose->private_data;
   482		struct pci_dn *pdn = pci_get_pdn(pdev);
   483		u32 bdfn = (pdn->busno << 8) | pdn->devfn;
   484		u64 base_addr = 0;
   485	
   486		int rc = opal_npu_mem_alloc(phb->opal_id, bdfn, size, &base_addr);
   487	
   488		WARN_ON(rc);
   489	
   490		base_addr = be64_to_cpu(base_addr);
   491	
 > 492		rc = check_hotplug_memory_addressable(base_addr, base_addr + size);
   493		if (rc) {
   494			dev_warn(&pdev->dev,
   495				 "LPC memory range 0x%llx-0x%llx is not fully addressable",
   496				 base_addr, base_addr + size - 1);
   497			return 0;
   498		}
   499	
   500	
   501		return base_addr;
   502	}
   503	EXPORT_SYMBOL_GPL(pnv_ocxl_platform_lpc_setup);
   504	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 62351 bytes --]

Re: [PATCH 2/5] powerpc: Map & release OpenCAPI LPC memory

[Frederic Barrat]

Le 17/09/2019 à 03:42, Alastair D'Silva a écrit :
> From: Alastair D'Silva <alastair@d-silva.org>
> 
> Map & release OpenCAPI LPC memory.
> 
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
>   arch/powerpc/include/asm/pnv-ocxl.h   |  2 ++
>   arch/powerpc/platforms/powernv/ocxl.c | 42 +++++++++++++++++++++++++++
>   2 files changed, 44 insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/pnv-ocxl.h b/arch/powerpc/include/asm/pnv-ocxl.h
> index 7de82647e761..f8f8ffb48aa8 100644
> --- a/arch/powerpc/include/asm/pnv-ocxl.h
> +++ b/arch/powerpc/include/asm/pnv-ocxl.h
> @@ -32,5 +32,7 @@ extern int pnv_ocxl_spa_remove_pe_from_cache(void *platform_data, int pe_handle)
>   
>   extern int pnv_ocxl_alloc_xive_irq(u32 *irq, u64 *trigger_addr);
>   extern void pnv_ocxl_free_xive_irq(u32 irq);
> +extern u64 pnv_ocxl_platform_lpc_setup(struct pci_dev *pdev, u64 size);
> +extern void pnv_ocxl_platform_lpc_release(struct pci_dev *pdev);
>   
>   #endif /* _ASM_PNV_OCXL_H */
> diff --git a/arch/powerpc/platforms/powernv/ocxl.c b/arch/powerpc/platforms/powernv/ocxl.c
> index 8c65aacda9c8..81393728d6a3 100644
> --- a/arch/powerpc/platforms/powernv/ocxl.c
> +++ b/arch/powerpc/platforms/powernv/ocxl.c
> @@ -475,6 +475,48 @@ void pnv_ocxl_spa_release(void *platform_data)
>   }
>   EXPORT_SYMBOL_GPL(pnv_ocxl_spa_release);
>   
> +u64 pnv_ocxl_platform_lpc_setup(struct pci_dev *pdev, u64 size)
> +{
> +	struct pci_controller *hose = pci_bus_to_host(pdev->bus);
> +	struct pnv_phb *phb = hose->private_data;
> +	struct pci_dn *pdn = pci_get_pdn(pdev);
> +	u32 bdfn = (pdn->busno << 8) | pdn->devfn;


We can spare a call to pci_get_pdn() with
bdfn = (pdev->bus->number << 8) | pdev->devfn;


> +	u64 base_addr = 0;
> +
> +	int rc = opal_npu_mem_alloc(phb->opal_id, bdfn, size, &base_addr);
> +
> +	WARN_ON(rc);

Instead of a WARN, we should catch the error and return a null address 
to the caller.

> +
> +	base_addr = be64_to_cpu(base_addr);
> +
> +	rc = check_hotplug_memory_addressable(base_addr, base_addr + size);


That code is missing?


> +	if (rc) {
> +		dev_warn(&pdev->dev,
> +			 "LPC memory range 0x%llx-0x%llx is not fully addressable",
> +			 base_addr, base_addr + size - 1);
> +		return 0;
> +	}
> +
> +
> +	return base_addr;
> +}
> +EXPORT_SYMBOL_GPL(pnv_ocxl_platform_lpc_setup);
> +
> +void pnv_ocxl_platform_lpc_release(struct pci_dev *pdev)
> +{
> +	struct pci_controller *hose = pci_bus_to_host(pdev->bus);
> +	struct pnv_phb *phb = hose->private_data;
> +	struct pci_dn *pdn = pci_get_pdn(pdev);
> +	u32 bdfn;
> +	int rc;
> +
> +	bdfn = (pdn->busno << 8) | pdn->devfn;
> +	rc = opal_npu_mem_release(phb->opal_id, bdfn);
> +	WARN_ON(rc);


Same comments as above.

   Fred



> +}
> +EXPORT_SYMBOL_GPL(pnv_ocxl_platform_lpc_release);
> +
> +
>   int pnv_ocxl_spa_remove_pe_from_cache(void *platform_data, int pe_handle)
>   {
>   	struct spa_data *data = (struct spa_data *) platform_data;
> 

Re: [PATCH 2/5] powerpc: Map & release OpenCAPI LPC memory

[Alastair D'Silva]

On Wed, 2019-09-18 at 16:03 +0200, Frederic Barrat wrote:
> 
> Le 17/09/2019 à 03:42, Alastair D'Silva a écrit :
> > From: Alastair D'Silva <alastair@d-silva.org>
> > 
> > Map & release OpenCAPI LPC memory.
> > 
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > ---
> >   arch/powerpc/include/asm/pnv-ocxl.h   |  2 ++
> >   arch/powerpc/platforms/powernv/ocxl.c | 42
> > +++++++++++++++++++++++++++
> >   2 files changed, 44 insertions(+)
> > 
> > diff --git a/arch/powerpc/include/asm/pnv-ocxl.h
> > b/arch/powerpc/include/asm/pnv-ocxl.h
> > index 7de82647e761..f8f8ffb48aa8 100644
> > --- a/arch/powerpc/include/asm/pnv-ocxl.h
> > +++ b/arch/powerpc/include/asm/pnv-ocxl.h
> > @@ -32,5 +32,7 @@ extern int pnv_ocxl_spa_remove_pe_from_cache(void
> > *platform_data, int pe_handle)
> >   
> >   extern int pnv_ocxl_alloc_xive_irq(u32 *irq, u64 *trigger_addr);
> >   extern void pnv_ocxl_free_xive_irq(u32 irq);
> > +extern u64 pnv_ocxl_platform_lpc_setup(struct pci_dev *pdev, u64
> > size);
> > +extern void pnv_ocxl_platform_lpc_release(struct pci_dev *pdev);
> >   
> >   #endif /* _ASM_PNV_OCXL_H */
> > diff --git a/arch/powerpc/platforms/powernv/ocxl.c
> > b/arch/powerpc/platforms/powernv/ocxl.c
> > index 8c65aacda9c8..81393728d6a3 100644
> > --- a/arch/powerpc/platforms/powernv/ocxl.c
> > +++ b/arch/powerpc/platforms/powernv/ocxl.c
> > @@ -475,6 +475,48 @@ void pnv_ocxl_spa_release(void *platform_data)
> >   }
> >   EXPORT_SYMBOL_GPL(pnv_ocxl_spa_release);
> >   
> > +u64 pnv_ocxl_platform_lpc_setup(struct pci_dev *pdev, u64 size)
> > +{
> > +	struct pci_controller *hose = pci_bus_to_host(pdev->bus);
> > +	struct pnv_phb *phb = hose->private_data;
> > +	struct pci_dn *pdn = pci_get_pdn(pdev);
> > +	u32 bdfn = (pdn->busno << 8) | pdn->devfn;
> 
> We can spare a call to pci_get_pdn() with
> bdfn = (pdev->bus->number << 8) | pdev->devfn;
> 

Ok.

> 
> > +	u64 base_addr = 0;
> > +
> > +	int rc = opal_npu_mem_alloc(phb->opal_id, bdfn, size,
> > &base_addr);
> > +
> > +	WARN_ON(rc);
> 
> Instead of a WARN, we should catch the error and return a null
> address 
> to the caller.
> 

base_addr will be 0 in the error case, are you suggesting we just
remove the WARN_ON()?

> > +
> > +	base_addr = be64_to_cpu(base_addr);
> > +
> > +	rc = check_hotplug_memory_addressable(base_addr, base_addr +
> > size);
> 
> That code is missing?
> 

That's added in the following patch on the mm list:
 [PATCH v3 1/2] memory_hotplug: Add a bounds check to
check_hotplug_memory_range()

> 
> > +	if (rc) {
> > +		dev_warn(&pdev->dev,
> > +			 "LPC memory range 0x%llx-0x%llx is not fully
> > addressable",
> > +			 base_addr, base_addr + size - 1);
> > +		return 0;
> > +	}
> > +
> > +
> > +	return base_addr;
> > +}
> > +EXPORT_SYMBOL_GPL(pnv_ocxl_platform_lpc_setup);
> > +
> > +void pnv_ocxl_platform_lpc_release(struct pci_dev *pdev)
> > +{
> > +	struct pci_controller *hose = pci_bus_to_host(pdev->bus);
> > +	struct pnv_phb *phb = hose->private_data;
> > +	struct pci_dn *pdn = pci_get_pdn(pdev);
> > +	u32 bdfn;
> > +	int rc;
> > +
> > +	bdfn = (pdn->busno << 8) | pdn->devfn;
> > +	rc = opal_npu_mem_release(phb->opal_id, bdfn);
> > +	WARN_ON(rc);
> 
> Same comments as above.
> 
>    Fred
> 
> 
> 
> > +}
> > +EXPORT_SYMBOL_GPL(pnv_ocxl_platform_lpc_release);
> > +
> > +
> >   int pnv_ocxl_spa_remove_pe_from_cache(void *platform_data, int
> > pe_handle)
> >   {
> >   	struct spa_data *data = (struct spa_data *) platform_data;
> > 
-- 
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819

Re: [PATCH 2/5] powerpc: Map & release OpenCAPI LPC memory

[Frederic Barrat]

Le 19/09/2019 à 02:58, Alastair D'Silva a écrit :
> On Wed, 2019-09-18 at 16:03 +0200, Frederic Barrat wrote:
>>
>> Le 17/09/2019 à 03:42, Alastair D'Silva a écrit :
>>> From: Alastair D'Silva <alastair@d-silva.org>
>>>
>>> Map & release OpenCAPI LPC memory.
>>>
>>> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
>>> ---
>>>    arch/powerpc/include/asm/pnv-ocxl.h   |  2 ++
>>>    arch/powerpc/platforms/powernv/ocxl.c | 42
>>> +++++++++++++++++++++++++++
>>>    2 files changed, 44 insertions(+)
>>>
>>> diff --git a/arch/powerpc/include/asm/pnv-ocxl.h
>>> b/arch/powerpc/include/asm/pnv-ocxl.h
>>> index 7de82647e761..f8f8ffb48aa8 100644
>>> --- a/arch/powerpc/include/asm/pnv-ocxl.h
>>> +++ b/arch/powerpc/include/asm/pnv-ocxl.h
>>> @@ -32,5 +32,7 @@ extern int pnv_ocxl_spa_remove_pe_from_cache(void
>>> *platform_data, int pe_handle)
>>>    
>>>    extern int pnv_ocxl_alloc_xive_irq(u32 *irq, u64 *trigger_addr);
>>>    extern void pnv_ocxl_free_xive_irq(u32 irq);
>>> +extern u64 pnv_ocxl_platform_lpc_setup(struct pci_dev *pdev, u64
>>> size);
>>> +extern void pnv_ocxl_platform_lpc_release(struct pci_dev *pdev);
>>>    
>>>    #endif /* _ASM_PNV_OCXL_H */
>>> diff --git a/arch/powerpc/platforms/powernv/ocxl.c
>>> b/arch/powerpc/platforms/powernv/ocxl.c
>>> index 8c65aacda9c8..81393728d6a3 100644
>>> --- a/arch/powerpc/platforms/powernv/ocxl.c
>>> +++ b/arch/powerpc/platforms/powernv/ocxl.c
>>> @@ -475,6 +475,48 @@ void pnv_ocxl_spa_release(void *platform_data)
>>>    }
>>>    EXPORT_SYMBOL_GPL(pnv_ocxl_spa_release);
>>>    
>>> +u64 pnv_ocxl_platform_lpc_setup(struct pci_dev *pdev, u64 size)
>>> +{
>>> +	struct pci_controller *hose = pci_bus_to_host(pdev->bus);
>>> +	struct pnv_phb *phb = hose->private_data;
>>> +	struct pci_dn *pdn = pci_get_pdn(pdev);
>>> +	u32 bdfn = (pdn->busno << 8) | pdn->devfn;
>>
>> We can spare a call to pci_get_pdn() with
>> bdfn = (pdev->bus->number << 8) | pdev->devfn;
>>
> 
> Ok.
> 
>>
>>> +	u64 base_addr = 0;
>>> +
>>> +	int rc = opal_npu_mem_alloc(phb->opal_id, bdfn, size,
>>> &base_addr);
>>> +
>>> +	WARN_ON(rc);
>>
>> Instead of a WARN, we should catch the error and return a null
>> address
>> to the caller.
>>
> 
> base_addr will be 0 in the error case, are you suggesting we just
> remove the WARN_ON()?


Well, we don't really have any reason to keep going if the opal call 
fails, right? And anyway, I wouldn't make any assumption on the content 
of base_addr if the call fails.
But my remark was really to avoid polluting the logs with the WARN 
output. The stack backtrace and register content is scary and is not 
going to help in that situation. A proper error message is more suitable.

   Fred



>>> +
>>> +	base_addr = be64_to_cpu(base_addr);
>>> +
>>> +	rc = check_hotplug_memory_addressable(base_addr, base_addr +
>>> size);
>>
>> That code is missing?
>>
> 
> That's added in the following patch on the mm list:
>   [PATCH v3 1/2] memory_hotplug: Add a bounds check to
> check_hotplug_memory_range()
> 
>>
>>> +	if (rc) {
>>> +		dev_warn(&pdev->dev,
>>> +			 "LPC memory range 0x%llx-0x%llx is not fully
>>> addressable",
>>> +			 base_addr, base_addr + size - 1);
>>> +		return 0;
>>> +	}
>>> +
>>> +
>>> +	return base_addr;
>>> +}
>>> +EXPORT_SYMBOL_GPL(pnv_ocxl_platform_lpc_setup);
>>> +
>>> +void pnv_ocxl_platform_lpc_release(struct pci_dev *pdev)
>>> +{
>>> +	struct pci_controller *hose = pci_bus_to_host(pdev->bus);
>>> +	struct pnv_phb *phb = hose->private_data;
>>> +	struct pci_dn *pdn = pci_get_pdn(pdev);
>>> +	u32 bdfn;
>>> +	int rc;
>>> +
>>> +	bdfn = (pdn->busno << 8) | pdn->devfn;
>>> +	rc = opal_npu_mem_release(phb->opal_id, bdfn);
>>> +	WARN_ON(rc);
>>
>> Same comments as above.
>>
>>     Fred
>>
>>
>>
>>> +}
>>> +EXPORT_SYMBOL_GPL(pnv_ocxl_platform_lpc_release);
>>> +
>>> +
>>>    int pnv_ocxl_spa_remove_pe_from_cache(void *platform_data, int
>>> pe_handle)
>>>    {
>>>    	struct spa_data *data = (struct spa_data *) platform_data;
>>>

[PATCH 3/5] ocxl: Tally up the LPC memory on a link & allow it to be mapped

[Alastair D'Silva]

From: Alastair D'Silva <alastair@d-silva.org>

Tally up the LPC memory on an OpenCAPI link & allow it to be mapped

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 drivers/misc/ocxl/core.c          |  9 +++++
 drivers/misc/ocxl/link.c          | 61 +++++++++++++++++++++++++++++++
 drivers/misc/ocxl/ocxl_internal.h | 42 +++++++++++++++++++++
 3 files changed, 112 insertions(+)

diff --git a/drivers/misc/ocxl/core.c b/drivers/misc/ocxl/core.c
index b7a09b21ab36..fdfe4e0a34e1 100644
--- a/drivers/misc/ocxl/core.c
+++ b/drivers/misc/ocxl/core.c
@@ -230,8 +230,17 @@ static int configure_afu(struct ocxl_afu *afu, u8 afu_idx, struct pci_dev *dev)
 	if (rc)
 		goto err_free_pasid;
 
+	if (afu->config.lpc_mem_size || afu->config.special_purpose_mem_size) {
+		rc = ocxl_link_add_lpc_mem(afu->fn->link,
+			afu->config.lpc_mem_size + afu->config.special_purpose_mem_size);
+		if (rc)
+			goto err_free_mmio;
+	}
+
 	return 0;
 
+err_free_mmio:
+	unmap_mmio_areas(afu);
 err_free_pasid:
 	reclaim_afu_pasid(afu);
 err_free_actag:
diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c
index 58d111afd9f6..2874811a4398 100644
--- a/drivers/misc/ocxl/link.c
+++ b/drivers/misc/ocxl/link.c
@@ -84,6 +84,11 @@ struct ocxl_link {
 	int dev;
 	atomic_t irq_available;
 	struct spa *spa;
+	struct mutex lpc_mem_lock;
+	u64 lpc_mem_sz; /* Total amount of LPC memory presented on the link */
+	u64 lpc_mem;
+	int lpc_consumers;
+
 	void *platform_data;
 };
 static struct list_head links_list = LIST_HEAD_INIT(links_list);
@@ -396,6 +401,8 @@ static int alloc_link(struct pci_dev *dev, int PE_mask, struct ocxl_link **out_l
 	if (rc)
 		goto err_spa;
 
+	mutex_init(&link->lpc_mem_lock);
+
 	/* platform specific hook */
 	rc = pnv_ocxl_spa_setup(dev, link->spa->spa_mem, PE_mask,
 				&link->platform_data);
@@ -711,3 +718,57 @@ void ocxl_link_free_irq(void *link_handle, int hw_irq)
 	atomic_inc(&link->irq_available);
 }
 EXPORT_SYMBOL_GPL(ocxl_link_free_irq);
+
+int ocxl_link_add_lpc_mem(void *link_handle, u64 size)
+{
+	struct ocxl_link *link = (struct ocxl_link *) link_handle;
+
+	u64 orig_size;
+	bool good = false;
+
+	mutex_lock(&link->lpc_mem_lock);
+	orig_size = link->lpc_mem_sz;
+	link->lpc_mem_sz += size;
+
+	good = orig_size < link->lpc_mem_sz;
+	mutex_unlock(&link->lpc_mem_lock);
+
+	// Check for overflow
+	return (good) ? 0 : -EINVAL;
+}
+EXPORT_SYMBOL_GPL(ocxl_link_add_lpc_mem);
+
+u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev)
+{
+	struct ocxl_link *link = (struct ocxl_link *) link_handle;
+
+	mutex_lock(&link->lpc_mem_lock);
+	if (link->lpc_mem) {
+		u64 lpc_mem = link->lpc_mem;
+
+		link->lpc_consumers++;
+		mutex_unlock(&link->lpc_mem_lock);
+		return lpc_mem;
+	}
+
+	link->lpc_mem = pnv_ocxl_platform_lpc_setup(pdev, link->lpc_mem_sz);
+	if (link->lpc_mem)
+		link->lpc_consumers++;
+	mutex_unlock(&link->lpc_mem_lock);
+
+	return link->lpc_mem;
+}
+
+void ocxl_link_lpc_release(void *link_handle, struct pci_dev *pdev)
+{
+	struct ocxl_link *link = (struct ocxl_link *) link_handle;
+
+	mutex_lock(&link->lpc_mem_lock);
+	link->lpc_consumers--;
+	if (link->lpc_consumers == 0) {
+		pnv_ocxl_platform_lpc_release(pdev);
+		link->lpc_mem = 0;
+	}
+
+	mutex_unlock(&link->lpc_mem_lock);
+}
diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
index 97415afd79f3..db2647a90fc8 100644
--- a/drivers/misc/ocxl/ocxl_internal.h
+++ b/drivers/misc/ocxl/ocxl_internal.h
@@ -141,4 +141,46 @@ int ocxl_irq_offset_to_id(struct ocxl_context *ctx, u64 offset);
 u64 ocxl_irq_id_to_offset(struct ocxl_context *ctx, int irq_id);
 void ocxl_afu_irq_free_all(struct ocxl_context *ctx);
 
+/**
+ * Increment the amount of memory required by an OpenCAPI link
+ *
+ * link_handle: The OpenCAPI link handle
+ * size: The amount of memory to increment by
+ *
+ * Return 0 on success, negative on overflow
+ */
+extern int ocxl_link_add_lpc_mem(void *link_handle, u64 size);
+
+/**
+ * Get the amount of memory required by an OpenCAPI link
+ *
+ * link_handle: The OpenCAPI link handle
+ *
+ * Return the amount of memory required by the link, this value is undefined if
+ * ocxl_link_add_lpc_mem failed.
+ */
+extern u64 ocxl_link_get_lpc_mem_sz(void *link_handle);
+
+/**
+ * Map the LPC memory for an OpenCAPI device
+ *
+ * Since LPC memory belongs to a link, the whole LPC memory available
+ * on the link bust be mapped in order to make it accessible to a device.
+ *
+ * @link_handle: The OpenCAPI link handle
+ * @pdev: A device that is on the link
+ */
+u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev);
+
+/**
+ * Release the LPC memory device for an OpenCAPI device
+ *
+ * Releases LPC memory on an OpenCAPI link for a device. If this is the
+ * last device on the link to release the memory, unmap it from the link.
+ *
+ * @link_handle: The OpenCAPI link handle
+ * @pdev: A device that is on the link
+ */
+void ocxl_link_lpc_release(void *link_handle, struct pci_dev *pdev);
+
 #endif /* _OCXL_INTERNAL_H_ */
-- 
2.21.0

Re: [PATCH 3/5] ocxl: Tally up the LPC memory on a link & allow it to be mapped

[Frederic Barrat]

Le 17/09/2019 à 03:42, Alastair D'Silva a écrit :
> From: Alastair D'Silva <alastair@d-silva.org>
> 
> Tally up the LPC memory on an OpenCAPI link & allow it to be mapped
> 
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
>   drivers/misc/ocxl/core.c          |  9 +++++
>   drivers/misc/ocxl/link.c          | 61 +++++++++++++++++++++++++++++++
>   drivers/misc/ocxl/ocxl_internal.h | 42 +++++++++++++++++++++
>   3 files changed, 112 insertions(+)
> 
> diff --git a/drivers/misc/ocxl/core.c b/drivers/misc/ocxl/core.c
> index b7a09b21ab36..fdfe4e0a34e1 100644
> --- a/drivers/misc/ocxl/core.c
> +++ b/drivers/misc/ocxl/core.c
> @@ -230,8 +230,17 @@ static int configure_afu(struct ocxl_afu *afu, u8 afu_idx, struct pci_dev *dev)
>   	if (rc)
>   		goto err_free_pasid;
>   
> +	if (afu->config.lpc_mem_size || afu->config.special_purpose_mem_size) {
> +		rc = ocxl_link_add_lpc_mem(afu->fn->link,
> +			afu->config.lpc_mem_size + afu->config.special_purpose_mem_size);


I don't think we should count the special purpose memory, as it's not 
meant to be accessed through the GPU mem BAR, but I'll check.

What happens when unconfiguring the AFU? We should reduce the range (see 
also below). Partial reconfig doesn't seem so far off, so we should take 
it into account.


> +		if (rc)
> +			goto err_free_mmio;
> +	}
> +
>   	return 0;
>   
> +err_free_mmio:
> +	unmap_mmio_areas(afu);
>   err_free_pasid:
>   	reclaim_afu_pasid(afu);
>   err_free_actag:
> diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c
> index 58d111afd9f6..2874811a4398 100644
> --- a/drivers/misc/ocxl/link.c
> +++ b/drivers/misc/ocxl/link.c
> @@ -84,6 +84,11 @@ struct ocxl_link {
>   	int dev;
>   	atomic_t irq_available;
>   	struct spa *spa;
> +	struct mutex lpc_mem_lock;
> +	u64 lpc_mem_sz; /* Total amount of LPC memory presented on the link */
> +	u64 lpc_mem;
> +	int lpc_consumers;
> +
>   	void *platform_data;
>   };
>   static struct list_head links_list = LIST_HEAD_INIT(links_list);
> @@ -396,6 +401,8 @@ static int alloc_link(struct pci_dev *dev, int PE_mask, struct ocxl_link **out_l
>   	if (rc)
>   		goto err_spa;
>   
> +	mutex_init(&link->lpc_mem_lock);
> +
>   	/* platform specific hook */
>   	rc = pnv_ocxl_spa_setup(dev, link->spa->spa_mem, PE_mask,
>   				&link->platform_data);
> @@ -711,3 +718,57 @@ void ocxl_link_free_irq(void *link_handle, int hw_irq)
>   	atomic_inc(&link->irq_available);
>   }
>   EXPORT_SYMBOL_GPL(ocxl_link_free_irq);
> +
> +int ocxl_link_add_lpc_mem(void *link_handle, u64 size)
> +{
> +	struct ocxl_link *link = (struct ocxl_link *) link_handle;
> +
> +	u64 orig_size;
> +	bool good = false;
> +
> +	mutex_lock(&link->lpc_mem_lock);
> +	orig_size = link->lpc_mem_sz;
> +	link->lpc_mem_sz += size;



We have a choice to make here:
1. either we only support one LPC memory-carrying AFU (and the above is 
overkill)
2. or we support multiple AFUs with LPC memory (on the same function), 
but then I think the above is too simple.

 From the opencapi spec, each AFU can define a chunk of memory with a 
starting address and a size. There's no rule which says they have to be 
contiguous. There's no rule which says it must start at 0. So to support 
multiple AFUs with LPC memory, we should record the current maximum 
range instead of just the global size. Ultimately, we need to tell the 
NPU the range of permissible addresses. It starts at 0, so we need to 
take into account any intial offset and holes.

I would go for option 2, to at least be consistent within ocxl and 
support multiple AFUs. Even though I don't think we'll see FPGA images 
with multiple AFUs with LPC memory any time soon.


> +	good = orig_size < link->lpc_mem_sz;
> +	mutex_unlock(&link->lpc_mem_lock);
> +
> +	// Check for overflow
> +	return (good) ? 0 : -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(ocxl_link_add_lpc_mem);


Do the symbol really need to be exported? IIUC, the next patch defines a 
higher level ocxl_afu_map_lpc_mem() which is meant to be called by a 
calling driver.


> +
> +u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev)
> +{
> +	struct ocxl_link *link = (struct ocxl_link *) link_handle;
> +
> +	mutex_lock(&link->lpc_mem_lock);
> +	if (link->lpc_mem) {
> +		u64 lpc_mem = link->lpc_mem;
> +
> +		link->lpc_consumers++;
> +		mutex_unlock(&link->lpc_mem_lock);
> +		return lpc_mem;
> +	}
> +
> +	link->lpc_mem = pnv_ocxl_platform_lpc_setup(pdev, link->lpc_mem_sz);
> +	if (link->lpc_mem)
> +		link->lpc_consumers++;
> +	mutex_unlock(&link->lpc_mem_lock);
> +
> +	return link->lpc_mem;


Should be cached in a temp variable, like on the fast path, otherwise 
it's accessed with no lock.


> +}
> +
> +void ocxl_link_lpc_release(void *link_handle, struct pci_dev *pdev)
> +{
> +	struct ocxl_link *link = (struct ocxl_link *) link_handle;
> +
> +	mutex_lock(&link->lpc_mem_lock);
> +	link->lpc_consumers--;
> +	if (link->lpc_consumers == 0) {
> +		pnv_ocxl_platform_lpc_release(pdev);
> +		link->lpc_mem = 0;
> +	}
> +
> +	mutex_unlock(&link->lpc_mem_lock);
> +}
> diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
> index 97415afd79f3..db2647a90fc8 100644
> --- a/drivers/misc/ocxl/ocxl_internal.h
> +++ b/drivers/misc/ocxl/ocxl_internal.h
> @@ -141,4 +141,46 @@ int ocxl_irq_offset_to_id(struct ocxl_context *ctx, u64 offset);
>   u64 ocxl_irq_id_to_offset(struct ocxl_context *ctx, int irq_id);
>   void ocxl_afu_irq_free_all(struct ocxl_context *ctx);
>   
> +/**
> + * Increment the amount of memory required by an OpenCAPI link
> + *
> + * link_handle: The OpenCAPI link handle
> + * size: The amount of memory to increment by
> + *
> + * Return 0 on success, negative on overflow
> + */
> +extern int ocxl_link_add_lpc_mem(void *link_handle, u64 size);


We've removed all the 'extern' in a previous patch.

> +
> +/**
> + * Get the amount of memory required by an OpenCAPI link
> + *
> + * link_handle: The OpenCAPI link handle
> + *
> + * Return the amount of memory required by the link, this value is undefined if
> + * ocxl_link_add_lpc_mem failed.
> + */
> +extern u64 ocxl_link_get_lpc_mem_sz(void *link_handle);


I don't see that one defined anywhere.

   Fred


> +
> +/**
> + * Map the LPC memory for an OpenCAPI device
> + *
> + * Since LPC memory belongs to a link, the whole LPC memory available
> + * on the link bust be mapped in order to make it accessible to a device.
> + *
> + * @link_handle: The OpenCAPI link handle
> + * @pdev: A device that is on the link
> + */
> +u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev);
> +
> +/**
> + * Release the LPC memory device for an OpenCAPI device
> + *
> + * Releases LPC memory on an OpenCAPI link for a device. If this is the
> + * last device on the link to release the memory, unmap it from the link.
> + *
> + * @link_handle: The OpenCAPI link handle
> + * @pdev: A device that is on the link
> + */
> +void ocxl_link_lpc_release(void *link_handle, struct pci_dev *pdev);
> +
>   #endif /* _OCXL_INTERNAL_H_ */
> 

Re: [PATCH 3/5] ocxl: Tally up the LPC memory on a link & allow it to be mapped

[Alastair D'Silva]

On Wed, 2019-09-18 at 16:02 +0200, Frederic Barrat wrote:
> 
> Le 17/09/2019 à 03:42, Alastair D'Silva a écrit :
> > From: Alastair D'Silva <alastair@d-silva.org>
> > 
> > Tally up the LPC memory on an OpenCAPI link & allow it to be mapped
> > 
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > ---
> >   drivers/misc/ocxl/core.c          |  9 +++++
> >   drivers/misc/ocxl/link.c          | 61
> > +++++++++++++++++++++++++++++++
> >   drivers/misc/ocxl/ocxl_internal.h | 42 +++++++++++++++++++++
> >   3 files changed, 112 insertions(+)
> > 
> > diff --git a/drivers/misc/ocxl/core.c b/drivers/misc/ocxl/core.c
> > index b7a09b21ab36..fdfe4e0a34e1 100644
> > --- a/drivers/misc/ocxl/core.c
> > +++ b/drivers/misc/ocxl/core.c
> > @@ -230,8 +230,17 @@ static int configure_afu(struct ocxl_afu *afu,
> > u8 afu_idx, struct pci_dev *dev)
> >   	if (rc)
> >   		goto err_free_pasid;
> >   
> > +	if (afu->config.lpc_mem_size || afu-
> > >config.special_purpose_mem_size) {
> > +		rc = ocxl_link_add_lpc_mem(afu->fn->link,
> > +			afu->config.lpc_mem_size + afu-
> > >config.special_purpose_mem_size);
> 
> I don't think we should count the special purpose memory, as it's
> not 
> meant to be accessed through the GPU mem BAR, but I'll check.

At least for OpenCAPI 3.0, there is no other in-spec way to access the
memory if it is not mapped by the NPU.

> 
> What happens when unconfiguring the AFU? We should reduce the range
> (see 
> also below). Partial reconfig doesn't seem so far off, so we should
> take 
> it into account.
> 

The mapping is left until the last AFU on the link offlines it's
memory, at which point we clear the mapping from the NPU.

> 
> > +		if (rc)
> > +			goto err_free_mmio;
> > +	}
> > +
> >   	return 0;
> >   
> > +err_free_mmio:
> > +	unmap_mmio_areas(afu);
> >   err_free_pasid:
> >   	reclaim_afu_pasid(afu);
> >   err_free_actag:
> > diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c
> > index 58d111afd9f6..2874811a4398 100644
> > --- a/drivers/misc/ocxl/link.c
> > +++ b/drivers/misc/ocxl/link.c
> > @@ -84,6 +84,11 @@ struct ocxl_link {
> >   	int dev;
> >   	atomic_t irq_available;
> >   	struct spa *spa;
> > +	struct mutex lpc_mem_lock;
> > +	u64 lpc_mem_sz; /* Total amount of LPC memory presented on the
> > link */
> > +	u64 lpc_mem;
> > +	int lpc_consumers;
> > +
> >   	void *platform_data;
> >   };
> >   static struct list_head links_list = LIST_HEAD_INIT(links_list);
> > @@ -396,6 +401,8 @@ static int alloc_link(struct pci_dev *dev, int
> > PE_mask, struct ocxl_link **out_l
> >   	if (rc)
> >   		goto err_spa;
> >   
> > +	mutex_init(&link->lpc_mem_lock);
> > +
> >   	/* platform specific hook */
> >   	rc = pnv_ocxl_spa_setup(dev, link->spa->spa_mem, PE_mask,
> >   				&link->platform_data);
> > @@ -711,3 +718,57 @@ void ocxl_link_free_irq(void *link_handle, int
> > hw_irq)
> >   	atomic_inc(&link->irq_available);
> >   }
> >   EXPORT_SYMBOL_GPL(ocxl_link_free_irq);
> > +
> > +int ocxl_link_add_lpc_mem(void *link_handle, u64 size)
> > +{
> > +	struct ocxl_link *link = (struct ocxl_link *) link_handle;
> > +
> > +	u64 orig_size;
> > +	bool good = false;
> > +
> > +	mutex_lock(&link->lpc_mem_lock);
> > +	orig_size = link->lpc_mem_sz;
> > +	link->lpc_mem_sz += size;
> 
> 
> We have a choice to make here:
> 1. either we only support one LPC memory-carrying AFU (and the above
> is 
> overkill)
> 2. or we support multiple AFUs with LPC memory (on the same
> function), 
> but then I think the above is too simple.
> 
>  From the opencapi spec, each AFU can define a chunk of memory with
> a 
> starting address and a size. There's no rule which says they have to
> be 
> contiguous. There's no rule which says it must start at 0. So to
> support 
> multiple AFUs with LPC memory, we should record the current maximum 
> range instead of just the global size. Ultimately, we need to tell
> the 
> NPU the range of permissible addresses. It starts at 0, so we need
> to 
> take into account any intial offset and holes.
> 
> I would go for option 2, to at least be consistent within ocxl and 
> support multiple AFUs. Even though I don't think we'll see FPGA
> images 
> with multiple AFUs with LPC memory any time soon.
> 

Ill rework this to take an offset & size, the NPU will map from the
base address up to the largest offset + size provided across all AFUs
on the link.

> 
> > +	good = orig_size < link->lpc_mem_sz;
> > +	mutex_unlock(&link->lpc_mem_lock);
> > +
> > +	// Check for overflow
> > +	return (good) ? 0 : -EINVAL;
> > +}
> > +EXPORT_SYMBOL_GPL(ocxl_link_add_lpc_mem);
> 
> Do the symbol really need to be exported? IIUC, the next patch
> defines a 
> higher level ocxl_afu_map_lpc_mem() which is meant to be called by a 
> calling driver.
> 

No, I'll remove it.

> 
> > +
> > +u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev)
> > +{
> > +	struct ocxl_link *link = (struct ocxl_link *) link_handle;
> > +
> > +	mutex_lock(&link->lpc_mem_lock);
> > +	if (link->lpc_mem) {
> > +		u64 lpc_mem = link->lpc_mem;
> > +
> > +		link->lpc_consumers++;
> > +		mutex_unlock(&link->lpc_mem_lock);
> > +		return lpc_mem;
> > +	}
> > +
> > +	link->lpc_mem = pnv_ocxl_platform_lpc_setup(pdev, link-
> > >lpc_mem_sz);
> > +	if (link->lpc_mem)
> > +		link->lpc_consumers++;
> > +	mutex_unlock(&link->lpc_mem_lock);
> > +
> > +	return link->lpc_mem;
> 
> Should be cached in a temp variable, like on the fast path,
> otherwise 
> it's accessed with no lock.

Good spotting, thanks.

> 
> > +}
> > +
> > +void ocxl_link_lpc_release(void *link_handle, struct pci_dev
> > *pdev)
> > +{
> > +	struct ocxl_link *link = (struct ocxl_link *) link_handle;
> > +
> > +	mutex_lock(&link->lpc_mem_lock);
> > +	link->lpc_consumers--;
> > +	if (link->lpc_consumers == 0) {
> > +		pnv_ocxl_platform_lpc_release(pdev);
> > +		link->lpc_mem = 0;
> > +	}
> > +
> > +	mutex_unlock(&link->lpc_mem_lock);
> > +}
> > diff --git a/drivers/misc/ocxl/ocxl_internal.h
> > b/drivers/misc/ocxl/ocxl_internal.h
> > index 97415afd79f3..db2647a90fc8 100644
> > --- a/drivers/misc/ocxl/ocxl_internal.h
> > +++ b/drivers/misc/ocxl/ocxl_internal.h
> > @@ -141,4 +141,46 @@ int ocxl_irq_offset_to_id(struct ocxl_context
> > *ctx, u64 offset);
> >   u64 ocxl_irq_id_to_offset(struct ocxl_context *ctx, int irq_id);
> >   void ocxl_afu_irq_free_all(struct ocxl_context *ctx);
> >   
> > +/**
> > + * Increment the amount of memory required by an OpenCAPI link
> > + *
> > + * link_handle: The OpenCAPI link handle
> > + * size: The amount of memory to increment by
> > + *
> > + * Return 0 on success, negative on overflow
> > + */
> > +extern int ocxl_link_add_lpc_mem(void *link_handle, u64 size);
> 
> We've removed all the 'extern' in a previous patch.

Thanks, I spotted this too (after I posted it).

> > +
> > +/**
> > + * Get the amount of memory required by an OpenCAPI link
> > + *
> > + * link_handle: The OpenCAPI link handle
> > + *
> > + * Return the amount of memory required by the link, this value is
> > undefined if
> > + * ocxl_link_add_lpc_mem failed.
> > + */
> > +extern u64 ocxl_link_get_lpc_mem_sz(void *link_handle);
> 
> I don't see that one defined anywhere.
> 

Whoops, I'll remove it.

>    Fred
> 
> 
> > +
> > +/**
> > + * Map the LPC memory for an OpenCAPI device
> > + *
> > + * Since LPC memory belongs to a link, the whole LPC memory
> > available
> > + * on the link bust be mapped in order to make it accessible to a
> > device.
> > + *
> > + * @link_handle: The OpenCAPI link handle
> > + * @pdev: A device that is on the link
> > + */
> > +u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev);
> > +
> > +/**
> > + * Release the LPC memory device for an OpenCAPI device
> > + *
> > + * Releases LPC memory on an OpenCAPI link for a device. If this
> > is the
> > + * last device on the link to release the memory, unmap it from
> > the link.
> > + *
> > + * @link_handle: The OpenCAPI link handle
> > + * @pdev: A device that is on the link
> > + */
> > +void ocxl_link_lpc_release(void *link_handle, struct pci_dev
> > *pdev);
> > +
> >   #endif /* _OCXL_INTERNAL_H_ */
> > 
-- 
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819

Re: [PATCH 3/5] ocxl: Tally up the LPC memory on a link & allow it to be mapped

[Frederic Barrat]

Le 19/09/2019 à 06:55, Alastair D'Silva a écrit :
> On Wed, 2019-09-18 at 16:02 +0200, Frederic Barrat wrote:
>>
>> Le 17/09/2019 à 03:42, Alastair D'Silva a écrit :
>>> From: Alastair D'Silva <alastair@d-silva.org>
>>>
>>> Tally up the LPC memory on an OpenCAPI link & allow it to be mapped
>>>
>>> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
>>> ---
>>>    drivers/misc/ocxl/core.c          |  9 +++++
>>>    drivers/misc/ocxl/link.c          | 61
>>> +++++++++++++++++++++++++++++++
>>>    drivers/misc/ocxl/ocxl_internal.h | 42 +++++++++++++++++++++
>>>    3 files changed, 112 insertions(+)
>>>
>>> diff --git a/drivers/misc/ocxl/core.c b/drivers/misc/ocxl/core.c
>>> index b7a09b21ab36..fdfe4e0a34e1 100644
>>> --- a/drivers/misc/ocxl/core.c
>>> +++ b/drivers/misc/ocxl/core.c
>>> @@ -230,8 +230,17 @@ static int configure_afu(struct ocxl_afu *afu,
>>> u8 afu_idx, struct pci_dev *dev)
>>>    	if (rc)
>>>    		goto err_free_pasid;
>>>    
>>> +	if (afu->config.lpc_mem_size || afu-
>>>> config.special_purpose_mem_size) {
>>> +		rc = ocxl_link_add_lpc_mem(afu->fn->link,
>>> +			afu->config.lpc_mem_size + afu-
>>>> config.special_purpose_mem_size);
>>
>> I don't think we should count the special purpose memory, as it's
>> not
>> meant to be accessed through the GPU mem BAR, but I'll check.
> 
> At least for OpenCAPI 3.0, there is no other in-spec way to access the
> memory if it is not mapped by the NPU.


Yes, that's clarified now and we should take the special purpose memory 
into account when defining the full range.

   Fred


>>
>> What happens when unconfiguring the AFU? We should reduce the range
>> (see
>> also below). Partial reconfig doesn't seem so far off, so we should
>> take
>> it into account.
>>
> 
> The mapping is left until the last AFU on the link offlines it's
> memory, at which point we clear the mapping from the NPU.
> 
>>
>>> +		if (rc)
>>> +			goto err_free_mmio;
>>> +	}
>>> +
>>>    	return 0;
>>>    
>>> +err_free_mmio:
>>> +	unmap_mmio_areas(afu);
>>>    err_free_pasid:
>>>    	reclaim_afu_pasid(afu);
>>>    err_free_actag:
>>> diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c
>>> index 58d111afd9f6..2874811a4398 100644
>>> --- a/drivers/misc/ocxl/link.c
>>> +++ b/drivers/misc/ocxl/link.c
>>> @@ -84,6 +84,11 @@ struct ocxl_link {
>>>    	int dev;
>>>    	atomic_t irq_available;
>>>    	struct spa *spa;
>>> +	struct mutex lpc_mem_lock;
>>> +	u64 lpc_mem_sz; /* Total amount of LPC memory presented on the
>>> link */
>>> +	u64 lpc_mem;
>>> +	int lpc_consumers;
>>> +
>>>    	void *platform_data;
>>>    };
>>>    static struct list_head links_list = LIST_HEAD_INIT(links_list);
>>> @@ -396,6 +401,8 @@ static int alloc_link(struct pci_dev *dev, int
>>> PE_mask, struct ocxl_link **out_l
>>>    	if (rc)
>>>    		goto err_spa;
>>>    
>>> +	mutex_init(&link->lpc_mem_lock);
>>> +
>>>    	/* platform specific hook */
>>>    	rc = pnv_ocxl_spa_setup(dev, link->spa->spa_mem, PE_mask,
>>>    				&link->platform_data);
>>> @@ -711,3 +718,57 @@ void ocxl_link_free_irq(void *link_handle, int
>>> hw_irq)
>>>    	atomic_inc(&link->irq_available);
>>>    }
>>>    EXPORT_SYMBOL_GPL(ocxl_link_free_irq);
>>> +
>>> +int ocxl_link_add_lpc_mem(void *link_handle, u64 size)
>>> +{
>>> +	struct ocxl_link *link = (struct ocxl_link *) link_handle;
>>> +
>>> +	u64 orig_size;
>>> +	bool good = false;
>>> +
>>> +	mutex_lock(&link->lpc_mem_lock);
>>> +	orig_size = link->lpc_mem_sz;
>>> +	link->lpc_mem_sz += size;
>>
>>
>> We have a choice to make here:
>> 1. either we only support one LPC memory-carrying AFU (and the above
>> is
>> overkill)
>> 2. or we support multiple AFUs with LPC memory (on the same
>> function),
>> but then I think the above is too simple.
>>
>>   From the opencapi spec, each AFU can define a chunk of memory with
>> a
>> starting address and a size. There's no rule which says they have to
>> be
>> contiguous. There's no rule which says it must start at 0. So to
>> support
>> multiple AFUs with LPC memory, we should record the current maximum
>> range instead of just the global size. Ultimately, we need to tell
>> the
>> NPU the range of permissible addresses. It starts at 0, so we need
>> to
>> take into account any intial offset and holes.
>>
>> I would go for option 2, to at least be consistent within ocxl and
>> support multiple AFUs. Even though I don't think we'll see FPGA
>> images
>> with multiple AFUs with LPC memory any time soon.
>>
> 
> Ill rework this to take an offset & size, the NPU will map from the
> base address up to the largest offset + size provided across all AFUs
> on the link.
> 
>>
>>> +	good = orig_size < link->lpc_mem_sz;
>>> +	mutex_unlock(&link->lpc_mem_lock);
>>> +
>>> +	// Check for overflow
>>> +	return (good) ? 0 : -EINVAL;
>>> +}
>>> +EXPORT_SYMBOL_GPL(ocxl_link_add_lpc_mem);
>>
>> Do the symbol really need to be exported? IIUC, the next patch
>> defines a
>> higher level ocxl_afu_map_lpc_mem() which is meant to be called by a
>> calling driver.
>>
> 
> No, I'll remove it.
> 
>>
>>> +
>>> +u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev)
>>> +{
>>> +	struct ocxl_link *link = (struct ocxl_link *) link_handle;
>>> +
>>> +	mutex_lock(&link->lpc_mem_lock);
>>> +	if (link->lpc_mem) {
>>> +		u64 lpc_mem = link->lpc_mem;
>>> +
>>> +		link->lpc_consumers++;
>>> +		mutex_unlock(&link->lpc_mem_lock);
>>> +		return lpc_mem;
>>> +	}
>>> +
>>> +	link->lpc_mem = pnv_ocxl_platform_lpc_setup(pdev, link-
>>>> lpc_mem_sz);
>>> +	if (link->lpc_mem)
>>> +		link->lpc_consumers++;
>>> +	mutex_unlock(&link->lpc_mem_lock);
>>> +
>>> +	return link->lpc_mem;
>>
>> Should be cached in a temp variable, like on the fast path,
>> otherwise
>> it's accessed with no lock.
> 
> Good spotting, thanks.
> 
>>
>>> +}
>>> +
>>> +void ocxl_link_lpc_release(void *link_handle, struct pci_dev
>>> *pdev)
>>> +{
>>> +	struct ocxl_link *link = (struct ocxl_link *) link_handle;
>>> +
>>> +	mutex_lock(&link->lpc_mem_lock);
>>> +	link->lpc_consumers--;
>>> +	if (link->lpc_consumers == 0) {
>>> +		pnv_ocxl_platform_lpc_release(pdev);
>>> +		link->lpc_mem = 0;
>>> +	}
>>> +
>>> +	mutex_unlock(&link->lpc_mem_lock);
>>> +}
>>> diff --git a/drivers/misc/ocxl/ocxl_internal.h
>>> b/drivers/misc/ocxl/ocxl_internal.h
>>> index 97415afd79f3..db2647a90fc8 100644
>>> --- a/drivers/misc/ocxl/ocxl_internal.h
>>> +++ b/drivers/misc/ocxl/ocxl_internal.h
>>> @@ -141,4 +141,46 @@ int ocxl_irq_offset_to_id(struct ocxl_context
>>> *ctx, u64 offset);
>>>    u64 ocxl_irq_id_to_offset(struct ocxl_context *ctx, int irq_id);
>>>    void ocxl_afu_irq_free_all(struct ocxl_context *ctx);
>>>    
>>> +/**
>>> + * Increment the amount of memory required by an OpenCAPI link
>>> + *
>>> + * link_handle: The OpenCAPI link handle
>>> + * size: The amount of memory to increment by
>>> + *
>>> + * Return 0 on success, negative on overflow
>>> + */
>>> +extern int ocxl_link_add_lpc_mem(void *link_handle, u64 size);
>>
>> We've removed all the 'extern' in a previous patch.
> 
> Thanks, I spotted this too (after I posted it).
> 
>>> +
>>> +/**
>>> + * Get the amount of memory required by an OpenCAPI link
>>> + *
>>> + * link_handle: The OpenCAPI link handle
>>> + *
>>> + * Return the amount of memory required by the link, this value is
>>> undefined if
>>> + * ocxl_link_add_lpc_mem failed.
>>> + */
>>> +extern u64 ocxl_link_get_lpc_mem_sz(void *link_handle);
>>
>> I don't see that one defined anywhere.
>>
> 
> Whoops, I'll remove it.
> 
>>     Fred
>>
>>
>>> +
>>> +/**
>>> + * Map the LPC memory for an OpenCAPI device
>>> + *
>>> + * Since LPC memory belongs to a link, the whole LPC memory
>>> available
>>> + * on the link bust be mapped in order to make it accessible to a
>>> device.
>>> + *
>>> + * @link_handle: The OpenCAPI link handle
>>> + * @pdev: A device that is on the link
>>> + */
>>> +u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev);
>>> +
>>> +/**
>>> + * Release the LPC memory device for an OpenCAPI device
>>> + *
>>> + * Releases LPC memory on an OpenCAPI link for a device. If this
>>> is the
>>> + * last device on the link to release the memory, unmap it from
>>> the link.
>>> + *
>>> + * @link_handle: The OpenCAPI link handle
>>> + * @pdev: A device that is on the link
>>> + */
>>> +void ocxl_link_lpc_release(void *link_handle, struct pci_dev
>>> *pdev);
>>> +
>>>    #endif /* _OCXL_INTERNAL_H_ */
>>>

[PATCH 4/5] ocxl: Add functions to map/unmap LPC memory

[Alastair D'Silva]

From: Alastair D'Silva <alastair@d-silva.org>

Add functions to map/unmap LPC memory

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 drivers/misc/ocxl/config.c        |  4 +++
 drivers/misc/ocxl/core.c          | 50 +++++++++++++++++++++++++++++++
 drivers/misc/ocxl/link.c          |  4 +--
 drivers/misc/ocxl/ocxl_internal.h | 10 +++++--
 include/misc/ocxl.h               | 18 +++++++++++
 5 files changed, 82 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
index c8e19bfb5ef9..fb0c3b6f8312 100644
--- a/drivers/misc/ocxl/config.c
+++ b/drivers/misc/ocxl/config.c
@@ -568,6 +568,10 @@ static int read_afu_lpc_memory_info(struct pci_dev *dev,
 		afu->special_purpose_mem_size =
 			total_mem_size - lpc_mem_size;
 	}
+
+	dev_info(&dev->dev, "Probed LPC memory of %#llx bytes and special purpose memory of %#llx bytes\n",
+		afu->lpc_mem_size, afu->special_purpose_mem_size);
+
 	return 0;
 }
 
diff --git a/drivers/misc/ocxl/core.c b/drivers/misc/ocxl/core.c
index fdfe4e0a34e1..eb24bb9d655f 100644
--- a/drivers/misc/ocxl/core.c
+++ b/drivers/misc/ocxl/core.c
@@ -210,6 +210,55 @@ static void unmap_mmio_areas(struct ocxl_afu *afu)
 	release_fn_bar(afu->fn, afu->config.global_mmio_bar);
 }
 
+int ocxl_map_lpc_mem(struct ocxl_afu *afu)
+{
+	struct pci_dev *dev = to_pci_dev(afu->fn->dev.parent);
+
+	if ((afu->config.lpc_mem_size + afu->config.special_purpose_mem_size) == 0)
+		return 0;
+
+	afu->lpc_base_addr = ocxl_link_lpc_online(afu->fn->link, dev);
+	if (afu->lpc_base_addr == 0)
+		return -EINVAL;
+
+	if (afu->config.lpc_mem_size) {
+		afu->lpc_res.start = afu->lpc_base_addr + afu->config.lpc_mem_offset;
+		afu->lpc_res.end = afu->lpc_res.start + afu->config.lpc_mem_size - 1;
+	}
+
+	if (afu->config.special_purpose_mem_size) {
+		afu->special_purpose_res.start = afu->lpc_base_addr +
+						 afu->config.special_purpose_mem_offset;
+		afu->special_purpose_res.end = afu->special_purpose_res.start +
+					       afu->config.special_purpose_mem_size - 1;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(ocxl_map_lpc_mem);
+
+struct resource *ocxl_afu_lpc_mem(struct ocxl_afu *afu)
+{
+	return &afu->lpc_res;
+}
+EXPORT_SYMBOL(ocxl_afu_lpc_mem);
+
+static void unmap_lpc_mem(struct ocxl_afu *afu)
+{
+	struct pci_dev *dev = to_pci_dev(afu->fn->dev.parent);
+
+	if (afu->lpc_res.start || afu->special_purpose_res.start) {
+		void *link = afu->fn->link;
+
+		ocxl_link_lpc_offline(link, dev);
+
+		afu->lpc_res.start = 0;
+		afu->lpc_res.end = 0;
+		afu->special_purpose_res.start = 0;
+		afu->special_purpose_res.end = 0;
+	}
+}
+
 static int configure_afu(struct ocxl_afu *afu, u8 afu_idx, struct pci_dev *dev)
 {
 	int rc;
@@ -250,6 +299,7 @@ static int configure_afu(struct ocxl_afu *afu, u8 afu_idx, struct pci_dev *dev)
 
 static void deconfigure_afu(struct ocxl_afu *afu)
 {
+	unmap_lpc_mem(afu);
 	unmap_mmio_areas(afu);
 	reclaim_afu_pasid(afu);
 	reclaim_afu_actag(afu);
diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c
index 2874811a4398..9e303a5f4d85 100644
--- a/drivers/misc/ocxl/link.c
+++ b/drivers/misc/ocxl/link.c
@@ -738,7 +738,7 @@ int ocxl_link_add_lpc_mem(void *link_handle, u64 size)
 }
 EXPORT_SYMBOL_GPL(ocxl_link_add_lpc_mem);
 
-u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev)
+u64 ocxl_link_lpc_online(void *link_handle, struct pci_dev *pdev)
 {
 	struct ocxl_link *link = (struct ocxl_link *) link_handle;
 
@@ -759,7 +759,7 @@ u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev)
 	return link->lpc_mem;
 }
 
-void ocxl_link_lpc_release(void *link_handle, struct pci_dev *pdev)
+void ocxl_link_lpc_offline(void *link_handle, struct pci_dev *pdev)
 {
 	struct ocxl_link *link = (struct ocxl_link *) link_handle;
 
diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
index db2647a90fc8..5656a4aab5b7 100644
--- a/drivers/misc/ocxl/ocxl_internal.h
+++ b/drivers/misc/ocxl/ocxl_internal.h
@@ -52,6 +52,12 @@ struct ocxl_afu {
 	void __iomem *global_mmio_ptr;
 	u64 pp_mmio_start;
 	void *private;
+	u64 lpc_base_addr; /* Covers both LPC & special purpose memory */
+	struct bin_attribute attr_global_mmio;
+	struct bin_attribute attr_lpc_mem;
+	struct resource lpc_res;
+	struct bin_attribute attr_special_purpose_mem;
+	struct resource special_purpose_res;
 };
 
 enum ocxl_context_status {
@@ -170,7 +176,7 @@ extern u64 ocxl_link_get_lpc_mem_sz(void *link_handle);
  * @link_handle: The OpenCAPI link handle
  * @pdev: A device that is on the link
  */
-u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev);
+u64 ocxl_link_lpc_online(void *link_handle, struct pci_dev *pdev);
 
 /**
  * Release the LPC memory device for an OpenCAPI device
@@ -181,6 +187,6 @@ u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev);
  * @link_handle: The OpenCAPI link handle
  * @pdev: A device that is on the link
  */
-void ocxl_link_lpc_release(void *link_handle, struct pci_dev *pdev);
+void ocxl_link_lpc_offline(void *link_handle, struct pci_dev *pdev);
 
 #endif /* _OCXL_INTERNAL_H_ */
diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
index 06dd5839e438..a1897737908d 100644
--- a/include/misc/ocxl.h
+++ b/include/misc/ocxl.h
@@ -212,6 +212,24 @@ int ocxl_irq_set_handler(struct ocxl_context *ctx, int irq_id,
 
 // AFU Metadata
 
+/**
+ * Map the LPC system & special purpose memory for an AFU
+ *
+ * Do not call this during device discovery, as there may me multiple
+ * devices on a link, and the memory is mapped for the whole link, not
+ * just one device. It should only be called after all devices have
+ * registered their memory on the link.
+ *
+ * afu: The AFU that has the LPC memory to map
+ */
+extern int ocxl_map_lpc_mem(struct ocxl_afu *afu);
+
+/**
+ * Get the physical address range of LPC memory for an AFU
+ * afu: The AFU associated with the LPC memory
+ */
+extern struct resource *ocxl_afu_lpc_mem(struct ocxl_afu *afu);
+
 /**
  * Get a pointer to the config for an AFU
  *
-- 
2.21.0

Re: [PATCH 4/5] ocxl: Add functions to map/unmap LPC memory

[Christoph Hellwig]

Please submit this together with actual users instead of adding
dead code to the kernel.  Same for any of the previous bits
that arent used with our without this.

Re: [PATCH 4/5] ocxl: Add functions to map/unmap LPC memory

[Frederic Barrat]

Le 17/09/2019 à 03:43, Alastair D'Silva a écrit :
> From: Alastair D'Silva <alastair@d-silva.org>
> 
> Add functions to map/unmap LPC memory
> 
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
>   drivers/misc/ocxl/config.c        |  4 +++
>   drivers/misc/ocxl/core.c          | 50 +++++++++++++++++++++++++++++++
>   drivers/misc/ocxl/link.c          |  4 +--
>   drivers/misc/ocxl/ocxl_internal.h | 10 +++++--
>   include/misc/ocxl.h               | 18 +++++++++++
>   5 files changed, 82 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
> index c8e19bfb5ef9..fb0c3b6f8312 100644
> --- a/drivers/misc/ocxl/config.c
> +++ b/drivers/misc/ocxl/config.c
> @@ -568,6 +568,10 @@ static int read_afu_lpc_memory_info(struct pci_dev *dev,
>   		afu->special_purpose_mem_size =
>   			total_mem_size - lpc_mem_size;
>   	}
> +
> +	dev_info(&dev->dev, "Probed LPC memory of %#llx bytes and special purpose memory of %#llx bytes\n",
> +		afu->lpc_mem_size, afu->special_purpose_mem_size);
> +
>   	return 0;
>   }
>   
> diff --git a/drivers/misc/ocxl/core.c b/drivers/misc/ocxl/core.c
> index fdfe4e0a34e1..eb24bb9d655f 100644
> --- a/drivers/misc/ocxl/core.c
> +++ b/drivers/misc/ocxl/core.c
> @@ -210,6 +210,55 @@ static void unmap_mmio_areas(struct ocxl_afu *afu)
>   	release_fn_bar(afu->fn, afu->config.global_mmio_bar);
>   }
>   
> +int ocxl_map_lpc_mem(struct ocxl_afu *afu)
> +{
> +	struct pci_dev *dev = to_pci_dev(afu->fn->dev.parent);
> +
> +	if ((afu->config.lpc_mem_size + afu->config.special_purpose_mem_size) == 0)
> +		return 0;
> +
> +	afu->lpc_base_addr = ocxl_link_lpc_online(afu->fn->link, dev);
> +	if (afu->lpc_base_addr == 0)
> +		return -EINVAL;
> +
> +	if (afu->config.lpc_mem_size) {
> +		afu->lpc_res.start = afu->lpc_base_addr + afu->config.lpc_mem_offset;
> +		afu->lpc_res.end = afu->lpc_res.start + afu->config.lpc_mem_size - 1;
> +	}
> +
> +	if (afu->config.special_purpose_mem_size) {
> +		afu->special_purpose_res.start = afu->lpc_base_addr +
> +						 afu->config.special_purpose_mem_offset;
> +		afu->special_purpose_res.end = afu->special_purpose_res.start +
> +					       afu->config.special_purpose_mem_size - 1;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(ocxl_map_lpc_mem);
> +
> +struct resource *ocxl_afu_lpc_mem(struct ocxl_afu *afu)
> +{
> +	return &afu->lpc_res;
> +}
> +EXPORT_SYMBOL(ocxl_afu_lpc_mem);
> +
> +static void unmap_lpc_mem(struct ocxl_afu *afu)
> +{
> +	struct pci_dev *dev = to_pci_dev(afu->fn->dev.parent);
> +
> +	if (afu->lpc_res.start || afu->special_purpose_res.start) {
> +		void *link = afu->fn->link;
> +
> +		ocxl_link_lpc_offline(link, dev);
> +
> +		afu->lpc_res.start = 0;
> +		afu->lpc_res.end = 0;
> +		afu->special_purpose_res.start = 0;
> +		afu->special_purpose_res.end = 0;
> +	}
> +}
> +
>   static int configure_afu(struct ocxl_afu *afu, u8 afu_idx, struct pci_dev *dev)
>   {
>   	int rc;
> @@ -250,6 +299,7 @@ static int configure_afu(struct ocxl_afu *afu, u8 afu_idx, struct pci_dev *dev)
>   
>   static void deconfigure_afu(struct ocxl_afu *afu)
>   {
> +	unmap_lpc_mem(afu);
>   	unmap_mmio_areas(afu);
>   	reclaim_afu_pasid(afu);
>   	reclaim_afu_actag(afu);
> diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c
> index 2874811a4398..9e303a5f4d85 100644
> --- a/drivers/misc/ocxl/link.c
> +++ b/drivers/misc/ocxl/link.c
> @@ -738,7 +738,7 @@ int ocxl_link_add_lpc_mem(void *link_handle, u64 size)
>   }
>   EXPORT_SYMBOL_GPL(ocxl_link_add_lpc_mem);
>   
> -u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev)
> +u64 ocxl_link_lpc_online(void *link_handle, struct pci_dev *pdev)
>   {
>   	struct ocxl_link *link = (struct ocxl_link *) link_handle;
>   
> @@ -759,7 +759,7 @@ u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev)
>   	return link->lpc_mem;
>   }
>   
> -void ocxl_link_lpc_release(void *link_handle, struct pci_dev *pdev)
> +void ocxl_link_lpc_offline(void *link_handle, struct pci_dev *pdev)


Could we avoid the renaming by squashing it with the previous patch?


>   {
>   	struct ocxl_link *link = (struct ocxl_link *) link_handle;
>   
> diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
> index db2647a90fc8..5656a4aab5b7 100644
> --- a/drivers/misc/ocxl/ocxl_internal.h
> +++ b/drivers/misc/ocxl/ocxl_internal.h
> @@ -52,6 +52,12 @@ struct ocxl_afu {
>   	void __iomem *global_mmio_ptr;
>   	u64 pp_mmio_start;
>   	void *private;
> +	u64 lpc_base_addr; /* Covers both LPC & special purpose memory */
> +	struct bin_attribute attr_global_mmio;
> +	struct bin_attribute attr_lpc_mem;
> +	struct resource lpc_res;
> +	struct bin_attribute attr_special_purpose_mem;
> +	struct resource special_purpose_res;
>   };
>   
>   enum ocxl_context_status {
> @@ -170,7 +176,7 @@ extern u64 ocxl_link_get_lpc_mem_sz(void *link_handle);
>    * @link_handle: The OpenCAPI link handle
>    * @pdev: A device that is on the link
>    */
> -u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev);
> +u64 ocxl_link_lpc_online(void *link_handle, struct pci_dev *pdev);
>   
>   /**
>    * Release the LPC memory device for an OpenCAPI device
> @@ -181,6 +187,6 @@ u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev);
>    * @link_handle: The OpenCAPI link handle
>    * @pdev: A device that is on the link
>    */
> -void ocxl_link_lpc_release(void *link_handle, struct pci_dev *pdev);
> +void ocxl_link_lpc_offline(void *link_handle, struct pci_dev *pdev);
>   
>   #endif /* _OCXL_INTERNAL_H_ */
> diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
> index 06dd5839e438..a1897737908d 100644
> --- a/include/misc/ocxl.h
> +++ b/include/misc/ocxl.h
> @@ -212,6 +212,24 @@ int ocxl_irq_set_handler(struct ocxl_context *ctx, int irq_id,
>   
>   // AFU Metadata
>   
> +/**
> + * Map the LPC system & special purpose memory for an AFU
> + *
> + * Do not call this during device discovery, as there may me multiple
> + * devices on a link, and the memory is mapped for the whole link, not
> + * just one device. It should only be called after all devices have
> + * registered their memory on the link.
> + *
> + * afu: The AFU that has the LPC memory to map
> + */
> +extern int ocxl_map_lpc_mem(struct ocxl_afu *afu);
> +
> +/**
> + * Get the physical address range of LPC memory for an AFU
> + * afu: The AFU associated with the LPC memory
> + */
> +extern struct resource *ocxl_afu_lpc_mem(struct ocxl_afu *afu);
> +
>   /**
>    * Get a pointer to the config for an AFU
>    *
> 

Re: [PATCH 4/5] ocxl: Add functions to map/unmap LPC memory

[Alastair D'Silva]

On Wed, 2019-09-18 at 16:03 +0200, Frederic Barrat wrote:
> 
> Le 17/09/2019 à 03:43, Alastair D'Silva a écrit :
> > From: Alastair D'Silva <alastair@d-silva.org>
> > 
> > Add functions to map/unmap LPC memory
> > 
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > ---
> >   drivers/misc/ocxl/config.c        |  4 +++
> >   drivers/misc/ocxl/core.c          | 50
> > +++++++++++++++++++++++++++++++
> >   drivers/misc/ocxl/link.c          |  4 +--
> >   drivers/misc/ocxl/ocxl_internal.h | 10 +++++--
> >   include/misc/ocxl.h               | 18 +++++++++++
> >   5 files changed, 82 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/misc/ocxl/config.c
> > b/drivers/misc/ocxl/config.c
> > index c8e19bfb5ef9..fb0c3b6f8312 100644
> > --- a/drivers/misc/ocxl/config.c
> > +++ b/drivers/misc/ocxl/config.c
> > @@ -568,6 +568,10 @@ static int read_afu_lpc_memory_info(struct
> > pci_dev *dev,
> >   		afu->special_purpose_mem_size =
> >   			total_mem_size - lpc_mem_size;
> >   	}
> > +
> > +	dev_info(&dev->dev, "Probed LPC memory of %#llx bytes and
> > special purpose memory of %#llx bytes\n",
> > +		afu->lpc_mem_size, afu->special_purpose_mem_size);
> > +
> >   	return 0;
> >   }
> >   
> > diff --git a/drivers/misc/ocxl/core.c b/drivers/misc/ocxl/core.c
> > index fdfe4e0a34e1..eb24bb9d655f 100644
> > --- a/drivers/misc/ocxl/core.c
> > +++ b/drivers/misc/ocxl/core.c
> > @@ -210,6 +210,55 @@ static void unmap_mmio_areas(struct ocxl_afu
> > *afu)
> >   	release_fn_bar(afu->fn, afu->config.global_mmio_bar);
> >   }
> >   
> > +int ocxl_map_lpc_mem(struct ocxl_afu *afu)
> > +{
> > +	struct pci_dev *dev = to_pci_dev(afu->fn->dev.parent);
> > +
> > +	if ((afu->config.lpc_mem_size + afu-
> > >config.special_purpose_mem_size) == 0)
> > +		return 0;
> > +
> > +	afu->lpc_base_addr = ocxl_link_lpc_online(afu->fn->link, dev);
> > +	if (afu->lpc_base_addr == 0)
> > +		return -EINVAL;
> > +
> > +	if (afu->config.lpc_mem_size) {
> > +		afu->lpc_res.start = afu->lpc_base_addr + afu-
> > >config.lpc_mem_offset;
> > +		afu->lpc_res.end = afu->lpc_res.start + afu-
> > >config.lpc_mem_size - 1;
> > +	}
> > +
> > +	if (afu->config.special_purpose_mem_size) {
> > +		afu->special_purpose_res.start = afu->lpc_base_addr +
> > +						 afu-
> > >config.special_purpose_mem_offset;
> > +		afu->special_purpose_res.end = afu-
> > >special_purpose_res.start +
> > +					       afu-
> > >config.special_purpose_mem_size - 1;
> > +	}
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(ocxl_map_lpc_mem);
> > +
> > +struct resource *ocxl_afu_lpc_mem(struct ocxl_afu *afu)
> > +{
> > +	return &afu->lpc_res;
> > +}
> > +EXPORT_SYMBOL(ocxl_afu_lpc_mem);
> > +
> > +static void unmap_lpc_mem(struct ocxl_afu *afu)
> > +{
> > +	struct pci_dev *dev = to_pci_dev(afu->fn->dev.parent);
> > +
> > +	if (afu->lpc_res.start || afu->special_purpose_res.start) {
> > +		void *link = afu->fn->link;
> > +
> > +		ocxl_link_lpc_offline(link, dev);
> > +
> > +		afu->lpc_res.start = 0;
> > +		afu->lpc_res.end = 0;
> > +		afu->special_purpose_res.start = 0;
> > +		afu->special_purpose_res.end = 0;
> > +	}
> > +}
> > +
> >   static int configure_afu(struct ocxl_afu *afu, u8 afu_idx, struct
> > pci_dev *dev)
> >   {
> >   	int rc;
> > @@ -250,6 +299,7 @@ static int configure_afu(struct ocxl_afu *afu,
> > u8 afu_idx, struct pci_dev *dev)
> >   
> >   static void deconfigure_afu(struct ocxl_afu *afu)
> >   {
> > +	unmap_lpc_mem(afu);
> >   	unmap_mmio_areas(afu);
> >   	reclaim_afu_pasid(afu);
> >   	reclaim_afu_actag(afu);
> > diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c
> > index 2874811a4398..9e303a5f4d85 100644
> > --- a/drivers/misc/ocxl/link.c
> > +++ b/drivers/misc/ocxl/link.c
> > @@ -738,7 +738,7 @@ int ocxl_link_add_lpc_mem(void *link_handle,
> > u64 size)
> >   }
> >   EXPORT_SYMBOL_GPL(ocxl_link_add_lpc_mem);
> >   
> > -u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev)
> > +u64 ocxl_link_lpc_online(void *link_handle, struct pci_dev *pdev)
> >   {
> >   	struct ocxl_link *link = (struct ocxl_link *) link_handle;
> >   
> > @@ -759,7 +759,7 @@ u64 ocxl_link_lpc_map(void *link_handle, struct
> > pci_dev *pdev)
> >   	return link->lpc_mem;
> >   }
> >   
> > -void ocxl_link_lpc_release(void *link_handle, struct pci_dev
> > *pdev)
> > +void ocxl_link_lpc_offline(void *link_handle, struct pci_dev
> > *pdev)
> 
> Could we avoid the renaming by squashing it with the previous patch?
> 

Yup, good catch.

> 
> >   {
> >   	struct ocxl_link *link = (struct ocxl_link *) link_handle;
> >   
> > diff --git a/drivers/misc/ocxl/ocxl_internal.h
> > b/drivers/misc/ocxl/ocxl_internal.h
> > index db2647a90fc8..5656a4aab5b7 100644
> > --- a/drivers/misc/ocxl/ocxl_internal.h
> > +++ b/drivers/misc/ocxl/ocxl_internal.h
> > @@ -52,6 +52,12 @@ struct ocxl_afu {
> >   	void __iomem *global_mmio_ptr;
> >   	u64 pp_mmio_start;
> >   	void *private;
> > +	u64 lpc_base_addr; /* Covers both LPC & special purpose memory
> > */
> > +	struct bin_attribute attr_global_mmio;
> > +	struct bin_attribute attr_lpc_mem;
> > +	struct resource lpc_res;
> > +	struct bin_attribute attr_special_purpose_mem;
> > +	struct resource special_purpose_res;
> >   };
> >   
> >   enum ocxl_context_status {
> > @@ -170,7 +176,7 @@ extern u64 ocxl_link_get_lpc_mem_sz(void
> > *link_handle);
> >    * @link_handle: The OpenCAPI link handle
> >    * @pdev: A device that is on the link
> >    */
> > -u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev);
> > +u64 ocxl_link_lpc_online(void *link_handle, struct pci_dev *pdev);
> >   
> >   /**
> >    * Release the LPC memory device for an OpenCAPI device
> > @@ -181,6 +187,6 @@ u64 ocxl_link_lpc_map(void *link_handle, struct
> > pci_dev *pdev);
> >    * @link_handle: The OpenCAPI link handle
> >    * @pdev: A device that is on the link
> >    */
> > -void ocxl_link_lpc_release(void *link_handle, struct pci_dev
> > *pdev);
> > +void ocxl_link_lpc_offline(void *link_handle, struct pci_dev
> > *pdev);
> >   
> >   #endif /* _OCXL_INTERNAL_H_ */
> > diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
> > index 06dd5839e438..a1897737908d 100644
> > --- a/include/misc/ocxl.h
> > +++ b/include/misc/ocxl.h
> > @@ -212,6 +212,24 @@ int ocxl_irq_set_handler(struct ocxl_context
> > *ctx, int irq_id,
> >   
> >   // AFU Metadata
> >   
> > +/**
> > + * Map the LPC system & special purpose memory for an AFU
> > + *
> > + * Do not call this during device discovery, as there may me
> > multiple
> > + * devices on a link, and the memory is mapped for the whole link,
> > not
> > + * just one device. It should only be called after all devices
> > have
> > + * registered their memory on the link.
> > + *
> > + * afu: The AFU that has the LPC memory to map
> > + */
> > +extern int ocxl_map_lpc_mem(struct ocxl_afu *afu);
> > +
> > +/**
> > + * Get the physical address range of LPC memory for an AFU
> > + * afu: The AFU associated with the LPC memory
> > + */
> > +extern struct resource *ocxl_afu_lpc_mem(struct ocxl_afu *afu);
> > +
> >   /**
> >    * Get a pointer to the config for an AFU
> >    *
> > 
-- 
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819

Re: [PATCH 4/5] ocxl: Add functions to map/unmap LPC memory

[Alastair D'Silva]

On Tue, 2019-09-17 at 11:43 +1000, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
> 
> Add functions to map/unmap LPC memory
> 
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
>  drivers/misc/ocxl/config.c        |  4 +++
>  drivers/misc/ocxl/core.c          | 50
> +++++++++++++++++++++++++++++++
>  drivers/misc/ocxl/link.c          |  4 +--
>  drivers/misc/ocxl/ocxl_internal.h | 10 +++++--
>  include/misc/ocxl.h               | 18 +++++++++++
>  5 files changed, 82 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
> index c8e19bfb5ef9..fb0c3b6f8312 100644
> --- a/drivers/misc/ocxl/config.c
> +++ b/drivers/misc/ocxl/config.c
> @@ -568,6 +568,10 @@ static int read_afu_lpc_memory_info(struct
> pci_dev *dev,
>  		afu->special_purpose_mem_size =
>  			total_mem_size - lpc_mem_size;
>  	}
> +
> +	dev_info(&dev->dev, "Probed LPC memory of %#llx bytes and
> special purpose memory of %#llx bytes\n",
> +		afu->lpc_mem_size, afu->special_purpose_mem_size);
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/misc/ocxl/core.c b/drivers/misc/ocxl/core.c
> index fdfe4e0a34e1..eb24bb9d655f 100644
> --- a/drivers/misc/ocxl/core.c
> +++ b/drivers/misc/ocxl/core.c
> @@ -210,6 +210,55 @@ static void unmap_mmio_areas(struct ocxl_afu
> *afu)
>  	release_fn_bar(afu->fn, afu->config.global_mmio_bar);
>  }
>  
> +int ocxl_map_lpc_mem(struct ocxl_afu *afu)
> +{
> +	struct pci_dev *dev = to_pci_dev(afu->fn->dev.parent);
> +
> +	if ((afu->config.lpc_mem_size + afu-
> >config.special_purpose_mem_size) == 0)
> +		return 0;
> +
> +	afu->lpc_base_addr = ocxl_link_lpc_online(afu->fn->link, dev);
> +	if (afu->lpc_base_addr == 0)
> +		return -EINVAL;
> +
> +	if (afu->config.lpc_mem_size) {
> +		afu->lpc_res.start = afu->lpc_base_addr + afu-
> >config.lpc_mem_offset;
> +		afu->lpc_res.end = afu->lpc_res.start + afu-
> >config.lpc_mem_size - 1;
> +	}
> +
> +	if (afu->config.special_purpose_mem_size) {
> +		afu->special_purpose_res.start = afu->lpc_base_addr +
> +						 afu-
> >config.special_purpose_mem_offset;
> +		afu->special_purpose_res.end = afu-
> >special_purpose_res.start +
> +					       afu-
> >config.special_purpose_mem_size - 1;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(ocxl_map_lpc_mem);
> +
> +struct resource *ocxl_afu_lpc_mem(struct ocxl_afu *afu)
> +{
> +	return &afu->lpc_res;
> +}
> +EXPORT_SYMBOL(ocxl_afu_lpc_mem);
> +
> +static void unmap_lpc_mem(struct ocxl_afu *afu)
> +{
> +	struct pci_dev *dev = to_pci_dev(afu->fn->dev.parent);
> +
> +	if (afu->lpc_res.start || afu->special_purpose_res.start) {
> +		void *link = afu->fn->link;
> +
> +		ocxl_link_lpc_offline(link, dev);
> +
> +		afu->lpc_res.start = 0;
> +		afu->lpc_res.end = 0;
> +		afu->special_purpose_res.start = 0;
> +		afu->special_purpose_res.end = 0;
> +	}
> +}
> +
>  static int configure_afu(struct ocxl_afu *afu, u8 afu_idx, struct
> pci_dev *dev)
>  {
>  	int rc;
> @@ -250,6 +299,7 @@ static int configure_afu(struct ocxl_afu *afu, u8
> afu_idx, struct pci_dev *dev)
>  
>  static void deconfigure_afu(struct ocxl_afu *afu)
>  {
> +	unmap_lpc_mem(afu);
>  	unmap_mmio_areas(afu);
>  	reclaim_afu_pasid(afu);
>  	reclaim_afu_actag(afu);
> diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c
> index 2874811a4398..9e303a5f4d85 100644
> --- a/drivers/misc/ocxl/link.c
> +++ b/drivers/misc/ocxl/link.c
> @@ -738,7 +738,7 @@ int ocxl_link_add_lpc_mem(void *link_handle, u64
> size)
>  }
>  EXPORT_SYMBOL_GPL(ocxl_link_add_lpc_mem);
>  
> -u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev)
> +u64 ocxl_link_lpc_online(void *link_handle, struct pci_dev *pdev)
>  {
>  	struct ocxl_link *link = (struct ocxl_link *) link_handle;
>  
> @@ -759,7 +759,7 @@ u64 ocxl_link_lpc_map(void *link_handle, struct
> pci_dev *pdev)
>  	return link->lpc_mem;
>  }
>  
> -void ocxl_link_lpc_release(void *link_handle, struct pci_dev *pdev)
> +void ocxl_link_lpc_offline(void *link_handle, struct pci_dev *pdev)
>  {
>  	struct ocxl_link *link = (struct ocxl_link *) link_handle;
>  
> diff --git a/drivers/misc/ocxl/ocxl_internal.h
> b/drivers/misc/ocxl/ocxl_internal.h
> index db2647a90fc8..5656a4aab5b7 100644
> --- a/drivers/misc/ocxl/ocxl_internal.h
> +++ b/drivers/misc/ocxl/ocxl_internal.h
> @@ -52,6 +52,12 @@ struct ocxl_afu {
>  	void __iomem *global_mmio_ptr;
>  	u64 pp_mmio_start;
>  	void *private;
> +	u64 lpc_base_addr; /* Covers both LPC & special purpose memory
> */
> +	struct bin_attribute attr_global_mmio;
> +	struct bin_attribute attr_lpc_mem;

The bin_attributes are not needed here.

> +	struct resource lpc_res;
> +	struct bin_attribute attr_special_purpose_mem;
> +	struct resource special_purpose_res;
>  };
>  
>  enum ocxl_context_status {
> @@ -170,7 +176,7 @@ extern u64 ocxl_link_get_lpc_mem_sz(void
> *link_handle);
>   * @link_handle: The OpenCAPI link handle
>   * @pdev: A device that is on the link
>   */
> -u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev);
> +u64 ocxl_link_lpc_online(void *link_handle, struct pci_dev *pdev);
>  
>  /**
>   * Release the LPC memory device for an OpenCAPI device
> @@ -181,6 +187,6 @@ u64 ocxl_link_lpc_map(void *link_handle, struct
> pci_dev *pdev);
>   * @link_handle: The OpenCAPI link handle
>   * @pdev: A device that is on the link
>   */
> -void ocxl_link_lpc_release(void *link_handle, struct pci_dev *pdev);
> +void ocxl_link_lpc_offline(void *link_handle, struct pci_dev *pdev);
>  
>  #endif /* _OCXL_INTERNAL_H_ */
> diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
> index 06dd5839e438..a1897737908d 100644
> --- a/include/misc/ocxl.h
> +++ b/include/misc/ocxl.h
> @@ -212,6 +212,24 @@ int ocxl_irq_set_handler(struct ocxl_context
> *ctx, int irq_id,
>  
>  // AFU Metadata
>  
> +/**
> + * Map the LPC system & special purpose memory for an AFU
> + *
> + * Do not call this during device discovery, as there may me
> multiple
> + * devices on a link, and the memory is mapped for the whole link,
> not
> + * just one device. It should only be called after all devices have
> + * registered their memory on the link.
> + *
> + * afu: The AFU that has the LPC memory to map
> + */
> +extern int ocxl_map_lpc_mem(struct ocxl_afu *afu);
> +
> +/**
> + * Get the physical address range of LPC memory for an AFU
> + * afu: The AFU associated with the LPC memory
> + */
> +extern struct resource *ocxl_afu_lpc_mem(struct ocxl_afu *afu);
> +
>  /**
>   * Get a pointer to the config for an AFU
>   *
-- 
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819

Re: [PATCH 4/5] ocxl: Add functions to map/unmap LPC memory

[Frederic Barrat]

> diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c
> index 2874811a4398..9e303a5f4d85 100644
> --- a/drivers/misc/ocxl/link.c
> +++ b/drivers/misc/ocxl/link.c
> @@ -738,7 +738,7 @@ int ocxl_link_add_lpc_mem(void *link_handle, u64 size)
>   }
>   EXPORT_SYMBOL_GPL(ocxl_link_add_lpc_mem);
>   
> -u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev)
> +u64 ocxl_link_lpc_online(void *link_handle, struct pci_dev *pdev)
>   {
>   	struct ocxl_link *link = (struct ocxl_link *) link_handle;
>   

A bit of a nitpick, but is there any specific reason to rename with 
"online" suffix? I'm discovering it myself, but with memory hotplug, 
"onlining" seems to refer to the second, a.k.a logical memory hotplug 
phase (as described in Documentation/admin-guide/mm/memory-hotplug.rst). 
We'll need to worry about it, but the function here is really doing the 
first phase, a.k.a physical memory hotplug.

   Fred


> @@ -759,7 +759,7 @@ u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev)
>   	return link->lpc_mem;
>   }
>   
> -void ocxl_link_lpc_release(void *link_handle, struct pci_dev *pdev)
> +void ocxl_link_lpc_offline(void *link_handle, struct pci_dev *pdev)
>   {
>   	struct ocxl_link *link = (struct ocxl_link *) link_handle;
>   
> diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
> index db2647a90fc8..5656a4aab5b7 100644
> --- a/drivers/misc/ocxl/ocxl_internal.h
> +++ b/drivers/misc/ocxl/ocxl_internal.h
> @@ -52,6 +52,12 @@ struct ocxl_afu {
>   	void __iomem *global_mmio_ptr;
>   	u64 pp_mmio_start;
>   	void *private;
> +	u64 lpc_base_addr; /* Covers both LPC & special purpose memory */
> +	struct bin_attribute attr_global_mmio;
> +	struct bin_attribute attr_lpc_mem;
> +	struct resource lpc_res;
> +	struct bin_attribute attr_special_purpose_mem;
> +	struct resource special_purpose_res;
>   };
>   
>   enum ocxl_context_status {
> @@ -170,7 +176,7 @@ extern u64 ocxl_link_get_lpc_mem_sz(void *link_handle);
>    * @link_handle: The OpenCAPI link handle
>    * @pdev: A device that is on the link
>    */
> -u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev);
> +u64 ocxl_link_lpc_online(void *link_handle, struct pci_dev *pdev);
>   
>   /**
>    * Release the LPC memory device for an OpenCAPI device
> @@ -181,6 +187,6 @@ u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev);
>    * @link_handle: The OpenCAPI link handle
>    * @pdev: A device that is on the link
>    */
> -void ocxl_link_lpc_release(void *link_handle, struct pci_dev *pdev);
> +void ocxl_link_lpc_offline(void *link_handle, struct pci_dev *pdev);
>   
>   #endif /* _OCXL_INTERNAL_H_ */
> diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
> index 06dd5839e438..a1897737908d 100644
> --- a/include/misc/ocxl.h
> +++ b/include/misc/ocxl.h
> @@ -212,6 +212,24 @@ int ocxl_irq_set_handler(struct ocxl_context *ctx, int irq_id,
>   
>   // AFU Metadata
>   
> +/**
> + * Map the LPC system & special purpose memory for an AFU
> + *
> + * Do not call this during device discovery, as there may me multiple
> + * devices on a link, and the memory is mapped for the whole link, not
> + * just one device. It should only be called after all devices have
> + * registered their memory on the link.
> + *
> + * afu: The AFU that has the LPC memory to map
> + */
> +extern int ocxl_map_lpc_mem(struct ocxl_afu *afu);
> +
> +/**
> + * Get the physical address range of LPC memory for an AFU
> + * afu: The AFU associated with the LPC memory
> + */
> +extern struct resource *ocxl_afu_lpc_mem(struct ocxl_afu *afu);
> +
>   /**
>    * Get a pointer to the config for an AFU
>    *
> 

Re: [PATCH 4/5] ocxl: Add functions to map/unmap LPC memory

[Alastair D'Silva]

On Mon, 2019-09-23 at 13:39 +0200, Frederic Barrat wrote:
> 
> 
> > diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c
> > index 2874811a4398..9e303a5f4d85 100644
> > --- a/drivers/misc/ocxl/link.c
> > +++ b/drivers/misc/ocxl/link.c
> > @@ -738,7 +738,7 @@ int ocxl_link_add_lpc_mem(void *link_handle,
> > u64 size)
> >   }
> >   EXPORT_SYMBOL_GPL(ocxl_link_add_lpc_mem);
> >   
> > -u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev)
> > +u64 ocxl_link_lpc_online(void *link_handle, struct pci_dev *pdev)
> >   {
> >   	struct ocxl_link *link = (struct ocxl_link *) link_handle;
> >   
> 
> A bit of a nitpick, but is there any specific reason to rename with 
> "online" suffix? I'm discovering it myself, but with memory hotplug, 
> "onlining" seems to refer to the second, a.k.a logical memory
> hotplug 
> phase (as described in Documentation/admin-guide/mm/memory-
> hotplug.rst). 
> We'll need to worry about it, but the function here is really doing
> the 
> first phase, a.k.a physical memory hotplug.
> 
>    Fred

It's been a while since I wrote that, so I can't remember why, but your
reasoning is sound, I'll drop the rename.

> 
> 
> > @@ -759,7 +759,7 @@ u64 ocxl_link_lpc_map(void *link_handle, struct
> > pci_dev *pdev)
> >   	return link->lpc_mem;
> >   }
> >   
> > -void ocxl_link_lpc_release(void *link_handle, struct pci_dev
> > *pdev)
> > +void ocxl_link_lpc_offline(void *link_handle, struct pci_dev
> > *pdev)
> >   {
> >   	struct ocxl_link *link = (struct ocxl_link *) link_handle;
> >   
> > diff --git a/drivers/misc/ocxl/ocxl_internal.h
> > b/drivers/misc/ocxl/ocxl_internal.h
> > index db2647a90fc8..5656a4aab5b7 100644
> > --- a/drivers/misc/ocxl/ocxl_internal.h
> > +++ b/drivers/misc/ocxl/ocxl_internal.h
> > @@ -52,6 +52,12 @@ struct ocxl_afu {
> >   	void __iomem *global_mmio_ptr;
> >   	u64 pp_mmio_start;
> >   	void *private;
> > +	u64 lpc_base_addr; /* Covers both LPC & special purpose memory
> > */
> > +	struct bin_attribute attr_global_mmio;
> > +	struct bin_attribute attr_lpc_mem;
> > +	struct resource lpc_res;
> > +	struct bin_attribute attr_special_purpose_mem;
> > +	struct resource special_purpose_res;
> >   };
> >   
> >   enum ocxl_context_status {
> > @@ -170,7 +176,7 @@ extern u64 ocxl_link_get_lpc_mem_sz(void
> > *link_handle);
> >    * @link_handle: The OpenCAPI link handle
> >    * @pdev: A device that is on the link
> >    */
> > -u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev);
> > +u64 ocxl_link_lpc_online(void *link_handle, struct pci_dev *pdev);
> >   
> >   /**
> >    * Release the LPC memory device for an OpenCAPI device
> > @@ -181,6 +187,6 @@ u64 ocxl_link_lpc_map(void *link_handle, struct
> > pci_dev *pdev);
> >    * @link_handle: The OpenCAPI link handle
> >    * @pdev: A device that is on the link
> >    */
> > -void ocxl_link_lpc_release(void *link_handle, struct pci_dev
> > *pdev);
> > +void ocxl_link_lpc_offline(void *link_handle, struct pci_dev
> > *pdev);
> >   
> >   #endif /* _OCXL_INTERNAL_H_ */
> > diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
> > index 06dd5839e438..a1897737908d 100644
> > --- a/include/misc/ocxl.h
> > +++ b/include/misc/ocxl.h
> > @@ -212,6 +212,24 @@ int ocxl_irq_set_handler(struct ocxl_context
> > *ctx, int irq_id,
> >   
> >   // AFU Metadata
> >   
> > +/**
> > + * Map the LPC system & special purpose memory for an AFU
> > + *
> > + * Do not call this during device discovery, as there may me
> > multiple
> > + * devices on a link, and the memory is mapped for the whole link,
> > not
> > + * just one device. It should only be called after all devices
> > have
> > + * registered their memory on the link.
> > + *
> > + * afu: The AFU that has the LPC memory to map
> > + */
> > +extern int ocxl_map_lpc_mem(struct ocxl_afu *afu);
> > +
> > +/**
> > + * Get the physical address range of LPC memory for an AFU
> > + * afu: The AFU associated with the LPC memory
> > + */
> > +extern struct resource *ocxl_afu_lpc_mem(struct ocxl_afu *afu);
> > +
> >   /**
> >    * Get a pointer to the config for an AFU
> >    *
> > 
-- 
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819

[PATCH 5/5] ocxl: Provide additional metadata to userspace

[Alastair D'Silva]

From: Alastair D'Silva <alastair@d-silva.org>

This patch exposes the OpenCAPI device serial number to
userspace.

It also includes placeholders for the LPC & special purpose
memory information (which will be populated in a subsequent patch)
to avoid creating excessive versions of the IOCTL.

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 drivers/misc/ocxl/config.c | 46 ++++++++++++++++++++++++++++++++++++++
 drivers/misc/ocxl/file.c   |  3 ++-
 include/misc/ocxl.h        |  1 +
 include/uapi/misc/ocxl.h   |  9 +++++++-
 4 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
index fb0c3b6f8312..a9203c309365 100644
--- a/drivers/misc/ocxl/config.c
+++ b/drivers/misc/ocxl/config.c
@@ -71,6 +71,51 @@ static int find_dvsec_afu_ctrl(struct pci_dev *dev, u8 afu_idx)
 	return 0;
 }
 
+/**
+ * Find a related PCI device (function 0)
+ * @device: PCI device to match
+ *
+ * Returns a pointer to the related device, or null if not found
+ */
+static struct pci_dev *get_function_0(struct pci_dev *dev)
+{
+	unsigned int devfn = PCI_DEVFN(PCI_SLOT(dev->devfn), 0); // Look for function 0
+
+	return pci_get_domain_bus_and_slot(pci_domain_nr(dev->bus),
+					dev->bus->number, devfn);
+}
+
+static void read_serial(struct pci_dev *dev, struct ocxl_fn_config *fn)
+{
+	u32 low, high;
+	int pos;
+
+	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DSN);
+	if (pos) {
+		pci_read_config_dword(dev, pos + 0x04, &low);
+		pci_read_config_dword(dev, pos + 0x08, &high);
+
+		fn->serial = low | ((u64)high) << 32;
+
+		return;
+	}
+
+	if (PCI_FUNC(dev->devfn) != 0) {
+		struct pci_dev *related = get_function_0(dev);
+
+		if (!related) {
+			fn->serial = 0;
+			return;
+		}
+
+		read_serial(related, fn);
+		pci_dev_put(related);
+		return;
+	}
+
+	fn->serial = 0;
+}
+
 static void read_pasid(struct pci_dev *dev, struct ocxl_fn_config *fn)
 {
 	u16 val;
@@ -208,6 +253,7 @@ int ocxl_config_read_function(struct pci_dev *dev, struct ocxl_fn_config *fn)
 	int rc;
 
 	read_pasid(dev, fn);
+	read_serial(dev, fn);
 
 	rc = read_dvsec_tl(dev, fn);
 	if (rc) {
diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
index 2870c25da166..08f6f594a11d 100644
--- a/drivers/misc/ocxl/file.c
+++ b/drivers/misc/ocxl/file.c
@@ -98,13 +98,14 @@ static long afu_ioctl_get_metadata(struct ocxl_context *ctx,
 
 	memset(&arg, 0, sizeof(arg));
 
-	arg.version = 0;
+	arg.version = 1;
 
 	arg.afu_version_major = ctx->afu->config.version_major;
 	arg.afu_version_minor = ctx->afu->config.version_minor;
 	arg.pasid = ctx->pasid;
 	arg.pp_mmio_size = ctx->afu->config.pp_mmio_stride;
 	arg.global_mmio_size = ctx->afu->config.global_mmio_size;
+	arg.serial = ctx->afu->fn->config.serial;
 
 	if (copy_to_user(uarg, &arg, sizeof(arg)))
 		return -EFAULT;
diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
index a1897737908d..da75db149e6c 100644
--- a/include/misc/ocxl.h
+++ b/include/misc/ocxl.h
@@ -46,6 +46,7 @@ struct ocxl_fn_config {
 	int dvsec_afu_info_pos; /* offset of the AFU information DVSEC */
 	s8 max_pasid_log;
 	s8 max_afu_index;
+	u64 serial;
 };
 
 enum ocxl_endian {
diff --git a/include/uapi/misc/ocxl.h b/include/uapi/misc/ocxl.h
index 6d29a60a896a..d4c6bf10580c 100644
--- a/include/uapi/misc/ocxl.h
+++ b/include/uapi/misc/ocxl.h
@@ -45,7 +45,14 @@ struct ocxl_ioctl_metadata {
 
 	/* End version 0 fields */
 
-	__u64 reserved[13]; /* Total of 16*u64 */
+	// Version 1 fields
+	__u64 lpc_mem_size;
+	__u64 special_purpose_mem_size;
+	__u64 serial;		// Device serial number
+
+	// End version 1 fields
+
+	__u64 reserved[10]; // Total of 16*u64
 };
 
 struct ocxl_ioctl_p9_wait {
-- 
2.21.0

Re: [PATCH 5/5] ocxl: Provide additional metadata to userspace

[Alastair D'Silva]

On Tue, 2019-09-17 at 11:43 +1000, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
> 
> This patch exposes the OpenCAPI device serial number to
> userspace.
> 
> It also includes placeholders for the LPC & special purpose
> memory information (which will be populated in a subsequent patch)
> to avoid creating excessive versions of the IOCTL.
> 

I think it makes more sense to fold in the population of LPC & special
purpose memory into this patch, I'll address this in the next spin.

> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
>  drivers/misc/ocxl/config.c | 46
> ++++++++++++++++++++++++++++++++++++++
>  drivers/misc/ocxl/file.c   |  3 ++-
>  include/misc/ocxl.h        |  1 +
>  include/uapi/misc/ocxl.h   |  9 +++++++-
>  4 files changed, 57 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
> index fb0c3b6f8312..a9203c309365 100644
> --- a/drivers/misc/ocxl/config.c
> +++ b/drivers/misc/ocxl/config.c
> @@ -71,6 +71,51 @@ static int find_dvsec_afu_ctrl(struct pci_dev
> *dev, u8 afu_idx)
>  	return 0;
>  }
>  
> +/**
> + * Find a related PCI device (function 0)
> + * @device: PCI device to match
> + *
> + * Returns a pointer to the related device, or null if not found
> + */
> +static struct pci_dev *get_function_0(struct pci_dev *dev)
> +{
> +	unsigned int devfn = PCI_DEVFN(PCI_SLOT(dev->devfn), 0); //
> Look for function 0
> +
> +	return pci_get_domain_bus_and_slot(pci_domain_nr(dev->bus),
> +					dev->bus->number, devfn);
> +}
> +
> +static void read_serial(struct pci_dev *dev, struct ocxl_fn_config
> *fn)
> +{
> +	u32 low, high;
> +	int pos;
> +
> +	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DSN);
> +	if (pos) {
> +		pci_read_config_dword(dev, pos + 0x04, &low);
> +		pci_read_config_dword(dev, pos + 0x08, &high);
> +
> +		fn->serial = low | ((u64)high) << 32;
> +
> +		return;
> +	}
> +
> +	if (PCI_FUNC(dev->devfn) != 0) {
> +		struct pci_dev *related = get_function_0(dev);
> +
> +		if (!related) {
> +			fn->serial = 0;
> +			return;
> +		}
> +
> +		read_serial(related, fn);
> +		pci_dev_put(related);
> +		return;
> +	}
> +
> +	fn->serial = 0;
> +}
> +
>  static void read_pasid(struct pci_dev *dev, struct ocxl_fn_config
> *fn)
>  {
>  	u16 val;
> @@ -208,6 +253,7 @@ int ocxl_config_read_function(struct pci_dev
> *dev, struct ocxl_fn_config *fn)
>  	int rc;
>  
>  	read_pasid(dev, fn);
> +	read_serial(dev, fn);
>  
>  	rc = read_dvsec_tl(dev, fn);
>  	if (rc) {
> diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
> index 2870c25da166..08f6f594a11d 100644
> --- a/drivers/misc/ocxl/file.c
> +++ b/drivers/misc/ocxl/file.c
> @@ -98,13 +98,14 @@ static long afu_ioctl_get_metadata(struct
> ocxl_context *ctx,
>  
>  	memset(&arg, 0, sizeof(arg));
>  
> -	arg.version = 0;
> +	arg.version = 1;
>  
>  	arg.afu_version_major = ctx->afu->config.version_major;
>  	arg.afu_version_minor = ctx->afu->config.version_minor;
>  	arg.pasid = ctx->pasid;
>  	arg.pp_mmio_size = ctx->afu->config.pp_mmio_stride;
>  	arg.global_mmio_size = ctx->afu->config.global_mmio_size;
> +	arg.serial = ctx->afu->fn->config.serial;
>  
>  	if (copy_to_user(uarg, &arg, sizeof(arg)))
>  		return -EFAULT;
> diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
> index a1897737908d..da75db149e6c 100644
> --- a/include/misc/ocxl.h
> +++ b/include/misc/ocxl.h
> @@ -46,6 +46,7 @@ struct ocxl_fn_config {
>  	int dvsec_afu_info_pos; /* offset of the AFU information DVSEC
> */
>  	s8 max_pasid_log;
>  	s8 max_afu_index;
> +	u64 serial;
>  };
>  
>  enum ocxl_endian {
> diff --git a/include/uapi/misc/ocxl.h b/include/uapi/misc/ocxl.h
> index 6d29a60a896a..d4c6bf10580c 100644
> --- a/include/uapi/misc/ocxl.h
> +++ b/include/uapi/misc/ocxl.h
> @@ -45,7 +45,14 @@ struct ocxl_ioctl_metadata {
>  
>  	/* End version 0 fields */
>  
> -	__u64 reserved[13]; /* Total of 16*u64 */
> +	// Version 1 fields
> +	__u64 lpc_mem_size;
> +	__u64 special_purpose_mem_size;
> +	__u64 serial;		// Device serial number
> +
> +	// End version 1 fields
> +
> +	__u64 reserved[10]; // Total of 16*u64
>  };
>  
>  struct ocxl_ioctl_p9_wait {
> -- 
> 2.21.0

-- 
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819

Re: [PATCH 5/5] ocxl: Provide additional metadata to userspace

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1110 bytes --]

Hi Alastair,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[cannot apply to v5.3 next-20190916]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Alastair-D-Silva/ocxl-Allow-external-drivers-to-access-LPC-memory/20190917-094857
config: x86_64-allyesconfig (attached as .config)
compiler: gcc-7 (Debian 7.4.0-11) 7.4.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from <command-line>:32:0:
>> ./usr/include/misc/ocxl.h:48:2: error: C++ style comments are not allowed in ISO C90
     // Version 1 fields
     ^
>> ./usr/include/misc/ocxl.h:48:2: error: (this will be reported only once per input file)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 69635 bytes --]