| Message ID | 20190722230859.5436-15-logang@deltatee.com |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI/P2PDMA: Support transactions that hit the host bridge | expand |
Am 23.07.19 um 01:08 schrieb Logan Gunthorpe: > pci_p2pdma_[un]map_resource() can be used to map a resource given > it's physical address and the backing pci_dev. The functions will call > dma_[un]map_resource() when appropriate. > > This is for demonstration purposes only as there are no users of this > function at this time. Thus, this patch should not be merged at > this time. > > Signed-off-by: Logan Gunthorpe <logang@deltatee.com> Not sure if pci_p2pdma_phys_to_bus actually needs to be exported. But apart from that looks fine to me. Reviewed-by: Christian König <christian.koenig@amd.com> Christian. > --- > drivers/pci/p2pdma.c | 85 ++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 85 insertions(+) > > diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c > index baf476039396..20c834cfd2d3 100644 > --- a/drivers/pci/p2pdma.c > +++ b/drivers/pci/p2pdma.c > @@ -874,6 +874,91 @@ void pci_p2pdma_unmap_sg_attrs(struct device *dev, struct scatterlist *sg, > } > EXPORT_SYMBOL_GPL(pci_p2pdma_unmap_sg_attrs); > > +static pci_bus_addr_t pci_p2pdma_phys_to_bus(struct pci_dev *dev, > + phys_addr_t start, size_t size) > +{ > + struct pci_host_bridge *bridge = pci_find_host_bridge(dev->bus); > + phys_addr_t end = start + size; > + struct resource_entry *window; > + > + resource_list_for_each_entry(window, &bridge->windows) { > + if (window->res->start <= start && window->res->end >= end) > + return start - window->offset; > + } > + > + return DMA_MAPPING_ERROR; > +} > +EXPORT_SYMBOL_GPL(pci_p2pdma_phys_to_bus); > + > +/** > + * pci_p2pdma_map_resource - map a PCI peer-to-peer physical address for DMA > + * @provider: pci device that provides the memory backed by phys_addr > + * @dma_dev: device doing the DMA request > + * @phys_addr: physical address of the memory to map > + * @size: size of the memory to map > + * @dir: DMA direction > + * @attrs: dma attributes passed to dma_map_resource() (if called) > + * > + * Maps a BAR physical address for programming a DMA engine. > + * > + * Returns the dma_addr_t to map or DMA_MAPPING_ERROR on failure > + */ > +dma_addr_t pci_p2pdma_map_resource(struct pci_dev *provider, > + struct device *dma_dev, phys_addr_t phys_addr, size_t size, > + enum dma_data_direction dir, unsigned long attrs) > +{ > + struct pci_dev *client; > + int dist; > + > + client = find_parent_pci_dev(dma_dev); > + if (!client) > + return DMA_MAPPING_ERROR; > + > + dist = upstream_bridge_distance(provider, client, NULL); > + if (dist & P2PDMA_NOT_SUPPORTED) > + return DMA_MAPPING_ERROR; > + > + if (dist & P2PDMA_THRU_HOST_BRIDGE) > + return dma_map_resource(dma_dev, phys_addr, size, dir, attrs); > + else > + return pci_p2pdma_phys_to_bus(provider, phys_addr, size); > +} > +EXPORT_SYMBOL_GPL(pci_p2pdma_map_resource); > + > +/** > + * pci_p2pdma_unmap_resource - unmap a resource mapped with > + * pci_p2pdma_map_resource() > + * @provider: pci device that provides the memory backed by phys_addr > + * @dma_dev: device doing the DMA request > + * @addr: dma address returned by pci_p2pdma_unmap_resource() > + * @size: size of the memory to map > + * @dir: DMA direction > + * @attrs: dma attributes passed to dma_unmap_resource() (if called) > + * > + * Maps a BAR physical address for programming a DMA engine. > + * > + * Returns the dma_addr_t to map or DMA_MAPPING_ERROR on failure > + */ > +void pci_p2pdma_unmap_resource(struct pci_dev *provider, > + struct device *dma_dev, dma_addr_t addr, size_t size, > + enum dma_data_direction dir, unsigned long attrs) > +{ > + struct pci_dev *client; > + int dist; > + > + client = find_parent_pci_dev(dma_dev); > + if (!client) > + return; > + > + dist = upstream_bridge_distance(provider, client, NULL); > + if (dist & P2PDMA_NOT_SUPPORTED) > + return; > + > + if (dist & P2PDMA_THRU_HOST_BRIDGE) > + dma_unmap_resource(dma_dev, addr, size, dir, attrs); > +} > +EXPORT_SYMBOL_GPL(pci_p2pdma_unmap_resource); > + > /** > * pci_p2pdma_enable_store - parse a configfs/sysfs attribute store > * to enable p2pdma
On 2019-07-23 10:28 a.m., Koenig, Christian wrote: > Am 23.07.19 um 01:08 schrieb Logan Gunthorpe: >> pci_p2pdma_[un]map_resource() can be used to map a resource given >> it's physical address and the backing pci_dev. The functions will call >> dma_[un]map_resource() when appropriate. >> >> This is for demonstration purposes only as there are no users of this >> function at this time. Thus, this patch should not be merged at >> this time. >> >> Signed-off-by: Logan Gunthorpe <logang@deltatee.com> > > Not sure if pci_p2pdma_phys_to_bus actually needs to be exported. But > apart from that looks fine to me. Yes, oops, it certainly shouldn't be exported if it's static. I'll fix that. > Reviewed-by: Christian König <christian.koenig@amd.com> > > Christian. > >> --- >> drivers/pci/p2pdma.c | 85 ++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 85 insertions(+) >> >> diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c >> index baf476039396..20c834cfd2d3 100644 >> --- a/drivers/pci/p2pdma.c >> +++ b/drivers/pci/p2pdma.c >> @@ -874,6 +874,91 @@ void pci_p2pdma_unmap_sg_attrs(struct device *dev, struct scatterlist *sg, >> } >> EXPORT_SYMBOL_GPL(pci_p2pdma_unmap_sg_attrs); >> >> +static pci_bus_addr_t pci_p2pdma_phys_to_bus(struct pci_dev *dev, >> + phys_addr_t start, size_t size) >> +{ >> + struct pci_host_bridge *bridge = pci_find_host_bridge(dev->bus); >> + phys_addr_t end = start + size; >> + struct resource_entry *window; >> + >> + resource_list_for_each_entry(window, &bridge->windows) { >> + if (window->res->start <= start && window->res->end >= end) >> + return start - window->offset; >> + } >> + >> + return DMA_MAPPING_ERROR; >> +} >> +EXPORT_SYMBOL_GPL(pci_p2pdma_phys_to_bus); >> + >> +/** >> + * pci_p2pdma_map_resource - map a PCI peer-to-peer physical address for DMA >> + * @provider: pci device that provides the memory backed by phys_addr >> + * @dma_dev: device doing the DMA request >> + * @phys_addr: physical address of the memory to map >> + * @size: size of the memory to map >> + * @dir: DMA direction >> + * @attrs: dma attributes passed to dma_map_resource() (if called) >> + * >> + * Maps a BAR physical address for programming a DMA engine. >> + * >> + * Returns the dma_addr_t to map or DMA_MAPPING_ERROR on failure >> + */ >> +dma_addr_t pci_p2pdma_map_resource(struct pci_dev *provider, >> + struct device *dma_dev, phys_addr_t phys_addr, size_t size, >> + enum dma_data_direction dir, unsigned long attrs) >> +{ >> + struct pci_dev *client; >> + int dist; >> + >> + client = find_parent_pci_dev(dma_dev); >> + if (!client) >> + return DMA_MAPPING_ERROR; >> + >> + dist = upstream_bridge_distance(provider, client, NULL); >> + if (dist & P2PDMA_NOT_SUPPORTED) >> + return DMA_MAPPING_ERROR; >> + >> + if (dist & P2PDMA_THRU_HOST_BRIDGE) >> + return dma_map_resource(dma_dev, phys_addr, size, dir, attrs); >> + else >> + return pci_p2pdma_phys_to_bus(provider, phys_addr, size); >> +} >> +EXPORT_SYMBOL_GPL(pci_p2pdma_map_resource); >> + >> +/** >> + * pci_p2pdma_unmap_resource - unmap a resource mapped with >> + * pci_p2pdma_map_resource() >> + * @provider: pci device that provides the memory backed by phys_addr >> + * @dma_dev: device doing the DMA request >> + * @addr: dma address returned by pci_p2pdma_unmap_resource() >> + * @size: size of the memory to map >> + * @dir: DMA direction >> + * @attrs: dma attributes passed to dma_unmap_resource() (if called) >> + * >> + * Maps a BAR physical address for programming a DMA engine. >> + * >> + * Returns the dma_addr_t to map or DMA_MAPPING_ERROR on failure >> + */ >> +void pci_p2pdma_unmap_resource(struct pci_dev *provider, >> + struct device *dma_dev, dma_addr_t addr, size_t size, >> + enum dma_data_direction dir, unsigned long attrs) >> +{ >> + struct pci_dev *client; >> + int dist; >> + >> + client = find_parent_pci_dev(dma_dev); >> + if (!client) >> + return; >> + >> + dist = upstream_bridge_distance(provider, client, NULL); >> + if (dist & P2PDMA_NOT_SUPPORTED) >> + return; >> + >> + if (dist & P2PDMA_THRU_HOST_BRIDGE) >> + dma_unmap_resource(dma_dev, addr, size, dir, attrs); >> +} >> +EXPORT_SYMBOL_GPL(pci_p2pdma_unmap_resource); >> + >> /** >> * pci_p2pdma_enable_store - parse a configfs/sysfs attribute store >> * to enable p2pdma >
> diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c > index baf476039396..20c834cfd2d3 100644 > --- a/drivers/pci/p2pdma.c > +++ b/drivers/pci/p2pdma.c > @@ -874,6 +874,91 @@ void pci_p2pdma_unmap_sg_attrs(struct device *dev, struct scatterlist *sg, > } > EXPORT_SYMBOL_GPL(pci_p2pdma_unmap_sg_attrs); > > +static pci_bus_addr_t pci_p2pdma_phys_to_bus(struct pci_dev *dev, > + phys_addr_t start, size_t size) > +{ > + struct pci_host_bridge *bridge = pci_find_host_bridge(dev->bus); > + phys_addr_t end = start + size; > + struct resource_entry *window; > + > + resource_list_for_each_entry(window, &bridge->windows) { > + if (window->res->start <= start && window->res->end >= end) > + return start - window->offset; > + } > + > + return DMA_MAPPING_ERROR; This does once again look very expensive for something called in the hot path.
On 2019-07-24 12:32 a.m., Christoph Hellwig wrote: >> diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c >> index baf476039396..20c834cfd2d3 100644 >> --- a/drivers/pci/p2pdma.c >> +++ b/drivers/pci/p2pdma.c >> @@ -874,6 +874,91 @@ void pci_p2pdma_unmap_sg_attrs(struct device *dev, struct scatterlist *sg, >> } >> EXPORT_SYMBOL_GPL(pci_p2pdma_unmap_sg_attrs); >> >> +static pci_bus_addr_t pci_p2pdma_phys_to_bus(struct pci_dev *dev, >> + phys_addr_t start, size_t size) >> +{ >> + struct pci_host_bridge *bridge = pci_find_host_bridge(dev->bus); >> + phys_addr_t end = start + size; >> + struct resource_entry *window; >> + >> + resource_list_for_each_entry(window, &bridge->windows) { >> + if (window->res->start <= start && window->res->end >= end) >> + return start - window->offset; >> + } >> + >> + return DMA_MAPPING_ERROR; > > This does once again look very expensive for something called in the > hot path. Yes. This is the downside of dealing only with a phys_addr_t: we have to look up against it. Unfortunately, I believe it's possible for different BARs on a device to be in different windows, so something like this is necessary unless we already know the BAR the phys_addr_t belongs to. It might probably be sped up a bit by storing the offsets of each bar instead of looping through all the bridge windows, but I don't think it will get you *that* much. As this is an example with no users, the answer here will really depend on what the use-case is doing. If they can lookup, ahead of time, the mapping type and offset then they don't have to do this work on the hot path and it means that pci_p2pdma_map_resource() is simply not a suitable API. Logan
On Wed, Jul 24, 2019 at 10:06:22AM -0600, Logan Gunthorpe wrote: > Yes. This is the downside of dealing only with a phys_addr_t: we have to > look up against it. Unfortunately, I believe it's possible for different > BARs on a device to be in different windows, so something like this is > necessary unless we already know the BAR the phys_addr_t belongs to. It > might probably be sped up a bit by storing the offsets of each bar > instead of looping through all the bridge windows, but I don't think it > will get you *that* much. > > As this is an example with no users, the answer here will really depend > on what the use-case is doing. If they can lookup, ahead of time, the > mapping type and offset then they don't have to do this work on the hot > path and it means that pci_p2pdma_map_resource() is simply not a > suitable API. Ok. So lets just keep this out as an RFC and don't merge it until an actual concrete user shows up.
On 2019-07-25 5:50 a.m., Christoph Hellwig wrote: > On Wed, Jul 24, 2019 at 10:06:22AM -0600, Logan Gunthorpe wrote: >> Yes. This is the downside of dealing only with a phys_addr_t: we have to >> look up against it. Unfortunately, I believe it's possible for different >> BARs on a device to be in different windows, so something like this is >> necessary unless we already know the BAR the phys_addr_t belongs to. It >> might probably be sped up a bit by storing the offsets of each bar >> instead of looping through all the bridge windows, but I don't think it >> will get you *that* much. >> >> As this is an example with no users, the answer here will really depend >> on what the use-case is doing. If they can lookup, ahead of time, the >> mapping type and offset then they don't have to do this work on the hot >> path and it means that pci_p2pdma_map_resource() is simply not a >> suitable API. > > Ok. So lets just keep this out as an RFC and don't merge it until an > actual concrete user shows up. Yup, that was my intention and I mentioned that in the commit message. Logan
diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c index baf476039396..20c834cfd2d3 100644 --- a/drivers/pci/p2pdma.c +++ b/drivers/pci/p2pdma.c @@ -874,6 +874,91 @@ void pci_p2pdma_unmap_sg_attrs(struct device *dev, struct scatterlist *sg, } EXPORT_SYMBOL_GPL(pci_p2pdma_unmap_sg_attrs); +static pci_bus_addr_t pci_p2pdma_phys_to_bus(struct pci_dev *dev, + phys_addr_t start, size_t size) +{ + struct pci_host_bridge *bridge = pci_find_host_bridge(dev->bus); + phys_addr_t end = start + size; + struct resource_entry *window; + + resource_list_for_each_entry(window, &bridge->windows) { + if (window->res->start <= start && window->res->end >= end) + return start - window->offset; + } + + return DMA_MAPPING_ERROR; +} +EXPORT_SYMBOL_GPL(pci_p2pdma_phys_to_bus); + +/** + * pci_p2pdma_map_resource - map a PCI peer-to-peer physical address for DMA + * @provider: pci device that provides the memory backed by phys_addr + * @dma_dev: device doing the DMA request + * @phys_addr: physical address of the memory to map + * @size: size of the memory to map + * @dir: DMA direction + * @attrs: dma attributes passed to dma_map_resource() (if called) + * + * Maps a BAR physical address for programming a DMA engine. + * + * Returns the dma_addr_t to map or DMA_MAPPING_ERROR on failure + */ +dma_addr_t pci_p2pdma_map_resource(struct pci_dev *provider, + struct device *dma_dev, phys_addr_t phys_addr, size_t size, + enum dma_data_direction dir, unsigned long attrs) +{ + struct pci_dev *client; + int dist; + + client = find_parent_pci_dev(dma_dev); + if (!client) + return DMA_MAPPING_ERROR; + + dist = upstream_bridge_distance(provider, client, NULL); + if (dist & P2PDMA_NOT_SUPPORTED) + return DMA_MAPPING_ERROR; + + if (dist & P2PDMA_THRU_HOST_BRIDGE) + return dma_map_resource(dma_dev, phys_addr, size, dir, attrs); + else + return pci_p2pdma_phys_to_bus(provider, phys_addr, size); +} +EXPORT_SYMBOL_GPL(pci_p2pdma_map_resource); + +/** + * pci_p2pdma_unmap_resource - unmap a resource mapped with + * pci_p2pdma_map_resource() + * @provider: pci device that provides the memory backed by phys_addr + * @dma_dev: device doing the DMA request + * @addr: dma address returned by pci_p2pdma_unmap_resource() + * @size: size of the memory to map + * @dir: DMA direction + * @attrs: dma attributes passed to dma_unmap_resource() (if called) + * + * Maps a BAR physical address for programming a DMA engine. + * + * Returns the dma_addr_t to map or DMA_MAPPING_ERROR on failure + */ +void pci_p2pdma_unmap_resource(struct pci_dev *provider, + struct device *dma_dev, dma_addr_t addr, size_t size, + enum dma_data_direction dir, unsigned long attrs) +{ + struct pci_dev *client; + int dist; + + client = find_parent_pci_dev(dma_dev); + if (!client) + return; + + dist = upstream_bridge_distance(provider, client, NULL); + if (dist & P2PDMA_NOT_SUPPORTED) + return; + + if (dist & P2PDMA_THRU_HOST_BRIDGE) + dma_unmap_resource(dma_dev, addr, size, dir, attrs); +} +EXPORT_SYMBOL_GPL(pci_p2pdma_unmap_resource); + /** * pci_p2pdma_enable_store - parse a configfs/sysfs attribute store * to enable p2pdma
pci_p2pdma_[un]map_resource() can be used to map a resource given it's physical address and the backing pci_dev. The functions will call dma_[un]map_resource() when appropriate. This is for demonstration purposes only as there are no users of this function at this time. Thus, this patch should not be merged at this time. Signed-off-by: Logan Gunthorpe <logang@deltatee.com> --- drivers/pci/p2pdma.c | 85 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+)