Re: [PATCH v2 2/2] xen: Populate xen.lds.h and make use of its macros

[Julien Grall <julien@xxxxxxx>]

Hi Jan,

On 29/03/2022 12:42, Jan Beulich wrote:
> On 29.03.2022 12:54, Julien Grall wrote:
> > On 29/03/2022 11:12, Michal Orzel wrote:
> > > On 29.03.2022 11:54, Julien Grall wrote:
> > > > On 22/03/2022 08:02, Michal Orzel wrote:
> > > > > --- a/xen/include/xen/xen.lds.h
> > > > > +++ b/xen/include/xen/xen.lds.h
> > > > > @@ -5,4 +5,104 @@
> > > > >      * Common macros to be used in architecture specific linker scripts.
> > > > >      */
> > > > >     +/* Macros to declare debug sections. */
> > > > > +#ifdef EFI
> > > > AFAIK, we don't define EFI on Arm (just CONFIG_EFI). Yet we do support EFI on 
> > > > arm64.
> > > >
> > > > As this #ifdef is now in generic code, can you explain how this is meant to be 
> > > > used?
> > > As we do not define EFI on arm, all the stuff protected by #ifdef EFI is x86 
> > > specific.
> > I find the name "EFI" too generic to figure out that this code can only
> > be used by x86.
> >
> > But, from my understanding, this header is meant to contain generic
> > code. It feels a bit odd that we are moving arch specific code.
> >
> > To be honest, I don't quite understand why we need to make the
> > diffferentiation on x86. So I guess the first question is how this is
> > meant to be used on x86?
> We produce two linker scripts from the single source file: One (with EFI
> undefined) to link the ELF binary, and another (with EFI defined) to link
> the PE/COFF output. If "EFI" is too imprecise as a name for the identifier,
> I wouldn't mind renaming it (to PE_COFF?), but at the same time I'm not
> convinced this is really necessary.
Thank for the explanation (and the other ones in this thread). You are 
right the confusion arised from "generating" vs "linking".
Renaming to PE_COFF may help to avoid the confusion with CONFIG_EFI. 
That said, it would possibly make more difficult to associate the flag 
with "linking an EFI binary".
I think some documentaion about the define EFI would be help so there 
are no more confusion between CONFIG_EFI/EFI. But I am not sure where to 
put it. Maybe at the top of the header?
Cheers,

--
Julien Grall