Skip to content

Commit 862ab9c

Browse files
author
Francesco Lavra
committed
xen: Fix dispatching of event handlers on multi-CPU platforms
The Xen interrupt handler is invoking the handlers of all pending events whose port numbers belong to the same `evtchn_pending` array element as the element for which there is a pending event for the current vCPU. This means that the handler for an event bound to a given vCPU can potentially be executed on a different vCPU. This creates issues in event handlers (such as `xennet_event_handler()` that assume they are always run sequentially and cannot run in parallel on different vCPUs. Fix the Xen interrupt handler so that it only invokes handlers for events bound to the current vCPU. The issue with seemingly spurious interrupts mentioned in a comment in the interrupt handler function cannot be reproduced (tested on t2.nano, t2.medium and t2.2xlarge AWS instances), thus the `evtchn_mask` array is no longer used. In addition, allow device drivers to request that their event handler is bound to a specific vCPU (via the newly defined xen_bind_evtchn() function); this allows spreading the interrupt load among different vCPUs.
1 parent 8ecff5a commit 862ab9c

4 files changed

Lines changed: 55 additions & 22 deletions

File tree

src/xen/xen.c

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ typedef struct xen_platform_info {
4141
evtchn_port_t xenstore_evtchn;
4242
struct spinlock xenstore_lock;
4343

44+
u64 vcpu_evtchn_enable[XEN_LEGACY_MAX_VCPUS][sizeof(u64) * 8];
4445
struct vector evtchn_handlers;
4546

4647
closure_struct(xenstore_watch_handler, watch_handler);
@@ -87,6 +88,7 @@ closure_function(0, 0, void, xen_interrupt)
8788
int vcpu = current_cpu()->id;
8889
volatile struct shared_info *si = xen_info->shared_info;
8990
volatile struct vcpu_info *vci = &si->vcpu_info[vcpu];
91+
u64 *evtchn_enable = xen_info->vcpu_evtchn_enable[vcpu];
9092

9193
xenint_debug("xen_interrupt enter");
9294
assert(vci->evtchn_upcall_pending);
@@ -97,24 +99,16 @@ closure_function(0, 0, void, xen_interrupt)
9799
/* this may not process in the right order, or it might not matter - care later */
98100
bitmap_word_foreach_set(l1_pending, bit1, i1, 0) {
99101
(void)i1;
100-
/* TODO: any per-cpu event mask would be also applied here... */
101-
u64 l2_pending = si->evtchn_pending[bit1] & ~si->evtchn_mask[bit1];
102+
u64 l2_pending = si->evtchn_pending[bit1] & evtchn_enable[bit1];
102103
xenint_debug("pending 0x%lx, mask 0x%lx, masked 0x%lx",
103-
si->evtchn_pending[bit1], si->evtchn_mask[bit1], l2_pending);
104+
si->evtchn_pending[bit1], ~evtchn_enable[bit1], l2_pending);
104105
__sync_and_and_fetch(&si->evtchn_pending[bit1], ~l2_pending);
105106
u64 l2_offset = bit1 << 6;
106107
bitmap_word_foreach_set(l2_pending, bit2, i2, l2_offset) {
107108
(void)i2;
108109
xenint_debug(" int %d pending", i2);
109110
thunk handler = vector_get(&xen_info->evtchn_handlers, i2);
110-
if (handler) {
111-
xenint_debug(" evtchn %d: applying handler %p", i2, handler);
112-
apply(handler);
113-
} else {
114-
/* XXX we have an issue with seemingly spurious interrupts at evtchn >= 2048... */
115-
xenint_debug(" evtchn %d: spurious interrupt", i2);
116-
si->evtchn_mask[bit1] |= U64_FROM_BIT(bit2);
117-
}
111+
apply(handler);
118112
}
119113
}
120114
vci->evtchn_upcall_mask = 0;
@@ -127,17 +121,30 @@ void xen_register_evtchn_handler(evtchn_port_t evtchn, thunk handler)
127121
assert(vector_set(&xen_info->evtchn_handlers, evtchn, handler));
128122
}
129123

