Re: fix ifconfig joinlist width bug
On Sat, Feb 22, 2020 at 06:38:36PM +0100, Stefan Sperling wrote:
>
> This fixes display of hex SSIDs in 'ifconfig joinlist' and prevents a
> negative number being passed to printf on the following line when 'maxlen'
> ends up being capped below the maximum value returned from len_string():
>
> printf("%-*s", maxlen - len, " ");
>
> Hex SSIDs can be as wide as IEEE80211_NWID_LEN * 2 + 2 /* "0x" */
>
> ok?
ok kevlo@
>
> diff c20bd74017ceeadb2db0f78a352ed1f1e2b77c2b /usr/src
> blob - e1dc9dbb07bf109c3ec7f5fd4d851a7dbb5692f1
> file + sbin/ifconfig/ifconfig.c
> --- sbin/ifconfig/ifconfig.c
> +++ sbin/ifconfig/ifconfig.c
> @@ -2571,16 +2571,14 @@ join_status(void)
>
> maxlen = 0;
> for (i = 0; i < ja.ja_nodes; i++) {
> len = len_string(jn[i].i_nwid, jn[i].i_len);
> if (len > maxlen)
> maxlen = len;
> }
> - if (maxlen > IEEE80211_NWID_LEN)
> - maxlen = IEEE80211_NWID_LEN - 1;
>
> for (i = 0; i < ja.ja_nodes; i++) {
> printf("\t ");
> if (jn[i].i_len > IEEE80211_NWID_LEN)
> jn[i].i_len = IEEE80211_NWID_LEN;
> len = print_string(jn[i].i_nwid, jn[i].i_len);
> printf("%-*s", maxlen - len, " ");
>
Re: ifconfig man page fix
On Sat, Feb 22, 2020 at 06:40:39PM +0100, Stefan Sperling wrote: > > SSIDs are required to contain printable ASCII only. > Otherwise, they must be specified in hex. > > Let's document this explicitly. ok kevlo@ > diff c20bd74017ceeadb2db0f78a352ed1f1e2b77c2b /usr/src > blob - 3fb0780ba7cf1333894f5c3485a95e71885fbd6d > file + sbin/ifconfig/ifconfig.8 > --- sbin/ifconfig/ifconfig.8 > +++ sbin/ifconfig/ifconfig.8 > @@ -972,8 +972,9 @@ list if they are found during a scan. > .Pp > The > .Ar id > -can either be any text string up to 32 characters in length, > -or a series of hexadecimal digits up to 64 digits. > +can either be a printable ASCII string up to 32 characters in length, > +or a series of hexadecimal digits up to 64 digits preceded by > +.Dq 0x . > If > .Ar id > is the empty string > @@ -1077,6 +1078,12 @@ Remove specified flag. > .It Cm nwid Ar id > Connect to the network with NWID/ESSID > .Ar id . > +The > +.Ar id > +can either be a printable ASCII string up to 32 characters in length, > +or a series of hexadecimal digits up to 64 digits preceded by > +.Dq 0x . > +.Pp > Unlike > .Cm join , > the >
Re: ifconfig with UTF-8 nwid
On Sat, Feb 22, 2020 at 10:21:56PM +, Stuart Henderson wrote:
> On 2020/02/22 17:24, Stefan Sperling wrote:
> > On Sat, Feb 22, 2020 at 02:56:54PM +0100, Mark Kettenis wrote:
> > > IMHO it is a bad idea to make the output of ifconfig locale-dependent.
> >
> > Fine. I'll drop this diff.
> >
>
> Pity, it is quite useful if you are somewhere that uses UTF-8 SSIDs,
> otherwise it's a complete pain to decode to figure out if you're
> connecting to the correct network. An alternative would be to filter
> ifconfig output through something like this
>
> while(<>) {
> if ($_ =~ m/nwid 0x([0-9a-f]+)\s/) {
> my $hex = $1;
> my $decoded = pack "H*", $hex;
> $_ =~ s/$hex/$hex ("$decoded")/;
> }
> print $_;
> }
>
> but it won't validate the characters so best redirect output to a file
> and open it in a UTF-8 editor to view it rather than displaying it
> directly on a terminal.
This removes any characters that are not marked as "printable", which I
expect is what folks want. Still doesn't always help since I still
haven't figured out font fallback in xterm so some chars don't exist in
my current font.
ifconfig wlan scan | perl -C -MEncode=decode -pE 's{\bnwid 0x(\p{AHex}+)\K\b}{
my $n = decode("UTF-8", pack "H*", $1) =~ s/\P{Print}//gr; $n ? qq{ ("$n")} :
"" }e'
Or, the more verbose:
$ cat ~/bin/wlanscan
#!/usr/bin/perl
# perldoc perlunicook
use utf8; # so literals and identifiers can be in UTF-8
use v5.12; # or later to get "unicode_strings" feature
use strict;# quote strings, declare variables
use warnings; # on by default
use warnings qw(FATAL utf8);# fatalize encoding glitches
use open qw(:std :encoding(UTF-8)); # undeclared streams in UTF-8
use Encode qw< decode >;
my $if = shift || 'wlan';
open my $fh, '-|', 'ifconfig', $if, 'scan' or die "Unable to exec ifconfig: $!";
while ( my $line = readline $fh ) {
$line =~ s{\b nwid \s+ 0x(\p{PosixXDigit}+)\K \b}{ decode_nwid($1) }ex;
print $line;
}
close $fh or die "Error closing ifconfig: $!";
sub decode_nwid {
my $nwid = decode 'UTF-8', pack "H*", $_[0];
$nwid =~ s/\P{Print}//g;
return $nwid ? qq{ ("$nwid")} : '';
}
Re: ifconfig with UTF-8 nwid
On 2020/02/23 05:01, Theo de Raadt wrote: > Stefan Sperling wrote: > > > I don't understand why the installer would matter. Nothing would actually > > change for the installer. We're sure as hell not going to add UTF-8 there. > > The installer's ifconfig would just deal in hex all the way as it does > > today. > > During install finding the right network could be tricky, but that's not a > > new problem. Upgrades would just work once hostname.if contains the right > > hex. > > In upgrade mode, the installer must be able to do the same work as > /etc/netstart -- the installer has a re-implimentation to parse and act > upon hostname.* files. If that behaves different because ifconfig doesn't > act the same way, upgrade won't work right in some circumstances. There's > a small chance your change in behaviour for ifconfig doesn't matter but > I am quite sceptical. > There's no problem for the installer, Stefan's diff doesn't change the method to set nwid in any case, that always uses 0xXXX, so there is no change to how hostname.if files are handled. The output format remains unchanged for SMALL too, so no need to touch ieee80211_scan() in install.sub. Some scripts do parse ifconfig output on a normal running system (net/wifind in ports, and some users will have local scripts), these need changing for correctness but will only actually break if users use them to connect to a network with a UTF-8 SSID, otherwise they will work as-is. Or most of these can be replaced with "join" instead. The main question is whether ifconfig should have locale support.
tcpdump: decode some Setup fields for Control transfers
Hi,
Currently the usbpcap is fairly terse in that it prints the endpoints,
type of transfer and data size. Below is a diff to start decoding more
data from the packets, currently focusing on control transfers:
- print the control stage
- for the Setup control transfer stage, print additional information
about the request. This diff contains the bits to decode the wValue
for GET_DESCRIPTOR requests, others will follow shortly after making sure
my approach is right.
with this diff the output (for a selection of packets) goes from this:
22:28:09.517113 bus 0 < addr 1: ep1 intr dlen=2
22:28:09.517145 bus 0 > addr 1: ep0 ctrl dlen=8
22:28:09.517146 bus 0 > addr 1: ep0 ctrl dlen=0
22:28:09.517148 bus 0 < addr 1: ep0 ctrl dlen=4
[..]
22:28:10.138325 bus 0 > addr 0: ep0 ctrl dlen=8
to this:
22:28:09.517113 bus 0 < addr 1: ep1 intr dlen=2
22:28:09.517145 bus 0 > addr 1: ep0 ctrl dlen=8 stage=setup
bmRequestType=UT_READ_CLASS_OTHER bRequest=GET_STATUS wValue=0x wIndex=0003
wLength=4
22:28:09.517146 bus 0 > addr 1: ep0 ctrl dlen=0 stage=status
22:28:09.517148 bus 0 < addr 1: ep0 ctrl dlen=4 stage=data
[..]
22:28:10.138325 bus 0 > addr 0: ep0 ctrl dlen=8 stage=setup
bmRequestType=UT_READ_DEVICE bRequest=GET_DESCRIPTOR type=DEVICE index=0x00
wIndex= wLength=8
OK?
Index: print-usbpcap.c
===
RCS file: /cvs/src/usr.sbin/tcpdump/print-usbpcap.c,v
retrieving revision 1.3
diff -u -p -r1.3 print-usbpcap.c
--- print-usbpcap.c 22 Feb 2020 14:05:08 - 1.3
+++ print-usbpcap.c 23 Feb 2020 13:06:55 -
@@ -29,6 +29,41 @@
const char *usbpcap_xfer_type[] = {"isoc", "intr", "ctrl", "bulk"};
+struct usbpcap_control_stage_fields {
+ uint8_t stage;
+ char*name;
+};
+
+static const struct usbpcap_control_stage_fields usb_control_stages[] = {
+ { USBPCAP_CONTROL_STAGE_SETUP, "setup" },
+ { USBPCAP_CONTROL_STAGE_DATA, "data" },
+ { USBPCAP_CONTROL_STAGE_STATUS, "status" },
+};
+
+struct usbpcap_request_code_fields {
+ uByterequest;
+ char*name;
+};
+
+static const struct usbpcap_request_code_fields usb_request_codes[] = {
+ { UR_GET_STATUS,"GET_STATUS" },
+ { UR_CLEAR_FEATURE, "CLEAR_FEATURE" },
+ { 0,NULL },
+ { UR_SET_FEATURE, "SET_FEATURE" },
+ { 0,NULL },
+ { UR_SET_ADDRESS, "SET_ADDRESS" },
+ { UR_GET_DESCRIPTOR,"GET_DESCRIPTOR" },
+ { UR_SET_DESCRIPTOR,"SET_DESCRIPTOR" },
+ { UR_GET_CONFIG,"GET_CONFIG" },
+ { UR_SET_CONFIG,"SET_CONFIG" },
+ { UR_GET_INTERFACE, "GET_INTERFACE" },
+ { UR_SET_INTERFACE, "SET_INTERFACE" },
+ { UR_SYNCH_FRAME, "SYNCH_FRAME" },
+};
+
+voidusbpcap_print_descriptor(int);
+voidusbpcap_print_request_type(uint8_t);
+
void
usbpcap_if_print(u_char *user, const struct pcap_pkthdr *h, const u_char *p)
{
@@ -65,6 +100,40 @@ usbpcap_if_print(u_char *user, const str
printf(" dlen=%u", letoh32(uph->uph_dlen));
+ if (uph->uph_xfertype == USBPCAP_TRANSFER_CONTROL) {
+ struct usbpcap_ctl_hdr *ctl_hdr = (struct usbpcap_ctl_hdr *)p;
+
+ if (ctl_hdr->uch_stage < nitems(usb_control_stages))
+ printf(" stage=%s",
usb_control_stages[ctl_hdr->uch_stage].name);
+ else
+ printf(" stage=?");
+
+ if (ctl_hdr->uch_stage == USBPCAP_CONTROL_STAGE_SETUP) {
+ /* Setup packets must be 8 bytes in size as per
+* 9.3 USB Device Requests. */
+ if (letoh32(uph->uph_dlen != 8))
+ goto trunc;
+
+ usb_device_request_t *req =
+ (usb_device_request_t *)(p+sizeof(struct
usbpcap_ctl_hdr));
+
+ usbpcap_print_request_type(req->bmRequestType);
+
+ if (req->bRequest < nitems(usb_request_codes))
+ printf(" bRequest=%s",
usb_request_codes[req->bRequest].name);
+ else
+ printf(" bRequest=?");
+
+ if (req->bRequest == UR_GET_DESCRIPTOR)
+ usbpcap_print_descriptor(UGETW(req->wValue));
+ else
+ printf(" wValue=0x%04x", UGETW(req->wValue));
+
+ printf(" wIndex=%04x", UGETW(req->wIndex));
+ printf(" wLength=%u", UGETW(req->wLength));
+ }
+ }
+
if (xflag)
default_print(p + sizeof(*uph), length - sizeof(*uph));
out:
@@ -72,4 +141,154 @@ out:
return;
trunc:
printf("[|usb]");
+}
+
+void
+usbpcap_print_descriptor(int value) {
+ printf(" type=");
+ switch (value >> 8) {
+ case UDESC_DEVICE:
+
Re: iked.conf.5: Provide GRE tunnel in transport mode example
On Sat, Feb 22, 2020 at 10:06:49PM +0100, Tobias Heider wrote: > Try this This makes iked use the reverse record of the FQDN's IP (which IP?). I have peers with both IPv4 and IPv6 addreses, one of those peers has an incorrect reverse record, thus iked will not end up using the FQDN I used as `peer ...' but rather the FQDN it gets after name resolution back and forth. For peers with proper DNS in both ways this diff yields the correct `dstid' without explicitly specifying it, but that is not the right approach; iked should simply copy over the value from `peer' to `dstid' as is.
Re: librthread sem_t opaqueness, storage & unnamed semaphore sharing
On Sun, Feb 23 2020 14:48:36 +0200, Lauri Tirkkonen wrote:
> So, diff below makes struct __sem non-opaque and removes the indirect
> allocations, so that the application is required to provide storage and
> can therefore control where it's stored (which could be eg. shm).
followup diff that makes sem_init() ignore pshared arg -- it doesn't
need to care, if the application puts the semaphore in shm. I tested an
unnamed shm_init() semaphore in shm_mkstemp() -created shm with this,
passing the fd to a child process and mmap()ing in the child process;
sharing a semaphore this way does seem to work.
diff --git a/lib/librthread/rthread_sem.c b/lib/librthread/rthread_sem.c
index 984e5fb0047..441843d87ca 100644
--- a/lib/librthread/rthread_sem.c
+++ b/lib/librthread/rthread_sem.c
@@ -105,41 +105,6 @@ sem_init(sem_t *sem, int pshared, unsigned int value)
return (-1);
}
- if (pshared) {
- errno = EPERM;
- return (-1);
-#ifdef notyet
- char name[SEM_RANDOM_NAME_LEN];
- sem_t *sempshared;
- int i;
-
- for (;;) {
- for (i = 0; i < SEM_RANDOM_NAME_LEN - 1; i++)
- name[i] = arc4random_uniform(255) + 1;
- name[SEM_RANDOM_NAME_LEN - 1] = '\0';
- sempshared = sem_open(name, O_CREAT | O_EXCL, 0, value);
- if (sempshared != SEM_FAILED)
- break;
- if (errno == EEXIST)
- continue;
- if (errno != EPERM)
- errno = ENOSPC;
- return (-1);
- }
-
- /* unnamed semaphore should not be opened twice */
- if (sem_unlink(name) == -1) {
- sem_close(sempshared);
- errno = ENOSPC;
- return (-1);
- }
-
- *semp = *sempshared;
- free(sempshared);
- return (0);
-#endif
- }
-
bzero(sem, sizeof(*sem));
sem->value = value;
diff --git a/lib/librthread/rthread_sem_compat.c
b/lib/librthread/rthread_sem_compat.c
index 91c888cd49b..8dc863a04d3 100644
--- a/lib/librthread/rthread_sem_compat.c
+++ b/lib/librthread/rthread_sem_compat.c
@@ -116,41 +116,6 @@ sem_init(sem_t *sem, int pshared, unsigned int value)
return (-1);
}
- if (pshared) {
- errno = EPERM;
- return (-1);
-#ifdef notyet
- char name[SEM_RANDOM_NAME_LEN];
- sem_t *sempshared;
- int i;
-
- for (;;) {
- for (i = 0; i < SEM_RANDOM_NAME_LEN - 1; i++)
- name[i] = arc4random_uniform(255) + 1;
- name[SEM_RANDOM_NAME_LEN - 1] = '\0';
- sempshared = sem_open(name, O_CREAT | O_EXCL, 0, value);
- if (sempshared != SEM_FAILED)
- break;
- if (errno == EEXIST)
- continue;
- if (errno != EPERM)
- errno = ENOSPC;
- return (-1);
- }
-
- /* unnamed semaphore should not be opened twice */
- if (sem_unlink(name) == -1) {
- sem_close(sempshared);
- errno = ENOSPC;
- return (-1);
- }
-
- *semp = *sempshared;
- free(sempshared);
- return (0);
-#endif
- }
-
bzero(sem, sizeof(*sem));
sem->lock = _SPINLOCK_UNLOCKED;
sem->value = value;
--
Lauri Tirkkonen | lotheac @ IRCnet
librthread sem_t opaqueness, storage & unnamed semaphore sharing
I was working on a make jobserver implementation that uses POSIX
semaphores as job tokens instead of a complicated socket-based approach.
Initially I used named semaphores, which work fine, except if child
processes with less privileges need to also open the named semaphore
(eg. 'make build' as root executing 'su build -c make'). For that reason
I wanted to use an unnamed semaphore (sem_init()) which is stored in shm
-- that way I could leave the shm fd open and pass it to children.
But unfortunately, sem_t is currently just a pointer to the opaque
struct __sem, and sem_int() always calloc()s the storage for the struct.
This means the application cannot control where unnamed semaphores are
stored, so I can't put it in shm.
I might be missing something here, but I don't see why sem_t is not just
typedef'd to a non-opaque struct. The equivalent structs have
definitions in semaphore.h on FreeBSD and glibc (though glibc doesn't
expose the members in this definition, it seems to be provided just for
sizing). NetBSD uses an opaque struct which is allocated, *except* in
the sem_init() pshared==1 case, where it instead returns a kernel
semaphore identifier cast into a sem_t*; I find that to be somewhat
convoluted.
So, diff below makes struct __sem non-opaque and removes the indirect
allocations, so that the application is required to provide storage and
can therefore control where it's stored (which could be eg. shm).
After bootstrap (installing new header, building and installing the new
library) this survived base and xenocara 'make build' on amd64.
---
include/semaphore.h | 11 +++--
lib/libc/include/thread_private.h | 7 ---
lib/librthread/rthread.h| 4 +-
lib/librthread/rthread_sem.c| 70 +
lib/librthread/rthread_sem_compat.c | 68 +---
lib/librthread/shlib_version| 4 +-
6 files changed, 53 insertions(+), 111 deletions(-)
diff --git a/include/semaphore.h b/include/semaphore.h
index 21dc1a25bed..98471644357 100644
--- a/include/semaphore.h
+++ b/include/semaphore.h
@@ -38,10 +38,15 @@
#define _SEMAPHORE_H_
#include
+#include
-/* Opaque type definition. */
-struct __sem;
-typedef struct __sem *sem_t;
+struct __sem {
+ _atomic_lock_t lock;
+ volatile int waitcount;
+ volatile int value;
+ int shared;
+};
+typedef struct __sem sem_t;
struct timespec;
#define SEM_FAILED ((sem_t *)0)
diff --git a/lib/libc/include/thread_private.h
b/lib/libc/include/thread_private.h
index 98dfaa63223..f39094e0274 100644
--- a/lib/libc/include/thread_private.h
+++ b/lib/libc/include/thread_private.h
@@ -273,13 +273,6 @@ __END_HIDDEN_DECLS
#define_SPINLOCK_UNLOCKED _ATOMIC_LOCK_UNLOCKED
-struct __sem {
- _atomic_lock_t lock;
- volatile int waitcount;
- volatile int value;
- int shared;
-};
-
TAILQ_HEAD(pthread_queue, pthread);
#ifdef FUTEX
diff --git a/lib/librthread/rthread.h b/lib/librthread/rthread.h
index 1af4509a9a0..89e39c9e0b6 100644
--- a/lib/librthread/rthread.h
+++ b/lib/librthread/rthread.h
@@ -79,8 +79,8 @@ struct pthread_spinlock {
(((size) + (_thread_pagesize - 1)) & ~(_thread_pagesize - 1))
__BEGIN_HIDDEN_DECLS
-int_sem_wait(sem_t, int, const struct timespec *, int *);
-int_sem_post(sem_t);
+int_sem_wait(sem_t *, int, const struct timespec *, int *);
+int_sem_post(sem_t *);
void _rthread_init(void);
struct stack *_rthread_alloc_stack(pthread_t);
diff --git a/lib/librthread/rthread_sem.c b/lib/librthread/rthread_sem.c
index bd96769dc39..984e5fb0047 100644
--- a/lib/librthread/rthread_sem.c
+++ b/lib/librthread/rthread_sem.c
@@ -55,7 +55,7 @@
* Internal implementation of semaphores
*/
int
-_sem_wait(sem_t sem, int can_eintr, const struct timespec *abstime,
+_sem_wait(sem_t *sem, int can_eintr, const struct timespec *abstime,
int *delayed_cancel)
{
unsigned int val;
@@ -86,7 +86,7 @@ _sem_wait(sem_t sem, int can_eintr, const struct timespec
*abstime,
/* always increment count */
int
-_sem_post(sem_t sem)
+_sem_post(sem_t *sem)
{
membar_exit_before_atomic();
atomic_inc_int(>value);
@@ -98,10 +98,8 @@ _sem_post(sem_t sem)
* exported semaphores
*/
int
-sem_init(sem_t *semp, int pshared, unsigned int value)
+sem_init(sem_t *sem, int pshared, unsigned int value)
{
- sem_t sem;
-
if (value > SEM_VALUE_MAX) {
errno = EINVAL;
return (-1);
@@ -142,26 +140,19 @@ sem_init(sem_t *semp, int pshared, unsigned int value)
#endif
}
- sem = calloc(1, sizeof(*sem));
- if (!sem) {
- errno = ENOSPC;
- return (-1);
- }
+ bzero(sem, sizeof(*sem));
sem->value = value;
- *semp = sem;
return (0);
}
int
-sem_destroy(sem_t *semp)
+sem_destroy(sem_t *sem)
{
- sem_t sem;
-
if (!_threads_ready) /* for SEM_MMAP_SIZE */
Re: ifconfig with UTF-8 nwid
On Sun, Feb 23, 2020 at 05:01:03AM -0700, Theo de Raadt wrote: > Stefan Sperling wrote: > > > I don't understand why the installer would matter. Nothing would actually > > change for the installer. We're sure as hell not going to add UTF-8 there. > > The installer's ifconfig would just deal in hex all the way as it does > > today. > > During install finding the right network could be tricky, but that's not a > > new problem. Upgrades would just work once hostname.if contains the right > > hex. > > In upgrade mode, the installer must be able to do the same work as > /etc/netstart -- the installer has a re-implimentation to parse and act > upon hostname.* files. If that behaves different because ifconfig doesn't > act the same way, upgrade won't work right in some circumstances. There's > a small chance your change in behaviour for ifconfig doesn't matter but > I am quite sceptical. Our expectations with respect to the contents of hostname.if remain unchanged. With and without this diff, using a network with non-ASCII characters requires adding a hex SSID in hostname.if. Nobody wants the complexity of UTF-8 SSIDs inside netstart, the installer, or on the argument list of ifconfig. This diff only intends to provide a hint (decoded UTF-8 SSID) when scan results are displayed by ifconfig in a fully installed UTF-8 enabled system. Not showing a decoded form means users may somehow try to obtain a decoded form to tell which networks they are looking at. And they might end up doing this in an ad-hoc unsafe way that doesn't filter control sequences. The diff allows ifconfig to do the decoding safely and only display data when it is actually printable, much like it filters out non-printable ASCII. In the default "C" locale, nothing changes. It's all hex, because there is no way to decode the data safely outside the UTF-8 locale. And this is also the world the installer remains in when it parses ifconfig scan output.
Re: ifconfig with UTF-8 nwid
Stefan Sperling wrote: > I don't understand why the installer would matter. Nothing would actually > change for the installer. We're sure as hell not going to add UTF-8 there. > The installer's ifconfig would just deal in hex all the way as it does today. > During install finding the right network could be tricky, but that's not a > new problem. Upgrades would just work once hostname.if contains the right hex. In upgrade mode, the installer must be able to do the same work as /etc/netstart -- the installer has a re-implimentation to parse and act upon hostname.* files. If that behaves different because ifconfig doesn't act the same way, upgrade won't work right in some circumstances. There's a small chance your change in behaviour for ifconfig doesn't matter but I am quite sceptical.
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: efiboot, serial port order
> Date: Sat, 22 Feb 2020 10:40:13 +0900 (JST)
> From: YASUOKA Masahiko
>
> Hi,
>
> efiboot is using ACPI UID to determine the minor number of comX.
>
> In sys/arch/amd64/stand/efiboot/efiboot.c:
> 646 for (i = 0; i < sz / sizeof(EFI_HANDLE); i++) {
> 647 /*
> 648 * Identify port number of the handle. This assumes ACPI
> 649 * UID 0-3 map to legacy COM[1-4] and they use the legacy
> 650 * port address.
> 651 */
> 652 status = EFI_CALL(BS->HandleProtocol, handles[i],
> _guid,
> 653 (void **));
> 654 if (EFI_ERROR(status))
> 655 continue;
> 656 uid = -1;
> 657 for (dp = dp0; !IsDevicePathEnd(dp);
> 658 dp = NextDevicePathNode(dp)) {
> 659 dpp = (EFI_DEV_PATH_PTR)dp;
> 660 if (DevicePathType(dp) == ACPI_DEVICE_PATH &&
> 661 DevicePathSubType(dp) == ACPI_DP)
> 662 if (dpp.Acpi->HID == EFI_PNP_ID(0x0501))
> {
> 663 uid = dpp.Acpi->UID;
> 664 break;
> 665 }
> 666 }
> 667 if (uid < 0 || nitems(serios) <= uid)
> 668 continue;
> 669
> 670 /* Prepare SERIAL_IO_INTERFACE */
> 671 status = EFI_CALL(BS->HandleProtocol, handles[i],
> _guid,
> 672 (void **));
> 673 if (EFI_ERROR(status))
> 674 continue;
> 675 serios[uid] = serio;
> 676 }
> 677 free(handles, sz);
> 678
> 679 for (i = 0; i < nitems(serios); i++) {
> 680 if (serios[i] != NULL)
> 681 printf(" com%d", i);
> 682 }
>
> I originally wrote this code, because I thought ACPI UID enumeration
> is better than the order of handles by EFI.
>
> On qemu or vmware, 2 serials mappped like the following:
>
> EFI handle ACPI UID I/O addr efiboot kernel
> 0 0 0x3f8 com0 com0
> 1 1 0x2f8 com1 com1
>
> EFI handle order and ACPI UID enumeration are same and they also match
> I/O address assignment.
>
> But on "HPE DL20 Gen10", 2 serials mappped like the following:
>
> EFI handle ACPI UID I/O addr efiboot kernel
> 0 1 0x3f8 com1 com0
> 1 0 0x2f8 com0 com1
>
> Note that EFI handle order and ACPI UID enumeration is different and
> ACPI UID enumeration doesn't match the order in I/O address
> assignment. In this case, since com0 or com1 are mixed up between
> efiboot and kernel, if serial is usable on efiboot, it becomes not
> usable on kernel.
>
> Fortunately we can use "machine comaddr" to fix up the problem.
>
>
> Also I don't know any actual case such that EFI handle order is wrong
> but ACPI UID is correct. If using ACPI UID is useless, we can apply
> the diff attached at last.
>
> comment?
I fear using the EFI handle order is going to cause similar problems
on some other machine. What we really need here is the io port
address. Unfortunately we can't map the UID into an address without a
full AML parser, which is not something we want in the bootloader.
On arm64 we use the SPCR table to select the serial port. This table
does contain the io address of the serial port. Unfortunately not all
machines have an SPCR table so we want to retain some of the existing
logic in case an SPCR table isn't provided.
> Index: sys/arch/amd64/stand/efiboot/efiboot.c
> ===
> RCS file: /disk/cvs/openbsd/src/sys/arch/amd64/stand/efiboot/efiboot.c,v
> retrieving revision 1.34
> diff -u -p -r1.34 efiboot.c
> --- sys/arch/amd64/stand/efiboot/efiboot.c29 Nov 2019 16:16:19 -
> 1.34
> +++ sys/arch/amd64/stand/efiboot/efiboot.c22 Feb 2020 01:34:59 -
> @@ -631,10 +631,8 @@ efi_com_probe(struct consdev *cn)
> EFI_HANDLE *handles = NULL;
> SERIAL_IO_INTERFACE *serio;
> EFI_STATUS status;
> - EFI_DEVICE_PATH *dp, *dp0;
> - EFI_DEV_PATH_PTR dpp;
> UINTNsz;
> - int i, uid = -1;
> + int i;
>
> cn->cn_pri = CN_LOWPRI;
> cn->cn_dev = makedev(8, 0);
> @@ -651,36 +649,12 @@ efi_com_probe(struct consdev *cn)
> return;
> }
>
> - for (i = 0; i < sz / sizeof(EFI_HANDLE); i++) {
> - /*
> - * Identify port number of the handle. This assumes ACPI
> - * UID 0-3 map to legacy COM[1-4] and they use the legacy
> - * port address.
Re: ifconfig with UTF-8 nwid
On Sat, Feb 22, 2020 at 10:21:56PM +, Stuart Henderson wrote: > On 2020/02/22 17:24, Stefan Sperling wrote: > > On Sat, Feb 22, 2020 at 02:56:54PM +0100, Mark Kettenis wrote: > > > IMHO it is a bad idea to make the output of ifconfig locale-dependent. > > > > Fine. I'll drop this diff. > > > > Pity, it is quite useful if you are somewhere that uses UTF-8 SSIDs, > otherwise it's a complete pain to decode to figure out if you're > connecting to the correct network. If ifconfig cannot be locale-dependent then there is no way to do this safely. This requirement alone means that we must stay with the status quo and this can never be improved in base. We are not going to add another tool in base alongside ifconfig just for this. Adding a new tool via a port might have more luck, but that would be colorls all over again unless the port also provides other features we're missing (such as safe WPA enterprise or wifi network selection via desktop menus). I don't understand why the installer would matter. Nothing would actually change for the installer. We're sure as hell not going to add UTF-8 there. The installer's ifconfig would just deal in hex all the way as it does today. During install finding the right network could be tricky, but that's not a new problem. Upgrades would just work once hostname.if contains the right hex.
Re: usbdevs: small addition
On Sun, Feb 23, 2020 at 11:14:37AM +0100, Jasper Lievisse Adriaanse wrote: > On Sun, Feb 23, 2020 at 11:52:10AM +1100, Jonathan Gray wrote: > > On Sat, Feb 22, 2020 at 04:22:25PM +0100, Jasper Lievisse Adriaanse wrote: > > > Hi, > > > > > > - add an AMD product found on the APU2 > > > > I would not consider 0x7900 a root hub as it attaches to another hub > > > > uhub2 at uhub1 port 1 configuration 1 interface 0 "Advanced Micro Devices > > product 0x7900" rev 2.00/0.18 addr 2 > I'll change it to 'Hub' then. > > > > - add vendor id for "Synaptics, Inc. > > > > I'd just go with "Synaptics" > Okey. > > > > - add synaptics fingerprint reader found on recent thinkpads; I couldn't > > > find a proper > > > name for this device in the Linux usb.ids repository so I went with the > > > generic > > > 'Fingerprint Reader" that's also used elsewhere in this file. > > > > does it not supply a string itself? > How would I get the device to do this? If the device has a string it won't show as "product 0x00bd" in dmesg. Going by a dmesg I found it doesn't have a string so adding it is fine. > > > the lenovo windows driver matches on 0x06cb:0x00bd and 0x06cb:0x00c2 > > and calls itself "Synaptics UWP WBDI" which isn't very helpful. > I've added the 0x00c2 to the diff below, re-using the original description > for now. ok jsg@ > > > Index: usbdevs > === > RCS file: /cvs/src/sys/dev/usb/usbdevs,v > retrieving revision 1.710 > diff -u -p -r1.710 usbdevs > --- usbdevs 27 Jan 2020 15:41:42 - 1.710 > +++ usbdevs 23 Feb 2020 10:12:02 - > @@ -262,6 +262,7 @@ vendor AGFA 0x06bd AGFA-Gevaert > vendor ASIAMD0x06be Asia Microelectronic Development > vendor PHIDGETS 0x06c2 Phidgets > vendor BIZLINK 0x06c4 Bizlink International > +vendor SYNAPTICS 0x06cb Synaptics > vendor KEYSPAN 0x06cd Keyspan > vendor AASHIMA 0x06d6 Aashima Technology > vendor LIEBERT 0x06da Liebert > @@ -888,6 +889,9 @@ product ALTI2 NEPTUNE30x6001 Neptune 3 > product AMBIT WLAN 0x0302 WLAN > product AMBIT NTL_2500x6098 NTL 250 cable modem > > +/* Advanced Micro Devices products */ > +product AMD HUB 0x7900 Hub > + > /* Amigo Technology products */ > product AMIGO RT2870_1 0x9031 RT2870 > product AMIGO RT2870_2 0x9041 RT2870 > @@ -4167,6 +4171,10 @@ product SWEEX2 LW153 0x0153 LW153 > product SWEEX2 LW154 0x0154 LW154 > product SWEEX2 LW303 0x0302 LW303 > product SWEEX2 LW313 0x0313 LW313 > + > +/* Synaptics products */ > +product SYNAPTICS FPRINT 0x00bd Fingerprint Reader > +product SYNAPTICS FPRINT_2 0x00c2 Fingerprint Reader > > /* Syntech Information products */ > product SYNTECH SERIAL 0x0001 Serial > > -- > jasper > >
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: usbdevs: small addition
On Sun, Feb 23, 2020 at 11:52:10AM +1100, Jonathan Gray wrote: > On Sat, Feb 22, 2020 at 04:22:25PM +0100, Jasper Lievisse Adriaanse wrote: > > Hi, > > > > - add an AMD product found on the APU2 > > I would not consider 0x7900 a root hub as it attaches to another hub > > uhub2 at uhub1 port 1 configuration 1 interface 0 "Advanced Micro Devices > product 0x7900" rev 2.00/0.18 addr 2 I'll change it to 'Hub' then. > > - add vendor id for "Synaptics, Inc. > > I'd just go with "Synaptics" Okey. > > - add synaptics fingerprint reader found on recent thinkpads; I couldn't > > find a proper > > name for this device in the Linux usb.ids repository so I went with the > > generic > > 'Fingerprint Reader" that's also used elsewhere in this file. > > does it not supply a string itself? How would I get the device to do this? > the lenovo windows driver matches on 0x06cb:0x00bd and 0x06cb:0x00c2 > and calls itself "Synaptics UWP WBDI" which isn't very helpful. I've added the 0x00c2 to the diff below, re-using the original description for now. Index: usbdevs === RCS file: /cvs/src/sys/dev/usb/usbdevs,v retrieving revision 1.710 diff -u -p -r1.710 usbdevs --- usbdevs 27 Jan 2020 15:41:42 - 1.710 +++ usbdevs 23 Feb 2020 10:12:02 - @@ -262,6 +262,7 @@ vendor AGFA 0x06bd AGFA-Gevaert vendor ASIAMD 0x06be Asia Microelectronic Development vendor PHIDGETS0x06c2 Phidgets vendor BIZLINK 0x06c4 Bizlink International +vendor SYNAPTICS 0x06cb Synaptics vendor KEYSPAN 0x06cd Keyspan vendor AASHIMA 0x06d6 Aashima Technology vendor LIEBERT 0x06da Liebert @@ -888,6 +889,9 @@ product ALTI2 NEPTUNE3 0x6001 Neptune 3 product AMBIT WLAN 0x0302 WLAN product AMBIT NTL_250 0x6098 NTL 250 cable modem +/* Advanced Micro Devices products */ +product AMD HUB0x7900 Hub + /* Amigo Technology products */ product AMIGO RT2870_1 0x9031 RT2870 product AMIGO RT2870_2 0x9041 RT2870 @@ -4167,6 +4171,10 @@ product SWEEX2 LW153 0x0153 LW153 product SWEEX2 LW154 0x0154 LW154 product SWEEX2 LW303 0x0302 LW303 product SWEEX2 LW313 0x0313 LW313 + +/* Synaptics products */ +product SYNAPTICS FPRINT 0x00bd Fingerprint Reader +product SYNAPTICS FPRINT_2 0x00c2 Fingerprint Reader /* Syntech Information products */ product SYNTECH SERIAL 0x0001 Serial -- jasper
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;
