[Qemu-devel] [PATCH 1/5] pci: Simpler implementation of PCI_COMMAND_MASTER bit

[David Gibson]

In commit 1c380f9460522f32c8dd2577b2a53d518ec91c6d "pci: honor
PCI_COMMAND_MASTER" the PCI_COMMAND_MASTER bit of the PCI command register
was implemented by toggling the enable bit on a memory region alias
interposed between the PCI device's dma address space and the main
system memory region.

Introducing an extra alias region for every PCI device just to implement
that bit seems like serious overkill.  Furthermore, it doesn't work when
there's a (guest side) iommu present, since that uses a different path for
constructing the PCI device's dma address space.

This patch removes the aliased window, instead implementing
PCI_COMMAND_MASTER with tests in the PCI DMA functions.

Signed-off-by: David Gibson <address@hidden>
---
 hw/pci/pci.c         |   14 +-------------
 include/hw/pci/pci.h |   19 ++++++++++++++-----
 2 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 051da67..2fdd4b2 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -425,10 +425,6 @@ static int get_pci_config_device(QEMUFile *f, void *pv, 
size_t size)
 
     pci_update_mappings(s);
 
-    memory_region_set_enabled(&s->bus_master_enable_region,
-                              pci_get_word(s->config + PCI_COMMAND)
-                              & PCI_COMMAND_MASTER);
-
     g_free(config);
     return 0;
 }
@@ -850,11 +846,7 @@ static PCIDevice *do_pci_register_device(PCIDevice 
*pci_dev, PCIBus *bus,
         /* FIXME: Make dma_context_fn use MemoryRegions instead, so this path 
is
          * taken unconditionally */
         /* FIXME: inherit memory region from bus creator */
-        memory_region_init_alias(&pci_dev->bus_master_enable_region, "bus 
master",
-                                 get_system_memory(), 0,
-                                 memory_region_size(get_system_memory()));
-        memory_region_set_enabled(&pci_dev->bus_master_enable_region, false);
-        address_space_init(&pci_dev->bus_master_as, 
&pci_dev->bus_master_enable_region);
+        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);
     }
@@ -913,7 +905,6 @@ static void do_pci_unregister_device(PCIDevice *pci_dev)
 
     if (!pci_dev->bus->dma_context_fn) {
         address_space_destroy(&pci_dev->bus_master_as);
-        memory_region_destroy(&pci_dev->bus_master_enable_region);
         g_free(pci_dev->dma);
         pci_dev->dma = NULL;
     }
@@ -1201,9 +1192,6 @@ void pci_default_write_config(PCIDevice *d, uint32_t 
addr, uint32_t val, int l)
 
     if (range_covers_byte(addr, l, PCI_COMMAND)) {
         pci_update_irq_disabled(d, was_irq_disabled);
-        memory_region_set_enabled(&d->bus_master_enable_region,
-                                  pci_get_word(d->config + PCI_COMMAND)
-                                    & PCI_COMMAND_MASTER);
     }
 
     msi_write_config(d, addr, val, l);
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 7e5986a..8f682cc 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -241,7 +241,6 @@ struct PCIDevice {
     char name[64];
     PCIIORegion io_regions[PCI_NUM_REGIONS];
     AddressSpace bus_master_as;
-    MemoryRegion bus_master_enable_region;
     DMAContext *dma;
 
     /* do not access the following fields */
@@ -648,8 +647,12 @@ static inline DMAContext *pci_dma_context(PCIDevice *dev)
 static inline int pci_dma_rw(PCIDevice *dev, dma_addr_t addr,
                              void *buf, dma_addr_t len, DMADirection dir)
 {
-    dma_memory_rw(pci_dma_context(dev), addr, buf, len, dir);
-    return 0;
+    if (pci_get_word(dev->config + PCI_COMMAND) & PCI_COMMAND_MASTER) {
+        dma_memory_rw(pci_dma_context(dev), addr, buf, len, dir);
+        return 0;
+    } else {
+        return -EPERM;
+    }
 }
 
 static inline int pci_dma_read(PCIDevice *dev, dma_addr_t addr,
@@ -668,12 +671,18 @@ static inline int pci_dma_write(PCIDevice *dev, 
dma_addr_t addr,
     static inline uint##_bits##_t ld##_l##_pci_dma(PCIDevice *dev,      \
                                                    dma_addr_t addr)     \
     {                                                                   \
-        return ld##_l##_dma(pci_dma_context(dev), addr);                \
+        if (pci_get_word(dev->config + PCI_COMMAND) & PCI_COMMAND_MASTER) { \
+            return ld##_l##_dma(pci_dma_context(dev), addr);            \
+        } else {                                                        \
+            return -1;                                                  \
+        }                                                               \
     }                                                                   \
     static inline void st##_s##_pci_dma(PCIDevice *dev,                 \
                                         dma_addr_t addr, uint##_bits##_t val) \
     {                                                                   \
-        st##_s##_dma(pci_dma_context(dev), addr, val);                  \
+        if (pci_get_word(dev->config + PCI_COMMAND) & PCI_COMMAND_MASTER) { \
+            st##_s##_dma(pci_dma_context(dev), addr, val);              \
+        }                                                               \
     }
 
 PCI_DMA_DEFINE_LDST(ub, b, 8);
-- 
1.7.10.4