[patch 1/2] x86, vmi: Fix vmi_get_timer_vector() to use IRQ0_VECTOR

[patch 1/2] x86, vmi: Fix vmi_get_timer_vector() to use IRQ0_VECTOR

[Suresh Siddha]

[-- Attachment #1: fix_vmi_vector.patch --]
[-- Type: text/plain, Size: 1135 bytes --]

FIRST_DEVICE_VECTOR is going away and it looks like a bad hack to steal
FIRST_DEVICE_VECTOR / FIRST_EXTERNAL_VECTOR, when it looks like it needs
IRQ0_VECTOR.

Fix vmi_get_timer_vector() to use IRQ0_VECTOR.

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Alok N Kataria <akataria@vmware.com>
Cc: Zach Amsden <zach@vmware.com>
---
 arch/x86/kernel/vmiclock_32.c |    8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

Index: tip/arch/x86/kernel/vmiclock_32.c
===================================================================
--- tip.orig/arch/x86/kernel/vmiclock_32.c
+++ tip/arch/x86/kernel/vmiclock_32.c
@@ -79,11 +79,7 @@ unsigned long vmi_tsc_khz(void)
 
 static inline unsigned int vmi_get_timer_vector(void)
 {
-#ifdef CONFIG_X86_IO_APIC
-	return FIRST_DEVICE_VECTOR;
-#else
-	return FIRST_EXTERNAL_VECTOR;
-#endif
+	return IRQ0_VECTOR;
 }
 
 /** vmi clockchip */
@@ -239,8 +235,6 @@ void __init vmi_time_init(void)
 
 	vmi_time_init_clockevent();
 	setup_irq(0, &vmi_clock_action);
-	for_each_possible_cpu(cpu)
-		per_cpu(vector_irq, cpu)[vmi_get_timer_vector()] = 0;
 }
 
 #ifdef CONFIG_X86_LOCAL_APIC

[patch 2/2] x86, irq: use 0x20 for the IRQ_MOVE_CLEANUP_VECTOR instead of 0x1f

[Suresh Siddha]

[-- Attachment #1: use_vector_0x20_for_low_priority.patch --]
[-- Type: text/plain, Size: 4784 bytes --]

After talking to some more folks inside intel (Peter Anvin, Asit Mallick),
the safest option (for future compatibility etc) seen was to use vector 0x20
for IRQ_MOVE_CLEANUP_VECTOR instead of using vector 0x1f (which is documented as
reserved vector in the Intel IA32 manuals).

Also we don't need to reserve the entire privilege level (all 16 vectors in
the priority bucket that IRQ_MOVE_CLEANUP_VECTOR falls into), as the
x86 architecture (section 10.9.3 in SDM Vol3a) specifies that with in the
priority level, the higher the vector number the higher the priority.
And hence we don't need to reserve the complete priority level 0x20-0x2f for
the IRQ migration cleanup logic.

So change the IRQ_MOVE_CLEANUP_VECTOR to 0x20 and  allow 0x21-0x2f to be used
for device interrupts. 0x30-0x3f will be used for ISA interrupts (these
also can be migrated in the context of IOAPIC and hence need to be at a higher
priority level than IRQ_MOVE_CLEANUP_VECTOR).

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Yinghai Lu <yinghai@kernel.org>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Maciej W. Rozycki <macro@linux-mips.org>
---
 arch/x86/include/asm/irq_vectors.h |   47 ++++++++++++-------------------------
 arch/x86/kernel/apic/io_apic.c     |    4 +--
 2 files changed, 18 insertions(+), 33 deletions(-)

Index: tip/arch/x86/include/asm/irq_vectors.h
===================================================================
--- tip.orig/arch/x86/include/asm/irq_vectors.h
+++ tip/arch/x86/include/asm/irq_vectors.h
@@ -28,19 +28,22 @@
 #define MCE_VECTOR			0x12
 
 /*
- * IDT vectors usable for external interrupt sources start
- * at 0x20:
- * hpa said we can start from 0x1f.
- *   0x1f is documented as reserved.  However, the ability for the APIC
- *   to generate vectors starting at 0x10 is documented, as is the
- *   ability for the CPU to receive any vector number as an interrupt.
- *   0x1f is used for IRQ_MOVE_CLEANUP_VECTOR since that vector needs
- *   an entire privilege level (16 vectors) all by itself at a higher
- *   priority than any actual device vector.  Thus, by placing it in the
- *   otherwise-unusable 0x10 privilege level, we avoid wasting a full
- *   16-vector block.
+ * IDT vectors usable for external interrupt sources start at 0x20.
+ * (0x80 is the syscall vector, 0x30-0x3f are for ISA)
  */
-#define FIRST_EXTERNAL_VECTOR		0x1f
+#define FIRST_EXTERNAL_VECTOR		0x20
+/*
+ * We start allocating at 0x21 to spread out vectors evenly between
+ * priority levels. (0x80 is the syscall vector)
+ */
+#define VECTOR_OFFSET_START		1
+
+/*
+ * Reserve the lowest usable vector (and hence lowest priority)  0x20 for
+ * triggering cleanup after irq migration. 0x21-0x2f will still be used
+ * for device interrupts.
+ */
+#define IRQ_MOVE_CLEANUP_VECTOR		FIRST_EXTERNAL_VECTOR
 
 #define IA32_SYSCALL_VECTOR		0x80
 #ifdef CONFIG_X86_32
@@ -48,17 +51,7 @@
 #endif
 
 /*
- * Reserve the lowest usable priority level 0x10 - 0x1f for triggering
- * cleanup after irq migration.
- * this overlaps with the reserved range for cpu exceptions so this
- * will need to be changed to 0x20 - 0x2f if the last cpu exception is
- * ever allocated.
- */
-
-#define IRQ_MOVE_CLEANUP_VECTOR		FIRST_EXTERNAL_VECTOR
-
-/*
- * Vectors 0x20-0x2f are used for ISA interrupts.
+ * Vectors 0x30-0x3f are used for ISA interrupts.
  *   round up to the next 16-vector boundary
  */
 #define IRQ0_VECTOR			((FIRST_EXTERNAL_VECTOR + 16) & ~15)
@@ -132,14 +125,6 @@
  */
 #define MCE_SELF_VECTOR			0xeb
 
-/*
- * First APIC vector available to drivers: (vectors 0x30-0xee).  We
- * start allocating at 0x31 to spread out vectors evenly between
- * priority levels. (0x80 is the syscall vector)
- */
-#define FIRST_DEVICE_VECTOR		(IRQ15_VECTOR + 1)
-#define VECTOR_OFFSET_START		1
-
 #define NR_VECTORS			 256
 
 #define FPU_IRQ				  13
Index: tip/arch/x86/kernel/apic/io_apic.c
===================================================================
--- tip.orig/arch/x86/kernel/apic/io_apic.c
+++ tip/arch/x86/kernel/apic/io_apic.c
@@ -1162,7 +1162,7 @@ __assign_irq_vector(int irq, struct irq_
 	 * Also, we've got to be careful not to trash gate
 	 * 0x80, because int 0x80 is hm, kind of importantish. ;)
 	 */
-	static int current_vector = FIRST_DEVICE_VECTOR + VECTOR_OFFSET_START;
+	static int current_vector = FIRST_EXTERNAL_VECTOR + VECTOR_OFFSET_START;
 	static int current_offset = VECTOR_OFFSET_START % 8;
 	unsigned int old_vector;
 	int cpu, err;
@@ -1199,7 +1199,7 @@ next:
 		if (vector >= first_system_vector) {
 			/* If out of vectors on large boxen, must share them. */
 			offset = (offset + 1) % 8;
-			vector = FIRST_DEVICE_VECTOR + offset;
+			vector = FIRST_EXTERNAL_VECTOR + offset;
 		}
 		if (unlikely(current_vector == vector))
 			continue;

[tip:x86/apic] x86, irq: Use 0x20 for the IRQ_MOVE_CLEANUP_VECTOR instead of 0x1f

[tip-bot for Suresh Siddha]

Commit-ID:  6579b474572fd54c583ac074e8e7aaae926c62ef
Gitweb:     http://git.kernel.org/tip/6579b474572fd54c583ac074e8e7aaae926c62ef
Author:     Suresh Siddha <suresh.b.siddha@intel.com>
AuthorDate: Wed, 13 Jan 2010 16:19:11 -0800
Committer:  H. Peter Anvin <hpa@zytor.com>
CommitDate: Mon, 18 Jan 2010 10:59:59 -0800

x86, irq: Use 0x20 for the IRQ_MOVE_CLEANUP_VECTOR instead of 0x1f

After talking to some more folks inside intel (Peter Anvin, Asit Mallick),
the safest option (for future compatibility etc) seen was to use vector 0x20
for IRQ_MOVE_CLEANUP_VECTOR instead of using vector 0x1f (which is documented as
reserved vector in the Intel IA32 manuals).

Also we don't need to reserve the entire privilege level (all 16 vectors in
the priority bucket that IRQ_MOVE_CLEANUP_VECTOR falls into), as the
x86 architecture (section 10.9.3 in SDM Vol3a) specifies that with in the
priority level, the higher the vector number the higher the priority.
And hence we don't need to reserve the complete priority level 0x20-0x2f for
the IRQ migration cleanup logic.

So change the IRQ_MOVE_CLEANUP_VECTOR to 0x20 and  allow 0x21-0x2f to be used
for device interrupts. 0x30-0x3f will be used for ISA interrupts (these
also can be migrated in the context of IOAPIC and hence need to be at a higher
priority level than IRQ_MOVE_CLEANUP_VECTOR).

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
LKML-Reference: <20100114002118.521826763@sbs-t61.sc.intel.com>
Cc: Yinghai Lu <yinghai@kernel.org>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Maciej W. Rozycki <macro@linux-mips.org>
Signed-off-by: H. Peter Anvin <hpa@zytor.com>
---
 arch/x86/include/asm/irq_vectors.h |   47 ++++++++++++-----------------------
 arch/x86/kernel/apic/io_apic.c     |    4 +-
 2 files changed, 18 insertions(+), 33 deletions(-)

diff --git a/arch/x86/include/asm/irq_vectors.h b/arch/x86/include/asm/irq_vectors.h
index 585a428..8767d99 100644
--- a/arch/x86/include/asm/irq_vectors.h
+++ b/arch/x86/include/asm/irq_vectors.h
@@ -28,19 +28,22 @@
 #define MCE_VECTOR			0x12
 
 /*
- * IDT vectors usable for external interrupt sources start
- * at 0x20:
- * hpa said we can start from 0x1f.
- *   0x1f is documented as reserved.  However, the ability for the APIC
- *   to generate vectors starting at 0x10 is documented, as is the
- *   ability for the CPU to receive any vector number as an interrupt.
- *   0x1f is used for IRQ_MOVE_CLEANUP_VECTOR since that vector needs
- *   an entire privilege level (16 vectors) all by itself at a higher
- *   priority than any actual device vector.  Thus, by placing it in the
- *   otherwise-unusable 0x10 privilege level, we avoid wasting a full
- *   16-vector block.
+ * IDT vectors usable for external interrupt sources start at 0x20.
+ * (0x80 is the syscall vector, 0x30-0x3f are for ISA)
  */
-#define FIRST_EXTERNAL_VECTOR		0x1f
+#define FIRST_EXTERNAL_VECTOR		0x20
+/*
+ * We start allocating at 0x21 to spread out vectors evenly between
+ * priority levels. (0x80 is the syscall vector)
+ */
+#define VECTOR_OFFSET_START		1
+
+/*
+ * Reserve the lowest usable vector (and hence lowest priority)  0x20 for
+ * triggering cleanup after irq migration. 0x21-0x2f will still be used
+ * for device interrupts.
+ */
+#define IRQ_MOVE_CLEANUP_VECTOR		FIRST_EXTERNAL_VECTOR
 
 #define IA32_SYSCALL_VECTOR		0x80
 #ifdef CONFIG_X86_32
@@ -48,17 +51,7 @@
 #endif
 
 /*
- * Reserve the lowest usable priority level 0x10 - 0x1f for triggering
- * cleanup after irq migration.
- * this overlaps with the reserved range for cpu exceptions so this
- * will need to be changed to 0x20 - 0x2f if the last cpu exception is
- * ever allocated.
- */
-
-#define IRQ_MOVE_CLEANUP_VECTOR		FIRST_EXTERNAL_VECTOR
-
-/*
- * Vectors 0x20-0x2f are used for ISA interrupts.
+ * Vectors 0x30-0x3f are used for ISA interrupts.
  *   round up to the next 16-vector boundary
  */
 #define IRQ0_VECTOR			((FIRST_EXTERNAL_VECTOR + 16) & ~15)
