On 30. May 2023, at 11:48, Ard Biesheuvel <ardb@kernel.org> wrote:

On Tue, 30 May 2023 at 11:47, Ard Biesheuvel <ardb@kernel.org> wrote:

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


On 30. May 2023, at 11:38, Ard Biesheuvel <ardb@kernel.org> wrote:

On Tue, 30 May 2023 at 11:07, 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.


If XIP for PE images with 4k section alignment is an issue, we could
always explore loading them into a separate allocation from PEI, just
like we do with DXE core itself.

This would actually simplify the loader code quite a lot, as we'd be
able to use the PEI core image loader directly. However, it means we'd
have to pass this information (array of <guid, base address> tuples
describing which images were already loaded by DxeIpl) via a HOB or
some other method.

I took a *very brief* look at the entire series now. Is this just to apply permissions before CpuDxe is loaded

Yes.


Well, actually, the first part of the series gets rid of some
pointless memory copies, which is an improvement in itself.

Sorry, I didn't mean the series, I meant the choice to handle things in DxeIpl over DxeCore in particular.

Is there even a non-FOSS producer of the CpuArch protocol? To my understanding, all (common) Intel platforms use the one in UefiCpuPkg. ARM and friends feel a lot less proprietary to begin with, but I could be wrong. We were quite successful so far with just merging CpuArch into DxeCore and setting it up quite early, but of course not at an industry-wide scale. :)