Re: [qemu-s390x] [PATCH v1 0/2] s390x/pci: PCI bridge plugging fixes

[Cornelia Huck]

On Wed, 23 Jan 2019 09:02:34 +0100
David Hildenbrand <address@hidden> wrote:

> On 22.01.19 14:01, Cornelia Huck wrote:
> > On Tue, 22 Jan 2019 13:51:31 +0100
> > David Hildenbrand <address@hidden> wrote:
> >   
> >> Hotplugging of PCI bridges is right now pretty much broken. Coldplugging
> >> and hotplugging will assign wrong primary bus numbers in some scenarios.
> >>
> >> I base my knowledge on how this is supposed to work on
> >> http://www.science.unitn.it/~fiorella/guidelinux/tlk/node76.html
> >>
> >> I did a couple of tests, building whole hierarchies of bridges, both
> >> hot and coldplugged. "info pci" as well as the Linux guests showed
> >> what I was expecting.
> >>
> >> David Hildenbrand (2):
> >>   s390x/pci: Fix primary bus number for PCI bridges
> >>   s390x/pci: Fix hotplugging of PCI bridges
> >>
> >>  hw/s390x/s390-pci-bus.c | 41 +++++++++++++++++++++++++++--------------
> >>  1 file changed, 27 insertions(+), 14 deletions(-)
> >>  
> > 
> > I'll leave the actual review of this to folks familiar with how zPCI is
> > supposed to work :)
> > 
> > Does the guest actually see anything of this? Consistency is good, and
> > not crashing even better, but I think all of the topology is invisible
> > on the guest side anyway...
> >   
> 
> I am no PCI expert, but I think the guest is able to read/write these
> numbers via the configuration space. As far as I can see, on x86 the
> BIOS builds the topology. On Power/s390x this job is delegated to
> firmware / QEMU.
> 
> There are quite some numbers in QEMU relying on these numbers to be
> correct: E.g. pci_secondary_bus_in_range()
> 
> If the guest relies on the topology to be correct, things can go wrong.
> I have no idea how Linux guests actually use the topology.

Ok, if this shows up in the config space, it is guest visible. I was
thinking mostly about the zPCI enumeration, which results in a flat
layout.

> 
> Also, the output of "info pci" will be wrong.
> 
> Anyhow, this is the right thing to do, but I agree that Patch #1 might
> not be as critical as patch #2.

I'm certainly not arguing against inclusion :)