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

Re: [Minios-devel] [UNIKRAFT PATCH v3 2/7] plat/kvm: Align linker script with Xen platform ones



Hi,
just one question inline


Simon Kuenzer <simon.kuenzer@xxxxxxxxx> writes:

> The linker scripts of Xen and KVM diverged too much. This patch
> is aligning KVMs with the ones from the Xen platform:
>
> - Unify symbols provided by linker script that mark start and end
>   of sections
> - Remove currently unused and unsupported eh_frame section
>   It may be added again when we officially introduce support
>   together with the other platforms
> - Use tabs for identation
> - Keep multiboot header just once
>
> Signed-off-by: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
> ---
>  plat/kvm/memory.c      | 32 +++++++++++------
>  plat/kvm/x86/entry64.S |  2 +-
>  plat/kvm/x86/link64.ld | 97 
> ++++++++++++++++++++++----------------------------
>  plat/kvm/x86/setup.c   |  2 +-
>  4 files changed, 67 insertions(+), 66 deletions(-)
>
> diff --git a/plat/kvm/memory.c b/plat/kvm/memory.c
> index cfb15a6..705c6df 100644
> --- a/plat/kvm/memory.c
> +++ b/plat/kvm/memory.c
> @@ -32,20 +32,21 @@ extern void *_libkvmplat_mem_end;
>  
>  int ukplat_memregion_count(void)
>  {
> -     return 5;
> +     return 6;
>  }
>  
>  int ukplat_memregion_get(int i, struct ukplat_memregion_desc *m)
>  {
> -     extern char _stext[], _etext[], _erodata[], _end[];
> +     extern char _text, _etext, _data, _edata, _rodata, _erodata,
> +                 __bss_start, _end;
>       int ret;
>  
>       UK_ASSERT(m);
>  
>       switch (i) {
>       case 0: /* text */
> -             m->base  = &_stext;
> -             m->len   = (size_t) &_etext - (size_t) &_stext;
> +             m->base  = &_text;
> +             m->len   = (size_t) &_etext - (size_t) &_text;
>               m->flags = (UKPLAT_MEMRF_RESERVED
>                           | UKPLAT_MEMRF_READABLE);
>  #if UKPLAT_MEMRNAME
> @@ -54,8 +55,8 @@ int ukplat_memregion_get(int i, struct 
> ukplat_memregion_desc *m)
>               ret = 0;
>               break;
>       case 1: /* rodata */
> -             m->base  = &_etext;
> -             m->len   = (size_t) &_erodata - (size_t) &_etext;
> +             m->base  = &_rodata;
> +             m->len   = (size_t) &_erodata - (size_t) &_rodata;
>               m->flags = (UKPLAT_MEMRF_RESERVED
>                           | UKPLAT_MEMRF_READABLE);
>  #if UKPLAT_MEMRNAME
> @@ -64,8 +65,8 @@ int ukplat_memregion_get(int i, struct 
> ukplat_memregion_desc *m)
>               ret = 0;
>               break;
>       case 2: /* data */
> -             m->base  = &_erodata;
> -             m->len   = (size_t) &_end - (size_t) &_erodata;
> +             m->base  = &_data;
> +             m->len   = (size_t) &_edata - (size_t) &_data;
>               m->flags = (UKPLAT_MEMRF_RESERVED
>                           | UKPLAT_MEMRF_READABLE
>                           | UKPLAT_MEMRF_WRITABLE);
> @@ -74,7 +75,18 @@ int ukplat_memregion_get(int i, struct 
> ukplat_memregion_desc *m)
>  #endif
>               ret = 0;
>               break;
> -     case 3: /* heap */
> +     case 3: /* bss */
> +             m->base  = &__bss_start;
> +             m->len   = (size_t) &_end - (size_t) &__bss_start;
> +             m->flags = (UKPLAT_MEMRF_RESERVED
> +                         | UKPLAT_MEMRF_READABLE
> +                         | UKPLAT_MEMRF_WRITABLE);
> +#if UKPLAT_MEMRNAME
> +             m->name  = "bss";
> +#endif
> +             ret = 0;
> +             break;
> +     case 4: /* heap */
>               m->base  = _libkvmplat_heap_start;
>               m->len   = (size_t) _libkvmplat_stack_top
>                          - (size_t) _libkvmplat_heap_start;
> @@ -84,7 +96,7 @@ int ukplat_memregion_get(int i, struct 
> ukplat_memregion_desc *m)
>  #endif
>               ret = 0;
>               break;
> -     case 4: /* stack */
> +     case 5: /* stack */
>               m->base  = _libkvmplat_stack_top;
>               m->len   = (size_t) _libkvmplat_mem_end
>                          - (size_t) _libkvmplat_stack_top;
> diff --git a/plat/kvm/x86/entry64.S b/plat/kvm/x86/entry64.S
> index 2d14386..47980ad 100644
> --- a/plat/kvm/x86/entry64.S
> +++ b/plat/kvm/x86/entry64.S
> @@ -46,7 +46,7 @@ _multiboot_header:
>  .long _multiboot_header
>  .long 0x100000
>  .long _edata
> -.long _ebss
> +.long _end
>  .long _libkvmplat_start32
>  
>  .section .bss
> diff --git a/plat/kvm/x86/link64.ld b/plat/kvm/x86/link64.ld
> index 85ea058..a9f3ac3 100644
> --- a/plat/kvm/x86/link64.ld
> +++ b/plat/kvm/x86/link64.ld
> @@ -6,7 +6,7 @@
>   *
>   * Copyright (c) 2016, IBM
>   *           (c) 2016-2017 Docker, Inc.
> - *           (c) 2017, NEC Europe Ltd.
> + *           (c) 2017-2018, NEC Europe Ltd.
>   *
>   * Permission to use, copy, modify, and/or distribute this software
>   * for any purpose with or without fee is hereby granted, provided
> @@ -24,63 +24,52 @@
>   */
>  
>  ENTRY(_libkvmplat_entry)
> +SECTIONS
> +{
> +     . = 0x100000;
>  
> -SECTIONS {
> -    . = 0x100000;
> +     /* Code */
> +     _text = .;
> +     .text :
> +     {
> +             /* prevent linker gc from removing multiboot header */
> +             KEEP (*(.data.multiboot))
As a side note, XEN does not have this KEEP in the linker. Is fine in
case of XEN?

>  
> -    /* Code */
> -    _stext = .;
> +             *(.text)
> +             *(.text.*)
> +     }
> +     _etext = .;
>  
> -    .text :
> -    {
> -        *(.data.multiboot)
> -        /* prevent linker gc from removing multiboot header */
> -        KEEP(*(.data.multiboot))
> -        *(.text)
> -        *(.text.*)
> -    }
> +     /* Read-only data */
> +     . = ALIGN(0x1000);
> +     _rodata = .;
> +     .rodata :
> +     {
> +             *(.rodata)
> +             *(.rodata.*)
> +     }
> +     _erodata = .;
>  
> -    _etext = .;
> +     /* Read-write data (initialized) */
> +     . = ALIGN(0x1000);
> +     _data = .;
> +     .data :
> +     {
> +             *(.data)
> +             *(.data.*)
> +     }
> +     _edata = .;
>  
> -    . = ALIGN(0x1000);
> -    /* Read-only data */
> -    .rodata :
> -    {
> -        *(.rodata)
> -        *(.rodata.*)
> -    }
> -    .eh_frame :
> -    {
> -        *(.eh_frame)
> -    }
> +     /* Read-write data (uninitialized) */
> +     . = ALIGN(0x1000);
> +     __bss_start = .;
> +     .bss :
> +     {
> +             *(.bss)
> +             *(.bss.*)
> +             *(COMMON)
> +             . = ALIGN(0x1000);
> +     }
>  
> -    _erodata = .;
> -
> -    . = ALIGN(0x1000);
> -    /* Read-write data (initialized) */
> -    .got :
> -    {
> -        *(.got.plt)
> -        *(.got)
> -    }
> -    .data :
> -    {
> -        *(.data)
> -        *(.data.*)
> -    }
> -
> -    _edata = .;
> -
> -    . = ALIGN(0x1000);
> -    /* Read-write data (uninitialized) */
> -    .bss :
> -    {
> -        *(.bss)
> -        *(.bss.*)
> -        *(COMMON)
> -    }
> -
> -    . = ALIGN(0x1000);
> -    _ebss = .;
> -    _end = .;
> +     _end = .;
>  }
> diff --git a/plat/kvm/x86/setup.c b/plat/kvm/x86/setup.c
> index 6895f29..332d10e 100644
> --- a/plat/kvm/x86/setup.c
> +++ b/plat/kvm/x86/setup.c
> @@ -78,7 +78,7 @@ static inline void _mb_get_cmdline(struct multiboot_info 
> *mi, char *cmdline,
>  
>  static inline void _mb_init_mem(struct multiboot_info *mi)
>  {
> -     extern char _end[];
> +     extern char _end;
>       multiboot_memory_map_t *m;
>       size_t offset, max_addr;
>  
> -- 
> 2.7.4
>
>
> _______________________________________________
> Minios-devel mailing list
> Minios-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/mailman/listinfo/minios-devel

-- 
Yuri Volchkov
Software Specialist

NEC Europe Ltd
Kurfürsten-Anlage 36
D-69115 Heidelberg

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

 


Rackspace

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