Re: [PATCH v2 3/3] x86/sev: Add SEV-SNP CipherTextHiding support

[Sean Christopherson <seanjc@google.com>]

On Tue, Nov 19, 2024, Ashish Kalra wrote:
> On 10/11/2024 11:04 AM, Sean Christopherson wrote:
> > On Wed, Oct 02, 2024, Ashish Kalra wrote:
> >> Yes, but there is going to be a separate set of patches to move all ASID
> >> handling code to CCP module.
> >>
> >> This refactoring won't be part of the SNP ciphertext hiding support patches.
> > 
> > It should, because that's not a "refactoring", that's a change of roles and
> > responsibilities.  And this series does the same; even worse, this series leaves
> > things in a half-baked state, where the CCP and KVM have a weird shared ownership
> > of ASID management.
> 
> Sorry for the delayed reply to your response, the SNP DOWNLOAD_FIRMWARE_EX
> patches got posted in the meanwhile and that had additional considerations of
> moving SNP GCTX pages stuff into the PSP driver from KVM and that again got
> into this discussion about splitting ASID management across KVM and PSP
> driver and as you pointed out on those patches that there is zero reason that
> the PSP driver needs to care about ASIDs. 
> 
> Well, CipherText Hiding (CTH) support is one reason where the PSP driver gets
> involved with ASIDs as CTH feature has to be enabled as part of SNP_INIT_EX
> and once CTH feature is enabled, the SEV-ES ASID space is split across
> SEV-SNP and SEV-ES VMs. 

Right, but that's just a case where KVM needs to react to the setup done by the
PSP, correct?  E.g. it's similar to SEV-ES being enabled/disabled in firmware,
only that "firmware" happens to be a kernel driver.

> With reference to SNP GCTX pages, we are looking at some possibilities to
> push the requirement to update SNP GCTX pages to SNP firmware and remove that
> requirement from the kernel/KVM side.

Heh, that'd work too.

> Considering that, I will still like to keep ASID management in KVM, there are
> issues with locking, for example, sev_deactivate_lock is used to protect SNP
> ASID allocations (or actually for protecting ASID reuse/lazy-allocation
> requiring WBINVD/DF_FLUSH) and guarding this DF_FLUSH from VM destruction
> (DEACTIVATE). Moving ASID management stuff into PSP driver will then add
> complexity of adding this synchronization between different kernel modules or
> handling locking in two different kernel modules, to guard ASID allocation in
> PSP driver with VM destruction in KVM module.
> 
> There is also this sev_vmcbs[] array indexed by ASID (part of svm_cpu_data)
> which gets referenced during the ASID free code path in KVM. It just makes it
> simpler to keep ASID management stuff in KVM. 
> 
> So probably we can add an API interface exported by the PSP driver something
> like is_sev_ciphertext_hiding_enabled() or sev_override_max_snp_asid()

What about adding a cc_attr_flags entry?