Re: [edk2-devel] [PATCH] UefiCpuPkg:Fixed AsmRelocateApLoopStart and ensure allocated memory <4GB

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

On 1/6/23 02:03, Gerd Hoffmann wrote:
> On Fri, Jan 06, 2023 at 07:42:20AM +0100, Laszlo Ersek wrote:
>> On 1/6/23 05:12, Ni, Ray wrote:
>>>
>>> Ard,
>>>
>>> Only AMD X64 (including SEV and without SEV) runs the code that
>>> switches to 32bit paging disabled mode.
>>> Intel X64 runs the code that stays at 64bit paging mode. So no need
>>> for <4G memory.
>>> All IA32 CPUs (including intel and AMD) stays at 32bit paging disabled
>>> mode. The AllocateReservedPages() call should not return a memory
>>> above 4GB in 32bit env.
>>
>> This argument about the allocations sounds valid, thanks.
>>
>> The code still remains incredibly hard to read. It needs serious
>> cleanup.
>>
>> (1) Wherever we have "Amd" in an identifier, let's rename it to "Amd64",
>>      to better reflect the revised check.
> 
> Maybe even better:  Use PcdConfidentialComputingGuestAttr to figure
> whenever SEV is active, if so branch into Amd assembler code.  Rename
> "Amd" to "AmdSev".
> 
> Otherwise just call normal X64 / Ia32 code.
> 
> Amd assembler code can subsequently be simplified, the checks for SEV
> are not needed any more (but should not harm either).
> 
> [ Adding Tom to CC ]

Yes, I agree with all this.

Thanks,
Tom

> 
>>      Commit 73ccde8f6d04 ("UefiCpuPkg: Has APs in 64 bit long-mode before
>>      booting to OS.", 2022-12-20) *removed* the executable marking.
>>
>> (4a) Is that not a problem?
> 
> I think so.
> 
> Booting with strict NX checking (PcdDxeNxMemoryProtectionPolicy =
> 0xC000000000007FD5) and "qemu -smp 2" makes my qemu hang with 200% CPU,
> so probably both vcpus are spinning in a dead loop.  For the BSP this is
> expected behavior (buggy grub.efi, see parallel thread).  For the AP it
> is not, so apparently it is not running idle in hlt like it is supposed
> to.
> 
>> Honestly, at this point I'm *even more convinced* that the original
>> series should be reverted, and redone from the ground up.
> 
> Yes, "back to drawing board" looks like the best option at this point.
> 
> take care,
>    Gerd
>