* [PATCH 01/10] KVM: x86: ensure all MSRs can always be KVM_GET/SET_MSR'd
2018-07-28 23:10 [PATCH v6 00/10] kvm: x86: migration of nested virtualization state Paolo Bonzini
@ 2018-07-28 23:10 ` Paolo Bonzini
2018-07-28 23:10 ` [PATCH 02/10] kvm: selftests: create a GDT and TSS Paolo Bonzini
` (8 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2018-07-28 23:10 UTC (permalink / raw)
To: linux-kernel, kvm
Cc: Liran Alon, KarimAllah Ahmed, Jim Mattson, rkrcmar, stable
Some of the MSRs returned by GET_MSR_INDEX_LIST currently cannot be sent back
to KVM_GET_MSR and/or KVM_SET_MSR; either they can never be sent back, or you
they are only accepted under special conditions. This makes the API a pain to
use.
To avoid this pain, this patch makes it so that the result of the get-list
ioctl can always be used for host-initiated get and set. Since we don't have
a separate way to check for read-only MSRs, this means some Hyper-V MSRs are
ignored when written. Arguably they should not even be in the result of
GET_MSR_INDEX_LIST, but I am leaving there in case userspace is using the
outcome of GET_MSR_INDEX_LIST to derive the support for the corresponding
Hyper-V feature.
Cc: stable@vger.kernel.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
arch/x86/kvm/hyperv.c | 27 ++++++++++++++++++++-------
arch/x86/kvm/hyperv.h | 2 +-
arch/x86/kvm/x86.c | 15 +++++++++------
3 files changed, 30 insertions(+), 14 deletions(-)
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index af8caf965baa..01d209ab5481 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -235,7 +235,7 @@ static int synic_set_msr(struct kvm_vcpu_hv_synic *synic,
struct kvm_vcpu *vcpu = synic_to_vcpu(synic);
int ret;
- if (!synic->active)
+ if (!synic->active && !host)
return 1;
trace_kvm_hv_synic_set_msr(vcpu->vcpu_id, msr, data, host);
@@ -295,11 +295,12 @@ static int synic_set_msr(struct kvm_vcpu_hv_synic *synic,
return ret;
}
-static int synic_get_msr(struct kvm_vcpu_hv_synic *synic, u32 msr, u64 *pdata)
+static int synic_get_msr(struct kvm_vcpu_hv_synic *synic, u32 msr, u64 *pdata,
+ bool host)
{
int ret;
- if (!synic->active)
+ if (!synic->active && !host)
return 1;
ret = 0;
@@ -1014,6 +1015,11 @@ static int kvm_hv_set_msr_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data,
case HV_X64_MSR_TSC_EMULATION_STATUS:
hv->hv_tsc_emulation_status = data;
break;
+ case HV_X64_MSR_TIME_REF_COUNT:
+ /* read-only, but still ignore it if host-initiated */
+ if (!host)
+ return 1;
+ break;
default:
vcpu_unimpl(vcpu, "Hyper-V uhandled wrmsr: 0x%x data 0x%llx\n",
msr, data);
@@ -1101,6 +1107,12 @@ static int kvm_hv_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host)
return stimer_set_count(vcpu_to_stimer(vcpu, timer_index),
data, host);
}
+ case HV_X64_MSR_TSC_FREQUENCY:
+ case HV_X64_MSR_APIC_FREQUENCY:
+ /* read-only, but still ignore it if host-initiated */
+ if (!host)
+ return 1;
+ break;
default:
vcpu_unimpl(vcpu, "Hyper-V uhandled wrmsr: 0x%x data 0x%llx\n",
msr, data);
@@ -1156,7 +1168,8 @@ static int kvm_hv_get_msr_pw(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
return 0;
}
-static int kvm_hv_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
+static int kvm_hv_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata,
+ bool host)
{
u64 data = 0;
struct kvm_vcpu_hv *hv = &vcpu->arch.hyperv;
@@ -1183,7 +1196,7 @@ static int kvm_hv_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
case HV_X64_MSR_SIMP:
case HV_X64_MSR_EOM:
case HV_X64_MSR_SINT0 ... HV_X64_MSR_SINT15:
- return synic_get_msr(vcpu_to_synic(vcpu), msr, pdata);
+ return synic_get_msr(vcpu_to_synic(vcpu), msr, pdata, host);
case HV_X64_MSR_STIMER0_CONFIG:
case HV_X64_MSR_STIMER1_CONFIG:
case HV_X64_MSR_STIMER2_CONFIG:
@@ -1229,7 +1242,7 @@ int kvm_hv_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host)
return kvm_hv_set_msr(vcpu, msr, data, host);
}
-int kvm_hv_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
+int kvm_hv_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata, bool host)
{
if (kvm_hv_msr_partition_wide(msr)) {
int r;
@@ -1239,7 +1252,7 @@ int kvm_hv_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
mutex_unlock(&vcpu->kvm->arch.hyperv.hv_lock);
return r;
} else
- return kvm_hv_get_msr(vcpu, msr, pdata);
+ return kvm_hv_get_msr(vcpu, msr, pdata, host);
}
static __always_inline int get_sparse_bank_no(u64 valid_bank_mask, int bank_no)
diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h
index 837465d69c6d..d6aa969e20f1 100644
--- a/arch/x86/kvm/hyperv.h
+++ b/arch/x86/kvm/hyperv.h
@@ -48,7 +48,7 @@ static inline struct kvm_vcpu *synic_to_vcpu(struct kvm_vcpu_hv_synic *synic)
}
int kvm_hv_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host);
-int kvm_hv_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata);
+int kvm_hv_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata, bool host);
bool kvm_hv_hypercall_enabled(struct kvm *kvm);
int kvm_hv_hypercall(struct kvm_vcpu *vcpu);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 5dd67d184b17..6cc29dd21519 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2166,10 +2166,11 @@ static int set_msr_mce(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
vcpu->arch.mcg_status = data;
break;
case MSR_IA32_MCG_CTL:
- if (!(mcg_cap & MCG_CTL_P))
+ if (!(mcg_cap & MCG_CTL_P) &&
+ (data || !msr_info->host_initiated))
return 1;
if (data != 0 && data != ~(u64)0)
- return -1;
+ return 1;
vcpu->arch.mcg_ctl = data;
break;
default:
@@ -2557,7 +2558,7 @@ int kvm_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
}
EXPORT_SYMBOL_GPL(kvm_get_msr);
-static int get_msr_mce(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
+static int get_msr_mce(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata, bool host)
{
u64 data;
u64 mcg_cap = vcpu->arch.mcg_cap;
@@ -2572,7 +2573,7 @@ static int get_msr_mce(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
data = vcpu->arch.mcg_cap;
break;
case MSR_IA32_MCG_CTL:
- if (!(mcg_cap & MCG_CTL_P))
+ if (!(mcg_cap & MCG_CTL_P) && !host)
return 1;
data = vcpu->arch.mcg_ctl;
break;
@@ -2705,7 +2706,8 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
case MSR_IA32_MCG_CTL:
case MSR_IA32_MCG_STATUS:
case MSR_IA32_MC0_CTL ... MSR_IA32_MCx_CTL(KVM_MAX_MCE_BANKS) - 1:
- return get_msr_mce(vcpu, msr_info->index, &msr_info->data);
+ return get_msr_mce(vcpu, msr_info->index, &msr_info->data,
+ msr_info->host_initiated);
case MSR_K7_CLK_CTL:
/*
* Provide expected ramp-up count for K7. All other
@@ -2726,7 +2728,8 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
case HV_X64_MSR_TSC_EMULATION_CONTROL:
case HV_X64_MSR_TSC_EMULATION_STATUS:
return kvm_hv_get_msr_common(vcpu,
- msr_info->index, &msr_info->data);
+ msr_info->index, &msr_info->data,
+ msr_info->host_initiated);
break;
case MSR_IA32_BBL_CR_CTL3:
/* This legacy MSR exists but isn't fully documented in current
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 02/10] kvm: selftests: create a GDT and TSS
2018-07-28 23:10 [PATCH v6 00/10] kvm: x86: migration of nested virtualization state Paolo Bonzini
2018-07-28 23:10 ` [PATCH 01/10] KVM: x86: ensure all MSRs can always be KVM_GET/SET_MSR'd Paolo Bonzini
@ 2018-07-28 23:10 ` Paolo Bonzini
2018-07-28 23:10 ` [PATCH 03/10] kvm: selftests: actually use all of lib/vmx.c Paolo Bonzini
` (7 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2018-07-28 23:10 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: Liran Alon, KarimAllah Ahmed, Jim Mattson, rkrcmar
The GDT and the TSS base were left to zero, and this has interesting effects
when the TSS descriptor is later read to set up a VMCS's TR_BASE. Basically
it worked by chance, and this patch fixes it by setting up all the protected
mode data structures properly.
Because the GDT and TSS addresses are virtual, the page tables now always
exist at the time of vcpu setup.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
tools/testing/selftests/kvm/include/kvm_util.h | 2 +-
tools/testing/selftests/kvm/include/x86.h | 11 +-
tools/testing/selftests/kvm/lib/kvm_util.c | 4 +-
.../testing/selftests/kvm/lib/kvm_util_internal.h | 5 +-
tools/testing/selftests/kvm/lib/x86.c | 111 +++++++++++++--------
5 files changed, 86 insertions(+), 47 deletions(-)
diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index 637b7017b6ee..87e05664c7f9 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -75,7 +75,7 @@ void vcpu_ioctl(struct kvm_vm *vm,
uint32_t vcpuid, unsigned long ioctl, void *arg);
void vm_ioctl(struct kvm_vm *vm, unsigned long ioctl, void *arg);
void vm_mem_region_set_flags(struct kvm_vm *vm, uint32_t slot, uint32_t flags);
-void vm_vcpu_add(struct kvm_vm *vm, uint32_t vcpuid);
+void vm_vcpu_add(struct kvm_vm *vm, uint32_t vcpuid, int pgd_memslot, int gdt_memslot);
vm_vaddr_t vm_vaddr_alloc(struct kvm_vm *vm, size_t sz, vm_vaddr_t vaddr_min,
uint32_t data_memslot, uint32_t pgd_memslot);
void *addr_gpa2hva(struct kvm_vm *vm, vm_paddr_t gpa);
diff --git a/tools/testing/selftests/kvm/include/x86.h b/tools/testing/selftests/kvm/include/x86.h
index 4a5b2c4c1a0f..d8788ddb0210 100644
--- a/tools/testing/selftests/kvm/include/x86.h
+++ b/tools/testing/selftests/kvm/include/x86.h
@@ -56,8 +56,8 @@ enum x86_register {
struct desc64 {
uint16_t limit0;
uint16_t base0;
- unsigned base1:8, type:5, dpl:2, p:1;
- unsigned limit1:4, zero0:3, g:1, base2:8;
+ unsigned base1:8, s:1, type:4, dpl:2, p:1;
+ unsigned limit1:4, avl:1, l:1, db:1, g:1, base2:8;
uint32_t base3;
uint32_t zero1;
} __attribute__((packed));
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 37e2a787d2fc..610d1326f03d 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -701,7 +701,7 @@ static int vcpu_mmap_sz(void)
* Creates and adds to the VM specified by vm and virtual CPU with
* the ID given by vcpuid.
*/
-void vm_vcpu_add(struct kvm_vm *vm, uint32_t vcpuid)
+void vm_vcpu_add(struct kvm_vm *vm, uint32_t vcpuid, int pgd_memslot, int gdt_memslot)
{
struct vcpu *vcpu;
@@ -736,7 +736,7 @@ void vm_vcpu_add(struct kvm_vm *vm, uint32_t vcpuid)
vcpu->next = vm->vcpu_head;
vm->vcpu_head = vcpu;
- vcpu_setup(vm, vcpuid);
+ vcpu_setup(vm, vcpuid, pgd_memslot, gdt_memslot);
}
/* VM Virtual Address Unused Gap
diff --git a/tools/testing/selftests/kvm/lib/kvm_util_internal.h b/tools/testing/selftests/kvm/lib/kvm_util_internal.h
index a0bd1980c81c..cbb40288890a 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util_internal.h
+++ b/tools/testing/selftests/kvm/lib/kvm_util_internal.h
@@ -51,13 +51,16 @@ struct kvm_vm {
struct userspace_mem_region *userspace_mem_region_head;
struct sparsebit *vpages_valid;
struct sparsebit *vpages_mapped;
+
bool pgd_created;
vm_paddr_t pgd;
+ vm_vaddr_t gdt;
+ vm_vaddr_t tss;
};
struct vcpu *vcpu_find(struct kvm_vm *vm,
uint32_t vcpuid);
-void vcpu_setup(struct kvm_vm *vm, int vcpuid);
+void vcpu_setup(struct kvm_vm *vm, int vcpuid, int pgd_memslot, int gdt_memslot);
void virt_dump(FILE *stream, struct kvm_vm *vm, uint8_t indent);
void regs_dump(FILE *stream, struct kvm_regs *regs,
uint8_t indent);
diff --git a/tools/testing/selftests/kvm/lib/x86.c b/tools/testing/selftests/kvm/lib/x86.c
index 2f17675f4275..024e95f1b470 100644
--- a/tools/testing/selftests/kvm/lib/x86.c
+++ b/tools/testing/selftests/kvm/lib/x86.c
@@ -239,25 +239,6 @@ void virt_pgd_alloc(struct kvm_vm *vm, uint32_t pgd_memslot)
vm_paddr_t paddr = vm_phy_page_alloc(vm,
KVM_GUEST_PAGE_TABLE_MIN_PADDR, pgd_memslot);
vm->pgd = paddr;
-
- /* Set pointer to pgd tables in all the VCPUs that
- * have already been created. Future VCPUs will have
- * the value set as each one is created.
- */
- for (struct vcpu *vcpu = vm->vcpu_head; vcpu;
- vcpu = vcpu->next) {
- struct kvm_sregs sregs;
-
- /* Obtain the current system register settings */
- vcpu_sregs_get(vm, vcpu->id, &sregs);
-
- /* Set and store the pointer to the start of the
- * pgd tables.
- */
- sregs.cr3 = vm->pgd;
- vcpu_sregs_set(vm, vcpu->id, &sregs);
- }
-
vm->pgd_created = true;
}
}
@@ -460,9 +441,32 @@ static void kvm_seg_set_unusable(struct kvm_segment *segp)
segp->unusable = true;
}
+static void kvm_seg_fill_gdt_64bit(struct kvm_vm *vm, struct kvm_segment *segp)
+{
+ void *gdt = addr_gva2hva(vm, vm->gdt);
+ struct desc64 *desc = gdt + (segp->selector >> 3) * 8;
+
+ desc->limit0 = segp->limit & 0xFFFF;
+ desc->base0 = segp->base & 0xFFFF;
+ desc->base1 = segp->base >> 16;
+ desc->s = segp->s;
+ desc->type = segp->type;
+ desc->dpl = segp->dpl;
+ desc->p = segp->present;
+ desc->limit1 = segp->limit >> 16;
+ desc->l = segp->l;
+ desc->db = segp->db;
+ desc->g = segp->g;
+ desc->base2 = segp->base >> 24;
+ if (!segp->s)
+ desc->base3 = segp->base >> 32;
+}
+
+
/* Set Long Mode Flat Kernel Code Segment
*
* Input Args:
+ * vm - VM whose GDT is being filled, or NULL to only write segp
* selector - selector value
*
* Output Args:
@@ -473,7 +477,7 @@ static void kvm_seg_set_unusable(struct kvm_segment *segp)
* Sets up the KVM segment pointed to by segp, to be a code segment
* with the selector value given by selector.
*/
-static void kvm_seg_set_kernel_code_64bit(uint16_t selector,
+static void kvm_seg_set_kernel_code_64bit(struct kvm_vm *vm, uint16_t selector,
struct kvm_segment *segp)
{
memset(segp, 0, sizeof(*segp));
@@ -486,11 +490,14 @@ static void kvm_seg_set_kernel_code_64bit(uint16_t selector,
segp->g = true;
segp->l = true;
segp->present = 1;
+ if (vm)
+ kvm_seg_fill_gdt_64bit(vm, segp);
}
/* Set Long Mode Flat Kernel Data Segment
*
* Input Args:
+ * vm - VM whose GDT is being filled, or NULL to only write segp
* selector - selector value
*
* Output Args:
@@ -501,7 +508,7 @@ static void kvm_seg_set_kernel_code_64bit(uint16_t selector,
* Sets up the KVM segment pointed to by segp, to be a data segment
* with the selector value given by selector.
*/
-static void kvm_seg_set_kernel_data_64bit(uint16_t selector,
+static void kvm_seg_set_kernel_data_64bit(struct kvm_vm *vm, uint16_t selector,
struct kvm_segment *segp)
{
memset(segp, 0, sizeof(*segp));
@@ -513,6 +520,8 @@ static void kvm_seg_set_kernel_data_64bit(uint16_t selector,
*/
segp->g = true;
segp->present = true;
+ if (vm)
+ kvm_seg_fill_gdt_64bit(vm, segp);
}
/* Address Guest Virtual to Guest Physical
@@ -575,13 +584,45 @@ vm_paddr_t addr_gva2gpa(struct kvm_vm *vm, vm_vaddr_t gva)
"gva: 0x%lx", gva);
}
-void vcpu_setup(struct kvm_vm *vm, int vcpuid)
+static void kvm_setup_gdt(struct kvm_vm *vm, struct kvm_dtable *dt, int gdt_memslot,
+ int pgd_memslot)
+{
+ if (!vm->gdt)
+ vm->gdt = vm_vaddr_alloc(vm, getpagesize(),
+ KVM_UTIL_MIN_VADDR, gdt_memslot, pgd_memslot);
+
+ dt->base = vm->gdt;
+ dt->limit = getpagesize();
+}
+
+static void kvm_setup_tss_64bit(struct kvm_vm *vm, struct kvm_segment *segp,
+ int selector, int gdt_memslot,
+ int pgd_memslot)
+{
+ if (!vm->tss)
+ vm->tss = vm_vaddr_alloc(vm, getpagesize(),
+ KVM_UTIL_MIN_VADDR, gdt_memslot, pgd_memslot);
+
+ memset(segp, 0, sizeof(*segp));
+ segp->base = vm->tss;
+ segp->limit = 0x67;
+ segp->selector = selector;
+ segp->type = 0xb;
+ segp->present = 1;
+ kvm_seg_fill_gdt_64bit(vm, segp);
+}
+
+void vcpu_setup(struct kvm_vm *vm, int vcpuid, int pgd_memslot, int gdt_memslot)
{
struct kvm_sregs sregs;
/* Set mode specific system register values. */
vcpu_sregs_get(vm, vcpuid, &sregs);
+ sregs.idt.limit = 0;
+
+ kvm_setup_gdt(vm, &sregs.gdt, gdt_memslot, pgd_memslot);
+
switch (vm->mode) {
case VM_MODE_FLAT48PG:
sregs.cr0 = X86_CR0_PE | X86_CR0_NE | X86_CR0_PG;
@@ -589,30 +630,18 @@ void vcpu_setup(struct kvm_vm *vm, int vcpuid)
sregs.efer |= (EFER_LME | EFER_LMA | EFER_NX);
kvm_seg_set_unusable(&sregs.ldt);
- kvm_seg_set_kernel_code_64bit(0x8, &sregs.cs);
- kvm_seg_set_kernel_data_64bit(0x10, &sregs.ds);
- kvm_seg_set_kernel_data_64bit(0x10, &sregs.es);
+ kvm_seg_set_kernel_code_64bit(vm, 0x8, &sregs.cs);
+ kvm_seg_set_kernel_data_64bit(vm, 0x10, &sregs.ds);
+ kvm_seg_set_kernel_data_64bit(vm, 0x10, &sregs.es);
+ kvm_setup_tss_64bit(vm, &sregs.tr, 0x18, gdt_memslot, pgd_memslot);
break;
default:
TEST_ASSERT(false, "Unknown guest mode, mode: 0x%x", vm->mode);
}
- vcpu_sregs_set(vm, vcpuid, &sregs);
-
- /* If virtual translation table have been setup, set system register
- * to point to the tables. It's okay if they haven't been setup yet,
- * in that the code that sets up the virtual translation tables, will
- * go back through any VCPUs that have already been created and set
- * their values.
- */
- if (vm->pgd_created) {
- struct kvm_sregs sregs;
- vcpu_sregs_get(vm, vcpuid, &sregs);
-
- sregs.cr3 = vm->pgd;
- vcpu_sregs_set(vm, vcpuid, &sregs);
- }
+ sregs.cr3 = vm->pgd;
+ vcpu_sregs_set(vm, vcpuid, &sregs);
}
/* Adds a vCPU with reasonable defaults (i.e., a stack)
*
@@ -629,7 +658,7 @@ void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code)
DEFAULT_GUEST_STACK_VADDR_MIN, 0, 0);
/* Create VCPU */
- vm_vcpu_add(vm, vcpuid);
+ vm_vcpu_add(vm, vcpuid, 0, 0);
/* Setup guest general purpose registers */
vcpu_regs_get(vm, vcpuid, ®s);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 03/10] kvm: selftests: actually use all of lib/vmx.c
2018-07-28 23:10 [PATCH v6 00/10] kvm: x86: migration of nested virtualization state Paolo Bonzini
2018-07-28 23:10 ` [PATCH 01/10] KVM: x86: ensure all MSRs can always be KVM_GET/SET_MSR'd Paolo Bonzini
2018-07-28 23:10 ` [PATCH 02/10] kvm: selftests: create a GDT and TSS Paolo Bonzini
@ 2018-07-28 23:10 ` Paolo Bonzini
2018-07-28 23:10 ` [PATCH 04/10] kvm: selftests: ensure vcpu file is released Paolo Bonzini
` (6 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2018-07-28 23:10 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: Liran Alon, KarimAllah Ahmed, Jim Mattson, rkrcmar
The allocation of the VMXON and VMCS is currently done twice, in
lib/vmx.c and in vmx_tsc_adjust_test.c. Reorganize the code to
provide a cleaner and easier to use API to the tests. lib/vmx.c
now does the complete setup of the VMX data structures, but does not
create the VM or set CPUID. This has to be done by the caller.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
tools/testing/selftests/kvm/cr4_cpuid_sync_test.c | 2 +-
tools/testing/selftests/kvm/include/vmx.h | 21 +++++--
tools/testing/selftests/kvm/lib/vmx.c | 71 ++++++++++++++---------
tools/testing/selftests/kvm/vmx_tsc_adjust_test.c | 69 +++++-----------------
4 files changed, 73 insertions(+), 90 deletions(-)
diff --git a/tools/testing/selftests/kvm/cr4_cpuid_sync_test.c b/tools/testing/selftests/kvm/cr4_cpuid_sync_test.c
index dbbaf3c3fbfe..8346b33c2073 100644
--- a/tools/testing/selftests/kvm/cr4_cpuid_sync_test.c
+++ b/tools/testing/selftests/kvm/cr4_cpuid_sync_test.c
@@ -95,7 +95,7 @@ int main(int argc, char *argv[])
setbuf(stdout, NULL);
/* Create VM */
- vm = vm_create_default_vmx(VCPU_ID, guest_code);
+ vm = vm_create_default(VCPU_ID, guest_code);
vcpu_set_cpuid(vm, VCPU_ID, kvm_get_supported_cpuid());
run = vcpu_state(vm, VCPU_ID);
diff --git a/tools/testing/selftests/kvm/include/vmx.h b/tools/testing/selftests/kvm/include/vmx.h
index 6ed8499807fd..9caaf56696a4 100644
--- a/tools/testing/selftests/kvm/include/vmx.h
+++ b/tools/testing/selftests/kvm/include/vmx.h
@@ -486,9 +486,22 @@ static inline uint32_t vmcs_revision(void)
return rdmsr(MSR_IA32_VMX_BASIC);
}
-void prepare_for_vmx_operation(void);
-void prepare_vmcs(void *guest_rip, void *guest_rsp);
-struct kvm_vm *vm_create_default_vmx(uint32_t vcpuid,
- vmx_guest_code_t guest_code);
+struct vmx_pages {
+ void *vmxon_hva;
+ uint64_t vmxon_gpa;
+ void *vmxon;
+
+ void *vmcs_hva;
+ uint64_t vmcs_gpa;
+ void *vmcs;
+
+ void *msr_hva;
+ uint64_t msr_gpa;
+ void *msr;
+};
+
+struct vmx_pages *vcpu_alloc_vmx(struct kvm_vm *vm, vm_vaddr_t *p_vmx_gva);
+bool prepare_for_vmx_operation(struct vmx_pages *vmx);
+void prepare_vmcs(struct vmx_pages *vmx, void *guest_rip, void *guest_rsp);
#endif /* !SELFTEST_KVM_VMX_H */
diff --git a/tools/testing/selftests/kvm/lib/vmx.c b/tools/testing/selftests/kvm/lib/vmx.c
index 0231bc0aae7b..5701e52a33ed 100644
--- a/tools/testing/selftests/kvm/lib/vmx.c
+++ b/tools/testing/selftests/kvm/lib/vmx.c
@@ -13,47 +13,43 @@
#include "x86.h"
#include "vmx.h"
-/* Create a default VM for VMX tests.
+/* Allocate memory regions for nested VMX tests.
*
* Input Args:
- * vcpuid - The id of the single VCPU to add to the VM.
- * guest_code - The vCPU's entry point
+ * vm - The VM to allocate guest-virtual addresses in.
*
- * Output Args: None
+ * Output Args:
+ * p_vmx_gva - The guest virtual address for the struct vmx_pages.
*
* Return:
- * Pointer to opaque structure that describes the created VM.
+ * Pointer to structure with the addresses of the VMX areas.
*/
-struct kvm_vm *
-vm_create_default_vmx(uint32_t vcpuid, vmx_guest_code_t guest_code)
+struct vmx_pages *
+vcpu_alloc_vmx(struct kvm_vm *vm, vm_vaddr_t *p_vmx_gva)
{
- struct kvm_cpuid2 *cpuid;
- struct kvm_vm *vm;
- vm_vaddr_t vmxon_vaddr;
- vm_paddr_t vmxon_paddr;
- vm_vaddr_t vmcs_vaddr;
- vm_paddr_t vmcs_paddr;
-
- vm = vm_create_default(vcpuid, (void *) guest_code);
-
- /* Enable nesting in CPUID */
- vcpu_set_cpuid(vm, vcpuid, kvm_get_supported_cpuid());
+ vm_vaddr_t vmx_gva = vm_vaddr_alloc(vm, getpagesize(), 0x10000, 0, 0);
+ struct vmx_pages *vmx = addr_gva2hva(vm, vmx_gva);
/* Setup of a region of guest memory for the vmxon region. */
- vmxon_vaddr = vm_vaddr_alloc(vm, getpagesize(), 0, 0, 0);
- vmxon_paddr = addr_gva2gpa(vm, vmxon_vaddr);
+ vmx->vmxon = (void *)vm_vaddr_alloc(vm, getpagesize(), 0x10000, 0, 0);
+ vmx->vmxon_hva = addr_gva2hva(vm, (uintptr_t)vmx->vmxon);
+ vmx->vmxon_gpa = addr_gva2gpa(vm, (uintptr_t)vmx->vmxon);
/* Setup of a region of guest memory for a vmcs. */
- vmcs_vaddr = vm_vaddr_alloc(vm, getpagesize(), 0, 0, 0);
- vmcs_paddr = addr_gva2gpa(vm, vmcs_vaddr);
+ vmx->vmcs = (void *)vm_vaddr_alloc(vm, getpagesize(), 0x10000, 0, 0);
+ vmx->vmcs_hva = addr_gva2hva(vm, (uintptr_t)vmx->vmcs);
+ vmx->vmcs_gpa = addr_gva2gpa(vm, (uintptr_t)vmx->vmcs);
- vcpu_args_set(vm, vcpuid, 4, vmxon_vaddr, vmxon_paddr, vmcs_vaddr,
- vmcs_paddr);
+ /* Setup of a region of guest memory for the MSR bitmap. */
+ vmx->msr = (void *)vm_vaddr_alloc(vm, getpagesize(), 0x10000, 0, 0);
+ vmx->msr_hva = addr_gva2hva(vm, (uintptr_t)vmx->msr);
+ vmx->msr_gpa = addr_gva2gpa(vm, (uintptr_t)vmx->msr);
- return vm;
+ *p_vmx_gva = vmx_gva;
+ return vmx;
}
-void prepare_for_vmx_operation(void)
+bool prepare_for_vmx_operation(struct vmx_pages *vmx)
{
uint64_t feature_control;
uint64_t required;
@@ -88,12 +84,27 @@ void prepare_for_vmx_operation(void)
feature_control = rdmsr(MSR_IA32_FEATURE_CONTROL);
if ((feature_control & required) != required)
wrmsr(MSR_IA32_FEATURE_CONTROL, feature_control | required);
+
+ /* Enter VMX root operation. */
+ *(uint32_t *)(vmx->vmxon) = vmcs_revision();
+ if (vmxon(vmx->vmxon_gpa))
+ return false;
+
+ /* Load a VMCS. */
+ *(uint32_t *)(vmx->vmcs) = vmcs_revision();
+ if (vmclear(vmx->vmcs_gpa))
+ return false;
+
+ if (vmptrld(vmx->vmcs_gpa))
+ return false;
+
+ return true;
}
/*
* Initialize the control fields to the most basic settings possible.
*/
-static inline void init_vmcs_control_fields(void)
+static inline void init_vmcs_control_fields(struct vmx_pages *vmx)
{
vmwrite(VIRTUAL_PROCESSOR_ID, 0);
vmwrite(POSTED_INTR_NV, 0);
@@ -119,6 +130,8 @@ static inline void init_vmcs_control_fields(void)
vmwrite(CR4_GUEST_HOST_MASK, 0);
vmwrite(CR0_READ_SHADOW, get_cr0());
vmwrite(CR4_READ_SHADOW, get_cr4());
+
+ vmwrite(MSR_BITMAP, vmx->msr_gpa);
}
/*
@@ -235,9 +248,9 @@ static inline void init_vmcs_guest_state(void *rip, void *rsp)
vmwrite(GUEST_SYSENTER_EIP, vmreadz(HOST_IA32_SYSENTER_EIP));
}
-void prepare_vmcs(void *guest_rip, void *guest_rsp)
+void prepare_vmcs(struct vmx_pages *vmx, void *guest_rip, void *guest_rsp)
{
- init_vmcs_control_fields();
+ init_vmcs_control_fields(vmx);
init_vmcs_host_state();
init_vmcs_guest_state(guest_rip, guest_rsp);
}
diff --git a/tools/testing/selftests/kvm/vmx_tsc_adjust_test.c b/tools/testing/selftests/kvm/vmx_tsc_adjust_test.c
index d7cb7944a42e..fc414c284368 100644
--- a/tools/testing/selftests/kvm/vmx_tsc_adjust_test.c
+++ b/tools/testing/selftests/kvm/vmx_tsc_adjust_test.c
@@ -46,11 +46,6 @@ enum {
PORT_DONE,
};
-struct vmx_page {
- vm_vaddr_t virt;
- vm_paddr_t phys;
-};
-
enum {
VMXON_PAGE = 0,
VMCS_PAGE,
@@ -67,9 +62,6 @@ struct kvm_single_msr {
/* The virtual machine object. */
static struct kvm_vm *vm;
-/* Array of vmx_page descriptors that is shared with the guest. */
-struct vmx_page *vmx_pages;
-
#define exit_to_l0(_port, _arg) do_exit_to_l0(_port, (unsigned long) (_arg))
static void do_exit_to_l0(uint16_t port, unsigned long arg)
{
@@ -105,7 +97,7 @@ static void l2_guest_code(void)
__asm__ __volatile__("vmcall");
}
-static void l1_guest_code(struct vmx_page *vmx_pages)
+static void l1_guest_code(struct vmx_pages *vmx_pages)
{
#define L2_GUEST_STACK_SIZE 64
unsigned long l2_guest_stack[L2_GUEST_STACK_SIZE];
@@ -116,23 +108,14 @@ static void l1_guest_code(struct vmx_page *vmx_pages)
wrmsr(MSR_IA32_TSC, rdtsc() - TSC_ADJUST_VALUE);
check_ia32_tsc_adjust(-1 * TSC_ADJUST_VALUE);
- prepare_for_vmx_operation();
-
- /* Enter VMX root operation. */
- *(uint32_t *)vmx_pages[VMXON_PAGE].virt = vmcs_revision();
- GUEST_ASSERT(!vmxon(vmx_pages[VMXON_PAGE].phys));
-
- /* Load a VMCS. */
- *(uint32_t *)vmx_pages[VMCS_PAGE].virt = vmcs_revision();
- GUEST_ASSERT(!vmclear(vmx_pages[VMCS_PAGE].phys));
- GUEST_ASSERT(!vmptrld(vmx_pages[VMCS_PAGE].phys));
+ GUEST_ASSERT(prepare_for_vmx_operation(vmx_pages));
/* Prepare the VMCS for L2 execution. */
- prepare_vmcs(l2_guest_code, &l2_guest_stack[L2_GUEST_STACK_SIZE]);
+ prepare_vmcs(vmx_pages, l2_guest_code,
+ &l2_guest_stack[L2_GUEST_STACK_SIZE]);
control = vmreadz(CPU_BASED_VM_EXEC_CONTROL);
control |= CPU_BASED_USE_MSR_BITMAPS | CPU_BASED_USE_TSC_OFFSETING;
vmwrite(CPU_BASED_VM_EXEC_CONTROL, control);
- vmwrite(MSR_BITMAP, vmx_pages[MSR_BITMAP_PAGE].phys);
vmwrite(TSC_OFFSET, TSC_OFFSET_VALUE);
/* Jump into L2. First, test failure to load guest CR3. */
@@ -152,33 +135,6 @@ static void l1_guest_code(struct vmx_page *vmx_pages)
exit_to_l0(PORT_DONE, 0);
}
-static void allocate_vmx_page(struct vmx_page *page)
-{
- vm_vaddr_t virt;
-
- virt = vm_vaddr_alloc(vm, PAGE_SIZE, 0, 0, 0);
- memset(addr_gva2hva(vm, virt), 0, PAGE_SIZE);
-
- page->virt = virt;
- page->phys = addr_gva2gpa(vm, virt);
-}
-
-static vm_vaddr_t allocate_vmx_pages(void)
-{
- vm_vaddr_t vmx_pages_vaddr;
- int i;
-
- vmx_pages_vaddr = vm_vaddr_alloc(
- vm, sizeof(struct vmx_page) * NUM_VMX_PAGES, 0, 0, 0);
-
- vmx_pages = (void *) addr_gva2hva(vm, vmx_pages_vaddr);
-
- for (i = 0; i < NUM_VMX_PAGES; i++)
- allocate_vmx_page(&vmx_pages[i]);
-
- return vmx_pages_vaddr;
-}
-
void report(int64_t val)
{
printf("IA32_TSC_ADJUST is %ld (%lld * TSC_ADJUST_VALUE + %lld).\n",
@@ -187,7 +143,8 @@ void report(int64_t val)
int main(int argc, char *argv[])
{
- vm_vaddr_t vmx_pages_vaddr;
+ struct vmx_pages *vmx_pages;
+ vm_vaddr_t vmx_pages_gva;
struct kvm_cpuid_entry2 *entry = kvm_get_supported_cpuid_entry(1);
if (!(entry->ecx & CPUID_VMX)) {
@@ -195,23 +152,23 @@ int main(int argc, char *argv[])
exit(KSFT_SKIP);
}
- vm = vm_create_default_vmx(VCPU_ID, (void *) l1_guest_code);
+ vm = vm_create_default(VCPU_ID, (void *) l1_guest_code);
+ vcpu_set_cpuid(vm, VCPU_ID, kvm_get_supported_cpuid());
/* Allocate VMX pages and shared descriptors (vmx_pages). */
- vmx_pages_vaddr = allocate_vmx_pages();
- vcpu_args_set(vm, VCPU_ID, 1, vmx_pages_vaddr);
+ vmx_pages = vcpu_alloc_vmx(vm, &vmx_pages_gva);
+ vcpu_args_set(vm, VCPU_ID, 1, vmx_pages_gva);
for (;;) {
volatile struct kvm_run *run = vcpu_state(vm, VCPU_ID);
struct kvm_regs regs;
vcpu_run(vm, VCPU_ID);
+ vcpu_regs_get(vm, VCPU_ID, ®s);
TEST_ASSERT(run->exit_reason == KVM_EXIT_IO,
- "Got exit_reason other than KVM_EXIT_IO: %u (%s),\n",
+ "Got exit_reason other than KVM_EXIT_IO: %u (%s), rip=%lx\n",
run->exit_reason,
- exit_reason_str(run->exit_reason));
-
- vcpu_regs_get(vm, VCPU_ID, ®s);
+ exit_reason_str(run->exit_reason), regs.rip);
switch (run->io.port) {
case PORT_ABORT:
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 04/10] kvm: selftests: ensure vcpu file is released
2018-07-28 23:10 [PATCH v6 00/10] kvm: x86: migration of nested virtualization state Paolo Bonzini
` (2 preceding siblings ...)
2018-07-28 23:10 ` [PATCH 03/10] kvm: selftests: actually use all of lib/vmx.c Paolo Bonzini
@ 2018-07-28 23:10 ` Paolo Bonzini
2018-07-28 23:10 ` [PATCH 05/10] kvm: selftests: add basic test for state save and restore Paolo Bonzini
` (5 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2018-07-28 23:10 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: Liran Alon, KarimAllah Ahmed, Jim Mattson, rkrcmar
The selftests were not munmap-ing the kvm_run area from the vcpu file descriptor.
The result was that kvm_vcpu_release was not called and a reference was left in the
parent "struct kvm". Ultimately this was visible in the upcoming state save/restore
test as an error when KVM attempted to create a duplicate debugfs entry.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
tools/testing/selftests/kvm/lib/kvm_util.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 610d1326f03d..163482873363 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -238,8 +238,12 @@ struct vcpu *vcpu_find(struct kvm_vm *vm,
static void vm_vcpu_rm(struct kvm_vm *vm, uint32_t vcpuid)
{
struct vcpu *vcpu = vcpu_find(vm, vcpuid);
+ int ret;
- int ret = close(vcpu->fd);
+ ret = munmap(vcpu->state, sizeof(*vcpu->state));
+ TEST_ASSERT(ret == 0, "munmap of VCPU fd failed, rc: %i "
+ "errno: %i", ret, errno);
+ close(vcpu->fd);
TEST_ASSERT(ret == 0, "Close of VCPU fd failed, rc: %i "
"errno: %i", ret, errno);
@@ -295,6 +299,10 @@ void kvm_vm_free(struct kvm_vm *vmp)
TEST_ASSERT(ret == 0, "Close of vm fd failed,\n"
" vmp->fd: %i rc: %i errno: %i", vmp->fd, ret, errno);
+ close(vmp->kvm_fd);
+ TEST_ASSERT(ret == 0, "Close of /dev/kvm fd failed,\n"
+ " vmp->kvm_fd: %i rc: %i errno: %i", vmp->kvm_fd, ret, errno);
+
/* Free the structure describing the VM. */
free(vmp);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 05/10] kvm: selftests: add basic test for state save and restore
2018-07-28 23:10 [PATCH v6 00/10] kvm: x86: migration of nested virtualization state Paolo Bonzini
` (3 preceding siblings ...)
2018-07-28 23:10 ` [PATCH 04/10] kvm: selftests: ensure vcpu file is released Paolo Bonzini
@ 2018-07-28 23:10 ` Paolo Bonzini
2018-07-28 23:10 ` [PATCH 06/10] KVM: x86: do not load vmcs12 pages while still in SMM Paolo Bonzini
` (4 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2018-07-28 23:10 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: Liran Alon, KarimAllah Ahmed, Jim Mattson, rkrcmar
The test calls KVM_RUN repeatedly, and creates an entirely new VM with the
old memory and vCPU state on every exit to userspace. The kvm_util API is
expanded with two functions that manage the lifetime of a kvm_vm struct:
the first closes the file descriptors and leaves the memory allocated,
and the second opens the file descriptors and reuses the memory from
the previous incarnation of the kvm_vm struct.
For now the test is very basic, as it does not test for example XSAVE or
vCPU events. However, it will test nested virtualization state starting
with the next patch.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
tools/testing/selftests/kvm/Makefile | 1 +
tools/testing/selftests/kvm/include/kvm_util.h | 2 +
tools/testing/selftests/kvm/include/x86.h | 4 +
tools/testing/selftests/kvm/lib/kvm_util.c | 88 +++++++++++----
.../testing/selftests/kvm/lib/kvm_util_internal.h | 2 +
tools/testing/selftests/kvm/lib/x86.c | 122 +++++++++++++++++++++
tools/testing/selftests/kvm/state_test.c | 119 ++++++++++++++++++++
7 files changed, 316 insertions(+), 22 deletions(-)
create mode 100644 tools/testing/selftests/kvm/state_test.c
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 65bda4fb0ad6..dd0e5163f01f 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -10,6 +10,7 @@ TEST_GEN_PROGS_x86_64 = set_sregs_test
TEST_GEN_PROGS_x86_64 += sync_regs_test
TEST_GEN_PROGS_x86_64 += vmx_tsc_adjust_test
TEST_GEN_PROGS_x86_64 += cr4_cpuid_sync_test
+TEST_GEN_PROGS_x86_64 += state_test
TEST_GEN_PROGS += $(TEST_GEN_PROGS_$(UNAME_M))
LIBKVM += $(LIBKVM_$(UNAME_M))
diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index 87e05664c7f9..d32632f71ab8 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -53,6 +53,8 @@ enum vm_mem_backing_src_type {
struct kvm_vm *vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm);
void kvm_vm_free(struct kvm_vm *vmp);
+void kvm_vm_restart(struct kvm_vm *vmp, int perm);
+void kvm_vm_release(struct kvm_vm *vmp);
int kvm_memcmp_hva_gva(void *hva,
struct kvm_vm *vm, const vm_vaddr_t gva, size_t len);
diff --git a/tools/testing/selftests/kvm/include/x86.h b/tools/testing/selftests/kvm/include/x86.h
index d8788ddb0210..2559f6d2d683 100644
--- a/tools/testing/selftests/kvm/include/x86.h
+++ b/tools/testing/selftests/kvm/include/x86.h
@@ -310,6 +310,10 @@ static inline unsigned long get_xmm(int n)
return 0;
}
+struct kvm_x86_state;
+struct kvm_x86_state *vcpu_save_state(struct kvm_vm *vm, uint32_t vcpuid);
+void vcpu_load_state(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_x86_state *state);
+
/*
* Basic CPU control in CR0
*/
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 163482873363..643309d6de74 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -62,6 +62,18 @@ int kvm_check_cap(long cap)
return ret;
}
+static void vm_open(struct kvm_vm *vm, int perm)
+{
+ vm->kvm_fd = open(KVM_DEV_PATH, perm);
+ if (vm->kvm_fd < 0)
+ exit(KSFT_SKIP);
+
+ /* Create VM. */
+ vm->fd = ioctl(vm->kvm_fd, KVM_CREATE_VM, NULL);
+ TEST_ASSERT(vm->fd >= 0, "KVM_CREATE_VM ioctl failed, "
+ "rc: %i errno: %i", vm->fd, errno);
+}
+
/* VM Create
*
* Input Args:
@@ -90,16 +102,7 @@ struct kvm_vm *vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm)
TEST_ASSERT(vm != NULL, "Insufficent Memory");
vm->mode = mode;
- kvm_fd = open(KVM_DEV_PATH, perm);
- if (kvm_fd < 0)
- exit(KSFT_SKIP);
-
- /* Create VM. */
- vm->fd = ioctl(kvm_fd, KVM_CREATE_VM, NULL);
- TEST_ASSERT(vm->fd >= 0, "KVM_CREATE_VM ioctl failed, "
- "rc: %i errno: %i", vm->fd, errno);
-
- close(kvm_fd);
+ vm_open(vm, perm);
/* Setup mode specific traits. */
switch (vm->mode) {
@@ -132,6 +135,39 @@ struct kvm_vm *vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm)
return vm;
}
+/* VM Restart
+ *
+ * Input Args:
+ * vm - VM that has been released before
+ * perm - permission
+ *
+ * Output Args: None
+ *
+ * Reopens the file descriptors associated to the VM and reinstates the
+ * global state, such as the irqchip and the memory regions that are mapped
+ * into the guest.
+ */
+void kvm_vm_restart(struct kvm_vm *vmp, int perm)
+{
+ struct userspace_mem_region *region;
+
+ vm_open(vmp, perm);
+ if (vmp->has_irqchip)
+ vm_create_irqchip(vmp);
+
+ for (region = vmp->userspace_mem_region_head; region;
+ region = region->next) {
+ int ret = ioctl(vmp->fd, KVM_SET_USER_MEMORY_REGION, ®ion->region);
+ TEST_ASSERT(ret == 0, "KVM_SET_USER_MEMORY_REGION IOCTL failed,\n"
+ " rc: %i errno: %i\n"
+ " slot: %u flags: 0x%x\n"
+ " guest_phys_addr: 0x%lx size: 0x%lx",
+ ret, errno, region->region.slot, region->region.flags,
+ region->region.guest_phys_addr,
+ region->region.memory_size);
+ }
+}
+
/* Userspace Memory Region Find
*
* Input Args:
@@ -256,6 +292,23 @@ static void vm_vcpu_rm(struct kvm_vm *vm, uint32_t vcpuid)
free(vcpu);
}
+void kvm_vm_release(struct kvm_vm *vmp)
+{
+ int ret;
+
+ /* Free VCPUs. */
+ while (vmp->vcpu_head)
+ vm_vcpu_rm(vmp, vmp->vcpu_head->id);
+
+ /* Close file descriptor for the VM. */
+ ret = close(vmp->fd);
+ TEST_ASSERT(ret == 0, "Close of vm fd failed,\n"
+ " vmp->fd: %i rc: %i errno: %i", vmp->fd, ret, errno);
+
+ close(vmp->kvm_fd);
+ TEST_ASSERT(ret == 0, "Close of /dev/kvm fd failed,\n"
+ " vmp->kvm_fd: %i rc: %i errno: %i", vmp->kvm_fd, ret, errno);
+}
/* Destroys and frees the VM pointed to by vmp.
*/
@@ -286,22 +339,11 @@ void kvm_vm_free(struct kvm_vm *vmp)
free(region);
}
- /* Free VCPUs. */
- while (vmp->vcpu_head)
- vm_vcpu_rm(vmp, vmp->vcpu_head->id);
-
/* Free sparsebit arrays. */
sparsebit_free(&vmp->vpages_valid);
sparsebit_free(&vmp->vpages_mapped);
- /* Close file descriptor for the VM. */
- ret = close(vmp->fd);
- TEST_ASSERT(ret == 0, "Close of vm fd failed,\n"
- " vmp->fd: %i rc: %i errno: %i", vmp->fd, ret, errno);
-
- close(vmp->kvm_fd);
- TEST_ASSERT(ret == 0, "Close of /dev/kvm fd failed,\n"
- " vmp->kvm_fd: %i rc: %i errno: %i", vmp->kvm_fd, ret, errno);
+ kvm_vm_release(vmp);
/* Free the structure describing the VM. */
free(vmp);
@@ -965,6 +1007,8 @@ void vm_create_irqchip(struct kvm_vm *vm)
ret = ioctl(vm->fd, KVM_CREATE_IRQCHIP, 0);
TEST_ASSERT(ret == 0, "KVM_CREATE_IRQCHIP IOCTL failed, "
"rc: %i errno: %i", ret, errno);
+
+ vm->has_irqchip = true;
}
/* VM VCPU State
diff --git a/tools/testing/selftests/kvm/lib/kvm_util_internal.h b/tools/testing/selftests/kvm/lib/kvm_util_internal.h
index cbb40288890a..542ed606b338 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util_internal.h
+++ b/tools/testing/selftests/kvm/lib/kvm_util_internal.h
@@ -43,6 +43,7 @@ struct vcpu {
struct kvm_vm {
int mode;
+ int kvm_fd;
int fd;
unsigned int page_size;
unsigned int page_shift;
@@ -52,6 +53,7 @@ struct kvm_vm {
struct sparsebit *vpages_valid;
struct sparsebit *vpages_mapped;
+ bool has_irqchip;
bool pgd_created;
vm_paddr_t pgd;
vm_vaddr_t gdt;
diff --git a/tools/testing/selftests/kvm/lib/x86.c b/tools/testing/selftests/kvm/lib/x86.c
index 024e95f1b470..78aabcb91de1 100644
--- a/tools/testing/selftests/kvm/lib/x86.c
+++ b/tools/testing/selftests/kvm/lib/x86.c
@@ -727,3 +727,119 @@ struct kvm_vm *vm_create_default(uint32_t vcpuid, void *guest_code)
return vm;
}
+
+struct kvm_x86_state {
+ struct kvm_vcpu_events events;
+ struct kvm_mp_state mp_state;
+ struct kvm_regs regs;
+ struct kvm_xsave xsave;
+ struct kvm_xcrs xcrs;
+ struct kvm_sregs sregs;
+ struct kvm_debugregs debugregs;
+ struct kvm_msrs msrs;
+};
+
+static int kvm_get_num_msrs(struct kvm_vm *vm)
+{
+ struct kvm_msr_list nmsrs;
+ int r;
+
+ nmsrs.nmsrs = 0;
+ r = ioctl(vm->kvm_fd, KVM_GET_MSR_INDEX_LIST, &nmsrs);
+ TEST_ASSERT(r == -1 && errno == E2BIG, "Unexpected result from KVM_GET_MSR_INDEX_LIST probe, r: %i",
+ r);
+
+ return nmsrs.nmsrs;
+}
+
+struct kvm_x86_state *vcpu_save_state(struct kvm_vm *vm, uint32_t vcpuid)
+{
+ struct vcpu *vcpu = vcpu_find(vm, vcpuid);
+ struct kvm_msr_list *list;
+ struct kvm_x86_state *state;
+ int nmsrs, r, i;
+
+ nmsrs = kvm_get_num_msrs(vm);
+ list = malloc(sizeof(*list) + nmsrs * sizeof(list->indices[0]));
+ list->nmsrs = nmsrs;
+ r = ioctl(vm->kvm_fd, KVM_GET_MSR_INDEX_LIST, list);
+ TEST_ASSERT(r == 0, "Unexpected result from KVM_GET_MSR_INDEX_LIST, r: %i",
+ r);
+
+ state = malloc(sizeof(*state) + nmsrs * sizeof(state->msrs.entries[0]));
+ r = ioctl(vcpu->fd, KVM_GET_VCPU_EVENTS, &state->events);
+ TEST_ASSERT(r == 0, "Unexpected result from KVM_GET_VCPU_EVENTS, r: %i",
+ r);
+
+ r = ioctl(vcpu->fd, KVM_GET_MP_STATE, &state->mp_state);
+ TEST_ASSERT(r == 0, "Unexpected result from KVM_GET_MP_STATE, r: %i",
+ r);
+
+ r = ioctl(vcpu->fd, KVM_GET_REGS, &state->regs);
+ TEST_ASSERT(r == 0, "Unexpected result from KVM_GET_REGS, r: %i",
+ r);
+
+ r = ioctl(vcpu->fd, KVM_GET_XSAVE, &state->xsave);
+ TEST_ASSERT(r == 0, "Unexpected result from KVM_GET_XSAVE, r: %i",
+ r);
+
+ r = ioctl(vcpu->fd, KVM_GET_XCRS, &state->xcrs);
+ TEST_ASSERT(r == 0, "Unexpected result from KVM_GET_XCRS, r: %i",
+ r);
+
+ r = ioctl(vcpu->fd, KVM_GET_SREGS, &state->sregs);
+ TEST_ASSERT(r == 0, "Unexpected result from KVM_GET_SREGS, r: %i",
+ r);
+
+ state->msrs.nmsrs = nmsrs;
+ for (i = 0; i < nmsrs; i++)
+ state->msrs.entries[i].index = list->indices[i];
+ r = ioctl(vcpu->fd, KVM_GET_MSRS, &state->msrs);
+ TEST_ASSERT(r == nmsrs, "Unexpected result from KVM_GET_MSRS, r: %i (failed at %x)",
+ r, r == nmsrs ? -1 : list->indices[r]);
+
+ r = ioctl(vcpu->fd, KVM_GET_DEBUGREGS, &state->debugregs);
+ TEST_ASSERT(r == 0, "Unexpected result from KVM_GET_DEBUGREGS, r: %i",
+ r);
+
+ free(list);
+ return state;
+}
+
+void vcpu_load_state(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_x86_state *state)
+{
+ struct vcpu *vcpu = vcpu_find(vm, vcpuid);
+ int r;
+
+ r = ioctl(vcpu->fd, KVM_SET_XSAVE, &state->xsave);
+ TEST_ASSERT(r == 0, "Unexpected result from KVM_SET_XSAVE, r: %i",
+ r);
+
+ r = ioctl(vcpu->fd, KVM_SET_XCRS, &state->xcrs);
+ TEST_ASSERT(r == 0, "Unexpected result from KVM_SET_XCRS, r: %i",
+ r);
+
+ r = ioctl(vcpu->fd, KVM_SET_SREGS, &state->sregs);
+ TEST_ASSERT(r == 0, "Unexpected result from KVM_SET_SREGS, r: %i",
+ r);
+
+ r = ioctl(vcpu->fd, KVM_SET_MSRS, &state->msrs);
+ TEST_ASSERT(r == state->msrs.nmsrs, "Unexpected result from KVM_SET_MSRS, r: %i (failed at %x)",
+ r, r == state->msrs.nmsrs ? -1 : state->msrs.entries[r].index);
+
+ r = ioctl(vcpu->fd, KVM_SET_VCPU_EVENTS, &state->events);
+ TEST_ASSERT(r == 0, "Unexpected result from KVM_SET_VCPU_EVENTS, r: %i",
+ r);
+
+ r = ioctl(vcpu->fd, KVM_SET_MP_STATE, &state->mp_state);
+ TEST_ASSERT(r == 0, "Unexpected result from KVM_SET_MP_STATE, r: %i",
+ r);
+
+ r = ioctl(vcpu->fd, KVM_SET_DEBUGREGS, &state->debugregs);
+ TEST_ASSERT(r == 0, "Unexpected result from KVM_SET_DEBUGREGS, r: %i",
+ r);
+
+ r = ioctl(vcpu->fd, KVM_SET_REGS, &state->regs);
+ TEST_ASSERT(r == 0, "Unexpected result from KVM_SET_REGS, r: %i",
+ r);
+}
diff --git a/tools/testing/selftests/kvm/state_test.c b/tools/testing/selftests/kvm/state_test.c
new file mode 100644
index 000000000000..e47b27b5406e
--- /dev/null
+++ b/tools/testing/selftests/kvm/state_test.c
@@ -0,0 +1,119 @@
+/*
+ * KVM_GET/SET_* tests
+ *
+ * Copyright (C) 2018, Red Hat, Inc.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.
+ *
+ * Tests for vCPU state save/restore, including nested guest state.
+ */
+#define _GNU_SOURCE /* for program_invocation_short_name */
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+
+#include "test_util.h"
+
+#include "kvm_util.h"
+#include "x86.h"
+
+#define VCPU_ID 5
+#define PORT_SYNC 0x1000
+#define PORT_ABORT 0x1001
+#define PORT_DONE 0x1002
+
+static inline void __exit_to_l0(uint16_t port, uint64_t arg0, uint64_t arg1)
+{
+ __asm__ __volatile__("in %[port], %%al"
+ :
+ : [port]"d"(port), "D"(arg0), "S"(arg1)
+ : "rax");
+}
+
+#define exit_to_l0(_port, _arg0, _arg1) \
+ __exit_to_l0(_port, (uint64_t) (_arg0), (uint64_t) (_arg1))
+
+#define GUEST_ASSERT(_condition) do { \
+ if (!(_condition)) \
+ exit_to_l0(PORT_ABORT, "Failed guest assert: " #_condition, __LINE__);\
+} while (0)
+
+#define GUEST_SYNC(stage) \
+ exit_to_l0(PORT_SYNC, "hello", stage);
+
+static bool have_nested_state;
+
+void guest_code(void)
+{
+ GUEST_SYNC(1);
+ GUEST_SYNC(2);
+
+ exit_to_l0(PORT_DONE, 0, 0);
+}
+
+int main(int argc, char *argv[])
+{
+ struct kvm_regs regs1, regs2;
+ struct kvm_vm *vm;
+ struct kvm_run *run;
+ struct kvm_x86_state *state;
+ int stage;
+
+ struct kvm_cpuid_entry2 *entry = kvm_get_supported_cpuid_entry(1);
+
+ /* Create VM */
+ vm = vm_create_default(VCPU_ID, guest_code);
+ vcpu_set_cpuid(vm, VCPU_ID, kvm_get_supported_cpuid());
+ run = vcpu_state(vm, VCPU_ID);
+
+ vcpu_regs_get(vm, VCPU_ID, ®s1);
+ for (stage = 1;; stage++) {
+ _vcpu_run(vm, VCPU_ID);
+ TEST_ASSERT(run->exit_reason == KVM_EXIT_IO,
+ "Unexpected exit reason: %u (%s),\n",
+ run->exit_reason,
+ exit_reason_str(run->exit_reason));
+
+ memset(®s1, 0, sizeof(regs1));
+ vcpu_regs_get(vm, VCPU_ID, ®s1);
+ switch (run->io.port) {
+ case PORT_ABORT:
+ TEST_ASSERT(false, "%s at %s:%d", (const char *) regs1.rdi,
+ __FILE__, regs1.rsi);
+ /* NOT REACHED */
+ case PORT_SYNC:
+ break;
+ case PORT_DONE:
+ goto done;
+ default:
+ TEST_ASSERT(false, "Unknown port 0x%x.", run->io.port);
+ }
+
+ /* PORT_SYNC is handled here. */
+ TEST_ASSERT(!strcmp((const char *)regs1.rdi, "hello") &&
+ regs1.rsi == stage, "Unexpected register values vmexit #%lx, got %lx",
+ stage, (ulong) regs1.rsi);
+
+ state = vcpu_save_state(vm, VCPU_ID);
+ kvm_vm_release(vm);
+
+ /* Restore state in a new VM. */
+ kvm_vm_restart(vm, O_RDWR);
+ vm_vcpu_add(vm, VCPU_ID, 0, 0);
+ vcpu_set_cpuid(vm, VCPU_ID, kvm_get_supported_cpuid());
+ vcpu_load_state(vm, VCPU_ID, state);
+ run = vcpu_state(vm, VCPU_ID);
+ free(state);
+
+ memset(®s2, 0, sizeof(regs2));
+ vcpu_regs_get(vm, VCPU_ID, ®s2);
+ TEST_ASSERT(!memcmp(®s1, ®s2, sizeof(regs2)),
+ "Unexpected register values after vcpu_load_state; rdi: %lx rsi: %lx",
+ (ulong) regs2.rdi, (ulong) regs2.rsi);
+ }
+
+done:
+ kvm_vm_free(vm);
+}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 06/10] KVM: x86: do not load vmcs12 pages while still in SMM
2018-07-28 23:10 [PATCH v6 00/10] kvm: x86: migration of nested virtualization state Paolo Bonzini
` (4 preceding siblings ...)
2018-07-28 23:10 ` [PATCH 05/10] kvm: selftests: add basic test for state save and restore Paolo Bonzini
@ 2018-07-28 23:10 ` Paolo Bonzini
2018-07-30 19:27 ` Jim Mattson
2018-07-28 23:10 ` [PATCH 07/10] kvm: nVMX: Introduce KVM_CAP_NESTED_STATE Paolo Bonzini
` (3 subsequent siblings)
9 siblings, 1 reply; 15+ messages in thread
From: Paolo Bonzini @ 2018-07-28 23:10 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: Liran Alon, KarimAllah Ahmed, Jim Mattson, rkrcmar
If the vCPU enters system management mode while running a nested guest,
RSM starts processing the vmentry while still in SMM. In that case,
however, the pages pointed to by the vmcs12 might be incorrectly
loaded from SMRAM. To avoid this, delay the handling of the pages
until just before the next vmentry. This is done with a new request
and a new entry in kvm_x86_ops, which we will be able to reuse for
nested VMX state migration.
Extracted from a patch by Jim Mattson and KarimAllah Ahmed.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
arch/x86/include/asm/kvm_host.h | 3 +++
arch/x86/kvm/vmx.c | 53 +++++++++++++++++++++++++++--------------
arch/x86/kvm/x86.c | 2 ++
3 files changed, 40 insertions(+), 18 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index c13cd28d9d1b..da957725992d 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 \
@@ -1085,6 +1086,8 @@ struct kvm_x86_ops {
void (*setup_mce)(struct kvm_vcpu *vcpu);
+ 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/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 2630ab38d72c..17aede06ae0e 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -10636,9 +10636,9 @@ 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 vmcs12 *vmcs12 = get_vmcs12(vcpu);
struct vcpu_vmx *vmx = to_vmx(vcpu);
struct page *page;
u64 hpa;
@@ -11750,13 +11750,18 @@ static int check_vmentry_postreqs(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12,
return 0;
}
-static int enter_vmx_non_root_mode(struct kvm_vcpu *vcpu)
+/*
+ * If p_exit_qual is NULL, this is being called from state restore (either
+ * kvm_set_nested_state or RSM). Otherwise it's called from vmlaunch/vmresume.
+ */
+static int enter_vmx_non_root_mode(struct kvm_vcpu *vcpu, u32 *p_exit_qual)
{
struct vcpu_vmx *vmx = to_vmx(vcpu);
struct vmcs12 *vmcs12 = get_vmcs12(vcpu);
+ bool from_vmentry = !!p_exit_qual;
u32 msr_entry_idx;
- u32 exit_qual;
- int r;
+ u32 dummy_exit_qual;
+ int r = 0;
enter_guest_mode(vcpu);
@@ -11770,17 +11775,27 @@ static int enter_vmx_non_root_mode(struct kvm_vcpu *vcpu)
vcpu->arch.tsc_offset += vmcs12->tsc_offset;
r = EXIT_REASON_INVALID_STATE;
- if (prepare_vmcs02(vcpu, vmcs12, &exit_qual))
+ if (prepare_vmcs02(vcpu, vmcs12, from_vmentry ? p_exit_qual : &dummy_exit_qual))
goto fail;
- nested_get_vmcs12_pages(vcpu, vmcs12);
+ if (from_vmentry) {
+ nested_get_vmcs12_pages(vcpu);
- r = EXIT_REASON_MSR_LOAD_FAIL;
- msr_entry_idx = nested_vmx_load_msr(vcpu,
- vmcs12->vm_entry_msr_load_addr,
- vmcs12->vm_entry_msr_load_count);
- if (msr_entry_idx)
- goto fail;
+ r = EXIT_REASON_MSR_LOAD_FAIL;
+ msr_entry_idx = nested_vmx_load_msr(vcpu,
+ vmcs12->vm_entry_msr_load_addr,
+ vmcs12->vm_entry_msr_load_count);
+ if (msr_entry_idx)
+ goto fail;
+ } else {
+ /*
+ * The MMU is not initialized to point at the right entities yet and
+ * "get pages" would need to read data from the guest (i.e. we will
+ * need to perform gpa to hpa translation). Request a call
+ * to nested_get_vmcs12_pages before the next VM-entry.
+ */
+ kvm_make_request(KVM_REQ_GET_VMCS12_PAGES, vcpu);
+ }
/*
* Note no nested_vmx_succeed or nested_vmx_fail here. At this point
@@ -11795,8 +11810,7 @@ static int enter_vmx_non_root_mode(struct kvm_vcpu *vcpu)
vcpu->arch.tsc_offset -= vmcs12->tsc_offset;
leave_guest_mode(vcpu);
vmx_switch_vmcs(vcpu, &vmx->vmcs01);
- nested_vmx_entry_failure(vcpu, vmcs12, r, exit_qual);
- return 1;
+ return r;
}
/*
@@ -11873,10 +11887,11 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch)
*/
vmx->nested.nested_run_pending = 1;
- ret = enter_vmx_non_root_mode(vcpu);
+ ret = enter_vmx_non_root_mode(vcpu, &exit_qual);
if (ret) {
+ nested_vmx_entry_failure(vcpu, vmcs12, ret, exit_qual);
vmx->nested.nested_run_pending = 0;
- return ret;
+ return 1;
}
/*
@@ -12962,7 +12977,7 @@ static int vmx_pre_leave_smm(struct kvm_vcpu *vcpu, u64 smbase)
if (vmx->nested.smm.guest_mode) {
vcpu->arch.hflags &= ~HF_SMM_MASK;
- ret = enter_vmx_non_root_mode(vcpu);
+ ret = enter_vmx_non_root_mode(vcpu, NULL);
vcpu->arch.hflags |= HF_SMM_MASK;
if (ret)
return ret;
@@ -13111,6 +13126,8 @@ static int enable_smi_window(struct kvm_vcpu *vcpu)
.setup_mce = vmx_setup_mce,
+ .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 2b812b3c5088..8ddf5f94876f 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7257,6 +7257,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))
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 06/10] KVM: x86: do not load vmcs12 pages while still in SMM
2018-07-28 23:10 ` [PATCH 06/10] KVM: x86: do not load vmcs12 pages while still in SMM Paolo Bonzini
@ 2018-07-30 19:27 ` Jim Mattson
2018-07-31 7:40 ` Paolo Bonzini
0 siblings, 1 reply; 15+ messages in thread
From: Jim Mattson @ 2018-07-30 19:27 UTC (permalink / raw)
To: Paolo Bonzini
Cc: LKML, kvm list, Liran Alon, KarimAllah Ahmed,
Radim Krčmář
On Sat, Jul 28, 2018 at 4:10 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> If the vCPU enters system management mode while running a nested guest,
> RSM starts processing the vmentry while still in SMM. In that case,
> however, the pages pointed to by the vmcs12 might be incorrectly
> loaded from SMRAM. To avoid this, delay the handling of the pages
> until just before the next vmentry. This is done with a new request
> and a new entry in kvm_x86_ops, which we will be able to reuse for
> nested VMX state migration.
>
> Extracted from a patch by Jim Mattson and KarimAllah Ahmed.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> arch/x86/include/asm/kvm_host.h | 3 +++
> arch/x86/kvm/vmx.c | 53 +++++++++++++++++++++++++++--------------
> arch/x86/kvm/x86.c | 2 ++
> 3 files changed, 40 insertions(+), 18 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index c13cd28d9d1b..da957725992d 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 \
> @@ -1085,6 +1086,8 @@ struct kvm_x86_ops {
>
> void (*setup_mce)(struct kvm_vcpu *vcpu);
>
> + 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/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 2630ab38d72c..17aede06ae0e 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -10636,9 +10636,9 @@ 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 vmcs12 *vmcs12 = get_vmcs12(vcpu);
> struct vcpu_vmx *vmx = to_vmx(vcpu);
> struct page *page;
> u64 hpa;
> @@ -11750,13 +11750,18 @@ static int check_vmentry_postreqs(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12,
> return 0;
> }
>
> -static int enter_vmx_non_root_mode(struct kvm_vcpu *vcpu)
> +/*
> + * If p_exit_qual is NULL, this is being called from state restore (either
> + * kvm_set_nested_state or RSM). Otherwise it's called from vmlaunch/vmresume.
> + */
> +static int enter_vmx_non_root_mode(struct kvm_vcpu *vcpu, u32 *p_exit_qual)
> {
> struct vcpu_vmx *vmx = to_vmx(vcpu);
> struct vmcs12 *vmcs12 = get_vmcs12(vcpu);
> + bool from_vmentry = !!p_exit_qual;
> u32 msr_entry_idx;
> - u32 exit_qual;
> - int r;
> + u32 dummy_exit_qual;
> + int r = 0;
>
> enter_guest_mode(vcpu);
>
> @@ -11770,17 +11775,27 @@ static int enter_vmx_non_root_mode(struct kvm_vcpu *vcpu)
> vcpu->arch.tsc_offset += vmcs12->tsc_offset;
>
> r = EXIT_REASON_INVALID_STATE;
> - if (prepare_vmcs02(vcpu, vmcs12, &exit_qual))
> + if (prepare_vmcs02(vcpu, vmcs12, from_vmentry ? p_exit_qual : &dummy_exit_qual))
> goto fail;
>
> - nested_get_vmcs12_pages(vcpu, vmcs12);
> + if (from_vmentry) {
> + nested_get_vmcs12_pages(vcpu);
>
> - r = EXIT_REASON_MSR_LOAD_FAIL;
> - msr_entry_idx = nested_vmx_load_msr(vcpu,
> - vmcs12->vm_entry_msr_load_addr,
> - vmcs12->vm_entry_msr_load_count);
> - if (msr_entry_idx)
> - goto fail;
> + r = EXIT_REASON_MSR_LOAD_FAIL;
> + msr_entry_idx = nested_vmx_load_msr(vcpu,
> + vmcs12->vm_entry_msr_load_addr,
> + vmcs12->vm_entry_msr_load_count);
> + if (msr_entry_idx)
> + goto fail;
> + } else {
> + /*
> + * The MMU is not initialized to point at the right entities yet and
> + * "get pages" would need to read data from the guest (i.e. we will
> + * need to perform gpa to hpa translation). Request a call
> + * to nested_get_vmcs12_pages before the next VM-entry.
> + */
> + kvm_make_request(KVM_REQ_GET_VMCS12_PAGES, vcpu);
> + }
The from_vmentry variable seems unnecessary. Why not (a) hoist the
call to nested_vmx_load_msr() into nested_vmx_run and (b) always use
the deferred KVM_REQ_GET_VMCS12_PAGES?
>
> /*
> * Note no nested_vmx_succeed or nested_vmx_fail here. At this point
> @@ -11795,8 +11810,7 @@ static int enter_vmx_non_root_mode(struct kvm_vcpu *vcpu)
> vcpu->arch.tsc_offset -= vmcs12->tsc_offset;
> leave_guest_mode(vcpu);
> vmx_switch_vmcs(vcpu, &vmx->vmcs01);
> - nested_vmx_entry_failure(vcpu, vmcs12, r, exit_qual);
> - return 1;
> + return r;
> }
>
> /*
> @@ -11873,10 +11887,11 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch)
> */
>
> vmx->nested.nested_run_pending = 1;
> - ret = enter_vmx_non_root_mode(vcpu);
> + ret = enter_vmx_non_root_mode(vcpu, &exit_qual);
> if (ret) {
> + nested_vmx_entry_failure(vcpu, vmcs12, ret, exit_qual);
> vmx->nested.nested_run_pending = 0;
> - return ret;
> + return 1;
> }
>
> /*
> @@ -12962,7 +12977,7 @@ static int vmx_pre_leave_smm(struct kvm_vcpu *vcpu, u64 smbase)
>
> if (vmx->nested.smm.guest_mode) {
> vcpu->arch.hflags &= ~HF_SMM_MASK;
> - ret = enter_vmx_non_root_mode(vcpu);
> + ret = enter_vmx_non_root_mode(vcpu, NULL);
> vcpu->arch.hflags |= HF_SMM_MASK;
> if (ret)
> return ret;
> @@ -13111,6 +13126,8 @@ static int enable_smi_window(struct kvm_vcpu *vcpu)
>
> .setup_mce = vmx_setup_mce,
>
> + .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 2b812b3c5088..8ddf5f94876f 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -7257,6 +7257,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))
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 06/10] KVM: x86: do not load vmcs12 pages while still in SMM
2018-07-30 19:27 ` Jim Mattson
@ 2018-07-31 7:40 ` Paolo Bonzini
0 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2018-07-31 7:40 UTC (permalink / raw)
To: Jim Mattson
Cc: LKML, kvm list, Liran Alon, KarimAllah Ahmed,
Radim Krčmář
On 30/07/2018 21:27, Jim Mattson wrote:
> On Sat, Jul 28, 2018 at 4:10 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> If the vCPU enters system management mode while running a nested guest,
>> RSM starts processing the vmentry while still in SMM. In that case,
>> however, the pages pointed to by the vmcs12 might be incorrectly
>> loaded from SMRAM. To avoid this, delay the handling of the pages
>> until just before the next vmentry. This is done with a new request
>> and a new entry in kvm_x86_ops, which we will be able to reuse for
>> nested VMX state migration.
>>
>> Extracted from a patch by Jim Mattson and KarimAllah Ahmed.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>> arch/x86/include/asm/kvm_host.h | 3 +++
>> arch/x86/kvm/vmx.c | 53 +++++++++++++++++++++++++++--------------
>> arch/x86/kvm/x86.c | 2 ++
>> 3 files changed, 40 insertions(+), 18 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
>> index c13cd28d9d1b..da957725992d 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 \
>> @@ -1085,6 +1086,8 @@ struct kvm_x86_ops {
>>
>> void (*setup_mce)(struct kvm_vcpu *vcpu);
>>
>> + 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/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index 2630ab38d72c..17aede06ae0e 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -10636,9 +10636,9 @@ 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 vmcs12 *vmcs12 = get_vmcs12(vcpu);
>> struct vcpu_vmx *vmx = to_vmx(vcpu);
>> struct page *page;
>> u64 hpa;
>> @@ -11750,13 +11750,18 @@ static int check_vmentry_postreqs(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12,
>> return 0;
>> }
>>
>> -static int enter_vmx_non_root_mode(struct kvm_vcpu *vcpu)
>> +/*
>> + * If p_exit_qual is NULL, this is being called from state restore (either
>> + * kvm_set_nested_state or RSM). Otherwise it's called from vmlaunch/vmresume.
>> + */
>> +static int enter_vmx_non_root_mode(struct kvm_vcpu *vcpu, u32 *p_exit_qual)
>> {
>> struct vcpu_vmx *vmx = to_vmx(vcpu);
>> struct vmcs12 *vmcs12 = get_vmcs12(vcpu);
>> + bool from_vmentry = !!p_exit_qual;
>> u32 msr_entry_idx;
>> - u32 exit_qual;
>> - int r;
>> + u32 dummy_exit_qual;
>> + int r = 0;
>>
>> enter_guest_mode(vcpu);
>>
>> @@ -11770,17 +11775,27 @@ static int enter_vmx_non_root_mode(struct kvm_vcpu *vcpu)
>> vcpu->arch.tsc_offset += vmcs12->tsc_offset;
>>
>> r = EXIT_REASON_INVALID_STATE;
>> - if (prepare_vmcs02(vcpu, vmcs12, &exit_qual))
>> + if (prepare_vmcs02(vcpu, vmcs12, from_vmentry ? p_exit_qual : &dummy_exit_qual))
>> goto fail;
>>
>> - nested_get_vmcs12_pages(vcpu, vmcs12);
>> + if (from_vmentry) {
>> + nested_get_vmcs12_pages(vcpu);
>>
>> - r = EXIT_REASON_MSR_LOAD_FAIL;
>> - msr_entry_idx = nested_vmx_load_msr(vcpu,
>> - vmcs12->vm_entry_msr_load_addr,
>> - vmcs12->vm_entry_msr_load_count);
>> - if (msr_entry_idx)
>> - goto fail;
>> + r = EXIT_REASON_MSR_LOAD_FAIL;
>> + msr_entry_idx = nested_vmx_load_msr(vcpu,
>> + vmcs12->vm_entry_msr_load_addr,
>> + vmcs12->vm_entry_msr_load_count);
>> + if (msr_entry_idx)
>> + goto fail;
>> + } else {
>> + /*
>> + * The MMU is not initialized to point at the right entities yet and
>> + * "get pages" would need to read data from the guest (i.e. we will
>> + * need to perform gpa to hpa translation). Request a call
>> + * to nested_get_vmcs12_pages before the next VM-entry.
>> + */
>> + kvm_make_request(KVM_REQ_GET_VMCS12_PAGES, vcpu);
>> + }
>
> The from_vmentry variable seems unnecessary. Why not (a) hoist the
> call to nested_vmx_load_msr() into nested_vmx_run and (b) always use
> the deferred KVM_REQ_GET_VMCS12_PAGES?
Yeah, I thought about that. from_vmentry is ugly.
However, for (a) you would have to duplicate the code at the "fail"
label, which is uglier than from_vmentry; as to (b), it is a little bit
slower and our nested vmentry is already slow enough...
Paolo
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 07/10] kvm: nVMX: Introduce KVM_CAP_NESTED_STATE
2018-07-28 23:10 [PATCH v6 00/10] kvm: x86: migration of nested virtualization state Paolo Bonzini
` (5 preceding siblings ...)
2018-07-28 23:10 ` [PATCH 06/10] KVM: x86: do not load vmcs12 pages while still in SMM Paolo Bonzini
@ 2018-07-28 23:10 ` Paolo Bonzini
2018-07-28 23:10 ` [PATCH 08/10] kvm: selftests: add test for nested state save/restore Paolo Bonzini
` (2 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2018-07-28 23:10 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: Liran Alon, KarimAllah Ahmed, Jim Mattson, rkrcmar
From: Jim Mattson <jmattson@google.com>
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_NESTED_STATE
and KVM_SET_NESTED_STATE. These can be used for saving and restoring a VM
that is in VMX operation.
Signed-off-by: Jim Mattson <jmattson@google.com>
[karahmed@ - rename structs and functions and make them ready for AMD and
address previous comments.
- handle nested.smm state.
- rebase & a bit of refactoring.
- Merge 7/8 and 8/8 into one patch. ]
Signed-off-by: KarimAllah Ahmed <karahmed@amazon.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
Documentation/virtual/kvm/api.txt | 52 ++++++++++++
arch/x86/include/asm/kvm_host.h | 6 ++
arch/x86/include/uapi/asm/kvm.h | 37 +++++++++
arch/x86/kvm/vmx.c | 167 ++++++++++++++++++++++++++++++++++++++
arch/x86/kvm/x86.c | 54 ++++++++++++
include/uapi/linux/kvm.h | 4 +
6 files changed, 320 insertions(+)
diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index d10944e619d3..84504566c4ff 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -3561,6 +3561,62 @@ 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_NESTED_STATE
+
+Capability: KVM_CAP_NESTED_STATE
+Architectures: x86
+Type: vcpu ioctl
+Parameters: struct kvm_nested_state (in/out)
+Returns: 0 on success, -1 on error
+Errors:
+ E2BIG: the total state size (including the fixed-size part of struct
+ kvm_nested_state) exceeds the value of 'size' specified by
+ the user; the size required will be written into size.
+
+struct kvm_nested_state {
+ __u16 flags;
+ __u16 format;
+ __u32 size;
+ union {
+ struct kvm_vmx_nested_state vmx;
+ struct kvm_svm_nested_state svm;
+ __u8 pad[120];
+ };
+ __u8 data[0];
+};
+
+#define KVM_STATE_NESTED_GUEST_MODE 0x00000001
+#define KVM_STATE_NESTED_RUN_PENDING 0x00000002
+
+#define KVM_STATE_NESTED_SMM_GUEST_MODE 0x00000001
+#define KVM_STATE_NESTED_SMM_VMXON 0x00000002
+
+struct kvm_vmx_nested_state {
+ __u64 vmxon_pa;
+ __u64 vmcs_pa;
+
+ struct {
+ __u16 flags;
+ } smm;
+};
+
+This ioctl copies the vcpu's nested virtualization state from the kernel to
+userspace.
+
+The maximum size of the state, including the fixed-size part of struct
+kvm_nested_state, can be retrieved by passing KVM_CAP_NESTED_STATE to
+the KVM_CHECK_EXTENSION ioctl().
+
+4.115 KVM_SET_NESTED_STATE
+
+Capability: KVM_CAP_NESTED_STATE
+Architectures: x86
+Type: vcpu ioctl
+Parameters: struct kvm_nested_state (in)
+Returns: 0 on success, -1 on error
+
+This copies the vcpu's kvm_nested_state struct from userspace to the kernel. For
+the definition of struct kvm_nested_state, see KVM_GET_NESTED_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 da957725992d..bd287b348751 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1086,6 +1086,12 @@ struct kvm_x86_ops {
void (*setup_mce)(struct kvm_vcpu *vcpu);
+ int (*get_nested_state)(struct kvm_vcpu *vcpu,
+ struct kvm_nested_state __user *user_kvm_nested_state,
+ unsigned user_data_size);
+ int (*set_nested_state)(struct kvm_vcpu *vcpu,
+ struct kvm_nested_state __user *user_kvm_nested_state,
+ struct kvm_nested_state *kvm_state);
void (*get_vmcs12_pages)(struct kvm_vcpu *vcpu);
int (*smi_allowed)(struct kvm_vcpu *vcpu);
diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index c535c2fdea13..86299efa804a 100644
--- a/arch/x86/include/uapi/asm/kvm.h
+++ b/arch/x86/include/uapi/asm/kvm.h
@@ -378,4 +378,41 @@ struct kvm_sync_regs {
#define KVM_X86_QUIRK_LINT0_REENABLED (1 << 0)
#define KVM_X86_QUIRK_CD_NW_CLEARED (1 << 1)
+#define KVM_STATE_NESTED_GUEST_MODE 0x00000001
+#define KVM_STATE_NESTED_RUN_PENDING 0x00000002
+
+#define KVM_STATE_NESTED_SMM_GUEST_MODE 0x00000001
+#define KVM_STATE_NESTED_SMM_VMXON 0x00000002
+
+struct kvm_vmx_nested_state {
+ __u64 vmxon_pa;
+ __u64 vmcs_pa;
+
+ struct {
+ __u16 flags;
+ } smm;
+};
+
+/* for KVM_CAP_NESTED_STATE */
+struct kvm_nested_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_nested_state vmx;
+
+ /* Pad the header to 128 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 17aede06ae0e..4168a40b9d5c 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -7565,6 +7565,11 @@ static __init int hardware_setup(void)
else
kvm_disable_tdp();
+ if (!nested) {
+ kvm_x86_ops->get_nested_state = NULL;
+ kvm_x86_ops->set_nested_state = NULL;
+ }
+
/*
* Only enable PML when hardware supports PML feature, and both EPT
* and EPT A/D bit features are enabled -- PML depends on them to work.
@@ -12992,6 +12997,170 @@ static int enable_smi_window(struct kvm_vcpu *vcpu)
return 0;
}
+static int vmx_get_nested_state(struct kvm_vcpu *vcpu,
+ struct kvm_nested_state __user *user_kvm_nested_state,
+ u32 user_data_size)
+{
+ struct vcpu_vmx *vmx;
+ struct vmcs12 *vmcs12;
+ struct kvm_nested_state kvm_state = {
+ .flags = 0,
+ .format = 0,
+ .size = sizeof(kvm_state),
+ .vmx.vmxon_pa = -1ull,
+ .vmx.vmcs_pa = -1ull,
+ };
+
+ if (!vcpu)
+ return kvm_state.size + 2 * VMCS12_SIZE;
+
+ vmx = to_vmx(vcpu);
+ vmcs12 = get_vmcs12(vcpu);
+ if (nested_vmx_allowed(vcpu) &&
+ (vmx->nested.vmxon || vmx->nested.smm.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 (vmx->nested.smm.vmxon)
+ kvm_state.vmx.smm.flags |= KVM_STATE_NESTED_SMM_VMXON;
+
+ if (vmx->nested.smm.guest_mode)
+ kvm_state.vmx.smm.flags |= KVM_STATE_NESTED_SMM_GUEST_MODE;
+
+ if (is_guest_mode(vcpu)) {
+ kvm_state.flags |= KVM_STATE_NESTED_GUEST_MODE;
+
+ if (vmx->nested.nested_run_pending)
+ kvm_state.flags |= KVM_STATE_NESTED_RUN_PENDING;
+ }
+ }
+
+ if (user_data_size < kvm_state.size)
+ goto out;
+
+ if (copy_to_user(user_kvm_nested_state, &kvm_state, sizeof(kvm_state)))
+ return -EFAULT;
+
+ if (vmx->nested.current_vmptr == -1ull)
+ goto out;
+
+ /*
+ * 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_nested_state->data, vmcs12, sizeof(*vmcs12)))
+ return -EFAULT;
+
+out:
+ return kvm_state.size;
+}
+
+static int vmx_set_nested_state(struct kvm_vcpu *vcpu,
+ struct kvm_nested_state __user *user_kvm_nested_state,
+ struct kvm_nested_state *kvm_state)
+{
+ struct vcpu_vmx *vmx = to_vmx(vcpu);
+ struct vmcs12 *vmcs12;
+ u32 exit_qual;
+ int ret;
+
+ if (kvm_state->format != 0)
+ return -EINVAL;
+
+ if (!nested_vmx_allowed(vcpu))
+ return kvm_state->vmx.vmxon_pa == -1ull ? 0 : -EINVAL;
+
+ if (kvm_state->vmx.vmxon_pa == -1ull) {
+ if (kvm_state->vmx.smm.flags)
+ return -EINVAL;
+
+ if (kvm_state->vmx.vmcs_pa != -1ull)
+ return -EINVAL;
+
+ vmx_leave_nested(vcpu);
+ return 0;
+ }
+
+ if (!page_address_valid(vcpu, kvm_state->vmx.vmxon_pa))
+ return -EINVAL;
+
+ if (kvm_state->size < sizeof(kvm_state) + sizeof(*vmcs12))
+ return -EINVAL;
+
+ if (kvm_state->vmx.vmcs_pa == kvm_state->vmx.vmxon_pa ||
+ !page_address_valid(vcpu, kvm_state->vmx.vmcs_pa))
+ return -EINVAL;
+
+ if ((kvm_state->vmx.smm.flags & KVM_STATE_NESTED_SMM_GUEST_MODE) &&
+ (kvm_state->flags & KVM_STATE_NESTED_GUEST_MODE))
+ return -EINVAL;
+
+ if (kvm_state->vmx.smm.flags &
+ ~(KVM_STATE_NESTED_SMM_GUEST_MODE | KVM_STATE_NESTED_SMM_VMXON))
+ return -EINVAL;
+
+ if ((kvm_state->vmx.smm.flags & KVM_STATE_NESTED_SMM_GUEST_MODE) &&
+ !(kvm_state->vmx.smm.flags & KVM_STATE_NESTED_SMM_VMXON))
+ return -EINVAL;
+
+ vmx_leave_nested(vcpu);
+ if (kvm_state->vmx.vmxon_pa == -1ull)
+ return 0;
+
+ vmx->nested.vmxon_ptr = kvm_state->vmx.vmxon_pa;
+ ret = enter_vmx_operation(vcpu);
+ if (ret)
+ return ret;
+
+ set_current_vmptr(vmx, kvm_state->vmx.vmcs_pa);
+
+ if (kvm_state->vmx.smm.flags & KVM_STATE_NESTED_SMM_VMXON) {
+ vmx->nested.smm.vmxon = true;
+ vmx->nested.vmxon = false;
+
+ if (kvm_state->vmx.smm.flags & KVM_STATE_NESTED_SMM_GUEST_MODE)
+ vmx->nested.smm.guest_mode = true;
+ }
+
+ vmcs12 = get_vmcs12(vcpu);
+ if (copy_from_user(vmcs12, user_kvm_nested_state->data, sizeof(*vmcs12)))
+ return -EFAULT;
+
+ if (vmcs12->revision_id != VMCS12_REVISION)
+ return -EINVAL;
+
+ if (!(kvm_state->flags & KVM_STATE_NESTED_GUEST_MODE))
+ return 0;
+
+ vmx->nested.nested_run_pending =
+ !!(kvm_state->flags & KVM_STATE_NESTED_RUN_PENDING);
+
+ if (check_vmentry_prereqs(vcpu, vmcs12) ||
+ check_vmentry_postreqs(vcpu, vmcs12, &exit_qual))
+ return -EINVAL;
+
+ if (kvm_state->flags & KVM_STATE_NESTED_RUN_PENDING)
+ vmx->nested.nested_run_pending = 1;
+
+ vmx->nested.dirty_vmcs12 = true;
+ ret = enter_vmx_non_root_mode(vcpu, NULL);
+ if (ret)
+ return -EINVAL;
+
+ return 0;
+}
+
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,
@@ -13126,6 +13291,8 @@ static int enable_smi_window(struct kvm_vcpu *vcpu)
.setup_mce = vmx_setup_mce,
+ .get_nested_state = vmx_get_nested_state,
+ .set_nested_state = vmx_set_nested_state,
.get_vmcs12_pages = nested_get_vmcs12_pages,
.smi_allowed = vmx_smi_allowed,
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 8ddf5f94876f..b1327b211452 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2944,6 +2944,10 @@ 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_NESTED_STATE:
+ r = kvm_x86_ops->get_nested_state ?
+ kvm_x86_ops->get_nested_state(NULL, 0, 0) : 0;
+ break;
default:
break;
}
@@ -3960,6 +3964,56 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
r = kvm_vcpu_ioctl_enable_cap(vcpu, &cap);
break;
}
+ case KVM_GET_NESTED_STATE: {
+ struct kvm_nested_state __user *user_kvm_nested_state = argp;
+ u32 user_data_size;
+
+ r = -EINVAL;
+ if (!kvm_x86_ops->get_nested_state)
+ break;
+
+ BUILD_BUG_ON(sizeof(user_data_size) != sizeof(user_kvm_nested_state->size));
+ if (get_user(user_data_size, &user_kvm_nested_state->size))
+ return -EFAULT;
+
+ r = kvm_x86_ops->get_nested_state(vcpu, user_kvm_nested_state,
+ user_data_size);
+ if (r < 0)
+ return r;
+
+ if (r > user_data_size) {
+ if (put_user(r, &user_kvm_nested_state->size))
+ return -EFAULT;
+ return -E2BIG;
+ }
+ r = 0;
+ break;
+ }
+ case KVM_SET_NESTED_STATE: {
+ struct kvm_nested_state __user *user_kvm_nested_state = argp;
+ struct kvm_nested_state kvm_state;
+
+ r = -EINVAL;
+ if (!kvm_x86_ops->set_nested_state)
+ break;
+
+ if (copy_from_user(&kvm_state, user_kvm_nested_state, sizeof(kvm_state)))
+ return -EFAULT;
+
+ if (kvm_state.size < sizeof(kvm_state))
+ return -EINVAL;
+
+ if (kvm_state.flags &
+ ~(KVM_STATE_NESTED_RUN_PENDING | KVM_STATE_NESTED_GUEST_MODE))
+ return -EINVAL;
+
+ /* nested_run_pending implies guest_mode. */
+ if (kvm_state.flags == KVM_STATE_NESTED_RUN_PENDING)
+ return -EINVAL;
+
+ r = kvm_x86_ops->set_nested_state(vcpu, user_kvm_nested_state, &kvm_state);
+ break;
+ }
default:
r = -EINVAL;
}
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index b6270a3b38e9..c2e91c28e09d 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -949,6 +949,7 @@ struct kvm_ppc_resize_hpt {
#define KVM_CAP_GET_MSR_FEATURES 153
#define KVM_CAP_HYPERV_EVENTFD 154
#define KVM_CAP_HYPERV_TLBFLUSH 155
+#define KVM_CAP_NESTED_STATE 156
#ifdef KVM_CAP_IRQ_ROUTING
@@ -1391,6 +1392,9 @@ struct kvm_enc_region {
/* Available with KVM_CAP_HYPERV_EVENTFD */
#define KVM_HYPERV_EVENTFD _IOW(KVMIO, 0xbd, struct kvm_hyperv_eventfd)
+/* Available with KVM_CAP_NESTED_STATE */
+#define KVM_GET_NESTED_STATE _IOWR(KVMIO, 0xbe, struct kvm_nested_state)
+#define KVM_SET_NESTED_STATE _IOW(KVMIO, 0xbf, struct kvm_nested_state)
/* Secure Encrypted Virtualization command */
enum sev_cmd_id {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 08/10] kvm: selftests: add test for nested state save/restore
2018-07-28 23:10 [PATCH v6 00/10] kvm: x86: migration of nested virtualization state Paolo Bonzini
` (6 preceding siblings ...)
2018-07-28 23:10 ` [PATCH 07/10] kvm: nVMX: Introduce KVM_CAP_NESTED_STATE Paolo Bonzini
@ 2018-07-28 23:10 ` Paolo Bonzini
2018-07-28 23:10 ` [PATCH 09/10] KVM: nVMX: include shadow vmcs12 in nested state Paolo Bonzini
2018-07-28 23:10 ` [PATCH 10/10] KVM: selftests: add tests for shadow VMCS save/restore Paolo Bonzini
9 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2018-07-28 23:10 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: Liran Alon, KarimAllah Ahmed, Jim Mattson, rkrcmar
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
tools/testing/selftests/kvm/include/vmx.h | 32 +++++++++++++
tools/testing/selftests/kvm/lib/x86.c | 27 ++++++++++-
tools/testing/selftests/kvm/state_test.c | 75 ++++++++++++++++++++++++++++++-
3 files changed, 131 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/kvm/include/vmx.h b/tools/testing/selftests/kvm/include/vmx.h
index 9caaf56696a4..2b37f9b6f7df 100644
--- a/tools/testing/selftests/kvm/include/vmx.h
+++ b/tools/testing/selftests/kvm/include/vmx.h
@@ -380,6 +380,30 @@ static inline int vmptrld(uint64_t vmcs_pa)
return ret;
}
+static inline int vmptrst(uint64_t *value)
+{
+ uint64_t tmp;
+ uint8_t ret;
+
+ __asm__ __volatile__("vmptrst %[value]; setna %[ret]"
+ : [value]"=m"(tmp), [ret]"=rm"(ret)
+ : : "cc", "memory");
+
+ *value = tmp;
+ return ret;
+}
+
+/*
+ * A wrapper around vmptrst that ignores errors and returns zero if the
+ * vmptrst instruction fails.
+ */
+static inline uint64_t vmptrstz(void)
+{
+ uint64_t value = 0;
+ vmptrst(&value);
+ return value;
+}
+
/*
* No guest state (e.g. GPRs) is established by this vmlaunch.
*/
@@ -444,6 +468,15 @@ static inline int vmresume(void)
return ret;
}
+static inline void vmcall(void)
+{
+ /* Currently, L1 destroys our GPRs during vmexits. */
+ __asm__ __volatile__("push %%rbp; vmcall; pop %%rbp" : : :
+ "rax", "rbx", "rcx", "rdx",
+ "rsi", "rdi", "r8", "r9", "r10", "r11", "r12",
+ "r13", "r14", "r15");
+}
+
static inline int vmread(uint64_t encoding, uint64_t *value)
{
uint64_t tmp;
diff --git a/tools/testing/selftests/kvm/lib/x86.c b/tools/testing/selftests/kvm/lib/x86.c
index 78aabcb91de1..e38345252df5 100644
--- a/tools/testing/selftests/kvm/lib/x86.c
+++ b/tools/testing/selftests/kvm/lib/x86.c
@@ -736,6 +736,10 @@ struct kvm_x86_state {
struct kvm_xcrs xcrs;
struct kvm_sregs sregs;
struct kvm_debugregs debugregs;
+ union {
+ struct kvm_nested_state nested;
+ char nested_[16384];
+ };
struct kvm_msrs msrs;
};
@@ -764,6 +762,14 @@ struct kvm_x86_state *vcpu_save_state(struct kvm_vm *vm, uint32_t vcpuid)
struct kvm_msr_list *list;
struct kvm_x86_state *state;
int nmsrs, r, i;
+ static int nested_size = -1;
+
+ if (nested_size == -1) {
+ nested_size = kvm_check_cap(KVM_CAP_NESTED_STATE);
+ TEST_ASSERT(nested_size <= sizeof(state->nested_),
+ "Nested state size too big, %i > %zi",
+ nested_size, sizeof(state->nested_));
+ }
nmsrs = kvm_get_num_msrs(vm);
list = malloc(sizeof(*list) + nmsrs * sizeof(list->indices[0]));
@@ -797,6 +803,17 @@ struct kvm_x86_state *vcpu_save_state(struct kvm_vm *vm, uint32_t vcpuid)
TEST_ASSERT(r == 0, "Unexpected result from KVM_GET_SREGS, r: %i",
r);
+ if (nested_size) {
+ state->nested.size = sizeof(state->nested_);
+ r = ioctl(vcpu->fd, KVM_GET_NESTED_STATE, &state->nested);
+ TEST_ASSERT(r == 0, "Unexpected result from KVM_GET_NESTED_STATE, r: %i",
+ r);
+ TEST_ASSERT(state->nested.size <= nested_size,
+ "Nested state size too big, %i (KVM_CHECK_CAP gave %i)",
+ state->nested.size, nested_size);
+ } else
+ state->nested.size = 0;
+
state->msrs.nmsrs = nmsrs;
for (i = 0; i < nmsrs; i++)
state->msrs.entries[i].index = list->indices[i];
@@ -817,6 +834,12 @@ void vcpu_load_state(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_x86_state *s
struct vcpu *vcpu = vcpu_find(vm, vcpuid);
int r;
+ if (state->nested.size) {
+ r = ioctl(vcpu->fd, KVM_SET_NESTED_STATE, &state->nested);
+ TEST_ASSERT(r == 0, "Unexpected result from KVM_SET_NESTED_STATE, r: %i",
+ r);
+ }
+
r = ioctl(vcpu->fd, KVM_SET_XSAVE, &state->xsave);
TEST_ASSERT(r == 0, "Unexpected result from KVM_SET_XSAVE, r: %i",
r);
diff --git a/tools/testing/selftests/kvm/state_test.c b/tools/testing/selftests/kvm/state_test.c
index e47b27b5406e..447da4f76b56 100644
--- a/tools/testing/selftests/kvm/state_test.c
+++ b/tools/testing/selftests/kvm/state_test.c
@@ -18,6 +18,7 @@
#include "kvm_util.h"
#include "x86.h"
+#include "vmx.h"
#define VCPU_ID 5
#define PORT_SYNC 0x1000
@@ -45,16 +46,75 @@ static inline void __exit_to_l0(uint16_t port, uint64_t arg0, uint64_t arg1)
static bool have_nested_state;
-void guest_code(void)
+void l2_guest_code(void)
+{
+ GUEST_SYNC(5);
+
+ /* Exit to L1 */
+ vmcall();
+
+ GUEST_SYNC(7);
+
+ /* Done, exit to L1 and never come back. */
+ vmcall();
+}
+
+void l1_guest_code(struct vmx_pages *vmx_pages)
+{
+#define L2_GUEST_STACK_SIZE 64
+ unsigned long l2_guest_stack[L2_GUEST_STACK_SIZE];
+
+ GUEST_ASSERT(vmx_pages->vmcs_gpa);
+ GUEST_ASSERT(prepare_for_vmx_operation(vmx_pages));
+ GUEST_ASSERT(vmptrstz() == vmx_pages->vmcs_gpa);
+
+ GUEST_SYNC(3);
+ GUEST_ASSERT(vmptrstz() == vmx_pages->vmcs_gpa);
+
+ prepare_vmcs(vmx_pages, l2_guest_code,
+ &l2_guest_stack[L2_GUEST_STACK_SIZE]);
+
+ GUEST_SYNC(4);
+ GUEST_ASSERT(vmptrstz() == vmx_pages->vmcs_gpa);
+ GUEST_ASSERT(!vmlaunch());
+ GUEST_ASSERT(vmptrstz() == vmx_pages->vmcs_gpa);
+ GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == EXIT_REASON_VMCALL);
+
+ /* Check that the launched state is preserved. */
+ GUEST_ASSERT(vmlaunch());
+
+ GUEST_ASSERT(!vmresume());
+ GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == EXIT_REASON_VMCALL);
+
+ GUEST_SYNC(6);
+ GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == EXIT_REASON_VMCALL);
+
+ GUEST_ASSERT(!vmresume());
+ GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == EXIT_REASON_VMCALL);
+
+ vmwrite(GUEST_RIP, vmreadz(GUEST_RIP) + 3);
+
+ GUEST_ASSERT(!vmresume());
+ GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == EXIT_REASON_VMCALL);
+ GUEST_SYNC(8);
+}
+
+void guest_code(struct vmx_pages *vmx_pages)
{
GUEST_SYNC(1);
GUEST_SYNC(2);
+ if (vmx_pages)
+ l1_guest_code(vmx_pages);
+
exit_to_l0(PORT_DONE, 0, 0);
}
int main(int argc, char *argv[])
{
+ struct vmx_pages *vmx_pages = NULL;
+ vm_vaddr_t vmx_pages_gva = 0;
+
struct kvm_regs regs1, regs2;
struct kvm_vm *vm;
struct kvm_run *run;
@@ -69,6 +133,15 @@ int main(int argc, char *argv[])
run = vcpu_state(vm, VCPU_ID);
vcpu_regs_get(vm, VCPU_ID, ®s1);
+
+ if (kvm_check_cap(KVM_CAP_NESTED_STATE)) {
+ vmx_pages = vcpu_alloc_vmx(vm, &vmx_pages_gva);
+ vcpu_args_set(vm, VCPU_ID, 1, vmx_pages_gva);
+ } else {
+ printf("will skip nested state checks\n");
+ vcpu_args_set(vm, VCPU_ID, 1, 0);
+ }
+
for (stage = 1;; stage++) {
_vcpu_run(vm, VCPU_ID);
TEST_ASSERT(run->exit_reason == KVM_EXIT_IO,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 09/10] KVM: nVMX: include shadow vmcs12 in nested state
2018-07-28 23:10 [PATCH v6 00/10] kvm: x86: migration of nested virtualization state Paolo Bonzini
` (7 preceding siblings ...)
2018-07-28 23:10 ` [PATCH 08/10] kvm: selftests: add test for nested state save/restore Paolo Bonzini
@ 2018-07-28 23:10 ` Paolo Bonzini
2018-07-30 19:11 ` Jim Mattson
2018-07-28 23:10 ` [PATCH 10/10] KVM: selftests: add tests for shadow VMCS save/restore Paolo Bonzini
9 siblings, 1 reply; 15+ messages in thread
From: Paolo Bonzini @ 2018-07-28 23:10 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: Liran Alon, KarimAllah Ahmed, Jim Mattson, rkrcmar
The shadow vmcs12 cannot be flushed on KVM_GET_NESTED_STATE,
because at that point guest memory is assumed by userspace to
be immutable. Capture the cache in vmx_get_nested_state, adding
another page at the end if there is an active shadow vmcs12.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
arch/x86/kvm/vmx.c | 31 ++++++++++++++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index b5ee9e08bb48..ce8c0c759a19 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -13167,9 +13167,15 @@ static int vmx_get_nested_state(struct kvm_vcpu *vcpu,
kvm_state.vmx.vmxon_pa = vmx->nested.vmxon_ptr;
kvm_state.vmx.vmcs_pa = vmx->nested.current_vmptr;
- if (vmx->nested.current_vmptr != -1ull)
+ if (vmx->nested.current_vmptr != -1ull) {
kvm_state.size += VMCS12_SIZE;
+ if (is_guest_mode(vcpu) &&
+ nested_cpu_has_shadow_vmcs(vmcs12) &&
+ vmcs12->vmcs_link_pointer != -1ull)
+ kvm_state.size += VMCS12_SIZE;
+ }
+
if (vmx->nested.smm.vmxon)
kvm_state.vmx.smm.flags |= KVM_STATE_NESTED_SMM_VMXON;
@@ -13208,6 +13214,13 @@ static int vmx_get_nested_state(struct kvm_vcpu *vcpu,
if (copy_to_user(user_kvm_nested_state->data, vmcs12, sizeof(*vmcs12)))
return -EFAULT;
+ if (nested_cpu_has_shadow_vmcs(vmcs12) &&
+ vmcs12->vmcs_link_pointer != -1ull) {
+ if (copy_to_user(user_kvm_nested_state->data + VMCS12_SIZE,
+ get_shadow_vmcs12(vcpu), sizeof(*vmcs12)))
+ return -EFAULT;
+ }
+
out:
return kvm_state.size;
}
@@ -13288,6 +13301,22 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu,
vmx->nested.nested_run_pending =
!!(kvm_state->flags & KVM_STATE_NESTED_RUN_PENDING);
+ if (nested_cpu_has_shadow_vmcs(vmcs12) &&
+ vmcs12->vmcs_link_pointer != -1ull) {
+ struct vmcs12 *shadow_vmcs12 = get_shadow_vmcs12(vcpu);
+ if (kvm_state->size < sizeof(kvm_state) + 2 * sizeof(*vmcs12))
+ return -EINVAL;
+
+ if (copy_from_user(shadow_vmcs12,
+ user_kvm_nested_state->data + VMCS12_SIZE,
+ sizeof(*vmcs12)))
+ return -EFAULT;
+
+ if (shadow_vmcs12->hdr.revision_id != VMCS12_REVISION ||
+ !shadow_vmcs12->hdr.shadow_vmcs)
+ return -EINVAL;
+ }
+
if (check_vmentry_prereqs(vcpu, vmcs12) ||
check_vmentry_postreqs(vcpu, vmcs12, &exit_qual))
return -EINVAL;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 09/10] KVM: nVMX: include shadow vmcs12 in nested state
2018-07-28 23:10 ` [PATCH 09/10] KVM: nVMX: include shadow vmcs12 in nested state Paolo Bonzini
@ 2018-07-30 19:11 ` Jim Mattson
2018-07-31 7:39 ` Paolo Bonzini
0 siblings, 1 reply; 15+ messages in thread
From: Jim Mattson @ 2018-07-30 19:11 UTC (permalink / raw)
To: Paolo Bonzini
Cc: LKML, kvm list, Liran Alon, KarimAllah Ahmed,
Radim Krčmář
On Sat, Jul 28, 2018 at 4:10 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> The shadow vmcs12 cannot be flushed on KVM_GET_NESTED_STATE,
> because at that point guest memory is assumed by userspace to
> be immutable. Capture the cache in vmx_get_nested_state, adding
> another page at the end if there is an active shadow vmcs12.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> arch/x86/kvm/vmx.c | 31 ++++++++++++++++++++++++++++++-
> 1 file changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index b5ee9e08bb48..ce8c0c759a19 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -13167,9 +13167,15 @@ static int vmx_get_nested_state(struct kvm_vcpu *vcpu,
> kvm_state.vmx.vmxon_pa = vmx->nested.vmxon_ptr;
> kvm_state.vmx.vmcs_pa = vmx->nested.current_vmptr;
>
> - if (vmx->nested.current_vmptr != -1ull)
> + if (vmx->nested.current_vmptr != -1ull) {
> kvm_state.size += VMCS12_SIZE;
>
> + if (is_guest_mode(vcpu) &&
> + nested_cpu_has_shadow_vmcs(vmcs12) &&
> + vmcs12->vmcs_link_pointer != -1ull)
> + kvm_state.size += VMCS12_SIZE;
> + }
> +
> if (vmx->nested.smm.vmxon)
> kvm_state.vmx.smm.flags |= KVM_STATE_NESTED_SMM_VMXON;
>
> @@ -13208,6 +13214,13 @@ static int vmx_get_nested_state(struct kvm_vcpu *vcpu,
> if (copy_to_user(user_kvm_nested_state->data, vmcs12, sizeof(*vmcs12)))
> return -EFAULT;
>
> + if (nested_cpu_has_shadow_vmcs(vmcs12) &&
> + vmcs12->vmcs_link_pointer != -1ull) {
> + if (copy_to_user(user_kvm_nested_state->data + VMCS12_SIZE,
> + get_shadow_vmcs12(vcpu), sizeof(*vmcs12)))
Does this work with CONFIG_HARDENED_USERCOPY?
Is VMCS12_SIZE better than sizeof(*vmcs12)? What if we are migrating
to a destination where sizeof(*vmcs12) is larger than it is on the
source? If userspace doesn't zero the buffer before calling the ioctl,
then the destination may interpret nonsense as actual VMCS field
values. However, if we copy the entire page from the kernel, then we
know that anything beyond the source's sizeof(*vmcs12) will be zero.
> + return -EFAULT;
> + }
> +
> out:
> return kvm_state.size;
> }
> @@ -13288,6 +13301,22 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu,
> vmx->nested.nested_run_pending =
> !!(kvm_state->flags & KVM_STATE_NESTED_RUN_PENDING);
>
> + if (nested_cpu_has_shadow_vmcs(vmcs12) &&
> + vmcs12->vmcs_link_pointer != -1ull) {
> + struct vmcs12 *shadow_vmcs12 = get_shadow_vmcs12(vcpu);
> + if (kvm_state->size < sizeof(kvm_state) + 2 * sizeof(*vmcs12))
> + return -EINVAL;
> +
> + if (copy_from_user(shadow_vmcs12,
> + user_kvm_nested_state->data + VMCS12_SIZE,
> + sizeof(*vmcs12)))
> + return -EFAULT;
> +
> + if (shadow_vmcs12->hdr.revision_id != VMCS12_REVISION ||
> + !shadow_vmcs12->hdr.shadow_vmcs)
> + return -EINVAL;
> + }
> +
> if (check_vmentry_prereqs(vcpu, vmcs12) ||
> check_vmentry_postreqs(vcpu, vmcs12, &exit_qual))
> return -EINVAL;
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 09/10] KVM: nVMX: include shadow vmcs12 in nested state
2018-07-30 19:11 ` Jim Mattson
@ 2018-07-31 7:39 ` Paolo Bonzini
0 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2018-07-31 7:39 UTC (permalink / raw)
To: Jim Mattson
Cc: LKML, kvm list, Liran Alon, KarimAllah Ahmed,
Radim Krčmář
On 30/07/2018 21:11, Jim Mattson wrote:
> Does this work with CONFIG_HARDENED_USERCOPY?
Yes, kmalloc objects are exempt from the check (see new_kmalloc_cache).
> Is VMCS12_SIZE better than sizeof(*vmcs12)? What if we are migrating
> to a destination where sizeof(*vmcs12) is larger than it is on the
> source?
Either those fields won't be used by the destination due to the VMX MSR
values, or migration should have failed due to invalid VMX MSR values.
VMCS12_SIZE is consistent with handle_vmptrld and nested_release_vmcs12.
If we use sizeof(*vmcs12) in vmx_get_nested_state, we will have to
adjust the copy from/to cached_vmcs12 and cached_shadow_vmcs12 to also
use sizeof(*vmcs12), otherwise:
1) nested_release_vmcs12 can leak arbitrary kernel data to userspace,
because the cached_vmcs12 and cached_shadow_vmcs12 are not zeroed when
allocated.
2) even if we zeroed the allocation in enter_vmx_operation, the guest
could observe a migration because it would change the padding at the end
of the VMCS changes to zeroes; this is strictly speaking not an issue,
but it's ugly.
Paolo
> If userspace doesn't zero the buffer before calling the ioctl,
> then the destination may interpret nonsense as actual VMCS field
> values. However, if we copy the entire page from the kernel, then we
> know that anything beyond the source's sizeof(*vmcs12) will be zero.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 10/10] KVM: selftests: add tests for shadow VMCS save/restore
2018-07-28 23:10 [PATCH v6 00/10] kvm: x86: migration of nested virtualization state Paolo Bonzini
` (8 preceding siblings ...)
2018-07-28 23:10 ` [PATCH 09/10] KVM: nVMX: include shadow vmcs12 in nested state Paolo Bonzini
@ 2018-07-28 23:10 ` Paolo Bonzini
9 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2018-07-28 23:10 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: Liran Alon, KarimAllah Ahmed, Jim Mattson, rkrcmar
This includes setting up the shadow VMCS and the secondary execution
controls in lib/vmx.c.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
tools/testing/selftests/kvm/include/vmx.h | 12 +++++++++++
tools/testing/selftests/kvm/lib/vmx.c | 33 +++++++++++++++++++++++++++---
tools/testing/selftests/kvm/state_test.c | 34 +++++++++++++++++++++++++++++--
3 files changed, 74 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/kvm/include/vmx.h b/tools/testing/selftests/kvm/include/vmx.h
index 2b37f9b6f7df..02cdecf9560c 100644
--- a/tools/testing/selftests/kvm/include/vmx.h
+++ b/tools/testing/selftests/kvm/include/vmx.h
@@ -530,6 +530,18 @@ struct vmx_pages {
void *msr_hva;
uint64_t msr_gpa;
void *msr;
+
+ void *shadow_vmcs_hva;
+ uint64_t shadow_vmcs_gpa;
+ void *shadow_vmcs;
+
+ void *vmread_hva;
+ uint64_t vmread_gpa;
+ void *vmread;
+
+ void *vmwrite_hva;
+ uint64_t vmwrite_gpa;
+ void *vmwrite;
};
struct vmx_pages *vcpu_alloc_vmx(struct kvm_vm *vm, vm_vaddr_t *p_vmx_gva);
diff --git a/tools/testing/selftests/kvm/lib/vmx.c b/tools/testing/selftests/kvm/lib/vmx.c
index 5701e52a33ed..b987c3c970eb 100644
--- a/tools/testing/selftests/kvm/lib/vmx.c
+++ b/tools/testing/selftests/kvm/lib/vmx.c
@@ -44,6 +44,23 @@ struct vmx_pages *
vmx->msr = (void *)vm_vaddr_alloc(vm, getpagesize(), 0x10000, 0, 0);
vmx->msr_hva = addr_gva2hva(vm, (uintptr_t)vmx->msr);
vmx->msr_gpa = addr_gva2gpa(vm, (uintptr_t)vmx->msr);
+ memset(vmx->msr_hva, 0, getpagesize());
+
+ /* Setup of a region of guest memory for the shadow VMCS. */
+ vmx->shadow_vmcs = (void *)vm_vaddr_alloc(vm, getpagesize(), 0x10000, 0, 0);
+ vmx->shadow_vmcs_hva = addr_gva2hva(vm, (uintptr_t)vmx->shadow_vmcs);
+ vmx->shadow_vmcs_gpa = addr_gva2gpa(vm, (uintptr_t)vmx->shadow_vmcs);
+
+ /* Setup of a region of guest memory for the VMREAD and VMWRITE bitmaps. */
+ vmx->vmread = (void *)vm_vaddr_alloc(vm, getpagesize(), 0x10000, 0, 0);
+ vmx->vmread_hva = addr_gva2hva(vm, (uintptr_t)vmx->vmread);
+ vmx->vmread_gpa = addr_gva2gpa(vm, (uintptr_t)vmx->vmread);
+ memset(vmx->vmread_hva, 0, getpagesize());
+
+ vmx->vmwrite = (void *)vm_vaddr_alloc(vm, getpagesize(), 0x10000, 0, 0);
+ vmx->vmwrite_hva = addr_gva2hva(vm, (uintptr_t)vmx->vmwrite);
+ vmx->vmwrite_gpa = addr_gva2gpa(vm, (uintptr_t)vmx->vmwrite);
+ memset(vmx->vmwrite_hva, 0, getpagesize());
*p_vmx_gva = vmx_gva;
return vmx;
@@ -98,6 +115,11 @@ bool prepare_for_vmx_operation(struct vmx_pages *vmx)
if (vmptrld(vmx->vmcs_gpa))
return false;
+ /* Setup shadow VMCS, do not load it yet. */
+ *(uint32_t *)(vmx->shadow_vmcs) = vmcs_revision() | 0x80000000ul;
+ if (vmclear(vmx->shadow_vmcs_gpa))
+ return false;
+
return true;
}
@@ -109,8 +131,12 @@ static inline void init_vmcs_control_fields(struct vmx_pages *vmx)
vmwrite(VIRTUAL_PROCESSOR_ID, 0);
vmwrite(POSTED_INTR_NV, 0);
- vmwrite(PIN_BASED_VM_EXEC_CONTROL, rdmsr(MSR_IA32_VMX_PINBASED_CTLS));
- vmwrite(CPU_BASED_VM_EXEC_CONTROL, rdmsr(MSR_IA32_VMX_PROCBASED_CTLS));
+ vmwrite(PIN_BASED_VM_EXEC_CONTROL, rdmsr(MSR_IA32_VMX_TRUE_PINBASED_CTLS));
+ if (!vmwrite(SECONDARY_VM_EXEC_CONTROL, 0))
+ vmwrite(CPU_BASED_VM_EXEC_CONTROL,
+ rdmsr(MSR_IA32_VMX_TRUE_PROCBASED_CTLS) | CPU_BASED_ACTIVATE_SECONDARY_CONTROLS);
+ else
+ vmwrite(CPU_BASED_VM_EXEC_CONTROL, rdmsr(MSR_IA32_VMX_TRUE_PROCBASED_CTLS));
vmwrite(EXCEPTION_BITMAP, 0);
vmwrite(PAGE_FAULT_ERROR_CODE_MASK, 0);
vmwrite(PAGE_FAULT_ERROR_CODE_MATCH, -1); /* Never match */
@@ -124,7 +150,6 @@ static inline void init_vmcs_control_fields(struct vmx_pages *vmx)
vmwrite(VM_ENTRY_MSR_LOAD_COUNT, 0);
vmwrite(VM_ENTRY_INTR_INFO_FIELD, 0);
vmwrite(TPR_THRESHOLD, 0);
- vmwrite(SECONDARY_VM_EXEC_CONTROL, 0);
vmwrite(CR0_GUEST_HOST_MASK, 0);
vmwrite(CR4_GUEST_HOST_MASK, 0);
@@ -132,6 +157,8 @@ static inline void init_vmcs_control_fields(struct vmx_pages *vmx)
vmwrite(CR4_READ_SHADOW, get_cr4());
vmwrite(MSR_BITMAP, vmx->msr_gpa);
+ vmwrite(VMREAD_BITMAP, vmx->vmread_gpa);
+ vmwrite(VMWRITE_BITMAP, vmx->vmwrite_gpa);
}
/*
diff --git a/tools/testing/selftests/kvm/state_test.c b/tools/testing/selftests/kvm/state_test.c
index 447da4f76b56..833319b06577 100644
--- a/tools/testing/selftests/kvm/state_test.c
+++ b/tools/testing/selftests/kvm/state_test.c
@@ -55,7 +55,15 @@ void l2_guest_code(void)
/* Exit to L1 */
vmcall();
- GUEST_SYNC(7);
+ /* L1 has now set up a shadow VMCS for us. */
+ GUEST_ASSERT(vmreadz(GUEST_RIP) == 0xc0ffee);
+ GUEST_SYNC(9);
+ GUEST_ASSERT(vmreadz(GUEST_RIP) == 0xc0ffee);
+ GUEST_ASSERT(!vmwrite(GUEST_RIP, 0xc0fffee));
+ GUEST_SYNC(10);
+ GUEST_ASSERT(vmreadz(GUEST_RIP) == 0xc0fffee);
+ GUEST_ASSERT(!vmwrite(GUEST_RIP, 0xc0ffffee));
+ GUEST_SYNC(11);
/* Done, exit to L1 and never come back. */
vmcall();
@@ -96,9 +104,31 @@ void l1_guest_code(struct vmx_pages *vmx_pages)
vmwrite(GUEST_RIP, vmreadz(GUEST_RIP) + 3);
+ vmwrite(SECONDARY_VM_EXEC_CONTROL, SECONDARY_EXEC_SHADOW_VMCS);
+ vmwrite(VMCS_LINK_POINTER, vmx_pages->shadow_vmcs_gpa);
+
+ GUEST_ASSERT(!vmptrld(vmx_pages->shadow_vmcs_gpa));
+ GUEST_ASSERT(vmlaunch());
+ GUEST_SYNC(7);
+ GUEST_ASSERT(vmlaunch());
+ GUEST_ASSERT(vmresume());
+
+ vmwrite(GUEST_RIP, 0xc0ffee);
+ GUEST_SYNC(8);
+ GUEST_ASSERT(vmreadz(GUEST_RIP) == 0xc0ffee);
+
+ GUEST_ASSERT(!vmptrld(vmx_pages->vmcs_gpa));
GUEST_ASSERT(!vmresume());
GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == EXIT_REASON_VMCALL);
- GUEST_SYNC(8);
+
+ GUEST_ASSERT(!vmptrld(vmx_pages->shadow_vmcs_gpa));
+ GUEST_ASSERT(vmreadz(GUEST_RIP) == 0xc0ffffee);
+ GUEST_ASSERT(vmlaunch());
+ GUEST_ASSERT(vmresume());
+ GUEST_SYNC(12);
+ GUEST_ASSERT(vmreadz(GUEST_RIP) == 0xc0ffffee);
+ GUEST_ASSERT(vmlaunch());
+ GUEST_ASSERT(vmresume());
}
void guest_code(struct vmx_pages *vmx_pages)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread