Re: [RFC PATCH v2 06/22] KVM: X86: Define tsm_get_vmid

[Alexey Kardashevskiy <aik@xxxxxxx>]

On 13/3/25 12:51, Dan Williams wrote:
> Alexey Kardashevskiy wrote:
> > In order to add a PCI VF into a secure VM, the TSM module needs to
> > perform a "TDI bind" operation. The secure module ("PSP" for AMD)
> > reuqires a VM id to associate with a VM and KVM has it. Since
> > KVM cannot directly bind a TDI (as it does not have all necesessary
> > data such as host/guest PCI BDFn). QEMU and IOMMUFD do know the BDFns
> > but they do not have a VM id recognisable by the PSP.
> >
> > Add get_vmid() hook to KVM. Implement it for AMD SEV to return a sum
> > of GCTX (a private page describing secure VM context) and ASID
> > (required on unbind for IOMMU unfencing, when needed).
> >
> > Signed-off-by: Alexey Kardashevskiy <aik@xxxxxxx>
> > ---
> >   arch/x86/include/asm/kvm-x86-ops.h |  1 +
> >   arch/x86/include/asm/kvm_host.h    |  2 ++
> >   include/linux/kvm_host.h           |  2 ++
> >   arch/x86/kvm/svm/svm.c             | 12 ++++++++++++
> >   virt/kvm/kvm_main.c                |  6 ++++++
> >   5 files changed, 23 insertions(+)
> >
> > diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
> > index c35550581da0..63102a224cd7 100644
> > --- a/arch/x86/include/asm/kvm-x86-ops.h
> > +++ b/arch/x86/include/asm/kvm-x86-ops.h
> > @@ -144,6 +144,7 @@ KVM_X86_OP_OPTIONAL(alloc_apic_backing_page)
> >   KVM_X86_OP_OPTIONAL_RET0(gmem_prepare)
> >   KVM_X86_OP_OPTIONAL_RET0(private_max_mapping_level)
> >   KVM_X86_OP_OPTIONAL(gmem_invalidate)
> > +KVM_X86_OP_OPTIONAL(tsm_get_vmid)
> >   
> >   #undef KVM_X86_OP
> >   #undef KVM_X86_OP_OPTIONAL
> > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > index b15cde0a9b5c..9330e8d4d29d 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -1875,6 +1875,8 @@ struct kvm_x86_ops {
> >   	int (*gmem_prepare)(struct kvm *kvm, kvm_pfn_t pfn, gfn_t gfn, int max_order);
> >   	void (*gmem_invalidate)(kvm_pfn_t start, kvm_pfn_t end);
> >   	int (*private_max_mapping_level)(struct kvm *kvm, kvm_pfn_t pfn);
> > +
> > +	u64 (*tsm_get_vmid)(struct kvm *kvm);
> >   };
> >   
> >   struct kvm_x86_nested_ops {
> > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> > index f34f4cfaa513..6cd351edb956 100644
> > --- a/include/linux/kvm_host.h
> > +++ b/include/linux/kvm_host.h
> > @@ -2571,4 +2571,6 @@ long kvm_arch_vcpu_pre_fault_memory(struct kvm_vcpu *vcpu,
> >   				    struct kvm_pre_fault_memory *range);
> >   #endif
> >   
> > +u64 kvm_arch_tsm_get_vmid(struct kvm *kvm);
> > +
> >   #endif
> > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> > index 7640a84e554a..0276d60c61d6 100644
> > --- a/arch/x86/kvm/svm/svm.c
> > +++ b/arch/x86/kvm/svm/svm.c
> > @@ -4998,6 +4998,16 @@ static void *svm_alloc_apic_backing_page(struct kvm_vcpu *vcpu)
> >   	return page_address(page);
> >   }
> >   
> > +static u64 svm_tsm_get_vmid(struct kvm *kvm)
> > +{
> > +	struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
> > +
> > +	if (!sev->es_active)
> > +		return 0;
> > +
> > +	return ((u64) sev->snp_context) | sev->asid;
> > +}
> > +
>
> Curious why KVM needs to be bothered by a new kvm_arch_tsm_get_vmid()
> and a vendor specific cookie "vmid" concept. In other words KVM never
> calls kvm_arch_tsm_get_vmid(), like other kvm_arch_*() support calls.
>
> Is this due to a restriction that something like tsm_tdi_bind() is
> disallowed from doing to_kvm_svm() on an opaque @kvm pointer? Or
> otherwise asking an arch/x86/kvm/svm/svm.c to do the same?

I saw someone already doing some sort of VMID thing and thought it is a 
good way of not spilling KVM details outside KVM.

> Effectively low level TSM drivers are extensions of arch code that
> routinely performs "container_of(kvm, struct kvm_$arch, kvm)".

The arch code is CCP and so far it avoided touching KVM, KVM calls CCP 
when it needs but not vice versa. Thanks,


--
Alexey