On Fri, May 29, 2015 at 12:14:46PM -0500, wdavis@xxxxxxxxxx wrote:
> From: Will Davis <wdavis@xxxxxxxxxx>
>
> Lookup the bus address of the resource by finding the parent host bridge,
> which may be different than the parent host bridge of the target device.
>
> Signed-off-by: Will Davis <wdavis@xxxxxxxxxx>
> ---
> arch/x86/kernel/pci-nommu.c | 32 ++++++++++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
>
> diff --git a/arch/x86/kernel/pci-nommu.c b/arch/x86/kernel/pci-nommu.c
> index da15918..6384482 100644
> --- a/arch/x86/kernel/pci-nommu.c
> +++ b/arch/x86/kernel/pci-nommu.c
> @@ -38,6 +38,37 @@ static dma_addr_t nommu_map_page(struct device *dev, struct page *page,
> return bus;
> }
>
> +static dma_addr_t nommu_map_resource(struct device *dev, struct resource *res,
> + unsigned long offset, size_t size,
> + enum dma_data_direction dir,
> + struct dma_attrs *attrs)
> +{
> + struct pci_bus *bus;
> + struct pci_host_bridge *bridge;
> + struct resource_entry *window;
> + resource_size_t bus_offset = 0;
> + dma_addr_t dma_address;
> +
> + /* Find the parent host bridge of the resource, and determine the
> + * relative offset.
> + */
> + list_for_each_entry(bus, &pci_root_buses, node) {
> + bridge = to_pci_host_bridge(bus->bridge);
> + resource_list_for_each_entry(window, &bridge->windows) {
> + if (resource_contains(window->res, res))
> + bus_offset = window->offset;
> + }
> + }
I don't think this is safe. Assume we have the following topology, and
we want to set it up so 0000:00:00.0 can perform peer-to-peer DMA to
0001:00:01.0:
pci_bus 0000:00: root bus resource [mem 0x80000000-0xffffffff] (bus address [0x80000000-0xffffffff])
pci 0000:00:00.0: ...
pci_bus 0001:00: root bus resource [mem 0x180000000-0x1ffffffff] (bus address [0x80000000-0xffffffff])
pci 0001:00:01.0: reg 0x10: [mem 0x180000000-0x1803fffff 64bit]
I assume the way this works is that the driver for 0000:00:00.0 would call
this function with 0001:00:01.0 and [mem 0x180000000-0x1803fffff 64bit].
We'll figure out that the resource belongs to 0001:00, so we return a
dma_addr of 0x80000000, which is the bus address as seen by 0001:00:01.0.
But if 0000:00:00.0 uses that address, it refers to something in the
0000:00 hierarchy, not the 0001:00 hierarchy.
We talked about pci_bus_address() and pcibios_resource_to_bus() earlier.
What's the subtlety that makes them unusable here? I'd rather not add more
uses of the pci_root_buses list if we can avoid it.
> + dma_address = (res->start - bus_offset) + offset;
> + WARN_ON(size == 0);
> + if (!check_addr("map_resource", dev, dma_address, size))
> + return DMA_ERROR_CODE;
> + flush_write_buffers();
> + return dma_address;
> +}
> +
> +
You added an extra blank line here (there was already an extra one before
nommu_sync_sg_for_device(), which is probably what you copied).
> /* Map a set of buffers described by scatterlist in streaming
> * mode for DMA. This is the scatter-gather version of the
> * above pci_map_single interface. Here the scatter gather list
> @@ -93,6 +124,7 @@ struct dma_map_ops nommu_dma_ops = {
> .free = dma_generic_free_coherent,
> .map_sg = nommu_map_sg,
> .map_page = nommu_map_page,
> + .map_resource = nommu_map_resource,
> .sync_single_for_device = nommu_sync_single_for_device,
> .sync_sg_for_device = nommu_sync_sg_for_device,
> .is_phys = 1,
> --
> 2.4.0
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
