Messages in this thread Patch in this message | | | From | Tom Lendacky <> | Subject | [PATCH v3 1/2] x86/sev: Put PSC struct on the stack in prep for unaccepted memory support | Date | Mon, 15 Aug 2022 10:57:42 -0500 |
| |
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.
Also, since set_pages_state() uses the per-CPU GHCB, add a static variable that indicates when per-CPU GHCBs are available. Until they are available, the GHCB MSR protocol is used to perform page state changes. For APs, the per-CPU GHCB is created before they are started and registered upon startup, so this flag can be used globally for the BSP and APs instead of creating a per-CPU flag.
If either the reduction in PSC entries or the use of the MSR protocol until the per-CPU GHCBs are available, result 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, or use the boot GHCB prior to per-CPU GHCBs, can be investigated.
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com> --- arch/x86/include/asm/sev-common.h | 9 +++++++-- arch/x86/kernel/sev.c | 32 +++++++++++++++++++++++-------- 2 files changed, 31 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 + * increase this value without evaluating the impact to stack usage. + */ +#define VMGEXIT_PSC_MAX_ENTRY 64 struct psc_hdr { u16 cur_entry; diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c index c05f0124c410..40268ce97aad 100644 --- a/arch/x86/kernel/sev.c +++ b/arch/x86/kernel/sev.c @@ -66,6 +66,17 @@ static struct ghcb boot_ghcb_page __bss_decrypted __aligned(PAGE_SIZE); */ static struct ghcb *boot_ghcb __section(".data"); +/* + * A flag used by set_pages_state() that indicates when the per-CPU GHCB has + * been created and registered and thus can be used instead of using the MSR + * protocol. The set_pages_state() function eventually invokes vmgexit_psc(), + * which only works with a per-CPU GHCB. + * + * For APs, the per-CPU GHCB is created before they are started and registered + * upon startup, so this flag can be used globally for the BSP and APs. + */ +static bool ghcb_percpu_ready __section(".data"); + /* Bitmap of SEV features supported by the hypervisor */ static u64 sev_hv_features __ro_after_init; @@ -660,7 +671,7 @@ static void pvalidate_pages(unsigned long vaddr, unsigned int npages, bool valid } } -static void __init early_set_pages_state(unsigned long paddr, unsigned int npages, enum psc_op op) +static void early_set_pages_state(unsigned long paddr, unsigned int npages, enum psc_op op) { unsigned long paddr_end; u64 val; @@ -751,6 +762,8 @@ static int vmgexit_psc(struct snp_psc_desc *desc) unsigned long flags; struct ghcb *ghcb; + WARN_ON_ONCE(!ghcb_percpu_ready); + /* * __sev_get_ghcb() needs to run with IRQs disabled because it is using * a per-CPU GHCB. @@ -868,11 +881,14 @@ static void __set_pages_state(struct snp_psc_desc *data, unsigned long vaddr, static void set_pages_state(unsigned long vaddr, unsigned int npages, int op) { unsigned long vaddr_end, next_vaddr; - struct snp_psc_desc *desc; + struct snp_psc_desc desc = {}; - desc = kmalloc(sizeof(*desc), GFP_KERNEL_ACCOUNT); - if (!desc) - panic("SNP: failed to allocate memory for PSC descriptor\n"); + /* + * Use the MSR protocol when the per-CPU GHCBs are not yet registered, + * since vmgexit_psc() uses the per-CPU GHCB. + */ + if (!ghcb_percpu_ready) + return early_set_pages_state(__pa(vaddr), npages, op); vaddr = vaddr & PAGE_MASK; vaddr_end = vaddr + (npages << PAGE_SHIFT); @@ -882,12 +898,10 @@ static void set_pages_state(unsigned long vaddr, unsigned int npages, int op) next_vaddr = min_t(unsigned long, vaddr_end, (VMGEXIT_PSC_MAX_ENTRY * PAGE_SIZE) + vaddr); - __set_pages_state(desc, vaddr, next_vaddr, op); + __set_pages_state(&desc, vaddr, next_vaddr, op); vaddr = next_vaddr; } - - kfree(desc); } void snp_set_memory_shared(unsigned long vaddr, unsigned int npages) @@ -1254,6 +1268,8 @@ void setup_ghcb(void) if (cc_platform_has(CC_ATTR_GUEST_SEV_SNP)) snp_register_per_cpu_ghcb(); + ghcb_percpu_ready = true; + return; } -- 2.36.1
| |