Re: [edk2-devel] [RFC PATCH 08/11] MdeModulePkg/DxeIpl: Relocate and remap XIP capable DXE drivers

["Ard Biesheuvel" <ardb@kernel.org>]

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.