1. Patchew
  2. EDK2
  3. View series

[edk2-devel] [PATCH] UefiCpuPkg: support single EFI_PEI_CORE_FV_LOCATION_PPI in PpiList

Chiu, Chasel posted 1 patch 4 years, 7 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
UefiCpuPkg/SecCore/SecMain.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
[edk2-devel] [PATCH] UefiCpuPkg: support single EFI_PEI_CORE_FV_LOCATION_PPI in PpiList
Posted by Chiu, Chasel 4 years, 7 months ago 
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2153

Current logic will skip searching EFI_PEI_CORE_FV_LOCATION_PPI when the
PPI in PpiList having EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST flag,
but platform may pass single PPI in PpiList that should be supported.

Changed the logic to verify PpiList first before checking
EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST flag.

Test: Verified single EFI_PEI_CORE_FV_LOCATION_PPI in PpiList and
      still can boot with the PeiCore specified by above PPI.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
---
 UefiCpuPkg/SecCore/SecMain.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/UefiCpuPkg/SecCore/SecMain.c b/UefiCpuPkg/SecCore/SecMain.c
index 66c952b897..6beb025b4b 100644
--- a/UefiCpuPkg/SecCore/SecMain.c
+++ b/UefiCpuPkg/SecCore/SecMain.c
@@ -238,9 +238,8 @@ SecStartupPhase2(
   // is enabled.
   //
   if (PpiList != NULL) {
-    for (Index = 0;
-      (PpiList[Index].Flags & EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST) != EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST;
-      Index++) {
+    Index = 0;
+    do {
       if (CompareGuid (PpiList[Index].Guid, &gEfiPeiCoreFvLocationPpiGuid) &&
           (((EFI_PEI_CORE_FV_LOCATION_PPI *) PpiList[Index].Ppi)->PeiCoreFvLocation != 0)
          ) {
@@ -261,7 +260,8 @@ SecStartupPhase2(
           CpuDeadLoop ();
         }
       }
-    }
+      Index++;
+    } while ((PpiList[Index].Flags & EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST) != EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST);
   }
   //
   // If EFI_PEI_CORE_FV_LOCATION_PPI not found, try to locate PeiCore from BFV.
-- 
2.13.3.windows.1


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

View/Reply Online (#46771): https://edk2.groups.io/g/devel/message/46771
Mute This Topic: https://groups.io/mt/33136208/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] UefiCpuPkg: support single EFI_PEI_CORE_FV_LOCATION_PPI in PpiList
Posted by Dong, Eric 4 years, 7 months ago 
Hi Chasel,

Thanks for your quick fix. 

Reviewed-by: Eric Dong <eric.dong@intel.com>

Thanks,
Eric

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Chiu,
> Chasel
> Sent: Wednesday, September 4, 2019 4:56 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Laszlo
> Ersek <lersek@redhat.com>
> Subject: [edk2-devel] [PATCH] UefiCpuPkg: support single
> EFI_PEI_CORE_FV_LOCATION_PPI in PpiList
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2153
> 
> Current logic will skip searching EFI_PEI_CORE_FV_LOCATION_PPI when the
> PPI in PpiList having EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST flag, but
> platform may pass single PPI in PpiList that should be supported.
> 
> Changed the logic to verify PpiList first before checking
> EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST flag.
> 
> Test: Verified single EFI_PEI_CORE_FV_LOCATION_PPI in PpiList and
>       still can boot with the PeiCore specified by above PPI.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
> ---
>  UefiCpuPkg/SecCore/SecMain.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/UefiCpuPkg/SecCore/SecMain.c b/UefiCpuPkg/SecCore/SecMain.c
> index 66c952b897..6beb025b4b 100644
> --- a/UefiCpuPkg/SecCore/SecMain.c
> +++ b/UefiCpuPkg/SecCore/SecMain.c
> @@ -238,9 +238,8 @@ SecStartupPhase2(
>    // is enabled.
>    //
>    if (PpiList != NULL) {
> -    for (Index = 0;
> -      (PpiList[Index].Flags & EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST) !=
> EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST;
> -      Index++) {
> +    Index = 0;
> +    do {
>        if (CompareGuid (PpiList[Index].Guid, &gEfiPeiCoreFvLocationPpiGuid) &&
>            (((EFI_PEI_CORE_FV_LOCATION_PPI *) PpiList[Index].Ppi)-
> >PeiCoreFvLocation != 0)
>           ) {
> @@ -261,7 +260,8 @@ SecStartupPhase2(
>            CpuDeadLoop ();
>          }
>        }
> -    }
> +      Index++;
> +    } while ((PpiList[Index].Flags &
> + EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST) !=
> + EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST);
>    }
>    //
>    // If EFI_PEI_CORE_FV_LOCATION_PPI not found, try to locate PeiCore from
> BFV.
> --
> 2.13.3.windows.1
> 
> 
> 


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

View/Reply Online (#46816): https://edk2.groups.io/g/devel/message/46816
Mute This Topic: https://groups.io/mt/33136208/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] UefiCpuPkg: support single EFI_PEI_CORE_FV_LOCATION_PPI in PpiList
Posted by Ni, Ray 4 years, 7 months ago 
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chiu, Chasel
> Sent: Wednesday, September 4, 2019 1:56 AM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: [edk2-devel] [PATCH] UefiCpuPkg: support single EFI_PEI_CORE_FV_LOCATION_PPI in PpiList
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2153
> 
> Current logic will skip searching EFI_PEI_CORE_FV_LOCATION_PPI when the
> PPI in PpiList having EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST flag,
> but platform may pass single PPI in PpiList that should be supported.
> 
> Changed the logic to verify PpiList first before checking
> EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST flag.
> 
> Test: Verified single EFI_PEI_CORE_FV_LOCATION_PPI in PpiList and
>       still can boot with the PeiCore specified by above PPI.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
> ---
>  UefiCpuPkg/SecCore/SecMain.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/UefiCpuPkg/SecCore/SecMain.c b/UefiCpuPkg/SecCore/SecMain.c
> index 66c952b897..6beb025b4b 100644
> --- a/UefiCpuPkg/SecCore/SecMain.c
> +++ b/UefiCpuPkg/SecCore/SecMain.c
> @@ -238,9 +238,8 @@ SecStartupPhase2(
>    // is enabled.
>    //
>    if (PpiList != NULL) {
> -    for (Index = 0;
> -      (PpiList[Index].Flags & EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST) != EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST;
> -      Index++) {
> +    Index = 0;
> +    do {
>        if (CompareGuid (PpiList[Index].Guid, &gEfiPeiCoreFvLocationPpiGuid) &&
>            (((EFI_PEI_CORE_FV_LOCATION_PPI *) PpiList[Index].Ppi)->PeiCoreFvLocation != 0)
>           ) {
> @@ -261,7 +260,8 @@ SecStartupPhase2(
>            CpuDeadLoop ();
>          }
>        }
> -    }
> +      Index++;
> +    } while ((PpiList[Index].Flags & EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST) != EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST);

Index is firstly increased, and comparison of the Flags is after that.
What if there is only one PPI in the list, the comparison happens in the random data out of the PPI list.

Thanks,
Ray

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

View/Reply Online (#46824): https://edk2.groups.io/g/devel/message/46824
Mute This Topic: https://groups.io/mt/33136208/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] UefiCpuPkg: support single EFI_PEI_CORE_FV_LOCATION_PPI in PpiList
Posted by Chiu, Chasel 4 years, 7 months ago 

> -----Original Message-----
> From: Ni, Ray
> Sent: Thursday, September 5, 2019 2:04 AM
> To: devel@edk2.groups.io; Chiu, Chasel <chasel.chiu@intel.com>
> Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: RE: [edk2-devel] [PATCH] UefiCpuPkg: support single
> EFI_PEI_CORE_FV_LOCATION_PPI in PpiList
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chiu,
> > Chasel
> > Sent: Wednesday, September 4, 2019 1:56 AM
> > To: devel@edk2.groups.io
> > Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>;
> > Laszlo Ersek <lersek@redhat.com>
> > Subject: [edk2-devel] [PATCH] UefiCpuPkg: support single
> > EFI_PEI_CORE_FV_LOCATION_PPI in PpiList
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2153
> >
> > Current logic will skip searching EFI_PEI_CORE_FV_LOCATION_PPI when
> > the PPI in PpiList having EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST flag,
> > but platform may pass single PPI in PpiList that should be supported.
> >
> > Changed the logic to verify PpiList first before checking
> > EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST flag.
> >
> > Test: Verified single EFI_PEI_CORE_FV_LOCATION_PPI in PpiList and
> >       still can boot with the PeiCore specified by above PPI.
> >
> > Cc: Eric Dong <eric.dong@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
> > ---
> >  UefiCpuPkg/SecCore/SecMain.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/UefiCpuPkg/SecCore/SecMain.c
> > b/UefiCpuPkg/SecCore/SecMain.c index 66c952b897..6beb025b4b 100644
> > --- a/UefiCpuPkg/SecCore/SecMain.c
> > +++ b/UefiCpuPkg/SecCore/SecMain.c
> > @@ -238,9 +238,8 @@ SecStartupPhase2(
> >    // is enabled.
> >    //
> >    if (PpiList != NULL) {
> > -    for (Index = 0;
> > -      (PpiList[Index].Flags &
> EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST) !=
> EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST;
> > -      Index++) {
> > +    Index = 0;
> > +    do {
> >        if (CompareGuid (PpiList[Index].Guid,
> &gEfiPeiCoreFvLocationPpiGuid) &&
> >            (((EFI_PEI_CORE_FV_LOCATION_PPI *)
> PpiList[Index].Ppi)->PeiCoreFvLocation != 0)
> >           ) {
> > @@ -261,7 +260,8 @@ SecStartupPhase2(
> >            CpuDeadLoop ();
> >          }
> >        }
> > -    }
> > +      Index++;
> > +    } while ((PpiList[Index].Flags &
> > + EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST) !=
> > + EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST);
> 
> Index is firstly increased, and comparison of the Flags is after that.
> What if there is only one PPI in the list, the comparison happens in the
> random data out of the PPI list.

Good catch! Let me update and re-test it again. Thanks!

> 
> Thanks,
> Ray

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

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

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.