diff mbox series

[kvm-unit-tests] x86: Restore VMCS state when test_vmcs_addr() is done

Message ID 20190502145741.7863-1-nadav.amit@gmail.com (mailing list archive)
State New, archived
Headers show
Series [kvm-unit-tests] x86: Restore VMCS state when test_vmcs_addr() is done | expand

Commit Message

Nadav Amit May 2, 2019, 2:57 p.m. UTC 
From: Nadav Amit <nadav.amit@gmail.com>

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.

Signed-off-by: Nadav Amit <nadav.amit@gmail.com>
---
 x86/vmx_tests.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Comments

Krish Sadhukhan May 10, 2019, 11:49 p.m. UTC | #1 
On 05/02/2019 07:57 AM, nadav.amit@gmail.com wrote:
> From: Nadav Amit <nadav.amit@gmail.com>
>
> 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.
>
> Signed-off-by: Nadav Amit <nadav.amit@gmail.com>
> ---
>   x86/vmx_tests.c | 12 +++++++-----
>   1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index e9010af..2d6b12d 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -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);
>   }
>   

The function name in the commit message (both header and body) is 
incorrect. It should be "test_tpr_threshold".
Nadav Amit May 10, 2019, 11:54 p.m. UTC | #2 
> On May 10, 2019, at 4:49 PM, Krish Sadhukhan <krish.sadhukhan@oracle.com> wrote:
> 
> 
> 
> On 05/02/2019 07:57 AM, nadav.amit@gmail.com wrote:
>> From: Nadav Amit <nadav.amit@gmail.com>
>> 
>> 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.
>> 
>> Signed-off-by: Nadav Amit <nadav.amit@gmail.com>
>> ---
>>  x86/vmx_tests.c | 12 +++++++-----
>>  1 file changed, 7 insertions(+), 5 deletions(-)
>> 
>> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
>> index e9010af..2d6b12d 100644
>> --- a/x86/vmx_tests.c
>> +++ b/x86/vmx_tests.c
>> @@ -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);
>>  }
>>  
> 
> The function name in the commit message (both header and body) is
> incorrect. It should be “test_tpr_threshold”.

Too many patches… Will fix.
diff mbox series

Patch

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index e9010af..2d6b12d 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -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);
 }