* [PATCH] UefiCpuPkg: support single EFI_PEI_CORE_FV_LOCATION_PPI in PpiList
@ 2019-09-04 8:56 Chiu, Chasel
2019-09-04 14:39 ` [edk2-devel] " Dong, Eric
2019-09-04 18:03 ` Ni, Ray
0 siblings, 2 replies; 4+ messages in thread
From: Chiu, Chasel @ 2019-09-04 8:56 UTC (permalink / raw)
To: devel; +Cc: Eric Dong, Ray Ni, Laszlo Ersek
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [PATCH] UefiCpuPkg: support single EFI_PEI_CORE_FV_LOCATION_PPI in PpiList
2019-09-04 8:56 [PATCH] UefiCpuPkg: support single EFI_PEI_CORE_FV_LOCATION_PPI in PpiList Chiu, Chasel
@ 2019-09-04 14:39 ` Dong, Eric
2019-09-04 18:03 ` Ni, Ray
1 sibling, 0 replies; 4+ messages in thread
From: Dong, Eric @ 2019-09-04 14:39 UTC (permalink / raw)
To: devel@edk2.groups.io, Chiu, Chasel; +Cc: Ni, Ray, Laszlo Ersek
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
>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [PATCH] UefiCpuPkg: support single EFI_PEI_CORE_FV_LOCATION_PPI in PpiList
2019-09-04 8:56 [PATCH] UefiCpuPkg: support single EFI_PEI_CORE_FV_LOCATION_PPI in PpiList Chiu, Chasel
2019-09-04 14:39 ` [edk2-devel] " Dong, Eric
@ 2019-09-04 18:03 ` Ni, Ray
2019-09-05 0:36 ` Chiu, Chasel
1 sibling, 1 reply; 4+ messages in thread
From: Ni, Ray @ 2019-09-04 18:03 UTC (permalink / raw)
To: devel@edk2.groups.io, Chiu, Chasel; +Cc: Dong, Eric, Laszlo Ersek
> -----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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [PATCH] UefiCpuPkg: support single EFI_PEI_CORE_FV_LOCATION_PPI in PpiList
2019-09-04 18:03 ` Ni, Ray
@ 2019-09-05 0:36 ` Chiu, Chasel
0 siblings, 0 replies; 4+ messages in thread
From: Chiu, Chasel @ 2019-09-05 0:36 UTC (permalink / raw)
To: Ni, Ray, devel@edk2.groups.io; +Cc: Dong, Eric, Laszlo Ersek
> -----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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-09-05 0:37 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-09-04 8:56 [PATCH] UefiCpuPkg: support single EFI_PEI_CORE_FV_LOCATION_PPI in PpiList Chiu, Chasel
2019-09-04 14:39 ` [edk2-devel] " Dong, Eric
2019-09-04 18:03 ` Ni, Ray
2019-09-05 0:36 ` Chiu, Chasel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox