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.
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