[PATCH] ia64: avoid broken SAL_CACHE_FLUSH implementations

[PATCH] ia64: avoid broken SAL_CACHE_FLUSH implementations

[Bjorn Helgaas]

If SAL_CACHE_FLUSH drops interrupts, complain about it and fall back to
using PAL_CACHE_FLUSH instead.

This is to work around a defect in HP rx5670 firmware: when an interrupt
occurs during SAL_CACHE_FLUSH, SAL drops the interrupt but leaves it marked
"in-service", which leaves the interrupt (and others of equal or lower
priority) masked.

HP internal defect reports: F1859, F2775, F3031.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Index: work9/arch/ia64/kernel/sal.c
=================================--- work9.orig/arch/ia64/kernel/sal.c	2006-01-30 09:50:16.000000000 -0700
+++ work9/arch/ia64/kernel/sal.c	2006-01-30 15:06:18.000000000 -0700
@@ -14,6 +14,7 @@
 #include <linux/spinlock.h>
 #include <linux/string.h>
 
+#include <asm/delay.h>
 #include <asm/page.h>
 #include <asm/sal.h>
 #include <asm/pal.h>
@@ -214,6 +215,78 @@
 static void __init sal_desc_ap_wakeup(void *p) { }
 #endif
 
+/*
+ * HP rx5670 firmware polls for interrupts during SAL_CACHE_FLUSH by reading
+ * cr.ivr, but it never writes cr.eoi.  This leaves any interrupt marked as
+ * "in-service" and masks other interrupts of equal or lower priority.
+ *
+ * HP internal defect reports: F1859, F2775, F3031.
+ */
+static int sal_cache_flush_drops_interrupts;
+
+static void __init
+check_sal_cache_flush (void)
+{
+	unsigned long flags, itv;
+	int cpu;
+	u64 vector;
+
+	cpu = get_cpu();
+	local_irq_save(flags);
+
+	/*
+	 * Schedule a timer interrupt, wait until it's reported, and see if
+	 * SAL_CACHE_FLUSH drops it.
+	 */
+	itv = ia64_get_itv();
+	BUG_ON((itv & (1 << 16)) = 0);
+
+	ia64_set_itv(IA64_TIMER_VECTOR);
+	ia64_set_itm(ia64_get_itc() + 1000);
+
+	while (!ia64_get_irr(IA64_TIMER_VECTOR))
+		;
+
+	ia64_sal_cache_flush(3);
+
+	if (ia64_get_irr(IA64_TIMER_VECTOR)) {
+		vector = ia64_get_ivr();
+		ia64_eoi();
+		WARN_ON(vector != IA64_TIMER_VECTOR);
+	} else {
+		sal_cache_flush_drops_interrupts = 1;
+		printk(KERN_ERR "SAL: SAL_CACHE_FLUSH drops interrupts; "
+			"PAL_CACHE_FLUSH will be used instead\n");
+		ia64_eoi();
+	}
+
+	ia64_set_itv(itv);
+	local_irq_restore(flags);
+	put_cpu();
+}
+
+s64
+ia64_sal_cache_flush (u64 cache_type)
+{
+	struct ia64_sal_retval isrv;
+
+	if (sal_cache_flush_drops_interrupts) {
+		unsigned long flags;
+		u64 progress;
+		s64 rc;
+
+		progress = 0;
+		local_irq_save(flags);
+		rc = ia64_pal_cache_flush(cache_type,
+			PAL_CACHE_FLUSH_INVALIDATE, &progress, NULL);
+		local_irq_restore(flags);
+		return rc;
+	}
+
+	SAL_CALL(isrv, SAL_CACHE_FLUSH, cache_type, 0, 0, 0, 0, 0, 0);
+	return isrv.status;
+}
+
 void __init
 ia64_sal_init (struct ia64_sal_systab *systab)
 {
@@ -262,6 +335,8 @@
 		}
 		p += SAL_DESC_SIZE(*p);
 	}
+
+	check_sal_cache_flush();
 }
 
 int
Index: work9/include/asm-ia64/sal.h
=================================--- work9.orig/include/asm-ia64/sal.h	2006-01-30 09:50:26.000000000 -0700
+++ work9/include/asm-ia64/sal.h	2006-01-30 14:36:00.000000000 -0700
@@ -658,15 +658,7 @@
 	return isrv.status;
 }
 
-/* Flush all the processor and platform level instruction and/or data caches */
-static inline s64
-ia64_sal_cache_flush (u64 cache_type)
-{
-	struct ia64_sal_retval isrv;
-	SAL_CALL(isrv, SAL_CACHE_FLUSH, cache_type, 0, 0, 0, 0, 0, 0);
-	return isrv.status;
-}
-
+extern s64 ia64_sal_cache_flush (u64 cache_type);
 
 /* Initialize all the processor and platform level instruction and data caches */
 static inline s64
Index: work9/include/asm-ia64/processor.h
=================================--- work9.orig/include/asm-ia64/processor.h	2006-01-19 16:59:35.000000000 -0700
+++ work9/include/asm-ia64/processor.h	2006-01-30 10:03:15.000000000 -0700
@@ -559,6 +559,23 @@
 
 #define cpu_relax()	ia64_hint(ia64_hint_pause)
 
+static inline int
+ia64_get_irr(unsigned int vector)
+{
+	unsigned int reg = vector / 64;
+	unsigned int bit = vector % 64;
+	u64 irr;
+
+	switch (reg) {
+	case 0: irr = ia64_getreg(_IA64_REG_CR_IRR0); break;
+	case 1: irr = ia64_getreg(_IA64_REG_CR_IRR1); break;
+	case 2: irr = ia64_getreg(_IA64_REG_CR_IRR2); break;
+	case 3: irr = ia64_getreg(_IA64_REG_CR_IRR3); break;
+	}
+
+	return test_bit(bit, &irr);
+}
+
 static inline void
 ia64_set_lrr0 (unsigned long val)
 {

Re: [PATCH] ia64: avoid broken SAL_CACHE_FLUSH implementations

[Keith Owens]

Bjorn Helgaas (on Mon, 30 Jan 2006 15:11:57 -0700) wrote:
>If SAL_CACHE_FLUSH drops interrupts, complain about it and fall back to
>using PAL_CACHE_FLUSH instead.
>+	while (!ia64_get_irr(IA64_TIMER_VECTOR))
>+		;

cpu_relax() instead of an empty loop?

Besides being the "right thing" for dual cores, it also guarantees that
the compiler will not optimize away or move the loop.  ia64_get_irr()
maps to ia64_getreg() which on gcc is not optimized away, but in icc
ia64_getreg() maps to __getReg() and I am not sure if that can be
optimized or moved.  The Intel compiler documentation is silent on this
topic.

FWIW, I tried the patch on SGI SN2 hardware - there was no error
message from check_sal_cache_flush().

Re: [PATCH] ia64: avoid broken SAL_CACHE_FLUSH implementations

[Bjorn Helgaas]

On Monday 30 January 2006 16:12, Keith Owens wrote:
> Bjorn Helgaas (on Mon, 30 Jan 2006 15:11:57 -0700) wrote:
> >If SAL_CACHE_FLUSH drops interrupts, complain about it and fall back to
> >using PAL_CACHE_FLUSH instead.
> >+	while (!ia64_get_irr(IA64_TIMER_VECTOR))
> >+		;
> 
> cpu_relax() instead of an empty loop?

Updated in patch below.

> FWIW, I tried the patch on SGI SN2 hardware - there was no error
> message from check_sal_cache_flush().

On the rx5670, this defect causes a hang because the BSP drops
a timer tick during the boot-time migration cost measurement.
The hang happens a bit later, when we start waiting for jiffies
to pass.  So check_sal_cache_flush() should only find a problem
on boxes that would run into bigger problems later on.

Thanks for trying this out.


ia64: avoid broken SAL_CACHE_FLUSH implementations

If SAL_CACHE_FLUSH drops interrupts, complain about it and fall back to
using PAL_CACHE_FLUSH instead.

This is to work around a defect in HP rx5670 firmware: when an interrupt
occurs during SAL_CACHE_FLUSH, SAL drops the interrupt but leaves it marked
"in-service", which leaves the interrupt (and others of equal or lower
priority) masked.

HP internal defect reports: F1859, F2775, F3031.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Index: work9/arch/ia64/kernel/sal.c
=================================--- work9.orig/arch/ia64/kernel/sal.c	2006-01-30 09:50:16.000000000 -0700
+++ work9/arch/ia64/kernel/sal.c	2006-01-30 16:16:39.000000000 -0700
@@ -14,6 +14,7 @@
 #include <linux/spinlock.h>
 #include <linux/string.h>
 
+#include <asm/delay.h>
 #include <asm/page.h>
 #include <asm/sal.h>
 #include <asm/pal.h>
@@ -214,6 +215,78 @@
 static void __init sal_desc_ap_wakeup(void *p) { }
 #endif
 
+/*
+ * HP rx5670 firmware polls for interrupts during SAL_CACHE_FLUSH by reading
+ * cr.ivr, but it never writes cr.eoi.  This leaves any interrupt marked as
+ * "in-service" and masks other interrupts of equal or lower priority.
+ *
+ * HP internal defect reports: F1859, F2775, F3031.
+ */
+static int sal_cache_flush_drops_interrupts;
+
+static void __init
+check_sal_cache_flush (void)
+{
+	unsigned long flags, itv;
+	int cpu;
+	u64 vector;
+
+	cpu = get_cpu();
+	local_irq_save(flags);
+
+	/*
+	 * Schedule a timer interrupt, wait until it's reported, and see if
+	 * SAL_CACHE_FLUSH drops it.
+	 */
+	itv = ia64_get_itv();
+	BUG_ON((itv & (1 << 16)) = 0);
+
+	ia64_set_itv(IA64_TIMER_VECTOR);
+	ia64_set_itm(ia64_get_itc() + 1000);
+
+	while (!ia64_get_irr(IA64_TIMER_VECTOR))
+		cpu_relax();
+
+	ia64_sal_cache_flush(3);
+
+	if (ia64_get_irr(IA64_TIMER_VECTOR)) {
+		vector = ia64_get_ivr();
+		ia64_eoi();
+		WARN_ON(vector != IA64_TIMER_VECTOR);
+	} else {
+		sal_cache_flush_drops_interrupts = 1;
+		printk(KERN_ERR "SAL: SAL_CACHE_FLUSH drops interrupts; "
+			"PAL_CACHE_FLUSH will be used instead\n");
+		ia64_eoi();
+	}
+
+	ia64_set_itv(itv);
+	local_irq_restore(flags);
+	put_cpu();
+}
+
+s64
+ia64_sal_cache_flush (u64 cache_type)
+{
+	struct ia64_sal_retval isrv;
+
+	if (sal_cache_flush_drops_interrupts) {
+		unsigned long flags;
+		u64 progress;
+		s64 rc;
+
+		progress = 0;
+		local_irq_save(flags);
+		rc = ia64_pal_cache_flush(cache_type,
+			PAL_CACHE_FLUSH_INVALIDATE, &progress, NULL);
+		local_irq_restore(flags);
+		return rc;
+	}
+
+	SAL_CALL(isrv, SAL_CACHE_FLUSH, cache_type, 0, 0, 0, 0, 0, 0);
+	return isrv.status;
+}
+
 void __init
 ia64_sal_init (struct ia64_sal_systab *systab)
 {
@@ -262,6 +335,8 @@
 		}
 		p += SAL_DESC_SIZE(*p);
 	}
+
+	check_sal_cache_flush();
 }
 
 int
Index: work9/include/asm-ia64/sal.h
=================================--- work9.orig/include/asm-ia64/sal.h	2006-01-30 09:50:26.000000000 -0700
+++ work9/include/asm-ia64/sal.h	2006-01-30 14:36:00.000000000 -0700
@@ -658,15 +658,7 @@
 	return isrv.status;
 }
 
-/* Flush all the processor and platform level instruction and/or data caches */
-static inline s64
-ia64_sal_cache_flush (u64 cache_type)
-{
-	struct ia64_sal_retval isrv;
-	SAL_CALL(isrv, SAL_CACHE_FLUSH, cache_type, 0, 0, 0, 0, 0, 0);
-	return isrv.status;
-}
-
+extern s64 ia64_sal_cache_flush (u64 cache_type);
 
 /* Initialize all the processor and platform level instruction and data caches */
 static inline s64
Index: work9/include/asm-ia64/processor.h
=================================--- work9.orig/include/asm-ia64/processor.h	2006-01-19 16:59:35.000000000 -0700
+++ work9/include/asm-ia64/processor.h	2006-01-30 10:03:15.000000000 -0700
@@ -559,6 +559,23 @@
 
 #define cpu_relax()	ia64_hint(ia64_hint_pause)
 
+static inline int
+ia64_get_irr(unsigned int vector)
+{
+	unsigned int reg = vector / 64;
+	unsigned int bit = vector % 64;
+	u64 irr;
+
+	switch (reg) {
+	case 0: irr = ia64_getreg(_IA64_REG_CR_IRR0); break;
+	case 1: irr = ia64_getreg(_IA64_REG_CR_IRR1); break;
+	case 2: irr = ia64_getreg(_IA64_REG_CR_IRR2); break;
+	case 3: irr = ia64_getreg(_IA64_REG_CR_IRR3); break;
+	}
+
+	return test_bit(bit, &irr);
+}
+
 static inline void
 ia64_set_lrr0 (unsigned long val)
 {

Re: [PATCH] ia64: avoid broken SAL_CACHE_FLUSH implementations

[Luck, Tony]

On Mon, Jan 30, 2006 at 04:51:57PM -0700, Bjorn Helgaas wrote:
> On Monday 30 January 2006 16:36, Luck, Tony wrote:
> > > If SAL_CACHE_FLUSH drops interrupts, complain about it and fall back to
> > > using PAL_CACHE_FLUSH instead.
> > 
> > But PAL_CACHE_FLUSH may not do all that SAL_CACHE_FLUSH does (e.g. if
> > the system has caches outside of the scope of PAL).  So there is an
> > assumption here that only systems where SAL_CACHE_FLUSH is equivalent
> > to PAL_CACHE_FLUSH have this bug in their SAL.
> 
> Right.  I verified with our firmware guys that in the case of the
> rx5670, SAL doesn't do anything extra.  So we should be safe there.
> 
> But you're right that in general, SAL may do something extra.
> 
> I'm hoping that the rx5670 is the only shipping box with this problem,
> and the error check should point out the problem so firmware for
> future boxes can be fixed.
> 
> I thought about also checking for "HP" or "rx5670" somewhere, but
> wasn't sure whether the extra effort of grubbing through DMI or
> something to find that would be worthwhile.
> 
> > Keith just confirmed that SGI doesn't have this SAL bug, but should I
> > be worried about other large ia64 boxes?
> 
> I expect that if other boxes have this problem, they should be seeing
> issues, now that we actually *use* SAL_CACHE_FLUSH for the migration
> cost measurements.  The dropped interrupt should cause pretty obvious
> problems.
> 
> (Did you mean to post this to the list?  If you meant to but forgot,
> you can quote my response.)

Doh!  Yes, I meant for this one to go to the list.  Perhaps the
printk(KERN_ERR "SAL: ...); will be enough of a warning in the
future to anyone unfortunate enough to have a SAL with this bug
and PAL_CACHE_FLUSH != SAL_CACHE_FLUSH.

-Tony

RE: [PATCH] ia64: avoid broken SAL_CACHE_FLUSH implementations

[Luck, Tony]

> ia64: avoid broken SAL_CACHE_FLUSH implementations
>
> If SAL_CACHE_FLUSH drops interrupts, complain about it and fall back to
> using PAL_CACHE_FLUSH instead.
>
> This is to work around a defect in HP rx5670 firmware: when an interrupt
> occurs during SAL_CACHE_FLUSH, SAL drops the interrupt but leaves it marked
> "in-service", which leaves the interrupt (and others of equal or lower
> priority) masked.
>
> HP internal defect reports: F1859, F2775, F3031.

Sometimes I forget to test patches against my zx2000 workstation (because
it is such a pain to restart all my apps after I reboot).  This patch
slipped through.  My zx2000 hasn't been booting since 2.6.15 and I
finally spent the time to track down why.  It hangs (or dies) inside
check_sal_cache_flush() :-(

-Tony

RE: [PATCH] ia64: avoid broken SAL_CACHE_FLUSH implementations

[Alex Williamson]

On Fri, 2006-02-17 at 12:03 -0800, Luck, Tony wrote:

> Sometimes I forget to test patches against my zx2000 workstation (because
> it is such a pain to restart all my apps after I reboot).  This patch
> slipped through.  My zx2000 hasn't been booting since 2.6.15 and I
> finally spent the time to track down why.  It hangs (or dies) inside
> check_sal_cache_flush() :-(

Tony,

   What firmware version do you have on your zx2000?  I saw the same
problem with my zx2000 running firmware 2.21 and was hacking at a work
around, when I realized version 2.31 is available on the HP website.
After upgrading, I don't see the hang any more.  Bjorn mentioned that he
hasn't been able to reproduce the problem on his zx2000, probably
because he has the latest firmware.  Please check and let us know.
Thanks,

	Alex

-- 
Alex Williamson                             HP Linux & Open Source Lab

RE: [PATCH] ia64: avoid broken SAL_CACHE_FLUSH implementations

[Luck, Tony]

>What firmware version do you have on your zx2000?  I saw the same
>problem with my zx2000 running firmware 2.21 and was hacking at a work
>around, when I realized version 2.31 is available on the HP website.
>After upgrading, I don't see the hang any more.  Bjorn mentioned that he
>hasn't been able to reproduce the problem on his zx2000, probably
>because he has the latest firmware.  Please check and let us know.

Alex,

Thanks.  I was on f/w 2.21.  Upgrading to 2.31 fixes it, the latest
kernel now boots just fine.

-Tony

RE: [PATCH] ia64: avoid broken SAL_CACHE_FLUSH implementations

[Gerald Pfeifer]

On Thu, 23 Feb 2006, Alex Williamson wrote:
> What firmware version do you have on your zx2000?  I saw the same
> problem with my zx2000 running firmware 2.21 and was hacking at a work
> around, when I realized version 2.31 is available on the HP website.

This sounds *very* familiar!  I've been experiencing similar troubles on 
zx6000, until a colleague helped with a firmware update.

In a nutshell:

  SAL 3.1: HP version 2.31 -- this you want

  SAL 3.1: HP version 2.21 -- this you want to upgrade (though it
                              had serviced me well for two years)

> Please check and let us know.

Yes, I can confirm what you've been experiencing.

Gerald

Re: [PATCH] ia64: avoid broken SAL_CACHE_FLUSH implementations

[Ian Wienand]

[-- Attachment #1: Type: text/plain, Size: 1484 bytes --]

On Mon, Jan 30, 2006 at 03:11:57PM -0700, Bjorn Helgaas wrote:
> +static void __init
> +check_sal_cache_flush (void)
> +{
> +	/*
> +	 * Schedule a timer interrupt, wait until it's reported, and see if
> +	 * SAL_CACHE_FLUSH drops it.
> +	 */
> +	itv = ia64_get_itv();
> +	BUG_ON((itv & (1 << 16)) == 0);
> +
> +	ia64_set_itv(IA64_TIMER_VECTOR);
... [stuff] ...
> +	ia64_set_itv(itv);

Sorry I didn't notice earlier, but that BUG_ON triggers for me on the
simulator.  AFAICS the mask for itv is set in cpu_init(), which comes
after sal_init().  Consequently on the simulator the itv still has its
start value of zero.  I've probably missed something, but I wonder why
at this stage of the boot you even need to save and restore the itv?

-i

Signed-Off-By: Ian Wienand <ianw@gelato.unsw.edu.au>

--- linux-2.6.17-rc3-lvhpt/arch/ia64/kernel/sal.c	2006-03-20 16:53:29.000000000 +1100
+++ linux-2.6.17-rc3/arch/ia64/kernel/sal.c	2006-05-01 17:20:11.000000000 +1000
@@ -227,7 +227,7 @@
 static void __init
 check_sal_cache_flush (void)
 {
-	unsigned long flags, itv;
+	unsigned long flags;
 	int cpu;
 	u64 vector;
 
@@ -238,9 +238,6 @@
 	 * Schedule a timer interrupt, wait until it's reported, and see if
 	 * SAL_CACHE_FLUSH drops it.
 	 */
-	itv = ia64_get_itv();
-	BUG_ON((itv & (1 << 16)) == 0);
-
 	ia64_set_itv(IA64_TIMER_VECTOR);
 	ia64_set_itm(ia64_get_itc() + 1000);
 
@@ -260,7 +257,6 @@
 		ia64_eoi();
 	}
 
-	ia64_set_itv(itv);
 	local_irq_restore(flags);
 	put_cpu();
 }




[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 191 bytes --]

Re: [PATCH] ia64: avoid broken SAL_CACHE_FLUSH implementations

[Robin Holt]

Isn't the right answer here to have the simulator simulate the actual
hardware and have the setup initilize itv to a correct value?

Thanks,
Robin

On Mon, May 01, 2006 at 05:20:55PM +1000, Ian Wienand wrote:
> On Mon, Jan 30, 2006 at 03:11:57PM -0700, Bjorn Helgaas wrote:
> > +static void __init
> > +check_sal_cache_flush (void)
> > +{
> > +	/*
> > +	 * Schedule a timer interrupt, wait until it's reported, and see if
> > +	 * SAL_CACHE_FLUSH drops it.
> > +	 */
> > +	itv = ia64_get_itv();
> > +	BUG_ON((itv & (1 << 16)) = 0);
> > +
> > +	ia64_set_itv(IA64_TIMER_VECTOR);
> ... [stuff] ...
> > +	ia64_set_itv(itv);
> 
> Sorry I didn't notice earlier, but that BUG_ON triggers for me on the
> simulator.  AFAICS the mask for itv is set in cpu_init(), which comes
> after sal_init().  Consequently on the simulator the itv still has its
> start value of zero.  I've probably missed something, but I wonder why
> at this stage of the boot you even need to save and restore the itv?
> 
> -i
> 
> Signed-Off-By: Ian Wienand <ianw@gelato.unsw.edu.au>
> 
> --- linux-2.6.17-rc3-lvhpt/arch/ia64/kernel/sal.c	2006-03-20 16:53:29.000000000 +1100
> +++ linux-2.6.17-rc3/arch/ia64/kernel/sal.c	2006-05-01 17:20:11.000000000 +1000
> @@ -227,7 +227,7 @@
>  static void __init
>  check_sal_cache_flush (void)
>  {
> -	unsigned long flags, itv;
> +	unsigned long flags;
>  	int cpu;
>  	u64 vector;
>  
> @@ -238,9 +238,6 @@
>  	 * Schedule a timer interrupt, wait until it's reported, and see if
>  	 * SAL_CACHE_FLUSH drops it.
>  	 */
> -	itv = ia64_get_itv();
> -	BUG_ON((itv & (1 << 16)) = 0);
> -
>  	ia64_set_itv(IA64_TIMER_VECTOR);
>  	ia64_set_itm(ia64_get_itc() + 1000);
>  
> @@ -260,7 +257,6 @@
>  		ia64_eoi();
>  	}
>  
> -	ia64_set_itv(itv);
>  	local_irq_restore(flags);
>  	put_cpu();
>  }
> 
> 
> 

Re: [PATCH] ia64: avoid broken SAL_CACHE_FLUSH implementations

[Ian Wienand]

[-- Attachment #1: Type: text/plain, Size: 639 bytes --]

On Mon, May 01, 2006 at 04:47:47AM -0500, Robin Holt wrote:
> Isn't the right answer here to have the simulator simulate the actual
> hardware and have the setup initilize itv to a correct value?

Well, as for actual hardware, I read a bit more about it and in SAL
3.2.4 ITV isn't mentioned, and so I guess the initial value is
platform dependent.  Clearly hardware sets it masked, else the code
wouldn't work.

But I don't think this code need bother checking?  It runs on the boot
processor before anything else is happening, and the interrupt is
correctly setup (disabled) when it needs to be for this CPU and others
in cpu_init().

-i

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 191 bytes --]