On Sun, 23 Feb 2020 12:47:51 +0100 (CET) Mark Kettenis <mark.kette...@xs4all.nl> wrote: >> Date: Sun, 23 Feb 2020 18:50:54 +0900 (JST) >> From: YASUOKA Masahiko <yasu...@yasuoka.net> >> >> On Sat, 22 Feb 2020 13:02:48 +1100 >> Jonathan Gray <j...@jsg.id.au> wrote: >> > On Fri, Feb 21, 2020 at 02:09:07PM +0900, YASUOKA Masahiko wrote: >> >> When efiboot starts the kernel, the video display becomes distorted >> >> and never recovered until CPU reset. >> >> >> >> Our kernel tries to initialized console twice, first trial is done >> >> before getting boot info and second trial is done after getting boot >> >> info. Since EFI framebuffer needs "boot info", it is initialized on >> >> second trial. >> >> >> >> On HPE DL20 Gen10, probing vga is succeeded on first trial, the kernel >> >> selects vga for the console, but actually it is broken. On usual >> >> machines which boot with EFI, the problem doesn't happen since they >> >> have no vga. >> >> >> >> The diff following fixes the problem by initializing efifb console >> >> even if the VGA is probed. >> >> >> >> # Also, HP DL20 Gen10 has "UEFI optimized boot" setting on BIOS and >> >> # disabling the setting avoids the problem happening. But since the >> >> # setting seems to be for old Windows, I think we should fix our >> >> # kernel. >> >> >> >> comment? ok? >> > >> > Is there a way to detect efi or bios before boot info is set? >> > Ideally vga_cnattach() would never be called when booting via efi. >> >> Yes. I've tried to find such the way, I found 2 ways. >> >> 1) ACPI has FADT_NO_VGA flag which indicate the system has VGA, but >> reading ACPI table at early of kernel boot is not good and difficult > > Reading it in efiboot would be fairly simple though.
I noticed FADT_NO_VGA is cleared on that machine... I'm sorry i hadn't checked this first. $ hexdump -C DL20.FACP.1 00000000 46 41 43 50 0c 01 00 00 06 ef 48 50 45 20 20 20 |FACP......HPE | 00000010 53 65 72 76 65 72 20 20 01 00 00 00 31 35 39 30 |Server ....1590| 00000020 01 00 00 00 00 00 dd 7b 00 40 fe 7b 00 04 09 00 |.......{.@.{....| 00000030 b2 00 00 00 a0 a1 f2 00 00 05 00 00 00 00 00 00 |................| 00000040 04 05 00 00 00 00 00 00 50 05 00 00 08 05 00 00 |........P.......| 00000050 60 05 00 00 00 00 00 00 04 02 01 04 20 00 10 00 |`........... ...| 00000060 65 00 e9 03 00 00 00 00 01 03 0d 00 32 33 00 00 |e...........23..| 00000070 a5 84 00 00 01 08 00 01 f9 0c 00 00 00 00 00 00 |................| 00000080 06 00 00 00 00 00 00 00 00 00 00 00 00 40 fe 7b |.............@.{| 00000090 00 00 00 00 01 20 00 02 00 05 00 00 00 00 00 00 |..... ..........| 000000a0 01 00 00 00 00 00 00 00 00 00 00 00 01 10 00 02 |................| 000000b0 04 05 00 00 00 00 00 00 01 00 00 00 00 00 00 00 |................| 000000c0 00 00 00 00 01 08 00 00 50 05 00 00 00 00 00 00 |........P.......| 000000d0 01 20 00 03 08 05 00 00 00 00 00 00 01 ff 00 01 |. ..............| 000000e0 60 05 00 00 00 00 00 00 01 00 00 00 00 00 00 00 |`...............| 000000f0 00 00 00 00 01 08 00 00 00 00 00 00 00 00 00 00 |................| 00000100 01 08 00 00 00 00 00 00 00 00 00 00 |............| 0000010c $ "iapc_boot_arch" field is at offset 0x6d-0x6e. It's 0x0033. #define FADT_LEGACY_DEVICES 0x0001 /* Legacy devices supported */ #define FADT_i8042 0x0002 /* Keyboard controller present */ #define FADT_NO_VGA 0x0004 /* Do not probe VGA */ #define FADT_NO_MSI 0x0008 /* Do not enable MSI */ The bit is cleared. >> 2) Pass a flag from efiboot. A diff for this is attached. > > So the EFI bootloader could pass a BAPIV_NOVGA fairly trivially. In > that case we probably should add the check for the flag in > wscn_video_init(). I created a diff doing this. Attached below. >> > Should the cninit() before the boot args are parsed be removed and just >> > have cninit() unconditionally after? This would make the debug printfs >> > in boot arg passing useless, but they already wouldn't work when booting >> > via efi. >> >> I think this is a straight way and no downside for efi. For a system >> booting via BIOS, there is a downside that panic or debug string isn't >> shown at very early part of kernel boot. >> Index: sys/stand/boot/bootarg.h =================================================================== RCS file: /var/cvs/openbsd/src/sys/stand/boot/bootarg.h,v retrieving revision 1.15 diff -u -p -r1.15 bootarg.h --- sys/stand/boot/bootarg.h 8 Apr 2018 13:24:36 -0000 1.15 +++ sys/stand/boot/bootarg.h 27 Feb 2020 11:48:08 -0000 @@ -32,6 +32,7 @@ #define BAPIV_VECTOR 0x00000002 /* MI vector of MD structures passed */ #define BAPIV_ENV 0x00000004 /* MI environment vars vector */ #define BAPIV_BMEMMAP 0x00000008 /* MI memory map passed is in bytes */ +#define BAPIV_NOVGA 0x00000100 /* MI VGA is not present */ typedef struct _boot_args { int ba_type; Index: sys/arch/amd64/amd64/wscons_machdep.c =================================================================== RCS file: /var/cvs/openbsd/src/sys/arch/amd64/amd64/wscons_machdep.c,v retrieving revision 1.14 diff -u -p -r1.14 wscons_machdep.c --- sys/arch/amd64/amd64/wscons_machdep.c 14 Oct 2017 04:44:43 -0000 1.14 +++ sys/arch/amd64/amd64/wscons_machdep.c 27 Feb 2020 11:48:08 -0000 @@ -33,6 +33,7 @@ #include <machine/bus.h> #include <dev/cons.h> +#include <stand/boot/bootarg.h> #include "vga.h" #include "pcdisplay.h" @@ -72,6 +73,7 @@ #if NEFIFB > 0 #include <machine/efifbvar.h> #endif +#include <machine/biosvar.h> int wscn_video_init(void); void wscn_input_init(int); @@ -141,7 +143,8 @@ wscn_video_init(void) return (0); #endif #if (NVGA > 0) - if (vga_cnattach(X86_BUS_SPACE_IO, X86_BUS_SPACE_MEM, -1, 1) == 0) + if (!ISSET(bootapiver, BAPIV_NOVGA) && + vga_cnattach(X86_BUS_SPACE_IO, X86_BUS_SPACE_MEM, -1, 1) == 0) return (0); #endif #if (NEFIFB > 0) @@ -149,7 +152,8 @@ wscn_video_init(void) return (0); #endif #if (NPCDISPLAY > 0) - if (pcdisplay_cnattach(X86_BUS_SPACE_IO, X86_BUS_SPACE_MEM) == 0) + if (!ISSET(bootapiver, BAPIV_NOVGA) && + pcdisplay_cnattach(X86_BUS_SPACE_IO, X86_BUS_SPACE_MEM) == 0) return (0); #endif return (-1); Index: sys/arch/amd64/stand/efiboot/efiboot.c =================================================================== RCS file: /var/cvs/openbsd/src/sys/arch/amd64/stand/efiboot/efiboot.c,v retrieving revision 1.34 diff -u -p -r1.34 efiboot.c --- sys/arch/amd64/stand/efiboot/efiboot.c 29 Nov 2019 16:16:19 -0000 1.34 +++ sys/arch/amd64/stand/efiboot/efiboot.c 27 Feb 2020 11:48:08 -0000 @@ -19,6 +19,7 @@ #include <sys/param.h> #include <sys/queue.h> #include <dev/cons.h> +#include <dev/acpi/acpireg.h> #include <dev/isa/isareg.h> #include <dev/ic/comreg.h> #include <sys/disklabel.h> @@ -65,6 +66,12 @@ static EFI_STATUS efi_gop_setmode(int mode); EFI_STATUS efi_main(EFI_HANDLE, EFI_SYSTEM_TABLE *); +static uint8_t acpi_cksum(u_char *, size_t); +static struct acpi_rsdp + *acpi_get_rsdp(uintptr_t); +static struct acpi_fadt + *acpi_get_fadt(struct acpi_rsdp *); + void (*run_i386)(u_long, u_long, int, int, int, int, int, int, int, int) __attribute__((noreturn)); @@ -822,7 +829,7 @@ efi_gop_setmode(int mode) } void -efi_makebootargs(void) +efi_makebootargs(int *bapiv_extra) { int i; EFI_STATUS status; @@ -831,6 +838,8 @@ efi_makebootargs(void) bios_efiinfo_t *ei = &bios_efiinfo; int curmode; UINTN sz, gopsiz, bestsiz = 0; + struct acpi_rsdp *rdsp; + struct acpi_fadt *fadt; /* * ACPI, BIOS configuration table @@ -913,6 +922,18 @@ efi_makebootargs(void) #endif addbootarg(BOOTARG_EFIINFO, sizeof(bios_efiinfo), &bios_efiinfo); + + /* + * Setup extra flags for boot api version + */ + if (ei->config_acpi != 0) { + /* Pass FADT_NO_VGA flag as BAPIV_NOVGA */ + if ((rdsp = acpi_get_rsdp(ei->config_acpi)) != NULL && + (fadt = acpi_get_fadt(rdsp)) != NULL) { + if ((fadt->iapc_boot_arch & FADT_NO_VGA) != 0) + *bapiv_extra |= BAPIV_NOVGA; + } + } } void @@ -1058,4 +1079,79 @@ Xgop_efi(void) printf("Current Mode = %d\n", gop->Mode->Mode); return (0); +} + +uint8_t +acpi_cksum(u_char *ptr, size_t len) +{ + int i; + uint8_t sum = 0; + + for (i = 0; i< len; i++) + sum += *(ptr + i); + + return (sum); +} + +struct acpi_rsdp * +acpi_get_rsdp(uintptr_t addr) +{ + struct acpi_rsdp *rsdp = (struct acpi_rsdp *)addr; + + /* has signature and valid checksum? */ + if (bcmp(rsdp->rsdp_signaturee, RSDP_SIG, + sizeof(rsdp->rsdp_signaturee)) == 0 && + acpi_cksum((u_char *)rsdp, sizeof(rsdp->rsdp1)) == 0) { + /* version 1 or version 2 with valid version 2 checksum? */ + if (rsdp->rsdp_revision == 1 || + (rsdp->rsdp_revision == 2 && + acpi_cksum((u_char *)rsdp, rsdp->rsdp_length) == 0)) + return (rsdp); + } + + return (NULL); +} + +struct acpi_fadt * +acpi_get_fadt(struct acpi_rsdp *rsdp) +{ + int i, n; + struct acpi_rsdt *rsdt; + struct acpi_xsdt *xsdt; + struct acpi_table_header + *hdr = NULL; + struct acpi_fadt *fadt = NULL; + + if (rsdp->rsdp_revision == 1) { + rsdt = (struct acpi_rsdt *)(uintptr_t)rsdp->rsdp_rsdt; + n = (rsdt->hdr_length - sizeof(struct acpi_table_header)) / + sizeof(uint32_t); + for (i = 0; i < n; i++) { + hdr = (struct acpi_table_header *)(uintptr_t) + rsdt->table_offsets[i]; + if (bcmp(hdr->signature, FADT_SIG, + sizeof(hdr->signature)) == 0) { + fadt = (struct acpi_fadt *)hdr; + break; + } + } + } else if (rsdp->rsdp_revision == 2) { + xsdt = (struct acpi_xsdt *)(uintptr_t)rsdp->rsdp_xsdt; + n = (xsdt->hdr_length - sizeof(struct acpi_table_header)) / + sizeof(uint64_t); + for (i = 0; i < n; i++) { + hdr = (struct acpi_table_header *) + xsdt->table_offsets[i]; + if (bcmp(hdr->signature, FADT_SIG, + sizeof(hdr->signature)) == 0) { + fadt = (struct acpi_fadt *)hdr; + break; + } + } + } + if (fadt != NULL) + if (acpi_cksum((u_char *)fadt, fadt->hdr_length) == 0) + return (fadt); + + return (NULL); } Index: sys/arch/amd64/stand/efiboot/efiboot.h =================================================================== RCS file: /var/cvs/openbsd/src/sys/arch/amd64/stand/efiboot/efiboot.h,v retrieving revision 1.4 diff -u -p -r1.4 efiboot.h --- sys/arch/amd64/stand/efiboot/efiboot.h 25 Nov 2017 19:02:07 -0000 1.4 +++ sys/arch/amd64/stand/efiboot/efiboot.h 27 Feb 2020 11:48:08 -0000 @@ -33,7 +33,7 @@ void efi_com_putc(dev_t, int); int Xvideo_efi(void); int Xgop_efi(void); int Xexit_efi(void); -void efi_makebootargs(void); +void efi_makebootargs(int *); int Xpoweroff_efi(void); Index: sys/arch/amd64/stand/efiboot/exec_i386.c =================================================================== RCS file: /var/cvs/openbsd/src/sys/arch/amd64/stand/efiboot/exec_i386.c,v retrieving revision 1.3 diff -u -p -r1.3 exec_i386.c --- sys/arch/amd64/stand/efiboot/exec_i386.c 12 Dec 2019 13:09:35 -0000 1.3 +++ sys/arch/amd64/stand/efiboot/exec_i386.c 27 Feb 2020 11:48:08 -0000 @@ -85,11 +85,13 @@ run_loadfile(uint64_t *marks, int howto) #endif int i; u_long delta; + u_int bootapiver = BOOTARG_APIVER; extern u_long efi_loadaddr; if ((av = alloc(ac)) == NULL) panic("alloc for bootarg"); - efi_makebootargs(); + efi_makebootargs(&bootapiver); + delta = DEFAULT_KERNEL_ADDRESS - efi_loadaddr; if (sa_cleanup != NULL) (*sa_cleanup)(); @@ -163,11 +165,11 @@ run_loadfile(uint64_t *marks, int howto) marks[i] += delta; #ifdef __amd64__ - (*run_i386)((u_long)run_i386, entry, howto, bootdev, BOOTARG_APIVER, + (*run_i386)((u_long)run_i386, entry, howto, bootdev, bootapiver, marks[MARK_END], extmem, cnvmem, ac, (intptr_t)av); #else /* stack and the gung is ok at this point, so, no need for asm setup */ - (*(startfuncp)entry)(howto, bootdev, BOOTARG_APIVER, marks[MARK_END], + (*(startfuncp)entry)(howto, bootdev, bootapiver, marks[MARK_END], extmem, cnvmem, ac, (int)av); #endif /* not reached */