Re: [Minios-devel] [UNIKRAFT PATCH] plat: Configure stack size page order

[Costin Lupu <costin.lup@xxxxxxxxx>]

Hi Simon,

Please see inline.

On 8/30/19 4:51 PM, Simon Kuenzer wrote:
> Hey Costin,
> 
> Thanks a lot for this patch. I am currently having a look but need some
> clarifications.
> 
> 1) Why did you expose the option int the platform submenu? It is
> obviously changing something in architecture headers?

Well, actually it is neither. I think it's actually a scheduling
abstraction, but one that is currently used for other stacks as well
(interrupts, traps). So it's true that it's weird to put it here, but
it's not an arch specific config either. This stack size value should be
the same regardless any arch or platform.

> 
> 2) Did you check the interrupt stack for Xen on x86? It seems that this
> one is just sized to PAGE_SIZE. I think this can get critical for thread
> current retrieval, right? See: plat/xen/x86/arch_events.c and
> plat/xen/x86/traps.c .

Well actually I see that the interrupt stack in
plat/xen/x86/arch_events.c has the right size, STACK_SIZE.

In deed, the trap stack in plat/xen/x86/traps.c is PAGE_SIZE and it
should be fixed.

> Do you by chance remember why we have the boot stack twice as big?
> See: xen/x86/setup.c

I don't remember, but it was the same with the interrupt stack in
plat/xen/x86/arch_events.c because the alignment was done at runtime. It
might be the same reason.

> 
> 3) More as a note: Xen on Arm32 seems not to follow any STACK_SIZE
> definition at all. We should probably put a note on this somewhere. I am
> not sure if it is worth fixing it - who knows what we are going to do
> with this architecture-platform-combination. I rather expect that we are
> going towards Arm64 for Xen in the future.

I'm not sure I follow. This patch fixes that and sets the same stack
size for ARM.

> 
> Thanks,
> 
> Simon
> 
> On 27.08.19 09:56, Costin Lupu wrote:
>> This patch adds a config option for configuring the stack size page
>> order. We
>> need this for supporting large stacks.
>>
>> Signed-off-by: Costin Lupu <costin.lupu@xxxxxxxxx>
>> ---
>>   arch/arm/arm/include/uk/asm/limits.h    | 2 +-
>>   arch/arm/arm64/include/uk/asm/limits.h  | 2 +-
>>   arch/x86/x86_64/include/uk/asm/limits.h | 2 +-
>>   plat/Config.uk                          | 9 +++++++++
>>   4 files changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/arm/include/uk/asm/limits.h
>> b/arch/arm/arm/include/uk/asm/limits.h
>> index 085761c3..e2298d6b 100644
>> --- a/arch/arm/arm/include/uk/asm/limits.h
>> +++ b/arch/arm/arm/include/uk/asm/limits.h
>> @@ -39,7 +39,7 @@
>>   #define __PAGE_MASK        (~((__PAGE_SIZE) - 1))
>>   #endif
>>   -#define __STACK_SIZE_PAGE_ORDER    2
>> +#define __STACK_SIZE_PAGE_ORDER    CONFIG_STACK_SIZE_PAGE_ORDER
>>   #define __STACK_SIZE        (__PAGE_SIZE * (1 <<
>> __STACK_SIZE_PAGE_ORDER))
>>     #define __WORDSIZE        32
>> diff --git a/arch/arm/arm64/include/uk/asm/limits.h
>> b/arch/arm/arm64/include/uk/asm/limits.h
>> index cec05641..fb70f2ba 100644
>> --- a/arch/arm/arm64/include/uk/asm/limits.h
>> +++ b/arch/arm/arm64/include/uk/asm/limits.h
>> @@ -40,7 +40,7 @@
>>   #define __PAGE_MASK        (~((__PAGE_SIZE) - 1))
>>   #endif
>>   -#define __STACK_SIZE_PAGE_ORDER    4
>> +#define __STACK_SIZE_PAGE_ORDER    CONFIG_STACK_SIZE_PAGE_ORDER
>>   #define __STACK_SIZE    (__PAGE_SIZE * (1 << __STACK_SIZE_PAGE_ORDER))
>>   #define __STACK_ALIGN_SIZE    16
>>   diff --git a/arch/x86/x86_64/include/uk/asm/limits.h
>> b/arch/x86/x86_64/include/uk/asm/limits.h
>> index a969bd17..21814044 100644
>> --- a/arch/x86/x86_64/include/uk/asm/limits.h
>> +++ b/arch/x86/x86_64/include/uk/asm/limits.h
>> @@ -39,7 +39,7 @@
>>   #define __PAGE_MASK        (~((__PAGE_SIZE) - 1))
>>   #endif
>>   -#define __STACK_SIZE_PAGE_ORDER    4
>> +#define __STACK_SIZE_PAGE_ORDER    CONFIG_STACK_SIZE_PAGE_ORDER
>>   #define __STACK_SIZE        (__PAGE_SIZE * (1 <<
>> __STACK_SIZE_PAGE_ORDER))
>>     #define __WORDSIZE        64
>> diff --git a/plat/Config.uk b/plat/Config.uk
>> index 8a878eb0..d0b99bd5 100644
>> --- a/plat/Config.uk
>> +++ b/plat/Config.uk
>> @@ -25,3 +25,12 @@ config HZ
>>       help
>>           Configure the timer interrupt frequency.
>>           Only change this if you know what you're doing.
>> +
>> +config STACK_SIZE_PAGE_ORDER
>> +    int
>> +    prompt "Stack size page order"
>> +    default 4
>> +    help
>> +        Indirectly configures the stack size by changing the stack
>> size page
>> +        order. Stack size is equal with 2^order * page size (e.g. 4KB).
>> +        Only change this if you know what you're doing.
>>
> 
> _______________________________________________
> Minios-devel mailing list
> Minios-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/mailman/listinfo/minios-devel

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel