| Message ID | 20240223104009.632194-5-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: SEV: allow customizing VMSA features | expand |
On Fri, Feb 23, 2024, Paolo Bonzini 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. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > Message-Id: <20240209183743.22030-5-pbonzini@redhat.com> Maybe in v3 we'll find out whether or not you can triple-stamp a double-stamp :-) > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c > index f760106c31f8..53e958805ab9 100644 > --- a/arch/x86/kvm/svm/sev.c > +++ b/arch/x86/kvm/svm/sev.c > @@ -59,10 +59,12 @@ module_param_named(sev_es, sev_es_enabled, bool, 0444); > /* enable/disable SEV-ES DebugSwap support */ > static bool sev_es_debug_swap_enabled = true; > module_param_named(debug_swap, sev_es_debug_swap_enabled, bool, 0444); > +static u64 sev_supported_vmsa_features; > #else > #define sev_enabled false > #define sev_es_enabled false > #define sev_es_debug_swap_enabled false > +#define sev_supported_vmsa_features 0 Ok, I've reached my breaking point. Compiling sev.c for CONFIG_KVM_AMD_SEV=n is getting untenable. Splattering #ifdefs _inside_ SEV specific functions is weird and confusing. And unless dead code elimination isn't as effective as I think it is, we don't even need any stuba since sev_guest() and sev_es_guest() are __always_inline specifically so that useless code can be elided. Or if we want to avoid use of IS_ENABLED(), we could add four stubs, which is still well worth it. Note, I also have a separate series that I will post today (I hope) that gives __svm_sev_es_vcpu_run() similar treatment (the 32-bit "support" in assembly is all kinds of stupid). Attached patches are compile-tested only, though I'll try to take them for a spin on hardware later today.
On 2/23/24 17:07, Sean Christopherson wrote: > And unless dead code elimination isn't as effective as I think it is, > we don't even need any stubs since sev_guest() and sev_es_guest() > are __always_inline specifically so that useless code can be elided. > Or if we want to avoid use of IS_ENABLED(), we could add four stubs, > which is still well worth it. This particular #ifdef was needed to avoid a compilation failure, but I'll check your patches and include them. Paolo
On 2/23/24 17:07, Sean Christopherson wrote: >> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c >> index f760106c31f8..53e958805ab9 100644 >> --- a/arch/x86/kvm/svm/sev.c >> +++ b/arch/x86/kvm/svm/sev.c >> @@ -59,10 +59,12 @@ module_param_named(sev_es, sev_es_enabled, bool, 0444); >> /* enable/disable SEV-ES DebugSwap support */ >> static bool sev_es_debug_swap_enabled = true; >> module_param_named(debug_swap, sev_es_debug_swap_enabled, bool, 0444); >> +static u64 sev_supported_vmsa_features; >> #else >> #define sev_enabled false >> #define sev_es_enabled false >> #define sev_es_debug_swap_enabled false >> +#define sev_supported_vmsa_features 0 > > Ok, I've reached my breaking point. Compiling sev.c for CONFIG_KVM_AMD_SEV=n is > getting untenable. Splattering #ifdefs _inside_ SEV specific functions is weird > and confusing. Ok, I think in some cases I prefer stubs but I'll weave your 4 patches in v3. Paolo > And unless dead code elimination isn't as effective as I think it is, we don't > even need any stuba since sev_guest() and sev_es_guest() are __always_inline > specifically so that useless code can be elided. Or if we want to avoid use of > IS_ENABLED(), we could add four stubs, which is still well worth it. > > Note, I also have a separate series that I will post today (I hope) that gives > __svm_sev_es_vcpu_run() similar treatment (the 32-bit "support" in assembly is > all kinds of stupid). > > Attached patches are compile-tested only, though I'll try to take them for a spin > on hardware later today.
On Fri, Feb 23, 2024, Paolo Bonzini wrote: > On 2/23/24 17:07, Sean Christopherson wrote: > > > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c > > > index f760106c31f8..53e958805ab9 100644 > > > --- a/arch/x86/kvm/svm/sev.c > > > +++ b/arch/x86/kvm/svm/sev.c > > > @@ -59,10 +59,12 @@ module_param_named(sev_es, sev_es_enabled, bool, 0444); > > > /* enable/disable SEV-ES DebugSwap support */ > > > static bool sev_es_debug_swap_enabled = true; > > > module_param_named(debug_swap, sev_es_debug_swap_enabled, bool, 0444); > > > +static u64 sev_supported_vmsa_features; > > > #else > > > #define sev_enabled false > > > #define sev_es_enabled false > > > #define sev_es_debug_swap_enabled false > > > +#define sev_supported_vmsa_features 0 > > > > Ok, I've reached my breaking point. Compiling sev.c for CONFIG_KVM_AMD_SEV=n is > > getting untenable. Splattering #ifdefs _inside_ SEV specific functions is weird > > and confusing. > > Ok, I think in some cases I prefer stubs but I'll weave your 4 patches in > v3. No problem, I don't have a strong preference. I initially added stubs instead of the IS_ENABLED(). The main reason I switched is when I realized that sev_set_cpu_caps() *cleared* capabilities, and so decided it would be safer to have a separate patch that effectively stubbed out the global SEV calls.
diff --git a/Documentation/virt/kvm/x86/amd-memory-encryption.rst b/Documentation/virt/kvm/x86/amd-memory-encryption.rst index 37c5c37f4f6e..5ed11bc16b96 100644 --- a/Documentation/virt/kvm/x86/amd-memory-encryption.rst +++ b/Documentation/virt/kvm/x86/amd-memory-encryption.rst @@ -424,6 +424,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 b305daff056e..cccaa5ff6d01 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 f760106c31f8..53e958805ab9 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -59,10 +59,12 @@ module_param_named(sev_es, sev_es_enabled, bool, 0444); /* enable/disable SEV-ES DebugSwap support */ static bool sev_es_debug_swap_enabled = true; module_param_named(debug_swap, sev_es_debug_swap_enabled, bool, 0444); +static u64 sev_supported_vmsa_features; #else #define sev_enabled false #define sev_es_enabled false #define sev_es_debug_swap_enabled false +#define sev_supported_vmsa_features 0 #endif /* CONFIG_KVM_AMD_SEV */ static u8 sev_enc_bit; @@ -612,8 +614,7 @@ 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; + save->sev_features = sev_supported_vmsa_features; pr_debug("Virtual Machine Save Area (VMSA):\n"); print_hex_dump_debug("", DUMP_PREFIX_NONE, 16, 1, save, sizeof(*save), false); @@ -1849,6 +1850,20 @@ 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) { +#ifdef CONFIG_KVM_AMD_SEV + case KVM_X86_SEV_VMSA_FEATURES: + *val = sev_supported_vmsa_features; + return 0; +#endif + + default: + return -ENXIO; + } +} + int sev_mem_enc_ioctl(struct kvm *kvm, void __user *argp) { struct kvm_sev_cmd sev_cmd; @@ -2276,6 +2291,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; #endif } diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index e90b429c84f1..aa1792f402ab 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -5014,6 +5014,7 @@ static struct kvm_x86_ops svm_x86_ops __initdata = { .enable_smi_window = svm_enable_smi_window, #endif + .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 8ef95139cd24..d630026b23b0 100644 --- a/arch/x86/kvm/svm/svm.h +++ b/arch/x86/kvm/svm/svm.h @@ -671,6 +671,7 @@ void avic_refresh_virtual_apic_mode(struct kvm_vcpu *vcpu); extern unsigned int max_sev_asid; void sev_vm_destroy(struct kvm *kvm); +int sev_dev_get_attr(u64 attr, u64 *val); int sev_mem_enc_ioctl(struct kvm *kvm, void __user *argp); int sev_mem_enc_register_region(struct kvm *kvm, struct kvm_enc_region *range);