RE: [PATCH 2/2] x86, apicv: Add Posted Interrupt supporting

From: "Zhang, Yang Z" <yang.z.zhang@intel.com>
To: Marcelo Tosatti <mtosatti@redhat.com>
Cc: "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"gleb@redhat.com" <gleb@redhat.com>,
	"Shan, Haitao" <haitao.shan@intel.com>,
	"Zhang, Xiantao" <xiantao.zhang@intel.com>,
	"Nakajima, Jun" <jun.nakajima@intel.com>,
	"Anvin, H Peter" <h.peter.anvin@intel.com>
Subject: RE: [PATCH 2/2] x86, apicv: Add Posted Interrupt supporting
Date: Wed, 23 Jan 2013 05:09:32 +0000	[thread overview]
Message-ID: <A9667DDFB95DB7438FA9D7D576C3D87E314715@SHSMSX101.ccr.corp.intel.com> (raw)
In-Reply-To: <20130122225922.GA19011@amt.cnet>

Marcelo Tosatti wrote on 2013-01-23:
> On Thu, Dec 13, 2012 at 03:29:40PM +0800, Yang Zhang wrote:
>> From: Yang Zhang <yang.z.zhang@Intel.com>
>> 
>> Posted Interrupt allows APIC interrupts to inject into guest directly
>> without any vmexit.
>> 
>> - When delivering a interrupt to guest, if target vcpu is running,
>>   update Posted-interrupt requests bitmap and send a notification event
>>   to the vcpu. Then the vcpu will handle this interrupt automatically,
>>   without any software involvemnt.
>> - If target vcpu is not running or there already a notification event
>>   pending in the vcpu, do nothing. The interrupt will be handled by
>>   next vm entry.
>> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
>> ---
> 
> <snip>
> 
>> +static void pi_handler(void)
>> +{
>> +	;
>> +}
>> +
>> +static int vmx_has_posted_interrupt(struct kvm_vcpu *vcpu)
>> +{
>> +	return irqchip_in_kernel(vcpu->kvm) && enable_apicv_pi;
>> +}
>> +
>> +static int vmx_send_nv(struct kvm_vcpu *vcpu,
>> +		int vector)
>> +{
>> +	struct vcpu_vmx *vmx = to_vmx(vcpu);
>> +
>> +	pi_set_pir(vector, vmx->pi);
> 
> Section 29.6 "Posted interrupt processing":
> 
> "No other agent can read or write a PIR bit (or groups of bits) between
> the time it is read (to determine what to OR into VIRR) and when it is
> cleared".
This means hardware can ensure the read and clear operation is atomic, for example, use locked cmpxchg.

>> +	if (!pi_test_and_set_on(vmx->pi) && (vcpu->mode == IN_GUEST_MODE)) {
>> +		apic->send_IPI_mask(get_cpu_mask(vcpu->cpu), POSTED_INTR_VECTOR);
>> +		return 1; +	} +	return 0;
> 
> What is the purpose of outstanding-notification bit? At first, its use as a
> "lock" for PIR posted-interrupt bits is limited because its cleared
> on step 3. before PIR is cleared. If it were cleared after step 5. then
> software could
> 
> 	if (!pi_test_and_set_on(vmx->pi)) { 		pi_set_pir(vector, vmx->pi);
> 		apic->send_IPI_mask(get_cpu_mask(vcpu->cpu), POSTED_INTR_VECTOR); 	}
> 
> Does this mean software has to read PIR _and_ outstanding notification
> bit to know when its possible to set bits in PIR + send IPI?
There is no limitation for software to set bits in PIR. Software can set PIR unconditionally with locked operation.
Software must to read ON bit to check whether the IPI is needed. If ON bit is set, this means an notification event already sent but not acked by target cpu and no need to resend it again.

> 
> Or is it really cleared after step 5?
No, it is in step 3.


Best regards,
Yang