On Wed, Apr 26, 2023 at 03:20:43PM +0100, Ard Biesheuvel wrote:
> On Wed, 26 Apr 2023 at 15:11, Ard Biesheuvel <ardb@xxxxxxxxxx> wrote:
> >
> > Instead of relying on a dodgy dd hack to copy the image code size from
> > the uncompressed image's PE header to the end of the compressed image,
>
>
> > let's grab the code size from the text_offset field of the arm64 image
> > header after decompression, which is where the arm64 specific EFI zboot
> > make rules will poke the code size when generating the zboot specific
> > version of the binary Image payload.
> >
>
> Apologies, I failed to update the above when I changed the actual
> patch. It should read:
>
> "let's grab the code size from the symbol that is injected into the
> ELF by the Kbuild rules that generate the compressed payload".
With the corrected wording:
Acked-by: Mark Rutland <mark.rutland@xxxxxxx>
Mark.
>
>
> > Signed-off-by: Ard Biesheuvel <ardb@xxxxxxxxxx>
> > ---
> > drivers/firmware/efi/libstub/Makefile.zboot | 14 +++-----------
> > drivers/firmware/efi/libstub/arm64.c | 19 +++++++++++++------
> > drivers/firmware/efi/libstub/efistub.h | 3 +--
> > drivers/firmware/efi/libstub/zboot.c | 15 ++++-----------
> > 4 files changed, 21 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/firmware/efi/libstub/Makefile.zboot b/drivers/firmware/efi/libstub/Makefile.zboot
> > index 1b101d9076fd49e5..89ef820f3b34483a 100644
> > --- a/drivers/firmware/efi/libstub/Makefile.zboot
> > +++ b/drivers/firmware/efi/libstub/Makefile.zboot
> > @@ -24,21 +24,13 @@ comp-type-$(CONFIG_KERNEL_ZSTD) := zstd22
> > # causing the original tools to complain when checking image integrity.
> > # So disregard it when calculating the payload size in the zimage header.
> > zboot-method-y := $(comp-type-y)_with_size
> > -zboot-size-len-y := 12
> > +zboot-size-len-y := 4
> >
> > zboot-method-$(CONFIG_KERNEL_GZIP) := gzip
> > -zboot-size-len-$(CONFIG_KERNEL_GZIP) := 8
> > -
> > -# Copy the SizeOfHeaders and SizeOfCode fields from the payload to the end of
> > -# the compressed image. Note that this presupposes a PE header offset of 64
> > -# bytes, which is what arm64, RISC-V and LoongArch use.
> > -quiet_cmd_compwithsize = $(quiet_cmd_$(zboot-method-y))
> > - cmd_compwithsize = $(cmd_$(zboot-method-y)) && ( \
> > - dd status=none if=$< bs=4 count=1 skip=37 ; \
> > - dd status=none if=$< bs=4 count=1 skip=23 ) >> $@
> > +zboot-size-len-$(CONFIG_KERNEL_GZIP) := 0
> >
> > $(obj)/vmlinuz: $(obj)/vmlinux.bin FORCE
> > - $(call if_changed,compwithsize)
> > + $(call if_changed,$(zboot-method-y))
> >
> > OBJCOPYFLAGS_vmlinuz.o := -I binary -O $(EFI_ZBOOT_BFD_TARGET) $(EFI_ZBOOT_OBJCOPY_FLAGS) \
> > --rename-section .data=.gzdata,load,alloc,readonly,contents
> > diff --git a/drivers/firmware/efi/libstub/arm64.c b/drivers/firmware/efi/libstub/arm64.c
> > index 8aad8c49d43f18e0..446e35eaf3d9434c 100644
> > --- a/drivers/firmware/efi/libstub/arm64.c
> > +++ b/drivers/firmware/efi/libstub/arm64.c
> > @@ -9,6 +9,7 @@
> >
> > #include <linux/efi.h>
> > #include <asm/efi.h>
> > +#include <asm/image.h>
> > #include <asm/memory.h>
> > #include <asm/sysreg.h>
> >
> > @@ -88,9 +89,10 @@ efi_status_t check_platform_features(void)
> > #define DCTYPE "cvau"
> > #endif
> >
> > +u32 __weak code_size;
> > +
> > void efi_cache_sync_image(unsigned long image_base,
> > - unsigned long alloc_size,
> > - unsigned long code_size)
> > + unsigned long alloc_size)
> > {
> > u32 ctr = read_cpuid_effective_cachetype();
> > u64 lsize = 4 << cpuid_feature_extract_unsigned_field(ctr,
> > @@ -98,16 +100,21 @@ void efi_cache_sync_image(unsigned long image_base,
> >
> > /* only perform the cache maintenance if needed for I/D coherency */
> > if (!(ctr & BIT(CTR_EL0_IDC_SHIFT))) {
> > + unsigned long base = image_base;
> > + unsigned long size = code_size;
> > +
> > do {
> > - asm("dc " DCTYPE ", %0" :: "r"(image_base));
> > - image_base += lsize;
> > - code_size -= lsize;
> > - } while (code_size >= lsize);
> > + asm("dc " DCTYPE ", %0" :: "r"(base));
> > + base += lsize;
> > + size -= lsize;
> > + } while (size >= lsize);
> > }
> >
> > asm("ic ialluis");
> > dsb(ish);
> > isb();
> > +
> > + efi_remap_image(image_base, alloc_size, code_size);
> > }
> >
> > unsigned long __weak primary_entry_offset(void)
> > diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
> > index 148013bcb5f89fdd..67d5a20802e0b7c6 100644
> > --- a/drivers/firmware/efi/libstub/efistub.h
> > +++ b/drivers/firmware/efi/libstub/efistub.h
> > @@ -1066,8 +1066,7 @@ struct screen_info *__alloc_screen_info(void);
> > void free_screen_info(struct screen_info *si);
> >
> > void efi_cache_sync_image(unsigned long image_base,
> > - unsigned long alloc_size,
> > - unsigned long code_size);
> > + unsigned long alloc_size);
> >
> > struct efi_smbios_record {
> > u8 type;
> > diff --git a/drivers/firmware/efi/libstub/zboot.c b/drivers/firmware/efi/libstub/zboot.c
> > index 63ece480090032c1..e5d7fa1f1d8fd160 100644
> > --- a/drivers/firmware/efi/libstub/zboot.c
> > +++ b/drivers/firmware/efi/libstub/zboot.c
> > @@ -50,8 +50,7 @@ static unsigned long alloc_preferred_address(unsigned long alloc_size)
> > }
> >
> > void __weak efi_cache_sync_image(unsigned long image_base,
> > - unsigned long alloc_size,
> > - unsigned long code_size)
> > + unsigned long alloc_size)
> > {
> > // Provided by the arch to perform the cache maintenance necessary for
> > // executable code loaded into memory to be safe for execution.
> > @@ -66,7 +65,7 @@ asmlinkage efi_status_t __efiapi
> > efi_zboot_entry(efi_handle_t handle, efi_system_table_t *systab)
> > {
> > unsigned long compressed_size = _gzdata_end - _gzdata_start;
> > - unsigned long image_base, alloc_size, code_size;
> > + unsigned long image_base, alloc_size;
> > efi_loaded_image_t *image;
> > efi_status_t status;
> > char *cmdline_ptr;
> > @@ -91,13 +90,9 @@ efi_zboot_entry(efi_handle_t handle, efi_system_table_t *systab)
> > efi_info("Decompressing Linux Kernel...\n");
> >
> > // SizeOfImage from the compressee's PE/COFF header
> > - alloc_size = round_up(get_unaligned_le32(_gzdata_end - 12),
> > + alloc_size = round_up(get_unaligned_le32(_gzdata_end - 4),
> > EFI_ALLOC_ALIGN);
> >
> > - // SizeOfHeaders and SizeOfCode from the compressee's PE/COFF header
> > - code_size = get_unaligned_le32(_gzdata_end - 4) +
> > - get_unaligned_le32(_gzdata_end - 8);
> > -
> > // If the architecture has a preferred address for the image,
> > // try that first.
> > image_base = alloc_preferred_address(alloc_size);
> > @@ -140,9 +135,7 @@ efi_zboot_entry(efi_handle_t handle, efi_system_table_t *systab)
> > goto free_image;
> > }
> >
> > - efi_cache_sync_image(image_base, alloc_size, code_size);
> > -
> > - efi_remap_image(image_base, alloc_size, code_size);
> > + efi_cache_sync_image(image_base, alloc_size);
> >
> > status = efi_stub_common(handle, image, image_base, cmdline_ptr);
> >
> > --
> > 2.39.2
> >
