Re: [PATCH 3/5]KVM:x86, apicv: enable virtual interrupt delivery for VMX

[Avi Kivity <avi@redhat.com>]

On 09/14/2012 05:17 PM, Li, Jiongxi wrote:
>> >
>> > -static void apic_send_ipi(struct kvm_lapic *apic)
>> > +/*
>> > + * this interface assumes a trap-like exit, which has already
>> > +finished
>> > + * desired side effect including vISR and vPPR update.
>> > + */
>> > +void kvm_apic_set_eoi(struct kvm_vcpu *vcpu, int vector) {
>> > +	struct kvm_lapic *apic = vcpu->arch.apic;
>> > +	int trigger_mode;
>> > +
>> > +	if (apic_test_and_clear_vector(vector, apic->regs + APIC_TMR))
>> > +		trigger_mode = IOAPIC_LEVEL_TRIG;
>> > +	else
>> > +		trigger_mode = IOAPIC_EDGE_TRIG;
>> > +
>> > +	if (!(apic_get_reg(apic, APIC_SPIV) & APIC_SPIV_DIRECTED_EOI))
>> > +		kvm_ioapic_update_eoi(apic->vcpu->kvm, vector, trigger_mode);
>> > +	kvm_make_request(KVM_REQ_EVENT, apic->vcpu); }
>> > +EXPORT_SYMBOL_GPL(kvm_apic_set_eoi);
>> 
>> What's the difference between this and apic_set_eoi()?

> In kvm_apic_set_eoi, We can use the vector directly from exit_qualification while there is EOI-induced VMExit, and doesn't need to do the 'clear isr', 'update ppr' things which are handled by hardware.

The name needs to reflect this.  For most functions we can add _traplike
to the name, but here the hardware has done even more things for us, so
we can call it _accelerated.  Please add a comment detailing the
differences.

>> 
>> >  /*
>> >   * If nested=1, nested virtualization is supported, i.e., guests may use
>> >   * VMX and be a hypervisor for its own guests. If nested=0, guests
>> > may not @@ -430,6 +433,8 @@ struct vcpu_vmx {
>> >
>> >  	bool rdtscp_enabled;
>> >
>> > +	u64 eoi_exit_bitmap[4];
>> > +
>> 
>> Unused?
> This is used in PATCH 4/5

Then move it there please.

>> >
>> > +	if (enable_apicv_vid) {
>> > +		vmcs_write64(EOI_EXIT_BITMAP0, 0);
>> > +		vmcs_write64(EOI_EXIT_BITMAP1, 0);
>> > +		vmcs_write64(EOI_EXIT_BITMAP2, 0);
>> > +		vmcs_write64(EOI_EXIT_BITMAP3, 0);
>> > +
>> > +		vmcs_write16(GUEST_INTR_STATUS, 0);
>> 
>> Need to update GUEST_INTR_STATUS after live migration (or perhaps also
>> when enabling the APIC?)
> After live migration, GUEST_INTR_STATUS will be updated in VMEntry. 'kvm_x86_ops->update_irq(vcpu)' function does that.

Ok.


-- 
error compiling committee.c: too many arguments to function