Re: [PATCH] spapr_pci: Robustify support of PCI bridges

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2850 bytes --]

On Thu, Jul 16, 2020 at 04:57:54PM +0200, Greg Kurz wrote:
> On Thu, 16 Jul 2020 16:23:52 +0200
> Markus Armbruster <armbru@redhat.com> wrote:
> 
> > David Gibson <david@gibson.dropbear.id.au> writes:
> > 
> > > On Thu, Jul 16, 2020 at 12:32:44PM +0200, Greg Kurz wrote:
> > >> On Thu, 16 Jul 2020 14:45:40 +1000
> > >> David Gibson <david@gibson.dropbear.id.au> wrote:
> > >> 
> > >> > On Thu, Jul 09, 2020 at 07:12:47PM +0200, Greg Kurz wrote:
> > >> > > Some recent error handling cleanups unveiled issues with our support of
> > >> > > PCI bridges:
> > >> > > 
> > >> > > 1) QEMU aborts when using non-standard PCI bridge types,
> > >> > >    unveiled by commit 7ef1553dac "spapr_pci: Drop some dead error handling"
> > >> > > 
> > >> > > $ qemu-system-ppc64 -M pseries -device pcie-pci-bridge
> > >> > > Unexpected error in object_property_find() at qom/object.c:1240:
> > >> > > qemu-system-ppc64: -device pcie-pci-bridge: Property '.chassis_nr' not found
> > >> > > Aborted (core dumped)
> > >> > 
> > >> > Oops, I thought we had a check that we actually had a "pci-bridge"
> > >> > device before continuing with the hotplug, but I guess not.
> > >> 
> > >> Ah... are you suggesting we should explicitly check the actual type
> > >> of the bridge rather than looking for the "chassis_nr" property ?
> > >
> > > Uh.. I thought about it, but I don't think it matters much which way
> > > we do it.
> > 
> > Would it make sense to add the "chassis_nr" property to *all* PCI
> > bridge devices?
> > 
> 
> I see that the "PCI Express to PCI/PCI-X Bridge Specification" mentions
> a "Chassis Number Register" which looks very similar to the what exists
> in standard PCI-to-PCI brdiges. This doesn't seem to be implemented in
> our "pcie-pci-bridge" device model though, but of course I have no idea
> why :)

We could consider it, but I don't think there's a lot to be gained by
it at this stage.  I don't think there's really any reason to want to
use bridges other than plain "pci-bridge" on the pseries machine.

PCI is a bit weird on pseries, since it's explicitly paravirt.
Although you can use extended config space, and thereby PCI-E devices
on it, the topology really looks pretty much identical to vanilla
PCI.  So, I don't think there's any reason to use PCI-E bridges on
pseries.

Other than PCI-E bridges of various sorts, a quick scan suggests all
the other bridge types in qemu are weird variants that are mostly
specific to some particular platform.  I don't see any reason we'd
want those on pseries either.

> Maybe Michael or Marcel (cc'd) can share some thoughts about that ?

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]