This is the same as v4, except for the following minor changes:
- moving the KVM_X86_SEV_VMSA_FEATURES attribute to a
separate group, KVM_X86_GRP_SEV [Isaku]
- as part of the previous change, retroactively define group 0
as "KVM_X86_GRP_SYSTEM"
- squashing in the "fixup! KVM: SEV: sync FPU and AVX state at
LAUNCH_UPDATE_VMSA time" patch
- disabling FPU and AVX sync for the old-style KVM_SEV_ES_INIT
ioctl [Michael]
- adding an fstp instruction to the new test case, in order to
keep the x87 stack balanced (just for cleanliness/paranoia)
Paolo Bonzini (16):
KVM: SVM: Compile sev.c if and only if CONFIG_KVM_AMD_SEV=y
KVM: x86: use u64_to_user_ptr()
KVM: introduce new vendor op for KVM_GET_DEVICE_ATTR
KVM: SEV: publish supported VMSA features
KVM: SEV: store VMSA features in kvm_sev_info
KVM: x86: add fields to struct kvm_arch for CoCo features
KVM: x86: Add supported_vm_types to kvm_caps
KVM: SEV: introduce to_kvm_sev_info
KVM: SEV: define VM types for SEV and SEV-ES
KVM: SEV: sync FPU and AVX state at LAUNCH_UPDATE_VMSA time
KVM: SEV: introduce KVM_SEV_INIT2 operation
KVM: SEV: allow SEV-ES DebugSwap again
selftests: kvm: add tests for KVM_SEV_INIT2
selftests: kvm: switch to using KVM_X86_*_VM
selftests: kvm: split "launch" phase of SEV VM creation
selftests: kvm: add test for transferring FPU state into VMSA
Sean Christopherson (1):
KVM: SVM: Invert handling of SEV and SEV_ES feature flags
Documentation/virt/kvm/api.rst | 2 +
.../virt/kvm/x86/amd-memory-encryption.rst | 52 ++++-
arch/x86/include/asm/fpu/api.h | 3 +
arch/x86/include/asm/kvm-x86-ops.h | 1 +
arch/x86/include/asm/kvm_host.h | 8 +-
arch/x86/include/uapi/asm/kvm.h | 20 +-
arch/x86/kernel/fpu/xstate.c | 1 +
arch/x86/kernel/fpu/xstate.h | 2 -
arch/x86/kvm/Makefile | 7 +-
arch/x86/kvm/cpuid.c | 2 +-
arch/x86/kvm/svm/sev.c | 190 ++++++++++++++----
arch/x86/kvm/svm/svm.c | 27 ++-
arch/x86/kvm/svm/svm.h | 54 +++--
arch/x86/kvm/x86.c | 165 +++++++++------
arch/x86/kvm/x86.h | 2 +
tools/testing/selftests/kvm/Makefile | 1 +
.../selftests/kvm/include/kvm_util_base.h | 11 +-
.../selftests/kvm/include/x86_64/processor.h | 6 -
.../selftests/kvm/include/x86_64/sev.h | 19 +-
tools/testing/selftests/kvm/lib/kvm_util.c | 1 -
.../selftests/kvm/lib/x86_64/processor.c | 14 +-
tools/testing/selftests/kvm/lib/x86_64/sev.c | 44 +++-
.../selftests/kvm/set_memory_region_test.c | 8 +-
.../selftests/kvm/x86_64/sev_init2_tests.c | 152 ++++++++++++++
.../selftests/kvm/x86_64/sev_smoke_test.c | 96 ++++++++-
25 files changed, 703 insertions(+), 185 deletions(-)
create mode 100644 tools/testing/selftests/kvm/x86_64/sev_init2_tests.c
--
2.43.0
This simplifies the implementation of KVM_CHECK_EXTENSION(KVM_CAP_VM_TYPES),
and also allows the vendor module to specify which VM types are supported.
Suggested-by: Sean Christopherson <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
---
arch/x86/kvm/x86.c | 12 ++++++------
arch/x86/kvm/x86.h | 2 ++
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index d4a8d896798f..d584f5739402 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -94,6 +94,7 @@
struct kvm_caps kvm_caps __read_mostly = {
.supported_mce_cap = MCG_CTL_P | MCG_SER_P,
+ .supported_vm_types = BIT(KVM_X86_DEFAULT_VM),
};
EXPORT_SYMBOL_GPL(kvm_caps);
@@ -4629,9 +4630,7 @@ static int kvm_ioctl_get_supported_hv_cpuid(struct kvm_vcpu *vcpu,
static bool kvm_is_vm_type_supported(unsigned long type)
{
- return type == KVM_X86_DEFAULT_VM ||
- (type == KVM_X86_SW_PROTECTED_VM &&
- IS_ENABLED(CONFIG_KVM_SW_PROTECTED_VM) && tdp_mmu_enabled);
+ return type < 32 && (kvm_caps.supported_vm_types & BIT(type));
}
int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
@@ -4832,9 +4831,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
r = kvm_caps.has_notify_vmexit;
break;
case KVM_CAP_VM_TYPES:
- r = BIT(KVM_X86_DEFAULT_VM);
- if (kvm_is_vm_type_supported(KVM_X86_SW_PROTECTED_VM))
- r |= BIT(KVM_X86_SW_PROTECTED_VM);
+ r = kvm_caps.supported_vm_types;
break;
default:
break;
@@ -9824,6 +9821,9 @@ int kvm_x86_vendor_init(struct kvm_x86_init_ops *ops)
kvm_register_perf_callbacks(ops->handle_intel_pt_intr);
+ if (IS_ENABLED(CONFIG_KVM_SW_PROTECTED_VM) && tdp_mmu_enabled)
+ kvm_caps.supported_vm_types |= BIT(KVM_X86_SW_PROTECTED_VM);
+
if (!kvm_cpu_cap_has(X86_FEATURE_XSAVES))
kvm_caps.supported_xss = 0;
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index a8b71803777b..d80a4c6b5a38 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -24,6 +24,8 @@ struct kvm_caps {
bool has_bus_lock_exit;
/* notify VM exit supported? */
bool has_notify_vmexit;
+ /* bit mask of VM types */
+ u32 supported_vm_types;
u64 supported_mce_cap;
u64 supported_xcr0;
--
2.43.0
Allow vendor modules to provide their own attributes on /dev/kvm.
To avoid proliferation of vendor ops, implement KVM_HAS_DEVICE_ATTR
and KVM_GET_DEVICE_ATTR in terms of the same function. You're not
supposed to use KVM_GET_DEVICE_ATTR to do complicated computations,
especially on /dev/kvm.
Reviewed-by: Michael Roth <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
---
arch/x86/include/asm/kvm-x86-ops.h | 1 +
arch/x86/include/asm/kvm_host.h | 1 +
arch/x86/kvm/x86.c | 38 +++++++++++++++++++-----------
3 files changed, 26 insertions(+), 14 deletions(-)
diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
index 110d7f29ca9a..5187fcf4b610 100644
--- a/arch/x86/include/asm/kvm-x86-ops.h
+++ b/arch/x86/include/asm/kvm-x86-ops.h
@@ -121,6 +121,7 @@ KVM_X86_OP(enter_smm)
KVM_X86_OP(leave_smm)
KVM_X86_OP(enable_smi_window)
#endif
+KVM_X86_OP_OPTIONAL(dev_get_attr)
KVM_X86_OP_OPTIONAL(mem_enc_ioctl)
KVM_X86_OP_OPTIONAL(mem_enc_register_region)
KVM_X86_OP_OPTIONAL(mem_enc_unregister_region)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 16e07a2eee19..04c430eb25cf 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1778,6 +1778,7 @@ struct kvm_x86_ops {
void (*enable_smi_window)(struct kvm_vcpu *vcpu);
#endif
+ int (*dev_get_attr)(u32 group, u64 attr, u64 *val);
int (*mem_enc_ioctl)(struct kvm *kvm, void __user *argp);
int (*mem_enc_register_region)(struct kvm *kvm, struct kvm_enc_region *argp);
int (*mem_enc_unregister_region)(struct kvm *kvm, struct kvm_enc_region *argp);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 3d2029402513..3934e7682734 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4842,34 +4842,44 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
return r;
}
-static int kvm_x86_dev_get_attr(struct kvm_device_attr *attr)
+static int __kvm_x86_dev_get_attr(struct kvm_device_attr *attr, u64 *val)
{
- u64 __user *uaddr = u64_to_user_ptr(attr->addr);
-
- if (attr->group)
+ if (attr->group) {
+ if (kvm_x86_ops.dev_get_attr)
+ return static_call(kvm_x86_dev_get_attr)(attr->group, attr->attr, val);
return -ENXIO;
+ }
switch (attr->attr) {
case KVM_X86_XCOMP_GUEST_SUPP:
- if (put_user(kvm_caps.supported_xcr0, uaddr))
- return -EFAULT;
+ *val = kvm_caps.supported_xcr0;
return 0;
default:
return -ENXIO;
}
}
+static int kvm_x86_dev_get_attr(struct kvm_device_attr *attr)
+{
+ u64 __user *uaddr = u64_to_user_ptr(attr->addr);
+ int r;
+ u64 val;
+
+ r = __kvm_x86_dev_get_attr(attr, &val);
+ if (r < 0)
+ return r;
+
+ if (put_user(val, uaddr))
+ return -EFAULT;
+
+ return 0;
+}
+
static int kvm_x86_dev_has_attr(struct kvm_device_attr *attr)
{
- if (attr->group)
- return -ENXIO;
+ u64 val;
- switch (attr->attr) {
- case KVM_X86_XCOMP_GUEST_SUPP:
- return 0;
- default:
- return -ENXIO;
- }
+ return __kvm_x86_dev_get_attr(attr, &val);
}
long kvm_arch_dev_ioctl(struct file *filp,
--
2.43.0
On Thu, Apr 04, 2024 at 08:13:14AM -0400,
Paolo Bonzini <[email protected]> wrote:
> Allow vendor modules to provide their own attributes on /dev/kvm.
> To avoid proliferation of vendor ops, implement KVM_HAS_DEVICE_ATTR
> and KVM_GET_DEVICE_ATTR in terms of the same function. You're not
> supposed to use KVM_GET_DEVICE_ATTR to do complicated computations,
> especially on /dev/kvm.
>
> Reviewed-by: Michael Roth <[email protected]>
> Signed-off-by: Paolo Bonzini <[email protected]>
> ---
> arch/x86/include/asm/kvm-x86-ops.h | 1 +
> arch/x86/include/asm/kvm_host.h | 1 +
> arch/x86/kvm/x86.c | 38 +++++++++++++++++++-----------
> 3 files changed, 26 insertions(+), 14 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
> index 110d7f29ca9a..5187fcf4b610 100644
> --- a/arch/x86/include/asm/kvm-x86-ops.h
> +++ b/arch/x86/include/asm/kvm-x86-ops.h
> @@ -121,6 +121,7 @@ KVM_X86_OP(enter_smm)
> KVM_X86_OP(leave_smm)
> KVM_X86_OP(enable_smi_window)
> #endif
> +KVM_X86_OP_OPTIONAL(dev_get_attr)
> KVM_X86_OP_OPTIONAL(mem_enc_ioctl)
> KVM_X86_OP_OPTIONAL(mem_enc_register_region)
> KVM_X86_OP_OPTIONAL(mem_enc_unregister_region)
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 16e07a2eee19..04c430eb25cf 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1778,6 +1778,7 @@ struct kvm_x86_ops {
> void (*enable_smi_window)(struct kvm_vcpu *vcpu);
> #endif
>
> + int (*dev_get_attr)(u32 group, u64 attr, u64 *val);
> int (*mem_enc_ioctl)(struct kvm *kvm, void __user *argp);
> int (*mem_enc_register_region)(struct kvm *kvm, struct kvm_enc_region *argp);
> int (*mem_enc_unregister_region)(struct kvm *kvm, struct kvm_enc_region *argp);
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 3d2029402513..3934e7682734 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -4842,34 +4842,44 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> return r;
> }
>
> -static int kvm_x86_dev_get_attr(struct kvm_device_attr *attr)
> +static int __kvm_x86_dev_get_attr(struct kvm_device_attr *attr, u64 *val)
> {
> - u64 __user *uaddr = u64_to_user_ptr(attr->addr);
> -
> - if (attr->group)
> + if (attr->group) {
> + if (kvm_x86_ops.dev_get_attr)
> + return static_call(kvm_x86_dev_get_attr)(attr->group, attr->attr, val);
> return -ENXIO;
> + }
>
> switch (attr->attr) {
> case KVM_X86_XCOMP_GUEST_SUPP:
> - if (put_user(kvm_caps.supported_xcr0, uaddr))
> - return -EFAULT;
> + *val = kvm_caps.supported_xcr0;
> return 0;
> default:
> return -ENXIO;
> }
> }
>
> +static int kvm_x86_dev_get_attr(struct kvm_device_attr *attr)
> +{
> + u64 __user *uaddr = u64_to_user_ptr(attr->addr);
> + int r;
> + u64 val;
> +
> + r = __kvm_x86_dev_get_attr(attr, &val);
> + if (r < 0)
> + return r;
> +
> + if (put_user(val, uaddr))
> + return -EFAULT;
> +
> + return 0;
> +}
> +
> static int kvm_x86_dev_has_attr(struct kvm_device_attr *attr)
> {
> - if (attr->group)
> - return -ENXIO;
> + u64 val;
>
> - switch (attr->attr) {
> - case KVM_X86_XCOMP_GUEST_SUPP:
> - return 0;
> - default:
> - return -ENXIO;
> - }
> + return __kvm_x86_dev_get_attr(attr, &val);
> }
>
> long kvm_arch_dev_ioctl(struct file *filp,
> --
> 2.43.0
>
>
>
Reviewed-by: Isaku Yamahata <[email protected]>
--
Isaku Yamahata <[email protected]>