| Message ID | 20250107101711.5980-4-jgross@suse.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | remove libxenctrl usage from xenstored | expand |
On 07.01.2025 11:17, Juergen Gross wrote: > XEN_DOMCTL_set_virq_handler will happily steal a global virq from the > current domain having bound it and assign it to another domain. The > former domain will just never receive any further events for that > virq without knowing what happened. Yet - see my reply on patch 2 - it may actually have been intentional to be this way, in case the new domain is indeed the designated new handler. Otherwise such a transfer would require more coordination - the original owner would first need to unbind, then signal the completion thereof to the new owner. Jan
On 07.01.25 16:38, Jan Beulich wrote: > On 07.01.2025 11:17, Juergen Gross wrote: >> XEN_DOMCTL_set_virq_handler will happily steal a global virq from the >> current domain having bound it and assign it to another domain. The >> former domain will just never receive any further events for that >> virq without knowing what happened. > > Yet - see my reply on patch 2 - it may actually have been intentional to > be this way, in case the new domain is indeed the designated new handler. > Otherwise such a transfer would require more coordination - the original > owner would first need to unbind, then signal the completion thereof to > the new owner. Yes, like today's handling of xenstored live update. It is the natural way to do that, as that coordination needs to be happening anyway, as I've outlined in my response to your remark on patch 2. Juergen
diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c index 62060dc66b..341221d420 100644 --- a/xen/common/event_channel.c +++ b/xen/common/event_channel.c @@ -988,7 +988,8 @@ void send_global_virq(uint32_t virq) int set_global_virq_handler(struct domain *d, uint32_t virq) { - struct domain *old; + struct domain *old, *hdl; + const struct vcpu *v; int rc = 0; if (virq >= NR_VIRQS) @@ -1012,7 +1013,22 @@ int set_global_virq_handler(struct domain *d, uint32_t virq) else { old = global_virq_handlers[virq]; - global_virq_handlers[virq] = d; + hdl = get_global_virq_handler(virq); + if ( hdl != d ) + { + read_lock(&hdl->event_lock); + + v = hdl->vcpu[0]; + if ( v && read_atomic(&v->virq_to_evtchn[virq]) ) + { + rc = -EBUSY; + old = d; + } + else + global_virq_handlers[virq] = d; + + read_unlock(&hdl->event_lock); + } } spin_unlock(&global_virq_handlers_lock);
XEN_DOMCTL_set_virq_handler will happily steal a global virq from the current domain having bound it and assign it to another domain. The former domain will just never receive any further events for that virq without knowing what happened. Change the behavior to allow XEN_DOMCTL_set_virq_handler only if the virq in question is not bound by the current domain allowed to use it. Currently the only user of XEN_DOMCTL_set_virq_handler in the Xen code base is init-xenstore-domain, so changing the behavior like above will not cause any problems. Signed-off-by: Juergen Gross <jgross@suse.com> --- V6: - new patch --- xen/common/event_channel.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-)