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;
> + UINTN sz;
> + 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_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 */
> +
> + com_addr = comports[minor(cn->cn_dev)];
> +}
> +
> +int
> +efi_com_getc(dev_t dev)
> +{
> + EFI_STATUS status;
> + SERIAL_IO_INTERFACE *serio;
> + UINTN sz;
> + u_char buf[1];
Wouldn't it be enough to define u_char buf; and pass &buf?
> + static u_char lastchar = 0;
> +
> + if (!efi_valid_com(dev & 0x7f))
> + panic("com%d is not probed", minor(dev));
> + serio = serios[minor(dev & 0x7f)];
> +
> + if (lastchar != 0) {
> + int r = lastchar;
> + if ((dev & 0x80) == 0)
> + lastchar = 0;
> + return (r);
> + }
> +
> + for (;;) {
> + sz = 1;
> + status = EFI_CALL(serio->Read, serio, &sz, buf);
> + if (status == EFI_SUCCESS && sz > 0)
> + break;
> + if (status != EFI_TIMEOUT && EFI_ERROR(status))
> + panic("Error reading from serial status=%d", status);
> + if (dev & 0x80)
> + return (0);
> + }
> +
> + if (dev & 0x80)
> + lastchar = buf[0];
> +
> + return (buf[0]);
> +}
> +
> +void
> +efi_com_putc(dev_t dev, int c)
> +{
> + SERIAL_IO_INTERFACE *serio;
> + UINTN sz = 1;
> + u_char buf[1];
Same question here.
> +
> + if (!efi_valid_com(dev))
> + panic("com%d is not probed", minor(dev));
> + serio = serios[minor(dev)];
> + buf[0] = c;
> + EFI_CALL(serio->Write, serio, &sz, buf);
> +}
> +
> /***********************************************************************
> * Miscellaneous
> ***********************************************************************/
> diff --git a/sys/arch/amd64/stand/efiboot/efiboot.h
> b/sys/arch/amd64/stand/efiboot/efiboot.h
> index 84cbcb537a4..fb7abb7c5f4 100644
> --- a/sys/arch/amd64/stand/efiboot/efiboot.h
> +++ b/sys/arch/amd64/stand/efiboot/efiboot.h
> @@ -17,14 +17,18 @@
> */
>
> void efi_cleanup(void);
> -void efi_cons_probe (struct consdev *);
> -void efi_memprobe (void);
> -void efi_hardprobe (void);
> -void efi_diskprobe (void);
> -void efi_cons_init (struct consdev *);
> -int efi_cons_getc (dev_t);
> -void efi_cons_putc (dev_t, int);
> +void efi_cons_probe(struct consdev *);
> +void efi_memprobe(void);
> +void efi_hardprobe(void);
> +void efi_diskprobe(void);
> +void efi_cons_init(struct consdev *);
> +int efi_cons_getc(dev_t);
> +void efi_cons_putc(dev_t, int);
> int efi_cons_getshifts(dev_t dev);
> +void efi_com_probe(struct consdev *);
> +void efi_com_init(struct consdev *);
> +int efi_com_getc(dev_t);
> +void efi_com_putc(dev_t, int);
> int Xvideo_efi(void);
> int Xexit_efi(void);
> void efi_makebootargs(void);
> diff --git a/sys/arch/amd64/stand/libsa/dev_i386.c
> b/sys/arch/amd64/stand/libsa/dev_i386.c
> index ee1a11ca6b0..a2b55ec556d 100644
> --- a/sys/arch/amd64/stand/libsa/dev_i386.c
> +++ b/sys/arch/amd64/stand/libsa/dev_i386.c
> @@ -182,10 +182,8 @@ ttydev(char *name)
> int
> cnspeed(dev_t dev, int sp)
> {
> -#ifndef EFIBOOT
> if (major(dev) == 8) /* comN */
> return comspeed(dev, sp);
> -#endif
>
> /* pc0 and anything else */
> return 9600;
>
