On 05/09/2024 4:35 pm, Jan Beulich wrote:
> On 05.09.2024 15:06, Andrew Cooper wrote:
>> asm/config.h is included in every translation unit (via xen/config.h), while
>> only a handful of functions actually interact with the trampoline.
>>
>> Move the infrastructure into its own header, and take the opportunity to
>> document everything.
>>
>> Also change trampoline_realmode_entry() and wakeup_start() to be nocall
>> functions, rather than char arrays.
>>
>> No functional change.
>>
>> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
> Reviewed-by: Jan Beulich <jbeul...@suse.com>
Thanks.
>
>> --- /dev/null
>> +++ b/xen/arch/x86/include/asm/trampoline.h
>> @@ -0,0 +1,98 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +#ifndef XEN_ASM_TRAMPOLINE_H
>> +#define XEN_ASM_TRAMPOLINE_H
> Not exactly usual a guard name, but once the new naming scheme is finalized
> most will need renaming anyway.
What would you prefer?
>
>> +/*
>> + * Calculate the physical address of a symbol in the trampoline.
>> + *
>> + * Should only be used on symbols declared later in this header. Specifying
>> + * other symbols will compile but malfunction when used, as will using this
>> + * helper before the trampoline is placed.
>> + */
> As to the last point made - we could of course overcome this by setting
> trampoline_phys to a suitable value initially, and only to its low-mem
> value once the trampoline was moved there.
Yes, but then we've got yet another variable needing to stay in sync
with xen_phys_start (for a while at least).
> As to compiling but malfunctioning - I'd kind of expect relocation
> overflows to be flagged by the linker.
What I meant by this is that things like bootsym(opt_cpu_info) will
compile but may #PF when used or corrupt data if not.
I couldn't think of any good way to bounds check sym between
trampoline_{start,end}[] at build time.
Doing it at runtime is possible, but some usage sites aren't
printk/panic friendly.
~Andrew
