Re: [Qemu-devel] [Qemu-ppc] [PATCH] spapr/pci: populate PCI DT in reverse order

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

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

On Thu, Dec 17, 2015 at 09:43:29AM +0100, Greg Kurz wrote:
> On Thu, 3 Dec 2015 15:53:17 +0100
> Greg Kurz <gkurz@linux.vnet.ibm.com> wrote:
> 
> > On Tue, 1 Dec 2015 22:48:38 +0100
> > Thomas Huth <thuth@redhat.com> wrote:
> > 
> > > On 30/11/15 11:45, Greg Kurz wrote:
> > > > Since commit 1d2d974244c6 "spapr_pci: enumerate and add PCI device tree", QEMU
> > > > populates the PCI device tree in the opposite order compared to SLOF.
> > > > 
> > > > Before 1d2d974244c6:
> > > > 
> > > > Populating /pci@800000020000000
> > > >                      00 0000 (D) : 1af4 1000    virtio [ net ]
> > > >                      00 0800 (D) : 1af4 1001    virtio [ block ]
> > > >                      00 1000 (D) : 1af4 1009    virtio [ network ]
> > > > Populating /pci@800000020000000/unknown-legacy-device@2
> > > > 
> > > > 
> > > > 7e5294b8 :  /pci@800000020000000
> > > > 7e52b998 :  |-- ethernet@0
> > > > 7e52c0c8 :  |-- scsi@1
> > > > 7e52c7e8 :  +-- unknown-legacy-device@2 ok
> > > > 
> > > > Since 1d2d974244c6:
> > > > 
> > > > Populating /pci@800000020000000
> > > >                      00 1000 (D) : 1af4 1009    virtio [ network ]
> > > > Populating /pci@800000020000000/unknown-legacy-device@2
> > > >                      00 0800 (D) : 1af4 1001    virtio [ block ]
> > > >                      00 0000 (D) : 1af4 1000    virtio [ net ]
> > > > 
> > > > 
> > > > 7e5e8118 :  /pci@800000020000000
> > > > 7e5ea6a0 :  |-- unknown-legacy-device@2
> > > > 7e5eadb8 :  |-- scsi@1
> > > > 7e5eb4d8 :  +-- ethernet@0 ok
> > > > 
> > > > This behaviour change is not actually a bug since no assumptions should be
> > > > made on DT ordering. But it has no real justification either, other than
> > > > being the consequence of the way fdt_add_subnode() inserts new elements
> > > > to the front of the FDT rather than adding them to the tail.
> > > > 
> > > > This patch reverts to the historical SLOF ordering by walking PCI devices in
> > > > reverse order.
> > > 
> > > I've applied your patch here locally, and indeed, the device tree looks
> > > nicer to me, too, when the nodes are listed in ascending order.
> > > 
> > > Tested-by: Thomas Huth <thuth@redhat.com>
> > > 
> > > 
> > 
> 
> Ping ?

Sorry I didn't reply.

I'm still dubious about this.  It seems like a fair bit of effort to
restore a behaviour that the client isn't supposed to be relying on
anyway.

Plus, the version with the changed order is already released, so
applying this will mean a second behaviour change.

-- 
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: 819 bytes --]