Re: efiboot serial console support

2017-06-01 Thread YASUOKA Masahiko
Yes, I think this is the proper fix.  Please commit.

ok yasuoka

On Thu, 1 Jun 2017 10:41:57 +0200
Patrick Wildt  wrote:
> On Thu, Jun 01, 2017 at 10:27:28AM +0200, Stefan Sperling wrote:
>> On Tue, May 30, 2017 at 02:31:48PM +0200, YASUOKA Masahiko wrote:
>> > +  status = EFI_CALL(BS->LocateHandle, ByProtocol, &serio_guid, 0, &sz, 0);
>> > +  if (status == EFI_BUFFER_TOO_SMALL) {
>> > +  handles = alloc(sz);
>> > +  status = EFI_CALL(BS->LocateHandle, ByProtocol, &serio_guid,
>> > +  0, &sz, handles);
>> > +  }
>> > +  if (handles == NULL || EFI_ERROR(status))
>> > +  panic("could not get handles of serial i/o");
>> 
>> Hi,
>> 
>> On my thinkpad helix 2 the boot loader now keeps rebooting with
>> the above panic message before I can even type anything.
>> 
>> Could this panic be changed into a non-fatal error?
>> 
>> Thanks!
>> 
> 
> This should probably fix it / work around it.
> 
> diff --git a/sys/arch/amd64/stand/efiboot/efiboot.c 
> b/sys/arch/amd64/stand/efiboot/efiboot.c
> index 25e34c1a93b..998e6875f08 100644
> --- a/sys/arch/amd64/stand/efiboot/efiboot.c
> +++ b/sys/arch/amd64/stand/efiboot/efiboot.c
> @@ -526,8 +526,10 @@ efi_com_probe(struct consdev *cn)
>   status = EFI_CALL(BS->LocateHandle, ByProtocol, &serio_guid,
>   0, &sz, handles);
>   }
> - if (handles == NULL || EFI_ERROR(status))
> - panic("could not get handles of serial i/o");
> + if (handles == NULL || EFI_ERROR(status)) {
> + free(handles, sz);
> + return;
> + }
>  
>   for (i = 0; i < sz / sizeof(EFI_HANDLE); i++) {
>   /*
> 



Re: efiboot serial console support

2017-06-01 Thread Patrick Wildt
On Thu, Jun 01, 2017 at 10:27:28AM +0200, Stefan Sperling wrote:
> On Tue, May 30, 2017 at 02:31:48PM +0200, YASUOKA Masahiko wrote:
> > +   status = EFI_CALL(BS->LocateHandle, ByProtocol, &serio_guid, 0, &sz, 0);
> > +   if (status == EFI_BUFFER_TOO_SMALL) {
> > +   handles = alloc(sz);
> > +   status = EFI_CALL(BS->LocateHandle, ByProtocol, &serio_guid,
> > +   0, &sz, handles);
> > +   }
> > +   if (handles == NULL || EFI_ERROR(status))
> > +   panic("could not get handles of serial i/o");
> 
> Hi,
> 
> On my thinkpad helix 2 the boot loader now keeps rebooting with
> the above panic message before I can even type anything.
> 
> Could this panic be changed into a non-fatal error?
> 
> Thanks!
> 

This should probably fix it / work around it.

diff --git a/sys/arch/amd64/stand/efiboot/efiboot.c 
b/sys/arch/amd64/stand/efiboot/efiboot.c
index 25e34c1a93b..998e6875f08 100644
--- a/sys/arch/amd64/stand/efiboot/efiboot.c
+++ b/sys/arch/amd64/stand/efiboot/efiboot.c
@@ -526,8 +526,10 @@ efi_com_probe(struct consdev *cn)
status = EFI_CALL(BS->LocateHandle, ByProtocol, &serio_guid,
0, &sz, handles);
}
-   if (handles == NULL || EFI_ERROR(status))
-   panic("could not get handles of serial i/o");
+   if (handles == NULL || EFI_ERROR(status)) {
+   free(handles, sz);
+   return;
+   }
 
for (i = 0; i < sz / sizeof(EFI_HANDLE); i++) {
/*



Re: efiboot serial console support

2017-06-01 Thread Stefan Sperling
On Tue, May 30, 2017 at 02:31:48PM +0200, YASUOKA Masahiko wrote:
> + status = EFI_CALL(BS->LocateHandle, ByProtocol, &serio_guid, 0, &sz, 0);
> + if (status == EFI_BUFFER_TOO_SMALL) {
> + handles = alloc(sz);
> + status = EFI_CALL(BS->LocateHandle, ByProtocol, &serio_guid,
> + 0, &sz, handles);
> + }
> + if (handles == NULL || EFI_ERROR(status))
> + panic("could not get handles of serial i/o");

Hi,

On my thinkpad helix 2 the boot loader now keeps rebooting with
the above panic message before I can even type anything.

Could this panic be changed into a non-fatal error?

Thanks!



Re: efiboot serial console support

2017-05-30 Thread YASUOKA Masahiko
Thanks,

Let me update the diff.

On Tue, 30 May 2017 13:58:29 +0200
Patrick Wildt  wrote:
> On Tue, May 30, 2017 at 12:58:48PM +0200, YASUOKA Masahiko wrote:
>> The following diff is to support serial console on efiboot.
> 
> Nice.
>> 
>> It uses ACPI UID to identify the port number (com0, com1 and so on) of
>> probed serial interface.  But I'm not sure wether com0-com3 are always
>> mapped UID 0-3 as expected.  Though I think this is good enough.
>> 
>> Comment?

diff --git a/sys/arch/amd64/stand/efiboot/conf.c 
b/sys/arch/amd64/stand/efiboot/conf.c
index 0b2933d4cff..913a33e77a6 100644
--- a/sys/arch/amd64/stand/efiboot/conf.c
+++ b/sys/arch/amd64/stand/efiboot/conf.c
@@ -85,6 +85,7 @@ int ndevs = nitems(devsw);
 
 struct consdev constab[] = {
{ efi_cons_probe, efi_cons_init, efi_cons_getc, efi_cons_putc },
+   { efi_com_probe, efi_com_init, efi_com_getc, efi_com_putc },
{ NULL }
 };
 struct consdev *cn_tab = constab;
diff --git a/sys/arch/amd64/stand/efiboot/efiboot.c 
b/sys/arch/amd64/stand/efiboot/efiboot.c
index d668258989f..fb7587fd50e 100644
--- a/sys/arch/amd64/stand/efiboot/efiboot.c
+++ b/sys/arch/amd64/stand/efiboot/efiboot.c
@@ -501,10 +501,177 @@ efi_cons_getshifts(dev_t dev)
return (0);
 }
 
-/* XXX: serial console is not supported yet */
 int com_addr = -1;
 int com_speed = -1;
 
+static SERIAL_IO_INTERFACE *serios[4];
+
+void
+efi_com_probe(struct consdev *cn)
+{
+   EFI_GUID serio_guid = SERIAL_IO_PROTOCOL;
+   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;
+
+   sz = 0;
+   status = EFI_CALL(BS->LocateHandle, ByProtocol, &serio_guid, 0, &sz, 0);
+   if (status == EFI_BUFFER_TOO_SMALL) {
+   handles = alloc(sz);
+   status = EFI_CALL(BS->LocateHandle, ByProtocol, &serio_guid,
+   0, &sz, handles);
+   }
+   if (handles == NULL || EFI_ERROR(status))
+   panic("could not get handles of serial i/o");
+
+   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.
+*/
+   status = EFI_CALL(BS->HandleProtocol, handles[i], &devp_guid,
+   (void **)&dp0);
+   if (EFI_ERROR(status))
+   continue;
+   uid = -1;
+   for (dp = dp0; !IsDevicePathEnd(dp);
+   dp = NextDevicePathNode(dp)) {
+   dpp = (EFI_DEV_PATH_PTR)dp;
+   if (DevicePathType(dp) == ACPI_DEVICE_PATH &&
+   DevicePathSubType(dp) == ACPI_DP)
+   if (dpp.Acpi->HID == EFI_PNP_ID(0x0501)) {
+   uid = dpp.Acpi->UID;
+   break;
+   }
+   }
+   if (uid < 0)
+   continue;
+
+   /* Prepare SERIAL_IO_INTERFACE */
+   status = EFI_CALL(BS->HandleProtocol, handles[i], &serio_guid,
+   (void **)&serio);
+   if (EFI_ERROR(status))
+   continue;
+   if (uid < nitems(serios))
+   serios[uid] = serio;
+   }
+   free(handles, sz);
+
+   for (i = 0; i < nitems(serios); i++) {
+   if (serios[i] != NULL)
+   printf(" com%d", i);
+   }
+   cn->cn_pri = CN_LOWPRI;
+   cn->cn_dev = makedev(8, 0);
+}
+
+int
+efi_valid_com(dev_t dev)
+{
+   return (0 <= minor(dev) && minor(dev) < nitems(serios) &&
+   serios[minor(dev)] != NULL);
+}
+
+int
+comspeed(dev_t dev, int sp)
+{
+   EFI_STATUS   status;
+   SERIAL_IO_INTERFACE *serio = serios[minor(dev)];
+   int  newsp;
+
+   if (sp <= 0)
+   return com_speed;
+
+   if (!efi_valid_com(dev))
+   return (-1);
+
+   if (serio->Mode->BaudRate != sp) {
+   status = EFI_CALL(serio->SetAttributes, serio,
+   sp, serio->Mode->ReceiveFifoDepth,
+   serio->Mode->Timeout, serio->Mode->Parity,
+   serio->Mode->DataBits, serio->Mode->StopBits);
+   if (EFI_ERROR(status)) {
+   printf("com%d: SetAttribute() failed with status=%d\n",
+   minor(dev), status);
+   return (-1);
+   }
+   if (com_speed != -1)
+   printf("\ncom%d: %d baud\n", minor(dev), sp);
+   }
+
+   /* same as comspeed() in libsa/bio

Re: efiboot serial console support

2017-05-30 Thread Patrick Wildt
On Tue, May 30, 2017 at 12:58:48PM +0200, YASUOKA Masahiko wrote:
> Hi,
> 
> The following diff is to support serial console on efiboot.

Nice.

> 
> It uses ACPI UID to identify the port number (com0, com1 and so on) of
> probed serial interface.  But I'm not sure wether com0-com3 are always
> mapped UID 0-3 as expected.  Though I think this is good enough.
> 
> Comment?
> 
> diff --git a/sys/arch/amd64/stand/efiboot/conf.c 
> b/sys/arch/amd64/stand/efiboot/conf.c
> index 0b2933d4cff..913a33e77a6 100644
> --- a/sys/arch/amd64/stand/efiboot/conf.c
> +++ b/sys/arch/amd64/stand/efiboot/conf.c
> @@ -85,6 +85,7 @@ int ndevs = nitems(devsw);
>  
>  struct consdev constab[] = {
>   { efi_cons_probe, efi_cons_init, efi_cons_getc, efi_cons_putc },
> + { efi_com_probe, efi_com_init, efi_com_getc, efi_com_putc },
>   { NULL }
>  };
>  struct consdev *cn_tab = constab;
> diff --git a/sys/arch/amd64/stand/efiboot/efiboot.c 
> b/sys/arch/amd64/stand/efiboot/efiboot.c
> index d668258989f..613ede425b6 100644
> --- a/sys/arch/amd64/stand/efiboot/efiboot.c
> +++ b/sys/arch/amd64/stand/efiboot/efiboot.c
> @@ -501,10 +501,171 @@ efi_cons_getshifts(dev_t dev)
>   return (0);
>  }
>  
> -/* XXX: serial console is not supported yet */
>  int com_addr = -1;
>  int com_speed = -1;
>  
> +static SERIAL_IO_INTERFACE   *serios[4];
> +const int comports[4] = { 0x3f8, 0x2f8, 0x3e8, 0x2e8 };
> +
> +void
> +efi_com_probe(struct consdev *cn)
> +{
> + EFI_GUID serio_guid = SERIAL_IO_PROTOCOL;
> + 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;
> +
> + sz = 0;
> + status = EFI_CALL(BS->LocateHandle, ByProtocol, &serio_guid, 0, &sz, 0);
> + if (status == EFI_BUFFER_TOO_SMALL) {
> + handles = alloc(sz);
> + status = EFI_CALL(BS->LocateHandle, ByProtocol, &serio_guid,
> + 0, &sz, handles);
> + }
> + if (handles == NULL || EFI_ERROR(status))
> + panic("could not get handles of serial i/o");
> +
> + for (i = 0; i < sz / sizeof(EFI_HANDLE); i++) {
> + /*
> +  * Identify port number of the handle.  This assumes ACPI
> +  * UID 0-4 map to legacy COM[1-4] and they use the legacy

I think youy meant UID 0-3.

> +  * port address.
> +  */
> + status = EFI_CALL(BS->HandleProtocol, handles[i], &devp_guid,
> + (void **)&dp0);
> + if (EFI_ERROR(status))
> + continue;
> + uid = -1;
> + for (dp = dp0; !IsDevicePathEnd(dp);
> + dp = NextDevicePathNode(dp)) {
> + dpp = (EFI_DEV_PATH_PTR)dp;
> + if (DevicePathType(dp) == ACPI_DEVICE_PATH &&
> + DevicePathSubType(dp) == ACPI_DP)
> + if (dpp.Acpi->HID == 0x50141d0 /* PNP0501 */) {

There is a macro for that.  EFI_PNP_ID(0x0501) should do.

> + uid = dpp.Acpi->UID;
> + break;
> + }
> + }
> + if (uid < 0)
> + continue;
> +
> + /* Prepare SERIAL_IO_INTERFACE */
> + status = EFI_CALL(BS->HandleProtocol, handles[i], &serio_guid,
> + (void **)&serio);
> + if (EFI_ERROR(status))
> + continue;
> + if (uid < nitems(serios))
> + serios[uid] = serio;
> + }
> + free(handles, sz);
> +
> + for (i = 0; i < nitems(serios); i++) {
> + if (serios[i] != NULL)
> + printf(" com%d", i);

Before the printf are 4 spaces instead of a tab.

> + }
> + cn->cn_pri = CN_LOWPRI;
> + cn->cn_dev = makedev(8, 0);
> +}
> +
> +int
> +efi_valid_com(dev_t dev)
> +{
> + return (0 <= minor(dev) && minor(dev) < nitems(serios) &&
> + serios[minor(dev)] != NULL);
> +}
> +
> +int
> +comspeed(dev_t dev, int sp)
> +{
> + EFI_STATUS   status;
> + SERIAL_IO_INTERFACE *serio = serios[minor(dev)];
> +
> + if (!efi_valid_com(dev))
> + return (-1);
> +
> + if (sp > 0 && serio->Mode->BaudRate != sp) {
> + status = EFI_CALL(serio->SetAttributes, serio,
> + sp, serio->Mode->ReceiveFifoDepth,
> + serio->Mode->Timeout, serio->Mode->Parity,
> + serio->Mode->DataBits, serio->Mode->StopBits);
> + if (EFI_ERROR(status)) {
> + painc("com%d: SetAttribute() failed with status=%d\n",
> + minor(dev), status);
> + }
> + com_speed = serio->Mode->BaudRate;
> + }
> + com_

efiboot serial console support

2017-05-30 Thread YASUOKA Masahiko
Hi,

The following diff is to support serial console on efiboot.

It uses ACPI UID to identify the port number (com0, com1 and so on) of
probed serial interface.  But I'm not sure wether com0-com3 are always
mapped UID 0-3 as expected.  Though I think this is good enough.

Comment?

diff --git a/sys/arch/amd64/stand/efiboot/conf.c 
b/sys/arch/amd64/stand/efiboot/conf.c
index 0b2933d4cff..913a33e77a6 100644
--- a/sys/arch/amd64/stand/efiboot/conf.c
+++ b/sys/arch/amd64/stand/efiboot/conf.c
@@ -85,6 +85,7 @@ int ndevs = nitems(devsw);
 
 struct consdev constab[] = {
{ efi_cons_probe, efi_cons_init, efi_cons_getc, efi_cons_putc },
+   { efi_com_probe, efi_com_init, efi_com_getc, efi_com_putc },
{ NULL }
 };
 struct consdev *cn_tab = constab;
diff --git a/sys/arch/amd64/stand/efiboot/efiboot.c 
b/sys/arch/amd64/stand/efiboot/efiboot.c
index d668258989f..613ede425b6 100644
--- a/sys/arch/amd64/stand/efiboot/efiboot.c
+++ b/sys/arch/amd64/stand/efiboot/efiboot.c
@@ -501,10 +501,171 @@ efi_cons_getshifts(dev_t dev)
return (0);
 }
 
-/* XXX: serial console is not supported yet */
 int com_addr = -1;
 int com_speed = -1;
 
+static SERIAL_IO_INTERFACE *serios[4];
+const int   comports[4] = { 0x3f8, 0x2f8, 0x3e8, 0x2e8 };
+
+void
+efi_com_probe(struct consdev *cn)
+{
+   EFI_GUID serio_guid = SERIAL_IO_PROTOCOL;
+   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;
+
+   sz = 0;
+   status = EFI_CALL(BS->LocateHandle, ByProtocol, &serio_guid, 0, &sz, 0);
+   if (status == EFI_BUFFER_TOO_SMALL) {
+   handles = alloc(sz);
+   status = EFI_CALL(BS->LocateHandle, ByProtocol, &serio_guid,
+   0, &sz, handles);
+   }
+   if (handles == NULL || EFI_ERROR(status))
+   panic("could not get handles of serial i/o");
+
+   for (i = 0; i < sz / sizeof(EFI_HANDLE); i++) {
+   /*
+* Identify port number of the handle.  This assumes ACPI
+* UID 0-4 map to legacy COM[1-4] and they use the legacy
+* port address.
+*/
+   status = EFI_CALL(BS->HandleProtocol, handles[i], &devp_guid,
+   (void **)&dp0);
+   if (EFI_ERROR(status))
+   continue;
+   uid = -1;
+   for (dp = dp0; !IsDevicePathEnd(dp);
+   dp = NextDevicePathNode(dp)) {
+   dpp = (EFI_DEV_PATH_PTR)dp;
+   if (DevicePathType(dp) == ACPI_DEVICE_PATH &&
+   DevicePathSubType(dp) == ACPI_DP)
+   if (dpp.Acpi->HID == 0x50141d0 /* PNP0501 */) {
+   uid = dpp.Acpi->UID;
+   break;
+   }
+   }
+   if (uid < 0)
+   continue;
+
+   /* Prepare SERIAL_IO_INTERFACE */
+   status = EFI_CALL(BS->HandleProtocol, handles[i], &serio_guid,
+   (void **)&serio);
+   if (EFI_ERROR(status))
+   continue;
+   if (uid < nitems(serios))
+   serios[uid] = serio;
+   }
+   free(handles, sz);
+
+   for (i = 0; i < nitems(serios); i++) {
+   if (serios[i] != NULL)
+   printf(" com%d", i);
+   }
+   cn->cn_pri = CN_LOWPRI;
+   cn->cn_dev = makedev(8, 0);
+}
+
+int
+efi_valid_com(dev_t dev)
+{
+   return (0 <= minor(dev) && minor(dev) < nitems(serios) &&
+   serios[minor(dev)] != NULL);
+}
+
+int
+comspeed(dev_t dev, int sp)
+{
+   EFI_STATUS   status;
+   SERIAL_IO_INTERFACE *serio = serios[minor(dev)];
+
+   if (!efi_valid_com(dev))
+   return (-1);
+
+   if (sp > 0 && serio->Mode->BaudRate != sp) {
+   status = EFI_CALL(serio->SetAttributes, serio,
+   sp, serio->Mode->ReceiveFifoDepth,
+   serio->Mode->Timeout, serio->Mode->Parity,
+   serio->Mode->DataBits, serio->Mode->StopBits);
+   if (EFI_ERROR(status)) {
+   painc("com%d: SetAttribute() failed with status=%d\n",
+   minor(dev), status);
+   }
+   com_speed = serio->Mode->BaudRate;
+   }
+   com_speed = sp;
+
+   return (serio->Mode->BaudRate);
+}
+
+void
+efi_com_init(struct consdev *cn)
+{
+   if (!efi_valid_com(cn->cn_dev))
+   panic("com%d is not probed", minor(cn->cn_dev));
+
+   if (com_speed == -1)
+   comspeed(cn->cn_dev, 9600); /* default speed is 9600 baud */
+
+