Re: [PATCH 2/5] powerpc: Map & release OpenCAPI LPC memory
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
Map & release OpenCAPI LPC memory.
Signed-off-by: Alastair D'Silva
---
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,
_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(>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
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
> >
> > Map & release OpenCAPI LPC memory.
> >
> > Signed-off-by: Alastair D'Silva
> > ---
> > 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,
> > _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(>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
Le 17/09/2019 à 03:42, Alastair D'Silva a écrit :
From: Alastair D'Silva
Map & release OpenCAPI LPC memory.
Signed-off-by: Alastair D'Silva
---
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, _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(>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
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
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,
_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(>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 infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
[PATCH 2/5] powerpc: Map & release OpenCAPI LPC memory
From: Alastair D'Silva
Map & release OpenCAPI LPC memory.
Signed-off-by: Alastair D'Silva
---
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, _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(>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
