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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.07.18 at 13:00, <daniel.kiper@xxxxxxxxxx> 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, <daniel.kiper@xxxxxxxxxx> wrote:
>> > > > > > On Thu, Jun 28, 2018 at 11:35:24PM -0600, Jan Beulich wrote:
>> > > > > >> >>> Roger Pau Monne <roger.pau@xxxxxxxxxx> 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) * 
>> > > > > >> >>>> 0xffff000000000000) | (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: 0000000000000000     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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel