Re: [Minios-devel] [UNIKRAFT PATCHv4 19/43] plat/kvm: Add link script for Arm64

[Wei Chen <Wei.Chen@xxxxxxx>]

Hi Julien,

> -----Original Message-----
> From: Julien Grall <julien.grall@xxxxxxx>
> Sent: 2018年7月9日 4:27
> To: Wei Chen <Wei.Chen@xxxxxxx>; minios-devel@xxxxxxxxxxxxxxxxxxxx;
> simon.kuenzer@xxxxxxxxx
> Cc: Kaly Xin <Kaly.Xin@xxxxxxx>; nd <nd@xxxxxxx>
> Subject: Re: [Minios-devel] [UNIKRAFT PATCHv4 19/43] plat/kvm: Add link script
> for Arm64
> 
> Hi,
> 
> On 07/06/2018 10:03 AM, Wei Chen wrote:
> > This link script is based on x86 version, except following
> > differences:
> > 1. Arm64 needs DTB to parse devices, and QEMU/KVM will revserve
> 
> s/revserve/reserved/
> 
> >     the first 64KB of RAM as DTB area. In this case, we add a DTB
> >     section to this link script.
> 
> Do you mind giving a pointer to the QEMU/KVM layout? But then, do we

What did you mean a pointer? A source code link or others?

> really want to tie the linker script to QEMU layout?
> 

Currently, Yes. I don't want to involve too many dynamic/flexible
setting for my first target. We can adopt it later, but not now.

> > 2. We will use mmu to control each section's attribute, so the
> >     boundaries of sections with different memory attributes must
> >     be 4KiB alignment. For instance, the dtb section is readonly,
> >     but the text section  is readonly+exec. So the _text must start
> >     at a 4KiB alignment  address.
> >
> > Signed-off-by: Wei Chen <Wei.Chen@xxxxxxx>
> > ---
> >   plat/kvm/arm/link64.ld | 111 +++++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 111 insertions(+)
> >   create mode 100644 plat/kvm/arm/link64.ld
> >
> > diff --git a/plat/kvm/arm/link64.ld b/plat/kvm/arm/link64.ld
> > new file mode 100644
> > index 0000000..8798a5b
> > --- /dev/null
> > +++ b/plat/kvm/arm/link64.ld
> > @@ -0,0 +1,111 @@
> > +/* SPDX-License-Identifier: ISC */
> 
> See previous e-mail on SPDX.
> 
> > +/*
> > + * Author(s): Dan Williams <djwillia@xxxxxxxxxx>
> > + *            Martin Lucina <martin.lucina@xxxxxxxxxx>
> > + *            Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
> > + *            Wei Chen <wei.chen@xxxxxxx>
> > + *
> > + * Copyright (c) 2016, IBM
> > + *           (c) 2016-2017 Docker, Inc.
> > + *           (c) 2017, NEC Europe Ltd.
> > + *           (c) 2018, Arm Ltd.
> > + *
> > + * Permission to use, copy, modify, and/or distribute this software
> > + * for any purpose with or without fee is hereby granted, provided
> > + * that the above copyright notice and this permission notice appear
> > + * in all copies.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
> > + * WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED
> > + * WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE
> > + * AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR
> > + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS
> > + * OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
> > + * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
> > + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> > + */
> > +
> > +OUTPUT_FORMAT("elf64-littleaarch64")
> > +OUTPUT_ARCH(aarch64)
> > +ENTRY(_libkvmplat_entry)
> > +
> > +/*
> > + * We use mmu to control each section's attribute, so the boundaries
> > + * of sections with different memory attributes must be 4KiB alignment.
> > + * For instance, the dtb section is readonly, but the text section
> > + * is readonly+exec. So the _text must start at a 4KiB alignment
> > + * address.
> > + */
> > +SECTIONS {
> > +   /* QEMU-AArch64 virt platform's ram base address */
> > +   . = 0x40000000;
> > +
> > +   /* Reserve first 64 KBytes for DTB */
> > +   _dtb = .;
> > +   . = . + 0x10000;
> 
> Please use a macro here to define 0x10000. This will be easier to see
> what is done.

Ok.

> 
> > +
> > +   /* Code */
> > +   _text = .;
> > +   .text :
> > +   {
> > +           *(.text)
> > +           *(.text.*)
> > +   }
> > +
> > +   . = ALIGN(0x1000);
> > +   _etext = .;
> > +
> > +   /* Read-only data */
> > +   _rodata = .;
> > +   .rodata :
> > +   {
> > +           *(.rodata)
> > +           *(.rodata.*)
> > +   }
> > +   .eh_frame :
> > +   {
> > +           *(.eh_frame)
> > +   }
> > +   _erodata = .;
> > +
> > +   /* Constructor tables (read-only) */
> > +   _ctors = .;
> > +   .preinit_array : {
> > +           . = ALIGN(0x8);
> > +           PROVIDE_HIDDEN (__preinit_array_start = .);
> > +           KEEP (*(.preinit_array))
> > +           PROVIDE_HIDDEN (__preinit_array_end = .);
> > +   }
> > +
> > +   .init_array : {
> > +           . = ALIGN(0x8);
> > +           PROVIDE_HIDDEN (__init_array_start = .);
> > +           KEEP (*(SORT_BY_INIT_PRIORITY(.init_array.*)
> SORT_BY_INIT_PRIORITY(.ctors.*)))
> > +           KEEP (*(.init_array .ctors))
> > +           PROVIDE_HIDDEN (__init_array_end = .);
> > +   }
> > +   . = ALIGN(0x1000);
> 
> I guess, you want this to be page-aligned. If so, please use PAGE_SIZE.
> 

Yes, I will use it.

> > +   _ectors = .;
> 
> I would move this before the . = ALIGN(...) to make clear the alignment
> is required for data.
> 

Ok.

> > +
> > +   /* Read-write data (initialized) */
> > +   _data = .;
> > +   .data :
> > +   {
> > +           *(.data)
> > +           *(.data.*)
> > +   }
> > +   _edata = .;
> > +
> > +   . = ALIGN(0x1000);
> 
> Same here.
> 
> > +   __bss_start = .;
> > +   /* Read-write data (uninitialized) */
> 
> The word "uninitialized" is a bit misleading here. In C, this section
> should be Zeroed. So there are not really "unitialized".
> 

Without GCC -fzero-initialized-in-bss, this section in image is not zero,
It is uninitialized. And here, the uninitialized means the objects in
Code are not explicitly initialized to a value. System behavior to
initialize BSS section to zero is not in this scope.

> This raise the question of who is going to initialize that region?
> 

I know some unix-like systems or Windows would initialize the bss section
to zero. But I am not sure whether we should initialize BSS.

How do you think about it @Simon?

> > +   .bss :
> > +   {
> > +           *(.bss)
> > +           *(.bss.*)
> > +           *(COMMON)
> > +           . = ALIGN(0x1000);
> > +   }
> > +
> > +   _end = .;
> > +}
> >
> 
> Cheers,
> 
> --
> Julien Grall
> 
> --
> Julien Grall
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel