Re: [PATCH v2] MdeModulePkg/UefiBootManagerLib: Match the nested partitions

["Ni, Ray" <ray.ni@intel.com>]

Much much better. Even better than recursive call. Thanks!
Reviewed-by: Ray Ni <ray.ni@intel.com>


> -----Original Message-----
> From: Gary Lin <glin@suse.com>
> Sent: Wednesday, January 23, 2019 12:40 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ray <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Wang,
> Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>
> Subject: [PATCH v2] MdeModulePkg/UefiBootManagerLib: Match the
> nested partitions
> 
> In some cases, such as MD RAID1 in Linux, the bootloader may be in a nested
> EFI system partition partition. For example, sda1 and sdb1 are combined as
> md0 and the first partition of md0, md0p1, is an EFI system partition. Then,
> the bootloader can be located by the following device
> paths:
> 
> PCI()/SATA(sda)/Partition(sda1)/Partition(md0p1)/File(bootloader.efi)
> PCI()/SATA(sdb)/Partition(sdb1)/Partition(md0p1)/File(bootloader.efi)
> 
> To make the boot option more resilient, we may create a boot option with
> the short-form device path like "Partition(md0p1)/File(bootloader.efi)".
> 
> However, BmMatchPartitionDevicePathNode() only matched the first
> partition node and ignored the nested partitions, so the firmware would
> refuse to load bootloader.efi since "Partition(md0p1)" doesn't match either
> "Partition(sda1)" or "Partition(sda2)".
> 
> This commit modifies BmMatchPartitionDevicePathNode() to iterate all
> nested partitions so that the above boot option could work.
> 
> v2 - Simplify the node matching logic
> 
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Gary Lin <glin@suse.com>
> ---
>  .../Library/UefiBootManagerLib/BmBoot.c       | 38 +++++++++----------
>  1 file changed, 17 insertions(+), 21 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> index 6a23477eb873..2284ce1a5646 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> @@ -1979,37 +1979,33 @@ BmMatchPartitionDevicePathNode (
>    }
> 
>    //
> -  // find the partition device path node
> +  // Match all the partition device path nodes including the nested
> + partition nodes
>    //
>    while (!IsDevicePathEnd (BlockIoDevicePath)) {
>      if ((DevicePathType (BlockIoDevicePath) == MEDIA_DEVICE_PATH) &&
>          (DevicePathSubType (BlockIoDevicePath) == MEDIA_HARDDRIVE_DP)
>          ) {
> -      break;
> +      //
> +      // See if the harddrive device path in blockio matches the orig Hard Drive
> Node
> +      //
> +      Node = (HARDDRIVE_DEVICE_PATH *) BlockIoDevicePath;
> +
> +      //
> +      // Match Signature and PartitionNumber.
> +      // Unused bytes in Signature are initiaized with zeros.
> +      //
> +      if ((Node->PartitionNumber == HardDriveDevicePath->PartitionNumber)
> &&
> +          (Node->MBRType == HardDriveDevicePath->MBRType) &&
> +          (Node->SignatureType == HardDriveDevicePath->SignatureType) &&
> +          (CompareMem (Node->Signature, HardDriveDevicePath->Signature,
> sizeof (Node->Signature)) == 0)) {
> +        return TRUE;
> +      }
>      }
> 
>      BlockIoDevicePath = NextDevicePathNode (BlockIoDevicePath);
>    }
> 
> -  if (IsDevicePathEnd (BlockIoDevicePath)) {
> -    return FALSE;
> -  }
> -
> -  //
> -  // See if the harddrive device path in blockio matches the orig Hard Drive
> Node
> -  //
> -  Node = (HARDDRIVE_DEVICE_PATH *) BlockIoDevicePath;
> -
> -  //
> -  // Match Signature and PartitionNumber.
> -  // Unused bytes in Signature are initiaized with zeros.
> -  //
> -  return (BOOLEAN) (
> -    (Node->PartitionNumber == HardDriveDevicePath->PartitionNumber) &&
> -    (Node->MBRType == HardDriveDevicePath->MBRType) &&
> -    (Node->SignatureType == HardDriveDevicePath->SignatureType) &&
> -    (CompareMem (Node->Signature, HardDriveDevicePath->Signature,
> sizeof (Node->Signature)) == 0)
> -    );
> +  return FALSE;
>  }
> 
>  /**
> --
> 2.20.1