Re: [PATCH 2/2] kvm: nVMX: Introduce KVM_CAP_STATE

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, 2018-04-14 at 15:56 +0000, Raslan, KarimAllah wrote:
> On Thu, 2018-04-12 at 17:12 +0200, KarimAllah Ahmed wrote:
> > 
> > From: Jim Mattson <jmattson@xxxxxxxxxx>
> > 
> > For nested virtualization L0 KVM is managing a bit of state for L2 guests,
> > this state can not be captured through the currently available IOCTLs. In
> > fact the state captured through all of these IOCTLs is usually a mix of L1
> > and L2 state. It is also dependent on whether the L2 guest was running at
> > the moment when the process was interrupted to save its state.
> > 
> > With this capability, there are two new vcpu ioctls: KVM_GET_VMX_STATE and
> > KVM_SET_VMX_STATE. These can be used for saving and restoring a VM that is
> > in VMX operation.
> > 
> > Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> > Cc: Radim Krčmář <rkrcmar@xxxxxxxxxx>
> > Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> > Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> > Cc: H. Peter Anvin <hpa@xxxxxxxxx>
> > Cc: x86@xxxxxxxxxx
> > Cc: kvm@xxxxxxxxxxxxxxx
> > Cc: linux-kernel@xxxxxxxxxxxxxxx
> > Signed-off-by: Jim Mattson <jmattson@xxxxxxxxxx>
> > [karahmed@ - rename structs and functions and make them ready for AMD and
> >              address previous comments.
> >            - rebase & a bit of refactoring.
> >            - Merge 7/8 and 8/8 into one patch.
> >            - Force a VMExit from L2 after reading the kvm_state to avoid
> >              mixed state between L1 and L2 on resurrecting the instance. ]
> > Signed-off-by: KarimAllah Ahmed <karahmed@xxxxxxxxx>
> > ---
> > v2 -> v3:
> > - Remove the forced VMExit from L2 after reading the kvm_state. The actual
> >   problem is solved.
> > - Rebase again!
> > - Set nested_run_pending during restore (not sure if it makes sense yet or
> >   not).
> > - Reduce KVM_REQUEST_ARCH_BASE to 7 instead of 8 (the other alternative is
> >   to switch everything to u64)
> > 
> > v1 -> v2:
> > - Rename structs and functions and make them ready for AMD and address
> >   previous comments.
> > - Rebase & a bit of refactoring.
> > - Merge 7/8 and 8/8 into one patch.
> > - Force a VMExit from L2 after reading the kvm_state to avoid mixed state
> >   between L1 and L2 on resurrecting the instance.
> > ---
> >  Documentation/virtual/kvm/api.txt |  47 ++++++++++
> >  arch/x86/include/asm/kvm_host.h   |   7 ++
> >  arch/x86/include/uapi/asm/kvm.h   |  38 ++++++++
> >  arch/x86/kvm/vmx.c                | 177 +++++++++++++++++++++++++++++++++++++-
> >  arch/x86/kvm/x86.c                |  21 +++++
> >  include/linux/kvm_host.h          |   2 +-
> >  include/uapi/linux/kvm.h          |   5 ++
> >  7 files changed, 292 insertions(+), 5 deletions(-)
> > 
> > diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> > index 1c7958b..c51d5d3 100644
> > --- a/Documentation/virtual/kvm/api.txt
> > +++ b/Documentation/virtual/kvm/api.txt
> > @@ -3548,6 +3548,53 @@ Returns: 0 on success,
> >  	-ENOENT on deassign if the conn_id isn't registered
> >  	-EEXIST on assign if the conn_id is already registered
> >  
> > +4.114 KVM_GET_STATE
> > +
> > +Capability: KVM_CAP_STATE
> > +Architectures: x86
> > +Type: vcpu ioctl
> > +Parameters: struct kvm_state (in/out)
> > +Returns: 0 on success, -1 on error
> > +Errors:
> > +  E2BIG:     the data size exceeds the value of 'size' specified by
> > +             the user (the size required will be written into size).
> > +
> > +struct kvm_state {
> > +	__u16 flags;
> > +	__u16 format;
> > +	__u32 size;
> > +	union {
> > +		struct kvm_vmx_state vmx;
> > +		struct kvm_svm_state svm;
> > +		__u8 pad[120];
> > +	};
> > +	__u8 data[0];
> > +};
> > +
> > +This ioctl copies the vcpu's kvm_state struct from the kernel to userspace.
> > +
> > +4.115 KVM_SET_STATE
> > +
> > +Capability: KVM_CAP_STATE
> > +Architectures: x86
> > +Type: vcpu ioctl
> > +Parameters: struct kvm_state (in)
> > +Returns: 0 on success, -1 on error
> > +
> > +struct kvm_state {
> > +	__u16 flags;
> > +	__u16 format;
> > +	__u32 size;
> > +	union {
> > +		struct kvm_vmx_state vmx;
> > +		struct kvm_svm_state svm;
> > +		__u8 pad[120];
> > +	};
> > +	__u8 data[0];
> > +};
> > +
> > +This copies the vcpu's kvm_state struct from userspace to the kernel.
> > +>>>>>>> 13a7c9e... kvm: nVMX: Introduce KVM_CAP_STATE
> >  
> >  5. The kvm_run structure
> >  ------------------------
> > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > index 9fa4f57..ad2116a 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -75,6 +75,7 @@
> >  #define KVM_REQ_HV_EXIT			KVM_ARCH_REQ(21)
> >  #define KVM_REQ_HV_STIMER		KVM_ARCH_REQ(22)
> >  #define KVM_REQ_LOAD_EOI_EXITMAP	KVM_ARCH_REQ(23)
> > +#define KVM_REQ_GET_VMCS12_PAGES	KVM_ARCH_REQ(24)
> >  
> >  #define CR0_RESERVED_BITS                                               \
> >  	(~(unsigned long)(X86_CR0_PE | X86_CR0_MP | X86_CR0_EM | X86_CR0_TS \
> > @@ -1084,6 +1085,12 @@ struct kvm_x86_ops {
> >  
> >  	void (*setup_mce)(struct kvm_vcpu *vcpu);
> >  
> > +	int (*get_state)(struct kvm_vcpu *vcpu,
> > +			 struct kvm_state __user *user_kvm_state);
> > +	int (*set_state)(struct kvm_vcpu *vcpu,
> > +			 struct kvm_state __user *user_kvm_state);
> > +	void (*get_vmcs12_pages)(struct kvm_vcpu *vcpu);
> > +
> >  	int (*smi_allowed)(struct kvm_vcpu *vcpu);
> >  	int (*pre_enter_smm)(struct kvm_vcpu *vcpu, char *smstate);
> >  	int (*pre_leave_smm)(struct kvm_vcpu *vcpu, u64 smbase);
> > diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
> > index c535c2f..d2c860a 100644
> > --- a/arch/x86/include/uapi/asm/kvm.h
> > +++ b/arch/x86/include/uapi/asm/kvm.h
> > @@ -378,4 +378,42 @@ struct kvm_sync_regs {
> >  #define KVM_X86_QUIRK_LINT0_REENABLED	(1 << 0)
> >  #define KVM_X86_QUIRK_CD_NW_CLEARED	(1 << 1)
> >  
> > +#define KVM_STATE_GUEST_MODE	0x00000001
> > +#define KVM_STATE_RUN_PENDING	0x00000002
> > +#define KVM_STATE_GIF		0x00000004
> > +
> > +struct kvm_vmx_state {
> > +	__u64 vmxon_pa;
> > +	__u64 vmcs_pa;
> > +};
> > +
> > +struct kvm_svm_state {
> > +	__u64 hsave_pa;
> > +	__u64 vmcb_pa;
> > +};
> > +
> > +/* for KVM_CAP_STATE */
> > +struct kvm_state {
> > +	/* KVM_STATE_* flags */
> > +	__u16 flags;
> > +
> > +	/* 0 for VMX, 1 for SVM.  */
> > +	__u16 format;
> > +
> > +	/* 128 for SVM, 128 + VMCS size for VMX.  */
> > +	__u32 size;
> > +
> > +	union {
> > +		/* VMXON, VMCS */
> > +		struct kvm_vmx_state vmx;
> > +		/* HSAVE_PA, VMCB */
> > +		struct kvm_svm_state svm;
> > +
> > +		/* Pad the union to 120 bytes.  */
> > +		__u8 pad[120];
> > +	};
> > +
> > +	__u8 data[0];
> > +};
> > +
> >  #endif /* _ASM_X86_KVM_H */
> > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> > index 2f57571..c2b436b 100644
> > --- a/arch/x86/kvm/vmx.c
> > +++ b/arch/x86/kvm/vmx.c
> > @@ -10359,10 +10359,10 @@ static void vmx_inject_page_fault_nested(struct kvm_vcpu *vcpu,
> >  static inline bool nested_vmx_prepare_msr_bitmap(struct kvm_vcpu *vcpu,
> >  						 struct vmcs12 *vmcs12);
> >  
> > -static void nested_get_vmcs12_pages(struct kvm_vcpu *vcpu,
> > -					struct vmcs12 *vmcs12)
> > +static void nested_get_vmcs12_pages(struct kvm_vcpu *vcpu)
> >  {
> >  	struct vcpu_vmx *vmx = to_vmx(vcpu);
> > +	struct vmcs12 *vmcs12 = get_vmcs12(vcpu);
> >  
> >  	if (nested_cpu_has2(vmcs12, SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES)) {
> >  		if (vmcs12->apic_access_addr != vmx->nested.apic_access_mapping.gfn << PAGE_SHIFT) {
> > @@ -11430,8 +11430,6 @@ static int enter_vmx_non_root_mode(struct kvm_vcpu *vcpu, bool from_vmentry)
> >  		return 1;
> >  	}
> >  
> > -	nested_get_vmcs12_pages(vcpu, vmcs12);
> > -
> >  	msr_entry_idx = nested_vmx_load_msr(vcpu,
> >  					    vmcs12->vm_entry_msr_load_addr,
> >  					    vmcs12->vm_entry_msr_load_count);
> > @@ -11529,6 +11527,8 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch)
> >  	if (ret)
> >  		return ret;
> >  
> > +	nested_get_vmcs12_pages(vcpu);
> > +
> >  	/*
> >  	 * If we're entering a halted L2 vcpu and the L2 vcpu won't be woken
> >  	 * by event injection, halt vcpu.
> > @@ -12595,6 +12595,171 @@ static int enable_smi_window(struct kvm_vcpu *vcpu)
> >  	return 0;
> >  }
> >  
> > +static int get_vmcs_cache(struct kvm_vcpu *vcpu,
> > +			  struct kvm_state __user *user_kvm_state)
> > +{
> > +	struct vcpu_vmx *vmx = to_vmx(vcpu);
> > +	struct vmcs12 *vmcs12 = get_vmcs12(vcpu);
> > +
> > +	/*
> > +	 * When running L2, the authoritative vmcs12 state is in the
> > +	 * vmcs02. When running L1, the authoritative vmcs12 state is
> > +	 * in the shadow vmcs linked to vmcs01, unless
> > +	 * sync_shadow_vmcs is set, in which case, the authoritative
> > +	 * vmcs12 state is in the vmcs12 already.
> > +	 */
> > +	if (is_guest_mode(vcpu))
> > +		sync_vmcs12(vcpu, vmcs12);
> > +	else if (enable_shadow_vmcs && !vmx->nested.sync_shadow_vmcs)
> > +		copy_shadow_to_vmcs12(vmx);
> > +
> > +	if (copy_to_user(user_kvm_state->data, vmcs12, sizeof(*vmcs12)))
> > +		return -EFAULT;
> > +
> > +	return 0;
> > +}
> > +
> > +static int get_vmx_state(struct kvm_vcpu *vcpu,
> > +			 struct kvm_state __user *user_kvm_state)
> > +{
> > +	u32 user_data_size;
> > +	struct vcpu_vmx *vmx = to_vmx(vcpu);
> > +	struct kvm_state kvm_state = {
> > +		.flags = 0,
> > +		.format = 0,
> > +		.size = sizeof(kvm_state),
> > +		.vmx.vmxon_pa = -1ull,
> > +		.vmx.vmcs_pa = -1ull,
> > +	};
> > +
> > +	if (copy_from_user(&user_data_size, &user_kvm_state->size,
> > +			   sizeof(user_data_size)))
> > +		return -EFAULT;
> > +
> > +	if (nested_vmx_allowed(vcpu) && vmx->nested.vmxon) {
> > +		kvm_state.vmx.vmxon_pa = vmx->nested.vmxon_ptr;
> > +		kvm_state.vmx.vmcs_pa = vmx->nested.current_vmptr;
> > +
> > +		if (vmx->nested.current_vmptr != -1ull)
> > +			kvm_state.size += VMCS12_SIZE;
> > +
> > +		if (is_guest_mode(vcpu)) {
> > +			kvm_state.flags |= KVM_STATE_GUEST_MODE;
> > +
> > +			if (vmx->nested.nested_run_pending)
> > +				kvm_state.flags |= KVM_STATE_RUN_PENDING;
> > +		}
> > +	}
> > +
> > +	if (user_data_size < kvm_state.size) {
> > +		if (copy_to_user(&user_kvm_state->size, &kvm_state.size,
> > +				 sizeof(kvm_state.size)))
> > +			return -EFAULT;
> > +		return -E2BIG;
> > +	}
> > +
> > +	if (copy_to_user(user_kvm_state, &kvm_state, sizeof(kvm_state)))
> > +		return -EFAULT;
> > +
> > +	if (vmx->nested.current_vmptr == -1ull)
> > +		return 0;
> > +
> > +	return get_vmcs_cache(vcpu, user_kvm_state);
> > +}
> > +
> > +static int set_vmcs_cache(struct kvm_vcpu *vcpu,
> > +			  struct kvm_state __user *user_kvm_state,
> > +			  struct kvm_state *kvm_state)
> > +
> > +{
> > +	struct vcpu_vmx *vmx = to_vmx(vcpu);
> > +	struct vmcs12 *vmcs12 = get_vmcs12(vcpu);
> > +	u32 exit_qual;
> > +	int ret;
> > +
> > +	if ((kvm_state->size < (sizeof(*vmcs12) + sizeof(*kvm_state))) ||
> > +	    kvm_state->vmx.vmcs_pa == kvm_state->vmx.vmxon_pa ||
> > +	    !page_address_valid(vcpu, kvm_state->vmx.vmcs_pa))
> > +		return -EINVAL;
> > +
> > +	if (copy_from_user(vmcs12, user_kvm_state->data, sizeof(*vmcs12)))
> > +		return -EFAULT;
> > +
> > +	if (vmcs12->revision_id != VMCS12_REVISION)
> > +		return -EINVAL;
> > +
> > +	set_current_vmptr(vmx, kvm_state->vmx.vmcs_pa);
> > +
> > +	if (!(kvm_state->flags & KVM_STATE_GUEST_MODE))
> > +		return 0;
> > +
> > +	if (kvm_state->flags & KVM_STATE_RUN_PENDING)
> > +		vmx->nested.nested_run_pending = 1;
> > +
> > +	if (check_vmentry_prereqs(vcpu, vmcs12) ||
> > +	    check_vmentry_postreqs(vcpu, vmcs12, &exit_qual))
> > +		return -EINVAL;
> > +
> > +	ret = enter_vmx_non_root_mode(vcpu, true);
> > +	if (ret)
> > +		return ret;
> > +
> > +	/*
> > +	 * This request will result in a call to
> > +	 * nested_get_vmcs12_pages before the next VM-entry.
> > +	 */
> > +	kvm_make_request(KVM_REQ_GET_VMCS12_PAGES, vcpu);
> > +
> > +	vmx->nested.nested_run_pending = 1;
> > +
> > +	return 0;
> > +}
> > +
> > +static int set_vmx_state(struct kvm_vcpu *vcpu,
> > +			 struct kvm_state __user *user_kvm_state)
> > +{
> > +	struct vcpu_vmx *vmx = to_vmx(vcpu);
> > +	struct kvm_state kvm_state;
> > +	int ret;
> > +
> > +	if (copy_from_user(&kvm_state, user_kvm_state, sizeof(kvm_state)))
> > +		return -EFAULT;
> > +
> > +	if (kvm_state.size < sizeof(kvm_state))
> > +		return -EINVAL;
> > +
> > +	if (kvm_state.format != 0)
> > +		return -EINVAL;
> > +
> > +	if (kvm_state.flags &
> > +	    ~(KVM_STATE_RUN_PENDING | KVM_STATE_GUEST_MODE))
> > +		return -EINVAL;
> > +
> > +	if (!nested_vmx_allowed(vcpu))
> > +		return kvm_state.vmx.vmxon_pa == -1ull ? 0 : -EINVAL;
> > +
> > +	vmx_leave_nested(vcpu);
> > +
> > +	vmx->nested.nested_run_pending =
> > +		!!(kvm_state.flags & KVM_STATE_RUN_PENDING);
> > +
> > +	if (kvm_state.vmx.vmxon_pa == -1ull)
> > +		return 0;
> > +
> > +	if (!page_address_valid(vcpu, kvm_state.vmx.vmxon_pa))
> > +		return -EINVAL;
> > +
> > +	vmx->nested.vmxon_ptr = kvm_state.vmx.vmxon_pa;
> > +	ret = enter_vmx_operation(vcpu);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (kvm_state.vmx.vmcs_pa == -1ull)
> > +		return 0;
> > +
> > +	return set_vmcs_cache(vcpu, user_kvm_state, &kvm_state);
> > +}
> > +
> >  static struct kvm_x86_ops vmx_x86_ops __ro_after_init = {
> >  	.cpu_has_kvm_support = cpu_has_kvm_support,
> >  	.disabled_by_bios = vmx_disabled_by_bios,
> > @@ -12728,6 +12893,10 @@ static struct kvm_x86_ops vmx_x86_ops __ro_after_init = {
> >  
> >  	.setup_mce = vmx_setup_mce,
> >  
> > +	.get_state = get_vmx_state,
> > +	.set_state = set_vmx_state,
> > +	.get_vmcs12_pages = nested_get_vmcs12_pages,
> > +
> >  	.smi_allowed = vmx_smi_allowed,
> >  	.pre_enter_smm = vmx_pre_enter_smm,
> >  	.pre_leave_smm = vmx_pre_leave_smm,
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 1a2ed92..565e1de 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -2923,6 +2923,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> >  	case KVM_CAP_X2APIC_API:
> >  		r = KVM_X2APIC_API_VALID_FLAGS;
> >  		break;
> > +	case KVM_CAP_STATE:
> > +		r = !!kvm_x86_ops->get_state;
> > +		break;
> >  	default:
> >  		break;
> >  	}
> > @@ -3941,6 +3944,22 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
> >  		r = kvm_vcpu_ioctl_enable_cap(vcpu, &cap);
> >  		break;
> >  	}
> > +	case KVM_GET_STATE: {
> > +		struct kvm_state __user *user_kvm_state = argp;
> > +
> > +		r = -EINVAL;
> > +		if (kvm_x86_ops->get_state)
> > +			r = kvm_x86_ops->get_state(vcpu, user_kvm_state);
> > +		break;
> > +	}
> > +	case KVM_SET_STATE: {
> > +		struct kvm_state __user *user_kvm_state = argp;
> > +
> > +		r = -EINVAL;
> > +		if (kvm_x86_ops->set_state)
> > +			r = kvm_x86_ops->set_state(vcpu, user_kvm_state);
> > +		break;
> > +	}
> >  	default:
> >  		r = -EINVAL;
> >  	}
> > @@ -7214,6 +7233,8 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
> >  	bool req_immediate_exit = false;
> >  
> >  	if (kvm_request_pending(vcpu)) {
> > +		if (kvm_check_request(KVM_REQ_GET_VMCS12_PAGES, vcpu))
> > +			kvm_x86_ops->get_vmcs12_pages(vcpu);
> >  		if (kvm_check_request(KVM_REQ_MMU_RELOAD, vcpu))
> >  			kvm_mmu_unload(vcpu);
> >  		if (kvm_check_request(KVM_REQ_MIGRATE_TIMER, vcpu))
> > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> > index 7a2889a..001b122 100644
> > --- a/include/linux/kvm_host.h
> > +++ b/include/linux/kvm_host.h
> > @@ -126,7 +126,7 @@ static inline bool is_error_page(struct page *page)
> >  #define KVM_REQ_MMU_RELOAD        (1 | KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
> >  #define KVM_REQ_PENDING_TIMER     2
> >  #define KVM_REQ_UNHALT            3
> > -#define KVM_REQUEST_ARCH_BASE     8
> > +#define KVM_REQUEST_ARCH_BASE     7
> 
> Paolo/Jim,
> 
> Is this bit acceptable for you or should I just go ahead and update
> "requests" to be 64 bit instead?
> 
> We ran out of architecture specific bits :)

I just decided to opt for using all the 64-bits in requests:
https://lkml.org/lkml/2018/4/14/93

So I will drop this change to KVM_REQUEST_ARCH_BASE in the next
version.

> 
> > 
> >  
> >  #define KVM_ARCH_REQ_FLAGS(nr, flags) ({ \
> >  	BUILD_BUG_ON((unsigned)(nr) >= 32 - KVM_REQUEST_ARCH_BASE); \
> > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> > index 077d16f..69e6791 100644
> > --- a/include/uapi/linux/kvm.h
> > +++ b/include/uapi/linux/kvm.h
> > @@ -961,6 +961,7 @@ struct kvm_ppc_resize_hpt {
> >  #define KVM_CAP_S390_BPB 152
> >  #define KVM_CAP_GET_MSR_FEATURES 153
> >  #define KVM_CAP_HYPERV_EVENTFD 154
> > +#define KVM_CAP_STATE 155
> >  
> >  #ifdef KVM_CAP_IRQ_ROUTING
> >  
> > @@ -1392,6 +1393,10 @@ struct kvm_s390_ucas_mapping {
> >  /* Memory Encryption Commands */
> >  #define KVM_MEMORY_ENCRYPT_OP      _IOWR(KVMIO, 0xba, unsigned long)
> >  
> > +/* Available with KVM_CAP_STATE */
> > +#define KVM_GET_STATE         _IOWR(KVMIO, 0xbb, struct kvm_vmx_state)
> > +#define KVM_SET_STATE         _IOW(KVMIO,  0xbc, struct kvm_vmx_state)
> > +
> >  struct kvm_enc_region {
> >  	__u64 addr;
> >  	__u64 size;
Amazon Development Center Germany GmbH
Berlin - Dresden - Aachen
main office: Krausenstr. 38, 10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux