From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56861) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z46k9-0002xg-Qi for qemu-devel@nongnu.org; Sun, 14 Jun 2015 08:10:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z46k6-0005xF-J2 for qemu-devel@nongnu.org; Sun, 14 Jun 2015 08:10:29 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38582) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z46k6-0005wK-B8 for qemu-devel@nongnu.org; Sun, 14 Jun 2015 08:10:26 -0400 Date: Sun, 14 Jun 2015 14:10:22 +0200 From: "Michael S. Tsirkin" Message-ID: <20150614140916-mutt-send-email-mst@redhat.com> References: <1434029828-31954-1-git-send-email-marcel@redhat.com> <20150611135841.GA7998@morn.localdomain> <55799CB5.5090502@redhat.com> <20150611164822.GA20655@morn.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20150611164822.GA20655@morn.localdomain> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH V2] pci: fixes to allow booting from extra root pci buses. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin O'Connor Cc: Marcel Apfelbaum , seabios@seabios.org, Laszlo Ersek , qemu-devel@nongnu.org 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. > > >=20 > > > Why is that wrong? The previous path looks correct to me. > > >=20 > > >> 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 nam= e > > >> using the following notation: > > >> driver-name@unit-address:device-arguments > > >> > > >> The driver name field is a sequence of between one and 3= 1 > > >> letters [...]. By convention, this name includes the nam= e of > > >> the device=E2=80=99s manufacturer and the device=E2=80=99= s model name separated by > > >> a =E2=80=9C,=E2=80=9D. > > >> > > >> 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. > > >=20 > > > 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. > > >=20 > > > One of the goals of using an "openfirmware" like address was so tha= t > > > 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. > >=20 > > 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 i= t. > > The hunk that changes that is not your main concern, right? (And Marc= el > > just described that hunk in more detail.) > >=20 > > The other question is how "x" is selected in "pci-root@x". > >=20 > > On the QEMU side, and in OVMF, "x" is keyed off of the bus_nr propert= y. > > 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, an= d > > 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. > >=20 > > 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 any= way. > >=20 > > Also, assuming you key off "x" of the running counter that counts roo= t > > 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 offs= et > > all the "x" values for the root buses that come after it by one. >=20 > 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: >=20 > /pci@i0cf8/pci-bridge@1/usb@1,2/hub@3/storage@1/channel@0/disk@0,0 >=20 > 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 driv= e > > and USB boot options. For hard disks, it is practically a relative > > device path that lacks the path fragment from the root node until jus= t > > before the GPT partition identifier. The idea being, if you plug your > > SCSI controller in another PCI slot, the change in the full device pa= th > > 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. > >=20 > > Short of such a "uniquely identifying relative devpath" trick, I don'= t > > think stability in firmware-stashed (ie. not regenerated) device path= s > > exists in general, if the underlying hardware configuration is change= d. >=20 > 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.) >=20 > > 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 numbe= r > > order, instead of their actual bus numbers, for identifying them. Tha= t'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. > >=20 > > Can you please elaborate? I'm confused. >=20 > -Kevin