Let me update the diff,
On Tue, 28 Feb 2017 22:41:21 +0900 (JST)
YASUOKA Masahiko <[email protected]> wrote:
> On Mon, 27 Feb 2017 11:10:31 +0100
> Patrick Wildt <[email protected]> wrote:
>> I'm surprised this didn't come up earlier, but I think the for-loop is a
>> bit wrong. What the code is supposed to be doing is going over each
>> device path node for the loaded image, which is supposed to be the path
>> to the device that efiboot was loaded from, and check for a node that
>> is a medium (as in hard drive, cdrom) and look for a hard drive.
>
> Yes, you are right it's wrong.
>
>> As it turns out, the code would actually skip all device paths that
>> are mediums and instead match on all non-media types. It's surprising
>> to me that this seems to work for people.
>>
>> On Qemu and VMware the first node in the device path is ACPI (2) with
>> subtype (1). Interestingly, the hard drive subtype in the media type
>> has the subtype (1) as well. So that might be a reason it did work.
>
> As far as my test on QEMU and VAIO, the first node is ACPI and subtype
> is 1.
>
> But we had another bug,
>
> 87 if (status == EFI_SUCCESS) {
> 88 for (dp = dp0; !IsDevicePathEnd(dp);
> 89 dp = NextDevicePathNode(dp)) {
> 90 if (DevicePathType(dp) == MEDIA_DEVICE_PATH)
> 91 continue;
> 92 if (DevicePathSubType(dp) ==
> MEDIA_HARDDRIVE_DP) {
> 93 bios_bootdev = 0x80;
> 94 efi_bootdp = dp;
> 95 break;
> 96 }
> 97 break;
> 98 }
> 99 }
>
> if we fix the bug at #90, at #94 efi_bootdp will not become the first
> dp node(dp0). Then efi_diskprobe() will fail to reorder hd? properly
> since the function assumes efi_bootdp is the first dp node.
>
> So I'd like to commit the diff below.
>
> diff --git a/sys/arch/amd64/stand/efiboot/efiboot.c
> b/sys/arch/amd64/stand/efiboot/efiboot.c
(snip)
> + if (efi_device_path_cmp(efi_bootdp, dp,
> MESSAGING_DEVICE_PATH)
> + == 0)
> bootdev = 1;
Not only the messaging device path but hardware device path and acpi
device path also must be compared.
diff --git a/sys/arch/amd64/stand/efiboot/efiboot.c
b/sys/arch/amd64/stand/efiboot/efiboot.c
index 1903862d271..2087a4112e3 100644
--- a/sys/arch/amd64/stand/efiboot/efiboot.c
+++ b/sys/arch/amd64/stand/efiboot/efiboot.c
@@ -41,7 +41,8 @@
EFI_SYSTEM_TABLE *ST;
EFI_BOOT_SERVICES *BS;
EFI_RUNTIME_SERVICES *RS;
-EFI_HANDLE IH, efi_bootdp = NULL;
+EFI_HANDLE IH;
+EFI_DEVICE_PATH *efi_bootdp = NULL;
EFI_PHYSICAL_ADDRESS heap;
EFI_LOADED_IMAGE *loadedImage;
UINTN heapsiz = 1 * 1024 * 1024;
@@ -51,6 +52,7 @@ static EFI_GUID blkio_guid = BLOCK_IO_PROTOCOL;
static EFI_GUID devp_guid = DEVICE_PATH_PROTOCOL;
u_long efi_loadaddr;
+static int efi_device_path_cmp(EFI_DEVICE_PATH *, EFI_DEVICE_PATH *, int);
static void efi_heap_init(void);
static void efi_memprobe_internal(void);
static void efi_video_init(void);
@@ -87,14 +89,12 @@ efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *systab)
if (status == EFI_SUCCESS) {
for (dp = dp0; !IsDevicePathEnd(dp);
dp = NextDevicePathNode(dp)) {
- if (DevicePathType(dp) == MEDIA_DEVICE_PATH)
- continue;
- if (DevicePathSubType(dp) == MEDIA_HARDDRIVE_DP) {
+ if (DevicePathType(dp) == MEDIA_DEVICE_PATH &&
+ DevicePathSubType(dp) == MEDIA_HARDDRIVE_DP) {
bios_bootdev = 0x80;
- efi_bootdp = dp;
+ efi_bootdp = dp0;
break;
}
- break;
}
}
@@ -166,7 +166,7 @@ efi_diskprobe(void)
EFI_BLOCK_IO *blkio;
EFI_BLOCK_IO_MEDIA *media;
struct diskinfo *di;
- EFI_DEVICE_PATH *dp, *bp;
+ EFI_DEVICE_PATH *dp;
TAILQ_INIT(&efi_disklist);
@@ -193,23 +193,18 @@ efi_diskprobe(void)
di = alloc(sizeof(struct diskinfo));
efid_init(di, blkio);
- if (efi_bootdp == NULL)
- goto next;
- status = EFI_CALL(BS->HandleProtocol, handles[i], &devp_guid,
- (void **)&dp);
- if (EFI_ERROR(status))
- goto next;
- bp = efi_bootdp;
- while (1) {
- if (IsDevicePathEnd(dp)) {
+ if (efi_bootdp != NULL) {
+ status = EFI_CALL(BS->HandleProtocol, handles[i],
&devp_guid,
+ (void **)&dp);
+ if (EFI_ERROR(status))
+ goto next;
+ if (efi_device_path_cmp(efi_bootdp, dp,
HARDWARE_DEVICE_PATH)
+ == 0 &&
+ efi_device_path_cmp(efi_bootdp, dp,
ACPI_DEVICE_PATH)
+ == 0 &&
+ efi_device_path_cmp(efi_bootdp, dp,
MESSAGING_DEVICE_PATH)
+ == 0)
bootdev = 1;
- break;
- }
- if (memcmp(dp, bp, sizeof(EFI_DEVICE_PATH)) != 0 ||
- memcmp(dp, bp, DevicePathNodeLength(dp)) != 0)
- break;
- dp = NextDevicePathNode(dp);
- bp = NextDevicePathNode(bp);
}
next:
if (bootdev)
@@ -221,6 +216,35 @@ next:
free(handles, sz);
}
+static int
+efi_device_path_cmp(EFI_DEVICE_PATH *dpa, EFI_DEVICE_PATH *dpb, int dptype)
+{
+ int cmp;
+ EFI_DEVICE_PATH *dp, *dpt_a = NULL, *dpt_b = NULL;
+
+ for (dp = dpa; !IsDevicePathEnd(dp); dp = NextDevicePathNode(dp)) {
+ if (DevicePathType(dp) == dptype) {
+ dpt_a = dp;
+ break;
+ }
+ }
+ for (dp = dpb; !IsDevicePathEnd(dp); dp = NextDevicePathNode(dp)) {
+ if (DevicePathType(dp) == dptype) {
+ dpt_b = dp;
+ break;
+ }
+ }
+
+ if (dpt_a && dpt_b) {
+ cmp = DevicePathNodeLength(dpt_a) - DevicePathNodeLength(dpt_b);
+ if (cmp)
+ return (cmp);
+ return (memcmp(dpt_a, dpt_b, DevicePathNodeLength(dpt_a)));
+ }
+
+ return ((uintptr_t)dpt_a - (uintptr_t)dpt_b);
+}
+
/***********************************************************************
* Memory
***********************************************************************/
