Re: [PATCH 5/5] efi/arm64: Drop build and linker script hacks to obtain image code size

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] efi/zboot: redundant full-image cache sync in
  efi_zboot_decompress()
It also noticed 1 related pre-existing issue this patch does not
introduce:
- [High] efi/libstub: arm64: missing DSB ISH in efi_cache_sync_image()
  [1]
--

commit 579820756d50227a71aeb397996f2064f5e5696a
Author: Ard Biesheuvel <ardb@kernel.org>

efi/arm64: Drop build and linker script hacks to obtain image code size

This commit replaces the build-time symbol injection and linker script
logic for obtaining the kernel code size in EFI zboot with offsets stored
in struct efi_image_info, and updates efi_cache_sync_image() to use its
alloc_size argument.

> diff --git a/drivers/firmware/efi/libstub/arm64.c b/drivers/firmware/efi/libstub/arm64.c
> index 190cf180bb234..da6fa2f6c8219 100644
> --- a/drivers/firmware/efi/libstub/arm64.c
> +++ b/drivers/firmware/efi/libstub/arm64.c
> @@ -88,8 +88,6 @@ 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)
>  {
> @@ -100,7 +98,7 @@ 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;
> +		unsigned long size = alloc_size;
>  
>  		do {
>  			asm("dc " DCTYPE ", %0" :: "r"(base));

[Severity: Low]
Does changing size from code_size to alloc_size cause efi_zboot_decompress()
to clean the data cache across the entire decompressed image allocation
before efi_zboot_remap_image() syncs the code region a second time?

Both gzip and zstd implementations of efi_zboot_decompress() still call
efi_cache_sync_image((unsigned long)out, outlen) with the full allocation
size (including rodata, data, and BSS), and efi_zboot_entry() then calls
efi_zboot_remap_image() -> efi_cache_sync_image(image_base, code_size)
immediately afterward.

[Severity: High]
This problem wasn't introduced by this patch, but is a dsb(ish) barrier
needed [1] between the data cache clean loop and ic ialluis in
efi_cache_sync_image()?

drivers/firmware/efi/libstub/arm64.c:efi_cache_sync_image() {
	...
	if (!(ctr & BIT(CTR_EL0_IDC_SHIFT))) {
		unsigned long base = image_base;
		unsigned long size = alloc_size;

		do {
			asm("dc " DCTYPE ", %0" :: "r"(base));
			base += lsize;
			size -= lsize;
		} while (size >= lsize);
	}

	asm("ic ialluis");
	dsb(ish);
	isb();
}

Without a barrier after the dc maintenance instructions, ic ialluis can
execute before the data cache cleans complete, allowing the instruction
cache to refetch stale instructions during early boot.

[1] https://sashiko.dev/#/bug/linux-3f5f98cd-053a-49ef-9e97-834ca6744a2c

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106141936.2744481-7-ardb+git@google.com?part=5