Re: [UNIKRAFT PATCH v2] include: Move sections.h to include/uk

[Simon Kuenzer <simon.kuenzer@xxxxxxxxx>]

Hey Stefan, hey Costin,

what is the reason that arch code needs sections.h? I can't read it from 
the commit message. Architecture code should only be the parts that are 
totally independent on the platform details. They are mostly helpers for 
arithmetics and CPU register definitions. I am concerned that 
architecture code may break when doing assumptions on symbol locations; 
imagine ASLR where locations are moved at runtime.
The NX bit patch is upstream.

Thanks,

Simon

On 26.10.20 17:14, stefanl.teodorescu@xxxxxxxxx wrote:
> From: Stefan Teodorescu <stefanl.teodorescu@xxxxxxxxx>
>
> Move plat/common/include/uk/plat/common/sections.h to
> include/uk/sections.h to be able to include it in arch code. This file
> is platform independent, so it does not affect anything.
>
> Signed-off-by: Stefan Teodorescu <stefanl.teodorescu@xxxxxxxxx>
> ---
>   {plat/common/include/uk/plat/common => include/uk}/sections.h | 0
>   plat/kvm/arm/entry64.S                                        | 2 +-
>   plat/kvm/arm/setup.c                                          | 2 +-
>   plat/kvm/memory.c                                             | 2 +-
>   plat/kvm/x86/setup.c                                          | 2 +-
>   plat/xen/arm/setup.c                                          | 2 +-
>   plat/xen/include/xen-arm/mm.h                                 | 2 +-
>   plat/xen/include/xen-x86/mm.h                                 | 2 +-
>   plat/xen/memory.c                                             | 2 +-
>   plat/xen/x86/mm.c                                             | 2 +-
>   10 files changed, 9 insertions(+), 9 deletions(-)
>   rename {plat/common/include/uk/plat/common => include/uk}/sections.h (100%)
>
> diff --git a/plat/common/include/uk/plat/common/sections.h 
> b/include/uk/sections.h
> similarity index 100%
> rename from plat/common/include/uk/plat/common/sections.h
> rename to include/uk/sections.h
> diff --git a/plat/kvm/arm/entry64.S b/plat/kvm/arm/entry64.S
> index c4de334c..8be3d59f 100644
> --- a/plat/kvm/arm/entry64.S
> +++ b/plat/kvm/arm/entry64.S
> @@ -35,7 +35,7 @@
>   #include <uk/asm.h>
>   #include <kvm-arm/mm.h>
>   #include <arm/cpu_defs.h>
> -#include <uk/plat/common/sections.h>
> +#include <uk/sections.h>
>   #include <uk/config.h>
>
>   .global page_table_size
> diff --git a/plat/kvm/arm/setup.c b/plat/kvm/arm/setup.c
> index 41e63755..8513b48c 100644
> --- a/plat/kvm/arm/setup.c
> +++ b/plat/kvm/arm/setup.c
> @@ -20,7 +20,7 @@
>    */
>   #include <uk/config.h>
>   #include <libfdt.h>
> -#include <uk/plat/common/sections.h>
> +#include <uk/sections.h>
>   #include <kvm/console.h>
>   #include <kvm/config.h>
>   #include <uk/assert.h>
> diff --git a/plat/kvm/memory.c b/plat/kvm/memory.c
> index 1d9269ec..61d3e6ca 100644
> --- a/plat/kvm/memory.c
> +++ b/plat/kvm/memory.c
> @@ -19,7 +19,7 @@
>    * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
>    */
>
> -#include <uk/plat/common/sections.h>
> +#include <uk/sections.h>
>   #include <sys/types.h>
>   #include <uk/plat/memory.h>
>   #include <uk/assert.h>
> diff --git a/plat/kvm/x86/setup.c b/plat/kvm/x86/setup.c
> index 9c7a93a8..1cab0a63 100644
> --- a/plat/kvm/x86/setup.c
> +++ b/plat/kvm/x86/setup.c
> @@ -27,7 +27,7 @@
>    */
>
>   #include <string.h>
> -#include <uk/plat/common/sections.h>
> +#include <uk/sections.h>
>   #include <x86/cpu.h>
>   #include <x86/traps.h>
>   #include <kvm/config.h>
> diff --git a/plat/xen/arm/setup.c b/plat/xen/arm/setup.c
> index 2df3b46c..bcbc939e 100644
> --- a/plat/xen/arm/setup.c
> +++ b/plat/xen/arm/setup.c
> @@ -25,7 +25,7 @@
>   /* Ported from Mini-OS */
>
>   #include <string.h>
> -#include <uk/plat/common/sections.h>
> +#include <uk/sections.h>
>   #include <xen-arm/os.h>
>   #include <xen-arm/mm.h>
>   #include <xen/xen.h>
> diff --git a/plat/xen/include/xen-arm/mm.h b/plat/xen/include/xen-arm/mm.h
> index 659de843..bbd31ddd 100644
> --- a/plat/xen/include/xen-arm/mm.h
> +++ b/plat/xen/include/xen-arm/mm.h
> @@ -28,7 +28,7 @@
>   #define _ARCH_MM_H_
>
>   #include <stdint.h>
> -#include <uk/plat/common/sections.h>
> +#include <uk/sections.h>
>   #include <uk/arch/limits.h>
>
>   typedef uint64_t paddr_t;
> diff --git a/plat/xen/include/xen-x86/mm.h b/plat/xen/include/xen-x86/mm.h
> index ffbedb09..99ba6d05 100644
> --- a/plat/xen/include/xen-x86/mm.h
> +++ b/plat/xen/include/xen-x86/mm.h
> @@ -25,7 +25,7 @@
>   #ifndef _ARCH_MM_H_
>   #define _ARCH_MM_H_
>
> -#include <uk/plat/common/sections.h>
> +#include <uk/sections.h>
>   #ifndef __ASSEMBLY__
>   #include <xen/xen.h>
>   #if defined(__i386__)
> diff --git a/plat/xen/memory.c b/plat/xen/memory.c
> index 8e7a7dda..27a317c4 100644
> --- a/plat/xen/memory.c
> +++ b/plat/xen/memory.c
> @@ -34,7 +34,7 @@
>    */
>
>   #include <string.h>
> -#include <uk/plat/common/sections.h>
> +#include <uk/sections.h>
>
>   #include <common/gnttab.h>
>   #if (defined __X86_32__) || (defined __X86_64__)
> diff --git a/plat/xen/x86/mm.c b/plat/xen/x86/mm.c
> index e006ab7f..9e352eed 100644
> --- a/plat/xen/x86/mm.c
> +++ b/plat/xen/x86/mm.c
> @@ -36,7 +36,7 @@
>    */
>
>   #include <string.h>
> -#include <uk/plat/common/sections.h>
> +#include <uk/sections.h>
>   #include <errno.h>
>   #include <uk/alloc.h>
>   #include <uk/plat/config.h>
> --
> 2.29.0