On Tue, 3 Mar 2020 at 23:12, Arvind Sankar <nivedita@xxxxxxxxxxxx> wrote:
>
> Add alignment slack to the PE image size, so that we can realign the
> decompression buffer within the space allocated for the image.
>
> Only relocate the kernel if it has been loaded at an unsuitable address:
> * Below LOAD_PHYSICAL_ADDR, or
> * Above 64T for 64-bit and 512MiB for 32-bit
>
> For 32-bit, the upper limit is conservative, but the exact limit can be
> difficult to calculate.
>
Could we get rid of the call to efi_low_alloc_above() in
efi_relocate_kernel(), and just allocate top down with the right
alignment? I'd like to get rid of efi_low_alloc() et al if we can.
> Signed-off-by: Arvind Sankar <nivedita@xxxxxxxxxxxx>
> ---
> arch/x86/boot/tools/build.c | 16 +++++-------
> drivers/firmware/efi/libstub/x86-stub.c | 33 ++++++++++++++++++++++---
> 2 files changed, 36 insertions(+), 13 deletions(-)
>
> diff --git a/arch/x86/boot/tools/build.c b/arch/x86/boot/tools/build.c
> index 3d03ad753ed5..db528961c283 100644
> --- a/arch/x86/boot/tools/build.c
> +++ b/arch/x86/boot/tools/build.c
> @@ -238,21 +238,17 @@ static void update_pecoff_text(unsigned int text_start, unsigned int file_sz,
>
> pe_header = get_unaligned_le32(&buf[0x3c]);
>
> -#ifdef CONFIG_EFI_MIXED
> /*
> - * In mixed mode, we will execute startup_32() at whichever offset in
> - * memory it happened to land when the PE/COFF loader loaded the image,
> - * which may be misaligned with respect to the kernel_alignment field
> - * in the setup header.
> + * The PE/COFF loader may load the image at an address which is
> + * misaligned with respect to the kernel_alignment field in the setup
> + * header.
> *
> - * In order for startup_32 to safely execute in place at this offset,
> - * we need to ensure that the CONFIG_PHYSICAL_ALIGN aligned allocation
> - * it creates for the page tables does not extend beyond the declared
> - * size of the image in the PE/COFF header. So add the required slack.
> + * In order to avoid relocating the kernel to correct the misalignment,
> + * add slack to allow the buffer to be aligned within the declared size
> + * of the image.
> */
> bss_sz += CONFIG_PHYSICAL_ALIGN;
> init_sz += CONFIG_PHYSICAL_ALIGN;
> -#endif
>
> /*
> * Size of code: Subtract the size of the first sector (512 bytes)
> diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
> index e71b8421e088..fbc4354f534c 100644
> --- a/drivers/firmware/efi/libstub/x86-stub.c
> +++ b/drivers/firmware/efi/libstub/x86-stub.c
> @@ -17,6 +17,9 @@
>
> #include "efistub.h"
>
> +/* Maximum physical address for 64-bit kernel with 4-level paging */
> +#define MAXMEM_X86_64_4LEVEL (1ull << 46)
> +
> static efi_system_table_t *sys_table;
> extern const bool efi_is64;
> extern u32 image_offset;
> @@ -717,6 +720,7 @@ unsigned long efi_main(efi_handle_t handle,
> struct boot_params *boot_params)
> {
> unsigned long bzimage_addr = (unsigned long)startup_32;
> + unsigned long buffer_start, buffer_end;
> struct setup_header *hdr = &boot_params->hdr;
> efi_status_t status;
> unsigned long cmdline_paddr;
> @@ -728,10 +732,33 @@ unsigned long efi_main(efi_handle_t handle,
> efi_exit(handle, EFI_INVALID_PARAMETER);
>
> /*
> - * If the kernel isn't already loaded at the preferred load
> - * address, relocate it.
> + * If the kernel isn't already loaded at a suitable address,
> + * relocate it.
> + *
> + * It must be loaded above LOAD_PHYSICAL_ADDR.
> + *
> + * The maximum address for 64-bit is 1 << 46 for 4-level paging. This
> + * is defined as the macro MAXMEM, but unfortunately that is not a
> + * compile-time constant if 5-level paging is configured, so we instead
> + * define our own macro for use here.
> + *
> + * For 32-bit, the maximum address is complicated to figure out, for
> + * now use KERNEL_IMAGE_SIZE, which will be 512MiB, the same as what
> + * KASLR uses.
> + *
> + * Also relocate it if image_offset is zero, i.e. we weren't loaded by
> + * LoadImage, but we are not aligned correctly.
> */
> - if (bzimage_addr - image_offset != hdr->pref_address) {
> +
> + buffer_start = ALIGN(bzimage_addr - image_offset,
> + hdr->kernel_alignment);
> + buffer_end = buffer_start + hdr->init_size;
> +
> + if ((buffer_start < LOAD_PHYSICAL_ADDR) ||
> + (IS_ENABLED(CONFIG_X86_32) && buffer_end > KERNEL_IMAGE_SIZE) ||
> + (IS_ENABLED(CONFIG_X86_64) && buffer_end > MAXMEM_X86_64_4LEVEL) ||
> + (image_offset == 0 && !IS_ALIGNED(bzimage_addr,
> + hdr->kernel_alignment))) {
> status = efi_relocate_kernel(&bzimage_addr,
> hdr->init_size, hdr->init_size,
> hdr->pref_address,
> --
> 2.24.1
>
