From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marcelo Tosatti <mtosatti@redhat.com>
Subject: Re: [PATCH v4 2/2] KVM: VMX: Add Posted Interrupt supporting
Date: Mon, 25 Feb 2013 10:01:25 -0300
Message-ID: <20130225130125.GB2057@amt.cnet>
References: <1361540552-2016-1-git-send-email-yang.z.zhang@intel.com>
 <1361540552-2016-3-git-send-email-yang.z.zhang@intel.com>
 <20130223134309.GA2632@amt.cnet>
 <A9667DDFB95DB7438FA9D7D576C3D87E099BAF6C@SHSMSX101.ccr.corp.intel.com>
 <20130223144406.GA4663@amt.cnet>
 <A9667DDFB95DB7438FA9D7D576C3D87E099BB015@SHSMSX101.ccr.corp.intel.com>
 <20130223165034.GA8236@amt.cnet>
 <A9667DDFB95DB7438FA9D7D576C3D87E099BB4EE@SHSMSX101.ccr.corp.intel.com>
 <20130224173902.GA9393@amt.cnet>
 <A9667DDFB95DB7438FA9D7D576C3D87E099BBA4B@SHSMSX101.ccr.corp.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"gleb@redhat.com" <gleb@redhat.com>,
	"avi.kivity@gmail.com" <avi.kivity@gmail.com>,
	"Zhang, Xiantao" <xiantao.zhang@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]:51695 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1755982Ab3BYOAi (ORCPT <rfc822;kvm@vger.kernel.org>);
	Mon, 25 Feb 2013 09:00:38 -0500
Content-Disposition: inline
In-Reply-To: <A9667DDFB95DB7438FA9D7D576C3D87E099BBA4B@SHSMSX101.ccr.corp.intel.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Mon, Feb 25, 2013 at 06:55:58AM +0000, Zhang, Yang Z wrote:
> > 
> > p1)
> > 
> >>> cpu0					cpu1		vcpu0
> >>> test_and_set_bit(PIR-A)
> >>> set KVM_REQ_EVENT
> >>> 							process REQ_EVENT
> >>> 							PIR-A->IRR
> >>> 
> >>> 							vcpu->mode=IN_GUEST
> >>> 
> >>> if (vcpu0->guest_mode)
> >>> 	if (!t_a_s_bit(PIR notif))
> >>> 		send IPI
> >>> 							linux_pir_handler
> >>> 
> >>> 					t_a_s_b(PIR-B)=1
> >>> 					no PIR IPI sent
> > 
> > p2)
> > 
> >> No, this exists with your implementation not mine.
> >> As I said, in this patch, it will make request after vcpu ==guest mode, then send
> > ipi. Even the ipi is lost, but the request still will be tracked. Because we have the
> > follow check:
> >> vcpu->mode = GUEST_MODE
> >> (ipi may arrived here and lost)
> >> local irq disable
> >> check request (this will ensure the pir modification will be picked up by vcpu
> > before vmentry)
> > 
> > Please read the sequence above again, between p1) and p2). Yes, if the
> > IPI is lost, the request _for the CPU which sent PIR IPI_ is guaranteed
> > to be processed, but not the request for another cpu (cpu1).
> If no PIR IPI sent in cpu1, the delivered is false, and it will roll back to old logic:
> __apic_accept_irq():
> if (!delivered) {
>              kvm_make_request(KVM_REQ_EVENT, vcpu);
>              kvm_vcpu_kick(vcpu);
> }
> This can solve the problem you mentioned above. Right?

Should not be doing this kick in the first place. One result of it is
that you'll always take a VM-exit if a second injection happens while a VCPU
has not handled the first one.

What is the drawback of the suggestion to unconditionally clear PIR
notification bit on VM-entry?

We can avoid it, but lets get it correct first  (BTW can stick a comment
saying FIXME: optimize) on that PIR clearing.

> > cpu0
> > 
> > check pir(pass)
> > check irr(pass)
> > injected = !test_and_set_bit(pir)
> > 
> > versus
> > 
> > cpu1
> >xchg(pir)

> > 
> > No information can be lost because all accesses to shared data are
> > atomic.
> Sorry, I cannot get your point. Why 'xchg(pir)' can solve the problem I mentioned above? Can you elaborate it?
> The spinlock I used is trying to protect the whole procedure of sync pir to irr, not just access pir.

You're right, its the same problem as with the hardware update.