Re: [PATCH v1 1/2] x86/sev: Use per-CPU PSC structure in prep for unaccepted memory support

[Tom Lendacky]

On 7/29/22 14:28, Dave Hansen wrote:
> On 7/29/22 12:22, Tom Lendacky wrote:
> > > I think this tidbit from the changelog threw me off:
> > >
> > > > Protect the use of the per-CPU structure by disabling interrupts during
> > > > memory acceptance.
> > >
> > > Could you please revise that to accurately capture the impact of this
> > > change?
> >
> > Is s/memory acceptance/page state changes/ enough of what you are
> > looking for or something more?
>
> That, plus a reminder of when "page state changes" are performed would
> be nice.  How frequent are they?  Are they performance sensitive?
> That'll help us decide if the design here is appropriate or not.

Without submitting a v2, here's what the updated paragraph would look like:

 Page state changes occur whenever DMA memory is allocated or memory needs
 to be shared with the hypervisor (kvmclock, attestation reports, etc.).
 A per-CPU structure is chosen over a single PSC structure protected with
 a lock because these changes can be initiated from interrupt or
 soft-interrupt context (e.g. the NVMe driver). Protect the use of the
 per-CPU structure by disabling interrupts during page state changes.
 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().

Hopefully there aren't a lot of page state changes occurring once a system 
has booted, so maybe a static struct with a lock would work. I am a bit 
worried about an NMI occurring during a page state change that requires a 
lock. I suppose, in_nmi() can be used to detect that and go the MSR 
protocol route to avoid a deadlock.

I can investigate that if the 2K-extra per-CPU is not desired.

Thanks,
Tom