Re: [PATCH 5/7] x86/kexec: Implement new EFI load types

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 09/06/2026 4:45 pm, Kevin Lampis wrote:
> From: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
>
> Add new EFI load type for kexec. This load type is suitable for use when 
> Secure
> Boot is enabled.
>
> When this load type is used, the caller should not pass purgatory as one of
> the kexec segments. Instead, Xen will prepare any glue code needed internally.
>
> Signed-off-by: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
> Signed-off-by: Gerald Elder-Vass <gerald.elder-vass@xxxxxxxxx>
> Signed-off-by: Kevin Lampis <klampis@xxxxxxxxxx>

Now that you've dropped regular EFI, the subject should become singular.

> diff --git a/xen/common/kimage.c b/xen/common/kimage.c
> index 1d872916b9..9b49455e8e 100644
> --- a/xen/common/kimage.c
> +++ b/xen/common/kimage.c
> @@ -22,6 +22,7 @@
>  #include <xen/kimage.h>
>  #include <xen/sha2.h>
>  
> +#include <asm/bzimage.h>
>  #include <asm/page.h>

This does not compile for non-x86.  But how to fix it depends on a later
question.


> @@ -1064,6 +1064,63 @@ done:
>      return ret;
>  }
>  
> +/*
> + * Find the entry point to the new kernel, we need to map the crash region 
> into
> + * memory in order to read the kernel header.
> + */
> +#define KERNEL_SEGMENT_IDX 0
> +static uint64_t kimage_find_kernel_entry_maddr(struct kexec_image *image)
> +{
> +    uint64_t alignment_addr;
> +    uint32_t alignment;
> +    unsigned long dest_mfn;
> +    void *dest_va;
> +
> +    alignment_addr = image->segments[KERNEL_SEGMENT_IDX].dest_maddr +
> +                         image->segments[KERNEL_SEGMENT_IDX].dest_offset +
> +                         kernel_alignment_offset();
> +
> +    dest_mfn = alignment_addr >> PAGE_SHIFT;
> +    dest_va = map_domain_page(_mfn(dest_mfn));
> +    if ( !dest_va )
> +        return -EINVAL;
> +
> +    alignment = *((uint32_t *) ((uint8_t *) dest_va +
> +                                                
> PAGE_OFFSET(alignment_addr)));
> +
> +    unmap_domain_page(dest_va);
> +
> +    /*
> +     * Ensure the kernel alignment is a valid LOAD_PHYSICAL_ADDR,
> +     * which ranges from 0x200000 (2MiB) to 0x1000000 (16Mib) on 64-bit 
> systems
> +     * as defined in the kernel x86 Kconfig
> +     */
> +    if ( alignment % 0x200000 != 0 ||
> +         alignment < 0x200000 ||
> +         alignment > 0x1000000 )
> +        return -EINVAL;
> +
> +    return ROUNDUP(image->segments[KERNEL_SEGMENT_IDX].dest_maddr +
> +                       image->segments[KERNEL_SEGMENT_IDX].dest_offset,
> +                   alignment) +
> +                   0x200;

Ok, so here is startup_64, hidden as a magic constant.

The rest doesn't make sense.  You're finding and checking the alignment
field is within the known bounds; fine-ish, but liable to go stale, and
not providing a diagnostic when the check fails.

But then, instead of confirming that that segment[0] has the desired
alignment, you're applying the alignment to the pointer and picking
startup_64 out of thin air.

Either it was loaded correctly (and the alignment is correct), or it was
loaded incorrectly and this is unsafe to do, is it not?

~Andrew