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

[Tom Lendacky <thomas.lendacky@amd.com>]

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