[PATCH 0/5] KVM: nVMX: Small fixes improving emulation accuracy

[PATCH 0/5] KVM: nVMX: Small fixes improving emulation accuracy

[Jan Kiszka]

Nothing critical, but it further improves emulation accuracy,
specifically helpful when analyzing guest bugs...

Corresponding kvm-unit-tests will be provided.

Jan Kiszka (5):
  KVM: nVMX: Fix returned value of MSR_IA32_VMX_PROCBASED_CTLS
  KVM: nVMX: Advertise support for MSR_IA32_VMX_TRUE_*_CTLS
  KVM: nVMX: Allow to disable CR3 access interception
  KVM: nVMX: Allow to disable VM_{ENTRY_LOAD,EXIT_SAVE}_DEBUG_CONTROLS
  KVM: nVMX: Fix returned value of MSR_IA32_VMX_VMCS_ENUM

 arch/x86/include/asm/vmx.h            |  3 ++
 arch/x86/include/uapi/asm/msr-index.h |  1 +
 arch/x86/kvm/vmx.c                    | 75 +++++++++++++++++++++++++----------
 3 files changed, 58 insertions(+), 21 deletions(-)

-- 
1.8.1.1.298.ge7eed54

[PATCH 1/5] KVM: nVMX: Fix returned value of MSR_IA32_VMX_PROCBASED_CTLS

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

SDM says bits 1, 4-6, 8, 13-16, and 26 have to be set. Fixing this
temporarily revokes the ability of L1 to control CR3 interceptions.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 arch/x86/include/asm/vmx.h | 3 +++
 arch/x86/kvm/vmx.c         | 5 +++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
index d989829..bcbfade 100644
--- a/arch/x86/include/asm/vmx.h
+++ b/arch/x86/include/asm/vmx.h
@@ -51,6 +51,9 @@
 #define CPU_BASED_MONITOR_EXITING               0x20000000
 #define CPU_BASED_PAUSE_EXITING                 0x40000000
 #define CPU_BASED_ACTIVATE_SECONDARY_CONTROLS   0x80000000
+
+#define CPU_BASED_ALWAYSON_WITHOUT_TRUE_MSR	0x0401e172
+
 /*
  * Definitions of Secondary Processor-Based VM-Execution Controls.
  */
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 801332e..b38e03a 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2314,7 +2314,7 @@ static __init void nested_vmx_setup_ctls_msrs(void)
 	/* cpu-based controls */
 	rdmsr(MSR_IA32_VMX_PROCBASED_CTLS,
 		nested_vmx_procbased_ctls_low, nested_vmx_procbased_ctls_high);
-	nested_vmx_procbased_ctls_low = 0;
+	nested_vmx_procbased_ctls_low = CPU_BASED_ALWAYSON_WITHOUT_TRUE_MSR;
 	nested_vmx_procbased_ctls_high &=
 		CPU_BASED_VIRTUAL_INTR_PENDING |
 		CPU_BASED_VIRTUAL_NMI_PENDING | CPU_BASED_USE_TSC_OFFSETING |
@@ -2335,7 +2335,8 @@ static __init void nested_vmx_setup_ctls_msrs(void)
 	 * can use it to avoid exits to L1 - even when L0 runs L2
 	 * without MSR bitmaps.
 	 */
-	nested_vmx_procbased_ctls_high |= CPU_BASED_USE_MSR_BITMAPS;
+	nested_vmx_procbased_ctls_high |= CPU_BASED_ALWAYSON_WITHOUT_TRUE_MSR |
+		CPU_BASED_USE_MSR_BITMAPS;
 
 	/* secondary cpu-based controls */
 	rdmsr(MSR_IA32_VMX_PROCBASED_CTLS2,
-- 
1.8.1.1.298.ge7eed54

[PATCH 2/5] KVM: nVMX: Advertise support for MSR_IA32_VMX_TRUE_*_CTLS

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

We already implemented them but failed to advertise them. Currently they
all return the identical values to the capability MSRs they are
augmenting. So there is no change in exposed features yet.

Drop related comments at this chance that are partially incorrect and
redundant anyway.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 arch/x86/include/uapi/asm/msr-index.h |  1 +
 arch/x86/kvm/vmx.c                    | 13 ++-----------
 2 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/arch/x86/include/uapi/asm/msr-index.h b/arch/x86/include/uapi/asm/msr-index.h
index fcf2b3a..579f62a 100644
--- a/arch/x86/include/uapi/asm/msr-index.h
+++ b/arch/x86/include/uapi/asm/msr-index.h
@@ -558,6 +558,7 @@
 
 /* VMX_BASIC bits and bitmasks */
 #define VMX_BASIC_VMCS_SIZE_SHIFT	32
+#define VMX_BASIC_TRUE_CTLS		0x0080000000000000LLU
 #define VMX_BASIC_64		0x0001000000000000LLU
 #define VMX_BASIC_MEM_TYPE_SHIFT	50
 #define VMX_BASIC_MEM_TYPE_MASK	0x003c000000000000LLU
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index b38e03a..cb43883 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2265,21 +2265,13 @@ static __init void nested_vmx_setup_ctls_msrs(void)
 	/* pin-based controls */
 	rdmsr(MSR_IA32_VMX_PINBASED_CTLS,
 	      nested_vmx_pinbased_ctls_low, nested_vmx_pinbased_ctls_high);
-	/*
-	 * According to the Intel spec, if bit 55 of VMX_BASIC is off (as it is
-	 * in our case), bits 1, 2 and 4 (i.e., 0x16) must be 1 in this MSR.
-	 */
 	nested_vmx_pinbased_ctls_low |= PIN_BASED_ALWAYSON_WITHOUT_TRUE_MSR;
 	nested_vmx_pinbased_ctls_high &= PIN_BASED_EXT_INTR_MASK |
 		PIN_BASED_NMI_EXITING | PIN_BASED_VIRTUAL_NMIS;
 	nested_vmx_pinbased_ctls_high |= PIN_BASED_ALWAYSON_WITHOUT_TRUE_MSR |
 		PIN_BASED_VMX_PREEMPTION_TIMER;
 
-	/*
-	 * Exit controls
-	 * If bit 55 of VMX_BASIC is off, bits 0-8 and 10, 11, 13, 14, 16 and
-	 * 17 must be 1.
-	 */
+	/* exit controls */
 	rdmsr(MSR_IA32_VMX_EXIT_CTLS,
 		nested_vmx_exit_ctls_low, nested_vmx_exit_ctls_high);
 	nested_vmx_exit_ctls_low = VM_EXIT_ALWAYSON_WITHOUT_TRUE_MSR;
@@ -2299,7 +2291,6 @@ static __init void nested_vmx_setup_ctls_msrs(void)
 	/* entry controls */
 	rdmsr(MSR_IA32_VMX_ENTRY_CTLS,
 		nested_vmx_entry_ctls_low, nested_vmx_entry_ctls_high);
-	/* If bit 55 of VMX_BASIC is off, bits 0-8 and 12 must be 1. */
 	nested_vmx_entry_ctls_low = VM_ENTRY_ALWAYSON_WITHOUT_TRUE_MSR;
 	nested_vmx_entry_ctls_high &=
 #ifdef CONFIG_X86_64
@@ -2395,7 +2386,7 @@ static int vmx_get_vmx_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata)
 		 * guest, and the VMCS structure we give it - not about the
 		 * VMX support of the underlying hardware.
 		 */
-		*pdata = VMCS12_REVISION |
+		*pdata = VMCS12_REVISION | VMX_BASIC_TRUE_CTLS |
 			   ((u64)VMCS12_SIZE << VMX_BASIC_VMCS_SIZE_SHIFT) |
 			   (VMX_BASIC_MEM_TYPE_WB << VMX_BASIC_MEM_TYPE_SHIFT);
 		break;
-- 
1.8.1.1.298.ge7eed54

[PATCH 3/5] KVM: nVMX: Allow to disable CR3 access interception

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

We already had this control enabled by exposing the broken
MSR_IA32_VMX_PROCBASED_CTLS value. This now advertises our capability by
clearing the right bits in MSR_IA32_VMX_TRUE_PROCBASED_CTLS. We also
have to ensure to test the right value on L2 entry.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 arch/x86/kvm/vmx.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index cb43883..475f2dc 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2239,6 +2239,7 @@ static inline bool nested_vmx_allowed(struct kvm_vcpu *vcpu)
  * or other means.
  */
 static u32 nested_vmx_procbased_ctls_low, nested_vmx_procbased_ctls_high;
+static u32 nested_vmx_true_procbased_ctls_low;
 static u32 nested_vmx_secondary_ctls_low, nested_vmx_secondary_ctls_high;
 static u32 nested_vmx_pinbased_ctls_low, nested_vmx_pinbased_ctls_high;
 static u32 nested_vmx_exit_ctls_low, nested_vmx_exit_ctls_high;
@@ -2329,6 +2330,10 @@ static __init void nested_vmx_setup_ctls_msrs(void)
 	nested_vmx_procbased_ctls_high |= CPU_BASED_ALWAYSON_WITHOUT_TRUE_MSR |
 		CPU_BASED_USE_MSR_BITMAPS;
 
+	/* We support free control of CR3 access interception. */
+	nested_vmx_true_procbased_ctls_low = nested_vmx_procbased_ctls_low &
+		~(CPU_BASED_CR3_LOAD_EXITING | CPU_BASED_CR3_STORE_EXITING);
+
 	/* secondary cpu-based controls */
 	rdmsr(MSR_IA32_VMX_PROCBASED_CTLS2,
 		nested_vmx_secondary_ctls_low, nested_vmx_secondary_ctls_high);
@@ -2396,6 +2401,9 @@ static int vmx_get_vmx_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata)
 					nested_vmx_pinbased_ctls_high);
 		break;
 	case MSR_IA32_VMX_TRUE_PROCBASED_CTLS:
+		*pdata = vmx_control_msr(nested_vmx_true_procbased_ctls_low,
+					nested_vmx_procbased_ctls_high);
+		break;
 	case MSR_IA32_VMX_PROCBASED_CTLS:
 		*pdata = vmx_control_msr(nested_vmx_procbased_ctls_low,
 					nested_vmx_procbased_ctls_high);
@@ -8128,7 +8136,8 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch)
 	}
 
 	if (!vmx_control_verify(vmcs12->cpu_based_vm_exec_control,
-	      nested_vmx_procbased_ctls_low, nested_vmx_procbased_ctls_high) ||
+				nested_vmx_true_procbased_ctls_low,
+				nested_vmx_procbased_ctls_high) ||
 	    !vmx_control_verify(vmcs12->secondary_vm_exec_control,
 	      nested_vmx_secondary_ctls_low, nested_vmx_secondary_ctls_high) ||
 	    !vmx_control_verify(vmcs12->pin_based_vm_exec_control,
-- 
1.8.1.1.298.ge7eed54

[PATCH 4/5] KVM: nVMX: Allow to disable VM_{ENTRY_LOAD,EXIT_SAVE}_DEBUG_CONTROLS

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

Allow L1 to "leak" its debug controls into L2, i.e. permit cleared
VM_{ENTRY_LOAD,EXIT_SAVE}_DEBUG_CONTROLS. This requires to manually
transfer the state of DR7 and IA32_DEBUGCTLMSR from L1 into L2 as both
run on different VMCS.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 arch/x86/kvm/vmx.c | 44 ++++++++++++++++++++++++++++++++++++++------
 1 file changed, 38 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 475f2dc..b31e9f1 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -383,6 +383,9 @@ struct nested_vmx {
 
 	struct hrtimer preemption_timer;
 	bool preemption_timer_expired;
+
+	/* to migrate it to L2 if VM_ENTRY_LOAD_DEBUG_CONTROLS is off */
+	u64 host_debugctl;
 };
 
 #define POSTED_INTR_ON  0
@@ -2243,7 +2246,9 @@ static u32 nested_vmx_true_procbased_ctls_low;
 static u32 nested_vmx_secondary_ctls_low, nested_vmx_secondary_ctls_high;
 static u32 nested_vmx_pinbased_ctls_low, nested_vmx_pinbased_ctls_high;
 static u32 nested_vmx_exit_ctls_low, nested_vmx_exit_ctls_high;
+static u32 nested_vmx_true_exit_ctls_low;
 static u32 nested_vmx_entry_ctls_low, nested_vmx_entry_ctls_high;
+static u32 nested_vmx_true_entry_ctls_low;
 static u32 nested_vmx_misc_low, nested_vmx_misc_high;
 static u32 nested_vmx_ept_caps;
 static __init void nested_vmx_setup_ctls_msrs(void)
@@ -2289,6 +2294,10 @@ static __init void nested_vmx_setup_ctls_msrs(void)
 	if (vmx_mpx_supported())
 		nested_vmx_exit_ctls_high |= VM_EXIT_CLEAR_BNDCFGS;
 
+	/* We support free control of debug control saving. */
+	nested_vmx_true_exit_ctls_low = nested_vmx_exit_ctls_low &
+		~VM_EXIT_SAVE_DEBUG_CONTROLS;
+
 	/* entry controls */
 	rdmsr(MSR_IA32_VMX_ENTRY_CTLS,
 		nested_vmx_entry_ctls_low, nested_vmx_entry_ctls_high);
@@ -2303,6 +2312,10 @@ static __init void nested_vmx_setup_ctls_msrs(void)
 	if (vmx_mpx_supported())
 		nested_vmx_entry_ctls_high |= VM_ENTRY_LOAD_BNDCFGS;
 
+	/* We support free control of debug control loading. */
+	nested_vmx_true_entry_ctls_low = nested_vmx_entry_ctls_low &
+		~VM_ENTRY_LOAD_DEBUG_CONTROLS;
+
 	/* cpu-based controls */
 	rdmsr(MSR_IA32_VMX_PROCBASED_CTLS,
 		nested_vmx_procbased_ctls_low, nested_vmx_procbased_ctls_high);
@@ -2409,11 +2422,17 @@ static int vmx_get_vmx_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata)
 					nested_vmx_procbased_ctls_high);
 		break;
 	case MSR_IA32_VMX_TRUE_EXIT_CTLS:
+		*pdata = vmx_control_msr(nested_vmx_true_exit_ctls_low,
+					nested_vmx_exit_ctls_high);
+		break;
 	case MSR_IA32_VMX_EXIT_CTLS:
 		*pdata = vmx_control_msr(nested_vmx_exit_ctls_low,
 					nested_vmx_exit_ctls_high);
 		break;
 	case MSR_IA32_VMX_TRUE_ENTRY_CTLS:
+		*pdata = vmx_control_msr(nested_vmx_true_entry_ctls_low,
+					nested_vmx_entry_ctls_high);
+		break;
 	case MSR_IA32_VMX_ENTRY_CTLS:
 		*pdata = vmx_control_msr(nested_vmx_entry_ctls_low,
 					nested_vmx_entry_ctls_high);
@@ -7836,7 +7855,13 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
 	vmcs_writel(GUEST_GDTR_BASE, vmcs12->guest_gdtr_base);
 	vmcs_writel(GUEST_IDTR_BASE, vmcs12->guest_idtr_base);
 
-	vmcs_write64(GUEST_IA32_DEBUGCTL, vmcs12->guest_ia32_debugctl);
+	if (vmcs12->vm_entry_controls & VM_ENTRY_LOAD_DEBUG_CONTROLS) {
+		kvm_set_dr(vcpu, 7, vmcs12->guest_dr7);
+		vmcs_write64(GUEST_IA32_DEBUGCTL, vmcs12->guest_ia32_debugctl);
+	} else {
+		kvm_set_dr(vcpu, 7, vcpu->arch.dr7);
+		vmcs_write64(GUEST_IA32_DEBUGCTL, vmx->nested.host_debugctl);
+	}
 	vmcs_write32(VM_ENTRY_INTR_INFO_FIELD,
 		vmcs12->vm_entry_intr_info_field);
 	vmcs_write32(VM_ENTRY_EXCEPTION_ERROR_CODE,
@@ -7846,7 +7871,6 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
 	vmcs_write32(GUEST_INTERRUPTIBILITY_INFO,
 		vmcs12->guest_interruptibility_info);
 	vmcs_write32(GUEST_SYSENTER_CS, vmcs12->guest_sysenter_cs);
-	kvm_set_dr(vcpu, 7, vmcs12->guest_dr7);
 	vmx_set_rflags(vcpu, vmcs12->guest_rflags);
 	vmcs_writel(GUEST_PENDING_DBG_EXCEPTIONS,
 		vmcs12->guest_pending_dbg_exceptions);
@@ -8143,9 +8167,11 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch)
 	    !vmx_control_verify(vmcs12->pin_based_vm_exec_control,
 	      nested_vmx_pinbased_ctls_low, nested_vmx_pinbased_ctls_high) ||
 	    !vmx_control_verify(vmcs12->vm_exit_controls,
-	      nested_vmx_exit_ctls_low, nested_vmx_exit_ctls_high) ||
+				nested_vmx_true_exit_ctls_low,
+				nested_vmx_exit_ctls_high) ||
 	    !vmx_control_verify(vmcs12->vm_entry_controls,
-	      nested_vmx_entry_ctls_low, nested_vmx_entry_ctls_high))
+				nested_vmx_true_entry_ctls_low,
+				nested_vmx_entry_ctls_high))
 	{
 		nested_vmx_failValid(vcpu, VMXERR_ENTRY_INVALID_CONTROL_FIELD);
 		return 1;
@@ -8222,6 +8248,9 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch)
 
 	vmx->nested.vmcs01_tsc_offset = vmcs_read64(TSC_OFFSET);
 
+	if (!(vmcs12->vm_entry_controls & VM_ENTRY_LOAD_DEBUG_CONTROLS))
+		vmx->nested.host_debugctl = vmcs_read64(GUEST_IA32_DEBUGCTL);
+
 	cpu = get_cpu();
 	vmx->loaded_vmcs = vmcs02;
 	vmx_vcpu_put(vcpu);
@@ -8399,7 +8428,6 @@ static void prepare_vmcs12(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12,
 	vmcs12->guest_cr0 = vmcs12_guest_cr0(vcpu, vmcs12);
 	vmcs12->guest_cr4 = vmcs12_guest_cr4(vcpu, vmcs12);
 
-	kvm_get_dr(vcpu, 7, (unsigned long *)&vmcs12->guest_dr7);
 	vmcs12->guest_rsp = kvm_register_read(vcpu, VCPU_REGS_RSP);
 	vmcs12->guest_rip = kvm_register_read(vcpu, VCPU_REGS_RIP);
 	vmcs12->guest_rflags = vmcs_readl(GUEST_RFLAGS);
@@ -8478,9 +8506,13 @@ static void prepare_vmcs12(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12,
 		(vmcs12->vm_entry_controls & ~VM_ENTRY_IA32E_MODE) |
 		(vm_entry_controls_get(to_vmx(vcpu)) & VM_ENTRY_IA32E_MODE);
 
+	if (vmcs12->vm_exit_controls & VM_EXIT_SAVE_DEBUG_CONTROLS) {
+		kvm_get_dr(vcpu, 7, (unsigned long *)&vmcs12->guest_dr7);
+		vmcs12->guest_ia32_debugctl = vmcs_read64(GUEST_IA32_DEBUGCTL);
+	}
+
 	/* TODO: These cannot have changed unless we have MSR bitmaps and
 	 * the relevant bit asks not to trap the change */
-	vmcs12->guest_ia32_debugctl = vmcs_read64(GUEST_IA32_DEBUGCTL);
 	if (vmcs12->vm_exit_controls & VM_EXIT_SAVE_IA32_PAT)
 		vmcs12->guest_ia32_pat = vmcs_read64(GUEST_IA32_PAT);
 	if (vmcs12->vm_exit_controls & VM_EXIT_SAVE_IA32_EFER)
-- 
1.8.1.1.298.ge7eed54

[PATCH 5/5] KVM: nVMX: Fix returned value of MSR_IA32_VMX_VMCS_ENUM

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

Many real CPUs get this wrong as well, but ours is totally off: bits 9:1
define the highest index value.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 arch/x86/kvm/vmx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index b31e9f1..2aeb8ac 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2461,7 +2461,7 @@ static int vmx_get_vmx_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata)
 		*pdata = -1ULL;
 		break;
 	case MSR_IA32_VMX_VMCS_ENUM:
-		*pdata = 0x1f;
+		*pdata = 0x2e; /* highest index: VMX_PREEMPTION_TIMER_VALUE */
 		break;
 	case MSR_IA32_VMX_PROCBASED_CTLS2:
 		*pdata = vmx_control_msr(nested_vmx_secondary_ctls_low,
-- 
1.8.1.1.298.ge7eed54

Re: [PATCH 4/5] KVM: nVMX: Allow to disable VM_{ENTRY_LOAD,EXIT_SAVE}_DEBUG_CONTROLS

[Paolo Bonzini]

Il 15/06/2014 16:18, Jan Kiszka ha scritto:
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> Allow L1 to "leak" its debug controls into L2, i.e. permit cleared
> VM_{ENTRY_LOAD,EXIT_SAVE}_DEBUG_CONTROLS. This requires to manually
> transfer the state of DR7 and IA32_DEBUGCTLMSR from L1 into L2 as both
> run on different VMCS.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  arch/x86/kvm/vmx.c | 44 ++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 38 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 475f2dc..b31e9f1 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -383,6 +383,9 @@ struct nested_vmx {
>
>  	struct hrtimer preemption_timer;
>  	bool preemption_timer_expired;
> +
> +	/* to migrate it to L2 if VM_ENTRY_LOAD_DEBUG_CONTROLS is off */
> +	u64 host_debugctl;

I think vmcs01_debugctl would be more descriptive of the role.  What do 
you think?  I can fix it up myself when merging the patch.

Paolo

>  };
>
>  #define POSTED_INTR_ON  0
> @@ -2243,7 +2246,9 @@ static u32 nested_vmx_true_procbased_ctls_low;
>  static u32 nested_vmx_secondary_ctls_low, nested_vmx_secondary_ctls_high;
>  static u32 nested_vmx_pinbased_ctls_low, nested_vmx_pinbased_ctls_high;
>  static u32 nested_vmx_exit_ctls_low, nested_vmx_exit_ctls_high;
> +static u32 nested_vmx_true_exit_ctls_low;
>  static u32 nested_vmx_entry_ctls_low, nested_vmx_entry_ctls_high;
> +static u32 nested_vmx_true_entry_ctls_low;
>  static u32 nested_vmx_misc_low, nested_vmx_misc_high;
>  static u32 nested_vmx_ept_caps;
>  static __init void nested_vmx_setup_ctls_msrs(void)
> @@ -2289,6 +2294,10 @@ static __init void nested_vmx_setup_ctls_msrs(void)
>  	if (vmx_mpx_supported())
>  		nested_vmx_exit_ctls_high |= VM_EXIT_CLEAR_BNDCFGS;
>
> +	/* We support free control of debug control saving. */
> +	nested_vmx_true_exit_ctls_low = nested_vmx_exit_ctls_low &
> +		~VM_EXIT_SAVE_DEBUG_CONTROLS;
> +
>  	/* entry controls */
>  	rdmsr(MSR_IA32_VMX_ENTRY_CTLS,
>  		nested_vmx_entry_ctls_low, nested_vmx_entry_ctls_high);
> @@ -2303,6 +2312,10 @@ static __init void nested_vmx_setup_ctls_msrs(void)
>  	if (vmx_mpx_supported())
>  		nested_vmx_entry_ctls_high |= VM_ENTRY_LOAD_BNDCFGS;
>
> +	/* We support free control of debug control loading. */
> +	nested_vmx_true_entry_ctls_low = nested_vmx_entry_ctls_low &
> +		~VM_ENTRY_LOAD_DEBUG_CONTROLS;
> +
>  	/* cpu-based controls */
>  	rdmsr(MSR_IA32_VMX_PROCBASED_CTLS,
>  		nested_vmx_procbased_ctls_low, nested_vmx_procbased_ctls_high);
> @@ -2409,11 +2422,17 @@ static int vmx_get_vmx_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata)
>  					nested_vmx_procbased_ctls_high);
>  		break;
>  	case MSR_IA32_VMX_TRUE_EXIT_CTLS:
> +		*pdata = vmx_control_msr(nested_vmx_true_exit_ctls_low,
> +					nested_vmx_exit_ctls_high);
> +		break;
>  	case MSR_IA32_VMX_EXIT_CTLS:
>  		*pdata = vmx_control_msr(nested_vmx_exit_ctls_low,
>  					nested_vmx_exit_ctls_high);
>  		break;
>  	case MSR_IA32_VMX_TRUE_ENTRY_CTLS:
> +		*pdata = vmx_control_msr(nested_vmx_true_entry_ctls_low,
> +					nested_vmx_entry_ctls_high);
> +		break;
>  	case MSR_IA32_VMX_ENTRY_CTLS:
>  		*pdata = vmx_control_msr(nested_vmx_entry_ctls_low,
>  					nested_vmx_entry_ctls_high);
> @@ -7836,7 +7855,13 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
>  	vmcs_writel(GUEST_GDTR_BASE, vmcs12->guest_gdtr_base);
>  	vmcs_writel(GUEST_IDTR_BASE, vmcs12->guest_idtr_base);
>
> -	vmcs_write64(GUEST_IA32_DEBUGCTL, vmcs12->guest_ia32_debugctl);
> +	if (vmcs12->vm_entry_controls & VM_ENTRY_LOAD_DEBUG_CONTROLS) {
> +		kvm_set_dr(vcpu, 7, vmcs12->guest_dr7);
> +		vmcs_write64(GUEST_IA32_DEBUGCTL, vmcs12->guest_ia32_debugctl);
> +	} else {
> +		kvm_set_dr(vcpu, 7, vcpu->arch.dr7);
> +		vmcs_write64(GUEST_IA32_DEBUGCTL, vmx->nested.host_debugctl);
> +	}
>  	vmcs_write32(VM_ENTRY_INTR_INFO_FIELD,
>  		vmcs12->vm_entry_intr_info_field);
>  	vmcs_write32(VM_ENTRY_EXCEPTION_ERROR_CODE,
> @@ -7846,7 +7871,6 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
>  	vmcs_write32(GUEST_INTERRUPTIBILITY_INFO,
>  		vmcs12->guest_interruptibility_info);
>  	vmcs_write32(GUEST_SYSENTER_CS, vmcs12->guest_sysenter_cs);
> -	kvm_set_dr(vcpu, 7, vmcs12->guest_dr7);
>  	vmx_set_rflags(vcpu, vmcs12->guest_rflags);
>  	vmcs_writel(GUEST_PENDING_DBG_EXCEPTIONS,
>  		vmcs12->guest_pending_dbg_exceptions);
> @@ -8143,9 +8167,11 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch)
>  	    !vmx_control_verify(vmcs12->pin_based_vm_exec_control,
>  	      nested_vmx_pinbased_ctls_low, nested_vmx_pinbased_ctls_high) ||
>  	    !vmx_control_verify(vmcs12->vm_exit_controls,
> -	      nested_vmx_exit_ctls_low, nested_vmx_exit_ctls_high) ||
> +				nested_vmx_true_exit_ctls_low,
> +				nested_vmx_exit_ctls_high) ||
>  	    !vmx_control_verify(vmcs12->vm_entry_controls,
> -	      nested_vmx_entry_ctls_low, nested_vmx_entry_ctls_high))
> +				nested_vmx_true_entry_ctls_low,
> +				nested_vmx_entry_ctls_high))
>  	{
>  		nested_vmx_failValid(vcpu, VMXERR_ENTRY_INVALID_CONTROL_FIELD);
>  		return 1;
> @@ -8222,6 +8248,9 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch)
>
>  	vmx->nested.vmcs01_tsc_offset = vmcs_read64(TSC_OFFSET);
>
> +	if (!(vmcs12->vm_entry_controls & VM_ENTRY_LOAD_DEBUG_CONTROLS))
> +		vmx->nested.host_debugctl = vmcs_read64(GUEST_IA32_DEBUGCTL);
> +
>  	cpu = get_cpu();
>  	vmx->loaded_vmcs = vmcs02;
>  	vmx_vcpu_put(vcpu);
> @@ -8399,7 +8428,6 @@ static void prepare_vmcs12(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12,
>  	vmcs12->guest_cr0 = vmcs12_guest_cr0(vcpu, vmcs12);
>  	vmcs12->guest_cr4 = vmcs12_guest_cr4(vcpu, vmcs12);
>
> -	kvm_get_dr(vcpu, 7, (unsigned long *)&vmcs12->guest_dr7);
>  	vmcs12->guest_rsp = kvm_register_read(vcpu, VCPU_REGS_RSP);
>  	vmcs12->guest_rip = kvm_register_read(vcpu, VCPU_REGS_RIP);
>  	vmcs12->guest_rflags = vmcs_readl(GUEST_RFLAGS);
> @@ -8478,9 +8506,13 @@ static void prepare_vmcs12(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12,
>  		(vmcs12->vm_entry_controls & ~VM_ENTRY_IA32E_MODE) |
>  		(vm_entry_controls_get(to_vmx(vcpu)) & VM_ENTRY_IA32E_MODE);
>
> +	if (vmcs12->vm_exit_controls & VM_EXIT_SAVE_DEBUG_CONTROLS) {
> +		kvm_get_dr(vcpu, 7, (unsigned long *)&vmcs12->guest_dr7);
> +		vmcs12->guest_ia32_debugctl = vmcs_read64(GUEST_IA32_DEBUGCTL);
> +	}
> +
>  	/* TODO: These cannot have changed unless we have MSR bitmaps and
>  	 * the relevant bit asks not to trap the change */
> -	vmcs12->guest_ia32_debugctl = vmcs_read64(GUEST_IA32_DEBUGCTL);
>  	if (vmcs12->vm_exit_controls & VM_EXIT_SAVE_IA32_PAT)
>  		vmcs12->guest_ia32_pat = vmcs_read64(GUEST_IA32_PAT);
>  	if (vmcs12->vm_exit_controls & VM_EXIT_SAVE_IA32_EFER)
>

Re: [PATCH 0/5] KVM: nVMX: Small fixes improving emulation accuracy

[Paolo Bonzini]

Il 15/06/2014 16:18, Jan Kiszka ha scritto:
> Nothing critical, but it further improves emulation accuracy,
> specifically helpful when analyzing guest bugs...
>
> Corresponding kvm-unit-tests will be provided.
>
> Jan Kiszka (5):
>   KVM: nVMX: Fix returned value of MSR_IA32_VMX_PROCBASED_CTLS
>   KVM: nVMX: Advertise support for MSR_IA32_VMX_TRUE_*_CTLS
>   KVM: nVMX: Allow to disable CR3 access interception
>   KVM: nVMX: Allow to disable VM_{ENTRY_LOAD,EXIT_SAVE}_DEBUG_CONTROLS
>   KVM: nVMX: Fix returned value of MSR_IA32_VMX_VMCS_ENUM
>
>  arch/x86/include/asm/vmx.h            |  3 ++
>  arch/x86/include/uapi/asm/msr-index.h |  1 +
>  arch/x86/kvm/vmx.c                    | 75 +++++++++++++++++++++++++----------
>  3 files changed, 58 insertions(+), 21 deletions(-)

If you reorder the first three patches as 2 then 3 then 1, you do not 
need to temporarily break CR3 access interception.  There is no 
conflict, just changes in the context.  Does that look good to you?

I had a minor comment on patch 4.

Paolo

Re: [PATCH 0/5] KVM: nVMX: Small fixes improving emulation accuracy

[Jan Kiszka]

On 2014-06-16 12:44, Paolo Bonzini wrote:
> Il 15/06/2014 16:18, Jan Kiszka ha scritto:
>> Nothing critical, but it further improves emulation accuracy,
>> specifically helpful when analyzing guest bugs...
>>
>> Corresponding kvm-unit-tests will be provided.
>>
>> Jan Kiszka (5):
>>   KVM: nVMX: Fix returned value of MSR_IA32_VMX_PROCBASED_CTLS
>>   KVM: nVMX: Advertise support for MSR_IA32_VMX_TRUE_*_CTLS
>>   KVM: nVMX: Allow to disable CR3 access interception
>>   KVM: nVMX: Allow to disable VM_{ENTRY_LOAD,EXIT_SAVE}_DEBUG_CONTROLS
>>   KVM: nVMX: Fix returned value of MSR_IA32_VMX_VMCS_ENUM
>>
>>  arch/x86/include/asm/vmx.h            |  3 ++
>>  arch/x86/include/uapi/asm/msr-index.h |  1 +
>>  arch/x86/kvm/vmx.c                    | 75
>> +++++++++++++++++++++++++----------
>>  3 files changed, 58 insertions(+), 21 deletions(-)
> 
> If you reorder the first three patches as 2 then 3 then 1, you do not
> need to temporarily break CR3 access interception.  There is no
> conflict, just changes in the context.  Does that look good to you?

Fine with me, but I guess I should adjust some commit message(s)
accordingly. Resend?

> 
> I had a minor comment on patch 4.

Which is fine with me.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH 0/5] KVM: nVMX: Small fixes improving emulation accuracy

[Paolo Bonzini]

Il 16/06/2014 13:25, Jan Kiszka ha scritto:
>> >
>> > If you reorder the first three patches as 2 then 3 then 1, you do not
>> > need to temporarily break CR3 access interception.  There is no
>> > conflict, just changes in the context.  Does that look good to you?
> Fine with me, but I guess I should adjust some commit message(s)
> accordingly. Resend?
>

Yes, please.

Paolo