Re: [PATCH 1/1] UefiCpuPkg/MpInitLib: Reduce reset vector memory pressure

["Lendacky, Thomas" <thomas.lendacky@amd.com>]

On 9/23/20 8:58 AM, Tom Lendacky wrote:
> On 9/23/20 3:31 AM, Laszlo Ersek wrote:
>> On 09/23/20 10:14, Laszlo Ersek wrote:
>>
>>> (3) Even better... can you modify GetApResetVectorSize() to take
>>> &CpuMpData rather than &CpuMpData->AddressMap, and then check
>>> CpuMpData->SevEsIsEnabled?
>>>
>>> Hmmm, wait, that's not really simple, as we call GetApResetVectorSize()
>>> from MpInitLibInitialize() too, way before we set
>>> CpuMpData->SevEsIsEnabled from the PCD.
>>>
>>> So I guess we should pass a dedicated BOOLEAN parameter to
>>> GetApResetVectorSize(), called "SevEsIsEnabled". At the call site in
>>> MpInitLibInitialize(), we should pass in the PCD's value. At the call
>>> site in AllocateResetVector(), we should pass in
>>> CpuMpData->SevEsIsEnabled.
>>>
>>> The reason I'm suggesting (3) is that I don't feel comfortable with
>>> checking dynamic PCDs outside of entry point functions / initialization
>>> functions.
>>
>> You know what, never mind (3) -- I've just realized that
>> PcdCpuMaxLogicalProcessorNumber may be a dynamic PCD too. It might
>> require a lot of work to restrict all dynamic PCD accesses to the init
>> function only, and I couldn't necessarily justify all that work at the
>> moment (for myself or for anyone else).
>>
>> So please consider (1), (2) and (4).
> 
> Yup, will do.

Re. #4, the code uses the APIC ID to calculate the stack start, so the
calculation, now in GetApResetVectorSize(), uses the full value of
PcdGet32(PcdCpuMaxLogicalProcessorNumber) instead of subtracting one from
it. I can always update MpInitLibSevEsAPReet() to subtract one from the
APIC ID value returned if we want to eliminate the extra 64 bytes.

Thanks,
Tom

> 
> Thanks,
> Tom
> 
>>
>> Thanks!
>> Laszlo
>>