Re: [Minios-devel] [UNIKRAFT PATCH v2 2/2] plat: Register eh_frame and eh_frame_hdr in memory.h

[Sharan Santhanam <sharan.santhanam@xxxxxxxxx>]

Hello Vlad,

There are some checkpatch warning/error

ERROR: trailing whitespace
#111: FILE: plat/xen/memory.c:108:
+^I^Im->len   = (size_t) __EH_FRAME_END $

ERROR: code indent should use tabs where possible
#112: FILE: plat/xen/memory.c:109:
+                - (size_t) __EH_FRAME_START;$

WARNING: please, no spaces at the start of a line
#112: FILE: plat/xen/memory.c:109:
+                - (size_t) __EH_FRAME_START;$

ERROR: trailing whitespace
#121: FILE: plat/xen/memory.c:118:
+^I^Im->len   = (size_t) __EH_FRAME_HDR_END $

ERROR: code indent should use tabs where possible
#122: FILE: plat/xen/memory.c:119:
+                - (size_t) __EH_FRAME_HDR_START;$

WARNING: please, no spaces at the start of a line
#122: FILE: plat/xen/memory.c:119:
+                - (size_t) __EH_FRAME_HDR_START;$

Please find the comments inline.


Thanks & Regards
Sharan

On 3/11/19 9:28 PM, Vlad-Andrei BĂDOIU (78692) wrote:
> Signed-off-by: Vlad-Andrei Badoiu <vlad_andrei.badoiu@xxxxxxxxxxxxxxx>
> ---
>   plat/common/include/sections.h | 32 +++++++++++++++++++++-----------
>   plat/kvm/memory.c              | 24 +++++++++++++++++++++++-
>   plat/xen/memory.c              | 20 ++++++++++++++++++++
>   3 files changed, 64 insertions(+), 12 deletions(-)
>
> diff --git a/plat/common/include/sections.h b/plat/common/include/sections.h
> index ecb36072..1052cfc7 100644
> --- a/plat/common/include/sections.h
> +++ b/plat/common/include/sections.h
> @@ -48,6 +48,12 @@ extern char _dtb[];
>   /* [_text, _etext]: contains .text.* sections */
>   extern char _text[], _etext[];
>   
> +/* [__eh_frame_start, __eh_frame_end]: contains .eh_frame section */
> +extern char __eh_frame_start, __eh_frame_end;
> +
> +/* [__eh_frame_hdr_start, __eh_frame_hdr_end]: contains .eh_frame_hdr section 
> */
> +extern char __eh_frame_hdr_start, __eh_frame_hdr_end;
> +
>   /* [_rodata, _erodata]: contains .rodata.* sections */
>   extern char _rodata[], _erodata[];
>   
> @@ -65,17 +71,21 @@ extern char _end[];
>   
>   #define __uk_image_symbol(addr)    ((unsigned long)(addr))
>   
> -#define __DTB		__uk_image_symbol(_dtb)
> -#define __TEXT         __uk_image_symbol(_text)
> -#define __ETEXT                __uk_image_symbol(_etext)
> -#define __RODATA       __uk_image_symbol(_rodata)
> -#define __ERODATA      __uk_image_symbol(_erodata)
> -#define __DATA         __uk_image_symbol(_data)
> -#define __EDATA                __uk_image_symbol(_edata)
> -#define __CTORS                __uk_image_symbol(_ctors)
> -#define __ECTORS       __uk_image_symbol(_ectors)
> -#define __BSS_START    __uk_image_symbol(__bss_start)
> -#define __END          __uk_image_symbol(_end)
> +#define __DTB                   __uk_image_symbol(_dtb)
> +#define __TEXT                  __uk_image_symbol(_text)
> +#define __ETEXT                 __uk_image_symbol(_etext)
> +#define __EH_FRAME_START        __uk_image_symbol(__eh_frame_start)
> +#define __EH_FRAME_END          __uk_image_symbol(__eh_frame_end)
> +#define __EH_FRAME_HDR_START    __uk_image_symbol(__eh_frame_hdr_start)
> +#define __EH_FRAME_HDR_END      __uk_image_symbol(__eh_frame_hdr_end)
> +#define __RODATA                __uk_image_symbol(_rodata)
> +#define __ERODATA               __uk_image_symbol(_erodata)
> +#define __DATA                  __uk_image_symbol(_data)
> +#define __EDATA                 __uk_image_symbol(_edata)
> +#define __CTORS                 __uk_image_symbol(_ctors)
> +#define __ECTORS                __uk_image_symbol(_ectors)
> +#define __BSS_START             __uk_image_symbol(__bss_start)
> +#define __END                   __uk_image_symbol(_end)
>   
>   #endif /*__ASSEMBLY__*/
>   
> diff --git a/plat/kvm/memory.c b/plat/kvm/memory.c
> index a7b4d5ef..6c520941 100644
> --- a/plat/kvm/memory.c
> +++ b/plat/kvm/memory.c
> @@ -33,7 +33,7 @@ extern void *_libkvmplat_mem_end;
>   
>   int ukplat_memregion_count(void)
>   {
> -       return 7;
> +       return 9;
>   }
>   
>   int ukplat_memregion_get(int i, struct ukplat_memregion_desc *m)
> @@ -115,6 +115,28 @@ int ukplat_memregion_get(int i, struct 
> ukplat_memregion_desc *m)
>                 ret = 0;
>   #if CONFIG_UKPLAT_MEMRNAME
>                 m->name  = "bstack";
> +#endif
> +               break;
How did you come about the numbering scheme?

Do you think it might be wiser to have the section based on the 
increasing order of the address?
> +       case 7: /* eh_frame */
> +               m->base  = (void *) __EH_FRAME_START;
> +               m->len   = (size_t) __EH_FRAME_END
> +                          - (size_t) __EH_FRAME_START;
> +               m->flags = (UKPLAT_MEMRF_RESERVED
> +                           | UKPLAT_MEMRF_READABLE);
> +               ret = 0;
> +#if CONFIG_UKPLAT_MEMRNAME
> +               m->name  = "eh_frame";
> +#endif
> +               break;
> +       case 8: /* eh_frame_hdr */
> +               m->base  = (void *) __EH_FRAME_HDR_START;
> +               m->len   = (size_t) __EH_FRAME_HDR_END
> +                          - (size_t) __EH_FRAME_HDR_START;
> +               m->flags = (UKPLAT_MEMRF_RESERVED
> +                           | UKPLAT_MEMRF_READABLE);
> +               ret = 0;
> +#if CONFIG_UKPLAT_MEMRNAME
> +               m->name  = "eh_frame_hdr";
>   #endif
>                 break;
>         default:
> diff --git a/plat/xen/memory.c b/plat/xen/memory.c
> index b63f11bb..2ffa6151 100644
> --- a/plat/xen/memory.c
> +++ b/plat/xen/memory.c
> @@ -101,6 +101,26 @@ int ukplat_memregion_get(int i, struct 
> ukplat_memregion_desc *m)
>                             | UKPLAT_MEMRF_WRITABLE);
>   #if CONFIG_UKPLAT_MEMRNAME
>                 m->name  = "bss";
> +#endif
> +               break;
> +       case 5: /* eh_frame */
> +               m->base  = (void *) __EH_FRAME_START;
> +               m->len   = (size_t) __EH_FRAME_END
> +                - (size_t) __EH_FRAME_START;
> +               m->flags = (UKPLAT_MEMRF_RESERVED
> +                           | UKPLAT_MEMRF_READABLE);
> +#if CONFIG_UKPLAT_MEMRNAME
> +               m->name  = "eh_frame";
> +#endif
> +               break;
> +       case 6: /* eh_frame_hdr */
> +               m->base  = (void *) __EH_FRAME_HDR_START;
> +               m->len   = (size_t) __EH_FRAME_HDR_END
> +                - (size_t) __EH_FRAME_HDR_START;
> +               m->flags = (UKPLAT_MEMRF_RESERVED
> +                           | UKPLAT_MEMRF_READABLE);
> +#if CONFIG_UKPLAT_MEMRNAME
> +               m->name  = "eh_frame_hdr";
>   #endif
>                 break;
>         default:
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel