Skip to contentSkip to conversation

sparc64: Implement local_irq_save_nmi().

PATCH In review David Miller 2 participants 4 messages Apr 6, 2010 – Apr 7, 2010

Subsystems (2)
Generic include/asm header files SPARC + UltraSPARC (sparc/sparc64)
lore
Share & tools
lore

David Miller

author maintainer
#0 lore raw

PATCH 1/2 sparc64: Implement local_irq_save_nmi().

It disables up to PIL_NMI instead of just PIL_NORMAL_MAX.

Signed-off-by David S. Miller <[email protected]>
arch/sparc/include/asm/irqflags_64.h +29 -0
index 8b49bf9..fa1e00e 100644
@@ -49,6 +49,16 @@ static inline void raw_local_irq_disable(void)
 	);
 }
 
+static inline void raw_local_irq_disable_nmi(void)
+{
+	__asm__ __volatile__(
+		"wrpr	%0, %%pil"
+		: /* no outputs */
+		: "i" (PIL_NMI)
+		: "memory"
+	);
+}
+
 static inline void raw_local_irq_enable(void)
 {
 	__asm__ __volatile__(

@@ -83,9 +93,28 @@ static inline unsigned long __raw_local_irq_save(void)
 	return flags;
 }
 
+static inline unsigned long __raw_local_irq_save_nmi(void)
+{
+	unsigned long flags = __raw_local_save_flags();
+
+	raw_local_irq_disable_nmi();
+
+	return flags;
+}
+
 #define raw_local_irq_save(flags) \
 		do { (flags) = __raw_local_irq_save(); } while (0)
 
+#define raw_local_irq_save_nmi(flags) \
+		do { (flags) = __raw_local_irq_save_nmi(); } while (0)
+
+#define local_irq_save_nmi(flags) 			\
+	do {						\
+		 typecheck(unsigned long, flags);	\
+		 raw_local_irq_save_nmi(flags);		\
+		 trace_hardirqs_off();			\
+	} while (0)
+
 #endif /* (__ASSEMBLY__) */
 
 #endif /* !(_ASM_IRQFLAGS_H) */
#1 lore raw
Quoted 12 lines · David Miller

It disables up to PIL_NMI instead of just PIL_NORMAL_MAX.

Signed-off-by David S. Miller <[email protected]>
arch/sparc/include/asm/irqflags_64.h +0 -0
index 8b49bf9..fa1e00e 100644
@@ -49,6 +49,16 @@ static inline void raw_local_irq_disable(void)
 	);
 }

arch/sparc/include/asm/irqflags_64.h static inline void raw_local_irq_disable(void) · line 61

Quoted 10 lines
arch/sparc/include/asm/irqflags_64.h +10 -0
+static inline void raw_local_irq_disable_nmi(void)
+{
+	__asm__ __volatile__(
+		"wrpr	%0, %%pil"
+		: /* no outputs */
+		: "i" (PIL_NMI)
+		: "memory"
+	);
+}
+

Isn't this wrong when used from !NMI context?

Should this thing do something like:

  if (rdpr() < PIL_NORMAL_MAX)
    wrpr(PIL_NORMAL_MAX);

so that it only disables IRQs, but doesn't enable NMIs.

author maintainer
#2 lore raw

From: Peter Zijlstra <[email protected]>
Date: Wed, 07 Apr 2010 08:52:26 +0200

Quoted 18 lines · David Miller
Quoted 4 lines
arch/sparc/include/asm/irqflags_64.h +0 -0
@@ -49,6 +49,16 @@ static inline void raw_local_irq_disable(void)
 	);
 }

arch/sparc/include/asm/irqflags_64.h static inline void raw_local_irq_disable(void) · line 61

Quoted 10 lines
arch/sparc/include/asm/irqflags_64.h +10 -0
+static inline void raw_local_irq_disable_nmi(void)
+{
+	__asm__ __volatile__(
+		"wrpr	%0, %%pil"
+		: /* no outputs */
+		: "i" (PIL_NMI)
+		: "memory"
+	);
+}
+

Isn't this wrong when used from !NMI context?

Quoted 6 lines

Should this thing do something like:

  if (rdpr() < PIL_NORMAL_MAX)
    wrpr(PIL_NORMAL_MAX);

so that it only disables IRQs, but doesn't enable NMIs.

It's immaterial, local_irq_restore() will do the right thing, and it's ok to disable NMIs in these few cases I think.

I desperately want to avoid that "test and maybe change the value %pil value we write" business, and honestly that's the whole point of this exercise.

#3 lore raw
Quoted 26 lines · David Miller

From: Peter Zijlstra <[email protected]>
Date: Wed, 07 Apr 2010 08:52:26 +0200

Quoted 23 lines · David Miller
Quoted 4 lines
arch/sparc/include/asm/irqflags_64.h +0 -0
@@ -49,6 +49,16 @@ static inline void raw_local_irq_disable(void)
 	);
 }

arch/sparc/include/asm/irqflags_64.h static inline void raw_local_irq_disable(void) · line 61

Quoted 10 lines
arch/sparc/include/asm/irqflags_64.h +10 -0
+static inline void raw_local_irq_disable_nmi(void)
+{
+	__asm__ __volatile__(
+		"wrpr	%0, %%pil"
+		: /* no outputs */
+		: "i" (PIL_NMI)
+		: "memory"
+	);
+}
+

Isn't this wrong when used from !NMI context?

Should this thing do something like:

  if (rdpr() < PIL_NORMAL_MAX)
    wrpr(PIL_NORMAL_MAX);
Quoted 8 lines
Quoted 1 line

so that it only disables IRQs, but doesn't enable NMIs.

It's immaterial, local_irq_restore() will do the right thing, and it's ok to disable NMIs in these few cases I think.

I desperately want to avoid that "test and maybe change the value %pil value we write" business, and honestly that's the whole point of this exercise.

Sure, its your architecture.. but could you explain why you're trying to avoid that compare so desperately, the local_irq_save_nmi() calls are few so surely they could carry that overhead.

Also, doesn't __raw_local_irq_save_flags() already do the read? So its really just the compare that's gone missing.