RE: [PATCH 4/5]KVM:x86, apicv: add interface for poking EOI exit bitmap

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Avi Kivity [mailto:avi@xxxxxxxxxx]
> Sent: Friday, September 07, 2012 12:38 AM
> To: Li, Jiongxi
> Cc: kvm@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH 4/5]KVM:x86, apicv: add interface for poking EOI exit
> bitmap
> 
> On 09/05/2012 08:41 AM, Li, Jiongxi wrote:
> > With APICv virtual interrupt delivery feature, EOI write from non root
> > mode doesn't cause VM-Exit unless set in EOI exit bitmap VMCS field.
> > Basically there're two methods to manipulate EOI exit bitmap:
> 
> Should be folded into the previous patch, otherwise the previous patch breaks
> level interrupts.
> 
> >
> > [Option 1]
> > Ideally only level triggered irq requires a hook in vLAPIC EOI write,
> > so that vIOAPIC EOI is triggered and emulated. So the simplest
> > approach is to manipulate EOI exit bitmap when vLAPIC acks a new
> > interrupt, based on value of TMR. There're several corner cases worthy
> > of note though:
> >
> >   - KVM has specific notifier hooks on vIOAPIC EOI path. So far two
> >     sources use it: INT-based device passthrough and PIT pending
> >     timers. For the former, it's virtually wired to vIOAPIC and
> >     thus TMR already covers it. PIT is special here, which is an
> >     edge triggered source. But since other timer sources like
> >     vLAPIC timer don't require this notifier hook, possibly PIT
> >     can be relaxed in the future too.
> 
> I would like to switch to changing the timer frequency when we need to catch
> up.  But that can be done later.
> 
> >
> >   - posted interrupt will update TMR directly, w/o chance for KVM
> >     to update EOI exit bitmap accordingly. This becomes a gap
> 
> Why not? we know what vector the PIT is wired to.
> 
> >
> > [Option 2]
> > Indicate EOI exit bitmap requirement ('need_eoi') directly from every
> > interrupt source device, and then check this requirement when vLAPIC
> > acks a new pending interrupt. This requires more intrusive changes to
> > current vLAPIC/vIOAPIC logic, so that the "irq_source_id" indicating
> > source of interrupt is passed through from origination point to vLAPIC
> > ack point. For natual requirement like vIOAPIC level triggered
> > entries, it can be implicitly deduced.
> > On the other hand for non-natural requirements like aformentioned PIT
> > or posted interrupt, this approach can handle it efficiently.
> >
> > For simplicity reason, now option 1 is used which should be enough to
> > test MSI-based device passthrough.
> 
> You can change kvm_register_irq_ack_notifier() to call the ioapic and pic to find
> out what vectors need EOI exits.
> 
> (alternatively, if we fix the PIT, then we only need ack notifiers for level
> interrupts).
> 
> >
> > Signed-off-by: Kevin Tian <kevin.tian@xxxxxxxxx>
> > Signed-off-by: Jiongxi Li <jiongxi.li@xxxxxxxxx>
> > ---
> >  arch/x86/include/asm/kvm_host.h |    1 +
> >  arch/x86/kvm/lapic.c            |    7 ++++++-
> >  arch/x86/kvm/vmx.c              |   37
> +++++++++++++++++++++++++++++++++++++
> >  3 files changed, 44 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/kvm_host.h
> > b/arch/x86/include/asm/kvm_host.h index ef74df5..4e06a82 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -671,6 +671,7 @@ struct kvm_x86_ops {
> >  	void (*update_cr8_intercept)(struct kvm_vcpu *vcpu, int tpr, int irr);
> >  	int (*has_virtual_interrupt_delivery)(struct kvm_vcpu *vcpu);
> >  	void (*update_irq)(struct kvm_vcpu *vcpu);
> > +	void (*set_eoi_exitmap)(struct kvm_vcpu *vcpu, int vector, int
> > +need_eoi);
> >  	int (*set_tss_addr)(struct kvm *kvm, unsigned int addr);
> >  	int (*get_tdp_level)(void);
> >  	u64 (*get_mt_mask)(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio);
> > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index
> > d203501..4058384 100644
> > --- a/arch/x86/kvm/lapic.c
> > +++ b/arch/x86/kvm/lapic.c
> > @@ -499,8 +499,13 @@ static int __apic_accept_irq(struct kvm_lapic *apic,
> int delivery_mode,
> >  		if (trig_mode) {
> >  			apic_debug("level trig mode for vector %d", vector);
> >  			apic_set_vector(vector, apic->regs + APIC_TMR);
> > -		} else
> > +			if (kvm_apic_vid_enabled(vcpu))
> > +				kvm_x86_ops->set_eoi_exitmap(vcpu, vector, 1);
> > +		} else {
> >  			apic_clear_vector(vector, apic->regs + APIC_TMR);
> > +			if (kvm_apic_vid_enabled(vcpu))
> > +				kvm_x86_ops->set_eoi_exitmap(vcpu, vector, 0);
> > +		}
> 
> This is way too late.  The flow should come from the IOAPIC and PIC, when
> setting up an irq, to the local APIC.
> 
I just wonder why you think it is too late to do that. Out of the reason that If post interrupt is enabled, here won't be called? Or because we can't set eoi bitmap for PIT here just according to TMR and needs to set it in a place where can recognize the PIT vector?
> 
> 
> --
> error compiling committee.c: too many arguments to function
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux