Re: diff: init efifb even if VGA is probed.
On Thu, May 28, 2020 at 02:06:28PM +0200, Mark Kettenis wrote: > > Date: Thu, 28 May 2020 20:49:18 +0900 (JST) > > From: YASUOKA Masahiko > > > > On Thu, 28 May 2020 12:31:31 +0200 (CEST) > > Mark Kettenis wrote: > > >> Date: Thu, 28 May 2020 17:01:48 +0900 (JST) > > >> From: YASUOKA Masahiko > > >> > > >> Hi, > > >> > > >> I'd like to conclude this issue. > > >> > > >> On Fri, 21 Feb 2020 14:09:07 +0900 (JST) > > >> YASUOKA Masahiko wrote: > > >> > I am testing a new hardware, HPE DL20 Gen10. > > >> > > > >> > 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. > > >> > > >> If we have a way to detect whether the machine has VGA. ACPI > > >> FADT_NO_VGA was a candidate. But that bit is cleard both on my "HPE > > >> DL20 Gen10" and Andrew Daugherity's Dell PowerEdge R230. Also the > > >> problem newly posted at misc@ (*) might be the same problem. > > >> > > >> (*) https://marc.info/?l=openbsd-misc=159064773219779=2 > > >> > > >> I think having the diff folowing is the best for this momemnt. > > >> The diff does: > > >> > > >> - move cninit() after parsing bootinfo > > >> - give up the debug message which can be enabled if BOOTINFO_DEBUG is > > >> defined > > >> > > >> ok? > > > > > > I suspect we have to accept that there is too much broken hardware out > > > there. > > > > Finally we might have no way other than having a configuration in > > boot.conf... > > > > > There is no real reason to drop the debug messages. > > > > OK, the debug messages are reverted. > > > > > I'd prefer to call cninit() directly from init_x86_64, so I'd just > > > move the call immediately after the block that calls getbootinfo(). > > > And emove the call from getbootinfo() of course. > > > > I think the last diff already satisfied these things. > > > > >> @@ -1395,11 +1395,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(); > > >> @@ -1421,6 +1416,8 @@ init_x86_64(paddr_t first_avail) > > >> } else > > >> panic("invalid /boot"); > > >> > > >> +cninit(); > > >> + > > >> /* > > >> * Memory on the AMD64 port is described by three different things. > > >> * > > > > A hidden line which calls getbootinfo() is at just before second > > chunk. The updated diff was created with "-U 4" to clarify this. > > > > Alternatively, are you suggesting > > > > getbootinfo(bootinfo, bootinfo_size); > > + cninit(); > > } else > > panic("invalid /boot"); > > > > ? > > > > Both is OK for me. > > > > How about this? > > The attached diff looks good to me. > > ok kettenis@ ok jsg@ on this one as well > > > Index: sys/arch/amd64/amd64/machdep.c > > === > > RCS file: /cvs/src/sys/arch/amd64/amd64/machdep.c,v > > retrieving revision 1.264 > > diff -u -p -U4 -r1.264 machdep.c > > --- sys/arch/amd64/amd64/machdep.c 16 May 2020 14:44:44 - 1.264 > > +++ sys/arch/amd64/amd64/machdep.c 28 May 2020 11:34:39 - > > @@ -1394,13 +1394,8 @@ 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(); > > > > @@ -1420,8 +1415,10 @@ init_x86_64(paddr_t first_avail) > > getbootinfo(bootinfo, bootinfo_size); > > } else > > panic("invalid /boot"); > > > > + cninit(); > > + > > /* > > * Memory on the AMD64 port is described by three different things. > > * > > * 1. biosbasemem - This is outdated, and should really only be used to > > @@ -1926,10 +1923,8 @@ getbootinfo(char *bootinfo, int bootinfo > > bootarg32_t *q; > > bios_ddb_t *bios_ddb; > > bios_bootduid_t *bios_bootduid; > > bios_bootsr_t *bios_bootsr; > > - int docninit = 0; > > - > > #undef BOOTINFO_DEBUG > > #ifdef BOOTINFO_DEBUG > > printf("bootargv:"); > > #endif > > @@ -1982,11 +1977,8 @@ getbootinfo(char
Re: diff: init efifb even if VGA is probed.
> Date: Thu, 28 May 2020 20:49:18 +0900 (JST) > From: YASUOKA Masahiko > > On Thu, 28 May 2020 12:31:31 +0200 (CEST) > Mark Kettenis wrote: > >> Date: Thu, 28 May 2020 17:01:48 +0900 (JST) > >> From: YASUOKA Masahiko > >> > >> Hi, > >> > >> I'd like to conclude this issue. > >> > >> On Fri, 21 Feb 2020 14:09:07 +0900 (JST) > >> YASUOKA Masahiko wrote: > >> > I am testing a new hardware, HPE DL20 Gen10. > >> > > >> > 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. > >> > >> If we have a way to detect whether the machine has VGA. ACPI > >> FADT_NO_VGA was a candidate. But that bit is cleard both on my "HPE > >> DL20 Gen10" and Andrew Daugherity's Dell PowerEdge R230. Also the > >> problem newly posted at misc@ (*) might be the same problem. > >> > >> (*) https://marc.info/?l=openbsd-misc=159064773219779=2 > >> > >> I think having the diff folowing is the best for this momemnt. > >> The diff does: > >> > >> - move cninit() after parsing bootinfo > >> - give up the debug message which can be enabled if BOOTINFO_DEBUG is > >> defined > >> > >> ok? > > > > I suspect we have to accept that there is too much broken hardware out > > there. > > Finally we might have no way other than having a configuration in > boot.conf... > > > There is no real reason to drop the debug messages. > > OK, the debug messages are reverted. > > > I'd prefer to call cninit() directly from init_x86_64, so I'd just > > move the call immediately after the block that calls getbootinfo(). > > And emove the call from getbootinfo() of course. > > I think the last diff already satisfied these things. > > >> @@ -1395,11 +1395,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(); > >> @@ -1421,6 +1416,8 @@ init_x86_64(paddr_t first_avail) > >>} else > >>panic("invalid /boot"); > >> > >> + cninit(); > >> + > >> /* > >> * Memory on the AMD64 port is described by three different things. > >> * > > A hidden line which calls getbootinfo() is at just before second > chunk. The updated diff was created with "-U 4" to clarify this. > > Alternatively, are you suggesting > > getbootinfo(bootinfo, bootinfo_size); > + cninit(); > } else > panic("invalid /boot"); > > ? > > Both is OK for me. > > How about this? The attached diff looks good to me. ok kettenis@ > Index: sys/arch/amd64/amd64/machdep.c > === > RCS file: /cvs/src/sys/arch/amd64/amd64/machdep.c,v > retrieving revision 1.264 > diff -u -p -U4 -r1.264 machdep.c > --- sys/arch/amd64/amd64/machdep.c16 May 2020 14:44:44 - 1.264 > +++ sys/arch/amd64/amd64/machdep.c28 May 2020 11:34:39 - > @@ -1394,13 +1394,8 @@ 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(); > > @@ -1420,8 +1415,10 @@ init_x86_64(paddr_t first_avail) > getbootinfo(bootinfo, bootinfo_size); > } else > panic("invalid /boot"); > > + cninit(); > + > /* > * Memory on the AMD64 port is described by three different things. > * > * 1. biosbasemem - This is outdated, and should really only be used to > @@ -1926,10 +1923,8 @@ getbootinfo(char *bootinfo, int bootinfo > bootarg32_t *q; > bios_ddb_t *bios_ddb; > bios_bootduid_t *bios_bootduid; > bios_bootsr_t *bios_bootsr; > - int docninit = 0; > - > #undef BOOTINFO_DEBUG > #ifdef BOOTINFO_DEBUG > printf("bootargv:"); > #endif > @@ -1982,11 +1977,8 @@ getbootinfo(char *bootinfo, int bootinfo > comconsunit = unit; > comconsaddr = consaddr; > comconsrate = cdp->conspeed; > comconsiot = X86_BUS_SPACE_IO; > - > - /* Probe the serial port this time. */ > -
Re: diff: init efifb even if VGA is probed.
On Thu, 28 May 2020 12:31:31 +0200 (CEST) Mark Kettenis wrote: >> Date: Thu, 28 May 2020 17:01:48 +0900 (JST) >> From: YASUOKA Masahiko >> >> Hi, >> >> I'd like to conclude this issue. >> >> On Fri, 21 Feb 2020 14:09:07 +0900 (JST) >> YASUOKA Masahiko wrote: >> > I am testing a new hardware, HPE DL20 Gen10. >> > >> > 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. >> >> If we have a way to detect whether the machine has VGA. ACPI >> FADT_NO_VGA was a candidate. But that bit is cleard both on my "HPE >> DL20 Gen10" and Andrew Daugherity's Dell PowerEdge R230. Also the >> problem newly posted at misc@ (*) might be the same problem. >> >> (*) https://marc.info/?l=openbsd-misc=159064773219779=2 >> >> I think having the diff folowing is the best for this momemnt. >> The diff does: >> >> - move cninit() after parsing bootinfo >> - give up the debug message which can be enabled if BOOTINFO_DEBUG is >> defined >> >> ok? > > I suspect we have to accept that there is too much broken hardware out > there. Finally we might have no way other than having a configuration in boot.conf... > There is no real reason to drop the debug messages. OK, the debug messages are reverted. > I'd prefer to call cninit() directly from init_x86_64, so I'd just > move the call immediately after the block that calls getbootinfo(). > And emove the call from getbootinfo() of course. I think the last diff already satisfied these things. >> @@ -1395,11 +1395,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(); >> @@ -1421,6 +1416,8 @@ init_x86_64(paddr_t first_avail) >> } else >> panic("invalid /boot"); >> >> +cninit(); >> + >> /* >> * Memory on the AMD64 port is described by three different things. >> * A hidden line which calls getbootinfo() is at just before second chunk. The updated diff was created with "-U 4" to clarify this. Alternatively, are you suggesting getbootinfo(bootinfo, bootinfo_size); + cninit(); } else panic("invalid /boot"); ? Both is OK for me. How about this? Index: sys/arch/amd64/amd64/machdep.c === RCS file: /cvs/src/sys/arch/amd64/amd64/machdep.c,v retrieving revision 1.264 diff -u -p -U4 -r1.264 machdep.c --- sys/arch/amd64/amd64/machdep.c 16 May 2020 14:44:44 - 1.264 +++ sys/arch/amd64/amd64/machdep.c 28 May 2020 11:34:39 - @@ -1394,13 +1394,8 @@ 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(); @@ -1420,8 +1415,10 @@ init_x86_64(paddr_t first_avail) getbootinfo(bootinfo, bootinfo_size); } else panic("invalid /boot"); + cninit(); + /* * Memory on the AMD64 port is described by three different things. * * 1. biosbasemem - This is outdated, and should really only be used to @@ -1926,10 +1923,8 @@ getbootinfo(char *bootinfo, int bootinfo bootarg32_t *q; bios_ddb_t *bios_ddb; bios_bootduid_t *bios_bootduid; bios_bootsr_t *bios_bootsr; - int docninit = 0; - #undef BOOTINFO_DEBUG #ifdef BOOTINFO_DEBUG printf("bootargv:"); #endif @@ -1982,11 +1977,8 @@ getbootinfo(char *bootinfo, int bootinfo comconsunit = unit; comconsaddr = consaddr; comconsrate = cdp->conspeed; comconsiot = X86_BUS_SPACE_IO; - - /* Probe the serial port this time. */ - docninit++; } #endif #ifdef BOOTINFO_DEBUG printf(" console 0x%x:%d", @@ -2022,10 +2014,8 @@ getbootinfo(char *bootinfo, int bootinfo break; case BOOTARG_EFIINFO: bios_efiinfo = (bios_efiinfo_t *)q->ba_arg;
Re: diff: init efifb even if VGA is probed.
> Date: Thu, 28 May 2020 17:01:48 +0900 (JST) > From: YASUOKA Masahiko > > Hi, > > I'd like to conclude this issue. > > On Fri, 21 Feb 2020 14:09:07 +0900 (JST) > YASUOKA Masahiko wrote: > > I am testing a new hardware, HPE DL20 Gen10. > > > > 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. > > If we have a way to detect whether the machine has VGA. ACPI > FADT_NO_VGA was a candidate. But that bit is cleard both on my "HPE > DL20 Gen10" and Andrew Daugherity's Dell PowerEdge R230. Also the > problem newly posted at misc@ (*) might be the same problem. > > (*) https://marc.info/?l=openbsd-misc=159064773219779=2 > > I think having the diff folowing is the best for this momemnt. > The diff does: > > - move cninit() after parsing bootinfo > - give up the debug message which can be enabled if BOOTINFO_DEBUG is > defined > > ok? I suspect we have to accept that there is too much broken hardware out there. There is no real reason to drop the debug messages. I'd prefer to call cninit() directly from init_x86_64, so I'd just move the call immediately after the block that calls getbootinfo(). And emove the call from getbootinfo() of course. > Index: sys/arch/amd64/amd64/machdep.c > === > RCS file: /disk/cvs/openbsd/src/sys/arch/amd64/amd64/machdep.c,v > retrieving revision 1.264 > diff -u -p -r1.264 machdep.c > --- sys/arch/amd64/amd64/machdep.c16 May 2020 14:44:44 - 1.264 > +++ sys/arch/amd64/amd64/machdep.c28 May 2020 07:40:14 - > @@ -1395,11 +1395,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(); > @@ -1421,6 +1416,8 @@ init_x86_64(paddr_t first_avail) > } else > panic("invalid /boot"); > > + cninit(); > + > /* > * Memory on the AMD64 port is described by three different things. > * > @@ -1927,12 +1924,6 @@ getbootinfo(char *bootinfo, int bootinfo > bios_ddb_t *bios_ddb; > bios_bootduid_t *bios_bootduid; > 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) && > @@ -1942,24 +1933,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 */ > @@ -1983,15 +1965,8 @@ getbootinfo(char *bootinfo, int bootinfo > comconsaddr = consaddr; > comconsrate = cdp->conspeed; > comconsiot = X86_BUS_SPACE_IO; > - > - /* Probe the serial port this time. */ > - docninit++; > } > #endif > -#ifdef BOOTINFO_DEBUG > - printf(" console 0x%x:%d", > - cdp->consdev, cdp->conspeed); > -#endif > } > break; > case BOOTARG_BOOTMAC: > @@ -2023,8 +1998,6 @@ getbootinfo(char *bootinfo, int bootinfo > > case BOOTARG_EFIINFO: > bios_efiinfo = (bios_efiinfo_t *)q->ba_arg; > - if (bios_efiinfo->fb_addr != 0) > -
Re: diff: init efifb even if VGA is probed.
On Thu, May 28, 2020 at 05:19:19PM +0900, YASUOKA Masahiko wrote: > On Thu, 28 May 2020 17:01:48 +0900 (JST) > YASUOKA Masahiko wrote: > > Hi, > > > > I'd like to conclude this issue. > > > > On Fri, 21 Feb 2020 14:09:07 +0900 (JST) > > YASUOKA Masahiko wrote: > >> I am testing a new hardware, HPE DL20 Gen10. > >> > >> 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. > > > > If we have a way to detect whether the machine has VGA. ACPI > > FADT_NO_VGA was a candidate. But that bit is cleard both on my "HPE > > DL20 Gen10" and Andrew Daugherity's Dell PowerEdge R230. Also the > > problem newly posted at misc@ (*) might be the same problem. > > Above paragraph may be unclear. Let me update it by the following > paragraph. > > If we have a way to detect whether the machine has VGA, we thought we > can select VGA or EFI framebuffer safely. ACPI FADT_NO_VGA was a > candidate. But the bit is cleared both on my "HPE DL20 Gen10" and > Andrew Daugherity's Dell PowerEdge R230. Also the problem newly > posted at misc@ (*) might be the same problem. I agree this approach of waiting for bootargs before probing for a console is the best way to handle this at the moment unless someone can come up with a way to stop vga matching. I have tested this without problems on bios boot serial console bios boot vga/inteldrm console efi boot efifb/amdgpu console ok jsg@ > > > (*) https://marc.info/?l=openbsd-misc=159064773219779=2 > > > > I think having the diff folowing is the best for this momemnt. > > The diff does: > > > > - move cninit() after parsing bootinfo > > - give up the debug message which can be enabled if BOOTINFO_DEBUG is > > defined > > > > ok? > > > > Index: sys/arch/amd64/amd64/machdep.c > > === > > RCS file: /disk/cvs/openbsd/src/sys/arch/amd64/amd64/machdep.c,v > > retrieving revision 1.264 > > diff -u -p -r1.264 machdep.c > > --- sys/arch/amd64/amd64/machdep.c 16 May 2020 14:44:44 - 1.264 > > +++ sys/arch/amd64/amd64/machdep.c 28 May 2020 07:40:14 - > > @@ -1395,11 +1395,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(); > > @@ -1421,6 +1416,8 @@ init_x86_64(paddr_t first_avail) > > } else > > panic("invalid /boot"); > > > > + cninit(); > > + > > /* > > * Memory on the AMD64 port is described by three different things. > > * > > @@ -1927,12 +1924,6 @@ getbootinfo(char *bootinfo, int bootinfo > > bios_ddb_t *bios_ddb; > > bios_bootduid_t *bios_bootduid; > > 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) && > > @@ -1942,24 +1933,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 */ > > @@ -1983,15 +1965,8 @@ getbootinfo(char *bootinfo, int bootinfo > > comconsaddr = consaddr; > > comconsrate = cdp->conspeed; > > comconsiot = X86_BUS_SPACE_IO; > > - > > - /* Probe the serial port this time. */ > > -
Re: diff: init efifb even if VGA is probed.
On Thu, 28 May 2020 17:01:48 +0900 (JST) YASUOKA Masahiko wrote: > Hi, > > I'd like to conclude this issue. > > On Fri, 21 Feb 2020 14:09:07 +0900 (JST) > YASUOKA Masahiko wrote: >> I am testing a new hardware, HPE DL20 Gen10. >> >> 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. > > If we have a way to detect whether the machine has VGA. ACPI > FADT_NO_VGA was a candidate. But that bit is cleard both on my "HPE > DL20 Gen10" and Andrew Daugherity's Dell PowerEdge R230. Also the > problem newly posted at misc@ (*) might be the same problem. Above paragraph may be unclear. Let me update it by the following paragraph. If we have a way to detect whether the machine has VGA, we thought we can select VGA or EFI framebuffer safely. ACPI FADT_NO_VGA was a candidate. But the bit is cleared both on my "HPE DL20 Gen10" and Andrew Daugherity's Dell PowerEdge R230. Also the problem newly posted at misc@ (*) might be the same problem. > (*) https://marc.info/?l=openbsd-misc=159064773219779=2 > > I think having the diff folowing is the best for this momemnt. > The diff does: > > - move cninit() after parsing bootinfo > - give up the debug message which can be enabled if BOOTINFO_DEBUG is > defined > > ok? > > Index: sys/arch/amd64/amd64/machdep.c > === > RCS file: /disk/cvs/openbsd/src/sys/arch/amd64/amd64/machdep.c,v > retrieving revision 1.264 > diff -u -p -r1.264 machdep.c > --- sys/arch/amd64/amd64/machdep.c16 May 2020 14:44:44 - 1.264 > +++ sys/arch/amd64/amd64/machdep.c28 May 2020 07:40:14 - > @@ -1395,11 +1395,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(); > @@ -1421,6 +1416,8 @@ init_x86_64(paddr_t first_avail) > } else > panic("invalid /boot"); > > + cninit(); > + > /* > * Memory on the AMD64 port is described by three different things. > * > @@ -1927,12 +1924,6 @@ getbootinfo(char *bootinfo, int bootinfo > bios_ddb_t *bios_ddb; > bios_bootduid_t *bios_bootduid; > 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) && > @@ -1942,24 +1933,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 */ > @@ -1983,15 +1965,8 @@ getbootinfo(char *bootinfo, int bootinfo > comconsaddr = consaddr; > comconsrate = cdp->conspeed; > comconsiot = X86_BUS_SPACE_IO; > - > - /* Probe the serial port this time. */ > - docninit++; > } > #endif > -#ifdef BOOTINFO_DEBUG > - printf(" console 0x%x:%d", > - cdp->consdev, cdp->conspeed); > -#endif > } > break; > case BOOTARG_BOOTMAC: > @@ -2023,8 +1998,6 @@ getbootinfo(char *bootinfo, int bootinfo > > case BOOTARG_EFIINFO: > bios_efiinfo = (bios_efiinfo_t *)q->ba_arg; > -
Re: diff: init efifb even if VGA is probed.
Hi, I'd like to conclude this issue. On Fri, 21 Feb 2020 14:09:07 +0900 (JST) YASUOKA Masahiko wrote: > I am testing a new hardware, HPE DL20 Gen10. > > 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. If we have a way to detect whether the machine has VGA. ACPI FADT_NO_VGA was a candidate. But that bit is cleard both on my "HPE DL20 Gen10" and Andrew Daugherity's Dell PowerEdge R230. Also the problem newly posted at misc@ (*) might be the same problem. (*) https://marc.info/?l=openbsd-misc=159064773219779=2 I think having the diff folowing is the best for this momemnt. The diff does: - move cninit() after parsing bootinfo - give up the debug message which can be enabled if BOOTINFO_DEBUG is defined ok? Index: sys/arch/amd64/amd64/machdep.c === RCS file: /disk/cvs/openbsd/src/sys/arch/amd64/amd64/machdep.c,v retrieving revision 1.264 diff -u -p -r1.264 machdep.c --- sys/arch/amd64/amd64/machdep.c 16 May 2020 14:44:44 - 1.264 +++ sys/arch/amd64/amd64/machdep.c 28 May 2020 07:40:14 - @@ -1395,11 +1395,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(); @@ -1421,6 +1416,8 @@ init_x86_64(paddr_t first_avail) } else panic("invalid /boot"); + cninit(); + /* * Memory on the AMD64 port is described by three different things. * @@ -1927,12 +1924,6 @@ getbootinfo(char *bootinfo, int bootinfo bios_ddb_t *bios_ddb; bios_bootduid_t *bios_bootduid; 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) && @@ -1942,24 +1933,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 */ @@ -1983,15 +1965,8 @@ getbootinfo(char *bootinfo, int bootinfo comconsaddr = consaddr; comconsrate = cdp->conspeed; comconsiot = X86_BUS_SPACE_IO; - - /* Probe the serial port this time. */ - docninit++; } #endif -#ifdef BOOTINFO_DEBUG - printf(" console 0x%x:%d", - cdp->consdev, cdp->conspeed); -#endif } break; case BOOTARG_BOOTMAC: @@ -2023,8 +1998,6 @@ getbootinfo(char *bootinfo, int bootinfo case BOOTARG_EFIINFO: bios_efiinfo = (bios_efiinfo_t *)q->ba_arg; - if (bios_efiinfo->fb_addr != 0) - docninit++; break; case BOOTARG_UCODE: @@ -2032,18 +2005,9 @@ 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
Re: diff: init efifb even if VGA is probed.
On Mon, Mar 9, 2020 at 9:24 AM YASUOKA Masahiko wrote: > > Hi, > > Thank you for your test and feedback. > > I see, first diff didn't fix the problem on your machine. > > > I haven't tried any of the later diffs as I'm not sure which are still > > recommended. > > The last diff should fix the problem since it will initialize efifb > before initializing VGA without condition. > > https://marc.info/?l=openbsd-tech=158280719421562=2 Yes, that one does indeed fix the problem! (That diff alone -- I reverted the first one.) I also tested that kernel on another machine in BIOS mode; doesn't seem to cause any problems there. Thanks a bunch! On a related note, since you are knowledgeable about EFI console stuff, I don't suppose you happen to know the magic necessary for efi_cons_getshifts() [1]? That is needed to allow "hold Ctrl to skip processing /etc/boot.conf" to work with efiboot. biosboot uses an interrupt 0x16 call [2] which I guess isn't available under EFI? [1] https://github.com/openbsd/src/blob/43e343f8aa17502e68dbb74fa3dd463280c74fe5/sys/arch/amd64/stand/efi64/efiboot.c#L514-L519 [2] https://github.com/openbsd/src/blob/0ac7e6ecda86f609ac723da7cedfc4880e082fdd/sys/arch/amd64/stand/libsa/bioscons.c#L100-L109 Thanks, Andrew
Re: diff: init efifb even if VGA is probed.
Hi, Thank you for your test and feedback. On Fri, 6 Mar 2020 16:38:24 -0600 Andrew Daugherity wrote: > On Sun, Mar 1, 2020 at 10:41 PM YASUOKA Masahiko wrote: >> >> Hi, >> >> The problems you are pointing seem to be the same problem. >> >> > I'll try to test this diff next week if I can schedule some downtime. >> >> Test is appreciated. >> >> Also I'd like to know the result of >> >> hexdump -C /var/db/acpi/FACP.1 >> >> when "Load Legacy Video Option ROM" setting is disabled. > > I just tested a -current kernel built yesterday with that diff (your > post on Feb. 20), but unfortunately it does not fix the issue on my > hardware. As before, if "Load Legacy Video Option ROM" is disabled, > output is squished to a purple line and when devices are initialized, > vga1 is the wsdisplay0 device: I see, first diff didn't fix the problem on your machine. > vga1 at pci7 dev 0 function 0 "Matrox MGA G200eR" rev 0x01 > wsdisplay0 at vga1 mux 1: console (80x25, vt100 emulation) > wsdisplay0: screen 1-5 added (80x25, vt100 emulation) > efifb0 at mainbus0: 1280x1024, 32bpp > wsdisplay at efifb0 not configured > > vs. with the legacy video ROM setting: > > "Matrox MGA G200eR" rev 0x01 at pci7 dev 0 function 0 not configured > efifb0 at mainbus0: 1024x768, 32bpp > wsdisplay0 at efifb0 mux 1 > wsdisplay0: screen 0-5 added (std, vt100 emulation) > > I'm using a serial console, if it matters. Hmm... I just noticed that > with the legacy ROM setting disabled, both wsdisplay0 at vga1 mux > 1/wskbd0 at ukbd0 *and* com1 claim to be the console. With the > setting enabled (and efifb working), only com1 is listed as console. > > I haven't tried any of the later diffs as I'm not sure which are still > recommended. The last diff should fix the problem since it will initialize efifb before initializing VGA without condition. https://marc.info/?l=openbsd-tech=158280719421562=2 > The FACP.1 table does not change when the "Load Legacy Video Option > ROM" setting is changed. Here is its hexdump: > andrew@gsc-lb1:~/acpidump$ hexdump -C legacy-2.8.1/FACP.1 > 46 41 43 50 0c 01 00 00 05 62 44 45 4c 4c 20 20 |FACP.bDELL | > 0010 50 45 5f 53 43 33 20 20 00 00 00 00 44 45 4c 4c |PE_SC3 DELL| > 0020 01 00 00 00 00 30 f8 8e 00 b0 fc 8e 00 04 09 00 |.0..| > 0030 b2 00 00 00 f0 f1 f2 00 00 18 00 00 00 00 00 00 || > 0040 04 18 00 00 00 00 00 00 50 18 00 00 08 18 00 00 |P...| > 0050 80 18 00 00 00 00 00 00 04 02 01 04 20 00 10 00 | ...| > 0060 65 00 e9 03 00 00 00 00 01 03 0d 00 32 11 00 00 |e...2...| > 0070 a5 86 00 00 01 08 00 01 f9 0c 00 00 00 00 00 00 || > 0080 06 00 00 00 00 00 00 00 00 00 00 00 00 b0 fc 8e || > 0090 00 00 00 00 01 20 00 02 00 18 00 00 00 00 00 00 |. ..| > 00a0 01 00 00 02 00 00 00 00 00 00 00 00 01 10 00 02 || > 00b0 04 18 00 00 00 00 00 00 01 00 00 02 00 00 00 00 || > 00c0 00 00 00 00 01 08 00 01 50 18 00 00 00 00 00 00 |P...| > 00d0 01 20 00 03 08 18 00 00 00 00 00 00 01 00 00 01 |. ..| > 00e0 80 18 00 00 00 00 00 00 01 00 00 01 00 00 00 00 || > 00f0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 || > 0100 00 00 00 00 00 00 00 00 00 00 00 00 || > 010c This was to check whether using "VGA Not Present" bit is useful on your machine. "Boot IA-PC Boot Architecture Flags" is 0x6D:6E = 0x0011, LEGACY_DEVICES bit is set, "VGA Not Present" is cleared. This means the bit isn't set as I expected, it isn't useful to know existance of VGA. > The only ACPI change made by toggling that option is the DMAR.25 > table. Here are both versions: > Legacy Video ROM enabled: > 44 4d 41 52 90 00 00 00 01 83 49 4e 54 45 4c 20 |DMAR..INTEL | > 0010 47 4e 4c 52 00 00 00 00 01 00 00 00 49 4e 54 4c |GNLRINTL| > 0020 01 00 00 00 26 01 00 00 00 00 00 00 00 00 00 00 |&...| > 0030 00 00 20 00 01 00 00 00 00 00 d9 fe 00 00 00 00 |.. .| > 0040 03 08 00 00 02 f0 1f 00 04 08 00 00 00 00 1f 00 || > 0050 01 00 20 00 00 00 00 00 00 b0 ba 7c 00 00 00 00 |.. || > 0060 ff 2f bb 84 00 00 00 00 01 08 00 00 00 01 00 00 |./..| > 0070 01 00 20 00 00 00 00 00 00 10 31 8e 00 00 00 00 |.. ...1.| > 0080 ff 0f 33 8e 00 00 00 00 01 08 00 00 00 00 14 00 |..3.| > 0090 > and disabled: > 44 4d 41 52 90 00 00 00 01 05 49 4e 54 45 4c 20 |DMAR..INTEL | > 0010 47 4e 4c 52 00 00 00 00 01 00 00 00 49 4e 54 4c |GNLRINTL| > 0020 01 00 00 00 26 01 00 00 00 00 00 00 00 00 00 00 |&...| > 0030 00 00 20 00 01 00 00 00 00 00 d9 fe 00 00 00 00 |.. .| > 0040 03 08 00 00 02 f0 1f 00 04 08
Re: diff: init efifb even if VGA is probed.
On Sun, Mar 1, 2020 at 10:41 PM YASUOKA Masahiko wrote: > > Hi, > > The problems you are pointing seem to be the same problem. > > > I'll try to test this diff next week if I can schedule some downtime. > > Test is appreciated. > > Also I'd like to know the result of > > hexdump -C /var/db/acpi/FACP.1 > > when "Load Legacy Video Option ROM" setting is disabled. I just tested a -current kernel built yesterday with that diff (your post on Feb. 20), but unfortunately it does not fix the issue on my hardware. As before, if "Load Legacy Video Option ROM" is disabled, output is squished to a purple line and when devices are initialized, vga1 is the wsdisplay0 device: vga1 at pci7 dev 0 function 0 "Matrox MGA G200eR" rev 0x01 wsdisplay0 at vga1 mux 1: console (80x25, vt100 emulation) wsdisplay0: screen 1-5 added (80x25, vt100 emulation) efifb0 at mainbus0: 1280x1024, 32bpp wsdisplay at efifb0 not configured vs. with the legacy video ROM setting: "Matrox MGA G200eR" rev 0x01 at pci7 dev 0 function 0 not configured efifb0 at mainbus0: 1024x768, 32bpp wsdisplay0 at efifb0 mux 1 wsdisplay0: screen 0-5 added (std, vt100 emulation) I'm using a serial console, if it matters. Hmm... I just noticed that with the legacy ROM setting disabled, both wsdisplay0 at vga1 mux 1/wskbd0 at ukbd0 *and* com1 claim to be the console. With the setting enabled (and efifb working), only com1 is listed as console. I haven't tried any of the later diffs as I'm not sure which are still recommended. The FACP.1 table does not change when the "Load Legacy Video Option ROM" setting is changed. Here is its hexdump: andrew@gsc-lb1:~/acpidump$ hexdump -C legacy-2.8.1/FACP.1 46 41 43 50 0c 01 00 00 05 62 44 45 4c 4c 20 20 |FACP.bDELL | 0010 50 45 5f 53 43 33 20 20 00 00 00 00 44 45 4c 4c |PE_SC3 DELL| 0020 01 00 00 00 00 30 f8 8e 00 b0 fc 8e 00 04 09 00 |.0..| 0030 b2 00 00 00 f0 f1 f2 00 00 18 00 00 00 00 00 00 || 0040 04 18 00 00 00 00 00 00 50 18 00 00 08 18 00 00 |P...| 0050 80 18 00 00 00 00 00 00 04 02 01 04 20 00 10 00 | ...| 0060 65 00 e9 03 00 00 00 00 01 03 0d 00 32 11 00 00 |e...2...| 0070 a5 86 00 00 01 08 00 01 f9 0c 00 00 00 00 00 00 || 0080 06 00 00 00 00 00 00 00 00 00 00 00 00 b0 fc 8e || 0090 00 00 00 00 01 20 00 02 00 18 00 00 00 00 00 00 |. ..| 00a0 01 00 00 02 00 00 00 00 00 00 00 00 01 10 00 02 || 00b0 04 18 00 00 00 00 00 00 01 00 00 02 00 00 00 00 || 00c0 00 00 00 00 01 08 00 01 50 18 00 00 00 00 00 00 |P...| 00d0 01 20 00 03 08 18 00 00 00 00 00 00 01 00 00 01 |. ..| 00e0 80 18 00 00 00 00 00 00 01 00 00 01 00 00 00 00 || 00f0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 || 0100 00 00 00 00 00 00 00 00 00 00 00 00 || 010c The only ACPI change made by toggling that option is the DMAR.25 table. Here are both versions: Legacy Video ROM enabled: 44 4d 41 52 90 00 00 00 01 83 49 4e 54 45 4c 20 |DMAR..INTEL | 0010 47 4e 4c 52 00 00 00 00 01 00 00 00 49 4e 54 4c |GNLRINTL| 0020 01 00 00 00 26 01 00 00 00 00 00 00 00 00 00 00 |&...| 0030 00 00 20 00 01 00 00 00 00 00 d9 fe 00 00 00 00 |.. .| 0040 03 08 00 00 02 f0 1f 00 04 08 00 00 00 00 1f 00 || 0050 01 00 20 00 00 00 00 00 00 b0 ba 7c 00 00 00 00 |.. || 0060 ff 2f bb 84 00 00 00 00 01 08 00 00 00 01 00 00 |./..| 0070 01 00 20 00 00 00 00 00 00 10 31 8e 00 00 00 00 |.. ...1.| 0080 ff 0f 33 8e 00 00 00 00 01 08 00 00 00 00 14 00 |..3.| 0090 and disabled: 44 4d 41 52 90 00 00 00 01 05 49 4e 54 45 4c 20 |DMAR..INTEL | 0010 47 4e 4c 52 00 00 00 00 01 00 00 00 49 4e 54 4c |GNLRINTL| 0020 01 00 00 00 26 01 00 00 00 00 00 00 00 00 00 00 |&...| 0030 00 00 20 00 01 00 00 00 00 00 d9 fe 00 00 00 00 |.. .| 0040 03 08 00 00 02 f0 1f 00 04 08 00 00 00 00 1f 00 || 0050 01 00 20 00 00 00 00 00 00 c0 e9 7c 00 00 00 00 |.. || 0060 ff 3f ea 84 00 00 00 00 01 08 00 00 00 01 00 00 |.?..| 0070 01 00 20 00 00 00 00 00 00 10 31 8e 00 00 00 00 |.. ...1.| 0080 ff 0f 33 8e 00 00 00 00 01 08 00 00 00 00 14 00 |..3.| 0090 I've attached full dmesgs for both legacy video ROM enabled and disabled. The system is currently on 6.5, but the efifb/wsdisplay/etc. messages are the same in -current. Thanks for trying to figure this out! -Andrew OpenBSD 6.5 (GENERIC.MP) #8: Tue Jan 14 23:16:33 MST 2020 t...@syspatch-65-amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP real mem = 8394772480 (8005MB) avail
Re: diff: init efifb even if VGA is probed.
Hi, On Fri, 21 Feb 2020 18:55:38 -0600 Andrew Daugherity wrote: > On Thu, Feb 20, 2020 at 11:10 PM YASUOKA Masahiko wrote: >> Hello, >> >> I am testing a new hardware, HPE DL20 Gen10. >> >> When efiboot starts the kernel, the video display becomes distorted >> and never recovered until CPU reset. >> [...] >> 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. > > This is exciting! Your HP server sounds very much like what I've > experienced on the Dell PowerEdge R230 [1] (probably also affects > other Dell Rx30 in UEFI mode, maybe Rx40 too?), and I would not be > surprised if your diff fixes mine too. > > >> # 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. > > OpenBSD squishes the video to a thin purple line unless I enable the > "Load Legacy Video Option ROM" setting in the Dell BIOS. However, > that setting is described as a compatibility shim for old OSes which > don't support EFI GOP natively, and enabling it restricts the efifb > resolution to 1024x768. Every other OS I tested (including FreeBSD & > Linux) worked properly without the legacy video ROM. > > I got as far a nasty hack of a diff on efifb where if probing failed, > attach it with hardcoded values matching my machine. With said diff > plus vga disabled via 'boot -c' (otherwise vga would steal the > console), kernel output was still scrambled, but userland output from > the boot process displayed correctly. Unfortunately the keyboard > wasn't attached to this console, but I could at least print stuff on > the screen from an ssh session, e.g. 'echo "Hello, world!" > > /dev/console'. At some point I had to actually use this server > though, so I abandoned that effort and put the server into production > (using the legacy video ROM). The problems you are pointing seem to be the same problem. > I'll try to test this diff next week if I can schedule some downtime. Test is appreciated. Also I'd like to know the result of hexdump -C /var/db/acpi/FACP.1 when "Load Legacy Video Option ROM" setting is disabled. Thanks, > -Andrew > > [1] https://marc.info/?l=openbsd-tech=150707255507032=2 > The first half isn't relevant, but the last part covers the R230, and > has (old) dmesg for with and without the legacy video ROM. > I should clarify "X still works in either case": with efifb active, X > will prefer wsfb unless you add an xorg.conf for mga. mga performs > better and can use any resolution, while wsfb is limited to the efifb > resolution.
Re: diff: init efifb even if VGA is probed.
On Sun, 23 Feb 2020 21:23:41 +1100 Jonathan Gray wrote: > On Sun, Feb 23, 2020 at 07:06:50PM +0900, YASUOKA Masahiko wrote: >> On Sun, 23 Feb 2020 18:50:54 +0900 (JST) >> YASUOKA Masahiko wrote: >> > On Sat, 22 Feb 2020 13:02:48 +1100 >> > Jonathan Gray 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 - 1.261 >> +++ sys/arch/amd64/amd64/machdep.c 23 Feb 2020 09:46:54 - >> @@ -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(); > > why move uvm_setpagesize? Because I thought moving uvm_setpagesize helps a panic() in the function. But it doesn't help so much since there is still many other panic()s. The updated diff doesn't move the function. >> - >> -/* >> * 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(); > > as cninit() is done here docninit and the cninit() call in getbootinfo() > could be removed. Yes, that's right. Updated 3rd diff is attached. I think this is the best way for this moment. Since it's simple and it seems that we don't have any other way to skip VGA than the way that skips VGA if the machine has efifb. Index: sys/arch/amd64/amd64/machdep.c === RCS file: /var/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 - 1.261 +++ sys/arch/amd64/amd64/machdep.c 27 Feb 2020 12:16:32 - @@ -1394,11 +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(); @@ -1420,6
Re: diff: init efifb even if VGA is probed.
On Sun, 23 Feb 2020 12:47:51 +0100 (CET) Mark Kettenis wrote: >> Date: Sun, 23 Feb 2020 18:50:54 +0900 (JST) >> From: YASUOKA Masahiko >> >> On Sat, 22 Feb 2020 13:02:48 +1100 >> Jonathan Gray 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 46 41 43 50 0c 01 00 00 06 ef 48 50 45 20 20 20 |FACP..HPE | 0010 53 65 72 76 65 72 20 20 01 00 00 00 31 35 39 30 |Server 1590| 0020 01 00 00 00 00 00 dd 7b 00 40 fe 7b 00 04 09 00 |...{.@.{| 0030 b2 00 00 00 a0 a1 f2 00 00 05 00 00 00 00 00 00 || 0040 04 05 00 00 00 00 00 00 50 05 00 00 08 05 00 00 |P...| 0050 60 05 00 00 00 00 00 00 04 02 01 04 20 00 10 00 |`... ...| 0060 65 00 e9 03 00 00 00 00 01 03 0d 00 32 33 00 00 |e...23..| 0070 a5 84 00 00 01 08 00 01 f9 0c 00 00 00 00 00 00 || 0080 06 00 00 00 00 00 00 00 00 00 00 00 00 40 fe 7b |.@.{| 0090 00 00 00 00 01 20 00 02 00 05 00 00 00 00 00 00 |. ..| 00a0 01 00 00 00 00 00 00 00 00 00 00 00 01 10 00 02 || 00b0 04 05 00 00 00 00 00 00 01 00 00 00 00 00 00 00 || 00c0 00 00 00 00 01 08 00 00 50 05 00 00 00 00 00 00 |P...| 00d0 01 20 00 03 08 05 00 00 00 00 00 00 01 ff 00 01 |. ..| 00e0 60 05 00 00 00 00 00 00 01 00 00 00 00 00 00 00 |`...| 00f0 00 00 00 00 01 08 00 00 00 00 00 00 00 00 00 00 || 0100 01 08 00 00 00 00 00 00 00 00 00 00 || 010c $ "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.h8 Apr 2018 13:24:36 - 1.15 +++ sys/stand/boot/bootarg.h27 Feb 2020 11:48:08 - @@ -32,6 +32,7 @@ #defineBAPIV_VECTOR0x0002 /* MI vector of MD structures passed */ #defineBAPIV_ENV 0x0004 /* MI environment vars vector */ #defineBAPIV_BMEMMAP 0x0008 /* MI memory map passed is in bytes */ +#defineBAPIV_NOVGA 0x0100 /* MI VGA is not
Re: diff: init efifb even if VGA is probed.
> Date: Sun, 23 Feb 2020 18:50:54 +0900 (JST) > From: YASUOKA Masahiko > > On Sat, 22 Feb 2020 13:02:48 +1100 > Jonathan Gray 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. > 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(). > > 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/arch/amd64/stand/efiboot/exec_i386.c > === > RCS file: /disk/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 - > 1.3 > +++ sys/arch/amd64/stand/efiboot/exec_i386.c 23 Feb 2020 09:49:48 - > @@ -163,11 +163,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, BOOTARG_APIVER | > BAPIV_EFI, > 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, BOOTARG_APIVER | BAPIV_EFI, > marks[MARK_END], > extmem, cnvmem, ac, (int)av); > #endif > /* not reached */ > 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.c24 Jan 2020 05:27:31 - 1.261 > +++ sys/arch/amd64/amd64/machdep.c23 Feb 2020 09:50:02 - > @@ -1396,7 +1396,8 @@ init_x86_64(paddr_t first_avail) > /* >* Attach the glass console early in case we need to display a panic. >*/ > - cninit(); > + if (!ISSET(bootapiver, BAPIV_EFI)) > + cninit(); > > /* >* Initialize PAGE_SIZE-dependent variables. > @@ -1420,6 +1421,9 @@ init_x86_64(paddr_t first_avail) > } else > panic("invalid /boot"); > > + /* EFI: bootinfo is required to initialize efifb */ > + if (ISSET(bootapiver, BAPIV_EFI)) > + cninit(); > /* > * Memory on the AMD64 port is described by three different things. > * > Index: sys/stand/boot/bootarg.h > === > RCS file: /disk/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 - 1.15 > +++ sys/stand/boot/bootarg.h 23 Feb 2020 09:50:02 - > @@ -32,6 +32,7 @@ > #define BAPIV_VECTOR0x0002 /* MI vector of MD structures > passed */ > #define BAPIV_ENV 0x0004 /* MI environment vars vector */ > #define BAPIV_BMEMMAP 0x0008 /* MI memory map passed is in >
Re: diff: init efifb even if VGA is probed.
On Sun, Feb 23, 2020 at 07:06:50PM +0900, YASUOKA Masahiko wrote: > On Sun, 23 Feb 2020 18:50:54 +0900 (JST) > YASUOKA Masahiko wrote: > > On Sat, 22 Feb 2020 13:02:48 +1100 > > Jonathan Gray 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.c24 Jan 2020 05:27:31 - 1.261 > +++ sys/arch/amd64/amd64/machdep.c23 Feb 2020 09:46:54 - > @@ -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(); why move 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(); as cninit() is done here docninit and the cninit() call in getbootinfo() could be removed. > + > + /* > + * 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;
Re: diff: init efifb even if VGA is probed.
On Sun, 23 Feb 2020 18:50:54 +0900 (JST) YASUOKA Masahiko wrote: > On Sat, 22 Feb 2020 13:02:48 +1100 > Jonathan Gray 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 - 1.261 +++ sys/arch/amd64/amd64/machdep.c 23 Feb 2020 09:46:54 - @@ -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
Re: diff: init efifb even if VGA is probed.
On Sat, 22 Feb 2020 13:02:48 +1100 Jonathan Gray 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. * * * Index: sys/arch/amd64/stand/efiboot/exec_i386.c === RCS file: /disk/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.c12 Dec 2019 13:09:35 - 1.3 +++ sys/arch/amd64/stand/efiboot/exec_i386.c23 Feb 2020 09:49:48 - @@ -163,11 +163,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, BOOTARG_APIVER | BAPIV_EFI, 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, BOOTARG_APIVER | BAPIV_EFI, marks[MARK_END], extmem, cnvmem, ac, (int)av); #endif /* not reached */ 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 - 1.261 +++ sys/arch/amd64/amd64/machdep.c 23 Feb 2020 09:50:02 - @@ -1396,7 +1396,8 @@ init_x86_64(paddr_t first_avail) /* * Attach the glass console early in case we need to display a panic. */ - cninit(); + if (!ISSET(bootapiver, BAPIV_EFI)) + cninit(); /* * Initialize PAGE_SIZE-dependent variables. @@ -1420,6 +1421,9 @@ init_x86_64(paddr_t first_avail) } else panic("invalid /boot"); + /* EFI: bootinfo is required to initialize efifb */ + if (ISSET(bootapiver, BAPIV_EFI)) + cninit(); /* * Memory on the AMD64 port is described by three different things. * Index: sys/stand/boot/bootarg.h === RCS file: /disk/cvs/openbsd/src/sys/stand/boot/bootarg.h,v retrieving revision 1.15 diff -u -p -r1.15 bootarg.h --- sys/stand/boot/bootarg.h8 Apr 2018 13:24:36 - 1.15 +++ sys/stand/boot/bootarg.h23 Feb 2020 09:50:02 - @@ -32,6 +32,7 @@ #defineBAPIV_VECTOR0x0002 /* MI vector of MD structures passed */ #defineBAPIV_ENV 0x0004 /* MI environment vars vector */ #defineBAPIV_BMEMMAP 0x0008 /* MI memory map passed is in bytes */ +#defineBAPIV_EFI 0x0010 /* MI booted from EFI */ typedef struct _boot_args { int ba_type;
Re: diff: init efifb even if VGA is probed.
On Fri, Feb 21, 2020 at 02:09:07PM +0900, YASUOKA Masahiko wrote: > Hello, > > I am testing a new hardware, HPE DL20 Gen10. > > 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. 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. > > Initialize efifb as a console even if the VGA is probed. > > 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 - > 1.14 > +++ sys/arch/amd64/amd64/wscons_machdep.c 21 Feb 2020 04:42:38 - > @@ -73,7 +73,7 @@ > #include > #endif > > -int wscn_video_init(void); > +int wscn_video_init(int); > void wscn_input_init(int); > > cons_decl(ws); > @@ -103,10 +103,12 @@ wscninit(struct consdev *cp) > { > static int initted = 0; > > - if (initted) > + if (initted) { > + wscn_video_init(1); > return; > + } > > - if (wscn_video_init() == 0) { > + if (wscn_video_init(0) == 0) { > initted = 1; > wscn_input_init(0); > } > @@ -134,11 +136,17 @@ wscnpollc(dev_t dev, int on) > * Configure the display part of the console. > */ > int > -wscn_video_init(void) > +wscn_video_init(int pass) > { > #if (NEFIFB > 0) > - if (efifb_cnattach() == 0) > - return (0); > + extern int vgaconsole; > + if (pass > 0) { > + if (efifb_cnattach() == 0) { > + vgaconsole = 0; > + return (0); > + } > + return (-1); > + } > #endif > #if (NVGA > 0) > if (vga_cnattach(X86_BUS_SPACE_IO, X86_BUS_SPACE_MEM, -1, 1) == 0) > >
Re: diff: init efifb even if VGA is probed.
On Thu, Feb 20, 2020 at 11:10 PM YASUOKA Masahiko wrote: > Hello, > > I am testing a new hardware, HPE DL20 Gen10. > > When efiboot starts the kernel, the video display becomes distorted > and never recovered until CPU reset. > [...] > 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. This is exciting! Your HP server sounds very much like what I've experienced on the Dell PowerEdge R230 [1] (probably also affects other Dell Rx30 in UEFI mode, maybe Rx40 too?), and I would not be surprised if your diff fixes mine too. > # 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. OpenBSD squishes the video to a thin purple line unless I enable the "Load Legacy Video Option ROM" setting in the Dell BIOS. However, that setting is described as a compatibility shim for old OSes which don't support EFI GOP natively, and enabling it restricts the efifb resolution to 1024x768. Every other OS I tested (including FreeBSD & Linux) worked properly without the legacy video ROM. I got as far a nasty hack of a diff on efifb where if probing failed, attach it with hardcoded values matching my machine. With said diff plus vga disabled via 'boot -c' (otherwise vga would steal the console), kernel output was still scrambled, but userland output from the boot process displayed correctly. Unfortunately the keyboard wasn't attached to this console, but I could at least print stuff on the screen from an ssh session, e.g. 'echo "Hello, world!" > /dev/console'. At some point I had to actually use this server though, so I abandoned that effort and put the server into production (using the legacy video ROM). I'll try to test this diff next week if I can schedule some downtime. -Andrew [1] https://marc.info/?l=openbsd-tech=150707255507032=2 The first half isn't relevant, but the last part covers the R230, and has (old) dmesg for with and without the legacy video ROM. I should clarify "X still works in either case": with efifb active, X will prefer wsfb unless you add an xorg.conf for mga. mga performs better and can use any resolution, while wsfb is limited to the efifb resolution.