Re: [PATCH] use EFLAGS #defines instead of inline constants

From: Randy.Dunlap
Date: Tue Apr 06 2004 - 23:58:34 EST

| From: Brian Gerst
|
| Randy.Dunlap wrote:
| > // linux-2.6.5
| > // use x86 EFLAGS defines in place of inline constants;
| >
| > diffstat:=
| > arch/i386/kernel/doublefault.c | 3 ++-
| > arch/i386/kernel/process.c | 2 +-
| > arch/i386/kernel/signal.c | 7 ++++++-
| > 3 files changed, 9 insertions(+), 3 deletions(-)
|
...
|
| kernel_thread() doesn't really needs PF and SF. They are just result
| flags that nothing should depend on. Some additional suggestions are
| cleanup the vm86 code to use the defines from processor.h instead of its
| own, and moving FIX_EFLAGS to processor.h (rename it X86_EFLAGS_USER or
| something).

Hi,

Thanks for the comments. Here's an updated patch.
More comments?

--
~Randy


// linux-2.6.5
// use x86 EFLAGS register defines instead of inline constants;

diffstat:=
arch/i386/kernel/doublefault.c | 3 +-
arch/i386/kernel/ioport.c | 6 +++--
arch/i386/kernel/process.c | 2 -
arch/i386/kernel/signal.c | 10 +++++----
arch/i386/kernel/traps.c | 12 +++++------
arch/i386/kernel/vm86.c | 43 +++++++++++++++++++++++------------------
arch/i386/mm/fault.c | 7 +++---
include/asm-i386/processor.h | 4 +++
include/asm-i386/ptrace.h | 2 -
include/asm-i386/vm86.h | 12 -----------
10 files changed, 54 insertions(+), 47 deletions(-)


diff -Naurp ./arch/i386/kernel/doublefault.c~use_eflags ./arch/i386/kernel/doublefault.c
--- ./arch/i386/kernel/doublefault.c~use_eflags 2004-03-10 18:55:22.000000000 -0800
+++ ./arch/i386/kernel/doublefault.c 2004-04-01 21:10:04.000000000 -0800
@@ -6,6 +6,7 @@

#include <asm/uaccess.h>
#include <asm/pgtable.h>
+#include <asm/processor.h>
#include <asm/desc.h>

#define DOUBLEFAULT_STACKSIZE (1024)
@@ -53,7 +54,7 @@ struct tss_struct doublefault_tss __cach
.io_bitmap_base = INVALID_IO_BITMAP_OFFSET,

.eip = (unsigned long) doublefault_fn,
- .eflags = 0x00000082,
+ .eflags = X86_EFLAGS_SF | 0x2, /* 0x2 bit is always set */
.esp = STACK_START,
.es = __USER_DS,
.cs = __KERNEL_CS,
diff -Naurp ./arch/i386/kernel/process.c~use_eflags ./arch/i386/kernel/process.c
--- ./arch/i386/kernel/process.c~use_eflags 2004-03-30 17:41:30.000000000 -0800
+++ ./arch/i386/kernel/process.c 2004-04-06 21:23:05.000000000 -0700
@@ -278,7 +278,7 @@ int kernel_thread(int (*fn)(void *), voi
regs.orig_eax = -1;
regs.eip = (unsigned long) kernel_thread_helper;
regs.xcs = __KERNEL_CS;
- regs.eflags = 0x286;
+ regs.eflags = X86_EFLAGS_IF | 0x2;

/* Ok, create the new process.. */
return do_fork(flags | CLONE_VM | CLONE_UNTRACED, 0, &regs, 0, NULL, NULL);
diff -Naurp ./arch/i386/kernel/signal.c~use_eflags ./arch/i386/kernel/signal.c
--- ./arch/i386/kernel/signal.c~use_eflags 2004-03-10 18:55:26.000000000 -0800
+++ ./arch/i386/kernel/signal.c 2004-04-06 22:08:36.000000000 -0700
@@ -20,6 +20,7 @@
#include <linux/personality.h>
#include <linux/suspend.h>
#include <linux/elf.h>
+#include <asm/processor.h>
#include <asm/ucontext.h>
#include <asm/uaccess.h>
#include <asm/i387.h>
@@ -170,7 +171,8 @@ restore_sigcontext(struct pt_regs *regs,
{
unsigned int tmpflags;
err |= __get_user(tmpflags, &sc->eflags);
- regs->eflags = (regs->eflags & ~0x40DD5) | (tmpflags & 0x40DD5);
+ regs->eflags = (regs->eflags & ~X86_EFLAGS_USER) |
+ (tmpflags & X86_EFLAGS_USER);
regs->orig_eax = -1; /* disable syscall checks */
}

@@ -401,7 +403,7 @@ static void setup_frame(int sig, struct
regs->xes = __USER_DS;
regs->xss = __USER_DS;
regs->xcs = __USER_CS;
- regs->eflags &= ~TF_MASK;
+ regs->eflags &= ~X86_EFLAGS_TF;

#if DEBUG_SIG
printk("SIG deliver (%s:%d): sp=%p pc=%p ra=%p\n",
@@ -482,7 +484,7 @@ static void setup_rt_frame(int sig, stru
regs->xes = __USER_DS;
regs->xss = __USER_DS;
regs->xcs = __USER_CS;
- regs->eflags &= ~TF_MASK;
+ regs->eflags &= ~X86_EFLAGS_TF;

#if DEBUG_SIG
printk("SIG deliver (%s:%d): sp=%p pc=%p ra=%p\n",
@@ -615,7 +617,7 @@ void do_notify_resume(struct pt_regs *re
{
/* Pending single-step? */
if (thread_info_flags & _TIF_SINGLESTEP) {
- regs->eflags |= TF_MASK;
+ regs->eflags |= X86_EFLAGS_TF;
clear_thread_flag(TIF_SINGLESTEP);
}
/* deal with pending signal delivery */
diff -Naurp ./arch/i386/kernel/vm86.c~use_eflags ./arch/i386/kernel/vm86.c
--- ./arch/i386/kernel/vm86.c~use_eflags 2004-03-10 18:55:22.000000000 -0800
+++ ./arch/i386/kernel/vm86.c 2004-04-06 22:16:06.000000000 -0700
@@ -45,6 +45,7 @@

#include <asm/uaccess.h>
#include <asm/pgalloc.h>
+#include <asm/processor.h>
#include <asm/io.h>
#include <asm/tlbflush.h>
#include <asm/irq.h>
@@ -86,8 +87,12 @@
#define set_flags(X,new,mask) \
((X) = ((X) & ~(mask)) | ((new) & (mask)))

-#define SAFE_MASK (0xDD5)
-#define RETURN_MASK (0xDFF)
+#define SAFE_MASK (X86_EFLAGS_OF | X86_EFLAGS_DF | X86_EFLAGS_TF | \
+ X86_EFLAGS_SF | X86_EFLAGS_ZF | X86_EFLAGS_AF | \
+ X86_EFLAGS_PF | X86_EFLAGS_CF)
+#define RETURN_MASK (X86_EFLAGS_OF | X86_EFLAGS_DF | X86_EFLAGS_TF | \
+ X86_EFLAGS_SF | X86_EFLAGS_ZF | 0x20 | X86_EFLAGS_AF | \
+ 0x8 | X86_EFLAGS_PF | 0x2 | X86_EFLAGS_CF)

#define VM86_REGS_PART2 orig_eax
#define VM86_REGS_SIZE1 \
@@ -112,7 +117,7 @@ struct pt_regs * fastcall save_v86_state
printk("no vm86_info: BAD\n");
do_exit(SIGSEGV);
}
- set_flags(regs->eflags, VEFLAGS, VIF_MASK | current->thread.v86mask);
+ set_flags(regs->eflags, VEFLAGS, X86_EFLAGS_VIF | current->thread.v86mask);
tmp = copy_to_user(&current->thread.vm86_info->regs,regs, VM86_REGS_SIZE1);
tmp += copy_to_user(&current->thread.vm86_info->regs.VM86_REGS_PART2,
&regs->VM86_REGS_PART2, VM86_REGS_SIZE2);
@@ -276,20 +281,22 @@ static void do_sys_vm86(struct kernel_vm
VEFLAGS = info->regs.eflags;
info->regs.eflags &= SAFE_MASK;
info->regs.eflags |= info->regs32->eflags & ~SAFE_MASK;
- info->regs.eflags |= VM_MASK;
+ info->regs.eflags |= X86_EFLAGS_VM;

switch (info->cpu_type) {
case CPU_286:
tsk->thread.v86mask = 0;
break;
case CPU_386:
- tsk->thread.v86mask = NT_MASK | IOPL_MASK;
+ tsk->thread.v86mask = X86_EFLAGS_NT | X86_EFLAGS_IOPL;
break;
case CPU_486:
- tsk->thread.v86mask = AC_MASK | NT_MASK | IOPL_MASK;
+ tsk->thread.v86mask = X86_EFLAGS_AC |
+ X86_EFLAGS_NT | X86_EFLAGS_IOPL;
break;
default:
- tsk->thread.v86mask = ID_MASK | AC_MASK | NT_MASK | IOPL_MASK;
+ tsk->thread.v86mask = X86_EFLAGS_ID |X86_EFLAGS_AC |
+ X86_EFLAGS_NT | X86_EFLAGS_IOPL;
break;
}

@@ -335,24 +342,24 @@ static inline void return_to_32bit(struc

static inline void set_IF(struct kernel_vm86_regs * regs)
{
- VEFLAGS |= VIF_MASK;
- if (VEFLAGS & VIP_MASK)
+ VEFLAGS |= X86_EFLAGS_VIF;
+ if (VEFLAGS & X86_EFLAGS_VIP)
return_to_32bit(regs, VM86_STI);
}

static inline void clear_IF(struct kernel_vm86_regs * regs)
{
- VEFLAGS &= ~VIF_MASK;
+ VEFLAGS &= ~X86_EFLAGS_VIF;
}

static inline void clear_TF(struct kernel_vm86_regs * regs)
{
- regs->eflags &= ~TF_MASK;
+ regs->eflags &= ~X86_EFLAGS_TF;
}

static inline void clear_AC(struct kernel_vm86_regs * regs)
{
- regs->eflags &= ~AC_MASK;
+ regs->eflags &= ~X86_EFLAGS_AC;
}

/* It is correct to call set_IF(regs) from the set_vflags_*
@@ -370,7 +377,7 @@ static inline void set_vflags_long(unsig
{
set_flags(VEFLAGS, eflags, current->thread.v86mask);
set_flags(regs->eflags, eflags, SAFE_MASK);
- if (eflags & IF_MASK)
+ if (eflags & X86_EFLAGS_IF)
set_IF(regs);
else
clear_IF(regs);
@@ -380,7 +387,7 @@ static inline void set_vflags_short(unsi
{
set_flags(VFLAGS, flags, current->thread.v86mask);
set_flags(regs->eflags, flags, SAFE_MASK);
- if (flags & IF_MASK)
+ if (flags & X86_EFLAGS_IF)
set_IF(regs);
else
clear_IF(regs);
@@ -390,8 +397,8 @@ static inline unsigned long get_vflags(s
{
unsigned long flags = regs->eflags & RETURN_MASK;

- if (VEFLAGS & VIF_MASK)
- flags |= IF_MASK;
+ if (VEFLAGS & X86_EFLAGS_VIF)
+ flags |= X86_EFLAGS_IF;
return flags | (VEFLAGS & current->thread.v86mask);
}

@@ -547,9 +554,9 @@ void handle_vm86_fault(struct kernel_vm8

#define CHECK_IF_IN_TRAP \
if (VMPI.vm86dbg_active && VMPI.vm86dbg_TFpendig) \
- newflags |= TF_MASK
+ newflags |= X86_EFLAGS_TF
#define VM86_FAULT_RETURN do { \
- if (VMPI.force_return_for_pic && (VEFLAGS & (IF_MASK | VIF_MASK))) \
+ if (VMPI.force_return_for_pic && (VEFLAGS & (X86_EFLAGS_IF | X86_EFLAGS_VIF))) \
return_to_32bit(regs, VM86_PICRETURN); \
return; } while (0)

diff -Naurp ./arch/i386/kernel/traps.c~use_eflags ./arch/i386/kernel/traps.c
--- ./arch/i386/kernel/traps.c~use_eflags 2004-03-30 17:41:30.000000000 -0800
+++ ./arch/i386/kernel/traps.c 2004-04-06 21:38:29.000000000 -0700
@@ -295,7 +295,7 @@ void die(const char * str, struct pt_reg

static inline void die_if_kernel(const char * str, struct pt_regs * regs, long err)
{
- if (!(regs->eflags & VM_MASK) && !(3 & regs->xcs))
+ if (!(regs->eflags & X86_EFLAGS_VM) && !(3 & regs->xcs))
die(str, regs, err);
}

@@ -311,7 +311,7 @@ static inline unsigned long get_cr2(void
static inline void do_trap(int trapnr, int signr, char *str, int vm86,
struct pt_regs * regs, long error_code, siginfo_t *info)
{
- if (regs->eflags & VM_MASK) {
+ if (regs->eflags & X86_EFLAGS_VM) {
if (vm86)
goto vm86_trap;
goto trap_signal;
@@ -394,7 +394,7 @@ asmlinkage void do_general_protection(st
if (regs->eflags & X86_EFLAGS_IF)
local_irq_enable();

- if (regs->eflags & VM_MASK)
+ if (regs->eflags & X86_EFLAGS_VM)
goto gp_in_vm86;

if (!(regs->xcs & 3))
@@ -557,7 +557,7 @@ asmlinkage void do_debug(struct pt_regs
goto clear_dr7;
}

- if (regs->eflags & VM_MASK)
+ if (regs->eflags & X86_EFLAGS_VM)
goto debug_vm86;

/* Save debug status register where ptrace can see it */
@@ -610,7 +610,7 @@ debug_vm86:
clear_TF_reenable:
set_tsk_thread_flag(tsk, TIF_SINGLESTEP);
clear_TF:
- regs->eflags &= ~TF_MASK;
+ regs->eflags &= ~X86_EFLAGS_TF;
return;
}

@@ -741,7 +741,7 @@ asmlinkage void do_simd_coprocessor_erro
* Handle strange cache flush from user space exception
* in all other cases. This is undocumented behaviour.
*/
- if (regs->eflags & VM_MASK) {
+ if (regs->eflags & X86_EFLAGS_VM) {
handle_vm86_fault((struct kernel_vm86_regs *)regs,
error_code);
return;
diff -Naurp ./arch/i386/kernel/ioport.c~use_eflags ./arch/i386/kernel/ioport.c
--- ./arch/i386/kernel/ioport.c~use_eflags 2004-03-10 18:55:27.000000000 -0800
+++ ./arch/i386/kernel/ioport.c 2004-04-06 21:54:52.000000000 -0700
@@ -16,6 +16,8 @@
#include <linux/slab.h>
#include <linux/thread_info.h>

+#include <asm/processor.h>
+
/* Set EXTENT bits starting at BASE in BITMAP to value TURN_ON. */
static void set_bitmap(unsigned long *bitmap, unsigned int base, unsigned int extent, int new_value)
{
@@ -109,7 +111,7 @@ asmlinkage long sys_iopl(unsigned long u
{
volatile struct pt_regs * regs = (struct pt_regs *) &unused;
unsigned int level = regs->ebx;
- unsigned int old = (regs->eflags >> 12) & 3;
+ unsigned int old = (regs->eflags & X86_EFLAGS_IOPL) >> 12;

if (level > 3)
return -EINVAL;
@@ -118,7 +120,7 @@ asmlinkage long sys_iopl(unsigned long u
if (!capable(CAP_SYS_RAWIO))
return -EPERM;
}
- regs->eflags = (regs->eflags &~ 0x3000UL) | (level << 12);
+ regs->eflags = (regs->eflags & ~X86_EFLAGS_IOPL) | (level << 12);
/* Make sure we return the long way (not sysenter) */
set_thread_flag(TIF_IRET);
return 0;
diff -Naurp ./arch/i386/mm/fault.c~use_eflags ./arch/i386/mm/fault.c
--- ./arch/i386/mm/fault.c~use_eflags 2004-03-10 18:55:20.000000000 -0800
+++ ./arch/i386/mm/fault.c 2004-04-06 21:58:12.000000000 -0700
@@ -25,6 +25,7 @@
#include <asm/system.h>
#include <asm/uaccess.h>
#include <asm/pgalloc.h>
+#include <asm/processor.h>
#include <asm/hardirq.h>
#include <asm/desc.h>

@@ -81,7 +82,7 @@ static inline unsigned long get_segment_
*eip_limit = (seg & 3) ? USER_DS.seg : KERNEL_DS.seg;

/* Unlikely, but must come before segment checks. */
- if (unlikely((regs->eflags & VM_MASK) != 0))
+ if (unlikely((regs->eflags & X86_EFLAGS_VM) != 0))
return eip + (seg << 4);

/* By far the most common cases. */
@@ -223,7 +224,7 @@ asmlinkage void do_page_fault(struct pt_
__asm__("movl %%cr2,%0":"=r" (address));

/* It's safe to allow irq's after cr2 has been saved */
- if (regs->eflags & (X86_EFLAGS_IF|VM_MASK))
+ if (regs->eflags & (X86_EFLAGS_IF | X86_EFLAGS_VM))
local_irq_enable();

tsk = current;
@@ -333,7 +334,7 @@ good_area:
/*
* Did it hit the DOS screen memory VA from vm86 mode?
*/
- if (regs->eflags & VM_MASK) {
+ if (regs->eflags & X86_EFLAGS_VM) {
unsigned long bit = (address - 0xA0000) >> PAGE_SHIFT;
if (bit < 32)
tsk->thread.screen_bitmap |= 1 << bit;
diff -Naurp ./include/asm-i386/ptrace.h~use_eflags ./include/asm-i386/ptrace.h
--- ./include/asm-i386/ptrace.h~use_eflags 2004-03-10 18:55:22.000000000 -0800
+++ ./include/asm-i386/ptrace.h 2004-04-06 22:19:10.000000000 -0700
@@ -55,7 +55,7 @@ struct pt_regs {
#define PTRACE_SET_THREAD_AREA 26

#ifdef __KERNEL__
-#define user_mode(regs) ((VM_MASK & (regs)->eflags) || (3 & (regs)->xcs))
+#define user_mode(regs) ((X86_EFLAGS_VM & (regs)->eflags) || (3 & (regs)->xcs))
#define instruction_pointer(regs) ((regs)->eip)
#endif

diff -Naurp ./include/asm-i386/processor.h~use_eflags ./include/asm-i386/processor.h
--- ./include/asm-i386/processor.h~use_eflags 2004-03-10 18:55:21.000000000 -0800
+++ ./include/asm-i386/processor.h 2004-04-06 21:20:57.000000000 -0700
@@ -121,6 +121,10 @@ extern void dodgy_tsc(void);
#define X86_EFLAGS_VIP 0x00100000 /* Virtual Interrupt Pending */
#define X86_EFLAGS_ID 0x00200000 /* CPUID detection flag */

+#define X86_EFLAGS_USER (X86_EFLAGS_AC | X86_EFLAGS_OF | X86_EFLAGS_DF | \
+ X86_EFLAGS_TF | X86_EFLAGS_SF | X86_EFLAGS_ZF | \
+ X86_EFLAGS_AF | X86_EFLAGS_PF | X86_EFLAGS_CF)
+
/*
* Generic CPUID function
*/
diff -Naurp ./include/asm-i386/vm86.h~use_eflags ./include/asm-i386/vm86.h
--- ./include/asm-i386/vm86.h~use_eflags 2004-03-10 18:55:35.000000000 -0800
+++ ./include/asm-i386/vm86.h 2004-04-06 21:58:20.000000000 -0700
@@ -4,7 +4,7 @@
/*
* I'm guessing at the VIF/VIP flag usage, but hope that this is how
* the Pentium uses them. Linux will return from vm86 mode when both
- * VIF and VIP is set.
+ * VIF and VIP are set.
*
* On a Pentium, we could probably optimize the virtual flags directly
* in the eflags register instead of doing it "by hand" in vflags...
@@ -12,16 +12,6 @@
* Linus
*/

-#define TF_MASK 0x00000100
-#define IF_MASK 0x00000200
-#define IOPL_MASK 0x00003000
-#define NT_MASK 0x00004000
-#define VM_MASK 0x00020000
-#define AC_MASK 0x00040000
-#define VIF_MASK 0x00080000 /* virtual interrupt flag */
-#define VIP_MASK 0x00100000 /* virtual interrupt pending */
-#define ID_MASK 0x00200000
-
#define BIOSSEG 0x0f000

#define CPU_086 0
--
~Randy
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/