Re: [Xen-devel] [PATCH] xen/x86: fix linker script to work with lld

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 11.07.18 at 11:02, <roger.pau@xxxxxxxxxx> wrote:
> On Wed, Jul 11, 2018 at 02:48:25AM -0600, Jan Beulich wrote:
>> >>> On 11.07.18 at 10:29, <roger.pau@xxxxxxxxxx> wrote:
>> > On Wed, Jul 11, 2018 at 01:09:30AM -0600, Jan Beulich wrote:
>> >> Another possible thing to try might be to make the extern declaration
>> >> of the symbol weak, and drop the offending line altogether.
>> > 
>> > Oh, that didn't occur to me, and does seem to work. Below is what I've
>> > successfully tested:
>> > 
>> > diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
>> > index 5f2392621d..d84704745e 100644
>> > --- a/xen/arch/x86/xen.lds.S
>> > +++ b/xen/arch/x86/xen.lds.S
>> > @@ -297,8 +297,6 @@ SECTIONS
>> >    } :text
>> >  #endif
>> >  
>> > -  efi = DEFINED(efi) ? efi : .;
>> > -
>> >    /* Sections to be discarded */
>> >    /DISCARD/ : {
>> >         *(.exit.text)
>> > diff --git a/xen/include/xen/efi.h b/xen/include/xen/efi.h
>> > index 44b7d3ec3a..8e25bfaebb 100644
>> > --- a/xen/include/xen/efi.h
>> > +++ b/xen/include/xen/efi.h
>> > @@ -21,7 +21,7 @@ struct efi {
>> >      unsigned long smbios3;      /* SMBIOS v3 table */
>> >  };
>> >  
>> > -extern struct efi efi;
>> > +extern struct efi efi __attribute__((weak));
>> >  
>> >  #ifndef __ASSEMBLY__
>> >  
>> > 
>> > If that's acceptable I will refresh and resend the patch.
>> 
>> Let's see what Daniel and maybe others say. If you go that route,
>> could I talk you into adding __weak to compiler.h (I dislike to suggest
>> the leading double underscores, but not using them there would just
>> be too inconsistent with the rest of this header)?
> 
> Hm, the header is kind of mixed on the usage of underscores. inline,
> always_inline or noreturn for example don't have them, so if you
> prefer I think it should be fine to add weak without underscores.

"weak" alone is too generic an identifier to be #define-d in a header
included by basically everything. And I dislike the "attribute_weak"
naming scheme as well, for yielding too long identifiers (i.e. not
providing any real savings over the spelled out __attribute__(())).

> There's an existing case of __attribute__((weak)) in
> livepatch_payload.h which I will replace in the same patch if that's
> fine.

Sure.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel