[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] Re: [Xen-devel] Xen crash on dom0 shutdown
On 24/9/08 10:13, "Keir Fraser" <keir.fraser@xxxxxxxxxxxxx> wrote: >> How about this one? It doesn't exactly follow the path you suggested, >> i.e. doesn't mess with event channels, but rather marks the >> irq<->vector mapping as invalid, allowing only a subsequent event >> channel unbind (i.e. close) to recover from that state (which seems >> better in terms of requiring proper discipline in the guest, as it >> prevents re-using the irq or vector without properly cleaning up). > > Yeah, this is the kind of thing I had in mind. I will work on this a bit more > (e.g., need to synchronise on d->evtchn_lock to avoid racing > EVTCHNOP_bind_pirq; also I'm afraid about leaking MSI vectors on domain > destruction). Thanks. Changeset 18539. I made the locking quite a bit stricter, by getting rid of 'irq_lock' and instead using 'evtchn_lock' (which may be better renamed now to 'event_lock' since it isn't protecting just event channels now). Now the pirq-to-vector mapping is protected by both evtchn_lock and irq_desc->lock. A user of the mapping can protect themselves with either lock (whichever is most convenient). Some TODOs: * There is no management of MSI vectors. They always leak! I didn't fix that here since it isn't a mere synchronisation race; the code just isn't there. * 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. ;-) Comments? -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |