Skip to contentSkip to conversation

pci: implement bridge filtering

PATCH In review Michael S. Tsirkin 3 participants 8 messages Sep 4, 2011 – Sep 20, 2011

lore
Share & tools
lore

Michael S. Tsirkin

author
#0 lore raw

PATCH 1/1 pci: implement bridge filtering

Support bridge filtering on top of the memory
API as suggested by Avi Kivity:

Create a memory region for the bridge's address space. This region is not directly added to system_memory or its descendants. Devices under the bridge see this region as its pci_address_space(). The region is as large as the entire address space - it does not take into account any windows.

For each of the three windows (pref, non-pref, vga), create an alias with the appropriate start and size. Map the alias into the bridge's parent's pci_address_space(), as subregions.

Signed-off-by Michael S. Tsirkin <[email protected]>

---

The below seems to work fine for me so I applied this. Still need to test bridge filtering, any help with this appreciated.

 hw/pci.c           |   70 +---------------------------------------
 hw/pci.h           |    2 -
 hw/pci_bridge.c    |   89 +++++++++++++++++++++++++++++++++++++++++++++++++---
 hw/pci_internals.h |    3 ++
 4 files changed, 89 insertions(+), 75 deletions(-)
hw/pci.c +2 -68
index 57ff7b1..56dfa18 100644
@@ -889,7 +889,6 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
     r = &pci_dev->io_regions[region_num];
     r->addr = PCI_BAR_UNMAPPED;
     r->size = size;
-    r->filtered_size = size;
     r->type = type;
     r->memory = NULL;
 

@@ -920,41 +919,6 @@ pcibus_t pci_get_bar_addr(PCIDevice *pci_dev, int region_num)
     return pci_dev->io_regions[region_num].addr;
 }
 
-static void pci_bridge_filter(PCIDevice *d, pcibus_t *addr, pcibus_t *size,
-                              uint8_t type)
-{
-    pcibus_t base = *addr;
-    pcibus_t limit = *addr + *size - 1;
-    PCIDevice *br;
-
-    for (br = d->bus->parent_dev; br; br = br->bus->parent_dev) {
-        uint16_t cmd = pci_get_word(d->config + PCI_COMMAND);
-
-        if (type & PCI_BASE_ADDRESS_SPACE_IO) {
-            if (!(cmd & PCI_COMMAND_IO)) {
-                goto no_map;
-            }
-        } else {
-            if (!(cmd & PCI_COMMAND_MEMORY)) {
-                goto no_map;
-            }
-        }
-
-        base = MAX(base, pci_bridge_get_base(br, type));
-        limit = MIN(limit, pci_bridge_get_limit(br, type));
-    }
-
-    if (base > limit) {
-        goto no_map;
-    }
-    *addr = base;
-    *size = limit - base + 1;
-    return;
-no_map:
-    *addr = PCI_BAR_UNMAPPED;
-    *size = 0;
-}
-
 static pcibus_t pci_bar_address(PCIDevice *d,
 				int reg, uint8_t type, pcibus_t size)
 {

@@ -1024,7 +988,7 @@ static void pci_update_mappings(PCIDevice *d)
 {
     PCIIORegion *r;
     int i;
-    pcibus_t new_addr, filtered_size;
+    pcibus_t new_addr;
 
     for(i = 0; i < PCI_NUM_REGIONS; i++) {
         r = &d->io_regions[i];

@@ -1035,14 +999,8 @@ static void pci_update_mappings(PCIDevice *d)
 
         new_addr = pci_bar_address(d, i, r->type, r->size);
 
-        /* bridge filtering */
-        filtered_size = r->size;
-        if (new_addr != PCI_BAR_UNMAPPED) {
-            pci_bridge_filter(d, &new_addr, &filtered_size, r->type);
-        }
-
         /* This bar isn't changed */
-        if (new_addr == r->addr && filtered_size == r->filtered_size)
+        if (new_addr == r->addr)
             continue;
 
         /* now do the real mapping */

@@ -1050,15 +1008,7 @@ static void pci_update_mappings(PCIDevice *d)
             memory_region_del_subregion(r->address_space, r->memory);
         }
         r->addr = new_addr;
-        r->filtered_size = filtered_size;
         if (r->addr != PCI_BAR_UNMAPPED) {
-            /*
-             * TODO: currently almost all the map funcions assumes
-             * filtered_size == size and addr & ~(size - 1) == addr.
-             * However with bridge filtering, they aren't always true.
-             * Teach them such cases, such that filtered_size < size and
-             * addr & (size - 1) != 0.
-             */
             if (r->type & PCI_BASE_ADDRESS_SPACE_IO) {
                 memory_region_add_subregion_overlap(r->address_space,
                                                     r->addr,

@@ -1576,22 +1526,6 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, const char *default_model,
     return res;
 }
 
-static void pci_bridge_update_mappings_fn(PCIBus *b, PCIDevice *d)
-{
-    pci_update_mappings(d);
-}
-
-void pci_bridge_update_mappings(PCIBus *b)
-{
-    PCIBus *child;
-
-    pci_for_each_device_under_bus(b, pci_bridge_update_mappings_fn);
-
-    QLIST_FOREACH(child, &b->child, sibling) {
-        pci_bridge_update_mappings(child);
-    }
-}
-
 /* Whether a given bus number is in range of the secondary
  * bus of the given bridge device. */
 static bool pci_secondary_bus_in_range(PCIDevice *dev, int bus_num)
hw/pci.h +0 -2
index 391217e..65e1568 100644
@@ -90,7 +90,6 @@ typedef struct PCIIORegion {
     pcibus_t addr; /* current PCI mapping address. -1 means not mapped */
 #define PCI_BAR_UNMAPPED (~(pcibus_t)0)
     pcibus_t size;
-    pcibus_t filtered_size;
     uint8_t type;
     MemoryRegion *memory;
     MemoryRegion *address_space;

@@ -277,7 +276,6 @@ int pci_read_devaddr(Monitor *mon, const char *addr, int *domp, int *busp,
 
 void do_pci_info_print(Monitor *mon, const QObject *data);
 void do_pci_info(Monitor *mon, QObject **ret_data);
-void pci_bridge_update_mappings(PCIBus *b);
 
 void pci_device_deassert_intx(PCIDevice *dev);
hw/pci_bridge.c +84 -5

115 lines not on this page · raw message

hw/pci_internals.h +3 -0
index c7fd23d..578c8d2 100644
@@ -27,6 +27,9 @@ struct PCIBus {
     target_phys_addr_t mem_base;
     MemoryRegion *address_space_mem;
     MemoryRegion *address_space_io;
+    MemoryRegion *alias_pref_mem;
+    MemoryRegion *alias_mem;
+    MemoryRegion *alias_io;
 
     QLIST_HEAD(, PCIBus) child; /* this will be replaced by qdev later */
     QLIST_ENTRY(PCIBus) sibling;/* this will be replaced by qdev later */
Signature · 1 line
1.7.5.53.gc233e
#1 lore raw

At 09/05/2011 02:13 AM, Michael S. Tsirkin Write:

Quoted 13 lines

Support bridge filtering on top of the memory
API as suggested by Avi Kivity:

Create a memory region for the bridge's address space. This region is not directly added to system_memory or its descendants. Devices under the bridge see this region as its pci_address_space(). The region is as large as the entire address space - it does not take into account any windows.

For each of the three windows (pref, non-pref, vga), create an alias with the appropriate start and size. Map the alias into the bridge's parent's pci_address_space(), as subregions.

Quoted 7 lines
Signed-off-by Michael S. Tsirkin <[email protected]>

---

The below seems to work fine for me so I applied this. Still need to test bridge filtering, any help with this appreciated.

I test bridge filtering, and the BAR still can be visible on guest even if I change the memory region.

Thanks
Wen Congyang

#2 lore raw

At 09/14/2011 09:48 AM, Wen Congyang Write:

Quoted 17 lines

At 09/05/2011 02:13 AM, Michael S. Tsirkin Write:

Quoted 16 lines

Support bridge filtering on top of the memory
API as suggested by Avi Kivity:

Create a memory region for the bridge's address space. This region is not directly added to system_memory or its descendants. Devices under the bridge see this region as its pci_address_space(). The region is as large as the entire address space - it does not take into account any windows.

For each of the three windows (pref, non-pref, vga), create an alias with the appropriate start and size. Map the alias into the bridge's parent's pci_address_space(), as subregions.

Signed-off-by Michael S. Tsirkin <[email protected]>

---

Quoted 8 lines
Quoted 4 lines

The below seems to work fine for me so I applied this. Still need to test bridge filtering, any help with this appreciated.

I test bridge filtering, and the BAR still can be visible on guest even if I change the memory region.

Hi Michael S. Tsirkin:
I test pci bridge filtering on real hardware, and I find that I can mmap the resource after I change the memory base and memory limit(The BAR should be not visible on OS after changing the memory region).

So I try to write and read to the BAR. Here is my test result:
1. Before changing the pci bridge's memory region, I can read and write to the memory, and I can get the same value that I write.

2. After changing the pci bridge's memory region, I can still read and write to the memory, but it is very slow, and I can not get the same value that I write(The value is always 0).

Does this result means that pci bridge filtering works fine?

Thanks
Wen Congyang

#3 lore raw
Quoted 32 lines · Wen Congyang

At 09/14/2011 09:48 AM, Wen Congyang Write:

Quoted 17 lines

At 09/05/2011 02:13 AM, Michael S. Tsirkin Write:

Quoted 16 lines

Support bridge filtering on top of the memory
API as suggested by Avi Kivity:

Create a memory region for the bridge's address space. This region is not directly added to system_memory or its descendants. Devices under the bridge see this region as its pci_address_space(). The region is as large as the entire address space - it does not take into account any windows.

For each of the three windows (pref, non-pref, vga), create an alias with the appropriate start and size. Map the alias into the bridge's parent's pci_address_space(), as subregions.

Signed-off-by Michael S. Tsirkin<[email protected]>

---

Quoted 8 lines
Quoted 4 lines

The below seems to work fine for me so I applied this. Still need to test bridge filtering, any help with this appreciated.

I test bridge filtering, and the BAR still can be visible on guest even if I change the memory region.

Hi Michael S. Tsirkin:
I test pci bridge filtering on real hardware, and I find that I can mmap the resource after I change the memory base and memory limit(The BAR should be not visible on OS after changing the memory region).

Quoted 9 lines

So I try to write and read to the BAR. Here is my test result:
1. Before changing the pci bridge's memory region, I can read and write to the memory, and I can get the same value that I write.

2. After changing the pci bridge's memory region, I can still read and write to the memory, but it is very slow, and I can not get the same value that I write(The value is always 0).

Does this result means that pci bridge filtering works fine?

Yes. Instead of hitting the BAR, you hit the default mmio handler.

Signature · 1 line
error compiling committee.c: too many arguments to function
author
#6 lore raw
In reply to Wen Congyang
Quoted 36 lines · Wen Congyang

At 09/14/2011 09:48 AM, Wen Congyang Write:

Quoted 17 lines

At 09/05/2011 02:13 AM, Michael S. Tsirkin Write:

Quoted 16 lines

Support bridge filtering on top of the memory
API as suggested by Avi Kivity:

Create a memory region for the bridge's address space. This region is not directly added to system_memory or its descendants. Devices under the bridge see this region as its pci_address_space(). The region is as large as the entire address space - it does not take into account any windows.

For each of the three windows (pref, non-pref, vga), create an alias with the appropriate start and size. Map the alias into the bridge's parent's pci_address_space(), as subregions.

Signed-off-by Michael S. Tsirkin <[email protected]>

---

Quoted 8 lines
Quoted 4 lines

The below seems to work fine for me so I applied this. Still need to test bridge filtering, any help with this appreciated.

I test bridge filtering, and the BAR still can be visible on guest even if I change the memory region.

Hi Michael S. Tsirkin:
I test pci bridge filtering on real hardware, and I find that I can mmap the resource after I change the memory base and memory limit(The BAR should be not visible on OS after changing the memory region).

So I try to write and read to the BAR. Here is my test result:
1. Before changing the pci bridge's memory region, I can read and write to the memory, and I can get the same value that I write.

Quoted 7 lines

2. After changing the pci bridge's memory region, I can still read and write to the memory, but it is very slow, and I can not get the same value that I write(The value is always 0).

Does this result means that pci bridge filtering works fine?

Thanks
Wen Congyang

Sounds more or less right except I expect to get ffffffff and not 0. Avi, any idea?

Signature · 1 line
MST
#7 lore raw
Quoted 13 lines · Michael S. Tsirkin
Quoted 13 lines

Hi Michael S. Tsirkin:
I test pci bridge filtering on real hardware, and I find that I can mmap the resource after I change the memory base and memory limit(The BAR should be not visible on OS after changing the memory region).

So I try to write and read to the BAR. Here is my test result:
1. Before changing the pci bridge's memory region, I can read and write to the memory, and I can get the same value that I write.

2. After changing the pci bridge's memory region, I can still read and write to the memory, but it is very slow, and I can not get the same value that I write(The value is always 0).

Quoted 8 lines
Quoted 4 lines

Does this result means that pci bridge filtering works fine?

Thanks
Wen Congyang

Sounds more or less right except I expect to get ffffffff and not 0. Avi, any idea?

No, what does the default handler do?

Signature · 1 line
error compiling committee.c: too many arguments to function
author
#4 lore raw
In reply to Avi Kivity
Quoted 37 lines · Avi Kivity
Quoted 36 lines · Wen Congyang

At 09/14/2011 09:48 AM, Wen Congyang Write:

Quoted 17 lines

At 09/05/2011 02:13 AM, Michael S. Tsirkin Write:

Quoted 16 lines

Support bridge filtering on top of the memory
API as suggested by Avi Kivity:

Create a memory region for the bridge's address space. This region is not directly added to system_memory or its descendants. Devices under the bridge see this region as its pci_address_space(). The region is as large as the entire address space - it does not take into account any windows.

For each of the three windows (pref, non-pref, vga), create an alias with the appropriate start and size. Map the alias into the bridge's parent's pci_address_space(), as subregions.

Signed-off-by Michael S. Tsirkin<[email protected]>

---

Quoted 8 lines
Quoted 4 lines

The below seems to work fine for me so I applied this. Still need to test bridge filtering, any help with this appreciated.

I test bridge filtering, and the BAR still can be visible on guest even if I change the memory region.

Hi Michael S. Tsirkin:
I test pci bridge filtering on real hardware, and I find that I can mmap the resource after I change the memory base and memory limit(The BAR should be not visible on OS after changing the memory region).

So I try to write and read to the BAR. Here is my test result:
1. Before changing the pci bridge's memory region, I can read and write to the memory, and I can get the same value that I write.

Quoted 7 lines
Quoted 5 lines

2. After changing the pci bridge's memory region, I can still read and write to the memory, but it is very slow, and I can not get the same value that I write(The value is always 0).

Does this result means that pci bridge filtering works fine?

Yes. Instead of hitting the BAR, you hit the default mmio handler.

Hmm, not sure what's right in that case. But, same if BAR is disabled? Would be nice to make some handler in bridge to get called, to set master abort flag etc.

Quoted 2 lines
Signature · 1 line
error compiling committee.c: too many arguments to function
#5 lore raw
Quoted 7 lines · Michael S. Tsirkin
Quoted 2 lines

Yes. Instead of hitting the BAR, you hit the default mmio handler.

Hmm, not sure what's right in that case. But, same if BAR is disabled? Would be nice to make some handler in bridge to get called, to set master abort flag etc.

Put an mmio region spanning the entire pci address range as a child of the pci address space. Make sure it has lower priority than any of the BARs (or vga areas).

Signature · 1 line
error compiling committee.c: too many arguments to function