| Message ID | 1366804881-553-3-git-send-email-david@gibson.dropbear.id.au |
|---|---|
| State | New |
| Headers | show |
Il 24/04/2013 14:01, David Gibson ha scritto: > So while its true that the DMAContext handling needs to be better > integrated with the MemoryRegion and AddressSpace handling, that commit > wasn't actually a step in the right direction for it. Since then, the > DMAContext has been extended so it can backend onto an AddressSpace, and > thereby, a MemoryRegion. Effectively a DMAContext is now an AddressSpace > with iommu translation handling on top. > > Therefore, this patch essentially reverts the earlier commit, making all > PCI devices by default shared the global dma_context_memory which backs > onto main system memory. Those cases which need to set up different DMA > address spaces for each PCI device should supply a suitable dma_context_fn > in the bus to correctly assign / create suitable DMAContext structures. I think this will be handled correctly when I submit IOMMU AddressSpace patches (next week or so). The structure will be PCI device 1 PCI device 2 --------------------------------------------------------------- AddressSpace 1 AddressSpace 2 | | | (enable/disable) | (enable/disable) '-------------------. .----------------' v v IOMMU AddressSpace | | (translation) v system memory VFIO will be able to access the IOMMU AddressSpace simply via pci_dev->iommu, and that field will be ==-identical for different PCI devices. Paolo
On Wed, Apr 24, 2013 at 02:34:21PM +0200, Paolo Bonzini wrote: > Il 24/04/2013 14:01, David Gibson ha scritto: > > So while its true that the DMAContext handling needs to be better > > integrated with the MemoryRegion and AddressSpace handling, that commit > > wasn't actually a step in the right direction for it. Since then, the > > DMAContext has been extended so it can backend onto an AddressSpace, and > > thereby, a MemoryRegion. Effectively a DMAContext is now an AddressSpace > > with iommu translation handling on top. > > > > Therefore, this patch essentially reverts the earlier commit, making all > > PCI devices by default shared the global dma_context_memory which backs > > onto main system memory. Those cases which need to set up different DMA > > address spaces for each PCI device should supply a suitable dma_context_fn > > in the bus to correctly assign / create suitable DMAContext structures. > > I think this will be handled correctly when I submit IOMMU AddressSpace > patches (next week or so). The structure will be > > PCI device 1 PCI device 2 > --------------------------------------------------------------- > AddressSpace 1 AddressSpace 2 > | | > | (enable/disable) | (enable/disable) > '-------------------. .----------------' > v v > IOMMU AddressSpace > | > | (translation) > v > system memory > > VFIO will be able to access the IOMMU AddressSpace simply via > pci_dev->iommu, and that field will be ==-identical for different PCI > devices. Hrm, ok. Still seems excessively complicated, but as long as there's a way to get from the pci device to the common address space, I can work with that.
diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 2fdd4b2..038be92 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -843,12 +843,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, if (bus->dma_context_fn) { pci_dev->dma = bus->dma_context_fn(bus, bus->dma_context_opaque, devfn); } else { - /* FIXME: Make dma_context_fn use MemoryRegions instead, so this path is - * taken unconditionally */ - /* FIXME: inherit memory region from bus creator */ - address_space_init(&pci_dev->bus_master_as, get_system_memory()); - pci_dev->dma = g_new(DMAContext, 1); - dma_context_init(pci_dev->dma, &pci_dev->bus_master_as, NULL, NULL, NULL); + pci_dev->dma = &dma_context_memory; } pci_dev->devfn = devfn; pstrcpy(pci_dev->name, sizeof(pci_dev->name), name); @@ -902,12 +897,7 @@ static void do_pci_unregister_device(PCIDevice *pci_dev) qemu_free_irqs(pci_dev->irq); pci_dev->bus->devices[pci_dev->devfn] = NULL; pci_config_free(pci_dev); - - if (!pci_dev->bus->dma_context_fn) { - address_space_destroy(&pci_dev->bus_master_as); - g_free(pci_dev->dma); - pci_dev->dma = NULL; - } + pci_dev->dma = NULL; } static void pci_unregister_io_regions(PCIDevice *pci_dev) diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index 8f682cc..206da1f 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -240,7 +240,6 @@ struct PCIDevice { int32_t devfn; char name[64]; PCIIORegion io_regions[PCI_NUM_REGIONS]; - AddressSpace bus_master_as; DMAContext *dma; /* do not access the following fields */
Commit 817dcc5368988b023c5e1d3f1444fd370c77c6a9 "pci: give each device its own address space" did exactly what the comment suggests, it creates an AddressSpace object for every PCI device to represent the address space it uses for DMA. All the objects so constructed are basically identical to address_space_memory. While its true that PCI devices can have different effective DMA address spaces, they usually don't. Further, the PCI structure already had a way to represent its DMA address space, through its DMAContext pointer. The way those are assigned through an optional callback in the bus also addresses the "FIXME: inherit memory region from bus creator". So while its true that the DMAContext handling needs to be better integrated with the MemoryRegion and AddressSpace handling, that commit wasn't actually a step in the right direction for it. Since then, the DMAContext has been extended so it can backend onto an AddressSpace, and thereby, a MemoryRegion. Effectively a DMAContext is now an AddressSpace with iommu translation handling on top. Therefore, this patch essentially reverts the earlier commit, making all PCI devices by default shared the global dma_context_memory which backs onto main system memory. Those cases which need to set up different DMA address spaces for each PCI device should supply a suitable dma_context_fn in the bus to correctly assign / create suitable DMAContext structures. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> --- hw/pci/pci.c | 14 ++------------ include/hw/pci/pci.h | 1 - 2 files changed, 2 insertions(+), 13 deletions(-)