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 */
