Skip to contentSkip to conversation

x86: Restore VMCS state when test_vmcs_addr() is done

PATCH Superseded [email protected] 2 participants 3 messages May 2, 2019 – May 10, 2019 KVM

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 x86: Restore VMCS state when test_vmcs_addr() is done

From: Nadav Amit <[email protected]>

The VMCS fields of APIC_VIRT_ADDR and TPR_THRESHOLD are modified by test_vmcs_addr() but are not restored to their original value. Save and restore them.

---
 x86/vmx_tests.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
x86/vmx_tests.c +7 -5
index e9010af..2d6b12d 100644
@@ -4432,6 +4432,8 @@ static void test_tpr_threshold_values(void)
 static void test_tpr_threshold(void)
 {
 	u32 primary = vmcs_read(CPU_EXEC_CTRL0);
+	u64 apic_virt_addr = vmcs_read(APIC_VIRT_ADDR);
+	u64 threshold = vmcs_read(TPR_THRESHOLD);
 	void *virtual_apic_page;
 
 	if (!(ctrl_cpu_rev[0].clr & CPU_TPR_SHADOW))
@@ -4451,11 +4453,8 @@ static void test_tpr_threshold(void)
 	report_prefix_pop();
 
 	if (!((ctrl_cpu_rev[0].clr & CPU_SECONDARY) &&
-	    (ctrl_cpu_rev[1].clr & (CPU_VINTD  | CPU_VIRT_APIC_ACCESSES)))) {
-		vmcs_write(CPU_EXEC_CTRL0, primary);
-		return;
-	}
-
+	    (ctrl_cpu_rev[1].clr & (CPU_VINTD  | CPU_VIRT_APIC_ACCESSES))))
+		goto out;
 	u32 secondary = vmcs_read(CPU_EXEC_CTRL1);
 
 	if (ctrl_cpu_rev[1].clr & CPU_VINTD) {
@@ -4505,6 +4504,9 @@ static void test_tpr_threshold(void)
 	}
 
 	vmcs_write(CPU_EXEC_CTRL1, secondary);
+out:
+	vmcs_write(TPR_THRESHOLD, threshold);
+	vmcs_write(APIC_VIRT_ADDR, apic_virt_addr);
 	vmcs_write(CPU_EXEC_CTRL0, primary);
 }
 
Signature · 1 line
2.17.1
#1 lore raw
Quoted 39 lines · On 05/02/2019 07:57 AM, [email protected] wrote:

From: Nadav Amit <[email protected]>

The VMCS fields of APIC_VIRT_ADDR and TPR_THRESHOLD are modified by test_vmcs_addr() but are not restored to their original value. Save and restore them.

---
  x86/vmx_tests.c | 12 +++++++-----
  1 file changed, 7 insertions(+), 5 deletions(-)
x86/vmx_tests.c +4 -5
index e9010af..2d6b12d 100644
@@ -4432,6 +4432,8 @@ static void test_tpr_threshold_values(void)
  static void test_tpr_threshold(void)
  {
  	u32 primary = vmcs_read(CPU_EXEC_CTRL0);
+	u64 apic_virt_addr = vmcs_read(APIC_VIRT_ADDR);
+	u64 threshold = vmcs_read(TPR_THRESHOLD);
  	void *virtual_apic_page;
 
  	if (!(ctrl_cpu_rev[0].clr & CPU_TPR_SHADOW))
@@ -4451,11 +4453,8 @@ static void test_tpr_threshold(void)
  	report_prefix_pop();
 
  	if (!((ctrl_cpu_rev[0].clr & CPU_SECONDARY) &&
-	    (ctrl_cpu_rev[1].clr & (CPU_VINTD  | CPU_VIRT_APIC_ACCESSES)))) {
-		vmcs_write(CPU_EXEC_CTRL0, primary);
-		return;
-	}
-
+	    (ctrl_cpu_rev[1].clr & (CPU_VINTD  | CPU_VIRT_APIC_ACCESSES))))
+		goto out;
  	u32 secondary = vmcs_read(CPU_EXEC_CTRL1);
 
  	if (ctrl_cpu_rev[1].clr & CPU_VINTD) {

x86/vmx_tests.c static void test_tpr_threshold(void) · line 4512

Quoted 9 lines
x86/vmx_tests.c +3 -0
@@ -4505,6 +4504,9 @@ static void test_tpr_threshold(void)
  	}
 
  	vmcs_write(CPU_EXEC_CTRL1, secondary);
+out:
+	vmcs_write(TPR_THRESHOLD, threshold);
+	vmcs_write(APIC_VIRT_ADDR, apic_virt_addr);
  	vmcs_write(CPU_EXEC_CTRL0, primary);
  }
 

The function name in the commit message (both header and body) is incorrect. It should be "test_tpr_threshold".

author
#2 lore raw
Quoted 44 lines · Krish Sadhukhan
Quoted 39 lines · On 05/02/2019 07:57 AM, [email protected] wrote:

From: Nadav Amit <[email protected]>

The VMCS fields of APIC_VIRT_ADDR and TPR_THRESHOLD are modified by test_vmcs_addr() but are not restored to their original value. Save and restore them.

---
 x86/vmx_tests.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
x86/vmx_tests.c +5 -5
index e9010af..2d6b12d 100644
@@ -4432,6 +4432,8 @@ static void test_tpr_threshold_values(void)
 static void test_tpr_threshold(void)
 {
 	u32 primary = vmcs_read(CPU_EXEC_CTRL0);
+	u64 apic_virt_addr = vmcs_read(APIC_VIRT_ADDR);
+	u64 threshold = vmcs_read(TPR_THRESHOLD);
 	void *virtual_apic_page;
   	if (!(ctrl_cpu_rev[0].clr & CPU_TPR_SHADOW))
@@ -4451,11 +4453,8 @@ static void test_tpr_threshold(void)
 	report_prefix_pop();
   	if (!((ctrl_cpu_rev[0].clr & CPU_SECONDARY) &&
-	    (ctrl_cpu_rev[1].clr & (CPU_VINTD  | CPU_VIRT_APIC_ACCESSES)))) {
-		vmcs_write(CPU_EXEC_CTRL0, primary);
-		return;
-	}
-
+	    (ctrl_cpu_rev[1].clr & (CPU_VINTD  | CPU_VIRT_APIC_ACCESSES))))
+		goto out;
 	u32 secondary = vmcs_read(CPU_EXEC_CTRL1);
   	if (ctrl_cpu_rev[1].clr & CPU_VINTD) {
@@ -4505,6 +4504,9 @@ static void test_tpr_threshold(void)
 	}
   	vmcs_write(CPU_EXEC_CTRL1, secondary);
+out:
Quoted 8 lines

x86/vmx_tests.c static void test_tpr_threshold(void) · line 4512

Quoted 5 lines
x86/vmx_tests.c +2 -0
+	vmcs_write(TPR_THRESHOLD, threshold);
+	vmcs_write(APIC_VIRT_ADDR, apic_virt_addr);
 	vmcs_write(CPU_EXEC_CTRL0, primary);
 }
 

The function name in the commit message (both header and body) is incorrect. It should be “test_tpr_threshold”.

Too many patches… Will fix.