[SLOF] [PATCH] pci: walk through the PCI DT in reverse order

[Greg Kurz]

On Fri, 27 Nov 2015 22:59:31 +0100
Thomas Huth <thuth at redhat.com> wrote:

> On 27/11/15 10:29, Greg Kurz wrote:
> > On Fri, 27 Nov 2015 09:47:56 +0530
> > Nikunj A Dadhania <nikunj at linux.vnet.ibm.com> wrote:
> > 
> >> David Gibson <david at gibson.dropbear.id.au> writes:
> >>
> >>> On Thu, Nov 26, 2015 at 08:26:50PM +0100, Thomas Huth wrote:
> >>>> On 26/11/15 17:34, Greg Kurz wrote:
> >>>>> With the same set of devices, if QEMU does not do the PCI Enumeration (old
> >>>>> QEMU that doesn't set the qemu,phb-enumerated property), we get:
> >>>>>
> >>>>> Populating /pci at 800000020000000
> >>>>>  Adapters on 0800000020000000
> >>>>>                      00 0800 (D) : 1af4 1000    virtio [ net ]
> >>>>>                      00 1000 (D) : 106b 003f    serial bus [ usb-ohci ]
> >>>>>                      00 1800 (D) : 1af4 1003    communication-controller*
> >>>>>                      00 2000 (D) : 1af4 1001    virtio [ block ]
> >>>>>                      00 2800 (D) : 1af4 1001    virtio [ block ]
> >>>>>                      00 3000 (D) : 1af4 1001    virtio [ block ]
> >>>>>                      00 3800 (D) : 1af4 1002    unknown-legacy-device*
> >>>>>
> >>>>> but in the case of a newer QEMU, when SLOF walks through the DT, we get:
> >>>>>
> >>>>> Populating /pci at 800000020000000
> >>>>>                      00 3800 (D) : 1af4 1002    unknown-legacy-device*
> >>>>>                      00 3000 (D) : 1af4 1001    virtio [ block ]
> >>>>>                      00 2800 (D) : 1af4 1001    virtio [ block ]
> >>>>>                      00 2000 (D) : 1af4 1001    virtio [ block ]
> >>>>>                      00 1800 (D) : 1af4 1003    communication-controller*
> >>>>>                      00 1000 (D) : 106b 003f    serial bus [ usb-ohci ]
> >>>>>                      00 0800 (D) : 1af4 1000    virtio [ net ]
> >>>>>
> >>>>> This is a confusing behaviour change for users. This patch fixes that: we
> >>>>> push all the child nodes to the stack and configure them in reverse
> >>>>> order.
> >>
> >> I liked your qemu patch that just uses _reverse variant of pci device
> >> find.
> >>
> >>>
> >>> Really?  I mean the change this causes in kernel discovery order I can
> >>> see the problem with, although you're not supposed to depend on
> >>> discovery order or default kernel naming.  Is the simple order of
> >>> listing the devices really important?
> >>>
> >>>> With your patch, the output during boot is in the right order, but the
> >>>> nodes in the device tree are still ordered wrong (type "dev /" and then
> >>>> "ls" at the SLOF prompt).
> >>>> Where does the wrong order come from? Is it QEMU who's doing this wrong
> >>>> already? Then it should maybe be rather fixed there?
> >>>
> >>> qemu did change the order things get emitted into the flattened tree
> >>> (because fdt_add_subnode() adds to the front of the list of children)
> >>> - but things are not supposed to rely on the order of things in the
> >>> fdt.  If you care about order you're supposed to sort them yourself
> >>> (on reg value, or whatever criterion makes sense).
> >>
> >> All this is coming from libvirt user not using boot_order in the
> >> commandline. Once that is used there is no confusion over the order that
> >> the user passes the devices.
> >>
> > 
> > Yes, this was my initial concern. With the new QEMU behaviour, users
> > who care *must* pass boot_order to virt-install, use <boot order> in
> > libvirt XML or pass bootindex to QEMU.
> > 
> > Since the <target dev> setting in libvirt has never been a strong
> > feature, and no assumptions should be made on the order of things in
> > the FDT, I guess we should just do nothing then :)
> 
> The ascending ordered device tree and firmware output IMHO still looked
> nicer... does something speak against fixing QEMU to put the nodes in
> the right order into the device tree?
> 
>  Thomas
> 

I already have a patch for that. I'll send it on monday to continue the
discussion on qemu-devel and qemu-ppc.

Cheers.

--
Greg