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

[Costin Lupu <costin.lupu@xxxxxxxxx>]

Hi Simon,

Regardless whether these changes are needed or not by arch code (I'll
leave that for Ștefan to explain), the definitions in sections.h should
stay under include/uk in order to be accessible from everywhere.
Moreover, the definitions in it are related to neither arch or platform.

Cheers,
Costin

On 10/28/20 3:14 PM, Simon Kuenzer wrote:
> 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
>>
>>