Re: [PATCH v4 1/4] x86/sev: Put PSC struct on the stack in prep for unaccepted memory support

[Borislav Petkov]

On Thu, Aug 25, 2022 at 09:23:14AM -0500, Tom Lendacky wrote:
> In advance of providing support for unaccepted memory, switch from using
> kmalloc() for allocating the Page State Change (PSC) structure to using a
> local variable that lives on the stack. This is needed to avoid a possible
> recursive call into set_pages_state() if the kmalloc() call requires
> (more) memory to be accepted, which would result in a hang.
> 
> The current size of the PSC struct is 2,032 bytes. To make the struct more
> stack friendly, reduce the number of PSC entries from 253 down to 64,
> resulting in a size of 520 bytes. This is a nice compromise on struct size
> and total PSC requests while still allowing parallel PSC operations across
> vCPUs.
> 
> If the reduction in PSC entries results in any kind of performance issue
> (that is not seen at the moment), use of a larger static PSC struct, with
> fallback to the smaller stack version, can be investigated.

"For more background info on this decision see the subthread in the Link
tag below."

> Signed-off-by: Tom Lendacky <thomas.lendacky@xxxxxxx>

Link: https://lore.kernel.org/lkml/658c455c40e8950cb046dd885dd19dc1c52d060a.1659103274.git.thomas.lendacky@xxxxxxx

> ---
>  arch/x86/include/asm/sev-common.h |  9 +++++++--
>  arch/x86/kernel/sev.c             | 10 ++--------
>  2 files changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h
> index b8357d6ecd47..6c3d61c5f6a3 100644
> --- a/arch/x86/include/asm/sev-common.h
> +++ b/arch/x86/include/asm/sev-common.h
> @@ -106,8 +106,13 @@ enum psc_op {
>  #define GHCB_HV_FT_SNP			BIT_ULL(0)
>  #define GHCB_HV_FT_SNP_AP_CREATION	BIT_ULL(1)
>  
> -/* SNP Page State Change NAE event */
> -#define VMGEXIT_PSC_MAX_ENTRY		253
> +/*
> + * SNP Page State Change NAE event
> + *   The VMGEXIT_PSC_MAX_ENTRY determines the size of the PSC structure,
> + *   which is a local variable (stack usage) in set_pages_state(). Do not

... which is a local stack variable...


> + *   increase this value without evaluating the impact to stack usage.
> + */
...

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette