diff mbox series

[RFC,v3,14/59] KVM: x86: Add vm_type to differentiate legacy VMs from protected VMs

Message ID 60a163e818b9101dce94973a2b44662ba3d53f97.1637799475.git.isaku.yamahata@intel.com (mailing list archive)
State New, archived
Headers show
Series KVM: X86: TDX support | expand

Commit Message

Isaku Yamahata Nov. 25, 2021, 12:19 a.m. UTC
From: Sean Christopherson <sean.j.christopherson@intel.com>

Add a capability to effectively allow userspace to query what VM types
are supported by KVM.

Co-developed-by: Xiaoyao Li <xiaoyao.li@intel.com>
Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
---
 arch/x86/include/asm/kvm-x86-ops.h    | 1 +
 arch/x86/include/asm/kvm_host.h       | 2 ++
 arch/x86/include/uapi/asm/kvm.h       | 4 ++++
 arch/x86/kvm/svm/svm.c                | 6 ++++++
 arch/x86/kvm/vmx/vmx.c                | 6 ++++++
 arch/x86/kvm/x86.c                    | 9 ++++++++-
 include/uapi/linux/kvm.h              | 2 ++
 tools/arch/x86/include/uapi/asm/kvm.h | 4 ++++
 tools/include/uapi/linux/kvm.h        | 2 ++
 9 files changed, 35 insertions(+), 1 deletion(-)

Comments

Thomas Gleixner Nov. 25, 2021, 7:08 p.m. UTC | #1
On Wed, Nov 24 2021 at 16:19, isaku yamahata wrote:
> From: Sean Christopherson <sean.j.christopherson@intel.com>
>
> Add a capability to effectively allow userspace to query what VM types
> are supported by KVM.

I really don't see why this has to be named legacy. There are enough
reasonable use cases which are perfectly fine using the non-encrypted
muck. Just because there is a new hyped feature does not make anything
else legacy.

Thanks,

        tglx
Sean Christopherson Nov. 29, 2021, 5:35 p.m. UTC | #2
On Thu, Nov 25, 2021, Thomas Gleixner wrote:
> On Wed, Nov 24 2021 at 16:19, isaku yamahata wrote:
> > From: Sean Christopherson <sean.j.christopherson@intel.com>
> >
> > Add a capability to effectively allow userspace to query what VM types
> > are supported by KVM.
> 
> I really don't see why this has to be named legacy. There are enough
> reasonable use cases which are perfectly fine using the non-encrypted
> muck. Just because there is a new hyped feature does not make anything
> else legacy.

Yeah, this was brought up in the past.  The current proposal is to use
KVM_X86_DEFAULT_VM[1], though at one point the plan was to use a generic
KVM_VM_TYPE_DEFAULT for all architectures[2], not sure what happened to that idea.

[1] https://lore.kernel.org/all/YY6aqVkHNEfEp990@google.com/
[2] https://lore.kernel.org/all/YQsjQ5aJokV1HZ8N@google.com/
Isaku Yamahata Dec. 1, 2021, 7:37 p.m. UTC | #3
On Mon, Nov 29, 2021 at 05:35:34PM +0000,
Sean Christopherson <seanjc@google.com> wrote:

> On Thu, Nov 25, 2021, Thomas Gleixner wrote:
> > On Wed, Nov 24 2021 at 16:19, isaku yamahata wrote:
> > > From: Sean Christopherson <sean.j.christopherson@intel.com>
> > >
> > > Add a capability to effectively allow userspace to query what VM types
> > > are supported by KVM.
> > 
> > I really don't see why this has to be named legacy. There are enough
> > reasonable use cases which are perfectly fine using the non-encrypted
> > muck. Just because there is a new hyped feature does not make anything
> > else legacy.
> 
> Yeah, this was brought up in the past.  The current proposal is to use
> KVM_X86_DEFAULT_VM[1], though at one point the plan was to use a generic
> KVM_VM_TYPE_DEFAULT for all architectures[2], not sure what happened to that idea.
> 
> [1] https://lore.kernel.org/all/YY6aqVkHNEfEp990@google.com/
> [2] https://lore.kernel.org/all/YQsjQ5aJokV1HZ8N@google.com/

Currently <feature>_{unsupported, disallowed} are added and the check is
 sprinkled and warn in the corresponding low level tdx code.  It helped to
 detect dubious behavior of guest or qemu.

