1. Patchew
  2. EDK2
  3. [edk2-devel] [PATCH V3 0/3] MdeModulePkg/PartitionDxe: Fix the partition check issue
  4. View patch

[edk2-devel] [PATCH V3 3/3] MdeModulePkg/PartitionDxe: Add already start check for child hanldes

Gao, Zhichao posted 3 patches 5 years, 7 months ago
[edk2-devel] [PATCH V3 3/3] MdeModulePkg/PartitionDxe: Add already start check for child hanldes
Posted by Gao, Zhichao 5 years, 7 months ago 
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2823

Treat the EFI_ALREADY_STARTED as EFI_SUCCESS to avoid the partition driver
continuely check next routine function.

Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
---
 MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
index d1c878ad2e..6a43c3cafb 100644
--- a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
+++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
@@ -1276,6 +1276,15 @@ PartitionInstallChildHandle (
   } else {
     FreePool (Private->DevicePath);
     FreePool (Private);
+
+    //
+    // if the Status == EFI_ALREADY_STARTED, it means the child handles
+    // are already installed. So return EFI_SUCCESS to avoid do the next
+    // partition type check.
+    //
+    if (Status == EFI_ALREADY_STARTED) {
+      Status = EFI_SUCCESS;
+    }
   }
 
   return Status;
-- 
2.21.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62462): https://edk2.groups.io/g/devel/message/62462
Mute This Topic: https://groups.io/mt/75490642/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V3 3/3] MdeModulePkg/PartitionDxe: Add already start check for child hanldes
Posted by Ni, Ray 5 years, 7 months ago 
Zhichao,
Can you add more information in the commit message on what bug the patch can fix?
With that, Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gao,
> Zhichao
> Sent: Tuesday, July 14, 2020 9:23 AM
> To: devel@edk2.groups.io
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: [edk2-devel] [PATCH V3 3/3] MdeModulePkg/PartitionDxe: Add
> already start check for child hanldes
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2823
> 
> Treat the EFI_ALREADY_STARTED as EFI_SUCCESS to avoid the partition driver
> continuely check next routine function.
> 
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
> ---
>  MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
> b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
> index d1c878ad2e..6a43c3cafb 100644
> --- a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
> +++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
> @@ -1276,6 +1276,15 @@ PartitionInstallChildHandle (
>    } else {
>      FreePool (Private->DevicePath);
>      FreePool (Private);
> +
> +    //
> +    // if the Status == EFI_ALREADY_STARTED, it means the child handles
> +    // are already installed. So return EFI_SUCCESS to avoid do the next
> +    // partition type check.
> +    //
> +    if (Status == EFI_ALREADY_STARTED) {
> +      Status = EFI_SUCCESS;
> +    }
>    }
> 
>    return Status;
> --
> 2.21.0.windows.1
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62465): https://edk2.groups.io/g/devel/message/62465
Mute This Topic: https://groups.io/mt/75490642/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V3 3/3] MdeModulePkg/PartitionDxe: Add already start check for child hanldes
Posted by Gao, Zhichao 5 years, 7 months ago 
Sure.

Update the commit message here:

The partition binding driver would run serval times during BDS. If the partition supports MBR, it would pass the first connect in the MBR routine function. The second connect would return already started which would be treated as not found and continue to run next routine. That is incorrect behavior. The device should only support one partition format. Treat the already started as success to avoid incorrect next partition routine check.

Thanks,
Zhichao

> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Tuesday, July 14, 2020 10:04 AM
> To: devel@edk2.groups.io; Gao, Zhichao <zhichao.gao@intel.com>
> Cc: Wu, Hao A <hao.a.wu@intel.com>
> Subject: RE: [edk2-devel] [PATCH V3 3/3] MdeModulePkg/PartitionDxe: Add
> already start check for child hanldes
> 
> Zhichao,
> Can you add more information in the commit message on what bug the patch
> can fix?
> With that, Reviewed-by: Ray Ni <ray.ni@intel.com>
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gao,
> > Zhichao
> > Sent: Tuesday, July 14, 2020 9:23 AM
> > To: devel@edk2.groups.io
> > Cc: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>
> > Subject: [edk2-devel] [PATCH V3 3/3] MdeModulePkg/PartitionDxe: Add
> > already start check for child hanldes
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2823
> >
> > Treat the EFI_ALREADY_STARTED as EFI_SUCCESS to avoid the partition
> > driver continuely check next routine function.
> >
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> > Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
> > ---
> >  MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
> > b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
> > index d1c878ad2e..6a43c3cafb 100644
> > --- a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
> > +++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
> > @@ -1276,6 +1276,15 @@ PartitionInstallChildHandle (
> >    } else {
> >      FreePool (Private->DevicePath);
> >      FreePool (Private);
> > +
> > +    //
> > +    // if the Status == EFI_ALREADY_STARTED, it means the child handles
> > +    // are already installed. So return EFI_SUCCESS to avoid do the next
> > +    // partition type check.
> > +    //
> > +    if (Status == EFI_ALREADY_STARTED) {
> > +      Status = EFI_SUCCESS;
> > +    }
> >    }
> >
> >    return Status;
> > --
> > 2.21.0.windows.1
> >
> >
> > 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62476): https://edk2.groups.io/g/devel/message/62476
Mute This Topic: https://groups.io/mt/75490642/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V3 3/3] MdeModulePkg/PartitionDxe: Add already start check for child hanldes
Posted by Ni, Ray 5 years, 7 months ago 
Looks good to me! Thanks!!

> -----Original Message-----
> From: Gao, Zhichao <zhichao.gao@intel.com>
> Sent: Tuesday, July 14, 2020 1:56 PM
> To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
> Cc: Wu, Hao A <hao.a.wu@intel.com>
> Subject: RE: [edk2-devel] [PATCH V3 3/3] MdeModulePkg/PartitionDxe: Add
> already start check for child hanldes
> 
> Sure.
> 
> Update the commit message here:
> 
> The partition binding driver would run serval times during BDS. If the partition
> supports MBR, it would pass the first connect in the MBR routine function. The
> second connect would return already started which would be treated as not
> found and continue to run next routine. That is incorrect behavior. The device
> should only support one partition format. Treat the already started as success
> to avoid incorrect next partition routine check.
> 
> Thanks,
> Zhichao
> 
> > -----Original Message-----
> > From: Ni, Ray <ray.ni@intel.com>
> > Sent: Tuesday, July 14, 2020 10:04 AM
> > To: devel@edk2.groups.io; Gao, Zhichao <zhichao.gao@intel.com>
> > Cc: Wu, Hao A <hao.a.wu@intel.com>
> > Subject: RE: [edk2-devel] [PATCH V3 3/3] MdeModulePkg/PartitionDxe: Add
> > already start check for child hanldes
> >
> > Zhichao,
> > Can you add more information in the commit message on what bug the patch
> > can fix?
> > With that, Reviewed-by: Ray Ni <ray.ni@intel.com>
> >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gao,
> > > Zhichao
> > > Sent: Tuesday, July 14, 2020 9:23 AM
> > > To: devel@edk2.groups.io
> > > Cc: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>
> > > Subject: [edk2-devel] [PATCH V3 3/3] MdeModulePkg/PartitionDxe: Add
> > > already start check for child hanldes
> > >
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2823
> > >
> > > Treat the EFI_ALREADY_STARTED as EFI_SUCCESS to avoid the partition
> > > driver continuely check next routine function.
> > >
> > > Cc: Hao A Wu <hao.a.wu@intel.com>
> > > Cc: Ray Ni <ray.ni@intel.com>
> > > Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> > > Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
> > > ---
> > >  MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c | 9 +++++++++
> > >  1 file changed, 9 insertions(+)
> > >
> > > diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
> > > b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
> > > index d1c878ad2e..6a43c3cafb 100644
> > > --- a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
> > > +++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
> > > @@ -1276,6 +1276,15 @@ PartitionInstallChildHandle (
> > >    } else {
> > >      FreePool (Private->DevicePath);
> > >      FreePool (Private);
> > > +
> > > +    //
> > > +    // if the Status == EFI_ALREADY_STARTED, it means the child handles
> > > +    // are already installed. So return EFI_SUCCESS to avoid do the next
> > > +    // partition type check.
> > > +    //
> > > +    if (Status == EFI_ALREADY_STARTED) {
> > > +      Status = EFI_SUCCESS;
> > > +    }
> > >    }
> > >
> > >    return Status;
> > > --
> > > 2.21.0.windows.1
> > >
> > >
> > > 
> >
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62477): https://edk2.groups.io/g/devel/message/62477
Mute This Topic: https://groups.io/mt/75490642/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V3 3/3] MdeModulePkg/PartitionDxe: Add already start check for child hanldes
Posted by Gary Lin 5 years, 6 months ago 
On Tue, Jul 14, 2020 at 09:22:59AM +0800, Gao, Zhichao via groups.io wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2823
> 
> Treat the EFI_ALREADY_STARTED as EFI_SUCCESS to avoid the partition driver
> continuely check next routine function.
Hi Zhichao,

