| Message ID | 20190122125133.1191-1-david@redhat.com |
|---|---|
| Headers | show |
| Series | s390x/pci: PCI bridge plugging fixes | expand |
On Tue, 22 Jan 2019 13:51:31 +0100 David Hildenbrand <david@redhat.com> 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...
On 22.01.19 14:01, Cornelia Huck wrote: > On Tue, 22 Jan 2019 13:51:31 +0100 > David Hildenbrand <david@redhat.com> 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. 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.
On Wed, 23 Jan 2019 09:02:34 +0100 David Hildenbrand <david@redhat.com> wrote: > On 22.01.19 14:01, Cornelia Huck wrote: > > On Tue, 22 Jan 2019 13:51:31 +0100 > > David Hildenbrand <david@redhat.com> 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 :)
On Tue, 22 Jan 2019 13:51:31 +0100 David Hildenbrand <david@redhat.com> 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(-) > So, if I understand the discussion correctly: - Patch 1 is fine, just needs a description tweak. - Patch 2 fixes a crash, and certainly makes things better. However, current handling seems to be at least a bit odd, if not broken; something to be looked into later. I'd like to queue these, can I get an ack/feedback from IBM?