On Sun, 23 Feb 2020 18:50:54 +0900 (JST)
YASUOKA Masahiko <[email protected]> wrote:
> On Sat, 22 Feb 2020 13:02:48 +1100
> Jonathan Gray <[email protected]> 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
>
> 2) Pass a flag from efiboot. A diff for this is attached.
>
>> 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.
A diff for this is attached.
1st diff
- initialize efifb even if vga is probed
2nd diff
- pass a flag from efiboot, then initialize vga/efifb properly with it
3rd diff
- parse bootarg first, then initialize vga/efifb properly
I think 3rd diff is the best. Because it makes the code simple and
the downside doesn't seem so serious.
Index: sys/arch/amd64/amd64/machdep.c
===================================================================
RCS file: /disk/cvs/openbsd/src/sys/arch/amd64/amd64/machdep.c,v
retrieving revision 1.261
diff -u -p -r1.261 machdep.c
--- sys/arch/amd64/amd64/machdep.c 24 Jan 2020 05:27:31 -0000 1.261
+++ sys/arch/amd64/amd64/machdep.c 23 Feb 2020 09:46:54 -0000
@@ -1394,16 +1394,6 @@ init_x86_64(paddr_t first_avail)
i8254_startclock();
/*
- * Attach the glass console early in case we need to display a panic.
- */
- cninit();
-
- /*
- * Initialize PAGE_SIZE-dependent variables.
- */
- uvm_setpagesize();
-
- /*
* Boot arguments are in a single page specified by /boot.
*
* We require the "new" vector form, as well as memory ranges
@@ -1420,6 +1410,16 @@ init_x86_64(paddr_t first_avail)
} else
panic("invalid /boot");
+ /*
+ * Attach the glass console early in case we need to display a panic.
+ */
+ cninit();
+
+ /*
+ * Initialize PAGE_SIZE-dependent variables.
+ */
+ uvm_setpagesize();
+
/*
* Memory on the AMD64 port is described by three different things.
*
@@ -1928,11 +1928,6 @@ getbootinfo(char *bootinfo, int bootinfo
bios_bootsr_t *bios_bootsr;
int docninit = 0;
-#undef BOOTINFO_DEBUG
-#ifdef BOOTINFO_DEBUG
- printf("bootargv:");
-#endif
-
for (q = (bootarg32_t *)bootinfo;
(q->ba_type != BOOTARG_END) &&
((((char *)q) - bootinfo) < bootinfo_size);
@@ -1941,24 +1936,15 @@ getbootinfo(char *bootinfo, int bootinfo
switch (q->ba_type) {
case BOOTARG_MEMMAP:
bios_memmap = (bios_memmap_t *)q->ba_arg;
-#ifdef BOOTINFO_DEBUG
- printf(" memmap %p", bios_memmap);
-#endif
break;
case BOOTARG_DISKINFO:
bios_diskinfo = (bios_diskinfo_t *)q->ba_arg;
-#ifdef BOOTINFO_DEBUG
- printf(" diskinfo %p", bios_diskinfo);
-#endif
break;
case BOOTARG_APMINFO:
/* generated by i386 boot loader */
break;
case BOOTARG_CKSUMLEN:
bios_cksumlen = *(u_int32_t *)q->ba_arg;
-#ifdef BOOTINFO_DEBUG
- printf(" cksumlen %d", bios_cksumlen);
-#endif
break;
case BOOTARG_PCIINFO:
/* generated by i386 boot loader */
@@ -1987,10 +1973,6 @@ getbootinfo(char *bootinfo, int bootinfo
docninit++;
}
#endif
-#ifdef BOOTINFO_DEBUG
- printf(" console 0x%x:%d",
- cdp->consdev, cdp->conspeed);
-#endif
}
break;
case BOOTARG_BOOTMAC:
@@ -2031,18 +2013,11 @@ getbootinfo(char *bootinfo, int bootinfo
break;
default:
-#ifdef BOOTINFO_DEBUG
- printf(" unsupported arg (%d) %p", q->ba_type,
- q->ba_arg);
-#endif
break;
}
}
if (docninit > 0)
cninit();
-#ifdef BOOTINFO_DEBUG
- printf("\n");
-#endif
}
int
