Hi Marvin,

Thanks for the feedbacks.

The corrected implementation is in patch 5.

The stacks are located high and the function is located low. With padding, it ensures page alignment.

I'll resend the patch to make sure the changes are included in patch 2.

Best Regards,

Yuanhao

From: Marvin Häuser <mhaeuser@posteo.de>
Sent: Wednesday, February 8, 2023 12:41 AM
To: Xie, Yuanhao <yuanhao.xie@intel.com>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH 2/5] UefiCpuPkg: Contiguous memory allocation and code clean-up.

Hi Yuanhao,

1) The code comments and copy code suggest that the stacks are located low and the function is located high (good). However, the SetMemorySpaceAttributes() call un-XP's Address, which is the low address. So, do I misunderstand the changes, or are you un-XP'ing the first stack (and keep the function XP'd)?

2) The same SetMemorySpaceAttributes() call, you now pass ApLoopFuncSize over ApSafeBufferSize. The latter was explicitly page-aligned, while the former is not. How is it guaranteed it is indeed aligned? If it is not, I don't think this is supported, at least universally.

3) Similar to 2), the stack size is much smaller than the page size, no? How do you guarantee the function is on a page boundary for memory protection?

4) A proper W^X flow should be to wait with un-XP till the CopyMem() for the function code has returned. Right before that, the copied code should be marked read-only.

Best regards,
Marvin