[Qemu-devel] [PATCH] pci: pass bridge update to secondary bus

[Qemu-devel] [PATCH] pci: pass bridge update to secondary bus

[Michael S. Tsirkin]

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

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

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(-)

diff --git a/hw/pci.c b/hw/pci.c
index 926cf63..011d83e 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -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

[Qemu-devel] Re: [PATCH] pci: pass bridge update to secondary bus

[Isaku Yamahata]

On Tue, Jul 06, 2010 at 02:23:27PM +0300, Michael S. Tsirkin wrote:
> bridge config write should trigger updates
> on the secondary bus. never on the primary bus.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> 
> 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.

> 
>  hw/pci.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/pci.c b/hw/pci.c
> index 926cf63..011d83e 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -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
> 

-- 
yamahata

Re: [Qemu-devel] [PATCH] pci: pass bridge update to secondary bus

[Blue Swirl]

On Tue, Jul 6, 2010 at 11:23 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> 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?

>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>
> 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(-)
>
> diff --git a/hw/pci.c b/hw/pci.c
> index 926cf63..011d83e 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -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
>
>

Re: [Qemu-devel] [PATCH] pci: pass bridge update to secondary bus

[Michael S. Tsirkin]

On Wed, Jul 07, 2010 at 05:31:39PM +0000, Blue Swirl wrote:
> On Tue, Jul 6, 2010 at 11:23 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > 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.

> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >
> > 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(-)
> >
> > diff --git a/hw/pci.c b/hw/pci.c
> > index 926cf63..011d83e 100644
> > --- a/hw/pci.c
> > +++ b/hw/pci.c
> > @@ -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
> >
> >

[Qemu-devel] Re: [PATCH] pci: pass bridge update to secondary bus

[Michael S. Tsirkin]

On Wed, Jul 07, 2010 at 11:04:51AM +0900, Isaku Yamahata wrote:
> On Tue, Jul 06, 2010 at 02:23:27PM +0300, Michael S. Tsirkin wrote:
> > bridge config write should trigger updates
> > on the secondary bus. never on the primary bus.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> > 
> > 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.

Sure. Send a patch.

> > 
> >  hw/pci.c |    4 +++-
> >  1 files changed, 3 insertions(+), 1 deletions(-)
> > 
> > diff --git a/hw/pci.c b/hw/pci.c
> > index 926cf63..011d83e 100644
> > --- a/hw/pci.c
> > +++ b/hw/pci.c
> > @@ -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
> > 
> 
> -- 
> yamahata