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

2018-07-11 Thread Daniel Kiper
On Wed, Jul 11, 2018 at 11:36:40AM +0200, Roger Pau Monné wrote:
> On Wed, Jul 11, 2018 at 11:11:05AM +0200, Daniel Kiper wrote:
> > On Wed, Jul 11, 2018 at 10:59:20AM +0200, Roger Pau Monné wrote:
> > > On Wed, Jul 11, 2018 at 10:55:56AM +0200, Daniel Kiper wrote:
> > > > If this works in all cases (build with tools with and without PE 
> > > > support,
> > > > xen.efi and xen.gz via Multiboot2 proto boot correctly on EFI platforms,
> > > > etc.) then I am OK with that.
> > >
> > > Yes, I've tested it with both linkers (PE capable and incapable) and
> > > it works.
> >
> > Great! What about boot on EFI platforms? Probably it works but I would
> > like to be sure.
>
> I'm afraid I don't have any ATM. If I post the complete patch, could
> you give it a try on a EFI platform?

No problem, will do.

Daniel

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

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

2018-07-11 Thread Jan Beulich
>>> On 11.07.18 at 11:02,  wrote:
> On Wed, Jul 11, 2018 at 02:48:25AM -0600, Jan Beulich wrote:
>> >>> On 11.07.18 at 10:29,  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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

2018-07-11 Thread Daniel Kiper
On Wed, Jul 11, 2018 at 10:59:20AM +0200, Roger Pau Monné wrote:
> On Wed, Jul 11, 2018 at 10:55:56AM +0200, Daniel Kiper wrote:
> > On Wed, Jul 11, 2018 at 02:48:25AM -0600, Jan Beulich wrote:
> > > >>> On 11.07.18 at 10:29,  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)?
> >
> > If this works in all cases (build with tools with and without PE support,
> > xen.efi and xen.gz via Multiboot2 proto boot correctly on EFI platforms,
> > etc.) then I am OK with that.
>
> Yes, I've tested it with both linkers (PE capable and incapable) and
> it works.

Great! What about boot on EFI platforms? Probably it works but I would
like to be sure.

Daniel

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

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

2018-07-11 Thread Roger Pau Monné
On Wed, Jul 11, 2018 at 02:48:25AM -0600, Jan Beulich wrote:
> >>> On 11.07.18 at 10:29,  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.

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

Thanks, Roger.

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

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

2018-07-11 Thread Roger Pau Monné
On Wed, Jul 11, 2018 at 10:55:56AM +0200, Daniel Kiper wrote:
> On Wed, Jul 11, 2018 at 02:48:25AM -0600, Jan Beulich wrote:
> > >>> On 11.07.18 at 10:29,  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)?
> 
> If this works in all cases (build with tools with and without PE support,
> xen.efi and xen.gz via Multiboot2 proto boot correctly on EFI platforms,
> etc.) then I am OK with that.

Yes, I've tested it with both linkers (PE capable and incapable) and
it works.

Thanks, Roger.

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

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

2018-07-11 Thread Daniel Kiper
On Wed, Jul 11, 2018 at 02:48:25AM -0600, Jan Beulich wrote:
> >>> On 11.07.18 at 10:29,  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)?

If this works in all cases (build with tools with and without PE support,
xen.efi and xen.gz via Multiboot2 proto boot correctly on EFI platforms,
etc.) then I am OK with that.

Daniel

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

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

2018-07-11 Thread Jan Beulich
>>> On 11.07.18 at 10:29,  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)?

Jan



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

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

2018-07-11 Thread Roger Pau Monné
On Wed, Jul 11, 2018 at 01:09:30AM -0600, Jan Beulich wrote:
> >>> On 10.07.18 at 17:35,  wrote:
> > On Tue, Jul 10, 2018 at 08:09:06AM -0600, Jan Beulich wrote:
> >> >>> On 10.07.18 at 15:49,  wrote:
> >> > On Tue, Jul 10, 2018 at 05:47:19AM -0600, Jan Beulich wrote:
> >> >> >>> On 10.07.18 at 13:00,  wrote:
> >> >> > On Mon, Jul 09, 2018 at 06:45:16PM +0200, Roger Pau Monné wrote:
> >> >> >> Sorry for asking so many questions, but I would like to try to avoid
> >> >> >> the DEFINED conditional in the linker script if possible.
> >> >> > 
> >> >> > If you wish to do that then put dummy efi symbol into 
> >> >> > xen/arch/x86/efi/stub.c.
> >> >> 
> >> >> I think I've clearly objected to that before - I simply see no need for
> >> >> wasting the space in an EFI-incapable binary.
> >> > 
> >> > But such waste is also added regardless of whether you define the
> >> > symbol it in the linker script or in a C file?
> >> 
> >> How that? There's no space associated with the symbol when defined
> >> this way. From the perspective of the binary, the space lives past the
> >> end of the image, but since the symbol is never accessed (or else code
> >> is broken somewhere) in the non-EFI case, this is okay.
> > 
> > Oh right, that's not inside of any section.
> > 
> > I've tried placing an efi symbol inside of the .discard section but
> > that doesn't work.
> 
> In which way?

linker complains about missing symbol:

`efi' referenced in section `.init.text' of prelink.o: defined in discarded 
section `.discard' of prelink.o
`efi' referenced in section `.init.text' of prelink.o: defined in discarded 
section `.discard' of prelink.o
`efi' referenced in section `.init.text' of prelink.o: defined in discarded 
section `.discard' of prelink.o
`efi' referenced in section `.init.text' of prelink.o: defined in discarded 
section `.discard' of prelink.o
`efi' referenced in section `.init.text' of prelink.o: defined in discarded 
section `.discard' of prelink.o
`efi' referenced in section `.init.text' of prelink.o: defined in discarded 
section `.discard' of prelink.o
`efi' referenced in section `.init.text' of prelink.o: defined in discarded 
section `.discard' of prelink.o
`efi' referenced in section `.init.text' of prelink.o: defined in discarded 
section `.discard' of prelink.o
`efi' referenced in section `.init.text' of prelink.o: defined in discarded 
section `.discard' of prelink.o

This is with:

diff --git a/xen/arch/x86/efi/stub.c b/xen/arch/x86/efi/stub.c
index 0c481e3235..fa848effa6 100644
--- a/xen/arch/x86/efi/stub.c
+++ b/xen/arch/x86/efi/stub.c
@@ -17,6 +17,8 @@
  * efi_multiboot2() is an exception. Please look below for more details.
  */
 
+struct efi __used_section(".discard") efi;
+
 void __init noreturn efi_multiboot2(EFI_HANDLE ImageHandle,
 EFI_SYSTEM_TABLE *SystemTable)
 {
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)

> > I'm open to suggestions since I don't have any more
> > ideas about how to fix this.
> 
> 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.

Thanks, Roger.

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

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

2018-07-11 Thread Jan Beulich
>>> On 10.07.18 at 17:35,  wrote:
> On Tue, Jul 10, 2018 at 08:09:06AM -0600, Jan Beulich wrote:
>> >>> On 10.07.18 at 15:49,  wrote:
>> > On Tue, Jul 10, 2018 at 05:47:19AM -0600, Jan Beulich wrote:
>> >> >>> On 10.07.18 at 13:00,  wrote:
>> >> > On Mon, Jul 09, 2018 at 06:45:16PM +0200, Roger Pau Monné wrote:
>> >> >> Sorry for asking so many questions, but I would like to try to avoid
>> >> >> the DEFINED conditional in the linker script if possible.
>> >> > 
>> >> > If you wish to do that then put dummy efi symbol into 
>> >> > xen/arch/x86/efi/stub.c.
>> >> 
>> >> I think I've clearly objected to that before - I simply see no need for
>> >> wasting the space in an EFI-incapable binary.
>> > 
>> > But such waste is also added regardless of whether you define the
>> > symbol it in the linker script or in a C file?
>> 
>> How that? There's no space associated with the symbol when defined
>> this way. From the perspective of the binary, the space lives past the
>> end of the image, but since the symbol is never accessed (or else code
>> is broken somewhere) in the non-EFI case, this is okay.
> 
> Oh right, that's not inside of any section.
> 
> I've tried placing an efi symbol inside of the .discard section but
> that doesn't work.

In which way?

> I'm open to suggestions since I don't have any more
> ideas about how to fix this.

Another possible thing to try might be to make the extern declaration
of the symbol weak, and drop the offending line altogether.

Jan


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

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

2018-07-10 Thread Roger Pau Monné
On Tue, Jul 10, 2018 at 08:09:06AM -0600, Jan Beulich wrote:
> >>> On 10.07.18 at 15:49,  wrote:
> > On Tue, Jul 10, 2018 at 05:47:19AM -0600, Jan Beulich wrote:
> >> >>> On 10.07.18 at 13:00,  wrote:
> >> > On Mon, Jul 09, 2018 at 06:45:16PM +0200, Roger Pau Monné wrote:
> >> >> Sorry for asking so many questions, but I would like to try to avoid
> >> >> the DEFINED conditional in the linker script if possible.
> >> > 
> >> > If you wish to do that then put dummy efi symbol into 
> >> > xen/arch/x86/efi/stub.c.
> >> 
> >> I think I've clearly objected to that before - I simply see no need for
> >> wasting the space in an EFI-incapable binary.
> > 
> > But such waste is also added regardless of whether you define the
> > symbol it in the linker script or in a C file?
> 
> How that? There's no space associated with the symbol when defined
> this way. From the perspective of the binary, the space lives past the
> end of the image, but since the symbol is never accessed (or else code
> is broken somewhere) in the non-EFI case, this is okay.

Oh right, that's not inside of any section.

I've tried placing an efi symbol inside of the .discard section but
that doesn't work. I'm open to suggestions since I don't have any more
ideas about how to fix this.

Thanks, Roger.

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

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

2018-07-10 Thread Jan Beulich
>>> On 10.07.18 at 15:49,  wrote:
> On Tue, Jul 10, 2018 at 05:47:19AM -0600, Jan Beulich wrote:
>> >>> On 10.07.18 at 13:00,  wrote:
>> > On Mon, Jul 09, 2018 at 06:45:16PM +0200, Roger Pau Monné wrote:
>> >> Sorry for asking so many questions, but I would like to try to avoid
>> >> the DEFINED conditional in the linker script if possible.
>> > 
>> > If you wish to do that then put dummy efi symbol into 
>> > xen/arch/x86/efi/stub.c.
>> 
>> I think I've clearly objected to that before - I simply see no need for
>> wasting the space in an EFI-incapable binary.
> 
> But such waste is also added regardless of whether you define the
> symbol it in the linker script or in a C file?

How that? There's no space associated with the symbol when defined
this way. From the perspective of the binary, the space lives past the
end of the image, but since the symbol is never accessed (or else code
is broken somewhere) in the non-EFI case, this is okay.

Jan


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

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

2018-07-10 Thread Roger Pau Monné
On Tue, Jul 10, 2018 at 05:47:19AM -0600, Jan Beulich wrote:
> >>> On 10.07.18 at 13:00,  wrote:
> > On Mon, Jul 09, 2018 at 06:45:16PM +0200, Roger Pau Monné wrote:
> >> Sorry for asking so many questions, but I would like to try to avoid
> >> the DEFINED conditional in the linker script if possible.
> > 
> > If you wish to do that then put dummy efi symbol into 
> > xen/arch/x86/efi/stub.c.
> 
> I think I've clearly objected to that before - I simply see no need for
> wasting the space in an EFI-incapable binary.

But such waste is also added regardless of whether you define the
symbol it in the linker script or in a C file?

Thank, Roger.

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

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

2018-07-10 Thread Jan Beulich
>>> On 10.07.18 at 13:00,  wrote:
> On Mon, Jul 09, 2018 at 06:45:16PM +0200, Roger Pau Monné wrote:
>> On Fri, Jul 06, 2018 at 03:48:39PM +0200, Daniel Kiper wrote:
>> > On Thu, Jul 05, 2018 at 09:47:57AM +0200, Roger Pau Monné wrote:
>> > > On Wed, Jul 04, 2018 at 12:49:00PM +0200, Daniel Kiper wrote:
>> > > > On Wed, Jul 04, 2018 at 01:57:58AM -0600, Jan Beulich wrote:
>> > > > > >>> On 03.07.18 at 18:02,  wrote:
>> > > > > > On Thu, Jun 28, 2018 at 11:35:24PM -0600, Jan Beulich wrote:
>> > > > > >> >>> Roger Pau Monne  06/28/18 5:38 PM >>>
>> > > > > >> >lld (the llvm linker) has some issues with Xen linker script. It
>> > > > > >> >doesn't understand '||' in assert expressions:
>> > > > > >> >
>> > > > > >> >ld-melf_x86_64_fbsd  -T xen.lds -N prelink.o --build-id=sha1 
>> > > > > >> >\
>> > > > > >> >/root/src/xen/xen/common/symbols-dummy.o -o 
>> > > > > >> >/root/src/xen/xen/.xen-syms.0
>> > > > > >> >ld: error: xen.lds:260: malformed number: |
>> > > > > >>  ASSERT(__image_base__ > (261 >> 8) * 
>> > > > > >>  0x) | (261 << 
> 39)))
>> > > > > > + ((1 << 39) / 2)) + (64 << 30)) + (1 << 30)) + (1 << 30))) ||
>> > > > > >> 
>> > > > > >   ^
>> > > > > >> >
>> > > > > >> >And doesn't work properly with the 'DEFINED(foo) ? foo : ...'
>> > > > > >> >expression:
>> > > > > >> >
>> > > > > >> >ld-melf_x86_64_fbsd  -T xen.lds -N prelink.o --build-id=sha1 
>> > > > > >> >\
>> > > > > >> >/root/src/xen/xen/common/symbols-dummy.o -o 
>> > > > > >> >/root/src/xen/xen/.xen-syms.0
>> > > > > >> >ld: error: xen.lds:233: symbol not found: efi
>> > > > > >
>> > > > > > This smells like lld bug. efi symbol is clearly undefined in 
>> > > > > > prelink.o
>> > > > > > (lld does not support i386pep emulation):
>> > > > > >
>> > > > > >  11147:  0 NOTYPE  GLOBAL HIDDEN   UND efi
>> > > > > >
>> > > > > > However, surprisingly DEFINED() states that it is and ternary 
>> > > > > > operator
>> > > > > > fires address calculation from undefined symbol. So, I think that 
>> > > > > > lld
>> > > > > > have to be fixed instead of Xen. It should be noted that binutils 
>> > > > > > ld
>> > > > > > works without any issue.
>> > > > >
>> > > > > Right, but if we can make Xen build nevertheless, this would be 
>> > > > > better.
>> > > > > So what we need here is a (re-)explanation of why you've needed to
>> > > > > do the very change Roger is suggesting to revert.
>> > > >
>> > > > After commit b199c44 both ELF and PE output need an efi symbol. So, we
>> > > > cannot use simple "#ifdef EFI" for differentiation as it was earlier.
>> > >
>> > > I'm not sure I follow, why is that not an option?
>> > >
>> > > Can you detail which conditions will make the build to fail with the
>> > > proposed patch?
>> >
>> > The problem is that your patch will change efi symbol address in ELF
>> > output. The symbol is provided by Xen EFI runtime code. So, this way
>> > Xen will not work on EFI platform if it is loaded from GRUB2.
>> >
>> > > I assume that for the ELF output the EFI code is not added to the
>> > > image, and as such it requires the efi symbol to be defined because
>> > > it's referenced from common code?
>> >
>> > EFI code is added to the ELF image because it has to run on EFI
>> > platforms too. Of course via GRUB2 or to be more precise any
>> > Multiboot2 compatible boot loader.
>>
>> So now both the ELF and the PE binaries will contain the efi symbol?
>> Then which binary doesn't contain it?
> 
> Both always. The difference is where it come from. If build tools
> (compiler and linker) are able to generate PE files then efi symbol
> is derived from source files (xen/common/efi/runtime.c to be precise).
> Otherwise it is derived from xen.lds.S.
> 
>> Sorry for asking so many questions, but I would like to try to avoid
>> the DEFINED conditional in the linker script if possible.
> 
> If you wish to do that then put dummy efi symbol into 
> xen/arch/x86/efi/stub.c.

I think I've clearly objected to that before - I simply see no need for
wasting the space in an EFI-incapable binary.

Jan


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

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

2018-07-10 Thread Daniel Kiper
On Mon, Jul 09, 2018 at 06:45:16PM +0200, Roger Pau Monné wrote:
> On Fri, Jul 06, 2018 at 03:48:39PM +0200, Daniel Kiper wrote:
> > On Thu, Jul 05, 2018 at 09:47:57AM +0200, Roger Pau Monné wrote:
> > > On Wed, Jul 04, 2018 at 12:49:00PM +0200, Daniel Kiper wrote:
> > > > On Wed, Jul 04, 2018 at 01:57:58AM -0600, Jan Beulich wrote:
> > > > > >>> On 03.07.18 at 18:02,  wrote:
> > > > > > On Thu, Jun 28, 2018 at 11:35:24PM -0600, Jan Beulich wrote:
> > > > > >> >>> Roger Pau Monne  06/28/18 5:38 PM >>>
> > > > > >> >lld (the llvm linker) has some issues with Xen linker script. It
> > > > > >> >doesn't understand '||' in assert expressions:
> > > > > >> >
> > > > > >> >ld-melf_x86_64_fbsd  -T xen.lds -N prelink.o --build-id=sha1 \
> > > > > >> >/root/src/xen/xen/common/symbols-dummy.o -o 
> > > > > >> >/root/src/xen/xen/.xen-syms.0
> > > > > >> >ld: error: xen.lds:260: malformed number: |
> > > > > >>  ASSERT(__image_base__ > (261 >> 8) * 
> > > > > >>  0x) | (261 << 39)))
> > > > > > + ((1 << 39) / 2)) + (64 << 30)) + (1 << 30)) + (1 << 30))) ||
> > > > > >> 
> > > > > >   ^
> > > > > >> >
> > > > > >> >And doesn't work properly with the 'DEFINED(foo) ? foo : ...'
> > > > > >> >expression:
> > > > > >> >
> > > > > >> >ld-melf_x86_64_fbsd  -T xen.lds -N prelink.o --build-id=sha1 \
> > > > > >> >/root/src/xen/xen/common/symbols-dummy.o -o 
> > > > > >> >/root/src/xen/xen/.xen-syms.0
> > > > > >> >ld: error: xen.lds:233: symbol not found: efi
> > > > > >
> > > > > > This smells like lld bug. efi symbol is clearly undefined in 
> > > > > > prelink.o
> > > > > > (lld does not support i386pep emulation):
> > > > > >
> > > > > >  11147:  0 NOTYPE  GLOBAL HIDDEN   UND efi
> > > > > >
> > > > > > However, surprisingly DEFINED() states that it is and ternary 
> > > > > > operator
> > > > > > fires address calculation from undefined symbol. So, I think that 
> > > > > > lld
> > > > > > have to be fixed instead of Xen. It should be noted that binutils ld
> > > > > > works without any issue.
> > > > >
> > > > > Right, but if we can make Xen build nevertheless, this would be 
> > > > > better.
> > > > > So what we need here is a (re-)explanation of why you've needed to
> > > > > do the very change Roger is suggesting to revert.
> > > >
> > > > After commit b199c44 both ELF and PE output need an efi symbol. So, we
> > > > cannot use simple "#ifdef EFI" for differentiation as it was earlier.
> > >
> > > I'm not sure I follow, why is that not an option?
> > >
> > > Can you detail which conditions will make the build to fail with the
> > > proposed patch?
> >
> > The problem is that your patch will change efi symbol address in ELF
> > output. The symbol is provided by Xen EFI runtime code. So, this way
> > Xen will not work on EFI platform if it is loaded from GRUB2.
> >
> > > I assume that for the ELF output the EFI code is not added to the
> > > image, and as such it requires the efi symbol to be defined because
> > > it's referenced from common code?
> >
> > EFI code is added to the ELF image because it has to run on EFI
> > platforms too. Of course via GRUB2 or to be more precise any
> > Multiboot2 compatible boot loader.
>
> So now both the ELF and the PE binaries will contain the efi symbol?
> Then which binary doesn't contain it?

Both always. The difference is where it come from. If build tools
(compiler and linker) are able to generate PE files then efi symbol
is derived from source files (xen/common/efi/runtime.c to be precise).
Otherwise it is derived from xen.lds.S.

> Sorry for asking so many questions, but I would like to try to avoid
> the DEFINED conditional in the linker script if possible.

If you wish to do that then put dummy efi symbol into xen/arch/x86/efi/stub.c.

Daniel

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

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

2018-07-09 Thread Roger Pau Monné
On Fri, Jul 06, 2018 at 03:48:39PM +0200, Daniel Kiper wrote:
> On Thu, Jul 05, 2018 at 09:47:57AM +0200, Roger Pau Monné wrote:
> > On Wed, Jul 04, 2018 at 12:49:00PM +0200, Daniel Kiper wrote:
> > > On Wed, Jul 04, 2018 at 01:57:58AM -0600, Jan Beulich wrote:
> > > > >>> On 03.07.18 at 18:02,  wrote:
> > > > > On Thu, Jun 28, 2018 at 11:35:24PM -0600, Jan Beulich wrote:
> > > > >> >>> Roger Pau Monne  06/28/18 5:38 PM >>>
> > > > >> >lld (the llvm linker) has some issues with Xen linker script. It
> > > > >> >doesn't understand '||' in assert expressions:
> > > > >> >
> > > > >> >ld-melf_x86_64_fbsd  -T xen.lds -N prelink.o --build-id=sha1 \
> > > > >> >/root/src/xen/xen/common/symbols-dummy.o -o 
> > > > >> >/root/src/xen/xen/.xen-syms.0
> > > > >> >ld: error: xen.lds:260: malformed number: |
> > > > >>  ASSERT(__image_base__ > (261 >> 8) * 
> > > > >>  0x) | (261 << 39)))
> > > > > + ((1 << 39) / 2)) + (64 << 30)) + (1 << 30)) + (1 << 30))) ||
> > > > >> 
> > > > >   ^
> > > > >> >
> > > > >> >And doesn't work properly with the 'DEFINED(foo) ? foo : ...'
> > > > >> >expression:
> > > > >> >
> > > > >> >ld-melf_x86_64_fbsd  -T xen.lds -N prelink.o --build-id=sha1 \
> > > > >> >/root/src/xen/xen/common/symbols-dummy.o -o 
> > > > >> >/root/src/xen/xen/.xen-syms.0
> > > > >> >ld: error: xen.lds:233: symbol not found: efi
> > > > >
> > > > > This smells like lld bug. efi symbol is clearly undefined in prelink.o
> > > > > (lld does not support i386pep emulation):
> > > > >
> > > > >  11147:  0 NOTYPE  GLOBAL HIDDEN   UND efi
> > > > >
> > > > > However, surprisingly DEFINED() states that it is and ternary operator
> > > > > fires address calculation from undefined symbol. So, I think that lld
> > > > > have to be fixed instead of Xen. It should be noted that binutils ld
> > > > > works without any issue.
> > > >
> > > > Right, but if we can make Xen build nevertheless, this would be better.
> > > > So what we need here is a (re-)explanation of why you've needed to
> > > > do the very change Roger is suggesting to revert.
> > >
> > > After commit b199c44 both ELF and PE output need an efi symbol. So, we
> > > cannot use simple "#ifdef EFI" for differentiation as it was earlier.
> >
> > I'm not sure I follow, why is that not an option?
> >
> > Can you detail which conditions will make the build to fail with the
> > proposed patch?
> 
> The problem is that your patch will change efi symbol address in ELF
> output. The symbol is provided by Xen EFI runtime code. So, this way
> Xen will not work on EFI platform if it is loaded from GRUB2.
> 
> > I assume that for the ELF output the EFI code is not added to the
> > image, and as such it requires the efi symbol to be defined because
> > it's referenced from common code?
> 
> EFI code is added to the ELF image because it has to run on EFI
> platforms too. Of course via GRUB2 or to be more precise any
> Multiboot2 compatible boot loader.

So now both the ELF and the PE binaries will contain the efi symbol?
Then which binary doesn't contain it?

Sorry for asking so many questions, but I would like to try to avoid
the DEFINED conditional in the linker script if possible.

Thanks, Roger.

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

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

2018-07-06 Thread Daniel Kiper
On Thu, Jul 05, 2018 at 09:47:57AM +0200, Roger Pau Monné wrote:
> On Wed, Jul 04, 2018 at 12:49:00PM +0200, Daniel Kiper wrote:
> > On Wed, Jul 04, 2018 at 01:57:58AM -0600, Jan Beulich wrote:
> > > >>> On 03.07.18 at 18:02,  wrote:
> > > > On Thu, Jun 28, 2018 at 11:35:24PM -0600, Jan Beulich wrote:
> > > >> >>> Roger Pau Monne  06/28/18 5:38 PM >>>
> > > >> >lld (the llvm linker) has some issues with Xen linker script. It
> > > >> >doesn't understand '||' in assert expressions:
> > > >> >
> > > >> >ld-melf_x86_64_fbsd  -T xen.lds -N prelink.o --build-id=sha1 \
> > > >> >/root/src/xen/xen/common/symbols-dummy.o -o 
> > > >> >/root/src/xen/xen/.xen-syms.0
> > > >> >ld: error: xen.lds:260: malformed number: |
> > > >>  ASSERT(__image_base__ > (261 >> 8) * 0x) 
> > > >>  | (261 << 39)))
> > > > + ((1 << 39) / 2)) + (64 << 30)) + (1 << 30)) + (1 << 30))) ||
> > > >> 
> > > >   ^
> > > >> >
> > > >> >And doesn't work properly with the 'DEFINED(foo) ? foo : ...'
> > > >> >expression:
> > > >> >
> > > >> >ld-melf_x86_64_fbsd  -T xen.lds -N prelink.o --build-id=sha1 \
> > > >> >/root/src/xen/xen/common/symbols-dummy.o -o 
> > > >> >/root/src/xen/xen/.xen-syms.0
> > > >> >ld: error: xen.lds:233: symbol not found: efi
> > > >
> > > > This smells like lld bug. efi symbol is clearly undefined in prelink.o
> > > > (lld does not support i386pep emulation):
> > > >
> > > >  11147:  0 NOTYPE  GLOBAL HIDDEN   UND efi
> > > >
> > > > However, surprisingly DEFINED() states that it is and ternary operator
> > > > fires address calculation from undefined symbol. So, I think that lld
> > > > have to be fixed instead of Xen. It should be noted that binutils ld
> > > > works without any issue.
> > >
> > > Right, but if we can make Xen build nevertheless, this would be better.
> > > So what we need here is a (re-)explanation of why you've needed to
> > > do the very change Roger is suggesting to revert.
> >
> > After commit b199c44 both ELF and PE output need an efi symbol. So, we
> > cannot use simple "#ifdef EFI" for differentiation as it was earlier.
>
> I'm not sure I follow, why is that not an option?
>
> Can you detail which conditions will make the build to fail with the
> proposed patch?

The problem is that your patch will change efi symbol address in ELF
output. The symbol is provided by Xen EFI runtime code. So, this way
Xen will not work on EFI platform if it is loaded from GRUB2.

> I assume that for the ELF output the EFI code is not added to the
> image, and as such it requires the efi symbol to be defined because
> it's referenced from common code?

EFI code is added to the ELF image because it has to run on EFI
platforms too. Of course via GRUB2 or to be more precise any
Multiboot2 compatible boot loader.

Daniel

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

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

2018-07-05 Thread Roger Pau Monné
On Wed, Jul 04, 2018 at 12:49:00PM +0200, Daniel Kiper wrote:
> On Wed, Jul 04, 2018 at 01:57:58AM -0600, Jan Beulich wrote:
> > >>> On 03.07.18 at 18:02,  wrote:
> > > On Thu, Jun 28, 2018 at 11:35:24PM -0600, Jan Beulich wrote:
> > >> >>> Roger Pau Monne  06/28/18 5:38 PM >>>
> > >> >lld (the llvm linker) has some issues with Xen linker script. It
> > >> >doesn't understand '||' in assert expressions:
> > >> >
> > >> >ld-melf_x86_64_fbsd  -T xen.lds -N prelink.o --build-id=sha1 \
> > >> >/root/src/xen/xen/common/symbols-dummy.o -o 
> > >> >/root/src/xen/xen/.xen-syms.0
> > >> >ld: error: xen.lds:260: malformed number: |
> > >>  ASSERT(__image_base__ > (261 >> 8) * 0x) | 
> > >>  (261 << 39)))
> > > + ((1 << 39) / 2)) + (64 << 30)) + (1 << 30)) + (1 << 30))) ||
> > >> 
> > >   ^
> > >> >
> > >> >And doesn't work properly with the 'DEFINED(foo) ? foo : ...'
> > >> >expression:
> > >> >
> > >> >ld-melf_x86_64_fbsd  -T xen.lds -N prelink.o --build-id=sha1 \
> > >> >/root/src/xen/xen/common/symbols-dummy.o -o 
> > >> >/root/src/xen/xen/.xen-syms.0
> > >> >ld: error: xen.lds:233: symbol not found: efi
> > >
> > > This smells like lld bug. efi symbol is clearly undefined in prelink.o
> > > (lld does not support i386pep emulation):
> > >
> > >  11147:  0 NOTYPE  GLOBAL HIDDEN   UND efi
> > >
> > > However, surprisingly DEFINED() states that it is and ternary operator
> > > fires address calculation from undefined symbol. So, I think that lld
> > > have to be fixed instead of Xen. It should be noted that binutils ld
> > > works without any issue.
> >
> > Right, but if we can make Xen build nevertheless, this would be better.
> > So what we need here is a (re-)explanation of why you've needed to
> > do the very change Roger is suggesting to revert.
> 
> After commit b199c44 both ELF and PE output need an efi symbol. So, we
> cannot use simple "#ifdef EFI" for differentiation as it was earlier.

I'm not sure I follow, why is that not an option?

Can you detail which conditions will make the build to fail with the
proposed patch?

I assume that for the ELF output the EFI code is not added to the
image, and as such it requires the efi symbol to be defined because
it's referenced from common code?

Thanks, Roger.

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

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

2018-07-04 Thread Daniel Kiper
On Wed, Jul 04, 2018 at 01:57:58AM -0600, Jan Beulich wrote:
> >>> On 03.07.18 at 18:02,  wrote:
> > On Thu, Jun 28, 2018 at 11:35:24PM -0600, Jan Beulich wrote:
> >> >>> Roger Pau Monne  06/28/18 5:38 PM >>>
> >> >lld (the llvm linker) has some issues with Xen linker script. It
> >> >doesn't understand '||' in assert expressions:
> >> >
> >> >ld-melf_x86_64_fbsd  -T xen.lds -N prelink.o --build-id=sha1 \
> >> >/root/src/xen/xen/common/symbols-dummy.o -o /root/src/xen/xen/.xen-syms.0
> >> >ld: error: xen.lds:260: malformed number: |
> >>  ASSERT(__image_base__ > (261 >> 8) * 0x) | 
> >>  (261 << 39)))
> > + ((1 << 39) / 2)) + (64 << 30)) + (1 << 30)) + (1 << 30))) ||
> >> 
> >   ^
> >> >
> >> >And doesn't work properly with the 'DEFINED(foo) ? foo : ...'
> >> >expression:
> >> >
> >> >ld-melf_x86_64_fbsd  -T xen.lds -N prelink.o --build-id=sha1 \
> >> >/root/src/xen/xen/common/symbols-dummy.o -o /root/src/xen/xen/.xen-syms.0
> >> >ld: error: xen.lds:233: symbol not found: efi
> >
> > This smells like lld bug. efi symbol is clearly undefined in prelink.o
> > (lld does not support i386pep emulation):
> >
> >  11147:  0 NOTYPE  GLOBAL HIDDEN   UND efi
> >
> > However, surprisingly DEFINED() states that it is and ternary operator
> > fires address calculation from undefined symbol. So, I think that lld
> > have to be fixed instead of Xen. It should be noted that binutils ld
> > works without any issue.
>
> Right, but if we can make Xen build nevertheless, this would be better.
> So what we need here is a (re-)explanation of why you've needed to
> do the very change Roger is suggesting to revert.

After commit b199c44 both ELF and PE output need an efi symbol. So, we
cannot use simple "#ifdef EFI" for differentiation as it was earlier.
Going further if build tools are able to generate EFI code then native
Xen EFI code which provides efi symbol is build. Otherwise Xen EFI stub
code is used and efi symbol is provided by linker script. IIRC you
insisted on that. The issue can be solved by adding efi symbol to the
Xen EFI stub code (we can just simply copy existing efi struct from
native Xen EFI code) and removing relevant line from linker script.

Daniel

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

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

2018-07-04 Thread Jan Beulich
>>> On 03.07.18 at 18:02,  wrote:
> On Thu, Jun 28, 2018 at 11:35:24PM -0600, Jan Beulich wrote:
>> >>> Roger Pau Monne  06/28/18 5:38 PM >>>
>> >lld (the llvm linker) has some issues with Xen linker script. It
>> >doesn't understand '||' in assert expressions:
>> >
>> >ld-melf_x86_64_fbsd  -T xen.lds -N prelink.o --build-id=sha1 \
>> >/root/src/xen/xen/common/symbols-dummy.o -o /root/src/xen/xen/.xen-syms.0
>> >ld: error: xen.lds:260: malformed number: |
>>  ASSERT(__image_base__ > (261 >> 8) * 0x) | (261 
>>  << 39))) 
> + ((1 << 39) / 2)) + (64 << 30)) + (1 << 30)) + (1 << 30))) ||
>>  
>>  
>   ^
>> >
>> >And doesn't work properly with the 'DEFINED(foo) ? foo : ...'
>> >expression:
>> >
>> >ld-melf_x86_64_fbsd  -T xen.lds -N prelink.o --build-id=sha1 \
>> >/root/src/xen/xen/common/symbols-dummy.o -o /root/src/xen/xen/.xen-syms.0
>> >ld: error: xen.lds:233: symbol not found: efi
> 
> This smells like lld bug. efi symbol is clearly undefined in prelink.o
> (lld does not support i386pep emulation):
> 
>  11147:  0 NOTYPE  GLOBAL HIDDEN   UND efi
> 
> However, surprisingly DEFINED() states that it is and ternary operator
> fires address calculation from undefined symbol. So, I think that lld
> have to be fixed instead of Xen. It should be noted that binutils ld
> works without any issue.

Right, but if we can make Xen build nevertheless, this would be better.
So what we need here is a (re-)explanation of why you've needed to
do the very change Roger is suggesting to revert.

Jan



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

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

2018-07-03 Thread Roger Pau Monné
On Tue, Jul 03, 2018 at 06:02:27PM +0200, Daniel Kiper wrote:
> On Thu, Jun 28, 2018 at 11:35:24PM -0600, Jan Beulich wrote:
> > >>> Roger Pau Monne  06/28/18 5:38 PM >>>
> > >lld (the llvm linker) has some issues with Xen linker script. It
> > >doesn't understand '||' in assert expressions:
> > >
> > >ld-melf_x86_64_fbsd  -T xen.lds -N prelink.o --build-id=sha1 \
> > >/root/src/xen/xen/common/symbols-dummy.o -o /root/src/xen/xen/.xen-syms.0
> > >ld: error: xen.lds:260: malformed number: |
> >  ASSERT(__image_base__ > (261 >> 8) * 0x) | 
> >  (261 << 39))) + ((1 << 39) / 2)) + (64 << 30)) + (1 << 30)) + (1 << 
> >  30))) ||
> > 
> > 
> >  ^
> > >
> > >And doesn't work properly with the 'DEFINED(foo) ? foo : ...'
> > >expression:
> > >
> > >ld-melf_x86_64_fbsd  -T xen.lds -N prelink.o --build-id=sha1 \
> > >/root/src/xen/xen/common/symbols-dummy.o -o /root/src/xen/xen/.xen-syms.0
> > >ld: error: xen.lds:233: symbol not found: efi
> 
> This smells like lld bug. efi symbol is clearly undefined in prelink.o
> (lld does not support i386pep emulation):
> 
>  11147:  0 NOTYPE  GLOBAL HIDDEN   UND efi
> 
> However, surprisingly DEFINED() states that it is and ternary operator
> fires address calculation from undefined symbol. So, I think that lld
> have to be fixed instead of Xen. It should be noted that binutils ld
> works without any issue.

Yes, I know it's a bug in llvm ld. The '||' issue has already been
fixed upstream, and I hope the DEFINED one won't take long to get
fixed also.

Thanks, Roger.

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

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

2018-07-03 Thread Daniel Kiper
On Thu, Jun 28, 2018 at 11:35:24PM -0600, Jan Beulich wrote:
> >>> Roger Pau Monne  06/28/18 5:38 PM >>>
> >lld (the llvm linker) has some issues with Xen linker script. It
> >doesn't understand '||' in assert expressions:
> >
> >ld-melf_x86_64_fbsd  -T xen.lds -N prelink.o --build-id=sha1 \
> >/root/src/xen/xen/common/symbols-dummy.o -o /root/src/xen/xen/.xen-syms.0
> >ld: error: xen.lds:260: malformed number: |
>  ASSERT(__image_base__ > (261 >> 8) * 0x) | (261 
>  << 39))) + ((1 << 39) / 2)) + (64 << 30)) + (1 << 30)) + (1 << 30))) ||
>   
>   ^
> >
> >And doesn't work properly with the 'DEFINED(foo) ? foo : ...'
> >expression:
> >
> >ld-melf_x86_64_fbsd  -T xen.lds -N prelink.o --build-id=sha1 \
> >/root/src/xen/xen/common/symbols-dummy.o -o /root/src/xen/xen/.xen-syms.0
> >ld: error: xen.lds:233: symbol not found: efi

This smells like lld bug. efi symbol is clearly undefined in prelink.o
(lld does not support i386pep emulation):

 11147:  0 NOTYPE  GLOBAL HIDDEN   UND efi

However, surprisingly DEFINED() states that it is and ternary operator
fires address calculation from undefined symbol. So, I think that lld
have to be fixed instead of Xen. It should be noted that binutils ld
works without any issue.

Daniel

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

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

2018-06-28 Thread Jan Beulich
>>> Roger Pau Monne  06/28/18 5:38 PM >>>
>lld (the llvm linker) has some issues with Xen linker script. It
>doesn't understand '||' in assert expressions:
>
>ld-melf_x86_64_fbsd  -T xen.lds -N prelink.o --build-id=sha1 \
>/root/src/xen/xen/common/symbols-dummy.o -o /root/src/xen/xen/.xen-syms.0
>ld: error: xen.lds:260: malformed number: |
 ASSERT(__image_base__ > (261 >> 8) * 0x) | (261 << 
 39))) + ((1 << 39) / 2)) + (64 << 30)) + (1 << 30)) + (1 << 30))) ||

^
>
>And doesn't work properly with the 'DEFINED(foo) ? foo : ...'
>expression:
>
>ld-melf_x86_64_fbsd  -T xen.lds -N prelink.o --build-id=sha1 \
>/root/src/xen/xen/common/symbols-dummy.o -o /root/src/xen/xen/.xen-syms.0
>ld: error: xen.lds:233: symbol not found: efi
>
>Fix the first issue by using '|' instead of '||', and the second one
>by defining the efi symbol based on the EFI define (like it was done
>prior to commit b199c44afa).

Hmm, the second one is a direct revert of the original commit's hunk. There must
have been a reason for Daniel to change it, and hence undoing it might break 
what
he wants needs. You should have Cc-ed him right away.

Jan



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