| Message ID | 20240209183743.22030-9-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: SEV: allow customizing VMSA features | expand |
On Fri, Feb 09, 2024 at 01:37:40PM -0500, Paolo Bonzini wrote: > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > Documentation/virt/kvm/api.rst | 2 ++ > arch/x86/include/uapi/asm/kvm.h | 2 ++ > arch/x86/kvm/svm/sev.c | 18 +++++++++++++++++- > arch/x86/kvm/svm/svm.c | 11 +++++++++++ > arch/x86/kvm/svm/svm.h | 2 ++ > arch/x86/kvm/x86.c | 4 ++++ > 6 files changed, 38 insertions(+), 1 deletion(-) > > diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst > index 3ec0b7a455a0..bf957bb70e4b 100644 > --- a/Documentation/virt/kvm/api.rst > +++ b/Documentation/virt/kvm/api.rst > @@ -8790,6 +8790,8 @@ means the VM type with value @n is supported. Possible values of @n are:: > > #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 > > 9. Known KVM API problems > ========================= > diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h > index 6c74db23257e..7c46e96cfe62 100644 > --- a/arch/x86/include/uapi/asm/kvm.h > +++ b/arch/x86/include/uapi/asm/kvm.h > @@ -854,5 +854,7 @@ struct kvm_hyperv_eventfd { > > #define KVM_X86_DEFAULT_VM 0 > #define KVM_X86_SW_PROTECTED_VM (KVM_X86_DEFAULT_VM | __KVM_X86_PRIVATE_MEM_TYPE) > +#define KVM_X86_SEV_VM 8 Hmm... would it make sense to decouple the VM types and their associated capabilities? Only bit 2 is left in the lower range after this, and using any bits beyond TDX's bit 4 risks overflowing check_extension ioctl's 32-bit return value. Maybe a separate lookup table instead? > +#define KVM_X86_SEV_ES_VM (KVM_X86_SEV_VM | __KVM_X86_PROTECTED_STATE_TYPE) > > #endif /* _ASM_X86_KVM_H */ > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c > index 712bfbc0028a..acf5c45ef14e 100644 > --- a/arch/x86/kvm/svm/sev.c > +++ b/arch/x86/kvm/svm/sev.c > @@ -260,6 +260,9 @@ static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp) > if (kvm->created_vcpus) > return -EINVAL; > > + if (kvm->arch.vm_type != KVM_X86_DEFAULT_VM) > + return -EINVAL; > + > ret = -EBUSY; > if (unlikely(sev->active)) > return ret; > @@ -279,6 +282,7 @@ static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp) > > INIT_LIST_HEAD(&sev->regions_list); > INIT_LIST_HEAD(&sev->mirror_vms); > + sev->need_init = false; > > kvm_set_apicv_inhibit(kvm, APICV_INHIBIT_REASON_SEV); > > @@ -1814,7 +1818,8 @@ int sev_vm_move_enc_context_from(struct kvm *kvm, unsigned int source_fd) > if (ret) > goto out_fput; > > - if (sev_guest(kvm) || !sev_guest(source_kvm)) { > + if (kvm->arch.vm_type != source_kvm->arch.vm_type || > + sev_guest(kvm) || !sev_guest(source_kvm)) { > ret = -EINVAL; > goto out_unlock; > } > @@ -2135,6 +2140,7 @@ int sev_vm_copy_enc_context_from(struct kvm *kvm, unsigned int source_fd) > mirror_sev->asid = source_sev->asid; > mirror_sev->fd = source_sev->fd; > mirror_sev->es_active = source_sev->es_active; > + mirror_sev->need_init = false; > mirror_sev->handle = source_sev->handle; > INIT_LIST_HEAD(&mirror_sev->regions_list); > INIT_LIST_HEAD(&mirror_sev->mirror_vms); > @@ -3192,3 +3198,13 @@ void sev_vcpu_deliver_sipi_vector(struct kvm_vcpu *vcpu, u8 vector) > > ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, 1); > } > + > +bool sev_is_vm_type_supported(unsigned long type) > +{ > + if (type == KVM_X86_SEV_VM) > + return sev_enabled; > + if (type == KVM_X86_SEV_ES_VM) > + return sev_es_enabled; > + > + return false; > +} > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c > index 392b9c2e2ce1..87541c84d07e 100644 > --- a/arch/x86/kvm/svm/svm.c > +++ b/arch/x86/kvm/svm/svm.c > @@ -4087,6 +4087,11 @@ static void svm_cancel_injection(struct kvm_vcpu *vcpu) > > static int svm_vcpu_pre_run(struct kvm_vcpu *vcpu) > { > + struct kvm_sev_info *sev = &to_kvm_svm(vcpu->kvm)->sev_info; > + > + if (sev->need_init) > + return -EINVAL; > + > return 1; > } > > @@ -4888,6 +4893,11 @@ static void svm_vm_destroy(struct kvm *kvm) > > static int svm_vm_init(struct kvm *kvm) > { > + if (kvm->arch.vm_type) { > + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info; > + sev->need_init = true; > + } > + > if (!pause_filter_count || !pause_filter_thresh) > kvm->arch.pause_in_guest = true; > > @@ -4914,6 +4924,7 @@ static struct kvm_x86_ops svm_x86_ops __initdata = { > .vcpu_free = svm_vcpu_free, > .vcpu_reset = svm_vcpu_reset, > > + .is_vm_type_supported = sev_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/svm/svm.h b/arch/x86/kvm/svm/svm.h > index 864c782eaa58..63be26d4a024 100644 > --- a/arch/x86/kvm/svm/svm.h > +++ b/arch/x86/kvm/svm/svm.h > @@ -79,6 +79,7 @@ enum { > struct kvm_sev_info { > bool active; /* SEV enabled guest */ > bool es_active; /* SEV-ES enabled guest */ > + bool need_init; /* waiting for SEV_INIT2 */ Seems like this should be a separate patch. -Mike > unsigned int asid; /* ASID used for this guest */ > unsigned int handle; /* SEV firmware handle */ > int fd; /* SEV device fd */ > @@ -696,6 +697,7 @@ void sev_es_vcpu_reset(struct vcpu_svm *svm); > void sev_vcpu_deliver_sipi_vector(struct kvm_vcpu *vcpu, u8 vector); > void sev_es_prepare_switch_to_guest(struct vcpu_svm *svm, struct sev_es_save_area *hostsa); > void sev_es_unmap_ghcb(struct vcpu_svm *svm); > +bool sev_is_vm_type_supported(unsigned long type); > > /* vmenter.S */ > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index c89ddaa1e09f..dfc66ee091a1 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -4795,6 +4795,10 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) > r = BIT(KVM_X86_DEFAULT_VM); > if (kvm_is_vm_type_supported(KVM_X86_SW_PROTECTED_VM)) > r |= BIT(KVM_X86_SW_PROTECTED_VM); > + if (kvm_is_vm_type_supported(KVM_X86_SEV_VM)) > + r |= BIT(KVM_X86_SEV_VM); > + if (kvm_is_vm_type_supported(KVM_X86_SEV_ES_VM)) > + r |= BIT(KVM_X86_SEV_ES_VM); > break; > default: > break; > -- > 2.39.0 > >
On 2/15/24 02:19, Michael Roth wrote: >> #define KVM_X86_DEFAULT_VM 0 >> #define KVM_X86_SW_PROTECTED_VM (KVM_X86_DEFAULT_VM | __KVM_X86_PRIVATE_MEM_TYPE) >> +#define KVM_X86_SEV_VM 8 > Hmm... would it make sense to decouple the VM types and their associated > capabilities? Only bit 2 is left in the lower range after this, and using any > bits beyond TDX's bit 4 risks overflowing check_extension ioctl's 32-bit return > value. Yes, the idea was to leave 0..7 for vendor independent types (with 0 and 1 in use), 8..15 for AMD (3 of them being reserved already for SEV/SEV-ES/SEV-SNP), 16..23 for Intel. > Maybe a separate lookup table instead? The mask was nice because it can be used in relatively hot paths... I'll keep them but move the constants away from uapi/ headers. Paolo
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst index 3ec0b7a455a0..bf957bb70e4b 100644 --- a/Documentation/virt/kvm/api.rst +++ b/Documentation/virt/kvm/api.rst @@ -8790,6 +8790,8 @@ means the VM type with value @n is supported. Possible values of @n are:: #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 9. Known KVM API problems ========================= diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h index 6c74db23257e..7c46e96cfe62 100644 --- a/arch/x86/include/uapi/asm/kvm.h +++ b/arch/x86/include/uapi/asm/kvm.h @@ -854,5 +854,7 @@ struct kvm_hyperv_eventfd { #define KVM_X86_DEFAULT_VM 0 #define KVM_X86_SW_PROTECTED_VM (KVM_X86_DEFAULT_VM | __KVM_X86_PRIVATE_MEM_TYPE) +#define KVM_X86_SEV_VM 8 +#define KVM_X86_SEV_ES_VM (KVM_X86_SEV_VM | __KVM_X86_PROTECTED_STATE_TYPE) #endif /* _ASM_X86_KVM_H */ diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index 712bfbc0028a..acf5c45ef14e 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -260,6 +260,9 @@ static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp) if (kvm->created_vcpus) return -EINVAL; + if (kvm->arch.vm_type != KVM_X86_DEFAULT_VM) + return -EINVAL; + ret = -EBUSY; if (unlikely(sev->active)) return ret; @@ -279,6 +282,7 @@ static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp) INIT_LIST_HEAD(&sev->regions_list); INIT_LIST_HEAD(&sev->mirror_vms); + sev->need_init = false; kvm_set_apicv_inhibit(kvm, APICV_INHIBIT_REASON_SEV); @@ -1814,7 +1818,8 @@ int sev_vm_move_enc_context_from(struct kvm *kvm, unsigned int source_fd) if (ret) goto out_fput; - if (sev_guest(kvm) || !sev_guest(source_kvm)) { + if (kvm->arch.vm_type != source_kvm->arch.vm_type || + sev_guest(kvm) || !sev_guest(source_kvm)) { ret = -EINVAL; goto out_unlock; } @@ -2135,6 +2140,7 @@ int sev_vm_copy_enc_context_from(struct kvm *kvm, unsigned int source_fd) mirror_sev->asid = source_sev->asid; mirror_sev->fd = source_sev->fd; mirror_sev->es_active = source_sev->es_active; + mirror_sev->need_init = false; mirror_sev->handle = source_sev->handle; INIT_LIST_HEAD(&mirror_sev->regions_list); INIT_LIST_HEAD(&mirror_sev->mirror_vms); @@ -3192,3 +3198,13 @@ void sev_vcpu_deliver_sipi_vector(struct kvm_vcpu *vcpu, u8 vector) ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, 1); } + +bool sev_is_vm_type_supported(unsigned long type) +{ + if (type == KVM_X86_SEV_VM) + return sev_enabled; + if (type == KVM_X86_SEV_ES_VM) + return sev_es_enabled; + + return false; +} diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index 392b9c2e2ce1..87541c84d07e 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -4087,6 +4087,11 @@ static void svm_cancel_injection(struct kvm_vcpu *vcpu) static int svm_vcpu_pre_run(struct kvm_vcpu *vcpu) { + struct kvm_sev_info *sev = &to_kvm_svm(vcpu->kvm)->sev_info; + + if (sev->need_init) + return -EINVAL; + return 1; } @@ -4888,6 +4893,11 @@ static void svm_vm_destroy(struct kvm *kvm) static int svm_vm_init(struct kvm *kvm) { + if (kvm->arch.vm_type) { + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info; + sev->need_init = true; + } + if (!pause_filter_count || !pause_filter_thresh) kvm->arch.pause_in_guest = true; @@ -4914,6 +4924,7 @@ static struct kvm_x86_ops svm_x86_ops __initdata = { .vcpu_free = svm_vcpu_free, .vcpu_reset = svm_vcpu_reset, + .is_vm_type_supported = sev_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/svm/svm.h b/arch/x86/kvm/svm/svm.h index 864c782eaa58..63be26d4a024 100644 --- a/arch/x86/kvm/svm/svm.h +++ b/arch/x86/kvm/svm/svm.h @@ -79,6 +79,7 @@ enum { struct kvm_sev_info { bool active; /* SEV enabled guest */ bool es_active; /* SEV-ES enabled guest */ + bool need_init; /* waiting for SEV_INIT2 */ unsigned int asid; /* ASID used for this guest */ unsigned int handle; /* SEV firmware handle */ int fd; /* SEV device fd */ @@ -696,6 +697,7 @@ void sev_es_vcpu_reset(struct vcpu_svm *svm); void sev_vcpu_deliver_sipi_vector(struct kvm_vcpu *vcpu, u8 vector); void sev_es_prepare_switch_to_guest(struct vcpu_svm *svm, struct sev_es_save_area *hostsa); void sev_es_unmap_ghcb(struct vcpu_svm *svm); +bool sev_is_vm_type_supported(unsigned long type); /* vmenter.S */ diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index c89ddaa1e09f..dfc66ee091a1 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -4795,6 +4795,10 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) r = BIT(KVM_X86_DEFAULT_VM); if (kvm_is_vm_type_supported(KVM_X86_SW_PROTECTED_VM)) r |= BIT(KVM_X86_SW_PROTECTED_VM); + if (kvm_is_vm_type_supported(KVM_X86_SEV_VM)) + r |= BIT(KVM_X86_SEV_VM); + if (kvm_is_vm_type_supported(KVM_X86_SEV_ES_VM)) + r |= BIT(KVM_X86_SEV_ES_VM); break; default: break;
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- Documentation/virt/kvm/api.rst | 2 ++ arch/x86/include/uapi/asm/kvm.h | 2 ++ arch/x86/kvm/svm/sev.c | 18 +++++++++++++++++- arch/x86/kvm/svm/svm.c | 11 +++++++++++ arch/x86/kvm/svm/svm.h | 2 ++ arch/x86/kvm/x86.c | 4 ++++ 6 files changed, 38 insertions(+), 1 deletion(-)