Re: [PATCH v7 4/7] KVM: Call common update function when ioapic entry changed.

[Gleb Natapov <gleb@redhat.com>]

On Sun, Apr 07, 2013 at 02:00:04PM +0000, Zhang, Yang Z wrote:
> Gleb Natapov wrote on 2013-04-07:
> > On Mon, Apr 01, 2013 at 11:32:32AM +0800, Yang Zhang wrote:
> >> From: Yang Zhang <yang.z.zhang@Intel.com>
> >> 
> >> Both TMR and EOI exit bitmap need to be updated when ioapic changed
> >> or vcpu's id/ldr/dfr changed. So use common function instead eoi exit
> >> bitmap specific function.
> >> 
> >> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> >> ---
> >>  arch/ia64/kvm/lapic.h    |    6 ------
> >>  arch/x86/kvm/lapic.c     |    2 +-
> >>  arch/x86/kvm/vmx.c       |    3 +++
> >>  arch/x86/kvm/x86.c       |   11 +++++++----
> >>  include/linux/kvm_host.h |    4 ++--
> >>  virt/kvm/ioapic.c        |   26 +++++++++++++++-----------
> >>  virt/kvm/ioapic.h        |    7 +++----
> >>  virt/kvm/irq_comm.c      |    4 ++--
> >>  virt/kvm/kvm_main.c      |    4 ++--
> >>  9 files changed, 35 insertions(+), 32 deletions(-)
> >> diff --git a/arch/ia64/kvm/lapic.h b/arch/ia64/kvm/lapic.h
> >> index c3e2935..c5f92a9 100644
> >> --- a/arch/ia64/kvm/lapic.h
> >> +++ b/arch/ia64/kvm/lapic.h
> >> @@ -27,10 +27,4 @@ int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct
> > kvm_lapic_irq *irq);
> >>  #define kvm_apic_present(x) (true)
> >>  #define kvm_lapic_enabled(x) (true)
> >> -static inline bool kvm_apic_vid_enabled(void)
> >> -{
> >> -	/* IA64 has no apicv supporting, do nothing here */
> >> -	return false;
> >> -}
> >> -
> >>  #endif
> >> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> >> index e227474..ce8d6f6 100644
> >> --- a/arch/x86/kvm/lapic.c
> >> +++ b/arch/x86/kvm/lapic.c
> >> @@ -209,7 +209,7 @@ out:
> >>  	if (old)
> >>  		kfree_rcu(old, rcu);
> >> -	kvm_ioapic_make_eoibitmap_request(kvm);
> >> +	kvm_vcpu_scan_ioapic(kvm);
> >>  }
> >>  
> >>  static inline void kvm_apic_set_id(struct kvm_lapic *apic, u8 id)
> >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index
> >> b2e95bc..edfc87a 100644 --- a/arch/x86/kvm/vmx.c +++
> >> b/arch/x86/kvm/vmx.c @@ -6420,6 +6420,9 @@ static void
> >> vmx_hwapic_irr_update(struct kvm_vcpu *vcpu, int max_irr)
> >> 
> >>  static void vmx_load_eoi_exitmap(struct kvm_vcpu *vcpu, u64
> >>  *eoi_exit_bitmap) {
> >> +	if (!vmx_vm_has_apicv(vcpu->kvm))
> >> +		return;
> >> +
> >>  	vmcs_write64(EOI_EXIT_BITMAP0, eoi_exit_bitmap[0]);
> >>  	vmcs_write64(EOI_EXIT_BITMAP1, eoi_exit_bitmap[1]);
> >>  	vmcs_write64(EOI_EXIT_BITMAP2, eoi_exit_bitmap[2]);
> >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> >> index 4d42fe1..64241b6 100644
> >> --- a/arch/x86/kvm/x86.c
> >> +++ b/arch/x86/kvm/x86.c
> >> @@ -5647,13 +5647,16 @@ static void kvm_gen_update_masterclock(struct
> > kvm *kvm)
> >>  #endif
> >>  }
> >> -static void update_eoi_exitmap(struct kvm_vcpu *vcpu)
> >> +static void vcpu_scan_ioapic(struct kvm_vcpu *vcpu)
> >>  {
> >>  	u64 eoi_exit_bitmap[4];
> >> +	if (!kvm_lapic_enabled(vcpu))
> >> +		return;
> >> +
> > Why is this needed here?
> We don't need to calculate eoi_exit_bitmap and TMR if lapic is not enabled. Also, ioapic is meaningless for the vcpu that doesn't enable the lapic.
> 
OK, but then let's use apic_enabled() since kvm_lapic_enabled() also
checks for in kernel apic and we should not be here if apic is not
emulated in kernel. Also please make sure that we rescan ioapic on all
apic state changes.

--
			Gleb.