From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.2.1) Gecko/20030225
Description of problem:
local_save_flags(flags) is used to obtain the processor eflags register on x86_64. On a bare metal kernel, each of the bits in the flags register has some significance. It's not uncommon for code to test this flags value for the X86_EFLAGS_IF bit, which indicates whether or not interrupts are currently enabled on the local cpu. If that bit is set, then interrupts are enabled.
unsigned long flags;
if (flags & X86_EFLAGS_IF)
do_A(); // because interrupts are enabled
do_B(); // because they aren't.
Under a xen kernel (paravirt), local_save_flags(flags) loads the flags variable with the event channel mask instead (see __raw_local_save_flags). And this value is 1 when interrupts are masked, 0 when they are enabled. The result of this is that the code above will make the opposite decision under a paravirt kernel than from the bare metal kernel.
Doesn't seem like it would be too hard to cobble up a simulated flags register under the paravirt kernel. The end of __raw_local_save_flags could go like this:
if (_vcpu->evtchn_upcall_mask == 0)
That would at least allow code like the above example to run in the same way. A couple of other xen-kernel functions would have to be changed in a similar fashion to make that work.
FWIW, most of the paravirt kernel has been purged of code that would get tripped up by this. But what about 3rd party kernel modules?
Version-Release number of selected component (if applicable):
Steps to Reproduce:
1. Run the code mentioned above in both the bare-metal and paravirt kernels
The code in a paravirt kernel believes interrupts are always disabled
Be nice if the sample code behaved the same in bare-metal and paravirt kernels.
Nice find. I agree that this inconsistency probably should be fixed.
However, this is not a place where we can deviate from upstream easily. The
change you suggest really needs to be merged in the upstream Xen tree before
we can take it in RHEL.
It might be better to have an abstraction like "is_interrupt_enabled()"; that
way the number returned by "local_save_flags()" would be opaque to the caller.
Eduardo points out that we already have "irqs_disabled()", which is modified
appropriately for Xen. Would that work for you, Kimball?
Cool. Yes it does, and I'll use that. I can do this because I've stumbled into
the problem head-first and can now change my code. But what about all the other
3rd party modules that may stumble into this? How do we give them a warning to
avoid the X86_EFLAGS_IF bit?
Hm, we could do something where we #define X86_EFLAGS_IF bit to a piece of
invalid code in the Xen case, so that compilation would fail. That might be a
bit heavy handed, though. I think we would need to consult upstream about
something like that.
change QA contact
Kimball is no longer the onsite engineer, please reopen (Simon) if you'd like to
pick this up. Neither of these bug btw have been prioritized correctly.