| Message ID | 20200221032720.33893-4-alastair@au1.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for OpenCAPI Persistent Memory devices | expand |
On 21/2/20 2:26 pm, Alastair D'Silva wrote: > From: Alastair D'Silva <alastair@d-silva.org> > > This patch adds platform support to map & release LPC memory. > > Signed-off-by: Alastair D'Silva <alastair@d-silva.org> Nothing seems obviously wrong here. Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>
On 24/2/20 1:51 pm, Andrew Donnellan wrote: > On 21/2/20 2:26 pm, Alastair D'Silva wrote: >> From: Alastair D'Silva <alastair@d-silva.org> >> >> This patch adds platform support to map & release LPC memory. >> >> Signed-off-by: Alastair D'Silva <alastair@d-silva.org> > > Nothing seems obviously wrong here. > > Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com> Oh, commit message nitpick :) Summary should be powerpc/powernv. Commit message should explain that this is for the powernv platform and presents an interface that drivers can use to make use of the new OPAL calls.
Le 21/02/2020 à 04:26, Alastair D'Silva a écrit : > From: Alastair D'Silva <alastair@d-silva.org> > > This patch adds platform support to map & release LPC memory. > > Signed-off-by: Alastair D'Silva <alastair@d-silva.org> > --- > arch/powerpc/include/asm/pnv-ocxl.h | 4 +++ > arch/powerpc/platforms/powernv/ocxl.c | 43 +++++++++++++++++++++++++++ > 2 files changed, 47 insertions(+) > > diff --git a/arch/powerpc/include/asm/pnv-ocxl.h b/arch/powerpc/include/asm/pnv-ocxl.h > index 7de82647e761..0b2a6707e555 100644 > --- a/arch/powerpc/include/asm/pnv-ocxl.h > +++ b/arch/powerpc/include/asm/pnv-ocxl.h > @@ -32,5 +32,9 @@ 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); > +#ifdef CONFIG_MEMORY_HOTPLUG_SPARSE > +u64 pnv_ocxl_platform_lpc_setup(struct pci_dev *pdev, u64 size); > +void pnv_ocxl_platform_lpc_release(struct pci_dev *pdev); > +#endif This breaks the compilation of the ocxl driver if CONFIG_MEMORY_HOTPLUG=n Those functions still make sense even without memory hotplug, for example in the context of the implementation you had to access opencapi LPC memory through mmap(). The #ifdef is really needed only around the check_hotplug_memory_addressable() call. Fred > #endif /* _ASM_PNV_OCXL_H */ > diff --git a/arch/powerpc/platforms/powernv/ocxl.c b/arch/powerpc/platforms/powernv/ocxl.c > index 8c65aacda9c8..f2edbcc67361 100644 > --- a/arch/powerpc/platforms/powernv/ocxl.c > +++ b/arch/powerpc/platforms/powernv/ocxl.c > @@ -475,6 +475,49 @@ void pnv_ocxl_spa_release(void *platform_data) > } > EXPORT_SYMBOL_GPL(pnv_ocxl_spa_release); > > +#ifdef CONFIG_MEMORY_HOTPLUG_SPARSE > +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; > + u32 bdfn = pci_dev_id(pdev); > + __be64 base_addr_be64; > + u64 base_addr; > + int rc; > + > + rc = opal_npu_mem_alloc(phb->opal_id, bdfn, size, &base_addr_be64); > + if (rc) { > + dev_warn(&pdev->dev, > + "OPAL could not allocate LPC memory, rc=%d\n", rc); > + return 0; > + } > + > + base_addr = be64_to_cpu(base_addr_be64); > + > + rc = check_hotplug_memory_addressable(base_addr >> PAGE_SHIFT, > + size >> PAGE_SHIFT); > + if (rc) > + 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; > + u32 bdfn = pci_dev_id(pdev); > + int rc; > + > + rc = opal_npu_mem_release(phb->opal_id, bdfn); > + if (rc) > + dev_warn(&pdev->dev, > + "OPAL reported rc=%d when releasing LPC memory\n", rc); > +} > +EXPORT_SYMBOL_GPL(pnv_ocxl_platform_lpc_release); > +#endif > + > int pnv_ocxl_spa_remove_pe_from_cache(void *platform_data, int pe_handle) > { > struct spa_data *data = (struct spa_data *) platform_data; >
On Tue, 2020-02-25 at 11:02 +0100, Frederic Barrat wrote: > > Le 21/02/2020 à 04:26, Alastair D'Silva a écrit : > > From: Alastair D'Silva <alastair@d-silva.org> > > > > This patch adds platform support to map & release LPC memory. > > > > Signed-off-by: Alastair D'Silva <alastair@d-silva.org> > > --- > > arch/powerpc/include/asm/pnv-ocxl.h | 4 +++ > > arch/powerpc/platforms/powernv/ocxl.c | 43 > > +++++++++++++++++++++++++++ > > 2 files changed, 47 insertions(+) > > > > diff --git a/arch/powerpc/include/asm/pnv-ocxl.h > > b/arch/powerpc/include/asm/pnv-ocxl.h > > index 7de82647e761..0b2a6707e555 100644 > > --- a/arch/powerpc/include/asm/pnv-ocxl.h > > +++ b/arch/powerpc/include/asm/pnv-ocxl.h > > @@ -32,5 +32,9 @@ 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); > > +#ifdef CONFIG_MEMORY_HOTPLUG_SPARSE > > +u64 pnv_ocxl_platform_lpc_setup(struct pci_dev *pdev, u64 size); > > +void pnv_ocxl_platform_lpc_release(struct pci_dev *pdev); > > +#endif > > This breaks the compilation of the ocxl driver if > CONFIG_MEMORY_HOTPLUG=n > > Those functions still make sense even without memory hotplug, for > example in the context of the implementation you had to access > opencapi > LPC memory through mmap(). The #ifdef is really needed only around > the > check_hotplug_memory_addressable() call. > > Fred Hmm, we do still need sparsemem though. Let me think about his some more. > > > > #endif /* _ASM_PNV_OCXL_H */ > > diff --git a/arch/powerpc/platforms/powernv/ocxl.c > > b/arch/powerpc/platforms/powernv/ocxl.c > > index 8c65aacda9c8..f2edbcc67361 100644 > > --- a/arch/powerpc/platforms/powernv/ocxl.c > > +++ b/arch/powerpc/platforms/powernv/ocxl.c > > @@ -475,6 +475,49 @@ void pnv_ocxl_spa_release(void *platform_data) > > } > > EXPORT_SYMBOL_GPL(pnv_ocxl_spa_release); > > > > +#ifdef CONFIG_MEMORY_HOTPLUG_SPARSE > > +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; > > + u32 bdfn = pci_dev_id(pdev); > > + __be64 base_addr_be64; > > + u64 base_addr; > > + int rc; > > + > > + rc = opal_npu_mem_alloc(phb->opal_id, bdfn, size, > > &base_addr_be64); > > + if (rc) { > > + dev_warn(&pdev->dev, > > + "OPAL could not allocate LPC memory, rc=%d\n", > > rc); > > + return 0; > > + } > > + > > + base_addr = be64_to_cpu(base_addr_be64); > > + > > + rc = check_hotplug_memory_addressable(base_addr >> PAGE_SHIFT, > > + size >> PAGE_SHIFT); > > + if (rc) > > + 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; > > + u32 bdfn = pci_dev_id(pdev); > > + int rc; > > + > > + rc = opal_npu_mem_release(phb->opal_id, bdfn); > > + if (rc) > > + dev_warn(&pdev->dev, > > + "OPAL reported rc=%d when releasing LPC > > memory\n", rc); > > +} > > +EXPORT_SYMBOL_GPL(pnv_ocxl_platform_lpc_release); > > +#endif > > + > > int pnv_ocxl_spa_remove_pe_from_cache(void *platform_data, int > > pe_handle) > > { > > struct spa_data *data = (struct spa_data *) platform_data; > >
On 21/2/20 2:26 pm, Alastair D'Silva wrote:> +#ifdef CONFIG_MEMORY_HOTPLUG_SPARSE > +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; > + u32 bdfn = pci_dev_id(pdev); > + __be64 base_addr_be64; > + u64 base_addr; > + int rc; > + > + rc = opal_npu_mem_alloc(phb->opal_id, bdfn, size, &base_addr_be64); Sparse warning: https://openpower.xyz/job/snowpatch/job/snowpatch-linux-sparse/15776//artifact/linux/report.txt I think in patch 1 we need to change a uint64_t to a __be64.
On Tue, 2020-03-03 at 17:10 +1100, Andrew Donnellan wrote: > On 21/2/20 2:26 pm, Alastair D'Silva wrote:> +#ifdef > CONFIG_MEMORY_HOTPLUG_SPARSE > > +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; > > + u32 bdfn = pci_dev_id(pdev); > > + __be64 base_addr_be64; > > + u64 base_addr; > > + int rc; > > + > > + rc = opal_npu_mem_alloc(phb->opal_id, bdfn, size, > > &base_addr_be64); > > Sparse warning: > > https://openpower.xyz/job/snowpatch/job/snowpatch-linux-sparse/15776//artifact/linux/report.txt > > I think in patch 1 we need to change a uint64_t to a __be64. > Ok, thanks
diff --git a/arch/powerpc/include/asm/pnv-ocxl.h b/arch/powerpc/include/asm/pnv-ocxl.h index 7de82647e761..0b2a6707e555 100644 --- a/arch/powerpc/include/asm/pnv-ocxl.h +++ b/arch/powerpc/include/asm/pnv-ocxl.h @@ -32,5 +32,9 @@ 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); +#ifdef CONFIG_MEMORY_HOTPLUG_SPARSE +u64 pnv_ocxl_platform_lpc_setup(struct pci_dev *pdev, u64 size); +void pnv_ocxl_platform_lpc_release(struct pci_dev *pdev); +#endif #endif /* _ASM_PNV_OCXL_H */ diff --git a/arch/powerpc/platforms/powernv/ocxl.c b/arch/powerpc/platforms/powernv/ocxl.c index 8c65aacda9c8..f2edbcc67361 100644 --- a/arch/powerpc/platforms/powernv/ocxl.c +++ b/arch/powerpc/platforms/powernv/ocxl.c @@ -475,6 +475,49 @@ void pnv_ocxl_spa_release(void *platform_data) } EXPORT_SYMBOL_GPL(pnv_ocxl_spa_release); +#ifdef CONFIG_MEMORY_HOTPLUG_SPARSE +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; + u32 bdfn = pci_dev_id(pdev); + __be64 base_addr_be64; + u64 base_addr; + int rc; + + rc = opal_npu_mem_alloc(phb->opal_id, bdfn, size, &base_addr_be64); + if (rc) { + dev_warn(&pdev->dev, + "OPAL could not allocate LPC memory, rc=%d\n", rc); + return 0; + } + + base_addr = be64_to_cpu(base_addr_be64); + + rc = check_hotplug_memory_addressable(base_addr >> PAGE_SHIFT, + size >> PAGE_SHIFT); + if (rc) + 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; + u32 bdfn = pci_dev_id(pdev); + int rc; + + rc = opal_npu_mem_release(phb->opal_id, bdfn); + if (rc) + dev_warn(&pdev->dev, + "OPAL reported rc=%d when releasing LPC memory\n", rc); +} +EXPORT_SYMBOL_GPL(pnv_ocxl_platform_lpc_release); +#endif + int pnv_ocxl_spa_remove_pe_from_cache(void *platform_data, int pe_handle) { struct spa_data *data = (struct spa_data *) platform_data;