From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44474) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZhfjT-0003eO-K4 for qemu-devel@nongnu.org; Thu, 01 Oct 2015 11:25:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZhfjP-0001wx-FN for qemu-devel@nongnu.org; Thu, 01 Oct 2015 11:25:19 -0400 Received: from mx1.redhat.com ([209.132.183.28]:32950) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZhfjP-0001wh-8E for qemu-devel@nongnu.org; Thu, 01 Oct 2015 11:25:15 -0400 References: <1443701677-13629-1-git-send-email-markmb@redhat.com> <1443701819-13855-1-git-send-email-markmb@redhat.com> <1443701819-13855-7-git-send-email-markmb@redhat.com> From: Laszlo Ersek Message-ID: <560D5057.6010308@redhat.com> Date: Thu, 1 Oct 2015 17:25:11 +0200 MIME-Version: 1.0 In-Reply-To: <1443701819-13855-7-git-send-email-markmb@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4 6/7] Make the kernel image in the fw_cfg DMA interface bootable List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?Q?Marc_Mar=c3=ad?= , qemu-devel@nongnu.org Cc: "Gabriel L. Somlo" , Stefan Hajnoczi , Drew , Kevin O'Connor , Gerd Hoffmann On 10/01/15 14:16, Marc Mar=C3=AD wrote: > Add an entry to the bootorder file with name "vmlinux". > Give this entry more priority than the romfile. >=20 > Signed-off-by: Marc Mar=C3=AD > --- > hw/i386/pc.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) >=20 > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index 81d93b4..c4c51f7 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -1012,8 +1012,10 @@ static void load_linux(PCMachineState *pcms, > fw_cfg_add_bytes(fw_cfg, FW_CFG_SETUP_DATA, setup, setup_size); > =20 > option_rom[nb_option_roms].name =3D "linuxboot.bin"; > - option_rom[nb_option_roms].bootindex =3D 0; > + option_rom[nb_option_roms].bootindex =3D 1; > nb_option_roms++; > + > + add_boot_device_path(0, NULL, "vmlinux"); > } > =20 > #define NE2000_NB_MAX 6 >=20 Where does this idea come from? This will yet again break the invariant that the bootorder fw_cfg file is a list of OpenFirmware device paths. (The other annoying offender being "HALT", which caused me huge grief in the OVMF OpenFirmware devpath parser parser, when libvirt decided that "-boot strict=3Don" would become default.) OVMF (and AAVMF) have been able to boot kernels directly from fw_cfg for quite some time now, without the above change. They look at the fw_cfg key 0x0008 (FW_CFG_KERNEL_SIZE). Direct kernel boot is being requested iff the (little endian encoded) uint32 value is nonzero. In QEMU, this role of FW_CFG_KERNEL_SIZE is true for: - arm_load_kernel_notify() [hw/arm/boot.c], relied upon by AAVMF, - load_multiboot() [hw/i386/multiboot.c] and load_linux() [hw/i386/pc.c], relied upon by OVMF, - "hw/ppc/mac_newworld.c", "hw/ppc/mac_oldworld.c", "hw/sparc/sun4m.c", and "hw/sparc64/sun4u.c", relied upon by whatever boot firmware they have. Why is this necessary for SeaBIOS? ... I can see the function bootprio_find_vmlinux(), in SeaBIOS patch [PATCH v4 2/2] Boot Linux using QEMU fw_cfg DMA interface Given that direct kernel boot is always expected to take priority over anything else (which is ensured by this QEMU patch too), can bootprio_find_vmlinux() in SeaBIOS just look at the same fw_cfg key (0x0008)? I checked the QEMU_CFG_* macros in "src/fw/paravirt.c", and I think when SeaBIOS boots an fw_cfg kernel *now*, it doesn't do it with its own implementation; it probably launches the "linuxboot.bin" oprom (from QEMU -- "pc-bios/optionrom/linuxboot.S"). I vaguely recall that this assembly code has been deemed unwieldy for implementing the DMA interface (and I fully agree), which is why the above-referenced SeaBIOS patch adds the capability to SeaBIOS itself. I agree with that too. But, instead of messing up the "bootorder" fw_cfg file, can bootprio_find_vmlinux() look at the non-nullity of the QEMU_CFG_KERNEL_SIZE key? Such as: - read four bytes from under the fw_cfg selector QEMU_CFG_KERNEL_SIZE (0x0008), - if it is zero, return -1 --> no kernel boot requested, - if it is nonzero, return 0 --> which means "top priority". In other words, I agree with: > - option_rom[nb_option_roms].bootindex =3D 0; > + option_rom[nb_option_roms].bootindex =3D 1; in this patch, but I disagree with: > + add_boot_device_path(0, NULL, "vmlinux"); Thank you Laszlo