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

[Gary Lin]

On Wed, Jan 23, 2019 at 03:49:16AM +, Ni, Ray wrote:
> 
> 
> > -Original Message-
> > From: Ni, Ray
> > Sent: Wednesday, January 23, 2019 11:48 AM
> > To: 'Gary Lin' 
> > Cc: edk2-devel@lists.01.org; Zeng, Star ; Wang, Jian J
> > ; Wu, Hao A 
> > Subject: RE: [PATCH] MdeModulePkg/UefiBootManagerLib: Match the
> > nested partitions
> > 
> > 
> > 
> > > -Original Message-
> > > From: Gary Lin 
> > > Sent: Wednesday, January 23, 2019 10:13 AM
> > > To: Ni, Ray 
> > > Cc: edk2-devel@lists.01.org; Zeng, Star ; Wang,
> > > Jian J ; Wu, Hao A 
> > > Subject: Re: [PATCH] MdeModulePkg/UefiBootManagerLib: Match the
> > nested
> > > partitions
> > >
> > > On Tue, Jan 22, 2019 at 04:01:57PM +, Ni, Ray wrote:
> > > > (Send again. Previous mail might be lost.) Gary,
> > > Hi Ray,
> > >
> > > > I have two comments:
> > > > 1. Do you need to consider the case when the multiple partition
> > > > nodes
> > > might not be adjacent?
> > > In my cases, they are all partitions based on another partition, so
> > > all those nested partitions are adjacent to the "root" partition.
> > >
> > > I'm not sure if it's possible to create the node other than a nested
> > > partition node or a file node on a partition node.
> > Sure. Let's just assume the multiple partition nodes are 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.
> > > Since my targets are the partition nodes after a partition, loop
> > > brings a bit better performance by skipping the non-partition node
> > > check and saves the overhead of function call.
> > > If you prefer recursive call, I'll modify the patch accordingly.
> > I just want to avoid having two code blocks to check device path node 
> > against
> > END and partition node.
> > First one is in original while(). Second one is introduced by this patch.
> 
> If we can avoid the duplicated code, using loop is also fine.
Ok. I'll see what I can do.

Thanks,

Gary Lin

> 
> > 
> > >
> > > Thanks,
> > >
> > > Gary Lin
> > >
> > > > > -Original Message-
> > > > > From: Gary Lin 
> > > > > Sent: Tuesday, January 15, 2019 5:46 PM
> > > > > To: edk2-devel@lists.01.org
> > > > > Cc: Ni, Ray ; Zeng, Star ;
> > > > > Wang, Jian J ; Wu, Hao A
> > > > > 
> > > > > 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.e
> > > > > fi
> > > > > )
> > > > > PCI()/SATA(sdb)/Partition(sdb1)/Partition(md0p1)/File(bootloader.e
> > > > > fi
> > > > > )
> > > > >
> > > > > 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 
> > > > > Cc: Star Zeng 
> > > > > Cc: Jian J Wang 
> > > > > Cc: Hao Wu 
> > > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > > Signed-off-by: Gary Lin 
> > > > > ---
> > > > >  .../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.
> > > 

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

[Ni, Ray]

> -Original Message-
> From: Ni, Ray
> Sent: Wednesday, January 23, 2019 11:48 AM
> To: 'Gary Lin' 
> Cc: edk2-devel@lists.01.org; Zeng, Star ; Wang, Jian J
> ; Wu, Hao A 
> Subject: RE: [PATCH] MdeModulePkg/UefiBootManagerLib: Match the
> nested partitions
> 
> 
> 
> > -Original Message-
> > From: Gary Lin 
> > Sent: Wednesday, January 23, 2019 10:13 AM
> > To: Ni, Ray 
> > Cc: edk2-devel@lists.01.org; Zeng, Star ; Wang,
> > Jian J ; Wu, Hao A 
> > Subject: Re: [PATCH] MdeModulePkg/UefiBootManagerLib: Match the
> nested
> > partitions
> >
> > On Tue, Jan 22, 2019 at 04:01:57PM +, Ni, Ray wrote:
> > > (Send again. Previous mail might be lost.) Gary,
> > Hi Ray,
> >
> > > I have two comments:
> > > 1. Do you need to consider the case when the multiple partition
> > > nodes
> > might not be adjacent?
> > In my cases, they are all partitions based on another partition, so
> > all those nested partitions are adjacent to the "root" partition.
> >
> > I'm not sure if it's possible to create the node other than a nested
> > partition node or a file node on a partition node.
> Sure. Let's just assume the multiple partition nodes are 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.
> > Since my targets are the partition nodes after a partition, loop
> > brings a bit better performance by skipping the non-partition node
> > check and saves the overhead of function call.
> > If you prefer recursive call, I'll modify the patch accordingly.
> I just want to avoid having two code blocks to check device path node against
> END and partition node.
> First one is in original while(). Second one is introduced by this patch.

If we can avoid the duplicated code, using loop is also fine.

