> Date: Sat, 22 Feb 2020 10:40:13 +0900 (JST)
> From: YASUOKA Masahiko <yasu...@openbsd.org>
>
> 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],
> &devp_guid,
> 653 (void **)&dp0);
> 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],
> &serio_guid,
> 672 (void **)&serio);
> 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.c 29 Nov 2019 16:16:19 -0000
> 1.34
> +++ sys/arch/amd64/stand/efiboot/efiboot.c 22 Feb 2020 01:34:59 -0000
> @@ -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;
> UINTN sz;
> - 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.
> - */
> - 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 || nitems(serios) <= uid)
> - continue;
> -
> - /* Prepare SERIAL_IO_INTERFACE */
> + for (i = 0; i < sz / sizeof(EFI_HANDLE) && i < nitems(serios); i++) {
> status = EFI_CALL(BS->HandleProtocol, handles[i], &serio_guid,
> (void **)&serio);
> if (EFI_ERROR(status))
> continue;
> - serios[uid] = serio;
> + serios[i] = serio;
> }
> free(handles, sz);
>
>
>
