On Sat, 11 Mar 2023 at 16:02, Evgeniy Baskov <baskov@xxxxxxxxx> wrote: > > On 2023-03-10 18:17, Ard Biesheuvel wrote: > > On Thu, 15 Dec 2022 at 13:42, Evgeniy Baskov <baskov@xxxxxxxxx> wrote: > >> > >> Currently kernel image is not fully compliant PE image, so it may > >> fail to boot with stricter implementations of UEFI PE loaders. > >> > >> Set minimal alignments and sizes specified by PE documentation [1] > >> referenced by UEFI specification [2]. Align PE header to 8 bytes. > >> > >> Generate PE sections dynamically. This simplifies code, since with > >> current implementation all of the sections needs to be defined in > >> header.S, where most section header fields do not hold valid values, > >> except for their names. Before the change, it also held flags, > >> but now flags depend on kernel configuration and it is simpler > >> to set them from build.c too. > >> > >> Setup sections protection. Since we cannot fit every needed section, > >> set a part of protection flags dynamically during initialization. > >> This step is omitted if CONFIG_EFI_DXE_MEM_ATTRIBUTES is not set. > >> > >> [1] > >> https://download.microsoft.com/download/9/c/5/9c5b2167-8017-4bae-9fde-d599bac8184a/pecoff_v83.docx > >> [2] > >> https://uefi.org/sites/default/files/resources/UEFI_Spec_2_9_2021_03_18.pdf > >> > >> Tested-by: Peter Jones <pjones@xxxxxxxxxx> > >> Signed-off-by: Evgeniy Baskov <baskov@xxxxxxxxx> > > > > I would prefer it if we didn't rewrite the build tool this way. > > > > Having the sections in header.S in the order they appear in the binary > > is rather useful, and I don't think we should manipulate the section > > flags based on whether CONFIG_DXE_MEM_ATTRIBUTES is set. I also don't > > think we need more than .text / .,data (as discussed in the other > > thread on linux-efi@) > > > > Furthermore, I had a look at the audk PE loader [0], and I think it is > > being overly pedantic. > > > > The PE/COFF spec does not require that all sections are virtually > > contiguous, and it does not require that the file content is > > completely covered by either the header or by a section. > > > > So what I would prefer to do is the following: > > > > Sections: > > Idx Name Size VMA Type > > 0 .reloc 00000200 0000000000002000 DATA > > 1 .compat 00000200 0000000000003000 DATA > > 2 .text 00bee000 0000000000004000 TEXT > > 3 .data 00002200 0000000000bf2000 DATA > > > > using 4k section alignment and 512 byte file alignment, and a header > > size of 0x200 as before (This requires my patch that allows the setup > > header to remain unmapped when running the stub [1]) > > > > The reloc and compat payloads are placed at the end of the setup data > > as before, but increased in size to 512 bytes each, and then mapped > > non-1:1 into the RVA space. > > > > This works happily with both the existing PE loader as well as the > > audk one, but with the pedantic flags disabled. > > > > This makes sense. I'll change this patch to use this layout and > to keep sections in headers.S before sending v5. (and I guess I'll > make the compressed kernel a part of .text). I have a few questions > though: > > This layout assumes having the local copy of the bootparams as > in your RFC patches, right? > Indeed. Otherwise, the setup header may not have been copied to memory by the loader. > Can I keep the .rodata -- 5th section fits in the section table > without much work? > You could, but at least the current PE/COFF loader in EDK2 will map it read/write, as it only distinguishes between executable sections and non-executable sections. > Also, why .reloc is at offset 0x2000 and not just 0x1000, is there > anything important I am missing? I understand that is cannot be 0 > and should be aligned on page size, but nothing else comes to my > mind... > That was just arbitrary, because the raw allocations of reloc and compat are also allocated towards the end. But I guess starting at 0x1000 for .reloc makes more sense so feel free to change that.