> 
> >
> > Thanks,
> >
> > Gary Lin
> >
> > > > -Original Message-
> > > > From: Gary Lin 
> > > > Sent: Tuesday, January 15, 2019 5:46 PM
> > > > To: edk2-devel@lists.01.org
> > > > Cc: Ni, Ray ; Zeng, Star ;
> > > > Wang, Jian J ; Wu, Hao A
> > > > 
> > > > 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.e
> > > > fi
> > > > )
> > > > PCI()/SATA(sdb)/Partition(sdb1)/Partition(md0p1)/File(bootloader.e
> > > > fi
> > > > )
> > > >
> > > > 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 
> > > > Cc: Star Zeng 
> > > > Cc: Jian J Wang 
> > > > Cc: Hao Wu 
> > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > Signed-off-by: Gary Lin 
> > > > ---
> > > >  .../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 

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

[Ni, Ray]

> -Original Message-
> From: Gary Lin 
> Sent: Wednesday, January 23, 2019 10:13 AM
> To: Ni, Ray 
> Cc: edk2-devel@lists.01.org; Zeng, Star ; Wang, Jian J
> ; Wu, Hao A 
> Subject: Re: [PATCH] MdeModulePkg/UefiBootManagerLib: Match the
> nested partitions
> 
> On Tue, Jan 22, 2019 at 04:01:57PM +, Ni, Ray wrote:
> > (Send again. Previous mail might be lost.) Gary,
> Hi Ray,
> 
> > I have two comments:
> > 1. Do you need to consider the case when the multiple partition nodes
> might not be adjacent?
> In my cases, they are all partitions based on another partition, so all those
> nested partitions are adjacent to the "root" partition.
> 
> I'm not sure if it's possible to create the node other than a nested partition
> node or a file node on a partition node.
Sure. Let's just assume the multiple partition nodes are 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.
> Since my targets are the partition nodes after a partition, loop brings a bit
> better performance by skipping the non-partition node check and saves the
> overhead of function call.
> If you prefer recursive call, I'll modify the patch accordingly.
I just want to avoid having two code blocks to check device path node against
END and partition node.
First one is in original while(). Second one is introduced by this patch.

> 
> Thanks,
> 
> Gary Lin
> 
> > > -Original Message-
> > > From: Gary Lin 
> > > Sent: Tuesday, January 15, 2019 5:46 PM
> > > To: edk2-devel@lists.01.org
> > > Cc: Ni, Ray ; Zeng, Star ;
> > > Wang, Jian J ; Wu, Hao A 
> > > 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 
> > > Cc: Star Zeng 
> > > Cc: Jian J Wang 
> > > Cc: Hao Wu 
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Gary Lin 
> > > ---
> > >  .../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)) {
> > > +  

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

[Gary Lin]

On Tue, Jan 22, 2019 at 04:01:57PM +, Ni, Ray wrote:
> (Send again. Previous mail might be lost.)
> Gary,
Hi Ray,

> I have two comments:
> 1. Do you need to consider the case when the multiple partition nodes might 
> not be adjacent?
In my cases, they are all partitions based on another partition, so all
those nested partitions are adjacent to the "root" partition.

I'm not sure if it's possible to create the node other than a nested
partition node or a file node on a partition node.

> 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.
Since my targets are the partition nodes after a partition, loop brings
a bit better performance by skipping the non-partition node check and
saves the overhead of function call.
If you prefer recursive call, I'll modify the patch accordingly.

Thanks,

Gary Lin

> > -Original Message-
> > From: Gary Lin 
> > Sent: Tuesday, January 15, 2019 5:46 PM
> > To: edk2-devel@lists.01.org
> > Cc: Ni, Ray ; Zeng, Star ; Wang, 
> > Jian J
> > ; Wu, Hao A 
> > 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 
> > Cc: Star Zeng 
> > Cc: Jian J Wang 
> > Cc: Hao Wu 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Gary Lin 
> > ---
> >  .../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
> 
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

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

[Ni, Ray]

(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 
> Sent: Tuesday, January 15, 2019 5:46 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ray ; Zeng, Star ; Wang, Jian J
> ; Wu, Hao A 
> 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 
> Cc: Star Zeng 
> Cc: Jian J Wang 
> Cc: Hao Wu 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Gary Lin 
> ---
>  .../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

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

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

[Laszlo Ersek]

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
>> , does
>> Linux use "footers" instead of "headers"?
>>
> See the "Sub-versions of the version-1 superblock" section:
> 
> 
> 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 
>>> Cc: Star Zeng 
>>> Cc: Jian J Wang 
>>> Cc: Hao Wu 
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Gary Lin 
>>> ---
>>>  .../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
>>

___
edk2-devel mailing list

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

[Gary Lin]

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
> , does
> Linux use "footers" instead of "headers"?
> 
See the "Sub-versions of the version-1 superblock" section:


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,

Gary Lin

> Thanks,
> 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 
> > Cc: Star Zeng 
> > Cc: Jian J Wang 
> > Cc: Hao Wu 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Gary Lin 
> > ---
> >  .../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
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

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

[Laszlo Ersek]

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
, does
Linux use "footers" instead of "headers"?

Thanks,
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 
> Cc: Star Zeng 
> Cc: Jian J Wang 
> Cc: Hao Wu 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Gary Lin 
> ---
>  .../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

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

[Gary Lin]

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 
Cc: Star Zeng 
Cc: Jian J Wang 
Cc: Hao Wu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Gary Lin 
---
 .../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

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel