On 05/09/2024 5:34 pm, Frediano Ziglio wrote: > On Thu, Sep 5, 2024 at 5:10 PM Andrew Cooper > <andrew.coop...@citrix.com> wrote: > > On 05/09/2024 4:42 pm, Jan Beulich wrote: > > On 05.09.2024 15:06, Andrew Cooper wrote: > >> --- a/xen/arch/x86/efi/efi-boot.h > >> +++ b/xen/arch/x86/efi/efi-boot.h > >> @@ -102,9 +102,6 @@ static void __init > efi_arch_relocate_image(unsigned long delta) > >> } > >> } > >> > >> -extern const s32 __trampoline_rel_start[], > __trampoline_rel_stop[]; > >> -extern const s32 __trampoline_seg_start[], > __trampoline_seg_stop[]; > > I'd prefer if these stayed here, leaving the 4 symbols as > minimally exposed as > > possible. Recall that efi-boot.h isn't really a header in that > sense, but > > rather a .c file. Elsewhere we keep decls in .c files when > they're used in just > > one CU. > > See Frediano's RFC series, which needs to change this in order to move > the 32bit relocation logic from asm to C. > > Not strictly necessary, I can declare in the C file as they were > declared in efi-boot.h (which is more a .c file as an header as Jan said). > I think the idea of declaring into a source file is that if another > file wants to use it has to declare it again, so a bit more friction. > But any access to trampoline variables should be considered as > something to limit in any case, so having in a separate header helps > (this does not mean that removing from the header is still increasing > the friction). > > Personally, I'm not strong about the 2 options here. Slightly prefer > having all variable declared in a single header.
Declaring the same symbol in multiple places is a hard MISRA failure now. Hence why I was trying to clean this up to simplify your patches. ~Andrew
