On Fri, 21 Feb 2020 at 17:39, Arvind Sankar <nivedita@xxxxxxxxxxxx> wrote:
>
> On Mon, Feb 17, 2020 at 03:48:21PM +0100, Ard Biesheuvel wrote:
> > Add support for booting 64-bit x86 kernels from 32-bit firmware running
> > on 64-bit capable CPUs without requiring the bootloader to implement
> > the EFI handover protocol or allocate the setup block, etc etc, all of
> > which can be done by the stub itself, using code that already exists.
> >
> > Instead, create an ordinary EFI application entrypoint but implemented
> > in 32-bit code [so that it can be invoked by 32-bit firmware], and stash
> > the address of this 32-bit entrypoint in the .compat section where the
> > bootloader can find it.
> >
> > Note that we use the setup block embedded in the binary to go through
> > startup_32(), but it gets reallocated and copied in efi_pe_entry(),
> > using the same code that runs when the x86 kernel is booted in EFI
> > mode from native firmware. This requires the loaded image protocol to
> > be installed on the kernel image's EFI handle, and point to the kernel
> > image itself and not to its loader. This, in turn, requires the
> > bootloader to use the LoadImage() boot service to load the 64-bit
> > image from 32-bit firmware, which is in fact supported by firmware
> > based on EDK2. (Only StartImage() will fail, and instead, the newly
> > added entrypoint needs to be invoked)
> >
> > Signed-off-by: Ard Biesheuvel <ardb@xxxxxxxxxx>
>
> I think there's one issue with this. startup_32 is 14KiB from the start
> of the image because of .setup. This means the code in startup_32 that
> rounds the load address up to kernel_alignment will likely calculate it
> as 2MiB from the image address (if the image address was 2MiB-aligned),
> and the page tables constructed by the 32-bit code will be beyond the
> space allocated for the image.
>
Right. Image address could be any multiple of 4 KB so we'll have to
deal with that.
> I think the simplest fix would be to increase SizeOfImage by
> kernel_alignment to allow enough slop space for the alignment.
So we basically need at least 2 MB - 14 KB slack at the top, right?
That's easily done.
> We should
> also increase it by text_start, since we need init_size beginning from
> startup_32, not from the image address.
So something like the below?
--- a/arch/x86/boot/tools/build.c
+++ b/arch/x86/boot/tools/build.c
@@ -236,14 +236,23 @@
pe_header = get_unaligned_le32(&buf[0x3c]);
+#ifdef CONFIG_EFI_MIXED
+ /*
+ * In order for startup_32 to safely execute in place, we need to give
+ * it a bit of headroom to create its page tables.
+ */
+ bss_sz += text_start + CONFIG_PHYSICAL_ALIGN;
+ init_sz += text_start + CONFIG_PHYSICAL_ALIGN;
+#endif
+
/*
* Size of code: Subtract the size of the first sector (512 bytes)
* which includes the header.
*/
- put_unaligned_le32(file_sz - 512, &buf[pe_header + 0x1c]);
+ put_unaligned_le32(file_sz + bss_sz- 512, &buf[pe_header + 0x1c]);
/* Size of uninitialized data */
- put_unaligned_le32(bss_sz, &buf[pe_header + 0x24]);
+ put_unaligned_le32(0, &buf[pe_header + 0x24]);
/* Size of image */
put_unaligned_le32(init_sz, &buf[pe_header + 0x50]);
