2024-03-18 23:34:47

by Paolo Bonzini

[permalink] [raw]
Subject: [PATCH v4 05/15] KVM: SEV: publish supported VMSA features

Compute the set of features to be stored in the VMSA when KVM is
initialized; move it from there into kvm_sev_info when SEV is initialized,
and then into the initial VMSA.

The new variable can then be used to return the set of supported features
to userspace, via the KVM_GET_DEVICE_ATTR ioctl.

Signed-off-by: Paolo Bonzini <[email protected]>
---
.../virt/kvm/x86/amd-memory-encryption.rst | 12 +++++++++++
arch/x86/include/uapi/asm/kvm.h | 1 +
arch/x86/kvm/svm/sev.c | 21 +++++++++++++++++--
arch/x86/kvm/svm/svm.c | 1 +
arch/x86/kvm/svm/svm.h | 2 ++
5 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/Documentation/virt/kvm/x86/amd-memory-encryption.rst b/Documentation/virt/kvm/x86/amd-memory-encryption.rst
index 84335d119ff1..fb41470c0310 100644
--- a/Documentation/virt/kvm/x86/amd-memory-encryption.rst
+++ b/Documentation/virt/kvm/x86/amd-memory-encryption.rst
@@ -425,6 +425,18 @@ issued by the hypervisor to make the guest ready for execution.

Returns: 0 on success, -negative on error

+Device attribute API
+====================
+
+Attributes of the SEV implementation can be retrieved through the
+``KVM_HAS_DEVICE_ATTR`` and ``KVM_GET_DEVICE_ATTR`` ioctls on the ``/dev/kvm``
+device node.
+
+Currently only one attribute is implemented:
+
+* group 0, attribute ``KVM_X86_SEV_VMSA_FEATURES``: return the set of all
+ bits that are accepted in the ``vmsa_features`` of ``KVM_SEV_INIT2``.
+
Firmware Management
===================

diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index ef11aa4cab42..d0c1b459f7e9 100644
--- a/arch/x86/include/uapi/asm/kvm.h
+++ b/arch/x86/include/uapi/asm/kvm.h
@@ -459,6 +459,7 @@ struct kvm_sync_regs {

/* attributes for system fd (group 0) */
#define KVM_X86_XCOMP_GUEST_SUPP 0
+#define KVM_X86_SEV_VMSA_FEATURES 1

struct kvm_vmx_nested_state_data {
__u8 vmcs12[KVM_STATE_NESTED_VMX_VMCS_SIZE];
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 73fee5f08391..22c35a39c25f 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -43,6 +43,7 @@ module_param_named(sev_es, sev_es_enabled, bool, 0444);
/* enable/disable SEV-ES DebugSwap support */
static bool sev_es_debug_swap_enabled = false;
module_param_named(debug_swap, sev_es_debug_swap_enabled, bool, 0444);
+static u64 sev_supported_vmsa_features;

static u8 sev_enc_bit;
static DECLARE_RWSEM(sev_deactivate_lock);
@@ -600,8 +601,8 @@ static int sev_es_sync_vmsa(struct vcpu_svm *svm)
save->xss = svm->vcpu.arch.ia32_xss;
save->dr6 = svm->vcpu.arch.dr6;

- if (sev_es_debug_swap_enabled) {
- save->sev_features |= SVM_SEV_FEAT_DEBUG_SWAP;
+ if (sev_supported_vmsa_features) {
+ save->sev_features = sev_supported_vmsa_features;
pr_warn_once("Enabling DebugSwap with KVM_SEV_ES_INIT. "
"This will not work starting with Linux 6.10\n");
}
@@ -1840,6 +1841,18 @@ int sev_vm_move_enc_context_from(struct kvm *kvm, unsigned int source_fd)
return ret;
}

+int sev_dev_get_attr(u64 attr, u64 *val)
+{
+ switch (attr) {
+ case KVM_X86_SEV_VMSA_FEATURES:
+ *val = sev_supported_vmsa_features;
+ return 0;
+
+ default:
+ return -ENXIO;
+ }
+}
+
int sev_mem_enc_ioctl(struct kvm *kvm, void __user *argp)
{
struct kvm_sev_cmd sev_cmd;
@@ -2272,6 +2285,10 @@ void __init sev_hardware_setup(void)
if (!sev_es_enabled || !cpu_feature_enabled(X86_FEATURE_DEBUG_SWAP) ||
!cpu_feature_enabled(X86_FEATURE_NO_NESTED_DATA_BP))
sev_es_debug_swap_enabled = false;
+
+ sev_supported_vmsa_features = 0;
+ if (sev_es_debug_swap_enabled)
+ sev_supported_vmsa_features |= SVM_SEV_FEAT_DEBUG_SWAP;
}

void sev_hardware_unsetup(void)
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index e7f47a1f3eb1..450535d6757f 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -5026,6 +5026,7 @@ static struct kvm_x86_ops svm_x86_ops __initdata = {
#endif

#ifdef CONFIG_KVM_AMD_SEV
+ .dev_get_attr = sev_dev_get_attr,
.mem_enc_ioctl = sev_mem_enc_ioctl,
.mem_enc_register_region = sev_mem_enc_register_region,
.mem_enc_unregister_region = sev_mem_enc_unregister_region,
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index d20e48c31210..864fac367424 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -695,6 +695,7 @@ void __init sev_set_cpu_caps(void);
void __init sev_hardware_setup(void);
void sev_hardware_unsetup(void);
int sev_cpu_init(struct svm_cpu_data *sd);
+int sev_dev_get_attr(u64 attr, u64 *val);
extern unsigned int max_sev_asid;
#else
static inline struct page *snp_safe_alloc_page(struct kvm_vcpu *vcpu) {
@@ -707,6 +708,7 @@ static inline void __init sev_set_cpu_caps(void) {}
static inline void __init sev_hardware_setup(void) {}
static inline void sev_hardware_unsetup(void) {}
static inline int sev_cpu_init(struct svm_cpu_data *sd) { return 0; }
+static inline int sev_dev_get_attr(u64 attr, u64 *val) { return -ENXIO; }
#define max_sev_asid 0
#endif

--
2.43.0




2024-03-25 23:59:34

by Isaku Yamahata

[permalink] [raw]
Subject: Re: [PATCH v4 05/15] KVM: SEV: publish supported VMSA features

On Mon, Mar 18, 2024 at 07:33:42PM -0400,
Paolo Bonzini <[email protected]> wrote:

> Compute the set of features to be stored in the VMSA when KVM is
> initialized; move it from there into kvm_sev_info when SEV is initialized,
> and then into the initial VMSA.
>
> The new variable can then be used to return the set of supported features
> to userspace, via the KVM_GET_DEVICE_ATTR ioctl.

Hi. The current TDX KVM introduces KVM_TDX_CAPABILITIES and struct
kvm_tdx_capabilities for feature enumeration. I'm wondering if TDX should also
use/switch to KVM_GET_DEVICE_ATTR with its own group. What do you think?
Something like

#define KVM_DEVICE_ATTR_GROUP_SEV 1
#define KVM_X86_SEV_VMSA_FEATURES 1
#define KVM_X86_SEV_xxx ...

#define KVM_DEVICE_ATTR_GROUP_TDX 2
#define KVM_X86_TDX_xxx ...

Thanks,
--
Isaku Yamahata <[email protected]>

2024-04-04 11:46:34

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH v4 05/15] KVM: SEV: publish supported VMSA features

On Tue, Mar 26, 2024 at 1:04 AM Isaku Yamahata <[email protected]> wrote:
>
> On Mon, Mar 18, 2024 at 07:33:42PM -0400,
> Paolo Bonzini <[email protected]> wrote:
>
> > Compute the set of features to be stored in the VMSA when KVM is
> > initialized; move it from there into kvm_sev_info when SEV is initialized,
> > and then into the initial VMSA.
> >
> > The new variable can then be used to return the set of supported features
> > to userspace, via the KVM_GET_DEVICE_ATTR ioctl.
>
> Hi. The current TDX KVM introduces KVM_TDX_CAPABILITIES and struct
> kvm_tdx_capabilities for feature enumeration. I'm wondering if TDX should also
> use/switch to KVM_GET_DEVICE_ATTR with its own group. What do you think?
> Something like
>
> #define KVM_DEVICE_ATTR_GROUP_SEV 1
> #define KVM_X86_SEV_VMSA_FEATURES 1
> #define KVM_X86_SEV_xxx ...
>
> #define KVM_DEVICE_ATTR_GROUP_TDX 2
> #define KVM_X86_TDX_xxx ...

Yes, that's a very good idea. I've added the group argument in v5.

Paolo