On Thu, 28 May 2020 12:31:31 +0200 (CEST)
Mark Kettenis <mark.kette...@xs4all.nl> wrote:
>> Date: Thu, 28 May 2020 17:01:48 +0900 (JST)
>> From: YASUOKA Masahiko <yasu...@openbsd.org>
>>
>> Hi,
>>
>> I'd like to conclude this issue.
>>
>> On Fri, 21 Feb 2020 14:09:07 +0900 (JST)
>> YASUOKA Masahiko <yasu...@openbsd.org> 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&m=159064773219779&w=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 -0000 1.264
+++ sys/arch/amd64/amd64/machdep.c 28 May 2020 11:34:39 -0000
@@ -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;
- if (bios_efiinfo->fb_addr != 0)
- docninit++;
break;
case BOOTARG_UCODE:
bios_ucode = (bios_ucode_t *)q->ba_arg;
@@ -2038,10 +2028,8 @@ getbootinfo(char *bootinfo, int bootinfo
#endif
break;
}
}
- if (docninit > 0)
- cninit();
#ifdef BOOTINFO_DEBUG
printf("\n");
#endif
}