The other approach is to silently ignore them (SMI, INIT, IRQ etc) without
such check.  The pros is, the code would be simpler and it's what SEV does today.
the cons is, it would bes hard to track down such cases and the user would
be confused.  For example, when user requests reset/SMI, it's silently ignored.
The some check would still be needed.
Any thoughts?
Sean Christopherson Dec. 3, 2021, 4:14 p.m. UTC | #4
On Wed, Dec 01, 2021, Isaku Yamahata wrote:
> On Mon, Nov 29, 2021 at 05:35:34PM +0000,
> Sean Christopherson <seanjc@google.com> wrote:
> 
> > On Thu, Nov 25, 2021, Thomas Gleixner wrote:
> > > On Wed, Nov 24 2021 at 16:19, isaku yamahata wrote:
> > > > From: Sean Christopherson <sean.j.christopherson@intel.com>
> > > >
> > > > Add a capability to effectively allow userspace to query what VM types
> > > > are supported by KVM.
> > > 
> > > I really don't see why this has to be named legacy. There are enough
> > > reasonable use cases which are perfectly fine using the non-encrypted
> > > muck. Just because there is a new hyped feature does not make anything
> > > else legacy.
> > 
> > Yeah, this was brought up in the past.  The current proposal is to use
> > KVM_X86_DEFAULT_VM[1], though at one point the plan was to use a generic
> > KVM_VM_TYPE_DEFAULT for all architectures[2], not sure what happened to that idea.
> > 
> > [1] https://lore.kernel.org/all/YY6aqVkHNEfEp990@google.com/
> > [2] https://lore.kernel.org/all/YQsjQ5aJokV1HZ8N@google.com/
> 
> Currently <feature>_{unsupported, disallowed} are added and the check is
>  sprinkled and warn in the corresponding low level tdx code.  It helped to
>  detect dubious behavior of guest or qemu.

KVM shouldn't log a message or WARN unless the issue is detected at a late sanity
check, i.e. where failure indicates a KVM bug.  Other than that, I agree that KVM
should reject ioctls() that directly violate the rules of a confidential VM with
an appropriate error code.  I don't think KVM should reject everything though,
e.g. if the guest attempts to send an SMI, dropping the request on the floor is
the least awful option because we can't communicate an error to the guest without
making up our own architecture, and exiting to userspace with -EINVAL from deep
in KVM would be both painful to implement and an overreaction since doing so would
likely kill the guest.

> The other approach is to silently ignore them (SMI, INIT, IRQ etc) without
> such check.  The pros is, the code would be simpler and it's what SEV does today.
> the cons is, it would bes hard to track down such cases and the user would
> be confused.  For example, when user requests reset/SMI, it's silently ignored.
> The some check would still be needed.
> Any thoughts?
> 
> -- 
> Isaku Yamahata <isaku.yamahata@gmail.com>
diff mbox series

Patch

diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
index cefe1d81e2e8..4aa0a1c10b84 100644
--- a/arch/x86/include/asm/kvm-x86-ops.h
+++ b/arch/x86/include/asm/kvm-x86-ops.h
@@ -18,6 +18,7 @@  KVM_X86_OP_NULL(hardware_unsetup)
 KVM_X86_OP_NULL(cpu_has_accelerated_tpr)
 KVM_X86_OP(has_emulated_msr)
 KVM_X86_OP(vcpu_after_set_cpuid)
+KVM_X86_OP(is_vm_type_supported)
 KVM_X86_OP(vm_init)
 KVM_X86_OP_NULL(vm_destroy)
 KVM_X86_OP(vcpu_create)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 5b31d8e5fcbf..cdb908ed7d5b 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1037,6 +1037,7 @@  struct kvm_x86_msr_filter {
 #define APICV_INHIBIT_REASON_BLOCKIRQ	6
 
 struct kvm_arch {
+	unsigned long vm_type;
 	unsigned long n_used_mmu_pages;
 	unsigned long n_requested_mmu_pages;
 	unsigned long n_max_mmu_pages;
@@ -1311,6 +1312,7 @@  struct kvm_x86_ops {
 	bool (*has_emulated_msr)(struct kvm *kvm, u32 index);
 	void (*vcpu_after_set_cpuid)(struct kvm_vcpu *vcpu);
 
+	bool (*is_vm_type_supported)(unsigned long vm_type);
 	unsigned int vm_size;
 	int (*vm_init)(struct kvm *kvm);
 	void (*vm_destroy)(struct kvm *kvm);
diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index 5a776a08f78c..a0805a2a81f8 100644
--- a/arch/x86/include/uapi/asm/kvm.h
+++ b/arch/x86/include/uapi/asm/kvm.h
@@ -508,4 +508,8 @@  struct kvm_pmu_event_filter {
 #define KVM_VCPU_TSC_CTRL 0 /* control group for the timestamp counter (TSC) */
 #define   KVM_VCPU_TSC_OFFSET 0 /* attribute for the TSC offset */
 
+#define KVM_X86_LEGACY_VM	0
+#define KVM_X86_SEV_ES_VM	1
+#define KVM_X86_TDX_VM		2
+
 #endif /* _ASM_X86_KVM_H */
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 5630c241d5f6..2ef77d4566a9 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -4562,6 +4562,11 @@  static void svm_vm_destroy(struct kvm *kvm)
 	sev_vm_destroy(kvm);
 }
 
+static bool svm_is_vm_type_supported(unsigned long type)
+{
+	return type == KVM_X86_LEGACY_VM;
+}
+
 static int svm_vm_init(struct kvm *kvm)
 {
 	if (!pause_filter_count || !pause_filter_thresh)
@@ -4589,6 +4594,7 @@  static struct kvm_x86_ops svm_x86_ops __initdata = {
 	.vcpu_free = svm_free_vcpu,
 	.vcpu_reset = svm_vcpu_reset,
 
+	.is_vm_type_supported = svm_is_vm_type_supported,
 	.vm_size = sizeof(struct kvm_svm),
 	.vm_init = svm_vm_init,
 	.vm_destroy = svm_vm_destroy,
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index fe4f3ee61db3..f2905e00b063 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -6851,6 +6851,11 @@  static int vmx_create_vcpu(struct kvm_vcpu *vcpu)
 	return err;
 }
 
+static bool vmx_is_vm_type_supported(unsigned long type)
+{
+	return type == KVM_X86_LEGACY_VM;
+}
+
 #define L1TF_MSG_SMT "L1TF CPU bug present and SMT on, data leak possible. See CVE-2018-3646 and https://www.kernel.org/doc/html/latest/admin-guide/hw-vuln/l1tf.html for details.\n"
 #define L1TF_MSG_L1D "L1TF CPU bug present and virtualization mitigation disabled, data leak possible. See CVE-2018-3646 and https://www.kernel.org/doc/html/latest/admin-guide/hw-vuln/l1tf.html for details.\n"
 
@@ -7505,6 +7510,7 @@  static struct kvm_x86_ops vmx_x86_ops __initdata = {
 	.cpu_has_accelerated_tpr = report_flexpriority,
 	.has_emulated_msr = vmx_has_emulated_msr,
 
+	.is_vm_type_supported = vmx_is_vm_type_supported,
 	.vm_size = sizeof(struct kvm_vmx),
 	.vm_init = vmx_vm_init,
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 6f3c77524426..5f4b6f70489b 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4225,6 +4225,11 @@  int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 		else
 			r = 0;
 		break;
+	case KVM_CAP_VM_TYPES:
+		r = BIT(KVM_X86_LEGACY_VM);
+		if (static_call(kvm_x86_is_vm_type_supported)(KVM_X86_TDX_VM))
+			r |= BIT(KVM_X86_TDX_VM);
+		break;
 	default:
 		break;
 	}
@@ -11320,9 +11325,11 @@  int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 	int ret;
 	unsigned long flags;
 
-	if (type)
+	if (!static_call(kvm_x86_is_vm_type_supported)(type))
 		return -EINVAL;
 
+	kvm->arch.vm_type = type;
+
 	ret = kvm_page_track_init(kvm);
 	if (ret)
 		return ret;
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 1daa45268de2..bb49e095867e 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1132,6 +1132,8 @@  struct kvm_ppc_resize_hpt {
 #define KVM_CAP_ARM_MTE 205
 #define KVM_CAP_VM_MOVE_ENC_CONTEXT_FROM 206
 
+#define KVM_CAP_VM_TYPES 1000
+
 #ifdef KVM_CAP_IRQ_ROUTING
 
 struct kvm_irq_routing_irqchip {
diff --git a/tools/arch/x86/include/uapi/asm/kvm.h b/tools/arch/x86/include/uapi/asm/kvm.h
index 2ef1f6513c68..96b0064cff5c 100644
--- a/tools/arch/x86/include/uapi/asm/kvm.h
+++ b/tools/arch/x86/include/uapi/asm/kvm.h
@@ -504,4 +504,8 @@  struct kvm_pmu_event_filter {
 #define KVM_PMU_EVENT_ALLOW 0
 #define KVM_PMU_EVENT_DENY 1
 
+#define KVM_X86_LEGACY_VM	0
+#define KVM_X86_SEV_ES_VM	1
+#define KVM_X86_TDX_VM		2
+
 #endif /* _ASM_X86_KVM_H */
diff --git a/tools/include/uapi/linux/kvm.h b/tools/include/uapi/linux/kvm.h
index a067410ebea5..e5aadad54ced 100644
--- a/tools/include/uapi/linux/kvm.h
+++ b/tools/include/uapi/linux/kvm.h
@@ -1113,6 +1113,8 @@  struct kvm_ppc_resize_hpt {
 #define KVM_CAP_EXIT_ON_EMULATION_FAILURE 204
 #define KVM_CAP_ARM_MTE 205
 
+#define KVM_CAP_VM_TYPES 1000
+
 #ifdef KVM_CAP_IRQ_ROUTING
 
 struct kvm_irq_routing_irqchip {