@@ -132,14 +125,6 @@
  */
 #define MCE_SELF_VECTOR			0xeb
 
-/*
- * First APIC vector available to drivers: (vectors 0x30-0xee).  We
- * start allocating at 0x31 to spread out vectors evenly between
- * priority levels. (0x80 is the syscall vector)
- */
-#define FIRST_DEVICE_VECTOR		(IRQ15_VECTOR + 1)
-#define VECTOR_OFFSET_START		1
-
 #define NR_VECTORS			 256
 
 #define FPU_IRQ				  13
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index e9ba090..409f494 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1162,7 +1162,7 @@ __assign_irq_vector(int irq, struct irq_cfg *cfg, const struct cpumask *mask)
 	 * Also, we've got to be careful not to trash gate
 	 * 0x80, because int 0x80 is hm, kind of importantish. ;)
 	 */
-	static int current_vector = FIRST_DEVICE_VECTOR + VECTOR_OFFSET_START;
+	static int current_vector = FIRST_EXTERNAL_VECTOR + VECTOR_OFFSET_START;
 	static int current_offset = VECTOR_OFFSET_START % 8;
 	unsigned int old_vector;
 	int cpu, err;
@@ -1199,7 +1199,7 @@ next:
 		if (vector >= first_system_vector) {
 			/* If out of vectors on large boxen, must share them. */
 			offset = (offset + 1) % 8;
-			vector = FIRST_DEVICE_VECTOR + offset;
+			vector = FIRST_EXTERNAL_VECTOR + offset;
 		}
 		if (unlikely(current_vector == vector))
 			continue;

Re: [patch 2/2] x86, irq: use 0x20 for the IRQ_MOVE_CLEANUP_VECTOR instead of 0x1f

[Maciej W. Rozycki]

On Wed, 13 Jan 2010, Suresh Siddha wrote:

> After talking to some more folks inside intel (Peter Anvin, Asit Mallick),
> the safest option (for future compatibility etc) seen was to use vector 0x20
> for IRQ_MOVE_CLEANUP_VECTOR instead of using vector 0x1f (which is documented as
> reserved vector in the Intel IA32 manuals).
> 
> Also we don't need to reserve the entire privilege level (all 16 vectors in
> the priority bucket that IRQ_MOVE_CLEANUP_VECTOR falls into), as the
> x86 architecture (section 10.9.3 in SDM Vol3a) specifies that with in the
> priority level, the higher the vector number the higher the priority.
> And hence we don't need to reserve the complete priority level 0x20-0x2f for
> the IRQ migration cleanup logic.
> 
> So change the IRQ_MOVE_CLEANUP_VECTOR to 0x20 and  allow 0x21-0x2f to be used
> for device interrupts. 0x30-0x3f will be used for ISA interrupts (these
> also can be migrated in the context of IOAPIC and hence need to be at a higher
> priority level than IRQ_MOVE_CLEANUP_VECTOR).

 I have troubles understanding what exactly this change is needed for 
(i.e. what's the difference between using vectors 0x20-0x2f and 0x30-0x3f 
as ExtINT interrupts, what's the gain from relocating them? -- they are 
transparent to the APIC, so the exact priority level used does not matter 
at all), but since I've been cc-ed, I have one question -- have you 
verified that with the new arrangement the mixed interrupt mode (where 
some interrupts come via the APIC and some via the 8259A PICs) still 
works?

  Maciej

Re: [patch 2/2] x86, irq: use 0x20 for the IRQ_MOVE_CLEANUP_VECTOR instead of 0x1f

[H. Peter Anvin]

On 01/23/2010 09:52 PM, Maciej W. Rozycki wrote:
> On Wed, 13 Jan 2010, Suresh Siddha wrote:
>
>> After talking to some more folks inside intel (Peter Anvin, Asit Mallick),
>> the safest option (for future compatibility etc) seen was to use vector 0x20
>> for IRQ_MOVE_CLEANUP_VECTOR instead of using vector 0x1f (which is documented as
>> reserved vector in the Intel IA32 manuals).
>>
>> Also we don't need to reserve the entire privilege level (all 16 vectors in
>> the priority bucket that IRQ_MOVE_CLEANUP_VECTOR falls into), as the
>> x86 architecture (section 10.9.3 in SDM Vol3a) specifies that with in the
>> priority level, the higher the vector number the higher the priority.
>> And hence we don't need to reserve the complete priority level 0x20-0x2f for
>> the IRQ migration cleanup logic.
>>
>> So change the IRQ_MOVE_CLEANUP_VECTOR to 0x20 and  allow 0x21-0x2f to be used
>> for device interrupts. 0x30-0x3f will be used for ISA interrupts (these
>> also can be migrated in the context of IOAPIC and hence need to be at a higher
>> priority level than IRQ_MOVE_CLEANUP_VECTOR).
>
>   I have troubles understanding what exactly this change is needed for
> (i.e. what's the difference between using vectors 0x20-0x2f and 0x30-0x3f
> as ExtINT interrupts, what's the gain from relocating them? -- they are
> transparent to the APIC, so the exact priority level used does not matter
> at all), but since I've been cc-ed, I have one question -- have you
> verified that with the new arrangement the mixed interrupt mode (where
> some interrupts come via the APIC and some via the 8259A PICs) still
> works?
>

The difference is relevant when they are *not* invoked as ExtInt 
interrupts, but when used as IOAPIC interrupts it matters.

	-hpa

Re: [patch 2/2] x86, irq: use 0x20 for the IRQ_MOVE_CLEANUP_VECTOR instead of 0x1f

[Maciej W. Rozycki]

On Sun, 24 Jan 2010, H. Peter Anvin wrote:

> > > So change the IRQ_MOVE_CLEANUP_VECTOR to 0x20 and  allow 0x21-0x2f to be
> > > used
> > > for device interrupts. 0x30-0x3f will be used for ISA interrupts (these
> > > also can be migrated in the context of IOAPIC and hence need to be at a
> > > higher
> > > priority level than IRQ_MOVE_CLEANUP_VECTOR).
> > 
> >   I have troubles understanding what exactly this change is needed for
> > (i.e. what's the difference between using vectors 0x20-0x2f and 0x30-0x3f
> > as ExtINT interrupts, what's the gain from relocating them? -- they are
> > transparent to the APIC, so the exact priority level used does not matter
> > at all), but since I've been cc-ed, I have one question -- have you
> > verified that with the new arrangement the mixed interrupt mode (where
> > some interrupts come via the APIC and some via the 8259A PICs) still
> > works?
> > 
> 
> The difference is relevant when they are *not* invoked as ExtInt interrupts,
> but when used as IOAPIC interrupts it matters.

 Hmm, I/O APIC interrupts coming from ISA devices used not to differ from 
ones from PCI devices and their vectors were evenly distributed across the 
whole device range (one reason for this was the (in)famous Pentium APIC 
limitation WRT multiple outstanding requests at the same priority level).  
Now what you've written suggests this has changed and now ISA devices only 
get vectors within a single priority level -- am I getting this right?

 If so, then to push my original question further: how are these vectors 
