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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 20 Sep 2022 18:15:07 +0200
From:   Borislav Petkov <bp@...en8.de>
To:     Tom Lendacky <thomas.lendacky@....com>
Cc:     linux-kernel@...r.kernel.org, x86@...nel.org,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        "Kirill A. Shutemov" <kirill@...temov.name>,
        "H. Peter Anvin" <hpa@...or.com>,
        Michael Roth <michael.roth@....com>,
        Joerg Roedel <jroedel@...e.de>,
        Andy Lutomirski <luto@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH v4 1/4] x86/sev: Put PSC struct on the stack in prep for
 unaccepted memory support

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@....com>

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

> ---
>  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