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

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

(Send again. Previous mail might be lost.)
Gary,
I have two comments:
1. Do you need to consider the case when the multiple partition nodes might not be adjacent?
2. Maybe you could recursively call the function itself instead of using a loop. In this new way, you might be able to save some code.

> -----Original Message-----
> From: Gary Lin <glin@suse.com>
> Sent: Tuesday, January 15, 2019 5:46 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] 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.
> 
> 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       | 37 ++++++++++++-------
>  1 file changed, 23 insertions(+), 14 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> index 6a23477eb873..8354c2af674b 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> @@ -1995,21 +1995,30 @@ BmMatchPartitionDevicePathNode (
>      return FALSE;
>    }
> 
> -  //
> -  // See if the harddrive device path in blockio matches the orig Hard Drive Node
> -  //
> -  Node = (HARDDRIVE_DEVICE_PATH *) BlockIoDevicePath;
> +  do {
> +    //
> +    // 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)
> -    );
> +    //
> +    // 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;
> +    }
> +
> +    // See if a nested partition exists
> +    BlockIoDevicePath = NextDevicePathNode (BlockIoDevicePath);  }
> + while (!IsDevicePathEnd (BlockIoDevicePath) &&
> +           (DevicePathType (BlockIoDevicePath) == MEDIA_DEVICE_PATH) &&
> +           (DevicePathSubType (BlockIoDevicePath) ==
> + MEDIA_HARDDRIVE_DP));
> +
> +  return FALSE;
>  }
> 
>  /**
> --
> 2.20.1