Skip to contentSkip to conversation

linux: prevent invalid or unsupportable PIRQs from being used

PATCH In review Jan Beulich 2 participants 6 messages Nov 5, 2008 Xen hypervisor interface

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 linux: prevent invalid or unsupportable PIRQs from being used

By keeping the respective irq_desc[] entries pointing to no_irq_type, setup_irq() (and thus request_irq()) will fail for such IRQs. This matches native behavior, which also only installs ioapic_*_type out of ioapic_register_intr().

At the same time, make assign_irq_vector() fail not only when Xen doesn't support the PIRQ, but also if the IRQ requested doesn't fall in the kernel's PIRQ space.

As usual, written and tested on 2.6.27.4 and made apply to the 2.6.18 tree without further testing.

arch/i386/kernel/io_apic-xen.c +3 -0
Index: head-2008-11-04/arch/i386/kernel/io_apic-xen.c
===================================================================
@@ -1229,6 +1229,9 @@ static int __assign_irq_vector(int irq)
 
 	BUG_ON(irq != AUTO_ASSIGN && (unsigned)irq >= NR_IRQ_VECTORS);
 
+	if (irq < PIRQ_BASE || irq - PIRQ_BASE > NR_PIRQS)
+		return -EINVAL;
+
 	spin_lock_irqsave(&vector_lock, flags);
 
 	if (irq != AUTO_ASSIGN && IO_APIC_VECTOR(irq) > 0) {
arch/x86_64/kernel/io_apic-xen.c +3 -0
Index: head-2008-11-04/arch/x86_64/kernel/io_apic-xen.c
===================================================================
@@ -855,6 +855,9 @@ static int __assign_irq_vector(int irq, 
   
   	BUG_ON(irq != AUTO_ASSIGN && (unsigned)irq >= NR_IRQ_VECTORS);
 
+	if (irq < PIRQ_BASE || irq - PIRQ_BASE > NR_PIRQS)
+		return -EINVAL;
+
 	spin_lock_irqsave(&vector_lock, flags);
 
   	if (irq != AUTO_ASSIGN && IO_APIC_VECTOR(irq) > 0) {
drivers/xen/core/evtchn.c +66 -69

159 lines not on this page · raw message

#1 lore raw
Quoted 8 lines · Jan Beulich

By keeping the respective irq_desc[] entries pointing to no_irq_type, setup_irq() (and thus request_irq()) will fail for such IRQs. This matches native behavior, which also only installs ioapic_*_type out of ioapic_register_intr().

At the same time, make assign_irq_vector() fail not only when Xen doesn't support the PIRQ, but also if the IRQ requested doesn't fall in the kernel's PIRQ space.

Reverted as it breaks the build non-trivially (at least: no set_irq_chip_and_handler_name(); no pirq_chip structure). The 2.6.18 isn't really the place for big code restructurings anyway (this was a big patch).

-- Keir

#2 lore raw

Re: [PATCH] linux: prevent invalid or unsupportablePIRQs from being used

Quoted 8 lines
Quoted 1 line
Quoted 1 line

Keir Fraser <[email protected]> 05.11.08 15:54 >>>

Quoted 5 lines · Jan Beulich

By keeping the respective irq_desc[] entries pointing to no_irq_type, setup_irq() (and thus request_irq()) will fail for such IRQs. This matches native behavior, which also only installs ioapic_*_type out of ioapic_register_intr().

Quoted 7 lines
Quoted 3 lines

At the same time, make assign_irq_vector() fail not only when Xen doesn't support the PIRQ, but also if the IRQ requested doesn't fall in the kernel's PIRQ space.

Reverted as it breaks the build non-trivially (at least: no set_irq_chip_and_handler_name(); no pirq_chip structure). The 2.6.18 isn't really the place for big code restructurings anyway (this was a big patch).

The size of the patch only comes from moving some code down in evtchn.c (in an attempt to avoid forward declarations). The actual changes aren't that big, and from what you write I conclude I just messed up a single line (or better - forgot to convert it down to 2.6.18 terms) - I'm sorry for that. Below is a fixed patch, in case you want to give this another try.

The point of sending the patch is that it fixes a bug you'd certainly also see in that tree (see my other follow-up mail). Plus it puts on more consistent feet the suppressing of undue auto-probing - the first solution to this was somewhat hackish in my opinion.

Jan

*******************************************************

By keeping the respective irq_desc[] entries pointing to no_irq_type, setup_irq() (and thus request_irq()) will fail for such IRQs. This matches native behavior, which also only installs ioapic_*_type out of ioapic_register_intr().

At the same time, make assign_irq_vector() fail not only when Xen doesn't support the PIRQ, but also if the IRQ requested doesn't fall in the kernel's PIRQ space.

As usual, written and tested on 2.6.27.4 and made apply to the 2.6.18 tree without further testing.

arch/i386/kernel/io_apic-xen.c +3 -0
Index: head-2008-11-04/arch/i386/kernel/io_apic-xen.c
===================================================================
@@ -1229,6 +1229,9 @@ static int __assign_irq_vector(int irq)
 
 	BUG_ON(irq != AUTO_ASSIGN && (unsigned)irq >= NR_IRQ_VECTORS);
 
+	if (irq < PIRQ_BASE || irq - PIRQ_BASE > NR_PIRQS)
+		return -EINVAL;
+
 	spin_lock_irqsave(&vector_lock, flags);
 
 	if (irq != AUTO_ASSIGN && IO_APIC_VECTOR(irq) > 0) {
arch/x86_64/kernel/io_apic-xen.c +3 -0
Index: head-2008-11-04/arch/x86_64/kernel/io_apic-xen.c
===================================================================
@@ -855,6 +855,9 @@ static int __assign_irq_vector(int irq, 
   
   	BUG_ON(irq != AUTO_ASSIGN && (unsigned)irq >= NR_IRQ_VECTORS);
 
+	if (irq < PIRQ_BASE || irq - PIRQ_BASE > NR_PIRQS)
+		return -EINVAL;
+
 	spin_lock_irqsave(&vector_lock, flags);
 
   	if (irq != AUTO_ASSIGN && IO_APIC_VECTOR(irq) > 0) {
drivers/xen/core/evtchn.c +66 -69

159 lines not on this page · raw message

#3 lore raw

Re: [PATCH] linux: prevent invalid or unsupportable PIRQsfrom being used

In reply to Jan Beulich
Quoted 9 lines
Quoted 1 line
Quoted 1 line

"Jan Beulich" <[email protected]> 05.11.08 15:13 >>>

By keeping the respective irq_desc[] entries pointing to no_irq_type, setup_irq() (and thus request_irq()) will fail for such IRQs. This matches native behavior, which also only installs ioapic_*_type out of ioapic_register_intr().

At the same time, make assign_irq_vector() fail not only when Xen doesn't support the PIRQ, but also if the IRQ requested doesn't fall in the kernel's PIRQ space.

I wrote this in response to a bug report from IBM where on a 4-node box they would see crashes when trying to request_irq() with an irq greater or equal to 256 (the box has 387 GSIs). The patch is assumed to fix that crash, but of course any device at this high an IRQ (and even at ones slightly below 256, in case MSI took up some of them) will not work, due to Xen refusing to map a vector for it.

Clearly, growing NR_IRQS will be necessary here, perhaps based on Linux' x86-64 scheme of assigning IRQs and their vectors on a per-CPU basis. Are there any plans in that direction?

Jan

P.S. The good news from that report from IBM is that they were able to bring up 96 processors.

#4 lore raw

Re: [PATCH] linux: prevent invalid or unsupportable PIRQsfrom being used

Quoted 3 lines · Jan Beulich

Clearly, growing NR_IRQS will be necessary here, perhaps based on Linux' x86-64 scheme of assigning IRQs and their vectors on a per-CPU basis. Are there any plans in that direction?

Not for 2.6.18. Perhaps for pv_ops, if latest kernels don't have such functionality already. Remember we're obsoleting the 2.6.18 tree before Xen 3.4, assuming Jeremy gets the dom0 stuff worked out by then (and there's no reason to think he won't).

-- Keir

#5 lore raw

Re: [PATCH] linux: prevent invalid or unsupportablePIRQsfrom being used

Quoted 11 lines
Quoted 1 line
Quoted 1 line

Keir Fraser <[email protected]> 05.11.08 16:26 >>>

Quoted 3 lines · Jan Beulich

Clearly, growing NR_IRQS will be necessary here, perhaps based on Linux' x86-64 scheme of assigning IRQs and their vectors on a per-CPU basis. Are there any plans in that direction?

Not for 2.6.18. Perhaps for pv_ops, if latest kernels don't have such functionality already. Remember we're obsoleting the 2.6.18 tree before Xen 3.4, assuming Jeremy gets the dom0 stuff worked out by then (and there's no reason to think he won't).

Oh, I meant NR_IRQS in Xen, not in Linux. I referred to Linux only because it already has a solution that likely could be leveraged.

Jan