130-
int xen_unmask_evtchn(evtchn_port_t evtchn)
124+
int xen_bind_evtchn(evtchn_port_t evtchn, int vcpu)
125+
{
126+
evtchn_op_t eop;
127+
eop.cmd = EVTCHNOP_bind_vcpu;
128+
eop.u.bind_vcpu.port = evtchn;
129+
eop.u.bind_vcpu.vcpu = vcpu;
130+
return HYPERVISOR_event_channel_op(&eop);
131+
}
132+
133+
int xen_unmask_evtchn(evtchn_port_t evtchn, int vcpu)
131134
{
132135
assert(evtchn > 0 && evtchn < EVTCHN_2L_NR_CHANNELS);
133136
evtchn_op_t eop;
134137
eop.cmd = EVTCHNOP_unmask;
135138
eop.u.unmask.port = evtchn;
136-
return HYPERVISOR_event_channel_op(&eop);
139+
int rv = HYPERVISOR_event_channel_op(&eop);
140+
if (rv == 0)
141+
atomic_set_bit(&xen_info->vcpu_evtchn_enable[vcpu][evtchn >> 6], evtchn & MASK(6));
142+
return rv;
137143
}
138144

139-
int xen_close_evtchn(evtchn_port_t evtchn)
145+
int xen_close_evtchn(evtchn_port_t evtchn, int vcpu)
140146
{
147+
atomic_clear_bit(&xen_info->vcpu_evtchn_enable[vcpu][evtchn >> 6], evtchn & MASK(6));
141148
vector_set(&xen_info->evtchn_handlers, evtchn, 0);
142149
evtchn_op_t eop;
143150
eop.cmd = EVTCHNOP_close;
@@ -357,7 +364,7 @@ static int xen_setup_vcpu(int vcpu, u64 shared_info_phys)
357364
xen_debug("cpu %d timer event channel %d", vcpu, timer_evtchn);
358365
xen_register_evtchn_handler(timer_evtchn,
359366
closure_func(xen_info->h, thunk, xen_runloop_timer_handler));
360-
assert(xen_unmask_evtchn(timer_evtchn) == 0);
367+
assert(xen_unmask_evtchn(timer_evtchn, vcpu) == 0);
361368
return 0;
362369
}
363370

@@ -540,7 +547,7 @@ boolean xen_detect(kernel_heaps kh)
540547
spin_lock_init(&xen_info->xenstore_lock);
541548
xen_register_evtchn_handler(xen_info->xenstore_evtchn,
542549
closure_func(h, thunk, xenstore_evtchn_handler));
543-
assert(xen_unmask_evtchn(xen_info->xenstore_evtchn) == 0);
550+
assert(xen_unmask_evtchn(xen_info->xenstore_evtchn, 0) == 0);
544551

