Re: [PATCH v2] UefiCpuPkg: support single EFI_PEI_CORE_FV_LOCATION_PPI in PpiList

["Laszlo Ersek" <lersek@redhat.com>]

Hi All,

On 09/05/19 08:46, Ni, Ray wrote:
> Reviewed-by: Ray Ni <ray.ni@intel.com>

Please go ahead with the push, I'll skip this.

Thanks
Laszlo

>> -----Original Message-----
>> From: Dong, Eric
>> Sent: Wednesday, September 4, 2019 11:15 PM
>> To: Chiu, Chasel <chasel.chiu@intel.com>; devel@edk2.groups.io
>> Cc: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>
>> Subject: RE: [PATCH v2] UefiCpuPkg: support single EFI_PEI_CORE_FV_LOCATION_PPI in PpiList
>>
>> Reviewed-by: Eric Dong <eric.dong@intel.com>
>>
>>> -----Original Message-----
>>> From: Chiu, Chasel
>>> Sent: Thursday, September 5, 2019 12:27 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: [PATCH v2] 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 both single EFI_PEI_CORE_FV_LOCATION_PPI and multiple
>>>       PPIs in PpiList cases and both can boot with the PeiCore
>>>       specified by EFI_PEI_CORE_FV_LOCATION_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 | 9 ++++-----
>>>  1 file changed, 4 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/UefiCpuPkg/SecCore/SecMain.c b/UefiCpuPkg/SecCore/SecMain.c
>>> index 66c952b897..5d5e7f17dc 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)
>>>           ) {
>>> @@ -256,12 +255,12 @@ SecStartupPhase2(
>>>            break;
>>>          } else {
>>>            //
>>> -          // PeiCore not found
>>> +          // Invalid PeiCore FV provided by platform
>>>            //
>>>            CpuDeadLoop ();
>>>          }
>>>        }
>>> -    }
>>> +    } 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
>