RE: Question about pirq_guest_unmask

RE: Question about pirq_guest_unmask

[Tian, Kevin]

>From: Keir Fraser [mailto:Keir.Fraser@cl.cam.ac.uk]
>Sent: 2006年4月27日 20:19
>On 27 Apr 2006, at 13:16, Tian, Kevin wrote:
>
>> If that's the case, it's possible for one EOI issued early than
>> expected.
>> Does I miss anything important? Is it OK to change it to service
>> desired
>>
>> pirq only?
>
>Yes, that needs doing. Pick a new physdevop number though. If we
>backport the patch to 3.0.2 then we'll need to keep the old scan-all
>physdevop as a fallback and for backward compatibility.
>
>  -- Keir

OK, I'll add one together with moving physdevop to common. :-)

Thanks,
Kevin

Re: Question about pirq_guest_unmask

[Keir Fraser]

On 27 Apr 2006, at 13:16, Tian, Kevin wrote:

> If that's the case, it's possible for one EOI issued early than
> expected.
> Does I miss anything important? Is it OK to change it to service 
> desired
>
> pirq only?

Yes, that needs doing. Pick a new physdevop number though. If we 
backport the patch to 3.0.2 then we'll need to keep the old scan-all 
physdevop as a fallback and for backward compatibility.

  -- Keir

Question about pirq_guest_unmask

[Tian, Kevin]

Hi, Keir,
	Though the logic to handle pirq has been enhanced recently, 
there's one point never changed which seems questionable to me. 
Why does pirq_guest_unmask need to scan all pirq_masks instead 
of the one that guest is exactly requesting? Does it aim to accelerate 
EOI when another vcpu is sitting between unmask_evtchn and 
pirq_unmask_notify?

	There seems to be a race condition by current logic:

__do_IRQ_guest:
	...
	if ( (action->ack_type != ACKTYPE_NONE) &&
             !test_and_set_bit(irq, &d->pirq_mask) )
            action->in_flight++;
    send_guest_pirq(d, irq);
<- At this point, the pirq_mask is set, while the evtchn_mask is likely 
to be cleared. Evtchn_mask will be set later when target vcpu is 
scheduled and ack_pirq is issued.

While in pirq_guest_unmask:
	...
	For all bits set in pirq_mask:
		if ( !test_bit(d->pirq_to_evtchn[pirq],
&s->evtchn_mask[0]) &&
             test_and_clear_bit(pirq, &d->pirq_mask) )
	...

Above logic in pirq_guest_unmask tempts to consider clearing related 
event mask as an indicator for subsequent pirq unmask request. 
However it's possible for above checking happening right between 
send_guest_irq and ack_pirq for another pirq injection.

If that's the case, it's possible for one EOI issued early than
expected. 
Does I miss anything important? Is it OK to change it to service desired

pirq only?

Thanks,
Kevin