545552
if (!xen_grant_init(kh, features)) {
546553
msg_err("xen: failed to set up grant tables");

src/xen/xen_internal.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,9 @@ typedef struct xen_dev {
3838
status xen_allocate_evtchn(domid_t other_id, evtchn_port_t *evtchn);
3939
void xen_register_evtchn_handler(evtchn_port_t evtchn, thunk handler);
4040
int xen_notify_evtchn(evtchn_port_t evtchn);
41-
int xen_unmask_evtchn(evtchn_port_t evtchn);
42-
int xen_close_evtchn(evtchn_port_t evtchn);
41+
int xen_bind_evtchn(evtchn_port_t evtchn, int vcpu);
42+
int xen_unmask_evtchn(evtchn_port_t evtchn, int vcpu);
43+
int xen_close_evtchn(evtchn_port_t evtchn, int vcpu);
4344

4445
grant_ref_t xen_grant_page_access(u16 domid, u64 phys, boolean readonly);
4546
void xen_revoke_page_access(grant_ref_t ref);

src/xen/xenblk.c

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ struct xenblk_dev {
3939
blkif_front_ring_t ring;
4040
grant_ref_t ring_gntref;
4141
evtchn_port_t evtchn;
42+
s32 evtchn_cpu;
4243
closure_struct(xenblk_io, read);
4344
closure_struct(xenblk_io, write);
4445
closure_struct(storage_simple_req_handler, req_handler);
@@ -275,7 +276,12 @@ static void xenblk_remove(xenblk_dev xbd)
275276
}
276277
xen_driver_unbind(xbd->meta);
277278
xenbus_set_state(0, xbd->dev.frontend, XenbusStateClosed);
278-
xen_close_evtchn(xbd->evtchn);
279+
s32 evtchn_cpu = xbd->evtchn_cpu;
280+
if (evtchn_cpu >= 0)
281+
irq_put_target_cpu(evtchn_cpu);
282+
else
283+
evtchn_cpu = 0;
284+
xen_close_evtchn(xbd->evtchn, evtchn_cpu);
279285
xen_revoke_page_access(xbd->ring_gntref);
280286
deallocate(xbd->contiguous, xbd->ring.sring, PAGESIZE);
281287
deallocate_vector(xbd->rreqs);
@@ -336,7 +342,18 @@ closure_func_basic(thunk, void, xenblk_watch_service)
336342
timm_dealloc(s);
337343
goto remove;
338344
}
339-
int rv = xen_unmask_evtchn(xbd->evtchn);
345+
u32 evtchn_cpu = irq_get_target_cpu(irange(0, 0));
346+
int rv = xen_bind_evtchn(xbd->evtchn, evtchn_cpu);
347+
if (rv < 0) {
348+
msg_warn("%s: failed to bind event channel %d to vCPU %d: rv %d", func_ss, xbd->evtchn,
349+
evtchn_cpu, rv);
350+
irq_put_target_cpu(evtchn_cpu);
351+
xbd->evtchn_cpu = -1;
352+
evtchn_cpu = 0;
353+
} else {
354+
xbd->evtchn_cpu = evtchn_cpu;
355+
}
356+
rv = xen_unmask_evtchn(xbd->evtchn, evtchn_cpu);
340357
if (rv < 0) {
341358
msg_err("%s: failed to unmask event channel %d: rv %d", func_ss, xbd->evtchn, rv);
342359
goto remove;
@@ -472,7 +489,7 @@ static status xenblk_enable(xenblk_dev xbd)
472489
}
473490
return STATUS_OK;
474491
out_evtchn:
475-
xen_close_evtchn(xbd->evtchn);
492+
xen_close_evtchn(xbd->evtchn, 0);
476493
out_revoke:
477494
xen_revoke_page_access(xbd->ring_gntref);
478495
out_dealloc:

src/xen/xennet.c

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -724,7 +724,15 @@ static status xennet_enable(xennet_dev xd)
724724
/* we're kind of always up ... start rx now */
725725
xd->rx_ring.sring->rsp_event = xd->rx_ring.rsp_cons + 1;
726726
write_barrier();
727-
int rv = xen_unmask_evtchn(xd->evtchn);
727+
u32 evtchn_cpu = irq_get_target_cpu(irange(0, 0));
728+
int rv = xen_bind_evtchn(xd->evtchn, evtchn_cpu);
729+
if (rv < 0) {
730+
msg_warn("%s: failed to bind event channel %d to vCPU %d: rv %d", func_ss, xd->evtchn,
731+
evtchn_cpu, rv);
732+
irq_put_target_cpu(evtchn_cpu);
733+
evtchn_cpu = 0;
734+
}
735+
rv = xen_unmask_evtchn(xd->evtchn, evtchn_cpu);
728736
if (rv < 0) {
729737
s = timm("result", "failed to unmask event channel %d: rv %d", xd->evtchn, rv);
730738
goto out_dealloc_rx_buffers;

0 commit comments

Comments
 (0)