[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] x86/EFI + Live Patch: avoid symbol address truncation



On Tue, Jun 28, 2016 at 08:03:57AM -0600, Jan Beulich wrote:
> ld associates __init_end, placed outside of any section by the linker
> script, with the following section, resulting in a huge (wrapped, as it
> would be negative) section relative offset. COFF symbol tables store
> section relative addresses, and hence the above leads to assembler

My recollection is it was the linker? Like so:

/home/konrad/xen/xen/.xen.efi.0s.S:21: Warning: value 0x7d2f8000052e truncated 
to 0x8000052e
/home/konrad/xen/xen/.xen.efi.0s.S:22: Warning: value 0x7d2f80000a05 truncated 
to 0x80000a05
/home/konrad/xen/xen/.xen.efi.0s.S:23: Warning: value 0x7d2f80000a07 truncated 
to 0x80000a07

With this patch I still see those warnings?

(This is on OL7, so ldd (GNU libc) 2.17 gcc (GCC) 4.8.5 20150623 (Red Hat 
4.8.5-4) 

Let me try on different version.

> truncation warnings when all symbols get included in the symbol table
> (for Live Patching code). To overcome this, move __init_end past both
> ALIGN() directives. The consuming code (init_done()) is fine with such
> an adjustment (the distinction really would only be relevant for the
> loop claring the pages, and I think it's acceptable to clear a few
> more on - for now - EFI). This effectively results in the
> (__init_begin,__init_end) and (__2M_init_start,__2M_init_end) pairs to
> become identical, with their different names only serving documentation
> purposes now.
> 
> Note that moving __init_end and __2M_init_end into .init is not a good
> idea, as that would significantly grow xen.efi binary size.
> 
> While inspecting symbol table and ld behavior I also noticed that
> __2M_text_start gets put at address zero in the EFI case, which hasn't
> caused problems solely because we don't actually reference that symbol.
> Correct the setting of the initial address, and comment out said symbol
> for the time being, as with the initial address correction it would in
> turn cause an assembler truncation warning similar to the one mentioned
> above.
> 
> While checking init_done() for correctness with the above changes I
> noticed that code can easily be folded there, at once correcting the
> logged amount of memory which has got freed for the 2M-alignment case
> (i.e. EFI right now).
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -515,6 +515,7 @@ static inline bool_t using_2M_mapping(vo
>  static void noinline init_done(void)
>  {
>      void *va;
> +    unsigned long start, end;
>  
>      system_state = SYS_STATE_active;
>  
> @@ -530,18 +531,18 @@ static void noinline init_done(void)
>      /* Destroy Xen's mappings, and reuse the pages. */
>      if ( using_2M_mapping() )
>      {
> -        destroy_xen_mappings((unsigned long)&__2M_init_start,
> -                             (unsigned long)&__2M_init_end);
> -        init_xenheap_pages(__pa(__2M_init_start), __pa(__2M_init_end));
> +        start = (unsigned long)&__2M_init_start,
> +        end   = (unsigned long)&__2M_init_end;
>      }
>      else
>      {
> -        destroy_xen_mappings((unsigned long)&__init_begin,
> -                             (unsigned long)&__init_end);
> -        init_xenheap_pages(__pa(__init_begin), __pa(__init_end));
> +        start = (unsigned long)&__init_begin;
> +        end   = (unsigned long)&__init_end;
>      }
>  
> -    printk("Freed %ldkB init memory.\n", 
> (long)(__init_end-__init_begin)>>10);
> +    destroy_xen_mappings(start, end);
> +    init_xenheap_pages(__pa(start), __pa(end));
> +    printk("Freed %ldkB init memory\n", (end - start) >> 10);
>  
>      startup_cpu_idle_loop();
>  }
> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -40,9 +40,20 @@ SECTIONS
>  #if !defined(EFI)
>    . = __XEN_VIRT_START;
>    __image_base__ = .;
> +#else
> +  . = __image_base__;
>  #endif
>  
> +#if 0
> +/*
> + * We don't really use this symbol anywhere, and the way it would get defined
> + * here would result in it having a negative (wrapped to huge positive)
> + * offset relative to the .text section. That, in turn, causes an assembler
> + * truncation warning when including all symbols in the symbol table for Live
> + * Patching code.
> + */
>    __2M_text_start = .;         /* Start of 2M superpages, mapped RX. */
> +#endif
>  
>    . = __XEN_VIRT_START + MB(1);
>    _start = .;
> @@ -194,14 +205,13 @@ SECTIONS
>         *(.ctors)
>         __ctors_end = .;
>    } :text
> -  . = ALIGN(PAGE_SIZE);
> -  __init_end = .;
>  
>  #ifdef EFI
>    . = ALIGN(MB(2));
>  #else
>    . = ALIGN(PAGE_SIZE);
>  #endif
> +  __init_end = .;
>    __2M_init_end = .;
>  
>    __2M_rwdata_start = .;       /* Start of 2M superpages, mapped RW. */
> @@ -296,7 +306,6 @@ ASSERT(__image_base__ > XEN_VIRT_START |
>  ASSERT(kexec_reloc_size - kexec_reloc <= PAGE_SIZE, "kexec_reloc is too 
> large")
>  #endif
>  
> -ASSERT(IS_ALIGNED(__2M_text_start,   MB(2)), "__2M_text_start misaligned")
>  #ifdef EFI
>  ASSERT(IS_ALIGNED(__2M_text_end,     MB(2)), "__2M_text_end misaligned")
>  ASSERT(IS_ALIGNED(__2M_rodata_start, MB(2)), "__2M_rodata_start misaligned")
> 
> 

> x86/EFI + Live Patch: avoid symbol address truncation
> 
> ld associates __init_end, placed outside of any section by the linker
> script, with the following section, resulting in a huge (wrapped, as it
> would be negative) section relative offset. COFF symbol tables store
> section relative addresses, and hence the above leads to assembler
> truncation warnings when all symbols get included in the symbol table
> (for Live Patching code). To overcome this, move __init_end past both
> ALIGN() directives. The consuming code (init_done()) is fine with such
> an adjustment (the distinction really would only be relevant for the
> loop claring the pages, and I think it's acceptable to clear a few
> more on - for now - EFI). This effectively results in the
> (__init_begin,__init_end) and (__2M_init_start,__2M_init_end) pairs to
> become identical, with their different names only serving documentation
> purposes now.
> 
> Note that moving __init_end and __2M_init_end into .init is not a good
> idea, as that would significantly grow xen.efi binary size.
> 
> While inspecting symbol table and ld behavior I also noticed that
> __2M_text_start gets put at address zero in the EFI case, which hasn't
> caused problems solely because we don't actually reference that symbol.
> Correct the setting of the initial address, and comment out said symbol
> for the time being, as with the initial address correction it would in
> turn cause an assembler truncation warning similar to the one mentioned
> above.
> 
> While checking init_done() for correctness with the above changes I
> noticed that code can easily be folded there, at once correcting the
> logged amount of memory which has got freed for the 2M-alignment case
> (i.e. EFI right now).
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -515,6 +515,7 @@ static inline bool_t using_2M_mapping(vo
>  static void noinline init_done(void)
>  {
>      void *va;
> +    unsigned long start, end;
>  
>      system_state = SYS_STATE_active;
>  
> @@ -530,18 +531,18 @@ static void noinline init_done(void)
>      /* Destroy Xen's mappings, and reuse the pages. */
>      if ( using_2M_mapping() )
>      {
> -        destroy_xen_mappings((unsigned long)&__2M_init_start,
> -                             (unsigned long)&__2M_init_end);
> -        init_xenheap_pages(__pa(__2M_init_start), __pa(__2M_init_end));
> +        start = (unsigned long)&__2M_init_start,
> +        end   = (unsigned long)&__2M_init_end;
>      }
>      else
>      {
> -        destroy_xen_mappings((unsigned long)&__init_begin,
> -                             (unsigned long)&__init_end);
> -        init_xenheap_pages(__pa(__init_begin), __pa(__init_end));
> +        start = (unsigned long)&__init_begin;
> +        end   = (unsigned long)&__init_end;
>      }
>  
> -    printk("Freed %ldkB init memory.\n", 
> (long)(__init_end-__init_begin)>>10);
> +    destroy_xen_mappings(start, end);
> +    init_xenheap_pages(__pa(start), __pa(end));
> +    printk("Freed %ldkB init memory\n", (end - start) >> 10);
>  
>      startup_cpu_idle_loop();
>  }
> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -40,9 +40,20 @@ SECTIONS
>  #if !defined(EFI)
>    . = __XEN_VIRT_START;
>    __image_base__ = .;
> +#else
> +  . = __image_base__;
>  #endif
>  
> +#if 0
> +/*
> + * We don't really use this symbol anywhere, and the way it would get defined
> + * here would result in it having a negative (wrapped to huge positive)
> + * offset relative to the .text section. That, in turn, causes an assembler
> + * truncation warning when including all symbols in the symbol table for Live
> + * Patching code.
> + */
>    __2M_text_start = .;         /* Start of 2M superpages, mapped RX. */
> +#endif
>  
>    . = __XEN_VIRT_START + MB(1);
>    _start = .;
> @@ -194,14 +205,13 @@ SECTIONS
>         *(.ctors)
>         __ctors_end = .;
>    } :text
> -  . = ALIGN(PAGE_SIZE);
> -  __init_end = .;
>  
>  #ifdef EFI
>    . = ALIGN(MB(2));
>  #else
>    . = ALIGN(PAGE_SIZE);
>  #endif
> +  __init_end = .;
>    __2M_init_end = .;
>  
>    __2M_rwdata_start = .;       /* Start of 2M superpages, mapped RW. */
> @@ -296,7 +306,6 @@ ASSERT(__image_base__ > XEN_VIRT_START |
>  ASSERT(kexec_reloc_size - kexec_reloc <= PAGE_SIZE, "kexec_reloc is too 
> large")
>  #endif
>  
> -ASSERT(IS_ALIGNED(__2M_text_start,   MB(2)), "__2M_text_start misaligned")
>  #ifdef EFI
>  ASSERT(IS_ALIGNED(__2M_text_end,     MB(2)), "__2M_text_end misaligned")
>  ASSERT(IS_ALIGNED(__2M_rodata_start, MB(2)), "__2M_rodata_start misaligned")


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.