I just found that this patch breaks the loading of openSUSE iso images[*].
Would you mind to take a look at it?

Thanks,

Gary Lin

[*] http://download.opensuse.org/distribution/leap/15.2/iso/openSUSE-Leap-15.2-NET-x86_64.iso

> 
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
> ---
>  MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
> index d1c878ad2e..6a43c3cafb 100644
> --- a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
> +++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
> @@ -1276,6 +1276,15 @@ PartitionInstallChildHandle (
>    } else {
>      FreePool (Private->DevicePath);
>      FreePool (Private);
> +
> +    //
> +    // if the Status == EFI_ALREADY_STARTED, it means the child handles
> +    // are already installed. So return EFI_SUCCESS to avoid do the next
> +    // partition type check.
> +    //
> +    if (Status == EFI_ALREADY_STARTED) {
> +      Status = EFI_SUCCESS;
> +    }
>    }
>  
>    return Status;
> -- 
> 2.21.0.windows.1
> 
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62648): https://edk2.groups.io/g/devel/message/62648
Mute This Topic: https://groups.io/mt/75490642/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V3 3/3] MdeModulePkg/PartitionDxe: Add already start check for child hanldes
Posted by Gao, Zhichao 5 years, 6 months ago 
Yes. I have checked the image with OVMF. The SUSE image has the MBR table but not like others, such as Ubuntu, RedHat and Fedora which would contain a partition entry with the entire disk.

That means it would pass the MBR child handle check and would never go to the UDF(or ElTorito) check.

I think we should update the invalid MBR check logic.

Thanks,
Zhichao

> -----Original Message-----
> From: Gary Lin <glin@suse.com>
> Sent: Thursday, July 16, 2020 11:33 AM
> To: devel@edk2.groups.io; Gao, Zhichao <zhichao.gao@intel.com>
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: Re: [edk2-devel] [PATCH V3 3/3] MdeModulePkg/PartitionDxe: Add
> already start check for child hanldes
> 
> On Tue, Jul 14, 2020 at 09:22:59AM +0800, Gao, Zhichao via groups.io wrote:
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2823
> >
> > Treat the EFI_ALREADY_STARTED as EFI_SUCCESS to avoid the partition
> > driver continuely check next routine function.
> Hi Zhichao,
> 
> I just found that this patch breaks the loading of openSUSE iso images[*].
> Would you mind to take a look at it?
> 
> Thanks,
> 
> Gary Lin
> 
> [*] http://download.opensuse.org/distribution/leap/15.2/iso/openSUSE-Leap-
> 15.2-NET-x86_64.iso
> 
> >
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> > Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
> > ---
> >  MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
> > b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
> > index d1c878ad2e..6a43c3cafb 100644
> > --- a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
> > +++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
> > @@ -1276,6 +1276,15 @@ PartitionInstallChildHandle (
> >    } else {
> >      FreePool (Private->DevicePath);
> >      FreePool (Private);
> > +
> > +    //
> > +    // if the Status == EFI_ALREADY_STARTED, it means the child handles
> > +    // are already installed. So return EFI_SUCCESS to avoid do the next
> > +    // partition type check.
> > +    //
> > +    if (Status == EFI_ALREADY_STARTED) {
> > +      Status = EFI_SUCCESS;
> > +    }
> >    }
> >
> >    return Status;
> > --
> > 2.21.0.windows.1
> >
> >
> > 
> >


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62650): https://edk2.groups.io/g/devel/message/62650
Mute This Topic: https://groups.io/mt/75490642/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V3 3/3] MdeModulePkg/PartitionDxe: Add already start check for child hanldes
Posted by Ni, Ray 5 years, 6 months ago 
Zhichao,
What does the SUSE MBR table look like?

Thanks,
Ray

> -----Original Message-----
> From: Gao, Zhichao <zhichao.gao@intel.com>
> Sent: Thursday, July 16, 2020 1:09 PM
> To: Gary Lin <glin@suse.com>; devel@edk2.groups.io
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: RE: [edk2-devel] [PATCH V3 3/3] MdeModulePkg/PartitionDxe: Add already start check for child hanldes
> 
> Yes. I have checked the image with OVMF. The SUSE image has the MBR table but not like others, such as Ubuntu, RedHat
> and Fedora which would contain a partition entry with the entire disk.
> 
> That means it would pass the MBR child handle check and would never go to the UDF(or ElTorito) check.
> 
> I think we should update the invalid MBR check logic.
> 
> Thanks,
> Zhichao
> 
> > -----Original Message-----
> > From: Gary Lin <glin@suse.com>
> > Sent: Thursday, July 16, 2020 11:33 AM
> > To: devel@edk2.groups.io; Gao, Zhichao <zhichao.gao@intel.com>
> > Cc: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>
> > Subject: Re: [edk2-devel] [PATCH V3 3/3] MdeModulePkg/PartitionDxe: Add
> > already start check for child hanldes
> >
> > On Tue, Jul 14, 2020 at 09:22:59AM +0800, Gao, Zhichao via groups.io wrote:
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2823
> > >
> > > Treat the EFI_ALREADY_STARTED as EFI_SUCCESS to avoid the partition
> > > driver continuely check next routine function.
> > Hi Zhichao,
> >
> > I just found that this patch breaks the loading of openSUSE iso images[*].
> > Would you mind to take a look at it?
> >
> > Thanks,
> >
> > Gary Lin
> >
> > [*] http://download.opensuse.org/distribution/leap/15.2/iso/openSUSE-Leap-
> > 15.2-NET-x86_64.iso
> >
> > >
> > > Cc: Hao A Wu <hao.a.wu@intel.com>
> > > Cc: Ray Ni <ray.ni@intel.com>
> > > Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> > > Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
> > > ---
> > >  MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c | 9 +++++++++
> > >  1 file changed, 9 insertions(+)
> > >
> > > diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
> > > b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
> > > index d1c878ad2e..6a43c3cafb 100644
> > > --- a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
> > > +++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
> > > @@ -1276,6 +1276,15 @@ PartitionInstallChildHandle (
> > >    } else {
> > >      FreePool (Private->DevicePath);
> > >      FreePool (Private);
> > > +
> > > +    //
> > > +    // if the Status == EFI_ALREADY_STARTED, it means the child handles
> > > +    // are already installed. So return EFI_SUCCESS to avoid do the next
> > > +    // partition type check.
> > > +    //
> > > +    if (Status == EFI_ALREADY_STARTED) {
> > > +      Status = EFI_SUCCESS;
> > > +    }
> > >    }
> > >
> > >    return Status;
> > > --
> > > 2.21.0.windows.1
> > >
> > >
> > > 
> > >
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62652): https://edk2.groups.io/g/devel/message/62652
Mute This Topic: https://groups.io/mt/75490642/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.