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

[Laszlo Ersek <lersek@redhat.com>]

On 01/16/19 03:16, Gary Lin wrote:
> On Tue, Jan 15, 2019 at 12:58:10PM +0100, Laszlo Ersek wrote:
>> On 01/15/19 10:45, Gary Lin wrote:
>>> 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)
>>
>> How does edk2 recognize the nested partition md0p1 in the first place?
>>
>> I would assume that the "outer" partitions (sda1, sdb1) start with some
>> kind of MD RAID1 header that allows Linux to combine sda1 and sdb1 into
>> md0p1. How does edk2 get past that header?
>>
>> Hmmm... based on
>> <https://raid.wiki.kernel.org/index.php/RAID_superblock_formats>, does
>> Linux use "footers" instead of "headers"?
>>
> See the "Sub-versions of the version-1 superblock" section:
> <https://raid.wiki.kernel.org/index.php/RAID_superblock_formats#Sub-versions_of_the_version-1_superblock>
> 
> For MD 0.9 and 1.0, the metadata is stored in the end of the disk, and
> those partitions, nested or not, are just like the normal partitions, so
> the firmare can see them without the knowledge of MD metadata. MD 1.1
> and 1.2 would be a different story because those two use "headers".
> 
> Anyway, I have tested RAID1 with MD 1.0 and OVMF actually appended md0p1
> in the device path. I only need to iterate the nested partitions in the
> match function to make my boot option work.

Thanks for the explanation! I'll let Ray review the change.
Laszlo

>>>
>>> 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;
>>>  }
>>>  
>>>  /**
>>>
>>
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
>>