Messages in this thread Patch in this message | | | Subject | Re: [PATCH v3 07/27] ocxl: Add functions to map/unmap LPC memory | From | Andrew Donnellan <> | Date | Mon, 24 Feb 2020 17:02:32 +1100 |
| |
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 >
-- Andrew Donnellan OzLabs, ADL Canberra ajd@linux.ibm.com IBM Australia Limited
| |