allocated -- are they identity mapped with the corresponding i8259A 
vectors?  And how does it play with the Pentium APIC limitation (that may 
actually apply to all the local APIC cores that use serial bus delivery; 
I'm not sure) I mentioned above?

  Maciej

Re: [patch 2/2] x86, irq: use 0x20 for the IRQ_MOVE_CLEANUP_VECTOR instead of 0x1f

[Suresh Siddha]

On Sat, 2010-01-30 at 23:19 -0800, Maciej W. Rozycki wrote:
> On Sun, 24 Jan 2010, H. Peter Anvin wrote:
> 
> > > > So change the IRQ_MOVE_CLEANUP_VECTOR to 0x20 and  allow 0x21-0x2f to be
> > > > used
> > > > for device interrupts. 0x30-0x3f will be used for ISA interrupts (these
> > > > also can be migrated in the context of IOAPIC and hence need to be at a
> > > > higher
> > > > priority level than IRQ_MOVE_CLEANUP_VECTOR).
> > > 
> > >   I have troubles understanding what exactly this change is needed for
> > > (i.e. what's the difference between using vectors 0x20-0x2f and 0x30-0x3f
> > > as ExtINT interrupts, what's the gain from relocating them? -- they are
> > > transparent to the APIC, so the exact priority level used does not matter
> > > at all), but since I've been cc-ed, I have one question -- have you
> > > verified that with the new arrangement the mixed interrupt mode (where
> > > some interrupts come via the APIC and some via the 8259A PICs) still
> > > works?
> > > 
> > 
> > The difference is relevant when they are *not* invoked as ExtInt interrupts,
> > but when used as IOAPIC interrupts it matters.
> 
>  Hmm, I/O APIC interrupts coming from ISA devices used not to differ from 
> ones from PCI devices and their vectors were evenly distributed across the 
> whole device range (one reason for this was the (in)famous Pentium APIC 
> limitation WRT multiple outstanding requests at the same priority level).  
> Now what you've written suggests this has changed and now ISA devices only 
> get vectors within a single priority level -- am I getting this right?

Even before the current changes (2.6.32 for example), for irq0 to irq15,
irrespective of whether those irq's were handled by PIC or an IO-APIC,
we were using 16 vectors with in a single priority level for these
irq's.

Looking at the changelog, it looks this issue happened when we merged
io_apic.c for 32-bit and 64-bit. 2.6.28 and beyond uses the same
priority level for irq0..irq15

32bit kernels 2.6.27 and before has the behavior of evenly distributed
vectors for all the io-apic irq's (legacy or non-legacy) but starting
from 2.6.28, only for non-legacy irq's (16 and above) we try to spread
the vectors uniformly across priority levels.

>  If so, then to push my original question further: how are these vectors 
> allocated -- are they identity mapped with the corresponding i8259A 
> vectors?  And how does it play with the Pentium APIC limitation (that may 
> actually apply to all the local APIC cores that use serial bus delivery; 
> I'm not sure) I mentioned above?

As we are using the code from 2.6.28 and no one noticed/complained about
this issue for more than 1.5 years, probably the pentium APIC issue is
not wide-spread.

If we care about this, I can post a fix (which is needed irrespective of
the current changes). 

thanks,
suresh

Re: [patch 2/2] x86, irq: use 0x20 for the IRQ_MOVE_CLEANUP_VECTOR instead of 0x1f

[H. Peter Anvin]

On 02/01/2010 01:24 PM, Suresh Siddha wrote:
> 
> As we are using the code from 2.6.28 and no one noticed/complained about
> this issue for more than 1.5 years, probably the pentium APIC issue is
> not wide-spread.
> 

I *think* it's applicable to all CPUs Pentium III or earlier (but not
Pentium 4 -- I'm unsure about the Pentium M.)  I don't know about
non-Intel CPUs; I have a vague memory of the Transmeta Efficeon (the
only Transmeta chip with an APIC) *not* having this limitation.

The exact reference is SDM vol 3A 10.8.4, page 10-41 [rev 033US Dec 2009]:

For the P6 family and Pentium processors, the IRR and ISR registers can
queue no more than two interrupts per priority level, and will reject
other interrupts that are received within the same priority level.

However, section 10.8.2 bullet 3 on page 10-38 (and the flowchart on
page 10-37) indicate that such an interrupt is returned to the IOAPIC
for a later retry, i.e. it's not lost.  As such, it's not clear to me
from reading the SDM that there is actually a problem here...

	-hpa

Re: [patch 2/2] x86, irq: use 0x20 for the IRQ_MOVE_CLEANUP_VECTOR instead of 0x1f

[Maciej W. Rozycki]

Hi,

 I have finally managed to get back to it -- sorry for the delay, I'm 
running out of my time.

On Mon, 1 Feb 2010, H. Peter Anvin wrote:

> > As we are using the code from 2.6.28 and no one noticed/complained about
> > this issue for more than 1.5 years, probably the pentium APIC issue is
> > not wide-spread.

 Correct, the problem only affected B1, B3 and B5 steppings of the P54C 
Pentium processor.  These are probably extremely rare these days.  It was 
fixed later on.

 But they can be run-time detected -- if we don't support them anymore 
(assuming keeping them supported is too much of maintenance hassle; Linux 
used to be proud to support hardware nobody else seemed to care of 
anymore, so it's really disappointing to see it go), we should panic() on 
bootstrap and print an appropriate message.  They are CPUID family 5, 
model 2 and steppings 1, 2 and 4, respectively.

 Also the note in arch/x86/kernel/smp.c should be adjusted accordingly 
stating that the erratum is no longer worked around (preferably stating 
the last Linux version it was).

> I *think* it's applicable to all CPUs Pentium III or earlier (but not
> Pentium 4 -- I'm unsure about the Pentium M.)  I don't know about
> non-Intel CPUs; I have a vague memory of the Transmeta Efficeon (the
> only Transmeta chip with an APIC) *not* having this limitation.
> 
> The exact reference is SDM vol 3A 10.8.4, page 10-41 [rev 033US Dec 2009]:
> 
> For the P6 family and Pentium processors, the IRR and ISR registers can
> queue no more than two interrupts per priority level, and will reject
> other interrupts that are received within the same priority level.
> 
> However, section 10.8.2 bullet 3 on page 10-38 (and the flowchart on
> page 10-37) indicate that such an interrupt is returned to the IOAPIC
> for a later retry, i.e. it's not lost.  As such, it's not clear to me
> from reading the SDM that there is actually a problem here...

 Here's the text of the relevant erratum:

 "4AP. Three Interrupts of the Same Priority Causes Lost Local Interrupt

PROBLEM: If three interrupts of the same priority level (priority is 
defined in the 4MSB of the interrupt vector), arrive in the following 
circumstance:

1. A interrupt is being serviced by the CPU, and the proper bit is set in 
   the ISR register.

2. A second interrupt is received from the serial bus.

3. At the same time a third interrupt is received from a local interrupt 
   source, which could include local pins (LVT), an APIC timer (Timer), 
   self-interrupt, or an APIC error interrupt.

If the first two conditions are met the third interrupt will be lost, and 
not serviced.

IMPLICATION: The third interrupt will be ignored and not serviced if the 
specific scenario happens as listed above.

WORKAROUND: The problem can be avoided if different priority levels are 
assigned to serial interrupts, than to local interrupts.

STATUS: For the steppings affected see the Summary Table of Changes at the 
beginning of this section."

so you can see the retry mechanism is not the problem here (or, to be 
exact, the lack of an equivalent for local interrupts seems to be).

 I'm not sure how fatal for Linux the implications are though; even then 
it looks to me the approach we took was an overkill.  It's enough to 
guarantee that the APIC error interrupt, the APIC timer interrupt and 
self-IPIs (do we use any at all though?) do not share their priority 
level(s) with any external interrupt (but they can share the level with 
one another).  We only use ever LINT0/1 interrupts as NMIs (for the NMI 
watchdog and the system error, respectively), or ExtINT (in the case of 
LINT0), so this erratum does not apply to them.

 So what priority level(s) do we use for the APIC error and timer 
interrupts (and self-IPIs, if any) these days and how does it correspond 
to the priorities of external interrupts?  It looks like we can work 
around this erratum indefinitely quite cheaply (and should document it 
decently so that newcomers do not break it like it happened with many bits 
in our APIC code many times already; yes, lost hope, I know...).

  Maciej

Re: [patch 2/2] x86, irq: use 0x20 for the IRQ_MOVE_CLEANUP_VECTOR instead of 0x1f

[H. Peter Anvin]

On 02/20/2010 09:20 PM, Maciej W. Rozycki wrote:
> 
>  Correct, the problem only affected B1, B3 and B5 steppings of the P54C 
> Pentium processor.  These are probably extremely rare these days.  It was 
> fixed later on.
> 
>  But they can be run-time detected -- if we don't support them anymore 
> (assuming keeping them supported is too much of maintenance hassle; Linux 
> used to be proud to support hardware nobody else seemed to care of 
> anymore, so it's really disappointing to see it go), we should panic() on 
> bootstrap and print an appropriate message.  They are CPUID family 5, 
> model 2 and steppings 1, 2 and 4, respectively.
> 
>  Also the note in arch/x86/kernel/smp.c should be adjusted accordingly 
> stating that the erratum is no longer worked around (preferably stating 
> the last Linux version it was).
> 

My philosophy is generally that I'm happy to keep old hardware (that
actually exist in any kind of meaningful quantity) alive, but I'm not
willing to go through herculean efforts nor willing to make widely
available modern hardware suck over it.

It looks like this really isn't too hard to deal with, though.

>  I'm not sure how fatal for Linux the implications are though; even then 
> it looks to me the approach we took was an overkill.  It's enough to 
> guarantee that the APIC error interrupt, the APIC timer interrupt and 
> self-IPIs (do we use any at all though?) do not share their priority 
> level(s) with any external interrupt (but they can share the level with 
> one another).  We only use ever LINT0/1 interrupts as NMIs (for the NMI 
> watchdog and the system error, respectively), or ExtINT (in the case of 
> LINT0), so this erratum does not apply to them.

The APIC error is on vector 0xfe, the APIC timer is on vector 0xef, and
self IPI (vector 0xeb) we only use for MCA, which wouldn't be supported
on these processors.

However, these are mixed with externally-generated IPIs which will be
seen as serial interrupts: in particular 0xf0-0xfd are all different IPI
which share with the error vector 0xde, and 0xeb shares with 0xed is
used for "platform" IPIs.

It sounds like the right solution for supporting these processors would
be to reshuffle the special vectors so that we use one level (presumably
0xfx) for locally generated interrupts and one (presumably 0xex) for
external IPIs, and make sure that it is not possible for external
interrupts to get assigned to the local-only level.  The assignment of
external interrupts, which seems to be where focus has been in the past,
is actually irrelevant (but might still be good for performance, by
maximizing the number of interrupts that can be held in the LAPIC and
not bounced.)

Either way, this doesn't exactly sound too bad.  A bigger question is if
we want to do this globally or end up having different vector
assignments for these oddball CPUs.  Testing it, too, will be almost
impossible...

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [patch 2/2] x86, irq: use 0x20 for the IRQ_MOVE_CLEANUP_VECTOR instead of 0x1f

[Alan Cox]

> Either way, this doesn't exactly sound too bad.  A bigger question is if
> we want to do this globally or end up having different vector
> assignments for these oddball CPUs.  Testing it, too, will be almost
> impossible...

If you panic it won't get tested. If its done (even as 'experimental' then
it'll either get tested or not matter). They also have relatively few IRQ
sources by modern standards so there isn't a lot to map.

The bigger problem with the old processors is that some Pentium era SMP
boards actually get material numbers of repeated interrupts where one IPI
arrives twice on another CPU. That's something present in a lot of the
later chips (pre P4 I believe but don't quote me on that) but very
unusual to observe except on a few specific boards (eg the BP6))


Alan

[tip:x86/apic] x86, vmi: Fix vmi_get_timer_vector() to use IRQ0_VECTOR

[tip-bot for Suresh Siddha]

Commit-ID:  722b3654852e48b93367a63f8ada9ee1cd43f2d3
Gitweb:     http://git.kernel.org/tip/722b3654852e48b93367a63f8ada9ee1cd43f2d3
Author:     Suresh Siddha <suresh.b.siddha@intel.com>
AuthorDate: Wed, 13 Jan 2010 16:19:10 -0800
Committer:  H. Peter Anvin <hpa@zytor.com>
CommitDate: Mon, 18 Jan 2010 10:59:50 -0800

x86, vmi: Fix vmi_get_timer_vector() to use IRQ0_VECTOR

FIRST_DEVICE_VECTOR is going away and it looks like a bad hack to steal
FIRST_DEVICE_VECTOR / FIRST_EXTERNAL_VECTOR, when it looks like it needs
IRQ0_VECTOR.

Fix vmi_get_timer_vector() to use IRQ0_VECTOR.

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
LKML-Reference: <20100114002118.436172066@sbs-t61.sc.intel.com>
Cc: Alok N Kataria <akataria@vmware.com>
Cc: Zach Amsden <zach@vmware.com>
Signed-off-by: H. Peter Anvin <hpa@zytor.com>
---
 arch/x86/kernel/vmiclock_32.c |    8 +-------
 1 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/vmiclock_32.c b/arch/x86/kernel/vmiclock_32.c
index 74c92bb..1268d99 100644
--- a/arch/x86/kernel/vmiclock_32.c
+++ b/arch/x86/kernel/vmiclock_32.c
@@ -79,11 +79,7 @@ unsigned long vmi_tsc_khz(void)
 
 static inline unsigned int vmi_get_timer_vector(void)
 {
-#ifdef CONFIG_X86_IO_APIC
-	return FIRST_DEVICE_VECTOR;
-#else
-	return FIRST_EXTERNAL_VECTOR;
-#endif
+	return IRQ0_VECTOR;
 }
 
 /** vmi clockchip */
@@ -239,8 +235,6 @@ void __init vmi_time_init(void)
 
 	vmi_time_init_clockevent();
 	setup_irq(0, &vmi_clock_action);
-	for_each_possible_cpu(cpu)
-		per_cpu(vector_irq, cpu)[vmi_get_timer_vector()] = 0;
 }
 
 #ifdef CONFIG_X86_LOCAL_APIC