On 18/03/2025 5:35 pm, Roger Pau Monne wrote:
> diff --git a/xen/arch/x86/boot/reloc-trampoline.c
> b/xen/arch/x86/boot/reloc-trampoline.c
> index e35e7c78aa86..ac54aef14eaf 100644
> --- a/xen/arch/x86/boot/reloc-trampoline.c
> +++ b/xen/arch/x86/boot/reloc-trampoline.c
> @@ -20,19 +20,19 @@ void reloc_trampoline64(void)
> uint32_t phys = trampoline_phys;
> const int32_t *trampoline_ptr;
>
> - /*
> - * Apply relocations to trampoline.
> - *
> - * This modifies the trampoline in place within Xen, so that it will
> - * operate correctly when copied into place.
> - */
> + /* Apply relocations to trampoline after copy to destination. */
I think this needs expanding on a bit.
The relocations in __trampoline_*_{start,stop} relate to the trampoline
as it lives compiled into Xen, but we're applying them to the trampoline
already copied into low memory.
> +#define RELA_TARGET(ptr, bits) \
> + *(uint ## bits ## _t *)(phys + *ptr + (long)ptr - (long)trampoline_start)
> +
> for ( trampoline_ptr = __trampoline_rel_start;
> trampoline_ptr < __trampoline_rel_stop;
> ++trampoline_ptr )
> - *(uint32_t *)(*trampoline_ptr + (long)trampoline_ptr) += phys;
> + RELA_TARGET(trampoline_ptr, 32) += phys;
>
> for ( trampoline_ptr = __trampoline_seg_start;
> trampoline_ptr < __trampoline_seg_stop;
> ++trampoline_ptr )
> - *(uint16_t *)(*trampoline_ptr + (long)trampoline_ptr) = phys >> 4;
> + RELA_TARGET(trampoline_ptr, 16) = phys >> 4;
> +
> +#undef RELA_TARGET
I have a patch renaming trampoline_ptr to just ptr, on the grounds of
verbosity. I'm not sure if it want's to go in ahead, merged with, or
after this patch.
Also, encoding bits in RELA_TARGET() isn't terribly nice. What's wrong
with keeping the casts as-are, and having RELA_TARGET() only taking ptr?
~Andrew
