Re: [Minios-devel] [UNIKRAFT PATCH v3 1/2] plat: Add the eh_frame and eh_frame_hdr sections

[Vlad-Andrei BĂDOIU (78692) <vlad_andrei.badoiu@xxxxxxxxxxxxxxx>]

Hey Sharan,

Please see my answers inline:

Thanks

Vlad

On 4/1/19 6:39 PM, Sharan Santhanam wrote:
> Hello Vlad,
>
> Please find the comments inline:
>
> Thanks & Regards
> Sharan
>
> On 3/20/19 1:03 PM, Vlad-Andrei BĂDOIU (78692) wrote:
>> From: Vlad-Andrei BĂDOIU (78692) <vlad_andrei.badoiu@xxxxxxxxxxxxxxx>
>>
>> This patch adds the eh_frame and eh_frame hdr sections to
>> the linker script for both XEN and KVM.
>>
>> A new linker script has been created plat/common/x86/link64.lds in
>> order to avoid code duplication.
>>
>> This patch adds the frames to the linker scripts.
>>
>> Signed-off-by: Vlad-Andrei Badoiu <vlad_andrei.badoiu@xxxxxxxxxxxxxxx>
>> ---
>>   plat/common/x86/link64.lds | 50 ++++++++++++++++++++++++++++++++++++++
>>   plat/kvm/arm/link64.lds.S  | 26 +++++++++++++++++---
>>   plat/kvm/x86/link64.lds    |  2 ++
>>   plat/xen/arm/link32.lds    | 20 +++++++++++++++
>>   plat/xen/x86/link64.lds    |  2 ++
>>   5 files changed, 96 insertions(+), 4 deletions(-)
>>   create mode 100644 plat/common/x86/link64.lds
>>
>> diff --git a/plat/common/x86/link64.lds b/plat/common/x86/link64.lds
>> new file mode 100644
>> index 00000000..fc3316f6
>> --- /dev/null
>> +++ b/plat/common/x86/link64.lds
>> @@ -0,0 +1,50 @@
>> +/* SPDX-License-Identifier: BSD-3-Clause */
>> +/*
>> + * Authors: Vlad-Andrei Badoiu <vlad_andrei.badoiu@xxxxxxxxxxxxxxx>
>> + *
>> + * Copyright (c) 2018, NEC Europe Ltd., NEC Corporation. All rights 
>> reserved.
>> + *
>> + * Redistribution and use in source and binary forms, with or without
>> + * modification, are permitted provided that the following conditions
>> + * are met:
>> + *
>> + * 1. Redistributions of source code must retain the above copyright
>> + *    notice, this list of conditions and the following disclaimer.
>> + * 2. Redistributions in binary form must reproduce the above copyright
>> + *    notice, this list of conditions and the following disclaimer 
>> in the
>> + *    documentation and/or other materials provided with the 
>> distribution.
>> + * 3. Neither the name of the copyright holder nor the names of its
>> + *    contributors may be used to endorse or promote products 
>> derived from
>> + *    this software without specific prior written permission.
>> + *
>> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND 
>> CONTRIBUTORS "AS IS"
>> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED 
>> TO, THE
>> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A 
>> PARTICULAR PURPOSE
>> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR 
>> CONTRIBUTORS BE
>> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
>> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
>> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR 
>> BUSINESS
>> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, 
>> WHETHER IN
>> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR 
>> OTHERWISE)
>> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF 
>> ADVISED OF THE
>> + * POSSIBILITY OF SUCH DAMAGE.
>> + *
>> + * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
>> + */
>> +
>> +. = ALIGN(0x1000);
>> +__eh_frame_start = .;
>> +.eh_frame :
>> +{
>> +    *(.eh_frame)
>> +    *(.eh_frame.*)
>> +}
>> +__eh_frame_end = .;
>> +
>> +__eh_frame_hdr_start = .;
>> +.eh_frame_hdr :
>> +{
>> +    *(.eh_frame_hdr)
>> +    *(.eh_frame_hdr.*)
>> +}
>> +__eh_frame_hdr_end = .;
>> diff --git a/plat/kvm/arm/link64.lds.S b/plat/kvm/arm/link64.lds.S
>> index 1c2ad8c9..6aa955f5 100644
>> --- a/plat/kvm/arm/link64.lds.S
>> +++ b/plat/kvm/arm/link64.lds.S
>> @@ -68,6 +68,26 @@ SECTIONS {
>>       . = ALIGN(__PAGE_SIZE);
>>       _etext = .;
>>   +    /* Exception frame */
>> +    __eh_frame_start = .;
>> +    .eh_frame :
>> +    {
>> +        *(.eh_frame)
>
> Why would we have ).*? Shouldn't it be .*) instead?
I agree, it was a typo on my side.
>
>> +        *(.eh_frame).*
>> +    }
>> +    __eh_frame_end = .;
>> +
>> +    /* Exception frame header */
>> +    __eh_frame_hdr_start = .;
>> +    .eh_frame_hdr :
>> +    {
>> +
>> +        *(.eh_frame_hdr)
>> +        *(.eh_frame_hdr.*)
>> +    }
>> +    . = ALIGN(__PAGE_SIZE);
>> +    __eh_frame_hdr_end = .;
>> +
>>       /* Read-only data */
>>       _rodata = .;
>>       .rodata :
>> @@ -75,10 +95,8 @@ SECTIONS {
>>           *(.rodata)
>>           *(.rodata.*)
>>       }
>> -    .eh_frame :
>> -    {
>> -        *(.eh_frame)
>> -    }
>> +
>> +
>>       _erodata = .;
>>         /* Constructor tables (read-only) */
>> diff --git a/plat/kvm/x86/link64.lds b/plat/kvm/x86/link64.lds
>> index 60b5d214..c96f7501 100644
>> --- a/plat/kvm/x86/link64.lds
>> +++ b/plat/kvm/x86/link64.lds
>> @@ -41,6 +41,8 @@ SECTIONS
>>       }
>>       _etext = .;
>>   +    INCLUDE plat/common/x86/link64.lds
>> +
>>       /* Read-only data */
>>       . = ALIGN(0x1000);
>>       _rodata = .;
>> diff --git a/plat/xen/arm/link32.lds b/plat/xen/arm/link32.lds
>> index ce200b67..246244ec 100644
>> --- a/plat/xen/arm/link32.lds
>> +++ b/plat/xen/arm/link32.lds
>> @@ -47,6 +47,26 @@ SECTIONS
>>         _etext = .;            /* End of text section */
>
> Shouldn't we page align eh_frame_start?
Yes, it should be an . = ALIGN(4096); before it.
>> +    /* Exception frame */
>> +    __eh_frame_start = .;
>> +    .eh_frame :
>> +    {
>> +        *(.eh_frame)
>> +        *(.eh_frame.*)
>> +    }
>> +    __eh_frame_end = .;
>> +
>> +
>> +    /* Exception frame header */
>> +    __eh_frame_hdr_start = .;
>> +    .eh_frame_hdr :
>> +    {
>> +        *(.eh_frame_hdr)
>> +        *(.eh_frame_hdr.*)
>> +    }
>> +    . = ALIGN(4096);
>> +    __eh_frame_hdr_end = .;
>> +
>>       /* Read-only data */
>>       _rodata = .;
>>       .rodata :
>> diff --git a/plat/xen/x86/link64.lds b/plat/xen/x86/link64.lds
>> index dce0baf2..82ab653a 100644
>> --- a/plat/xen/x86/link64.lds
>> +++ b/plat/xen/x86/link64.lds
>> @@ -39,6 +39,8 @@ SECTIONS
>>         _etext = .;            /* End of text section */
>>   +    INCLUDE plat/common/x86/link64.lds
>> +
>>       _rodata = .;
>>       .rodata : {
>>           *(.rodata)
>>
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel