From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marcelo Tosatti <mtosatti@redhat.com>
Subject: Re: [PATCH v8 2/3] x86, apicv: add virtual interrupt delivery support
Date: Tue, 8 Jan 2013 11:40:20 -0200
Message-ID: <20130108134020.GA15222@amt.cnet>
References: <1357524157-4666-1-git-send-email-yang.z.zhang@intel.com>
 <1357524157-4666-3-git-send-email-yang.z.zhang@intel.com>
 <20130107135221.GA25775@amt.cnet>
 <20130107174843.GA4872@redhat.com>
 <20130107213239.GB31677@amt.cnet>
 <A9667DDFB95DB7438FA9D7D576C3D87E2F341F@SHSMSX101.ccr.corp.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Gleb Natapov <gleb@redhat.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"Shan, Haitao" <haitao.shan@intel.com>,
	"Tian, Kevin" <kevin.tian@intel.com>
To: "Zhang, Yang Z" <yang.z.zhang@intel.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:41970 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1754754Ab3AHNkm (ORCPT <rfc822;kvm@vger.kernel.org>);
	Tue, 8 Jan 2013 08:40:42 -0500
Content-Disposition: inline
In-Reply-To: <A9667DDFB95DB7438FA9D7D576C3D87E2F341F@SHSMSX101.ccr.corp.intel.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

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.