From 84b95a121a4401be854614419ded3d383e14ac1f Mon Sep 17 00:00:00 2001 From: Michael Chang Date: Fri, 22 Mar 2024 17:38:45 +0800 Subject: [PATCH] ofdisk: Enhance canonical path handling for bootpath This commit addresses an issue where redundant canonical path translation is performed on the bootpath, potentially leading to incorrect results and subsequent boot failures, particularly in cases where firmware translations are inconsistent. To mitigate this, the commit introduces a check to determine if the bootpath is already in canonical form, avoiding unnecessary translation. Additionally, improvements have been made to enhance the resilience of device iteration, enhancing compatibility with cross-device booting scenarios and addressing potential issues related to firmware-based canonical path retrieval. These changes aim to improve the reliability and stability of the boot process. Signed-off-by: Michael Chang --- grub-core/disk/ieee1275/ofdisk.c | 75 +++++++++++++++++++++++--------- 1 file changed, 55 insertions(+), 20 deletions(-) diff --git a/grub-core/disk/ieee1275/ofdisk.c b/grub-core/disk/ieee1275/ofdisk.c index c5c20a5ec..36ee5314d 100644 --- a/grub-core/disk/ieee1275/ofdisk.c +++ b/grub-core/disk/ieee1275/ofdisk.c @@ -35,8 +35,13 @@ static grub_ieee1275_ihandle_t last_ihandle; #define IEEE1275_DISK_ALIAS "/disk@" #define IEEE1275_NVMEOF_DISK_ALIAS "/nvme-of/controller@" +/* Used to check boot_type, print debug message if doesn't match, this can be + * useful to measure boot delays */ static char *boot_type; +/* Used to restrict fcp to a physical boot path */ static char *boot_parent; +/* Knowing the nvmeof in advance to avoid blind open test during iteration to + * validate a path */ static int is_boot_nvmeof; struct ofdisk_hash_ent @@ -540,20 +545,30 @@ dev_iterate (const struct grub_ieee1275_devalias *alias) { if (grub_strcmp (alias->type, "fcp") == 0) { - if (boot_type && - grub_strcmp (boot_type, alias->type) != 0) + if (boot_parent && + grub_strcmp (boot_parent, alias->path) != 0) { - grub_dprintf ("ofdisk", "Skipped device: %s, type %s did not match boot_type %s\n", - alias->path, alias->type, boot_type); + grub_dprintf ("ofdisk", "Skipped device: %s, doesn't match boot_parent %s\n", + alias->path, boot_parent); goto iter_children; } - if (grub_strcmp (boot_parent, alias->path) == 0) + /* Allow set boot_parent and boot_type to NULL to force iteration */ + if (!boot_parent) { - if (is_boot_nvmeof) - dev_iterate_fcp_nvmeof(alias); - else - dev_iterate_fcp_disks(alias); + grub_dprintf ("ofdisk", "iterate %s\n", alias->path); + dev_iterate_fcp_nvmeof(alias); + dev_iterate_fcp_disks(alias); + } + else if (is_boot_nvmeof) + { + grub_dprintf ("ofdisk", "iterate nvmeof: %s\n", alias->path); + dev_iterate_fcp_nvmeof(alias); + } + else + { + grub_dprintf ("ofdisk", "iterate fcp: %s\n", alias->path); + dev_iterate_fcp_disks(alias); } } else if (grub_strcmp (alias->type, "vscsi") == 0) @@ -575,9 +590,8 @@ dev_iterate (const struct grub_ieee1275_devalias *alias) if (boot_type && grub_strcmp (boot_type, alias->type) != 0) { - grub_dprintf ("ofdisk", "Skipped device: %s, type %s did not match boot_type %s\n", + grub_dprintf ("ofdisk", "WARN: device: %s, type %s not match boot_type %s\n", alias->path, alias->type, boot_type); - return; } if (grub_ieee1275_open (alias->path, &ihandle)) @@ -646,9 +660,8 @@ dev_iterate (const struct grub_ieee1275_devalias *alias) if (boot_type && grub_strcmp (boot_type, alias->type) != 0) { - grub_dprintf ("ofdisk", "Skipped device: %s, type %s did not match boot_type %s\n", + grub_dprintf ("ofdisk", "WARN: device: %s, type %s not match boot_type %s\n", alias->path, alias->type, boot_type); - goto iter_children; } buf = grub_malloc (grub_strlen (alias->path) + @@ -1116,13 +1129,37 @@ get_parent_devname (const char *devname, int *is_nvmeof) return parent; } + +static int +is_canonical (const char *path) +{ + if (grub_strstr (path, IEEE1275_DISK_ALIAS) || + grub_strstr (path, IEEE1275_NVMEOF_DISK_ALIAS)) + return 1; + else + return 0; +} + static char * get_boot_device_parent (const char *bootpath, int *is_nvmeof) { - char *dev, *canon, *parent; + char *canon, *parent; + + if (is_canonical (bootpath)) + { + early_log ("Use %s as canonical\n", bootpath); + canon = grub_strdup (bootpath); + } + else + { + char *dev; - dev = grub_ieee1275_get_aliasdevname (bootpath); - canon = grub_ieee1275_canonicalise_devname (dev); + dev = grub_ieee1275_get_aliasdevname (bootpath); + canon = grub_ieee1275_canonicalise_devname (dev); + early_log ("bootpath: %s \n", bootpath); + early_log ("alias: %s\n", dev); + early_log ("canonical: %s\n", canon); + } if (!canon) { @@ -1131,8 +1168,6 @@ get_boot_device_parent (const char *bootpath, int *is_nvmeof) grub_print_error (); return NULL; } - else - early_log ("%s is canonical %s\n", bootpath, canon); parent = get_parent_devname (canon, is_nvmeof); early_log ("%s is parent of %s\n", parent, canon); @@ -1179,9 +1214,9 @@ insert_bootpath (void) boot_parent = get_boot_device_parent (bootpath, &is_boot_nvmeof); boot_type = grub_ieee1275_get_device_type (boot_parent); if (boot_type) - early_log ("the boot device type %s is used for root device discovery, others excluded\n", boot_type); + early_log ("the boot device type: %s\n", boot_type); else - early_log ("unknown boot device type, will use all devices to discover root and may be slow\n"); + early_log ("the boot device type is unknown\n"); } grub_free (type); grub_free (bootpath); -- 2.44.0