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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Jul 11, 2018 at 01:09:30AM -0600, Jan Beulich wrote:
> >>> On 10.07.18 at 17:35, <roger.pau@xxxxxxxxxx> wrote:
> > On Tue, Jul 10, 2018 at 08:09:06AM -0600, Jan Beulich wrote:
> >> >>> On 10.07.18 at 15:49, <roger.pau@xxxxxxxxxx> wrote:
> >> > On Tue, Jul 10, 2018 at 05:47:19AM -0600, Jan Beulich wrote:
> >> >> >>> 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:
> >> >> >> 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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel