Bug 241269 - local_save_flags behaves differently from bare-metal kernel
Summary: local_save_flags behaves differently from bare-metal kernel
Alias: None
Product: Red Hat Enterprise Linux 5
Classification: Red Hat
Component: kernel-xen   
(Show other bugs)
Version: 5.0
Hardware: x86_64 Linux
Target Milestone: ---
: ---
Assignee: Kimball Murray
QA Contact: Martin Jenner
Depends On:
TreeView+ depends on / blocked
Reported: 2007-05-24 17:57 UTC by Kimball Murray
Modified: 2008-01-29 19:31 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2008-01-29 19:31:45 UTC
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Attachments (Terms of Use)

Description Kimball Murray 2007-05-24 17:57:52 UTC
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.

Typical code:

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)
   return X86_EFLAGS_IF;
   return 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):

How reproducible:

Steps to Reproduce:
1. Run the code mentioned above in both the bare-metal and paravirt kernels

Actual Results:
The code in a paravirt kernel believes interrupts are always disabled

Expected Results:
Be nice if the sample code behaved the same in bare-metal and paravirt kernels.

Additional info:

Comment 1 Rik van Riel 2007-05-24 18:06:32 UTC
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.

Comment 2 Chris Lalancette 2007-05-24 18:09:22 UTC
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.

Chris Lalancette

Comment 3 Chris Lalancette 2007-05-24 18:24:44 UTC
Eduardo points out that we already have "irqs_disabled()", which is modified
appropriately for Xen.  Would that work for you, Kimball?

Chris Lalancette

Comment 4 Kimball Murray 2007-05-24 18:33:51 UTC
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?

Comment 5 Chris Lalancette 2007-05-24 18:44:46 UTC
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.

Chris Lalancette

Comment 6 Red Hat Bugzilla 2007-07-25 00:47:03 UTC
change QA contact

Comment 7 Andrius Benokraitis 2008-01-29 19:31:45 UTC
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.

Note You need to log in before you can comment on or make changes to this bug.