On Fri, Feb 21, 2020 at 06:12:40PM +0100, Ard Biesheuvel wrote: > 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? Yup. You might as well do the text_start bit unconditionally I think? If by some blind stroke of luck startup_32 ends up at pref_address and so we don't call efi_relocate_kernel, we'll need the room. > > --- 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]);
