On Fri, Feb 23, 2024 at 5:46 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > On Fri, Feb 23, 2024, Paolo Bonzini wrote: > > Some VM types have characteristics in common; in fact, the only use > > of VM types right now is kvm_arch_has_private_mem and it assumes that > > _all_ VM types have private memory. > > > > So, let the low bits specify the characteristics of the VM type. As > > of we have two special things: whether memory is private, and whether > > guest state is protected. The latter is similar to > > kvm->arch.guest_state_protected, but the latter is only set on a fully > > initialized VM. If both are set, ioctls to set registers will cause > > an error---SEV-ES did not do so, which is a problematic API. > > > > Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx> > > Message-Id: <20240209183743.22030-7-pbonzini@xxxxxxxxxx> > > Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx> > > --- > > arch/x86/include/asm/kvm_host.h | 9 +++- > > arch/x86/kvm/x86.c | 93 +++++++++++++++++++++++++++------ > > 2 files changed, 85 insertions(+), 17 deletions(-) > > > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > > index 0bcd9ae16097..15db2697863c 100644 > > --- a/arch/x86/include/asm/kvm_host.h > > +++ b/arch/x86/include/asm/kvm_host.h > > @@ -2135,8 +2135,15 @@ void kvm_mmu_new_pgd(struct kvm_vcpu *vcpu, gpa_t new_pgd); > > void kvm_configure_mmu(bool enable_tdp, int tdp_forced_root_level, > > int tdp_max_root_level, int tdp_huge_page_level); > > > > + > > +/* Low bits of VM types provide confidential computing capabilities. */ > > +#define __KVM_X86_PRIVATE_MEM_TYPE 1 > > +#define __KVM_X86_PROTECTED_STATE_TYPE 2 > > +#define __KVM_X86_VM_TYPE_FEATURES 3 > > +static_assert((KVM_X86_SW_PROTECTED_VM & __KVM_X86_VM_TYPE_FEATURES) == __KVM_X86_PRIVATE_MEM_TYPE); > > Aliasing bit 0 to KVM_X86_SW_PROTECTED_VM is gross, e.g. this > > #define KVM_X86_DEFAULT_VM 0 > #define KVM_X86_SW_PROTECTED_VM 1 > +#define KVM_X86_SEV_VM 8 > +#define KVM_X86_SEV_ES_VM 10 > > is _super_ confusing and bound to cause problems. > > Oh, good gravy, you're also aliasing __KVM_X86_PROTECTED_STATE_TYPE into SEV_ES_VM. > Curse you and your Rami Code induced decimal-based bitwise shenanigans!!! v1 was less gross but Mike talked me into this one. :) > I don't see any reason to bleed the flags into KVM's ABI. Even shoving the flags > into kvm->arch.vm_type is unncessary. Aha! As is storing vm_type as an "unsigned > long", since (a) it can't ever be bigger than u32, and (b) in practice a u8 will > suffice since as Mike pointed out we're effectively limited to 31 types before > kvm_vm_ioctl_check_extension() starts returning error codes. > > So I vote to skip the aliasing and simply do: Fair enough. Paolo
