[PATCH 0/9] x86: Fixes for distro booting
This series collects together various fixes found whn testing out booting of distros using QEMU on x86. Together they improve functionality, but it is not perfect yet. Some notes are included as to the current state.
Simon Glass (9): x86: spl: Drop unwanted debug() video: Tidy up Makefile rule for video x86: Run QEMU machine setup in SPL Revert "x86: Switch QEMU over to use the bochs driver" board_f: Fix corruption of relocaddr x86: Correct copying of BIOS mode information video: Add a Kconfig option for SPL video handoff x86: Enable useful options for qemu-86 x86: Update qemu documentation
arch/x86/cpu/qemu/Kconfig | 2 +- arch/x86/cpu/qemu/qemu.c | 2 +- arch/x86/include/asm/qemu.h | 14 +++++ arch/x86/lib/bios.c | 2 +- arch/x86/lib/spl.c | 4 +- common/board_f.c | 4 +- configs/qemu-x86_64_defconfig | 4 ++ configs/qemu-x86_defconfig | 12 +++++ doc/board/emulation/qemu-x86.rst | 88 ++++++++++++++++++++++++++++++-- drivers/Makefile | 4 +- drivers/video/Kconfig | 10 ++++ drivers/video/video-uclass.c | 2 +- include/vesa.h | 4 +- 13 files changed, 137 insertions(+), 15 deletions(-) create mode 100644 arch/x86/include/asm/qemu.h
This was left over from some previous debugging. Drop it.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/x86/lib/spl.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/arch/x86/lib/spl.c b/arch/x86/lib/spl.c index b6812bb8ca2b..55c061570864 100644 --- a/arch/x86/lib/spl.c +++ b/arch/x86/lib/spl.c @@ -137,7 +137,6 @@ static int x86_spl_init(void) }
#ifndef CONFIG_SYS_COREBOOT - log_debug("bss\n"); debug("BSS clear from %lx to %lx len %lx\n", (ulong)&__bss_start, (ulong)&__bss_end, (ulong)&__bss_end - (ulong)&__bss_start); memset(&__bss_start, 0, (ulong)&__bss_end - (ulong)&__bss_start);
On Mon, Jul 24, 2023 at 10:52 PM Simon Glass sjg@chromium.org wrote:
This was left over from some previous debugging. Drop it.
Signed-off-by: Simon Glass sjg@chromium.org
arch/x86/lib/spl.c | 1 - 1 file changed, 1 deletion(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Drop the duplication and add a single rule which can handle SPL as well.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/Makefile b/drivers/Makefile index 3bc6d279d7cb..7272681fc741 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -39,6 +39,8 @@ obj-$(CONFIG_$(SPL_TPL_)TPM) += tpm/ obj-$(CONFIG_$(SPL_)NVME) += nvme/ obj-$(CONFIG_XEN) += xen/ obj-$(CONFIG_$(SPL_)FPGA) += fpga/ +obj-$(CONFIG_$(SPL_TPL_)VIDEO) += video/ + obj-y += bus/
ifndef CONFIG_TPL_BUILD @@ -64,7 +66,6 @@ obj-$(CONFIG_SPL_USB_HOST) += usb/host/ obj-$(CONFIG_SPL_SATA) += ata/ scsi/ obj-$(CONFIG_SPL_LEGACY_BLOCK) += block/ obj-$(CONFIG_SPL_THERMAL) += thermal/ -obj-$(CONFIG_SPL_VIDEO) +=video/
endif endif @@ -99,7 +100,6 @@ obj-y += rtc/ obj-y += scsi/ obj-y += sound/ obj-y += spmi/ -obj-y += video/ obj-y += watchdog/ obj-$(CONFIG_QE) += qe/ obj-$(CONFIG_U_QE) += qe/
On 24/07/23 20:21, Simon Glass wrote:
Drop the duplication and add a single rule which can handle SPL as well.
Signed-off-by: Simon Glasssjg@chromium.org
drivers/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions
Reviewed-by: Nikhil M Jain n-jain1@ti.com
Hi Simon,
On Mon, Jul 24, 2023 at 10:52 PM Simon Glass sjg@chromium.org wrote:
Drop the duplication and add a single rule which can handle SPL as well.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/Makefile b/drivers/Makefile index 3bc6d279d7cb..7272681fc741 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -39,6 +39,8 @@ obj-$(CONFIG_$(SPL_TPL_)TPM) += tpm/ obj-$(CONFIG_$(SPL_)NVME) += nvme/ obj-$(CONFIG_XEN) += xen/ obj-$(CONFIG_$(SPL_)FPGA) += fpga/ +obj-$(CONFIG_$(SPL_TPL_)VIDEO) += video/
This should be $(SPL_) as the original codes do not enable video build for TPL
obj-y += bus/
ifndef CONFIG_TPL_BUILD @@ -64,7 +66,6 @@ obj-$(CONFIG_SPL_USB_HOST) += usb/host/ obj-$(CONFIG_SPL_SATA) += ata/ scsi/ obj-$(CONFIG_SPL_LEGACY_BLOCK) += block/ obj-$(CONFIG_SPL_THERMAL) += thermal/ -obj-$(CONFIG_SPL_VIDEO) +=video/
endif endif @@ -99,7 +100,6 @@ obj-y += rtc/ obj-y += scsi/ obj-y += sound/ obj-y += spmi/ -obj-y += video/ obj-y += watchdog/ obj-$(CONFIG_QE) += qe/ obj-$(CONFIG_U_QE) += qe/ --
Regards, Bin
Hi Bin,
On Fri, 28 Jul 2023 at 09:46, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Mon, Jul 24, 2023 at 10:52 PM Simon Glass sjg@chromium.org wrote:
Drop the duplication and add a single rule which can handle SPL as well.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/Makefile b/drivers/Makefile index 3bc6d279d7cb..7272681fc741 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -39,6 +39,8 @@ obj-$(CONFIG_$(SPL_TPL_)TPM) += tpm/ obj-$(CONFIG_$(SPL_)NVME) += nvme/ obj-$(CONFIG_XEN) += xen/ obj-$(CONFIG_$(SPL_)FPGA) += fpga/ +obj-$(CONFIG_$(SPL_TPL_)VIDEO) += video/
This should be $(SPL_) as the original codes do not enable video build for TPL
Yes, but SPL_TPL supports that, in that TPL_VIDEO is not enabled, so it will not be built for TPL. In general, we never need the $(SPL) variant.
Regards, Simon
Call the hardware-init function from QEMU from SPL. This allows the video BIOS to operate correctly.
Create an x86-wide qemu.h header to avoid having to #ifdef the header in spl.c
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/x86/cpu/qemu/qemu.c | 2 +- arch/x86/include/asm/qemu.h | 14 ++++++++++++++ arch/x86/lib/spl.c | 3 +++ 3 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 arch/x86/include/asm/qemu.h
diff --git a/arch/x86/cpu/qemu/qemu.c b/arch/x86/cpu/qemu/qemu.c index 274978c023b6..70414556086c 100644 --- a/arch/x86/cpu/qemu/qemu.c +++ b/arch/x86/cpu/qemu/qemu.c @@ -48,7 +48,7 @@ static void enable_pm_ich9(void) pci_write_config32(ICH9_PM, PMBA, CONFIG_ACPI_PM1_BASE | 1); }
-static void qemu_chipset_init(void) +void qemu_chipset_init(void) { u16 device, xbcs; int pam, i; diff --git a/arch/x86/include/asm/qemu.h b/arch/x86/include/asm/qemu.h new file mode 100644 index 000000000000..f1e95ffd7a4d --- /dev/null +++ b/arch/x86/include/asm/qemu.h @@ -0,0 +1,14 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Generic QEMU header + * + * Copyright 2023 Google LLC + */ + +#ifndef __QEMU_H +#define __QEMU_H + +/* set up the chipset for QEMU so that video can be used */ +void qemu_chipset_init(void); + +#endif diff --git a/arch/x86/lib/spl.c b/arch/x86/lib/spl.c index 55c061570864..74949b2c13cb 100644 --- a/arch/x86/lib/spl.c +++ b/arch/x86/lib/spl.c @@ -27,6 +27,7 @@ #include <asm/mtrr.h> #include <asm/pci.h> #include <asm/processor.h> +#include <asm/qemu.h> #include <asm/spl.h> #include <asm-generic/sections.h>
@@ -291,6 +292,8 @@ void spl_board_init(void) #ifndef CONFIG_TPL preloader_console_init(); #endif + if (IS_ENABLED(CONFIG_X86) && IS_ENABLED(CONFIG_QEMU)) + qemu_chipset_init();
if (CONFIG_IS_ENABLED(VIDEO)) { struct udevice *dev;
On Mon, Jul 24, 2023 at 10:52 PM Simon Glass sjg@chromium.org wrote:
Call the hardware-init function from QEMU from SPL. This allows the video BIOS to operate correctly.
Create an x86-wide qemu.h header to avoid having to #ifdef the header in spl.c
Signed-off-by: Simon Glass sjg@chromium.org
arch/x86/cpu/qemu/qemu.c | 2 +- arch/x86/include/asm/qemu.h | 14 ++++++++++++++ arch/x86/lib/spl.c | 3 +++ 3 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 arch/x86/include/asm/qemu.h
diff --git a/arch/x86/cpu/qemu/qemu.c b/arch/x86/cpu/qemu/qemu.c index 274978c023b6..70414556086c 100644 --- a/arch/x86/cpu/qemu/qemu.c +++ b/arch/x86/cpu/qemu/qemu.c @@ -48,7 +48,7 @@ static void enable_pm_ich9(void) pci_write_config32(ICH9_PM, PMBA, CONFIG_ACPI_PM1_BASE | 1); }
-static void qemu_chipset_init(void) +void qemu_chipset_init(void) { u16 device, xbcs; int pam, i; diff --git a/arch/x86/include/asm/qemu.h b/arch/x86/include/asm/qemu.h new file mode 100644 index 000000000000..f1e95ffd7a4d --- /dev/null +++ b/arch/x86/include/asm/qemu.h @@ -0,0 +1,14 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/*
- Generic QEMU header
- Copyright 2023 Google LLC
- */
+#ifndef __QEMU_H +#define __QEMU_H
+/* set up the chipset for QEMU so that video can be used */ +void qemu_chipset_init(void);
+#endif diff --git a/arch/x86/lib/spl.c b/arch/x86/lib/spl.c index 55c061570864..74949b2c13cb 100644 --- a/arch/x86/lib/spl.c +++ b/arch/x86/lib/spl.c @@ -27,6 +27,7 @@ #include <asm/mtrr.h> #include <asm/pci.h> #include <asm/processor.h> +#include <asm/qemu.h> #include <asm/spl.h> #include <asm-generic/sections.h>
@@ -291,6 +292,8 @@ void spl_board_init(void) #ifndef CONFIG_TPL preloader_console_init(); #endif
if (IS_ENABLED(CONFIG_X86) && IS_ENABLED(CONFIG_QEMU))
CONFIG_X86 is not necessary.
qemu_chipset_init(); if (CONFIG_IS_ENABLED(VIDEO)) { struct udevice *dev;--
Otherwise, Reviewed-by: Bin Meng bmeng.cn@gmail.com
Unfortunately the bochs driver does not currently work with distros. It causes a hang sometime between grub menu selection and the OS displaying something.
This reverts commit b8956425d525c3c25fd218f252f89a5e44df6a9f.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/x86/cpu/qemu/Kconfig | 2 +- configs/qemu-x86_64_defconfig | 4 ++++ configs/qemu-x86_defconfig | 3 +++ 3 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/x86/cpu/qemu/Kconfig b/arch/x86/cpu/qemu/Kconfig index aa329b0dab29..f8f2f6473088 100644 --- a/arch/x86/cpu/qemu/Kconfig +++ b/arch/x86/cpu/qemu/Kconfig @@ -12,7 +12,7 @@ config QEMU imply SYS_NS16550 imply USB imply USB_EHCI_HCD - imply VIDEO_BOCHS + imply VIDEO_VESA
if QEMU
diff --git a/configs/qemu-x86_64_defconfig b/configs/qemu-x86_64_defconfig index 4a15c516710e..b1400876a2ff 100644 --- a/configs/qemu-x86_64_defconfig +++ b/configs/qemu-x86_64_defconfig @@ -84,6 +84,10 @@ CONFIG_SPL_DM_RTC=y CONFIG_SYS_NS16550_PORT_MAPPED=y CONFIG_SPI=y CONFIG_USB_KEYBOARD=y +CONFIG_SPL_VIDEO=y +CONFIG_FRAMEBUFFER_SET_VESA_MODE=y +CONFIG_FRAMEBUFFER_VESA_MODE_USER=y +CONFIG_FRAMEBUFFER_VESA_MODE=0x144 CONFIG_CONSOLE_SCROLL_LINES=5 CONFIG_FAT_BLK_XLATE=y # CONFIG_SPL_USE_TINY_PRINTF is not set diff --git a/configs/qemu-x86_defconfig b/configs/qemu-x86_defconfig index 95a6ff9ae799..56788cd185fd 100644 --- a/configs/qemu-x86_defconfig +++ b/configs/qemu-x86_defconfig @@ -53,6 +53,9 @@ CONFIG_NVME_PCI=y CONFIG_SYS_NS16550_PORT_MAPPED=y CONFIG_SPI=y CONFIG_USB_KEYBOARD=y +CONFIG_FRAMEBUFFER_SET_VESA_MODE=y +CONFIG_FRAMEBUFFER_VESA_MODE_USER=y +CONFIG_FRAMEBUFFER_VESA_MODE=0x144 CONFIG_CONSOLE_SCROLL_LINES=5 CONFIG_GENERATE_ACPI_TABLE=y # CONFIG_GZIP is not set
Hi Simon,
On Mon, Jul 24, 2023 at 10:52 PM Simon Glass sjg@chromium.org wrote:
Unfortunately the bochs driver does not currently work with distros. It causes a hang sometime between grub menu selection and the OS displaying something.
Does this reproduce reliably?
This reverts commit b8956425d525c3c25fd218f252f89a5e44df6a9f.
Signed-off-by: Simon Glass sjg@chromium.org
arch/x86/cpu/qemu/Kconfig | 2 +- configs/qemu-x86_64_defconfig | 4 ++++ configs/qemu-x86_defconfig | 3 +++ 3 files changed, 8 insertions(+), 1 deletion(-)
Regards, Bin
Hi Bin,
On Fri, 28 Jul 2023 at 09:46, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Mon, Jul 24, 2023 at 10:52 PM Simon Glass sjg@chromium.org wrote:
Unfortunately the bochs driver does not currently work with distros. It causes a hang sometime between grub menu selection and the OS displaying something.
Does this reproduce reliably?
Yes, it does.
BTW I've also hit another problem with '-M accel-kvm' which hangs on the move to 64-bit mode. Oddly if I boot U-Boot from coreboot, the problem goes away. So perhaps coreboot is doing some x86 init which U-Boot is missing.
This reverts commit b8956425d525c3c25fd218f252f89a5e44df6a9f.
Signed-off-by: Simon Glass sjg@chromium.org
arch/x86/cpu/qemu/Kconfig | 2 +- configs/qemu-x86_64_defconfig | 4 ++++ configs/qemu-x86_defconfig | 3 +++ 3 files changed, 8 insertions(+), 1 deletion(-)
Regards, Bin
Regards, Simon
Hi Simon,
On Sat, Jul 29, 2023 at 1:32 AM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Fri, 28 Jul 2023 at 09:46, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Mon, Jul 24, 2023 at 10:52 PM Simon Glass sjg@chromium.org wrote:
Unfortunately the bochs driver does not currently work with distros. It causes a hang sometime between grub menu selection and the OS displaying something.
Does this reproduce reliably?
Yes, it does.
BTW I've also hit another problem with '-M accel-kvm' which hangs on the move to 64-bit mode. Oddly if I boot U-Boot from coreboot, the problem goes away. So perhaps coreboot is doing some x86 init which U-Boot is missing.
This indicates a kvm-x86 bug. I think you can simplify a test case and report it to the KVM mailing list.
This reverts commit b8956425d525c3c25fd218f252f89a5e44df6a9f.
Signed-off-by: Simon Glass sjg@chromium.org
arch/x86/cpu/qemu/Kconfig | 2 +- configs/qemu-x86_64_defconfig | 4 ++++ configs/qemu-x86_defconfig | 3 +++ 3 files changed, 8 insertions(+), 1 deletion(-)
Regards, Bin
Hi Bin,
On Mon, 31 Jul 2023 at 08:32, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sat, Jul 29, 2023 at 1:32 AM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Fri, 28 Jul 2023 at 09:46, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Mon, Jul 24, 2023 at 10:52 PM Simon Glass sjg@chromium.org wrote:
Unfortunately the bochs driver does not currently work with distros. It causes a hang sometime between grub menu selection and the OS displaying something.
Does this reproduce reliably?
Yes, it does.
BTW I've also hit another problem with '-M accel-kvm' which hangs on the move to 64-bit mode. Oddly if I boot U-Boot from coreboot, the problem goes away. So perhaps coreboot is doing some x86 init which U-Boot is missing.
This indicates a kvm-x86 bug. I think you can simplify a test case and report it to the KVM mailing list.
Firstly it doesn't really matter, since it doesn't work, so we do need to revert.
Secondly I am not sure it kvm's fault, since the problem does not happen with coreboot. I will see if I can isolate what coreboot is doing differently, but not sure that that fix will make the release.
Regards, Simon
Hi Simon,
On Mon, Jul 31, 2023 at 10:37 PM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Mon, 31 Jul 2023 at 08:32, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sat, Jul 29, 2023 at 1:32 AM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Fri, 28 Jul 2023 at 09:46, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Mon, Jul 24, 2023 at 10:52 PM Simon Glass sjg@chromium.org wrote:
Unfortunately the bochs driver does not currently work with distros. It causes a hang sometime between grub menu selection and the OS displaying something.
Does this reproduce reliably?
Yes, it does.
BTW I've also hit another problem with '-M accel-kvm' which hangs on the move to 64-bit mode. Oddly if I boot U-Boot from coreboot, the problem goes away. So perhaps coreboot is doing some x86 init which U-Boot is missing.
This indicates a kvm-x86 bug. I think you can simplify a test case and report it to the KVM mailing list.
Firstly it doesn't really matter, since it doesn't work, so we do need to revert.
Agreed, but before we revert this, I would like to see we investigate this issue a little bit further.
I enabled the grub debug output today and not sure if grub jumps to the kernel entry yet:
kern/verifiers.c:214: string: boot, type: 2 loader/efi/linux_sb.c:55: kernel_addr: 0x1000000 handover_offset: 0x190 params: 0x7ce63000
Secondly I am not sure it kvm's fault, since the problem does not happen with coreboot. I will see if I can isolate what coreboot is doing differently, but not sure that that fix will make the release.
Does the 64-bit mode switch ever work on a real x86 board? If yes, then it's very likely a KVM bug.
If not, then we can check the coreboot to find out the difference.
Regards, Bin
Hi Bin,
On Mon, 31 Jul 2023 at 08:46, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Mon, Jul 31, 2023 at 10:37 PM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Mon, 31 Jul 2023 at 08:32, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sat, Jul 29, 2023 at 1:32 AM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Fri, 28 Jul 2023 at 09:46, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Mon, Jul 24, 2023 at 10:52 PM Simon Glass sjg@chromium.org wrote:
Unfortunately the bochs driver does not currently work with distros. It causes a hang sometime between grub menu selection and the OS displaying something.
Does this reproduce reliably?
Yes, it does.
BTW I've also hit another problem with '-M accel-kvm' which hangs on the move to 64-bit mode. Oddly if I boot U-Boot from coreboot, the problem goes away. So perhaps coreboot is doing some x86 init which U-Boot is missing.
This indicates a kvm-x86 bug. I think you can simplify a test case and report it to the KVM mailing list.
Firstly it doesn't really matter, since it doesn't work, so we do need to revert.
Agreed, but before we revert this, I would like to see we investigate this issue a little bit further.
I enabled the grub debug output today and not sure if grub jumps to the kernel entry yet:
kern/verifiers.c:214: string: boot, type: 2 loader/efi/linux_sb.c:55: kernel_addr: 0x1000000 handover_offset: 0x190 params: 0x7ce63000
Secondly I am not sure it kvm's fault, since the problem does not happen with coreboot. I will see if I can isolate what coreboot is doing differently, but not sure that that fix will make the release.
Does the 64-bit mode switch ever work on a real x86 board? If yes, then it's very likely a KVM bug.
Yes it works fine on chromebook_link64, for example.
If not, then we can check the coreboot to find out the difference.
Yes...do you have any ideas?
If it helps, I am using something like this:
qemu-system-x86_64 -m 8G -smp 4 -bios /tmp/b/qemu-x86_64/u-boot.rom -drive id=fdisk,file=root.img,if=virtio,driver=raw -drive \ file=/vid/software/linux/ubuntu/ubuntu-23.04-desktop-amd64.iso,if=virtio,driver=raw \ -serial mon:stdio
For coreboot my script is:
UBTEST=/vid/software/devel/ubtest/ BUILD_DIR=/tmp/b/coreboot64 DISK=/vid/software/linux/ubuntu/ubuntu-23.04-desktop-amd64.iso
cp $UBTEST/coreboot.rom.in $BUILD_DIR/coreboot.rom cbfstool $BUILD_DIR/coreboot.rom add-flat-binary \ -f $BUILD_DIR/u-boot-x86-with-spl.bin \ -n fallback/payload -c LZMA -l 0x1110000 -e 0x1110000
Add this in below: #-M accel=kvm
qemu-system-x86_64 -m 2G -smp 4 -bios $BUILD_DIR/coreboot.rom -serial mon:stdio \ -drive id=disk,file=$DISK,if=none \ -device ahci,id=ahci \ -device ide-hd,drive=disk,bus=ahci.0
Regards, Simon
Hi Bin,
On Mon, 31 Jul 2023 at 11:07, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Mon, 31 Jul 2023 at 08:46, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Mon, Jul 31, 2023 at 10:37 PM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Mon, 31 Jul 2023 at 08:32, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sat, Jul 29, 2023 at 1:32 AM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Fri, 28 Jul 2023 at 09:46, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Mon, Jul 24, 2023 at 10:52 PM Simon Glass sjg@chromium.org wrote: > > Unfortunately the bochs driver does not currently work with distros. It > causes a hang sometime between grub menu selection and the OS displaying > something.
Does this reproduce reliably?
Yes, it does.
BTW I've also hit another problem with '-M accel-kvm' which hangs on the move to 64-bit mode. Oddly if I boot U-Boot from coreboot, the problem goes away. So perhaps coreboot is doing some x86 init which U-Boot is missing.
This indicates a kvm-x86 bug. I think you can simplify a test case and report it to the KVM mailing list.
Firstly it doesn't really matter, since it doesn't work, so we do need to revert.
Agreed, but before we revert this, I would like to see we investigate this issue a little bit further.
I enabled the grub debug output today and not sure if grub jumps to the kernel entry yet:
kern/verifiers.c:214: string: boot, type: 2 loader/efi/linux_sb.c:55: kernel_addr: 0x1000000 handover_offset: 0x190 params: 0x7ce63000
Secondly I am not sure it kvm's fault, since the problem does not happen with coreboot. I will see if I can isolate what coreboot is doing differently, but not sure that that fix will make the release.
Does the 64-bit mode switch ever work on a real x86 board? If yes, then it's very likely a KVM bug.
Yes it works fine on chromebook_link64, for example.
If not, then we can check the coreboot to find out the difference.
Yes...do you have any ideas?
If it helps, I am using something like this:
qemu-system-x86_64 -m 8G -smp 4 -bios /tmp/b/qemu-x86_64/u-boot.rom -drive id=fdisk,file=root.img,if=virtio,driver=raw -drive \ file=/vid/software/linux/ubuntu/ubuntu-23.04-desktop-amd64.iso,if=virtio,driver=raw \ -serial mon:stdio
For coreboot my script is:
UBTEST=/vid/software/devel/ubtest/ BUILD_DIR=/tmp/b/coreboot64 DISK=/vid/software/linux/ubuntu/ubuntu-23.04-desktop-amd64.iso
cp $UBTEST/coreboot.rom.in $BUILD_DIR/coreboot.rom cbfstool $BUILD_DIR/coreboot.rom add-flat-binary \ -f $BUILD_DIR/u-boot-x86-with-spl.bin \ -n fallback/payload -c LZMA -l 0x1110000 -e 0x1110000
Add this in below: #-M accel=kvm
qemu-system-x86_64 -m 2G -smp 4 -bios $BUILD_DIR/coreboot.rom -serial mon:stdio \ -drive id=disk,file=$DISK,if=none \ -device ahci,id=ahci \ -device ide-hd,drive=disk,bus=ahci.0
I haven't looked into the problem here, but I did notice this coreboot commit: https://review.coreboot.org/c/coreboot/+/49228
I am not sure it is related though, since we already have page tables in RAM.
Regards, Simon
On Mon, Jul 24, 2023 at 10:52 PM Simon Glass sjg@chromium.org wrote:
Unfortunately the bochs driver does not currently work with distros. It causes a hang sometime between grub menu selection and the OS displaying something.
I will reword the commit message a little bit.
This reverts commit b8956425d525c3c25fd218f252f89a5e44df6a9f.
Signed-off-by: Simon Glass sjg@chromium.org
arch/x86/cpu/qemu/Kconfig | 2 +- configs/qemu-x86_64_defconfig | 4 ++++ configs/qemu-x86_defconfig | 3 +++ 3 files changed, 8 insertions(+), 1 deletion(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
When the video framebuffer comes from the bloblist, we should not change relocaddr to this address, since it interfers with the normal memory allocation.
This fixes a boot loop in qemu-x86_64
Signed-off-by: Simon Glass sjg@chromium.org Fixes: 5bc610a7d9d ("common: board_f: Pass frame buffer info from SPL to u-boot") ---
common/board_f.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/common/board_f.c b/common/board_f.c index 7d2c380e91e2..5c8646b22283 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -419,7 +419,6 @@ static int reserve_video(void) if (!ho) return log_msg_ret("blf", -ENOENT); video_reserve_from_bloblist(ho); - gd->relocaddr = ho->fb; } else if (CONFIG_IS_ENABLED(VIDEO)) { ulong addr; int ret;
Hi Simon,
On 24/07/23 20:22, Simon Glass wrote:
When the video framebuffer comes from the bloblist, we should not change relocaddr to this address, since it interfers with the normal memory allocation.
This fixes a boot loop in qemu-x86_64
Signed-off-by: Simon Glass sjg@chromium.org Fixes: 5bc610a7d9d ("common: board_f: Pass frame buffer info from SPL to u-boot")
common/board_f.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/common/board_f.c b/common/board_f.c index 7d2c380e91e2..5c8646b22283 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -419,7 +419,6 @@ static int reserve_video(void) if (!ho) return log_msg_ret("blf", -ENOENT); video_reserve_from_bloblist(ho);
gd->relocaddr = ho->fb;
I think this change was done as relocaddr pointer was required to be updated to move after frame-buffer reserved area to ensure that any further memory reservations done using gd->relocaddr (for e.g. in reserve_trace/uboot/malloc) don't overlap with frame-buffer reserved area passed from blob, so I think removing this line may cause further memory reservations to overlap with reserved framebuffer.
Could you please confirm?
Regards Devarsh
} else if (CONFIG_IS_ENABLED(VIDEO)) { ulong addr; int ret;
Hi Devarsh,
On Tue, 25 Jul 2023 at 03:21, Devarsh Thakkar devarsht@ti.com wrote:
Hi Simon,
On 24/07/23 20:22, Simon Glass wrote:
When the video framebuffer comes from the bloblist, we should not change relocaddr to this address, since it interfers with the normal memory allocation.
This fixes a boot loop in qemu-x86_64
Signed-off-by: Simon Glass sjg@chromium.org Fixes: 5bc610a7d9d ("common: board_f: Pass frame buffer info from SPL to u-boot")
common/board_f.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/common/board_f.c b/common/board_f.c index 7d2c380e91e2..5c8646b22283 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -419,7 +419,6 @@ static int reserve_video(void) if (!ho) return log_msg_ret("blf", -ENOENT); video_reserve_from_bloblist(ho);
gd->relocaddr = ho->fb;I think this change was done as relocaddr pointer was required to be updated to move after frame-buffer reserved area to ensure that any further memory reservations done using gd->relocaddr (for e.g. in reserve_trace/uboot/malloc) don't overlap with frame-buffer reserved area passed from blob, so I think removing this line may cause further memory reservations to overlap with reserved framebuffer.
Could you please confirm?
SPL and U-Boot have different memory layouts. The current code is interrupting U-Boot's careful building up of chunks of memory used for different purposes.
The video memory has already been allocated by SPL, so we don't need to insert a new one here, as your code demonstrates.
But also we have no way of knowing if it is legal to relocate U-Boot (and various other things) just below the frame buffer chosen by SPL.
The SPL frame buffer needs to be considered pre-allocated. It makes no sense to set relocaddr to this value. It will break all sorts of things. E.g. qemu-x86_64 crashes with this.
Regards Devarsh
} else if (CONFIG_IS_ENABLED(VIDEO)) { ulong addr; int ret;
Regards, Simon
Hi Simon,
On 26/07/23 02:58, Simon Glass wrote:
Hi Devarsh,
On Tue, 25 Jul 2023 at 03:21, Devarsh Thakkar devarsht@ti.com wrote:
Hi Simon,
On 24/07/23 20:22, Simon Glass wrote:
When the video framebuffer comes from the bloblist, we should not change relocaddr to this address, since it interfers with the normal memory allocation.
This fixes a boot loop in qemu-x86_64
Signed-off-by: Simon Glass sjg@chromium.org Fixes: 5bc610a7d9d ("common: board_f: Pass frame buffer info from SPL to u-boot")
common/board_f.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/common/board_f.c b/common/board_f.c index 7d2c380e91e2..5c8646b22283 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -419,7 +419,6 @@ static int reserve_video(void) if (!ho) return log_msg_ret("blf", -ENOENT); video_reserve_from_bloblist(ho);
gd->relocaddr = ho->fb;I think this change was done as relocaddr pointer was required to be updated to move after frame-buffer reserved area to ensure that any further memory reservations done using gd->relocaddr (for e.g. in reserve_trace/uboot/malloc) don't overlap with frame-buffer reserved area passed from blob, so I think removing this line may cause further memory reservations to overlap with reserved framebuffer.
Could you please confirm?
SPL and U-Boot have different memory layouts. The current code is interrupting U-Boot's careful building up of chunks of memory used for different purposes.
But it is possible they could be using similar memory layout for some components like framebuffer. For e.g. in our case we are using same video_reserve func in A53 SPL too and which allocates framebuffer from gd->relocaddr as seen here:
https://source.denx.de/u-boot/u-boot/-/blob/v2023.10-rc1/common/board_f.c?re...
The video memory has already been allocated by SPL, so we don't need to insert a new one here, as your code demonstrates.
Agreed.
But also we have no way of knowing if it is legal to relocate U-Boot (and various other things) just below the frame buffer chosen by SPL.
Yes, so i suppose your case is that framebuffer address which is being passed by SPL is totally disjoint and too far away from gd->relocaddr, for e.g. it is at the start (or bottom of DDR) and doesn't interfere with gd->relocaddr in any manner since relocaddr points to ramtop (i.e. near to end address of DDR).
In that case I agree it doesn't make sense to move relocaddr to ho->fb.
But for the scenario where gd->relocaddr and ho->fb are nearby there is every possibility that gd->relocaddr may overlap with framebuffer, also the code in reserve_trace, reserve_uboot doesn't have any intelligence or check that it is overlapping with framebuffer area or not.
I think one thing that can probably be done here is to have a check that if passed framebuffer area falls within current relocaddr region, then update the relocaddr else don't touch relocaddr :
if (ho->fb <= gd->relocaddr - ho->size) //It means framebuffer are is overlapping with current relocaddr so update relocaddr gd->relocaddr = ho->fb else //don't update gd->relocaddr since ho->fb is disjoint to gd->relocaddr
Could you please share your opinion on this and if above logic suffice your case too ?
Regards Devarsh
The SPL frame buffer needs to be considered pre-allocated. It makes no sense to set relocaddr to this value. It will break all sorts of things. E.g. qemu-x86_64 crashes with this.
Regards Devarsh
} else if (CONFIG_IS_ENABLED(VIDEO)) { ulong addr; int ret;Regards, Simon
Hi Devarsh,
On Wed, 26 Jul 2023 at 05:09, Devarsh Thakkar devarsht@ti.com wrote:
Hi Simon,
On 26/07/23 02:58, Simon Glass wrote:
Hi Devarsh,
On Tue, 25 Jul 2023 at 03:21, Devarsh Thakkar devarsht@ti.com wrote:
Hi Simon,
On 24/07/23 20:22, Simon Glass wrote:
When the video framebuffer comes from the bloblist, we should not change relocaddr to this address, since it interfers with the normal memory allocation.
This fixes a boot loop in qemu-x86_64
Signed-off-by: Simon Glass sjg@chromium.org Fixes: 5bc610a7d9d ("common: board_f: Pass frame buffer info from SPL to u-boot")
common/board_f.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/common/board_f.c b/common/board_f.c index 7d2c380e91e2..5c8646b22283 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -419,7 +419,6 @@ static int reserve_video(void) if (!ho) return log_msg_ret("blf", -ENOENT); video_reserve_from_bloblist(ho);
gd->relocaddr = ho->fb;I think this change was done as relocaddr pointer was required to be updated to move after frame-buffer reserved area to ensure that any further memory reservations done using gd->relocaddr (for e.g. in reserve_trace/uboot/malloc) don't overlap with frame-buffer reserved area passed from blob, so I think removing this line may cause further memory reservations to overlap with reserved framebuffer.
Could you please confirm?
SPL and U-Boot have different memory layouts. The current code is interrupting U-Boot's careful building up of chunks of memory used for different purposes.
But it is possible they could be using similar memory layout for some components like framebuffer. For e.g. in our case we are using same video_reserve func in A53 SPL too and which allocates framebuffer from gd->relocaddr as seen here:
https://source.denx.de/u-boot/u-boot/-/blob/v2023.10-rc1/common/board_f.c?re...
Even if it is similar, the point is that U-Boot proper needs to do its own allocation stuff.
Of course, if SPL sets up the video, it will provide the framebuffer address, but only that. The other addresses need to be done using the normal mechanism.
The video memory has already been allocated by SPL, so we don't need to insert a new one here, as your code demonstrates.
Agreed.
But also we have no way of knowing if it is legal to relocate U-Boot (and various other things) just below the frame buffer chosen by SPL.
Yes, so i suppose your case is that framebuffer address which is being passed by SPL is totally disjoint and too far away from gd->relocaddr, for e.g. it is at the start (or bottom of DDR) and doesn't interfere with gd->relocaddr in any manner since relocaddr points to ramtop (i.e. near to end address of DDR).
In that case I agree it doesn't make sense to move relocaddr to ho->fb.
But for the scenario where gd->relocaddr and ho->fb are nearby there is every possibility that gd->relocaddr may overlap with framebuffer, also the code in reserve_trace, reserve_uboot doesn't have any intelligence or check that it is overlapping with framebuffer area or not.
I think one thing that can probably be done here is to have a check that if passed framebuffer area falls within current relocaddr region, then update the relocaddr else don't touch relocaddr :
if (ho->fb <= gd->relocaddr - ho->size) //It means framebuffer are is overlapping with current relocaddr so update relocaddr gd->relocaddr = ho->fb else //don't update gd->relocaddr since ho->fb is disjoint to gd->relocaddr
Could you please share your opinion on this and if above logic suffice your case too ?
I don't think this line is needed at all, which is why this patch removes it. What problem are you seeing?
Regards, Simon
Regards Devarsh
The SPL frame buffer needs to be considered pre-allocated. It makes no sense to set relocaddr to this value. It will break all sorts of things. E.g. qemu-x86_64 crashes with this.
Regards Devarsh
} else if (CONFIG_IS_ENABLED(VIDEO)) { ulong addr; int ret;Regards, Simon
Hi Simon,
On 27/07/23 06:23, Simon Glass wrote:
Hi Devarsh,
On Wed, 26 Jul 2023 at 05:09, Devarsh Thakkar devarsht@ti.com wrote:
Hi Simon,
On 26/07/23 02:58, Simon Glass wrote:
Hi Devarsh,
On Tue, 25 Jul 2023 at 03:21, Devarsh Thakkar devarsht@ti.com wrote:
Hi Simon,
On 24/07/23 20:22, Simon Glass wrote:
When the video framebuffer comes from the bloblist, we should not change relocaddr to this address, since it interfers with the normal memory allocation.
This fixes a boot loop in qemu-x86_64
Signed-off-by: Simon Glass sjg@chromium.org Fixes: 5bc610a7d9d ("common: board_f: Pass frame buffer info from SPL to u-boot")
common/board_f.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/common/board_f.c b/common/board_f.c index 7d2c380e91e2..5c8646b22283 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -419,7 +419,6 @@ static int reserve_video(void) if (!ho) return log_msg_ret("blf", -ENOENT); video_reserve_from_bloblist(ho);
gd->relocaddr = ho->fb;I think this change was done as relocaddr pointer was required to be updated to move after frame-buffer reserved area to ensure that any further memory reservations done using gd->relocaddr (for e.g. in reserve_trace/uboot/malloc) don't overlap with frame-buffer reserved area passed from blob, so I think removing this line may cause further memory reservations to overlap with reserved framebuffer.
Could you please confirm?
SPL and U-Boot have different memory layouts. The current code is interrupting U-Boot's careful building up of chunks of memory used for different purposes.
But it is possible they could be using similar memory layout for some components like framebuffer. For e.g. in our case we are using same video_reserve func in A53 SPL too and which allocates framebuffer from gd->relocaddr as seen here:
https://source.denx.de/u-boot/u-boot/-/blob/v2023.10-rc1/common/board_f.c?re...
Even if it is similar, the point is that U-Boot proper needs to do its own allocation stuff.
Of course, if SPL sets up the video, it will provide the framebuffer address, but only that. The other addresses need to be done using the normal mechanism.
The video memory has already been allocated by SPL, so we don't need to insert a new one here, as your code demonstrates.
Agreed.
But also we have no way of knowing if it is legal to relocate U-Boot (and various other things) just below the frame buffer chosen by SPL.
Yes, so i suppose your case is that framebuffer address which is being passed by SPL is totally disjoint and too far away from gd->relocaddr, for e.g. it is at the start (or bottom of DDR) and doesn't interfere with gd->relocaddr in any manner since relocaddr points to ramtop (i.e. near to end address of DDR).
In that case I agree it doesn't make sense to move relocaddr to ho->fb.
But for the scenario where gd->relocaddr and ho->fb are nearby there is every possibility that gd->relocaddr may overlap with framebuffer, also the code in reserve_trace, reserve_uboot doesn't have any intelligence or check that it is overlapping with framebuffer area or not.
I think one thing that can probably be done here is to have a check that if passed framebuffer area falls within current relocaddr region, then update the relocaddr else don't touch relocaddr :
if (ho->fb <= gd->relocaddr - ho->size) //It means framebuffer are is overlapping with current relocaddr so update relocaddr gd->relocaddr = ho->fb else //don't update gd->relocaddr since ho->fb is disjoint to gd->relocaddr
Could you please share your opinion on this and if above logic suffice your case too ?
I don't think this line is needed at all, which is why this patch removes it. What problem are you seeing?
Across SPL stage and U-boot we are keeping same memory layout and ensuring that same memory regions are used, this way it doesn't interfere in the way of u-boot while allocating memory regions for various purposes. This allowed us to display splash screen without any flicker across the stages.
Now if you remove the line gd->relocaddr = ho->fb, the frame buffer region will be used for reserving memory for other purposes which corrupts the frame buffer.
One solution which we are planning to implement is move the ram_top to a lower address leaving out a region for video buffer and u-boot can do the allocation from the new ram_top address without spl video handoff interfering in the u-boot's allocation of memory.The region above the ram_top can be used for video.
Present Scenario +---------------------+ram_top | | | page_table | | | | | +---------------------+ | | | | | | | | | | | video frame buffer | | | | | | | | | | | | | +---------------------+ | | | | | reserve_trace | | | | | | | +---------------------+
Proposed Solution +---------------------+ | | | | | | | | | | | video frame buffer | | | | | | | | | | | +---------------------+ram_top | | | | | page_table | | | | | | | +---------------------+ | | | | | reserve_trace | | | | | +---------------------+
Regards, Simon
Regards Devarsh
The SPL frame buffer needs to be considered pre-allocated. It makes no sense to set relocaddr to this value. It will break all sorts of things. E.g. qemu-x86_64 crashes with this.
Regards Devarsh
} else if (CONFIG_IS_ENABLED(VIDEO)) { ulong addr; int ret;Regards, Simon
Regards, Nikhil
Hi Nikhil,
On Wed, 26 Jul 2023 at 23:22, Nikhil M Jain n-jain1@ti.com wrote:
Hi Simon,
On 27/07/23 06:23, Simon Glass wrote:
Hi Devarsh,
On Wed, 26 Jul 2023 at 05:09, Devarsh Thakkar devarsht@ti.com wrote:
Hi Simon,
On 26/07/23 02:58, Simon Glass wrote:
Hi Devarsh,
On Tue, 25 Jul 2023 at 03:21, Devarsh Thakkar devarsht@ti.com wrote:
Hi Simon,
On 24/07/23 20:22, Simon Glass wrote:
When the video framebuffer comes from the bloblist, we should not change relocaddr to this address, since it interfers with the normal memory allocation.
This fixes a boot loop in qemu-x86_64
Signed-off-by: Simon Glass sjg@chromium.org Fixes: 5bc610a7d9d ("common: board_f: Pass frame buffer info from SPL to u-boot")
common/board_f.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/common/board_f.c b/common/board_f.c index 7d2c380e91e2..5c8646b22283 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -419,7 +419,6 @@ static int reserve_video(void) if (!ho) return log_msg_ret("blf", -ENOENT); video_reserve_from_bloblist(ho);
gd->relocaddr = ho->fb;I think this change was done as relocaddr pointer was required to be updated to move after frame-buffer reserved area to ensure that any further memory reservations done using gd->relocaddr (for e.g. in reserve_trace/uboot/malloc) don't overlap with frame-buffer reserved area passed from blob, so I think removing this line may cause further memory reservations to overlap with reserved framebuffer.
Could you please confirm?
SPL and U-Boot have different memory layouts. The current code is interrupting U-Boot's careful building up of chunks of memory used for different purposes.
But it is possible they could be using similar memory layout for some components like framebuffer. For e.g. in our case we are using same video_reserve func in A53 SPL too and which allocates framebuffer from gd->relocaddr as seen here:
https://source.denx.de/u-boot/u-boot/-/blob/v2023.10-rc1/common/board_f.c?re...
Even if it is similar, the point is that U-Boot proper needs to do its own allocation stuff.
Of course, if SPL sets up the video, it will provide the framebuffer address, but only that. The other addresses need to be done using the normal mechanism.
The video memory has already been allocated by SPL, so we don't need to insert a new one here, as your code demonstrates.
Agreed.
But also we have no way of knowing if it is legal to relocate U-Boot (and various other things) just below the frame buffer chosen by SPL.
Yes, so i suppose your case is that framebuffer address which is being passed by SPL is totally disjoint and too far away from gd->relocaddr, for e.g. it is at the start (or bottom of DDR) and doesn't interfere with gd->relocaddr in any manner since relocaddr points to ramtop (i.e. near to end address of DDR).
In that case I agree it doesn't make sense to move relocaddr to ho->fb.
But for the scenario where gd->relocaddr and ho->fb are nearby there is every possibility that gd->relocaddr may overlap with framebuffer, also the code in reserve_trace, reserve_uboot doesn't have any intelligence or check that it is overlapping with framebuffer area or not.
I think one thing that can probably be done here is to have a check that if passed framebuffer area falls within current relocaddr region, then update the relocaddr else don't touch relocaddr :
if (ho->fb <= gd->relocaddr - ho->size) //It means framebuffer are is overlapping with current relocaddr so update relocaddr gd->relocaddr = ho->fb else //don't update gd->relocaddr since ho->fb is disjoint to gd->relocaddr
Could you please share your opinion on this and if above logic suffice your case too ?
I don't think this line is needed at all, which is why this patch removes it. What problem are you seeing?
Across SPL stage and U-boot we are keeping same memory layout and ensuring that same memory regions are used, this way it doesn't interfere in the way of u-boot while allocating memory regions for various purposes. This allowed us to display splash screen without any flicker across the stages.
Now if you remove the line gd->relocaddr = ho->fb, the frame buffer region will be used for reserving memory for other purposes which corrupts the frame buffer.
One solution which we are planning to implement is move the ram_top to a lower address leaving out a region for video buffer and u-boot can do the allocation from the new ram_top address without spl video handoff interfering in the u-boot's allocation of memory.The region above the ram_top can be used for video.
Present Scenario +---------------------+ram_top | | | page_table | | | | | +---------------------+ | | | | | | | | | | | video frame buffer | | | | | | | | | | | | | +---------------------+ | | | | | reserve_trace | | | | | | | +---------------------+
Proposed Solution +---------------------+ | | | | | | | | | | | video frame buffer | | | | | | | | | | | +---------------------+ram_top | | | | | page_table | | | | | | | +---------------------+ | | | | | reserve_trace | | | | | +---------------------+
Sure, whatever you need to do is fine. You could pass the ram top from SPL to U-Boot perhaps.
Regards, Simon
Hi Simon,
On 27/07/23 23:31, Simon Glass wrote:
Hi Nikhil,
On Wed, 26 Jul 2023 at 23:22, Nikhil M Jain n-jain1@ti.com wrote:
Hi Simon,
On 27/07/23 06:23, Simon Glass wrote:
Hi Devarsh,
On Wed, 26 Jul 2023 at 05:09, Devarsh Thakkar devarsht@ti.com wrote:
Hi Simon,
On 26/07/23 02:58, Simon Glass wrote:
Hi Devarsh,
On Tue, 25 Jul 2023 at 03:21, Devarsh Thakkar devarsht@ti.com wrote:
Hi Simon,
On 24/07/23 20:22, Simon Glass wrote: > When the video framebuffer comes from the bloblist, we should not change > relocaddr to this address, since it interfers with the normal memory > allocation. > > This fixes a boot loop in qemu-x86_64 > > Signed-off-by: Simon Glass sjg@chromium.org > Fixes: 5bc610a7d9d ("common: board_f: Pass frame buffer info from SPL to u-boot") > --- > > common/board_f.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/common/board_f.c b/common/board_f.c > index 7d2c380e91e2..5c8646b22283 100644 > --- a/common/board_f.c > +++ b/common/board_f.c > @@ -419,7 +419,6 @@ static int reserve_video(void) > if (!ho) > return log_msg_ret("blf", -ENOENT); > video_reserve_from_bloblist(ho); > - gd->relocaddr = ho->fb;
I think this change was done as relocaddr pointer was required to be updated to move after frame-buffer reserved area to ensure that any further memory reservations done using gd->relocaddr (for e.g. in reserve_trace/uboot/malloc) don't overlap with frame-buffer reserved area passed from blob, so I think removing this line may cause further memory reservations to overlap with reserved framebuffer.
Could you please confirm?
SPL and U-Boot have different memory layouts. The current code is interrupting U-Boot's careful building up of chunks of memory used for different purposes.
But it is possible they could be using similar memory layout for some components like framebuffer. For e.g. in our case we are using same video_reserve func in A53 SPL too and which allocates framebuffer from gd->relocaddr as seen here:
https://source.denx.de/u-boot/u-boot/-/blob/v2023.10-rc1/common/board_f.c?re...
Even if it is similar, the point is that U-Boot proper needs to do its own allocation stuff.
Of course, if SPL sets up the video, it will provide the framebuffer address, but only that. The other addresses need to be done using the normal mechanism.
The video memory has already been allocated by SPL, so we don't need to insert a new one here, as your code demonstrates.
Agreed.
But also we have no way of knowing if it is legal to relocate U-Boot (and various other things) just below the frame buffer chosen by SPL.
Yes, so i suppose your case is that framebuffer address which is being passed by SPL is totally disjoint and too far away from gd->relocaddr, for e.g. it is at the start (or bottom of DDR) and doesn't interfere with gd->relocaddr in any manner since relocaddr points to ramtop (i.e. near to end address of DDR).
In that case I agree it doesn't make sense to move relocaddr to ho->fb.
But for the scenario where gd->relocaddr and ho->fb are nearby there is every possibility that gd->relocaddr may overlap with framebuffer, also the code in reserve_trace, reserve_uboot doesn't have any intelligence or check that it is overlapping with framebuffer area or not.
I think one thing that can probably be done here is to have a check that if passed framebuffer area falls within current relocaddr region, then update the relocaddr else don't touch relocaddr :
if (ho->fb <= gd->relocaddr - ho->size) //It means framebuffer are is overlapping with current relocaddr so update relocaddr gd->relocaddr = ho->fb
We should go ahead with this check because it won't disrupt u-boot's allocation of memory and will allow both the cases if a platform is using same memory layout or different memory layout across SPL and u-boot proper. Below are the logs for both scenarios.
https://gist.github.com/NikMJain/aca198ae77b6f1855459bc8fbdd683df
else //don't update gd->relocaddr since ho->fb is disjoint to gd->relocaddr
Could you please share your opinion on this and if above logic suffice your case too ?
I don't think this line is needed at all, which is why this patch removes it. What problem are you seeing?
Across SPL stage and U-boot we are keeping same memory layout and ensuring that same memory regions are used, this way it doesn't interfere in the way of u-boot while allocating memory regions for various purposes. This allowed us to display splash screen without any flicker across the stages.
Now if you remove the line gd->relocaddr = ho->fb, the frame buffer region will be used for reserving memory for other purposes which corrupts the frame buffer.
One solution which we are planning to implement is move the ram_top to a lower address leaving out a region for video buffer and u-boot can do the allocation from the new ram_top address without spl video handoff interfering in the u-boot's allocation of memory.The region above the ram_top can be used for video.
Present Scenario +---------------------+ram_top | | | page_table | | | | | +---------------------+ | | | | | | | | | | | video frame buffer | | | | | | | | | | | | | +---------------------+ | | | | | reserve_trace | | | | | | | +---------------------+
Proposed Solution +---------------------+ | | | | | | | | | | | video frame buffer | | | | | | | | | | | +---------------------+ram_top | | | | | page_table | | | | | | | +---------------------+ | | | | | reserve_trace | | | | | +---------------------+
Sure, whatever you need to do is fine. You could pass the ram top from SPL to U-Boot perhaps.
In this solution problem arises when user doesn't want's to hand-off buffer, the frame buffer region will be reallocated by u-boot or will require us to hard code buffer address, which we don't want to do.
Regards, Simon
Thanks, Nikhil
On 28/07/23 14:05, Nikhil M Jain wrote:
Hi Simon,
On 27/07/23 23:31, Simon Glass wrote:
Hi Nikhil,
On Wed, 26 Jul 2023 at 23:22, Nikhil M Jain n-jain1@ti.com wrote:
Hi Simon,
On 27/07/23 06:23, Simon Glass wrote:
Hi Devarsh,
On Wed, 26 Jul 2023 at 05:09, Devarsh Thakkar devarsht@ti.com wrote:
Hi Simon,
On 26/07/23 02:58, Simon Glass wrote:
Hi Devarsh,
On Tue, 25 Jul 2023 at 03:21, Devarsh Thakkar devarsht@ti.com wrote: > > Hi Simon, > > On 24/07/23 20:22, Simon Glass wrote: >> When the video framebuffer comes from the bloblist, we should >> not change >> relocaddr to this address, since it interfers with the normal >> memory >> allocation. >> >> This fixes a boot loop in qemu-x86_64 >> >> Signed-off-by: Simon Glass sjg@chromium.org >> Fixes: 5bc610a7d9d ("common: board_f: Pass frame buffer info >> from SPL to u-boot") >> --- >> >> common/board_f.c | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/common/board_f.c b/common/board_f.c >> index 7d2c380e91e2..5c8646b22283 100644 >> --- a/common/board_f.c >> +++ b/common/board_f.c >> @@ -419,7 +419,6 @@ static int reserve_video(void) >> if (!ho) >> return log_msg_ret("blf", -ENOENT); >> video_reserve_from_bloblist(ho); >> - gd->relocaddr = ho->fb; > > I think this change was done as relocaddr pointer was required to > be updated > to move after frame-buffer reserved area to ensure that any > further memory > reservations done using gd->relocaddr (for e.g. in > reserve_trace/uboot/malloc) > don't overlap with frame-buffer reserved area passed from blob, > so I think > removing this line may cause further memory reservations to > overlap with > reserved framebuffer. > > Could you please confirm?
SPL and U-Boot have different memory layouts. The current code is interrupting U-Boot's careful building up of chunks of memory used for different purposes.
But it is possible they could be using similar memory layout for some components like framebuffer. For e.g. in our case we are using same video_reserve func in A53 SPL too and which allocates framebuffer from gd->relocaddr as seen here:
https://source.denx.de/u-boot/u-boot/-/blob/v2023.10-rc1/common/board_f.c?re...
Even if it is similar, the point is that U-Boot proper needs to do its own allocation stuff.
Of course, if SPL sets up the video, it will provide the framebuffer address, but only that. The other addresses need to be done using the normal mechanism.
The video memory has already been allocated by SPL, so we don't need to insert a new one here, as your code demonstrates.
Agreed.
But also we have no way of knowing if it is legal to relocate U-Boot (and various other things) just below the frame buffer chosen by SPL.
Yes, so i suppose your case is that framebuffer address which is being passed by SPL is totally disjoint and too far away from gd->relocaddr, for e.g. it is at the start (or bottom of DDR) and doesn't interfere with gd->relocaddr in any manner since relocaddr points to ramtop (i.e. near to end address of DDR).
In that case I agree it doesn't make sense to move relocaddr to ho->fb.
But for the scenario where gd->relocaddr and ho->fb are nearby there is every possibility that gd->relocaddr may overlap with framebuffer, also the code in reserve_trace, reserve_uboot doesn't have any intelligence or check that it is overlapping with framebuffer area or not.
I think one thing that can probably be done here is to have a check that if passed framebuffer area falls within current relocaddr region, then update the relocaddr else don't touch relocaddr :
if (ho->fb <= gd->relocaddr - ho->size)
Just a small correction here if (ho->fb >= gd->relocaddr - ho->size)
//It means framebuffer are is overlapping with current relocaddr so update relocaddr gd->relocaddr = ho->fb
We should go ahead with this check because it won't disrupt u-boot's allocation of memory and will allow both the cases if a platform is using same memory layout or different memory layout across SPL and u-boot proper. Below are the logs for both scenarios.
https://gist.github.com/NikMJain/aca198ae77b6f1855459bc8fbdd683df
else //don't update gd->relocaddr since ho->fb is disjoint to gd->relocaddr
Could you please share your opinion on this and if above logic suffice your case too ?
I don't think this line is needed at all, which is why this patch removes it. What problem are you seeing?
Across SPL stage and U-boot we are keeping same memory layout and ensuring that same memory regions are used, this way it doesn't interfere in the way of u-boot while allocating memory regions for various purposes. This allowed us to display splash screen without any flicker across the stages.
Now if you remove the line gd->relocaddr = ho->fb, the frame buffer region will be used for reserving memory for other purposes which corrupts the frame buffer.
One solution which we are planning to implement is move the ram_top to a lower address leaving out a region for video buffer and u-boot can do the allocation from the new ram_top address without spl video handoff interfering in the u-boot's allocation of memory.The region above the ram_top can be used for video.
Present Scenario +---------------------+ram_top | | | page_table | | | | | +---------------------+ | | | | | | | | | | | video frame buffer | | | | | | | | | | | | | +---------------------+ | | | | | reserve_trace | | | | | | | +---------------------+
Proposed Solution +---------------------+ | | | | | | | | | | | video frame buffer | | | | | | | | | | | +---------------------+ram_top | | | | | page_table | | | | | | | +---------------------+ | | | | | reserve_trace | | | | | +---------------------+
Sure, whatever you need to do is fine. You could pass the ram top from SPL to U-Boot perhaps.
In this solution problem arises when user doesn't want's to hand-off buffer, the frame buffer region will be reallocated by u-boot or will require us to hard code buffer address, which we don't want to do.
Regards, Simon
Thanks, Nikhil
Hi,
+Tom Rini for comment
On Fri, 28 Jul 2023 at 02:35, Nikhil M Jain n-jain1@ti.com wrote:
Hi Simon,
On 27/07/23 23:31, Simon Glass wrote:
Hi Nikhil,
On Wed, 26 Jul 2023 at 23:22, Nikhil M Jain n-jain1@ti.com wrote:
Hi Simon,
On 27/07/23 06:23, Simon Glass wrote:
Hi Devarsh,
On Wed, 26 Jul 2023 at 05:09, Devarsh Thakkar devarsht@ti.com wrote:
Hi Simon,
On 26/07/23 02:58, Simon Glass wrote:
Hi Devarsh,
On Tue, 25 Jul 2023 at 03:21, Devarsh Thakkar devarsht@ti.com wrote: > > Hi Simon, > > On 24/07/23 20:22, Simon Glass wrote: >> When the video framebuffer comes from the bloblist, we should not change >> relocaddr to this address, since it interfers with the normal memory >> allocation. >> >> This fixes a boot loop in qemu-x86_64 >> >> Signed-off-by: Simon Glass sjg@chromium.org >> Fixes: 5bc610a7d9d ("common: board_f: Pass frame buffer info from SPL to u-boot") >> --- >> >> common/board_f.c | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/common/board_f.c b/common/board_f.c >> index 7d2c380e91e2..5c8646b22283 100644 >> --- a/common/board_f.c >> +++ b/common/board_f.c >> @@ -419,7 +419,6 @@ static int reserve_video(void) >> if (!ho) >> return log_msg_ret("blf", -ENOENT); >> video_reserve_from_bloblist(ho); >> - gd->relocaddr = ho->fb; > > I think this change was done as relocaddr pointer was required to be updated > to move after frame-buffer reserved area to ensure that any further memory > reservations done using gd->relocaddr (for e.g. in reserve_trace/uboot/malloc) > don't overlap with frame-buffer reserved area passed from blob, so I think > removing this line may cause further memory reservations to overlap with > reserved framebuffer. > > Could you please confirm?
SPL and U-Boot have different memory layouts. The current code is interrupting U-Boot's careful building up of chunks of memory used for different purposes.
But it is possible they could be using similar memory layout for some components like framebuffer. For e.g. in our case we are using same video_reserve func in A53 SPL too and which allocates framebuffer from gd->relocaddr as seen here:
https://source.denx.de/u-boot/u-boot/-/blob/v2023.10-rc1/common/board_f.c?re...
Even if it is similar, the point is that U-Boot proper needs to do its own allocation stuff.
Of course, if SPL sets up the video, it will provide the framebuffer address, but only that. The other addresses need to be done using the normal mechanism.
The video memory has already been allocated by SPL, so we don't need to insert a new one here, as your code demonstrates.
Agreed.
But also we have no way of knowing if it is legal to relocate U-Boot (and various other things) just below the frame buffer chosen by SPL.
Yes, so i suppose your case is that framebuffer address which is being passed by SPL is totally disjoint and too far away from gd->relocaddr, for e.g. it is at the start (or bottom of DDR) and doesn't interfere with gd->relocaddr in any manner since relocaddr points to ramtop (i.e. near to end address of DDR).
In that case I agree it doesn't make sense to move relocaddr to ho->fb.
But for the scenario where gd->relocaddr and ho->fb are nearby there is every possibility that gd->relocaddr may overlap with framebuffer, also the code in reserve_trace, reserve_uboot doesn't have any intelligence or check that it is overlapping with framebuffer area or not.
I think one thing that can probably be done here is to have a check that if passed framebuffer area falls within current relocaddr region, then update the relocaddr else don't touch relocaddr :
if (ho->fb <= gd->relocaddr - ho->size) //It means framebuffer are is overlapping with current relocaddr so update relocaddr gd->relocaddr = ho->fb
We should go ahead with this check because it won't disrupt u-boot's allocation of memory and will allow both the cases if a platform is using same memory layout or different memory layout across SPL and u-boot proper. Below are the logs for both scenarios.
https://gist.github.com/NikMJain/aca198ae77b6f1855459bc8fbdd683df
else //don't update gd->relocaddr since ho->fb is disjoint to gd->relocaddr
Could you please share your opinion on this and if above logic suffice your case too ?
I don't think this line is needed at all, which is why this patch removes it. What problem are you seeing?
Across SPL stage and U-boot we are keeping same memory layout and ensuring that same memory regions are used, this way it doesn't interfere in the way of u-boot while allocating memory regions for various purposes. This allowed us to display splash screen without any flicker across the stages.
Now if you remove the line gd->relocaddr = ho->fb, the frame buffer region will be used for reserving memory for other purposes which corrupts the frame buffer.
One solution which we are planning to implement is move the ram_top to a lower address leaving out a region for video buffer and u-boot can do the allocation from the new ram_top address without spl video handoff interfering in the u-boot's allocation of memory.The region above the ram_top can be used for video.
Present Scenario +---------------------+ram_top | | | page_table | | | | | +---------------------+ | | | | | | | | | | | video frame buffer | | | | | | | | | | | | | +---------------------+ | | | | | reserve_trace | | | | | | | +---------------------+
Proposed Solution +---------------------+ | | | | | | | | | | | video frame buffer | | | | | | | | | | | +---------------------+ram_top | | | | | page_table | | | | | | | +---------------------+ | | | | | reserve_trace | | | | | +---------------------+
Sure, whatever you need to do is fine. You could pass the ram top from SPL to U-Boot perhaps.
In this solution problem arises when user doesn't want's to hand-off buffer, the frame buffer region will be reallocated by u-boot or will require us to hard code buffer address, which we don't want to do.
How about using a Kconfig instead? It seems to me that the idea of SPL and U-Boot happening to place the video buffer at the same place is more the exception than the rule. I've tried to explain that the sequence of reserve...() calls that U-Boot proper does really should not be molested by some data arriving from SPL.
If you don't want to do a Kconfig, then please send a patch that works for you and I'll test it, then update my series.
Regards, Simon
This is copying beyond the end of the destination buffer. Correct the code by using a constant for the buffer size.
This long-standing bug prevents virtio bootdevs working correctly on qemu-x86 at present.
Signed-off-by: Simon Glass sjg@chromium.org Fixes: 0ca2426beae ("x86: Add support for running option ROMs natively") ---
arch/x86/lib/bios.c | 2 +- include/vesa.h | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/x86/lib/bios.c b/arch/x86/lib/bios.c index e29cae78e509..3a9d7f5ddd41 100644 --- a/arch/x86/lib/bios.c +++ b/arch/x86/lib/bios.c @@ -204,7 +204,7 @@ static u8 vbe_get_mode_info(struct vesa_state *mi)
realmode_interrupt(0x10, VESA_GET_MODE_INFO, 0x0000, mi->video_mode, 0x0000, buffer_seg, buffer_adr); - memcpy(mi->mode_info_block, buffer, sizeof(struct vesa_state)); + memcpy(mi->mode_info_block, buffer, VESA_MODE_INFO_SIZE); mi->valid = true;
return 0; diff --git a/include/vesa.h b/include/vesa.h index 9285bfa921a2..28828ab56aa4 100644 --- a/include/vesa.h +++ b/include/vesa.h @@ -83,12 +83,14 @@ struct __packed vesa_mode_info { u8 reserved[206]; };
+#define VESA_MODE_INFO_SIZE 256 + struct vesa_state { u16 video_mode; bool valid; union { struct vesa_mode_info vesa; - u8 mode_info_block[256]; + u8 mode_info_block[VESA_MODE_INFO_SIZE]; }; };
On Mon, Jul 24, 2023 at 10:52 PM Simon Glass sjg@chromium.org wrote:
This is copying beyond the end of the destination buffer. Correct the code by using a constant for the buffer size.
This long-standing bug prevents virtio bootdevs working correctly on qemu-x86 at present.
Nice catch!
Signed-off-by: Simon Glass sjg@chromium.org Fixes: 0ca2426beae ("x86: Add support for running option ROMs natively")
arch/x86/lib/bios.c | 2 +- include/vesa.h | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/x86/lib/bios.c b/arch/x86/lib/bios.c index e29cae78e509..3a9d7f5ddd41 100644 --- a/arch/x86/lib/bios.c +++ b/arch/x86/lib/bios.c @@ -204,7 +204,7 @@ static u8 vbe_get_mode_info(struct vesa_state *mi)
realmode_interrupt(0x10, VESA_GET_MODE_INFO, 0x0000, mi->video_mode, 0x0000, buffer_seg, buffer_adr);
memcpy(mi->mode_info_block, buffer, sizeof(struct vesa_state));
memcpy(mi->mode_info_block, buffer, VESA_MODE_INFO_SIZE);
or "sizeof(struct vesa_mode_info)"
mi->valid = true; return 0;diff --git a/include/vesa.h b/include/vesa.h index 9285bfa921a2..28828ab56aa4 100644 --- a/include/vesa.h +++ b/include/vesa.h @@ -83,12 +83,14 @@ struct __packed vesa_mode_info { u8 reserved[206]; };
+#define VESA_MODE_INFO_SIZE 256
struct vesa_state { u16 video_mode; bool valid; union { struct vesa_mode_info vesa;
u8 mode_info_block[256];
u8 mode_info_block[VESA_MODE_INFO_SIZE]; };};
Reviewed-by: Bin Meng bmeng.cn@gmail.com
At present this feature is enabled in SPL if a bloblist is available. Some platforms may not want to use this, so add an option to allow the feature to be disabled.
Note that the feature unfortunately only fills in part of the video-handoff information, so causes failures on x86 platforms. For now, disable it there.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/board_f.c | 3 +-- drivers/video/Kconfig | 10 ++++++++++ drivers/video/video-uclass.c | 2 +- 3 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/common/board_f.c b/common/board_f.c index 5c8646b22283..656bd716c7ec 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -411,8 +411,7 @@ __weak int arch_reserve_mmu(void)
static int reserve_video(void) { - if (IS_ENABLED(CONFIG_SPL_VIDEO) && spl_phase() > PHASE_SPL && - CONFIG_IS_ENABLED(BLOBLIST)) { + if (IS_ENABLED(CONFIG_SPL_VIDEO_HANDOFF) && spl_phase() > PHASE_SPL) { struct video_handoff *ho;
ho = bloblist_find(BLOBLISTT_U_BOOT_VIDEO, sizeof(*ho)); diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig index b41dc60cec59..0c9c14b021fe 100644 --- a/drivers/video/Kconfig +++ b/drivers/video/Kconfig @@ -1011,6 +1011,16 @@ config SPL_VIDEO if SPL_VIDEO source "drivers/video/tidss/Kconfig"
+config SPL_VIDEO_HANDOFF + bool "Pass the video frame-buffer through to U-Boot proper" + depends on SPL_BLOBLIST + default y if !X86 + help + Enable this to set up video-handoff information in SPL which can be + picked up in U-Boot proper. This includes the frame buffer and + various other pieces of information. With this enabled, SPL can set + up video and avoid re-initing it later. + config SPL_VIDEO_LOGO bool "Show the U-Boot logo on the display at SPL" default y if !SPL_SPLASH_SCREEN diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c index 949595f1bc69..86262a3435c2 100644 --- a/drivers/video/video-uclass.c +++ b/drivers/video/video-uclass.c @@ -141,7 +141,7 @@ int video_reserve(ulong *addrp) debug("Video frame buffers from %lx to %lx\n", gd->video_bottom, gd->video_top);
- if (spl_phase() == PHASE_SPL && CONFIG_IS_ENABLED(BLOBLIST)) { + if (spl_phase() == PHASE_SPL && CONFIG_IS_ENABLED(VIDEO_HANDOFF)) { struct video_handoff *ho;
ho = bloblist_add(BLOBLISTT_U_BOOT_VIDEO, sizeof(*ho), 0);
This build can be used to boot 32-bit standard-distro builds. Enable some more options, so that all possible EFI UUIDs are decoded, we can search memory for tables, support the full set of standard-boot features, have full logging along with debug UART and can boot from CDROM media.
This mirrors a similar patch for qemu-x86_64
Signed-off-by: Simon Glass sjg@chromium.org ---
configs/qemu-x86_defconfig | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/configs/qemu-x86_defconfig b/configs/qemu-x86_defconfig index 56788cd185fd..ff9df677ef4c 100644 --- a/configs/qemu-x86_defconfig +++ b/configs/qemu-x86_defconfig @@ -5,10 +5,14 @@ CONFIG_NR_DRAM_BANKS=8 CONFIG_ENV_SIZE=0x40000 CONFIG_MAX_CPUS=2 CONFIG_DEFAULT_DEVICE_TREE="qemu-x86_i440fx" +CONFIG_DEBUG_UART_BASE=0x3f8 +CONFIG_DEBUG_UART_CLOCK=1843200 +CONFIG_DEBUG_UART=y CONFIG_SMP=y CONFIG_GENERATE_PIRQ_TABLE=y CONFIG_GENERATE_MP_TABLE=y CONFIG_FIT=y +CONFIG_BOOTSTD_FULL=y CONFIG_BOOTSTD_DEFAULTS=y CONFIG_BOOTSTAGE=y CONFIG_BOOTSTAGE_REPORT=y @@ -16,6 +20,8 @@ CONFIG_SHOW_BOOT_PROGRESS=y CONFIG_USE_BOOTARGS=y CONFIG_BOOTARGS="root=/dev/sdb3 init=/sbin/init rootwait ro" CONFIG_SYS_CONSOLE_INFO_QUIET=y +CONFIG_LOG=y +CONFIG_LOGF_FUNC=y CONFIG_DISPLAY_BOARDINFO_LATE=y CONFIG_LAST_STAGE_INIT=y CONFIG_PCI_INIT_R=y @@ -23,11 +29,13 @@ CONFIG_SYS_PBSIZE=532 CONFIG_CMD_CPU=y CONFIG_CMD_BOOTEFI_SELFTEST=y CONFIG_CMD_NVEDIT_EFI=y +CONFIG_CMD_MEM_SEARCH=y CONFIG_CMD_IDE=y CONFIG_CMD_SPI=y CONFIG_CMD_USB=y # CONFIG_CMD_SETEXPR is not set CONFIG_BOOTP_BOOTFILESIZE=y +CONFIG_CMD_EFIDEBUG=y CONFIG_CMD_TIME=y CONFIG_CMD_QFW=y CONFIG_CMD_BOOTSTAGE=y @@ -57,5 +65,6 @@ CONFIG_FRAMEBUFFER_SET_VESA_MODE=y CONFIG_FRAMEBUFFER_VESA_MODE_USER=y CONFIG_FRAMEBUFFER_VESA_MODE=0x144 CONFIG_CONSOLE_SCROLL_LINES=5 +CONFIG_FAT_BLK_XLATE=y CONFIG_GENERATE_ACPI_TABLE=y # CONFIG_GZIP is not set
Add some hints and observations related to booting distros on QEMU on x86.
Signed-off-by: Simon Glass sjg@chromium.org ---
doc/board/emulation/qemu-x86.rst | 88 ++++++++++++++++++++++++++++++-- 1 file changed, 84 insertions(+), 4 deletions(-)
diff --git a/doc/board/emulation/qemu-x86.rst b/doc/board/emulation/qemu-x86.rst index e7dd4e994d38..15f56b6bc706 100644 --- a/doc/board/emulation/qemu-x86.rst +++ b/doc/board/emulation/qemu-x86.rst @@ -113,7 +113,87 @@ sure the specified CPU supports 64-bit like '-cpu core2duo'. Conversely '-cpu pentium' won't work for obvious reasons that the processor only supports 32-bit.
-Note 64-bit support is very preliminary at this point. Lots of features -are missing in the 64-bit world. One notable feature is the VGA console -support which is currently missing, so that you must specify '-nographic' -to get 64-bit U-Boot up and running. +Booting distros +--------------- + +It is possible to install and boot a standard Linux distribution using +qemu-x86_64 by setting up a root disk:: + + qemu-img create root.img 10G + +then using the installer to install. For example, with Ubuntu 2023.04:: + + qemu-system-x86_64 -m 8G -smp 4 -bios /tmp/b/qemu-x86_64/u-boot.rom \ + -drive file=root.img,if=virtio,driver=raw \ + -drive file=ubuntu-23.04-desktop-amd64.iso,if=virtio,driver=raw + +You can also add `-serial mon:stdio` if you want the serial console to show as +well as the video. + +The output will be something like this:: + + U-Boot SPL 2023.07 (Jul 23 2023 - 08:00:12 -0600) + Trying to boot from SPI + Jumping to 64-bit U-Boot: Note many features are missing + + + U-Boot 2023.07 (Jul 23 2023 - 08:00:12 -0600) + + CPU: QEMU Virtual CPU version 2.5+ + DRAM: 8 GiB + Core: 20 devices, 13 uclasses, devicetree: separate + Loading Environment from nowhere... OK + Model: QEMU x86 (I440FX) + Net: e1000: 52:54:00:12:34:56 + eth0: e1000#0 + Hit any key to stop autoboot: 0 + Scanning for bootflows in all bootdevs + Seq Method State Uclass Part Name Filename + --- ----------- ------ -------- ---- ------------------------ ---------------- + Scanning global bootmeth 'efi_mgr': + Hunting with: nvme + Hunting with: qfw + Hunting with: scsi + scanning bus for devices... + Hunting with: virtio + Scanning bootdev 'qfw_pio.bootdev': + fatal: no kernel available + Scanning bootdev 'virtio-blk#0.bootdev': + Scanning bootdev 'virtio-blk#1.bootdev': + 0 efi ready virtio 2 virtio-blk#1.bootdev.part efi/boot/bootx64.efi + ** Booting bootflow 'virtio-blk#1.bootdev.part_2' with efi + EFI using ACPI tables at f0060 + efi_install_fdt() WARNING: Can't have ACPI table and device tree - ignoring DT. + efi_run_image() Booting /efi\boot\bootx64.efi + error: file `/boot/' not found. + +Standard boot looks through various available devices and finds the virtio +disks, then boots from the first one. After a second or so the grub menu appears +and you can work through the installer flow normally. + +Note that standard boot will not find 32-bit distros, since it looks for a +different filename. + +Current limitations +------------------- + +Only qemu-x86-64 can be used for booting distros, since qemu-x86 (the 32-bit +version of U-Boot) seems to have an EFI bug leading to the boot handing after +Linux is selected from grub, e.g. with `debian-12.1.0-i386-netinst.iso`:: + + ** Booting bootflow 'virtio-blk#1.bootdev.part_2' with efi + EFI using ACPI tables at f0180 + efi_install_fdt() WARNING: Can't have ACPI table and device tree - ignoring DT. + efi_run_image() Booting /efi\boot\bootia32.efi + Failed to open efi\boot\root=/dev/sdb3 - Not Found + Failed to load image 큀緃: Not Found + start_image() returned Not Found, falling back to default loader + Welcome to GRUB! + +The bochs video driver also seems to cause problems before the OS is able to +show a display. + +Finally, the use of `-M accel=kvm` is intended to use the native CPU's +virtual-machine features to accelerate operation, but this causes U-Boot to hang +when jumping 64-bit mode, at least on AMD machines. This may be a bug in U-Boot +or something else.
participants (4)
-
Bin Meng -
Devarsh Thakkar -
Nikhil M Jain -
Simon Glass