Re: [PATCH v8 2/3] x86, apicv: add virtual interrupt delivery support

Marcelo Tosatti
2013-01-08T13:40:42Z

On Tue, Jan 08, 2013 at 12:43:22AM +0000, Zhang, Yang Z wrote:
> Marcelo Tosatti wrote on 2013-01-08:
> > On Mon, Jan 07, 2013 at 07:48:43PM +0200, Gleb Natapov wrote:
> >>> ioapic_write (or any other ioapic update)
> >>> lock()
> >>> perform update
> >>> make_all_vcpus_request(KVM_REQ_UPDATE_EOI_BITMAP) (*)
> >>> unlock()
> >>>
> >>> (*) Similarly to TLB flush.
> >>>
> >>> The advantage is that all work becomes vcpu local. The end result
> >>> is much simpler code.
> >> What complexity will it remove?
> >
> > Synchronization between multiple CPUs (except the KVM_REQ_ bit
> > processing, which is infrastructure shared by other parts of KVM).
> >
> > We agreed that performance is non issue here.
> The current logic is this:
> ioapic_write
> lock()
> perform update
> make request on each vcpu
> kick each vcpu
> unlock()
>
> The only difference is the way to make the request. And the complex part is performing update. With your suggestion, we still need to do the update. Why you think it is much simpler?

The update should be local, because there is no reason to make it
remote.

kvm_for_each_vcpu(index, vcpu, kvm)
kvm_x86_ops->update_exitmap_start(vcpu);

for (index = 0; index < IOAPIC_NUM_PINS; index++) {
e = &ioapic->redirtbl[index];
if (!e->fields.mask)
ioapic_update_eoi_exitmap_one(ioapic, index);
}

kvm_for_each_vcpu(index, vcpu, kvm) {
kvm_x86_ops->update_exitmap_end(vcpu);
kvm_vcpu_kick(vcpu);
}

No need for start, end, etc calls into vcpus. All ioapic updater does
is set a request bit, similar to a remote TLB flush. Update is then
entirely done vcpu local.