Skip to contentSkip to conversation

pci: pass bridge update to secondary bus

PATCH In review Michael S. Tsirkin 3 participants 5 messages Jul 6, 2010 – Jul 8, 2010

lore
Share & tools
lore

Michael S. Tsirkin

author
#0 lore raw

PATCH 1/1 pci: pass bridge update to secondary bus

bridge config write should trigger updates
on the secondary bus. never on the primary bus.

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

---

Compile-tested only.
Isaku Yamahata, could you review this please?
You wrote the code, and you seem to have some bridged setups.

 hw/pci.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
hw/pci.c +3 -1
index 926cf63..011d83e 100644
@@ -1513,7 +1513,9 @@ static void pci_bridge_write_config(PCIDevice *d,
         /* memory base/limit, prefetchable base/limit and
            io base/limit upper 16 */
         ranges_overlap(address, len, PCI_MEMORY_BASE, 20)) {
-        pci_bridge_update_mappings(d->bus);
+        PCIBridge *s = container_of(d, PCIBridge, dev);
+        PCIBus *secondary_bus = &s->bus;
+        pci_bridge_update_mappings(secondary_bus);
     }
 }
Signature · 1 line
1.7.2.rc0.14.g41c1c
#1 lore raw
Quoted 9 lines · Michael S. Tsirkin

bridge config write should trigger updates
on the secondary bus. never on the primary bus.

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

---

Compile-tested only.
Isaku Yamahata, could you review this please?
You wrote the code, and you seem to have some bridged setups.

The code looks good.

Should PCIBridge::bus be renamed to something like PCIBridge::secondary_bus (or sec_bus for short) in order to avoid confusion?
The redundant local variable, secondary_bus, was deliberately introduced to emphasize that it's a secondary bus. And I was confused by that.
Anyway such a change should be done by another patch.

Quoted 22 lines
 hw/pci.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
hw/pci.c +3 -1
index 926cf63..011d83e 100644
@@ -1513,7 +1513,9 @@ static void pci_bridge_write_config(PCIDevice *d,
         /* memory base/limit, prefetchable base/limit and
            io base/limit upper 16 */
         ranges_overlap(address, len, PCI_MEMORY_BASE, 20)) {
-        pci_bridge_update_mappings(d->bus);
+        PCIBridge *s = container_of(d, PCIBridge, dev);
+        PCIBus *secondary_bus = &s->bus;
+        pci_bridge_update_mappings(secondary_bus);
     }
 }
Signature · 1 line
1.7.2.rc0.14.g41c1c
Signature · 1 line
yamahata
#3 lore raw
In reply to Michael S. Tsirkin
Quoted 2 lines · Michael S. Tsirkin

bridge config write should trigger updates
on the secondary bus. never on the primary bus.

If this is true, shouldn't updates happen on all buses from secondary to subordinate? Do we know which of these are immediately below primary bus?

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

---

Compile-tested only.
Isaku Yamahata, could you review this please?
You wrote the code, and you seem to have some bridged setups.

 hw/pci.c |    4 +++-

 1 files changed, 3 insertions(+), 1 deletions(-)
hw/pci.c +0 -0
index 926cf63..011d83e 100644
@@ -1513,7 +1513,9 @@ static void pci_bridge_write_config(PCIDevice *d,
         /* memory base/limit, prefetchable base/limit and

           io base/limit upper 16 */ ranges_overlap(address, len, PCI_MEMORY_BASE, 20)) {
-        pci_bridge_update_mappings(d->bus);
+        PCIBridge *s = container_of(d, PCIBridge, dev);
+        PCIBus *secondary_bus = &s->bus;
+        pci_bridge_update_mappings(secondary_bus); }
 }

--
1.7.2.rc0.14.g41c1c

author
#4 lore raw
Quoted 7 lines · Blue Swirl
Quoted 2 lines · Michael S. Tsirkin

bridge config write should trigger updates
on the secondary bus. never on the primary bus.

If this is true, shouldn't updates happen on all buses from secondary to subordinate? Do we know which of these are immediately below primary bus?

pci_bridge_update_mappings does this already.

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

---

Compile-tested only.
Isaku Yamahata, could you review this please?
You wrote the code, and you seem to have some bridged setups.

 hw/pci.c |    4 +++-

 1 files changed, 3 insertions(+), 1 deletions(-)
hw/pci.c +0 -0
index 926cf63..011d83e 100644
@@ -1513,7 +1513,9 @@ static void pci_bridge_write_config(PCIDevice *d,
         /* memory base/limit, prefetchable base/limit and

           io base/limit upper 16 */ ranges_overlap(address, len, PCI_MEMORY_BASE, 20)) {
-        pci_bridge_update_mappings(d->bus);
+        PCIBridge *s = container_of(d, PCIBridge, dev);
+        PCIBus *secondary_bus = &s->bus;
+        pci_bridge_update_mappings(secondary_bus); }
 }

--
1.7.2.rc0.14.g41c1c

author
#2 lore raw
In reply to Isaku Yamahata
Quoted 13 lines · Isaku Yamahata
Quoted 9 lines · Michael S. Tsirkin

bridge config write should trigger updates
on the secondary bus. never on the primary bus.

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

---

Compile-tested only.
Isaku Yamahata, could you review this please?
You wrote the code, and you seem to have some bridged setups.

The code looks good.

Quoted 7 lines

Should PCIBridge::bus be renamed to something like PCIBridge::secondary_bus (or sec_bus for short) in order to avoid confusion?
The redundant local variable, secondary_bus, was deliberately introduced to emphasize that it's a secondary bus. And I was confused by that.
Anyway such a change should be done by another patch.

Sure. Send a patch.

Quoted 25 lines
Quoted 22 lines
 hw/pci.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
hw/pci.c +3 -1
index 926cf63..011d83e 100644
@@ -1513,7 +1513,9 @@ static void pci_bridge_write_config(PCIDevice *d,
         /* memory base/limit, prefetchable base/limit and
            io base/limit upper 16 */
         ranges_overlap(address, len, PCI_MEMORY_BASE, 20)) {
-        pci_bridge_update_mappings(d->bus);
+        PCIBridge *s = container_of(d, PCIBridge, dev);
+        PCIBus *secondary_bus = &s->bus;
+        pci_bridge_update_mappings(secondary_bus);
     }
 }
Signature · 1 line
1.7.2.rc0.14.g41c1c
Signature · 1 line
yamahata