On Thu, Jun 11, 2015 at 12:48:22PM -0400, Kevin O'Connor wrote: > On Thu, Jun 11, 2015 at 04:35:33PM +0200, Laszlo Ersek wrote: > > On 06/11/15 15:58, Kevin O'Connor wrote: > > > On Thu, Jun 11, 2015 at 04:37:08PM +0300, Marcel Apfelbaum wrote: > > >> The fixes solves the following issue: > > >> The PXB device exposes a new pci root bridge with the > > >> fw path: /pci-root@4/..., in which 4 is the root bus number. > > >> Before this patch the fw path was wrongly computed: > > >> /pci-root@1/pci@i0cf8/... > > >> Fix the above issues: Correct the bus number and remove the > > >> extra host bridge description. > > > > > > Why is that wrong? The previous path looks correct to me. > > > > > >> The IEEE Std 1275-1994: > > >> > > >> IEEE Standard for Boot (Initialization Configuration) > > >> Firmware: Core Requirements and Practices > > >> 3.2.1.1 Node names > > >> Each node in the device tree is identified by a node name > > >> using the following notation: > > >> driver-name@unit-address:device-arguments > > >> > > >> The driver name field is a sequence of between one and 31 > > >> letters [...]. By convention, this name includes the name of > > >> the device’s manufacturer and the device’s model name > > >> separated by > > >> a “,”. > > >> > > >> The unit address field is the text representation of the > > >> physical address of the device within the address space > > >> defined by its parent node. The form of the text > > >> representation is bus-dependent. > > > > > > Note the "physical address" part in the above. Your patch changes the > > > "pci-root@" syntax to use a logical address instead of a physical > > > address. That is, unless I've missed something, SeaBIOS today uses a > > > physical address (the n'th root bus) and the patch would change it to > > > use a logical address. > > > > > > One of the goals of using an "openfirmware" like address was so that > > > they would be stable across boots (the same mechanism is also used > > > with coreboot). Using a physical address is key for this, because > > > simply adding or removing a PCI device could cause the logical PCI > > > bridge enumeration to change - and that would mess up the bootorder > > > list if it was based on logical addresses. > > > > There are two questions here. The first is the inclusion of the > > "pci@i0cf8" node even if a "pci-root@x" node is present in front of it. > > The hunk that changes that is not your main concern, right? (And Marcel > > just described that hunk in more detail.) > > > > The other question is how "x" is selected in "pci-root@x". > > > > On the QEMU side, and in OVMF, "x" is keyed off of the bus_nr property. > > If you change that property from (say) 3 to 4, then the device paths > > exported by QEMU will change. However, the location (in the PCI > > hierarchy) of all the affected devices will *also* change at once, and > > their auto-enumerated, firmware-side device paths will reflect that. > > Therefore the new "bootorder" fw_cfg entries will match the freshly > > generated firmware-side device paths. > > > > So why is this not stable? If you change the hardware without > > automatically updating any stashed firmware-side device paths, then > > things will fall apart without "bootorder" entries in the picture anyway. > > > > Also, assuming you key off "x" of the running counter that counts root > > buses as they are found during enumeration, that's a possibility too, > > but I don't see how it gives more stability. If you insert a new root > > bus (with a device on it) between to preexistent ones, that will offset > > all the "x" values for the root buses that come after it by one. > > The SeaBIOS code is used on both virtual machines and real machines. > The bus number is something that is generated by software and it is > not assured to be stable between boots. (For example, if someone adds > a PCI device to their machine between boots then every bus number in > the system might be different on the next boot.) The open firmware > paths go to great length to avoid arbitrary bus numbers today - for > example: > > /pci@i0cf8/pci-bridge@1/usb@1,2/hub@3/storage@1/channel@0/disk@0,0 > > Given the complexity to avoid arbitrary bus numbers I'm confused why > one would want to add them.
Could you give an example real-hardware path when there are multiple roots though? I'd like to make sure what qemu generates matches that. > > In UEFI at least (I'm not speaking about OVMF in particular, but the > > UEFI spec), there is a "short-form device path" concept for hard drive > > and USB boot options. For hard disks, it is practically a relative > > device path that lacks the path fragment from the root node until just > > before the GPT partition identifier. The idea being, if you plug your > > SCSI controller in another PCI slot, the change in the full device path > > will be local to the path fragment that is not captured in the > > (persistent) boot option. The GPT GUID can identify the partition > > uniquely in the system wherever it exists, so it can be booted even > > without fully enumerating all devices and reproducing all the default > > boot options. > > > > Short of such a "uniquely identifying relative devpath" trick, I don't > > think stability in firmware-stashed (ie. not regenerated) device paths > > exists in general, if the underlying hardware configuration is changed. > > I'm not sure why you say that - it works just fine. The open firmware > device paths relate a physical path to the given hardware and as long > as one doesn't alter that physical path it will be the same path on > every boot. (Specifically, one can add or remove unrelated PCI > devices, USB devices, etc. without impacting the open firmware paths > to devices not modified.) > > > In summary: I think we could modify both QEMU and OVMF to use the > > "serial numbers" of the extra PCI root buses, in increasing bus number > > order, instead of their actual bus numbers, for identifying them. That's > > just a convention. Then the second hunk of this patch would not be > > necessary for SeaBIOS. But I think this convention would be only less > > logical, and not more stable. > > > > Can you please elaborate? I'm confused. > > -Kevin
