Re: [PATCH v1 5/6] ArmVirtPkg: Kvmtool: Increase primary core stack size

["Ard Biesheuvel" <ardb@kernel.org>]

On Thu, 18 May 2023 at 11:10, Sami Mujawar <sami.mujawar@arm.com> wrote:
>
> The patch "f07a9df9af60 ArmVirtPkg: Enable stack guard"
> enabled stack overflow detection for ArmVirtPkg. Following
> this patch, running UEFI shell command 'dmpstore' resulted
> in a crash indicating a stack overflow. Invoking 'dmpstore'
> results in recursive calls to CascadeProcessVariables ()
> which apparently consumes the available stack space and
> overflows.
>
> Therefore, increase the primary core stack size.
>

Thanks for the fix. I imagine diagnosing this may not have been trivial.

However, I don't think this is the right fix tbh. Normally, SEC and
PEI run off this initial stack, and the DxeIpl PEIM is in charging of
launching the DxeCore with a full sized stack, and remapping it
non-executable as well.

These PrePi platforms take some shortcuts and apparently, one of the
consequences is that DXE and BDS run off the initial stack, which
points into the firmware image IIRC.

IOW, it would be better to explicitly allocate 128 KiB worth of
bootservices data memory and let the DxeCore run off of that.


> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
>  ArmVirtPkg/ArmVirtKvmTool.dsc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc
> index 4541d03d23e0d98915b3d3ada688c48d979b75d2..664a624fd2a30bb466a3df2103482e3e6c1f303a 100644
> --- a/ArmVirtPkg/ArmVirtKvmTool.dsc
> +++ b/ArmVirtPkg/ArmVirtKvmTool.dsc
> @@ -126,7 +126,7 @@ [PcdsFixedAtBuild.common]
>    gArmTokenSpaceGuid.PcdVFPEnabled|1
>  !endif
>
> -  gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x4000
> +  gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x8000
>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
>
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>