[Xen-devel] RE: [Xen-changelog] [xen-unstable] x86: Properly synchronise updates to pirq-to-vector mapping.

["Jiang, Yunhong" <yunhong.jiang@xxxxxxxxx>]

Yes, I'm trying to fix this issue.

Yunhong Jiang

Keir Fraser <mailto:keir.fraser@xxxxxxxxxxxxx> wrote:
> On 26/9/08 05:44, "Cui, Dexuan" <dexuan.cui@xxxxxxxxx> wrote:
>
>> @@ -491,16 +512,15 @@ int pirq_guest_bind(struct vcpu *v, int
>>      int                 rc = 0;
>>      cpumask_t           cpumask = CPU_MASK_NONE;
>>
>> +    WARN_ON(!spin_is_locked(&v->domain->evtchn_lock));
>>
>> I find this WARN_ON() is triggered harmlessly when I assign device to HVM
>> guest. The call trace is XEN_DOMCTL_bind_pt_irq ->
>> pt_irq_create_bind_vtd() -> pirq_guest_bind(). Should we remove the
>> WARN_ON here?
>
> I put in that WARN_ON() deliberately, because I think HVM
> pt_irq locking
> needs some work. Yunhong Jiang is looking into it I believe (cc'ed).
> Obviously the current situation is temporary -- either the
> locking in the
> passthrough code will be fixed as I envisage, or we'll agree
> some other fix
> and the WARN_ON() will be removed or changed. Appended is a
> relevant section
> of en email I sent a couple of days ago.
>
> -- Keir
>
> ------------------
>
> * I decided to WARN_ON(!spin_is_locked(&d->evtchn_lock)) in
> pirq_guest_[un]bind(). The reason is that in any case those
> functions do not
> expect to be re-entered -- they really want to be per-domain serialised.
> Furthermore I am pretty certain that the HVM passthrough code is not
> synchronising properly with changes to the pirq-to-vector
> mapping (it uses
> domain_irq_to_vector() in many places with no care for
> locking) nor is it
> synchronised with other users of pirq_guest_bind() --- a reasonable
> semantics here would be that a domain pirq can be bound to
> once, either via
> an event channel, or through a virtual PIC in HVM emulation context. I
> therefore think that careful locking is required -- it may
> suffice to get
> rid of (or at least make less use of) the hvm_domain.irq_lock
> and replace
> its use with evtchn_lock (only consideration is that the latter is not
> IRQ-safe). The WARN_ON() is a nice reminder of work to be done here.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel