Skip to contentSkip to conversation

Define EFLAGS_IF

PATCH In review Rusty Russell 5 participants 13 messages Mar 22, 2007 – Apr 9, 2007 virtio core

lore
Share & tools
lore

Rusty Russell

author
#0 lore raw

PATCH 1/1 Define EFLAGS_IF

There is now more than one place where we use the fact that bit 9 of eflags is the interrupt-enabled flag, so define EFLAGS_IF. We make it 512 so it can be used in asm, too.

Signed-off-by Rusty Russell <[email protected]>
arch/i386/lguest/lguest.c +2 -3
@@ -107,9 +107,8 @@ static void fastcall irq_disable(void)
 
 static void fastcall irq_enable(void)
 {
-	/* Linux i386 code expects bit 9 set. */
 	/* FIXME: Check if interrupt pending... */
-	lguest_data.irq_enabled = 512;
+	lguest_data.irq_enabled = EFLAGS_IF;
 }
 
 static void fastcall lguest_load_gdt(const struct Xgt_desc_struct *desc)

@@ -394,7 +393,7 @@ static fastcall void lguest_write_idt_en
 	extern const char start_##name[], end_##name[];		\
 	asm("start_" #name ": " code "; end_" #name ":")
 DEF_LGUEST(cli, "movl $0," LGUEST_IRQ);
-DEF_LGUEST(sti, "movl $512," LGUEST_IRQ);
+DEF_LGUEST(sti, "movl $"__stringify(EFLAGS_IF)"," LGUEST_IRQ);
 DEF_LGUEST(popf, "movl %eax," LGUEST_IRQ);
 DEF_LGUEST(pushf, "movl " LGUEST_IRQ ",%eax");
 DEF_LGUEST(pushf_cli, "movl " LGUEST_IRQ ",%eax; movl $0," LGUEST_IRQ);

===================================================================

include/asm-i386/irqflags.h +4 -1
@@ -87,6 +87,9 @@ static inline unsigned long __raw_local_
 #endif /* __ASSEMBLY__ */
 #endif /* CONFIG_PARAVIRT */
 
+/* Bit 9 of eflags means interrupts are enabled: a raw int for asm. */
+#define EFLAGS_IF 512
+
 #ifndef __ASSEMBLY__
 #define raw_local_save_flags(flags) \
 		do { (flags) = __raw_local_save_flags(); } while (0)

@@ -96,7 +99,7 @@ static inline unsigned long __raw_local_
 
 static inline int raw_irqs_disabled_flags(unsigned long flags)
 {
-	return !(flags & (1 << 9));
+	return !(flags & EFLAGS_IF);
 }
 
 static inline int raw_irqs_disabled(void)
author
#1 lore raw
Quoted 3 lines · Rusty Russell

There is now more than one place where we use the fact that bit 9 of eflags is the interrupt-enabled flag, so define EFLAGS_IF. We make it 512 so it can be used in asm, too.

Belay this: there's a X86_EFLAGS_IF in asm/processor.h which we should use. Will send patch.

Thanks to Jeremy Fitzhardinge for the clue.

Rusty.

author
#2 lore raw

PATCH 1/1 Use X86_EFLAGS_IF in irqflags.h, lguest.

Quoted 2 lines · Rusty Russell

Belay this: there's a X86_EFLAGS_IF in asm/processor.h which we should use. Will send patch.

How's this. There may be other users, but they're not easy to grep for.

==
Move X86_EFLAGS_IF et al out to a new header: processor-flags.h, so we can include it from irqflags.h and use it in raw_irqs_disabled_flags().

As a side-effect, we could now use these flags in .S files.

Lguest also modified to use the flags.

Signed-off-by Rusty Russell <[email protected]>
include/asm-i386/processor-flags.h new file +26 -0
diff -r bd0a803d9948 include/asm-i386/processor-flags.h
@@ -0,0 +1,26 @@
+#ifndef __ASM_I386_PROCESSOR_FLAGS_H
+#define __ASM_I386_PROCESSOR_FLAGS_H
+/* Various flags defined: can be included from assembler. */
+
+/*
+ * EFLAGS bits
+ */
+#define X86_EFLAGS_CF	0x00000001 /* Carry Flag */
+#define X86_EFLAGS_PF	0x00000004 /* Parity Flag */
+#define X86_EFLAGS_AF	0x00000010 /* Auxillary carry Flag */
+#define X86_EFLAGS_ZF	0x00000040 /* Zero Flag */
+#define X86_EFLAGS_SF	0x00000080 /* Sign Flag */
+#define X86_EFLAGS_TF	0x00000100 /* Trap Flag */
+#define X86_EFLAGS_IF	0x00000200 /* Interrupt Flag */
+#define X86_EFLAGS_DF	0x00000400 /* Direction Flag */
+#define X86_EFLAGS_OF	0x00000800 /* Overflow Flag */
+#define X86_EFLAGS_IOPL	0x00003000 /* IOPL mask */
+#define X86_EFLAGS_NT	0x00004000 /* Nested Task */
+#define X86_EFLAGS_RF	0x00010000 /* Resume Flag */
+#define X86_EFLAGS_VM	0x00020000 /* Virtual Mode */
+#define X86_EFLAGS_AC	0x00040000 /* Alignment Check */
+#define X86_EFLAGS_VIF	0x00080000 /* Virtual Interrupt Flag */
+#define X86_EFLAGS_VIP	0x00100000 /* Virtual Interrupt Pending */
+#define X86_EFLAGS_ID	0x00200000 /* CPUID detection flag */
+
+#endif	/* __ASM_I386_PROCESSOR_FLAGS_H */
include/asm-i386/irqflags.h +2 -1
diff -r bd0a803d9948 include/asm-i386/irqflags.h
@@ -9,6 +9,7 @@
  */
 #ifndef _ASM_IRQFLAGS_H
 #define _ASM_IRQFLAGS_H
+#include <asm/processor-flags.h>
 
 #ifndef __ASSEMBLY__
 static inline unsigned long native_save_fl(void)

@@ -119,7 +120,7 @@ static inline unsigned long __raw_local_
 
 static inline int raw_irqs_disabled_flags(unsigned long flags)
 {
-	return !(flags & (1 << 9));
+	return !(flags & X86_EFLAGS_IF);
 }
 
 static inline int raw_irqs_disabled(void)
include/asm-i386/processor.h +1 -21
diff -r bd0a803d9948 include/asm-i386/processor.h
@@ -21,6 +21,7 @@
 #include <asm/percpu.h>
 #include <linux/cpumask.h>
 #include <linux/init.h>
+#include <asm/processor-flags.h>
 
 /* flag for disabling the tsc */
 extern int tsc_disable;

@@ -125,27 +126,6 @@ extern void detect_ht(struct cpuinfo_x86
 #else
 static inline void detect_ht(struct cpuinfo_x86 *c) {}
 #endif
-
-/*
- * EFLAGS bits
- */
-#define X86_EFLAGS_CF	0x00000001 /* Carry Flag */
-#define X86_EFLAGS_PF	0x00000004 /* Parity Flag */
-#define X86_EFLAGS_AF	0x00000010 /* Auxillary carry Flag */
-#define X86_EFLAGS_ZF	0x00000040 /* Zero Flag */
-#define X86_EFLAGS_SF	0x00000080 /* Sign Flag */
-#define X86_EFLAGS_TF	0x00000100 /* Trap Flag */
-#define X86_EFLAGS_IF	0x00000200 /* Interrupt Flag */
-#define X86_EFLAGS_DF	0x00000400 /* Direction Flag */
-#define X86_EFLAGS_OF	0x00000800 /* Overflow Flag */
-#define X86_EFLAGS_IOPL	0x00003000 /* IOPL mask */
-#define X86_EFLAGS_NT	0x00004000 /* Nested Task */
-#define X86_EFLAGS_RF	0x00010000 /* Resume Flag */
-#define X86_EFLAGS_VM	0x00020000 /* Virtual Mode */
-#define X86_EFLAGS_AC	0x00040000 /* Alignment Check */
-#define X86_EFLAGS_VIF	0x00080000 /* Virtual Interrupt Flag */
-#define X86_EFLAGS_VIP	0x00100000 /* Virtual Interrupt Pending */
-#define X86_EFLAGS_ID	0x00200000 /* CPUID detection flag */
 
 static inline void native_cpuid(unsigned int *eax, unsigned int *ebx,
 					 unsigned int *ecx, unsigned int *edx)
arch/i386/lguest/interrupts_and_traps.c +2 -2
diff -r bd0a803d9948 arch/i386/lguest/interrupts_and_traps.c
@@ -42,7 +42,7 @@ static void reflect_trap(struct lguest *
 	   (it's always 0, since irqs are enabled when guest is running). */
 	eflags = regs->eflags;
 	get_user(irq_enable, &lg->lguest_data->irq_enabled);
-	eflags |= (irq_enable & 512);
+	eflags |= (irq_enable & X86_EFLAGS_IF);
 
 	push_guest_stack(lg, &gstack, eflags);
 	push_guest_stack(lg, &gstack, regs->cs);

@@ -86,7 +86,7 @@ void maybe_do_interrupt(struct lguest *l
 	/* If they're halted, we re-enable interrupts. */
 	if (lg->halted) {
 		/* Re-enable interrupts. */
-		put_user(512, &lg->lguest_data->irq_enabled);
+		put_user(X86_EFLAGS_IF, &lg->lguest_data->irq_enabled);
 		lg->halted = 0;
 	} else {
 		/* Maybe they have interrupts disabled? */
arch/i386/lguest/lguest.c +2 -3
diff -r bd0a803d9948 arch/i386/lguest/lguest.c
@@ -107,9 +107,8 @@ static void fastcall irq_disable(void)
 
 static void fastcall irq_enable(void)
 {
-	/* Linux i386 code expects bit 9 set. */
 	/* FIXME: Check if interrupt pending... */
-	lguest_data.irq_enabled = 512;
+	lguest_data.irq_enabled = X86_EFLAGS_IF;
 }
 
 static void fastcall lguest_load_gdt(const struct Xgt_desc_struct *desc)

@@ -394,7 +393,7 @@ static fastcall void lguest_write_idt_en
 	extern const char start_##name[], end_##name[];		\
 	asm("start_" #name ": " code "; end_" #name ":")
 DEF_LGUEST(cli, "movl $0," LGUEST_IRQ);
-DEF_LGUEST(sti, "movl $512," LGUEST_IRQ);
+DEF_LGUEST(sti, "movl $"__stringify(X86_EFLAGS_IF)"," LGUEST_IRQ);
 DEF_LGUEST(popf, "movl %eax," LGUEST_IRQ);
 DEF_LGUEST(pushf, "movl " LGUEST_IRQ ",%eax");
 DEF_LGUEST(pushf_cli, "movl " LGUEST_IRQ ",%eax; movl $0," LGUEST_IRQ);
#3 lore raw

Re: [PATCH] Use X86_EFLAGS_IF in irqflags.h, lguest.

Quoted 5 lines · Rusty Russell (on Thu, 22 Mar 2007 14:52:29 +1100) wrote:
Quoted 2 lines · Rusty Russell

Belay this: there's a X86_EFLAGS_IF in asm/processor.h which we should use. Will send patch.

How's this. There may be other users, but they're not easy to grep for.

One less set of definitions for KDB to create - Thanks.

#4 lore raw
In reply to Rusty Russell
Quoted 3 lines · Rusty Russell

There is now more than one place where we use the fact that bit 9 of eflags is the interrupt-enabled flag, so define EFLAGS_IF. We make it 512 so it can be used in asm, too.

How about defining all the other EFLAGS in one place?

-hpa

#5 lore raw
Quoted 9 lines · H. Peter Anvin
Quoted 4 lines · Rusty Russell

There is now more than one place where we use the fact that bit 9 of eflags is the interrupt-enabled flag, so define EFLAGS_IF. We make it 512 so it can be used in asm, too.

How about defining all the other EFLAGS in one place?

That patch got dropped, and replaced by one which pulled all the flags definitions out of <asm/processor.h>

J

#6 lore raw
Quoted 4 lines · Jeremy Fitzhardinge

That patch got dropped, and replaced by one which pulled all the flags definitions out of <asm/processor.h>

Saw that a little too late :)

In general, it would be nice if the various CPU constants were all defined in one place, so I'd rather suggest protecting the appropriate parts of asm/processor.h with #ifndef __ASSEMBLY__.

-hpa

#7 lore raw
Quoted 11 lines · H. Peter Anvin
Quoted 4 lines · Jeremy Fitzhardinge

That patch got dropped, and replaced by one which pulled all the flags definitions out of <asm/processor.h>

Saw that a little too late :)

In general, it would be nice if the various CPU constants were all defined in one place, so I'd rather suggest protecting the appropriate parts of asm/processor.h with #ifndef __ASSEMBLY__.

No processor.h is such a hodgepodge of unrelated stuff that any splitting up is a good thing.

-Andi

#8 lore raw
Quoted 4 lines · Andi Kleen

No processor.h is such a hodgepodge of unrelated stuff that any splitting up is a good thing.

Fair enough. However, I'd still like to see the X86_CR* constants moved, too (and constants added for at least CR0 as well.)

-hpa

author
#9 lore raw
Quoted 8 lines · H. Peter Anvin
Quoted 4 lines · Andi Kleen

No processor.h is such a hodgepodge of unrelated stuff that any splitting up is a good thing.

Fair enough. However, I'd still like to see the X86_CR* constants moved, too (and constants added for at least CR0 as well.)

Agreed. This was on theory of minimum damage, but since it seems to have received a warm reception, I'd say moving the rest to processor-flags.h would be a welcome addition.

Cheers,
Rusty.

#10 lore raw
In reply to H. Peter Anvin
Quoted 8 lines · H. Peter Anvin
Quoted 4 lines · Andi Kleen

No processor.h is such a hodgepodge of unrelated stuff that any splitting up is a good thing.

Fair enough. However, I'd still like to see the X86_CR* constants moved, too (and constants added for at least CR0 as well.)

Send patches then.

-Andi

#11 lore raw
Quoted 8 lines · Andi Kleen
Quoted 5 lines · H. Peter Anvin
Quoted 3 lines · Andi Kleen

No processor.h is such a hodgepodge of unrelated stuff that any splitting up is a good thing.

Fair enough. However, I'd still like to see the X86_CR* constants moved, too (and constants added for at least CR0 as well.)

Send patches then.

I will, unless Rusty does, first. No desire to step on each other.

-hpa

author
#12 lore raw
Quoted 1 line · H. Peter Anvin

I will, unless Rusty does, first. No desire to step on each other.

Oh no, please, after you!

Rusty.