Re: diff: init efifb even if VGA is probed.

2020-05-28 Thread Jonathan Gray
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.

2020-05-28 Thread Mark Kettenis
> 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.

2020-05-28 Thread 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?

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.

2020-05-28 Thread Mark Kettenis
> 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.

2020-05-28 Thread Jonathan Gray
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.

2020-05-28 Thread YASUOKA Masahiko
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.

2020-05-28 Thread 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?

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.

2020-03-12 Thread Andrew Daugherity
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.

2020-03-09 Thread YASUOKA Masahiko
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.

2020-03-06 Thread Andrew Daugherity
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.

2020-03-01 Thread YASUOKA Masahiko
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.

2020-02-27 Thread YASUOKA Masahiko
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.

2020-02-27 Thread YASUOKA Masahiko
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.

2020-02-23 Thread Mark Kettenis
> 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.

2020-02-23 Thread Jonathan Gray
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.

2020-02-23 Thread YASUOKA Masahiko
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.

2020-02-23 Thread 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

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.

2020-02-21 Thread Jonathan Gray
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.

2020-02-21 Thread Andrew Daugherity
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.