Hi,

I've noticed a potential bug in the spllower() implementation for Xen:
the original code saves flags through read_psl(), disable interrupts 
('cli' on x86) and then even run Xspllower() (if pending interrupts) or 
set new level and restore flags (with write_psl()).
On x86, saving and restoring EFLAGS includes restoring the 'IF' flag (if 
it was previously set) cleared by disable_intr(), thus reenabling 
interrupts.
As the 'IF' flag isn't virtualized on Xen, we may end spllower() without 
reenabling interrupts.

A quick (and a bit dirty) fix could be:

mask = HYPERVISOR_shared_info->vcpu_info[cpu_number()].evtchn_upcall_mask;
psl = read_psl();
disable_intr();
if (ci->ci_ipending & imask) {
	Xspllower(nlevel);
	/* Xspllower does enable_intr() */
} else {
	ci->ci_ilevel = nlevel;
	write_psl(psl);
	HYPERVISOR_shared_info->vcpu_info[cpu_number()].evtchn_upcall_mask = mask;
}

Two notes on this:
1/ reading mask and disabling interrupts not atomically shouldn't be an 
issue as any interrupt that may occur between should not change the 
upcall_mask value.
2/ anyway, i'd prefer using a pair of new wrappers like 
xen_block_events() and xen_unblock_events() which would be something like

#define xen_block_events()   \
    atomic_xchg8(                     \
    &HYPERVISOR_shared_info->vcpu_info[cpu_number()].evtchn_upcall_mask, 1)

#define xen_unblock_events()   \
    atomic_xchg8(                     \
    &HYPERVISOR_shared_info->vcpu_info[cpu_number()].evtchn_upcall_mask, 0)

i'm sure whether the save/restore flags trick is used anywhere (i hope 
this is restricted to arch-dependent code), but that may be worth checking.
By the way, has anyone experienced troubles that may be related to this bug?

Cheers,
Mathieu