| Message ID | 20200221032720.33893-8-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:27 pm, 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/core.c | 51 +++++++++++++++++++++++++++++++ > drivers/misc/ocxl/ocxl_internal.h | 3 ++ > include/misc/ocxl.h | 21 +++++++++++++ > 3 files changed, 75 insertions(+) > > diff --git a/drivers/misc/ocxl/core.c b/drivers/misc/ocxl/core.c > index 2531c6cf19a0..75ff14e3882a 100644 > --- a/drivers/misc/ocxl/core.c > +++ b/drivers/misc/ocxl/core.c > @@ -210,6 +210,56 @@ static void unmap_mmio_areas(struct ocxl_afu *afu) > release_fn_bar(afu->fn, afu->config.global_mmio_bar); > } > > +int ocxl_afu_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; I'd prefer the comparison here to be: afu->config.lpc_mem_size == 0 && afu->config.special_purpose_mem_size == 0 so a reader doesn't have to think about what this means. > + > + afu->lpc_base_addr = ocxl_link_lpc_map(afu->fn->link, dev); > + if (afu->lpc_base_addr == 0) > + return -EINVAL; > + > + if (afu->config.lpc_mem_size > 0) { > + afu->lpc_res.start = afu->lpc_base_addr + afu->config.lpc_mem_offset; Maybe not for this series - hmm, I wonder if we should print a warning somewhere (maybe in read_afu_lpc_memory_info()?) if we see the case where (lpc_mem_offset > 0 && lpc_mem_size == 0). Likewise for special purpose? > + afu->lpc_res.end = afu->lpc_res.start + afu->config.lpc_mem_size - 1; > + } > + > + if (afu->config.special_purpose_mem_size > 0) { > + 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_GPL(ocxl_afu_map_lpc_mem); > + > +struct resource *ocxl_afu_lpc_mem(struct ocxl_afu *afu) > +{ > + return &afu->lpc_res; > +} > +EXPORT_SYMBOL_GPL(ocxl_afu_lpc_mem); What's the point of this function? A layer of indirection just in case we need it in future? > + > +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; > + > + // only release the link when the the last consumer calls release > + ocxl_link_lpc_release(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; > @@ -251,6 +301,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/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h > index d0c8c4838f42..ce0cac1da416 100644 > --- a/drivers/misc/ocxl/ocxl_internal.h > +++ b/drivers/misc/ocxl/ocxl_internal.h > @@ -52,6 +52,9 @@ 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 resource lpc_res; > + struct resource special_purpose_res; > }; > > enum ocxl_context_status { > diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h > index 357ef1aadbc0..d8b0b4d46bfb 100644 > --- a/include/misc/ocxl.h > +++ b/include/misc/ocxl.h > @@ -203,6 +203,27 @@ int ocxl_irq_set_handler(struct ocxl_context *ctx, int irq_id, > > // AFU Metadata > > +/** > + * ocxl_afu_map_lpc_mem() - Map the LPC system & special purpose memory for an AFU > + * Do not call this during device discovery, as there may me multiple be > + * 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 > + * > + * Returns 0 on success, negative on failure > + */ > +int ocxl_afu_map_lpc_mem(struct ocxl_afu *afu); > + > +/** > + * ocxl_afu_lpc_mem() - Get the physical address range of LPC memory for an AFU > + * @afu: The AFU associated with the LPC memory > + * > + * Returns a pointer to the resource struct for the physical address range > + */ > +struct resource *ocxl_afu_lpc_mem(struct ocxl_afu *afu); > + > /** > * ocxl_afu_config() - Get a pointer to the config for an AFU > * @afu: a pointer to the AFU to get the config for >
On Mon, 2020-02-24 at 17:02 +1100, Andrew Donnellan wrote: > On 21/2/20 2:27 pm, 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/core.c | 51 > > +++++++++++++++++++++++++++++++ > > drivers/misc/ocxl/ocxl_internal.h | 3 ++ > > include/misc/ocxl.h | 21 +++++++++++++ > > 3 files changed, 75 insertions(+) > > > > diff --git a/drivers/misc/ocxl/core.c b/drivers/misc/ocxl/core.c > > index 2531c6cf19a0..75ff14e3882a 100644 > > --- a/drivers/misc/ocxl/core.c > > +++ b/drivers/misc/ocxl/core.c > > @@ -210,6 +210,56 @@ static void unmap_mmio_areas(struct ocxl_afu > > *afu) > > release_fn_bar(afu->fn, afu->config.global_mmio_bar); > > } > > > > +int ocxl_afu_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; > > I'd prefer the comparison here to be: > > afu->config.lpc_mem_size == 0 && > afu->config.special_purpose_mem_size == 0 > > so a reader doesn't have to think about what this means. > Ok > > + > > + afu->lpc_base_addr = ocxl_link_lpc_map(afu->fn->link, dev); > > + if (afu->lpc_base_addr == 0) > > + return -EINVAL; > > + > > + if (afu->config.lpc_mem_size > 0) { > > + afu->lpc_res.start = afu->lpc_base_addr + afu- > > >config.lpc_mem_offset; > > Maybe not for this series - hmm, I wonder if we should print a > warning > somewhere (maybe in read_afu_lpc_memory_info()?) if we see the case > where (lpc_mem_offset > 0 && lpc_mem_size == 0). Likewise for > special > purpose? > Sounds reasonable, might as well add it here since there are other LPC changes. > > + afu->lpc_res.end = afu->lpc_res.start + afu- > > >config.lpc_mem_size - 1; > > + } > > + > > + if (afu->config.special_purpose_mem_size > 0) { > > + 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_GPL(ocxl_afu_map_lpc_mem); > > + > > +struct resource *ocxl_afu_lpc_mem(struct ocxl_afu *afu) > > +{ > > + return &afu->lpc_res; > > +} > > +EXPORT_SYMBOL_GPL(ocxl_afu_lpc_mem); > > What's the point of this function? A layer of indirection just in > case > we need it in future? > struct ocxl_afu is opaque outsite the ocxl driver. > > + > > +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; > > + > > + // only release the link when the the last consumer > > calls release > > + ocxl_link_lpc_release(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; > > @@ -251,6 +301,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/ocxl_internal.h > > b/drivers/misc/ocxl/ocxl_internal.h > > index d0c8c4838f42..ce0cac1da416 100644 > > --- a/drivers/misc/ocxl/ocxl_internal.h > > +++ b/drivers/misc/ocxl/ocxl_internal.h > > @@ -52,6 +52,9 @@ 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 resource lpc_res; > > + struct resource special_purpose_res; > > }; > > > > enum ocxl_context_status { > > diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h > > index 357ef1aadbc0..d8b0b4d46bfb 100644 > > --- a/include/misc/ocxl.h > > +++ b/include/misc/ocxl.h > > @@ -203,6 +203,27 @@ int ocxl_irq_set_handler(struct ocxl_context > > *ctx, int irq_id, > > > > // AFU Metadata > > > > +/** > > + * ocxl_afu_map_lpc_mem() - Map the LPC system & special purpose > > memory for an AFU > > + * Do not call this during device discovery, as there may me > > multiple > > be > > > + * 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 > > + * > > + * Returns 0 on success, negative on failure > > + */ > > +int ocxl_afu_map_lpc_mem(struct ocxl_afu *afu); > > + > > +/** > > + * ocxl_afu_lpc_mem() - Get the physical address range of LPC > > memory for an AFU > > + * @afu: The AFU associated with the LPC memory > > + * > > + * Returns a pointer to the resource struct for the physical > > address range > > + */ > > +struct resource *ocxl_afu_lpc_mem(struct ocxl_afu *afu); > > + > > /** > > * ocxl_afu_config() - Get a pointer to the config for an AFU > > * @afu: a pointer to the AFU to get the config for > >
Le 21/02/2020 à 04:27, 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> > --- It looks ok to me. Acked-by: Frederic Barrat <fbarrat@linux.ibm.com> > drivers/misc/ocxl/core.c | 51 +++++++++++++++++++++++++++++++ > drivers/misc/ocxl/ocxl_internal.h | 3 ++ > include/misc/ocxl.h | 21 +++++++++++++ > 3 files changed, 75 insertions(+) > > diff --git a/drivers/misc/ocxl/core.c b/drivers/misc/ocxl/core.c > index 2531c6cf19a0..75ff14e3882a 100644 > --- a/drivers/misc/ocxl/core.c > +++ b/drivers/misc/ocxl/core.c > @@ -210,6 +210,56 @@ static void unmap_mmio_areas(struct ocxl_afu *afu) > release_fn_bar(afu->fn, afu->config.global_mmio_bar); > } > > +int ocxl_afu_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_map(afu->fn->link, dev); > + if (afu->lpc_base_addr == 0) > + return -EINVAL; > + > + if (afu->config.lpc_mem_size > 0) { > + 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 > 0) { > + 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_GPL(ocxl_afu_map_lpc_mem); > + > +struct resource *ocxl_afu_lpc_mem(struct ocxl_afu *afu) > +{ > + return &afu->lpc_res; > +} > +EXPORT_SYMBOL_GPL(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; > + > + // only release the link when the the last consumer calls release > + ocxl_link_lpc_release(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; > @@ -251,6 +301,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/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h > index d0c8c4838f42..ce0cac1da416 100644 > --- a/drivers/misc/ocxl/ocxl_internal.h > +++ b/drivers/misc/ocxl/ocxl_internal.h > @@ -52,6 +52,9 @@ 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 resource lpc_res; > + struct resource special_purpose_res; > }; > > enum ocxl_context_status { > diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h > index 357ef1aadbc0..d8b0b4d46bfb 100644 > --- a/include/misc/ocxl.h > +++ b/include/misc/ocxl.h > @@ -203,6 +203,27 @@ int ocxl_irq_set_handler(struct ocxl_context *ctx, int irq_id, > > // AFU Metadata > > +/** > + * ocxl_afu_map_lpc_mem() - 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 > + * > + * Returns 0 on success, negative on failure > + */ > +int ocxl_afu_map_lpc_mem(struct ocxl_afu *afu); > + > +/** > + * ocxl_afu_lpc_mem() - Get the physical address range of LPC memory for an AFU > + * @afu: The AFU associated with the LPC memory > + * > + * Returns a pointer to the resource struct for the physical address range > + */ > +struct resource *ocxl_afu_lpc_mem(struct ocxl_afu *afu); > + > /** > * ocxl_afu_config() - Get a pointer to the config for an AFU > * @afu: a pointer to the AFU to get the config for >
diff --git a/drivers/misc/ocxl/core.c b/drivers/misc/ocxl/core.c index 2531c6cf19a0..75ff14e3882a 100644 --- a/drivers/misc/ocxl/core.c +++ b/drivers/misc/ocxl/core.c @@ -210,6 +210,56 @@ static void unmap_mmio_areas(struct ocxl_afu *afu) release_fn_bar(afu->fn, afu->config.global_mmio_bar); } +int ocxl_afu_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_map(afu->fn->link, dev); + if (afu->lpc_base_addr == 0) + return -EINVAL; + + if (afu->config.lpc_mem_size > 0) { + 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 > 0) { + 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_GPL(ocxl_afu_map_lpc_mem); + +struct resource *ocxl_afu_lpc_mem(struct ocxl_afu *afu) +{ + return &afu->lpc_res; +} +EXPORT_SYMBOL_GPL(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; + + // only release the link when the the last consumer calls release + ocxl_link_lpc_release(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; @@ -251,6 +301,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/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h index d0c8c4838f42..ce0cac1da416 100644 --- a/drivers/misc/ocxl/ocxl_internal.h +++ b/drivers/misc/ocxl/ocxl_internal.h @@ -52,6 +52,9 @@ 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 resource lpc_res; + struct resource special_purpose_res; }; enum ocxl_context_status { diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h index 357ef1aadbc0..d8b0b4d46bfb 100644 --- a/include/misc/ocxl.h +++ b/include/misc/ocxl.h @@ -203,6 +203,27 @@ int ocxl_irq_set_handler(struct ocxl_context *ctx, int irq_id, // AFU Metadata +/** + * ocxl_afu_map_lpc_mem() - 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 + * + * Returns 0 on success, negative on failure + */ +int ocxl_afu_map_lpc_mem(struct ocxl_afu *afu); + +/** + * ocxl_afu_lpc_mem() - Get the physical address range of LPC memory for an AFU + * @afu: The AFU associated with the LPC memory + * + * Returns a pointer to the resource struct for the physical address range + */ +struct resource *ocxl_afu_lpc_mem(struct ocxl_afu *afu); + /** * ocxl_afu_config() - Get a pointer to the config for an AFU * @afu: a pointer to the AFU to get the config for