Sorry for the late response > -----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. OK > > > > > [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. What PIT case are you talking about? Can you elaborate it? > > > > > [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. We will look into that, thanks for pointing out. And we will use option 1 here first > > (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. Ok, we will look into that, if it is the case, we will correct it > > > > -- > 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
