Messages in this thread | | | Date | Wed, 3 Aug 2022 13:18:22 -0500 | Subject | Re: [PATCH v1.1 1/2] x86/sev: Use per-CPU PSC structure in prep for unaccepted memory support | From | Tom Lendacky <> |
| |
On 8/3/22 13:11, Tom Lendacky wrote:
Of course, I'll fix the subject if submitting this for real... ugh.
Thanks, Tom
> In advance of providing support for unaccepted memory, switch from using > kmalloc() for allocating the Page State Change (PSC) structure to using a > static structure. 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. > > Page state changes occur whenever DMA memory is allocated or memory needs > to be shared with the hypervisor (kvmclock, attestation reports, etc.). > Since most page state changes occur early in boot and are limited in > number, a single static PSC structure is used and protected by a spin > lock with interrupts disabled. > > Even with interrupts disabled, an NMI can be raised while performing > memory acceptance. The NMI could then cause further memory acceptance to > be performed. To prevent a deadlock, use the MSR protocol if executing in > an NMI context. > > Since the set_pages_state() path is the only path into vmgexit_psc(), > rename vmgexit_psc() to __vmgexit_psc() and remove the calls to disable > interrupts which are now performed by set_pages_state(). > > Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com> > --- > arch/x86/kernel/sev.c | 55 +++++++++++++++++++++++++------------------ > 1 file changed, 32 insertions(+), 23 deletions(-) > > diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c > index c05f0124c410..84d94fd2ec53 100644 > --- a/arch/x86/kernel/sev.c > +++ b/arch/x86/kernel/sev.c > @@ -66,6 +66,9 @@ static struct ghcb boot_ghcb_page __bss_decrypted __aligned(PAGE_SIZE); > */ > static struct ghcb *boot_ghcb __section(".data"); > > +/* Flag to indicate when the first per-CPU GHCB is registered */ > +static bool ghcb_percpu_ready __section(".data"); > + > /* Bitmap of SEV features supported by the hypervisor */ > static u64 sev_hv_features __ro_after_init; > > @@ -122,6 +125,15 @@ struct sev_config { > > static struct sev_config sev_cfg __read_mostly; > > +/* > + * Page State Change structure for use when accepting memory or when changing > + * page state. Use is protected by a spinlock with interrupts disabled, but an > + * NMI could still be raised, so check if running in an NMI an use the MSR > + * protocol in these cases. > + */ > +static struct snp_psc_desc psc_desc; > +static DEFINE_SPINLOCK(psc_desc_lock); > + > static __always_inline bool on_vc_stack(struct pt_regs *regs) > { > unsigned long sp = regs->sp; > @@ -660,7 +672,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; > @@ -742,26 +754,17 @@ void __init snp_prep_memory(unsigned long paddr, unsigned int sz, enum psc_op op > WARN(1, "invalid memory op %d\n", op); > } > > -static int vmgexit_psc(struct snp_psc_desc *desc) > +static int __vmgexit_psc(struct snp_psc_desc *desc) > { > int cur_entry, end_entry, ret = 0; > struct snp_psc_desc *data; > struct ghcb_state state; > struct es_em_ctxt ctxt; > - unsigned long flags; > struct ghcb *ghcb; > > - /* > - * __sev_get_ghcb() needs to run with IRQs disabled because it is using > - * a per-CPU GHCB. > - */ > - local_irq_save(flags); > - > ghcb = __sev_get_ghcb(&state); > - if (!ghcb) { > - ret = 1; > - goto out_unlock; > - } > + if (!ghcb) > + return 1; > > /* Copy the input desc into GHCB shared buffer */ > data = (struct snp_psc_desc *)ghcb->shared_buffer; > @@ -820,9 +823,6 @@ static int vmgexit_psc(struct snp_psc_desc *desc) > out: > __sev_put_ghcb(&state); > > -out_unlock: > - local_irq_restore(flags); > - > return ret; > } > > @@ -861,18 +861,25 @@ static void __set_pages_state(struct snp_psc_desc *data, unsigned long vaddr, > i++; > } > > - if (vmgexit_psc(data)) > + if (__vmgexit_psc(data)) > sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_PSC); > } > > static void set_pages_state(unsigned long vaddr, unsigned int npages, int op) > { > unsigned long vaddr_end, next_vaddr; > - struct snp_psc_desc *desc; > + unsigned long flags; > > - desc = kmalloc(sizeof(*desc), GFP_KERNEL_ACCOUNT); > - if (!desc) > - panic("SNP: failed to allocate memory for PSC descriptor\n"); > + /* > + * Use the MSR protocol when either: > + * - executing in an NMI to avoid any possibility of a deadlock > + * - per-CPU GHCBs are not yet registered, since __vmgexit_psc() > + * uses the per-CPU GHCB. > + */ > + if (in_nmi() || !ghcb_percpu_ready) > + return early_set_pages_state(__pa(vaddr), npages, op); > + > + spin_lock_irqsave(&psc_desc_lock, flags); > > vaddr = vaddr & PAGE_MASK; > vaddr_end = vaddr + (npages << PAGE_SHIFT); > @@ -882,12 +889,12 @@ 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(&psc_desc, vaddr, next_vaddr, op); > > vaddr = next_vaddr; > } > > - kfree(desc); > + spin_unlock_irqrestore(&psc_desc_lock, flags); > } > > void snp_set_memory_shared(unsigned long vaddr, unsigned int npages) > @@ -1254,6 +1261,8 @@ void setup_ghcb(void) > if (cc_platform_has(CC_ATTR_GUEST_SEV_SNP)) > snp_register_per_cpu_ghcb(); > > + ghcb_percpu_ready = true; > + > return; > } >
| |