KVM: SVM: SEV-ES groundwork
v4 cover KVM: SVM: SEV-ES groundwork
Hi Paolo,
here is v4 of the groundwork patches for the upcoming SEV-ES support in the Linux kernel. They are part of both the client patch-set and the KVM hypervisor patches (under development).
Patch 1 necessary to fix a compile warning about a stack-frame getting too large. Patches 2 and 3 define the Guest Hypervisor Communication Block (GHCB) and accessor functions and the last patch is a cleanup.
This version addresses more review comments from Sean. Additionally the patches are now checkpatch-clean.
It would be great if you could consider them for v5.9, so that the client and the hypervisor patch-sets can be developed more independently of each other.
Please let me know what you think.
Regards,
Joerg
Borislav Petkov (1):
KVM: SVM: Use __packed shorthand
Joerg Roedel (2):
KVM: SVM: nested: Don't allocate VMCB structures on stack
KVM: SVM: Add GHCB Accessor functions
Tom Lendacky (1):
KVM: SVM: Add GHCB definitions
arch/x86/include/asm/svm.h | 100 ++++++++++++++++++++++++++++++++++--- arch/x86/kvm/svm/nested.c | 47 +++++++++++------ arch/x86/kvm/svm/svm.c | 2 + 3 files changed, 128 insertions(+), 21 deletions(-)
Signature · 1 line
2.17.1
PATCH v4 1/4 KVM: SVM: nested: Don't allocate VMCB structures on stack
From: Joerg Roedel <[email protected]>
Do not allocate a vmcb_control_area and a vmcb_save_area on the stack, as these structures will become larger with future extenstions of SVM and thus the svm_set_nested_state() function will become a too large stack frame.
--- arch/x86/kvm/svm/nested.c | 47 +++++++++++++++++++++++++++------------ 1 file changed, 33 insertions(+), 14 deletions(-)
arch/x86/kvm/svm/nested.c +33 -14
@@ -1061,10 +1061,14 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,struct vmcb *hsave = svm->nested.hsave;struct vmcb __user *user_vmcb = (struct vmcb __user *) &user_kvm_nested_state->data.svm[0 ]; -struct vmcb_control_area ctl; -struct vmcb_save_area save; +struct vmcb_control_area *ctl; +struct vmcb_save_area *save; +int ret;u32 cr0; +BUILD_BUG_ON (sizeof (struct vmcb_control_area ) +sizeof (struct vmcb_save_area ) > +KVM_STATE_NESTED_SVM_VMCB_SIZE ); +if (kvm_state->format !=KVM_STATE_NESTED_FORMAT_SVM )return -EINVAL ;
@@ -1096,13 +1100,22 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,return -EINVAL ;if (kvm_state->size <sizeof (*kvm_state) +KVM_STATE_NESTED_SVM_VMCB_SIZE )return -EINVAL ; -if (copy_from_user (&ctl, &user_vmcb->control,sizeof (ctl))) -return -EFAULT ; -if (copy_from_user (&save, &user_vmcb->save,sizeof (save))) -return -EFAULT ; -if (!nested_vmcb_check_controls (&ctl)) -return -EINVAL ; + ret = -ENOMEM ; + ctl =kzalloc (sizeof (*ctl),GFP_KERNEL ); + save =kzalloc (sizeof (*save),GFP_KERNEL ); +if (!ctl || !save) + goto out_free; + + ret = -EFAULT ; +if (copy_from_user (ctl, &user_vmcb->control,sizeof (*ctl))) + goto out_free; +if (copy_from_user (save, &user_vmcb->save,sizeof (*save))) + goto out_free; + + ret = -EINVAL ; +if (!nested_vmcb_check_controls (ctl)) + goto out_free;/* * Processor state contains L2 state. Check that it is
@@ -1110,15 +1123,15 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,*/ cr0 =kvm_read_cr0 (vcpu);if (((cr0 &X86_CR0_CD ) ==0 ) && (cr0 &X86_CR0_NW )) -return -EINVAL ; + goto out_free;/* * Validate host state saved from before VMRUN (see * nested_svm_check_permissions). * TODO: validate reserved bits for all saved state. */ -if (!(save.cr0 &X86_CR0_PG )) -return -EINVAL ; +if (!(save->cr0 &X86_CR0_PG )) + goto out_free;/* * All checks done, we can enter guest mode. L1 control fields
@@ -1127,15 +1140,21 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,* contains saved L1 state. */ copy_vmcb_control_area (&hsave->control, &svm->vmcb->control); - hsave->save = save; + hsave->save = *save; svm->nested.vmcb = kvm_state->hdr.svm.vmcb_pa; -load_nested_vmcb_control (svm, &ctl); +load_nested_vmcb_control (svm, ctl);nested_prepare_vmcb_control (svm); out_set_gif:svm_set_gif (svm, !!(kvm_state->flags &KVM_STATE_NESTED_GIF_SET )); -return 0 ; + + ret =0 ; +out_free: +kfree (save); +kfree (ctl); + +return ret; }struct kvm_x86_nested_ops svm_nested_ops = {
Signature · 1 line
2.17.1
PATCH v4 2/4 KVM: SVM: Add GHCB definitions
In reply to Joerg RoedelFrom: Tom Lendacky <[email protected]>
Extend the vmcb_safe_area with SEV-ES fields and add a new 'struct ghcb' which will be used for guest-hypervisor communication.
--- arch/x86/include/asm/svm.h | 45 +++++++++++++++++++++++++++++++++++++- arch/x86/kvm/svm/svm.c | 2 ++ 2 files changed, 46 insertions(+), 1 deletion(-)
arch/x86/include/asm/svm.h +44 -1
@@ -200,13 +200,56 @@ struct __attribute__ ((__packed__)) vmcb_save_area {u64 br_to;u64 last_excp_from;u64 last_excp_to; + +/* +* The following part of the save area is valid only for +* SEV-ES guests when referenced through the GHCB. +*/ +u8 reserved_7[104 ]; +u64 reserved_8;/* rax already available at 0x01f8 */ +u64 rcx; +u64 rdx; +u64 rbx; +u64 reserved_9;/* rsp already available at 0x01d8 */ +u64 rbp; +u64 rsi; +u64 rdi; +u64 r8; +u64 r9; +u64 r10; +u64 r11; +u64 r12; +u64 r13; +u64 r14; +u64 r15; +u8 reserved_10[16 ]; +u64 sw_exit_code; +u64 sw_exit_info_1; +u64 sw_exit_info_2; +u64 sw_scratch; +u8 reserved_11[56 ]; +u64 xcr0; +u8 valid_bitmap[16 ]; +u64 x87_state_gpa; }; +struct ghcb { +struct vmcb_save_area save; +u8 reserved_save[2048 -sizeof (struct vmcb_save_area )]; + +u8 shared_buffer[2032 ]; + +u8 reserved_1[10 ]; +u16 protocol_version;/* negotiated SEV-ES/GHCB protocol version */ +u32 ghcb_usage; +} __packed; +static inline void __unused_size_checks (void ) { -BUILD_BUG_ON (sizeof (struct vmcb_save_area ) !=0x298 ); +BUILD_BUG_ON (sizeof (struct vmcb_save_area ) !=1032 );BUILD_BUG_ON (sizeof (struct vmcb_control_area ) !=256 ); +BUILD_BUG_ON (sizeof (struct ghcb ) !=4096 ); }struct __attribute__ ((__packed__ ))vmcb {
arch/x86/kvm/svm/svm.c +2 -0
@@ -4161,6 +4161,8 @@ static struct kvm_x86_init_ops svm_init_ops __initdata = {static int __init svm_init (void ) { +__unused_size_checks (); +return kvm_init (&svm_init_ops,sizeof (struct vcpu_svm ), __alignof__(struct vcpu_svm ),THIS_MODULE ); }
Signature · 1 line
2.17.1
PATCH v4 3/4 KVM: SVM: Add GHCB Accessor functions
In reply to Joerg RoedelFrom: Joerg Roedel <[email protected]>
Building a correct GHCB for the hypervisor requires setting valid bits in the GHCB. Simplify that process by providing accessor functions to set values and to update the valid bitmap and to check the valid bitmap in KVM.
--- arch/x86/include/asm/svm.h | 43 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+)
arch/x86/include/asm/svm.h +43 -0
@@ -341,4 +341,47 @@ struct __attribute__ ((__packed__)) vmcb {#define SVM_CR0_SELECTIVE_MASK (X86_CR0_TS | X86_CR0_MP) +/* GHCB Accessor functions */ + +#define GHCB_BITMAP_IDX (field) \ + (offsetof(struct vmcb_save_area, field) / sizeof(u64)) + +#define DEFINE_GHCB_ACCESSORS (field) \ + static inline bool ghcb_##field##_is_valid(const struct ghcb *ghcb) \ + { \ + return test_bit(GHCB_BITMAP_IDX(field), \ + (unsigned long *)&ghcb->save.valid_bitmap); \ + } \ + \ + static inline void ghcb_set_##field(struct ghcb *ghcb, u64 value) \ + { \ + __set_bit(GHCB_BITMAP_IDX(field), \ + (unsigned long *)&ghcb->save.valid_bitmap); \ + ghcb->save.field = value; \ + } + +DEFINE_GHCB_ACCESSORS(cpl) +DEFINE_GHCB_ACCESSORS(rip) +DEFINE_GHCB_ACCESSORS(rsp) +DEFINE_GHCB_ACCESSORS(rax) +DEFINE_GHCB_ACCESSORS(rcx) +DEFINE_GHCB_ACCESSORS(rdx) +DEFINE_GHCB_ACCESSORS(rbx) +DEFINE_GHCB_ACCESSORS(rbp) +DEFINE_GHCB_ACCESSORS(rsi) +DEFINE_GHCB_ACCESSORS(rdi) +DEFINE_GHCB_ACCESSORS(r8) +DEFINE_GHCB_ACCESSORS(r9) +DEFINE_GHCB_ACCESSORS(r10) +DEFINE_GHCB_ACCESSORS(r11) +DEFINE_GHCB_ACCESSORS(r12) +DEFINE_GHCB_ACCESSORS(r13) +DEFINE_GHCB_ACCESSORS(r14) +DEFINE_GHCB_ACCESSORS(r15) +DEFINE_GHCB_ACCESSORS(sw_exit_code) +DEFINE_GHCB_ACCESSORS(sw_exit_info_1) +DEFINE_GHCB_ACCESSORS(sw_exit_info_2) +DEFINE_GHCB_ACCESSORS(sw_scratch) +DEFINE_GHCB_ACCESSORS(xcr0) +#endif
Signature · 1 line
2.17.1
PATCH v4 4/4 KVM: SVM: Use __packed shorthand
In reply to Joerg RoedelFrom: Borislav Petkov <[email protected]>
Use the shorthand to make it more readable.
No functional changes.
--- arch/x86/include/asm/svm.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
arch/x86/include/asm/svm.h +6 -6
@@ -150,14 +150,14 @@ struct __attribute__ ((__packed__)) vmcb_control_area {#define SVM_NESTED_CTL_NP_ENABLE BIT(0)#define SVM_NESTED_CTL_SEV_ENABLE BIT(1) -struct __attribute__ ((__packed__ ))vmcb_seg { +struct vmcb_seg {u16 selector;u16 attrib;u32 limit;u64 base; -}; +} __packed; -struct __attribute__ ((__packed__ ))vmcb_save_area { +struct vmcb_save_area {struct vmcb_seg es;struct vmcb_seg cs;struct vmcb_seg ss;
@@ -231,7 +231,7 @@ struct __attribute__ ((__packed__)) vmcb_save_area {u64 xcr0;u8 valid_bitmap[16 ];u64 x87_state_gpa; -}; +} __packed;struct ghcb {struct vmcb_save_area save;
@@ -252,11 +252,11 @@ static inline void __unused_size_checks(void)BUILD_BUG_ON (sizeof (struct ghcb ) !=4096 ); } -struct __attribute__ ((__packed__ ))vmcb { +struct vmcb {struct vmcb_control_area control;u8 reserved_control[1024 -sizeof (struct vmcb_control_area )];struct vmcb_save_area save; -}; +} __packed;#define SVM_CPUID_FUNC 0x8000000a
Signature · 1 line
2.17.1