On Fri, 21 Feb 2020 at 18:59, Arvind Sankar <nivedita@xxxxxxxxxxxx> wrote: > > 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. > Yeah, so this could already be an issue today ...