diff mbox series

[1/2] KVM: VMX: Refactor update_cr8_intercept()

Message ID 20191111123055.93270-2-liran.alon@oracle.com (mailing list archive)
State New, archived
Headers show
Series KVM: nVMX: Update vmcs01 TPR_THRESHOLD if L2 changed L1 TPR | expand

Commit Message

Liran Alon Nov. 11, 2019, 12:30 p.m. UTC 
No functional changes.

Reviewed-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Liran Alon <liran.alon@oracle.com>
---
 arch/x86/kvm/vmx/vmx.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

Comments

Paolo Bonzini Nov. 11, 2019, 2:57 p.m. UTC | #1 
On 11/11/19 13:30, Liran Alon wrote:
> No functional changes.
> 
> Reviewed-by: Joao Martins <joao.m.martins@oracle.com>
> Signed-off-by: Liran Alon <liran.alon@oracle.com>
> ---
>  arch/x86/kvm/vmx/vmx.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index f53b0c74f7c8..d5742378d031 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -6013,17 +6013,14 @@ static void vmx_l1d_flush(struct kvm_vcpu *vcpu)
>  static void update_cr8_intercept(struct kvm_vcpu *vcpu, int tpr, int irr)
>  {
>  	struct vmcs12 *vmcs12 = get_vmcs12(vcpu);
> +	int tpr_threshold;
>  
>  	if (is_guest_mode(vcpu) &&
>  		nested_cpu_has(vmcs12, CPU_BASED_TPR_SHADOW))
>  		return;
>  
> -	if (irr == -1 || tpr < irr) {
> -		vmcs_write32(TPR_THRESHOLD, 0);
> -		return;
> -	}
> -
> -	vmcs_write32(TPR_THRESHOLD, irr);
> +	tpr_threshold = ((irr == -1) || (tpr < irr)) ? 0 : irr;

Pascal parentheses? :)

Paolo

> +	vmcs_write32(TPR_THRESHOLD, tpr_threshold);
>  }
>  
>  void vmx_set_virtual_apic_mode(struct kvm_vcpu *vcpu)
>
Liran Alon Nov. 11, 2019, 3 p.m. UTC | #2 
> On 11 Nov 2019, at 16:57, Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> On 11/11/19 13:30, Liran Alon wrote:
>> No functional changes.
>> 
>> Reviewed-by: Joao Martins <joao.m.martins@oracle.com>
>> Signed-off-by: Liran Alon <liran.alon@oracle.com>
>> ---
>> arch/x86/kvm/vmx/vmx.c | 9 +++------
>> 1 file changed, 3 insertions(+), 6 deletions(-)
>> 
>> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>> index f53b0c74f7c8..d5742378d031 100644
>> --- a/arch/x86/kvm/vmx/vmx.c
>> +++ b/arch/x86/kvm/vmx/vmx.c
>> @@ -6013,17 +6013,14 @@ static void vmx_l1d_flush(struct kvm_vcpu *vcpu)
>> static void update_cr8_intercept(struct kvm_vcpu *vcpu, int tpr, int irr)
>> {
>> 	struct vmcs12 *vmcs12 = get_vmcs12(vcpu);
>> +	int tpr_threshold;
>> 
>> 	if (is_guest_mode(vcpu) &&
>> 		nested_cpu_has(vmcs12, CPU_BASED_TPR_SHADOW))
>> 		return;
>> 
>> -	if (irr == -1 || tpr < irr) {
>> -		vmcs_write32(TPR_THRESHOLD, 0);
>> -		return;
>> -	}
>> -
>> -	vmcs_write32(TPR_THRESHOLD, irr);
>> +	tpr_threshold = ((irr == -1) || (tpr < irr)) ? 0 : irr;
> 
> Pascal parentheses? :)
> 
> Paolo

What do you mean?

-Liran

> 
>> +	vmcs_write32(TPR_THRESHOLD, tpr_threshold);
>> }
>> 
>> void vmx_set_virtual_apic_mode(struct kvm_vcpu *vcpu)
>> 
>
Paolo Bonzini Nov. 11, 2019, 4:01 p.m. UTC | #3 
On 11/11/19 16:00, Liran Alon wrote:
> 
> 
>> On 11 Nov 2019, at 16:57, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> On 11/11/19 13:30, Liran Alon wrote:
>>> No functional changes.
>>>
>>> Reviewed-by: Joao Martins <joao.m.martins@oracle.com>
>>> Signed-off-by: Liran Alon <liran.alon@oracle.com>
>>> ---
>>> arch/x86/kvm/vmx/vmx.c | 9 +++------
>>> 1 file changed, 3 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>>> index f53b0c74f7c8..d5742378d031 100644
>>> --- a/arch/x86/kvm/vmx/vmx.c
>>> +++ b/arch/x86/kvm/vmx/vmx.c
>>> @@ -6013,17 +6013,14 @@ static void vmx_l1d_flush(struct kvm_vcpu *vcpu)
>>> static void update_cr8_intercept(struct kvm_vcpu *vcpu, int tpr, int irr)
>>> {
>>> 	struct vmcs12 *vmcs12 = get_vmcs12(vcpu);
>>> +	int tpr_threshold;
>>>
>>> 	if (is_guest_mode(vcpu) &&
>>> 		nested_cpu_has(vmcs12, CPU_BASED_TPR_SHADOW))
>>> 		return;
>>>
>>> -	if (irr == -1 || tpr < irr) {
>>> -		vmcs_write32(TPR_THRESHOLD, 0);
>>> -		return;
>>> -	}
>>> -
>>> -	vmcs_write32(TPR_THRESHOLD, irr);
>>> +	tpr_threshold = ((irr == -1) || (tpr < irr)) ? 0 : irr;
>>
>> Pascal parentheses? :)
> 
> What do you mean?

Redundant parentheses around && or || are usually avoided in the kernel,
and they are typical of Pascal (which had weird operator precedence
rules and thus required operands of AND/OR to be parenthesized).

I can remove them when committing the series.

Paolo
Liran Alon Nov. 11, 2019, 4:02 p.m. UTC | #4 
> On 11 Nov 2019, at 18:01, Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> On 11/11/19 16:00, Liran Alon wrote:
>> 
>> 
>>> On 11 Nov 2019, at 16:57, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>> 
>>> On 11/11/19 13:30, Liran Alon wrote:
>>>> No functional changes.
>>>> 
>>>> Reviewed-by: Joao Martins <joao.m.martins@oracle.com>
>>>> Signed-off-by: Liran Alon <liran.alon@oracle.com>
>>>> ---
>>>> arch/x86/kvm/vmx/vmx.c | 9 +++------
>>>> 1 file changed, 3 insertions(+), 6 deletions(-)
>>>> 
>>>> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>>>> index f53b0c74f7c8..d5742378d031 100644
>>>> --- a/arch/x86/kvm/vmx/vmx.c
>>>> +++ b/arch/x86/kvm/vmx/vmx.c
>>>> @@ -6013,17 +6013,14 @@ static void vmx_l1d_flush(struct kvm_vcpu *vcpu)
>>>> static void update_cr8_intercept(struct kvm_vcpu *vcpu, int tpr, int irr)
>>>> {
>>>> 	struct vmcs12 *vmcs12 = get_vmcs12(vcpu);
>>>> +	int tpr_threshold;
>>>> 
>>>> 	if (is_guest_mode(vcpu) &&
>>>> 		nested_cpu_has(vmcs12, CPU_BASED_TPR_SHADOW))
>>>> 		return;
>>>> 
>>>> -	if (irr == -1 || tpr < irr) {
>>>> -		vmcs_write32(TPR_THRESHOLD, 0);
>>>> -		return;
>>>> -	}
>>>> -
>>>> -	vmcs_write32(TPR_THRESHOLD, irr);
>>>> +	tpr_threshold = ((irr == -1) || (tpr < irr)) ? 0 : irr;
>>> 
>>> Pascal parentheses? :)
>> 
>> What do you mean?
> 
> Redundant parentheses around && or || are usually avoided in the kernel,
> and they are typical of Pascal (which had weird operator precedence
> rules and thus required operands of AND/OR to be parenthesized).
> 
> I can remove them when committing the series.
> 
> Paolo

I see. Sure no problem you can remove them.

Thanks,
-Liran
diff mbox series

Patch

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index f53b0c74f7c8..d5742378d031 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -6013,17 +6013,14 @@  static void vmx_l1d_flush(struct kvm_vcpu *vcpu)
 static void update_cr8_intercept(struct kvm_vcpu *vcpu, int tpr, int irr)
 {
 	struct vmcs12 *vmcs12 = get_vmcs12(vcpu);
+	int tpr_threshold;
 
 	if (is_guest_mode(vcpu) &&
 		nested_cpu_has(vmcs12, CPU_BASED_TPR_SHADOW))
 		return;
 
-	if (irr == -1 || tpr < irr) {
-		vmcs_write32(TPR_THRESHOLD, 0);
-		return;
-	}
-
-	vmcs_write32(TPR_THRESHOLD, irr);
+	tpr_threshold = ((irr == -1) || (tpr < irr)) ? 0 : irr;
+	vmcs_write32(TPR_THRESHOLD, tpr_threshold);
 }
 
 void vmx_set_virtual_apic_mode(struct kvm_vcpu *vcpu)