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?
