|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Minios-devel] [UNIKRAFT PATCH] plat: Configure stack size page order
Hi Simon, Please see inline. On 9/5/19 11:55 AM, Simon Kuenzer wrote: > On 04.09.19 09:15, Costin Lupu wrote: >> 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. >> > > I completely agree with this and I think we are going to introduce > supporting different stack sizes later. The whole discussion is related > what we had with patch ID 735752 (see patchwork.unikraft.org). Although > this patch is also an intermediate fix, I tend to put this option > directly under 'Architecture Selection' because it should be independent > to any platform. The other reason is that we have the stack size > currently defined within the arch headers. > Alright, I'll move it to arch in v2. >>> >>> 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. >> > > I understand. Probably also something to re-visit later but not with > this patch... > >>> >>> 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. >> > > Arm32 uses some internal hard-coded values and does not use the > STACK_SIZE definition. I think we should adopt this with this patch, too. > > In general, I am fine with introducing this stack size configuration as > option. > > Thanks, > > Simon > >>> >>> 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
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |