Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1329254 > unrolled thread

[PATCH 1/3] KVM: APIC: remove unnecessary double checks on APIC existence

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 1/3] KVM: APIC: remove unnecessary double checks on APIC existence Paolo Bonzini <pbonzini@redhat.com> - 2016-02-08 17:20 +0100
    Re: [PATCH 1/3] KVM: APIC: remove unnecessary double checks on APIC  existence Radim Krčmář <rkrcmar@redhat.com> - 2016-02-09 15:00 +0100
      Re: [PATCH 1/3] KVM: APIC: remove unnecessary double checks on APIC  existence Paolo Bonzini <pbonzini@redhat.com> - 2016-02-09 16:10 +0100
        Re: [PATCH 1/3] KVM: APIC: remove unnecessary double checks on APIC  existence Radim Krčmář <rkrcmar@redhat.com> - 2016-02-09 16:20 +0100

#1329254 — [PATCH 1/3] KVM: APIC: remove unnecessary double checks on APIC existence

Usually the in-kernel APIC's existence is checked in the caller.  Do not
bother checking it again in lapic.c.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/lapic.c | 19 +++----------------
 1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 1520d1acd0ad..b1029051f664 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -475,18 +475,12 @@ static inline void apic_clear_isr(int vec, struct kvm_lapic *apic)
 
 int kvm_lapic_find_highest_irr(struct kvm_vcpu *vcpu)
 {
-	int highest_irr;
-
 	/* This may race with setting of irr in __apic_accept_irq() and
 	 * value returned may be wrong, but kvm_vcpu_kick() in __apic_accept_irq
 	 * will cause vmexit immediately and the value will be recalculated
 	 * on the next vmentry.
 	 */
-	if (!kvm_vcpu_has_lapic(vcpu))
-		return 0;
-	highest_irr = apic_find_highest_irr(vcpu->arch.apic);
-
-	return highest_irr;
+	return apic_find_highest_irr(vcpu->arch.apic);
 }
 
 static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
@@ -1601,8 +1595,7 @@ static int apic_mmio_write(struct kvm_vcpu *vcpu, struct kvm_io_device *this,
 
 void kvm_lapic_set_eoi(struct kvm_vcpu *vcpu)
 {
-	if (kvm_vcpu_has_lapic(vcpu))
-		apic_reg_write(vcpu->arch.apic, APIC_EOI, 0);
+	apic_reg_write(vcpu->arch.apic, APIC_EOI, 0);
 }
 EXPORT_SYMBOL_GPL(kvm_lapic_set_eoi);
 
@@ -1676,9 +1669,6 @@ void kvm_lapic_set_tpr(struct kvm_vcpu *vcpu, unsigned long cr8)
 {
 	struct kvm_lapic *apic = vcpu->arch.apic;
 
-	if (!kvm_vcpu_has_lapic(vcpu))
-		return;
-
 	apic_set_tpr(apic, ((cr8 & 0x0f) << 4)
 		     | (kvm_apic_get_reg(apic, APIC_TASKPRI) & 4));
 }
@@ -1687,9 +1677,6 @@ u64 kvm_lapic_get_cr8(struct kvm_vcpu *vcpu)
 {
 	u64 tpr;
 
-	if (!kvm_vcpu_has_lapic(vcpu))
-		return 0;
-
 	tpr = (u64) kvm_apic_get_reg(vcpu->arch.apic, APIC_TASKPRI);
 
 	return (tpr & 0xf0) >> 4;
@@ -1912,7 +1899,7 @@ int kvm_apic_has_interrupt(struct kvm_vcpu *vcpu)
 	struct kvm_lapic *apic = vcpu->arch.apic;
 	int highest_irr;
 
-	if (!kvm_vcpu_has_lapic(vcpu) || !apic_enabled(apic))
+	if (!apic_enabled(apic))
 		return -1;
 
 	apic_update_ppr(apic);
-- 
1.8.3.1

[toc] | [next] | [standalone]

#1330244 — Re: [PATCH 1/3] KVM: APIC: remove unnecessary double checks on APIC existence

2016-02-08 17:15+0100, Paolo Bonzini:
> Usually the in-kernel APIC's existence is checked in the caller.  Do not
> bother checking it again in lapic.c.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
>  static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
> @@ -1601,8 +1595,7 @@ static int apic_mmio_write(struct kvm_vcpu *vcpu, struct kvm_io_device *this,
>  
>  void kvm_lapic_set_eoi(struct kvm_vcpu *vcpu)
>  {
> -	if (kvm_vcpu_has_lapic(vcpu))
> -		apic_reg_write(vcpu->arch.apic, APIC_EOI, 0);
> +	apic_reg_write(vcpu->arch.apic, APIC_EOI, 0);

This is most likely going to bug on the following path:
  handle_apic_access -> kvm_lapic_set_eoi

Before the change, handle_apic_access would just drop EOIs that should
have gone to user space ... I'm not sure if we tested it, or the path is
really never taken.

[toc] | [prev] | [next] | [standalone]

#1330307 — Re: [PATCH 1/3] KVM: APIC: remove unnecessary double checks on APIC existence

On 09/02/2016 14:55, Radim Krčmář wrote:
> This is most likely going to bug on the following path:
>   handle_apic_access -> kvm_lapic_set_eoi
> 
> Before the change, handle_apic_access would just drop EOIs that should
> have gone to user space ... I'm not sure if we tested it, or the path is
> really never taken.

cpu_need_virtualize_apic_accesses is needed to set
SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES (vmx_secondary_exec_control) and
SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES is required to get to
handle_apic_access.

Paolo

[toc] | [prev] | [next] | [standalone]

#1330331 — Re: [PATCH 1/3] KVM: APIC: remove unnecessary double checks on APIC existence

2016-02-09 16:06+0100, Paolo Bonzini:
> On 09/02/2016 14:55, Radim Krčmář wrote:
>> This is most likely going to bug on the following path:
>>   handle_apic_access -> kvm_lapic_set_eoi
>> 
>> Before the change, handle_apic_access would just drop EOIs that should
>> have gone to user space ... I'm not sure if we tested it, or the path is
>> really never taken.
> 
> cpu_need_virtualize_apic_accesses is needed to set
> SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES (vmx_secondary_exec_control) and
> SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES is required to get to
> handle_apic_access.

Thanks,

Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web