On 30. May 2023, at 11:06, Marvin Häuser <mhaeuser@posteo.de> wrote:

Hi Ard,

Native PE toolchains *generally* also generate XIP images (/ALIGN = /FILEALIGN, but with 32 Byte rather than 64 Byte alignment compared to GCC49+ / CLANGDWARF) [1]. However, because they are underaligned by default (even for RT images that run in an OS context and MM drivers... sigh...), platforms manually override SectionAlignment, but not necessarily FileAlignment [2], breaking XIP. I don't think what you are doing is perfectly safe for these, as they will have FileAlignment < SectionAlignment (and by all chances, BaseTools is borked too). In my opinion check for FileAlignment == SectionAlignment. I can't vouch for how likely FileAlignment has a sane value, the AUDK loader does not read it at all and instead checks PointerToRawData == VirtualAddress, etc.

BaseTools generally has poor support for non-XIP vs XIP, probably due to notorious underalignment since the very beginning. For PEIM XIPs for example, which must ship pre-relocated at least for Intel, GenFv just relocates the image in-memory and then copies the changes back to the FFS file [3]. There is no concept of changing the image file size within the procedure and as such, a non-XIP image cannot be converted to XIP on demand. This would be useful for a distinction between pre-memory and post-memory PEIMs, the former of which must be XIP (thus aligned), while the latter can be loaded and relocated in-RAM (thus can be underaligned w.r.t. FileAlignment), but alas.

Are there any docs w.r.t. the use-case for this? Since when are DXE XIPs a thing? The code above is opportunistic (i.e., failures are ignored), how does the dispatcher take this into account? Why is this applied during PEI, how this this work with the notion of payloads?

Sorry, I just realized this was a patch *series*! :(
For the questions that are answered in commit messages or implicitly by the code, please disregard, I'll read through them. Not sure the payload situation is obvious, as it's not all that obvious to me how they work in the first place (though I do know, you can both load a payload from edk2 PEI and have edk2 DXE loaded as a payload from something else, so this definitely is a concern in some way).


Thanks!

Best regards,
Marvin

[1] https://github.com/tianocore/edk2/blob/0f9283429dd487deeeb264ee5670551d596fc208/BaseTools/Conf/tools_def.template#L670-L672

[2] https://github.com/tianocore/edk2-platforms/blob/02fb8459d9c48a8ed4691e9c22f2516c15073835/Silicon/Intel/CoffeelakeSiliconPkg/SiPkgBuildOption.dsc#L129

[3] https://github.com/tianocore/edk2/blob/0f9283429dd487deeeb264ee5670551d596fc208/BaseTools/Source/C/GenFv/GenFvInternalLib.c#L3881-L3897