x86: Reserve FIRST_DEVICE_VECTOR in used_vectors bitmap

PATCH In review Stefan Bader 5 participants 16 messages Oct 10, 2008 x86

lore

Share & tools

lore

Stefan Bader

author

2008-10-10 20:02 UTC #0 lore raw

PATCH 1/1 x86: Reserve FIRST_DEVICE_VECTOR in used_vectors bitmap

Affected: 2.6.24-2.6.27

Someone from the community found out, that after repeatedly unloading and loading a device driver that uses MSI IRQs, the system eventually assigned the vector initially reserved for IRQ0 to the device driver.

The reason for this is, that although IRQ0 is tied to the FIRST_DEVICE_VECTOR when declaring the irq_vector table, the corresponding bit in the used_vectors map is not set. So, if vectors are released and assigned often enough, the vector will get assigned to another interrupt. This happens more often with MSI interrupts as those are exclusively using a vector.

Fix this by setting the bit for the FIRST_DEVICE_VECTOR in the bitmap.

Signed-off-by Stefan Bader <[email protected]>

Signature · 1 line

When all other means of communication fail, try words!

From a272a28f520c58a84853e098b4ee565486ca5062 Mon Sep 17 00:00:00 2001
From: Stefan Bader <[email protected]>
Date: Sat, 27 Sep 2008 11:07:30 -0400
Subject: [PATCH] x86: Reserve FIRST_DEVICE_VECTOR in used_vectors bitmap.

Fix this by setting the bit for the FIRST_DEVICE_VECTOR in the bitmap.

Signed-off-by Stefan Bader <[email protected]>

---
 arch/x86/kernel/io_apic_32.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

arch/x86/kernel/io_apic_32.c +3 -0

index 09cddb5..bfd9fc5 100644
@@ -2314,6 +2314,9 @@ void __init setup_IO_APIC(void)
 	for (i = first_system_vector; i < NR_VECTORS; i++)
 		set_bit(i, used_vectors);
 
+	/* Mark FIRST_DEVICE_VECTOR which is assigned to IRQ0 as used. */
+	set_bit(FIRST_DEVICE_VECTOR, used_vectors);
+
 	enable_IO_APIC();
 
 	io_apic_irqs = ~PIC_IRQS;

Signature · 1 line

1.5.4.3

Greg KH

2008-10-10 20:19 UTC #1 lore raw

Quoted 12 lines · Stefan Bader

Affected: 2.6.24-2.6.27
Someone from the community found out, that after repeatedly unloading and loading a device driver that uses MSI IRQs, the system eventually assigned the vector initially reserved for IRQ0 to the device driver.
The reason for this is, that although IRQ0 is tied to the FIRST_DEVICE_VECTOR when declaring the irq_vector table, the corresponding bit in the used_vectors map is not set. So, if vectors are released and assigned often enough, the vector will get assigned to another interrupt. This happens more often with MSI interrupts as those are exclusively using a vector.

Quoted 3 lines

Fix this by setting the bit for the FIRST_DEVICE_VECTOR in the bitmap.
Signed-off-by Stefan Bader <[email protected]>

Hint, if you want patches to go into the -stable tree, just add:
cc: Stable <[email protected]>
to the patch when you submit it in the signed-off-by area, and it will be automatically sent to us when it goes into the main kernel tree.

That makes it easier for the stable team, as we don't have to track things "by hand" to determine when and if a patch does go into Linus's tree.

So if you can, please do this in the future.

thanks,

greg k-h

Ingo Molnar

Acked-by

2008-10-10 20:24 UTC #2 lore raw

Quoted 13 lines · Greg KH

Quoted 12 lines · Stefan Bader
Affected: 2.6.24-2.6.27
Someone from the community found out, that after repeatedly unloading and loading a device driver that uses MSI IRQs, the system eventually assigned the vector initially reserved for IRQ0 to the device driver.
The reason for this is, that although IRQ0 is tied to the FIRST_DEVICE_VECTOR when declaring the irq_vector table, the corresponding bit in the used_vectors map is not set. So, if vectors are released and assigned often enough, the vector will get assigned to another interrupt. This happens more often with MSI interrupts as those are exclusively using a vector.

Quoted 8 lines

Quoted 3 lines
Fix this by setting the bit for the FIRST_DEVICE_VECTOR in the bitmap.
Signed-off-by Stefan Bader <[email protected]>
Hint, if you want patches to go into the -stable tree, just add:
cc: Stable <[email protected]>
to the patch when you submit it in the signed-off-by area, and it will be automatically sent to us when it goes into the main kernel tree.

yes. Note that this is a special case, as there will be no upstream commit to tag with Cc: <[email protected]>, because this bug got eliminated via not backportable means: APIC code unification.

Acked-by Ingo Molnar <[email protected]>

Ingo

Greg KH

2008-10-10 20:32 UTC #3 lore raw

Quoted 25 lines · Ingo Molnar

Quoted 13 lines · Greg KH
Quoted 12 lines · Stefan Bader
Affected: 2.6.24-2.6.27
Someone from the community found out, that after repeatedly unloading and loading a device driver that uses MSI IRQs, the system eventually assigned the vector initially reserved for IRQ0 to the device driver.
The reason for this is, that although IRQ0 is tied to the FIRST_DEVICE_VECTOR when declaring the irq_vector table, the corresponding bit in the used_vectors map is not set. So, if vectors are released and assigned often enough, the vector will get assigned to another interrupt. This happens more often with MSI interrupts as those are exclusively using a vector.
Quoted 8 lines
Quoted 3 lines
Fix this by setting the bit for the FIRST_DEVICE_VECTOR in the bitmap.
Signed-off-by Stefan Bader <[email protected]>
Hint, if you want patches to go into the -stable tree, just add:
cc: Stable <[email protected]>
to the patch when you submit it in the signed-off-by area, and it will be automatically sent to us when it goes into the main kernel tree.

Quoted 5 lines

yes. Note that this is a special case, as there will be no upstream commit to tag with Cc: <[email protected]>, because this bug got eliminated via not backportable means: APIC code unification.
Acked-by Ingo Molnar <[email protected]>

So this is a -stable release only patch? .28 will never get/need this?

thanks,

greg k-h

Stefan Bader

author

2008-10-10 20:36 UTC #4 lore raw

Quoted 27 lines · Greg KH

Quoted 26 lines · Ingo Molnar
Quoted 13 lines · Greg KH
Quoted 12 lines · Stefan Bader
Affected: 2.6.24-2.6.27
Someone from the community found out, that after repeatedly unloading and loading a device driver that uses MSI IRQs, the system eventually assigned the vector initially reserved for IRQ0 to the device driver.
The reason for this is, that although IRQ0 is tied to the FIRST_DEVICE_VECTOR when declaring the irq_vector table, the corresponding bit in the used_vectors map is not set. So, if vectors are released and assigned often enough, the vector will get assigned to another interrupt. This happens more often with MSI interrupts as those are exclusively using a vector.
Quoted 7 lines
Quoted 3 lines
Fix this by setting the bit for the FIRST_DEVICE_VECTOR in the bitmap.
Signed-off-by Stefan Bader <[email protected]>
Hint, if you want patches to go into the -stable tree, just add:
cc: Stable <[email protected]>
to the patch when you submit it in the signed-off-by area, and it will be automatically sent to us when it goes into the main kernel tree.
yes. Note that this is a special case, as there will be no upstream commit to tag with Cc: <[email protected]>, because this bug got eliminated via not backportable means: APIC code unification.

Quoted 7 lines

Quoted 1 line
Acked-by Ingo Molnar <[email protected]>
So this is a -stable release only patch? .28 will never get/need this?
thanks,
greg k-h

Correct. Tried to hint this by the Affected line but should have been more verbose.

Regards,
Stefan

Signature · 1 line

When all other means of communication fail, try words!

Greg KH

2008-10-10 20:45 UTC #5 lore raw

Quoted 30 lines · Stefan Bader

Quoted 29 lines · Greg KH
Quoted 27 lines · Ingo Molnar
Quoted 13 lines · Greg KH
Quoted 12 lines · Stefan Bader
Affected: 2.6.24-2.6.27
Someone from the community found out, that after repeatedly unloading and loading a device driver that uses MSI IRQs, the system eventually assigned the vector initially reserved for IRQ0 to the device driver.
The reason for this is, that although IRQ0 is tied to the FIRST_DEVICE_VECTOR when declaring the irq_vector table, the corresponding bit in the used_vectors map is not set. So, if vectors are released and assigned often enough, the vector will get assigned to another interrupt. This happens more often with MSI interrupts as those are exclusively using a vector.
Quoted 7 lines
Quoted 3 lines
Fix this by setting the bit for the FIRST_DEVICE_VECTOR in the bitmap.
Signed-off-by Stefan Bader <[email protected]>
Hint, if you want patches to go into the -stable tree, just add:
cc: Stable <[email protected]>
to the patch when you submit it in the signed-off-by area, and it will be automatically sent to us when it goes into the main kernel tree.
yes. Note that this is a special case, as there will be no upstream commit to tag with Cc: <[email protected]>, because this bug got eliminated via not backportable means: APIC code unification.
Acked-by Ingo Molnar <[email protected]>

Quoted 7 lines

Quoted 5 lines
So this is a -stable release only patch? .28 will never get/need this?
thanks,
greg k-h
Correct. Tried to hint this by the Affected line but should have been more verbose.

Great, thanks for letting me know, I'll queue it up for -stable.

Oh, next time, please don't attach patches in base64 format, that's a pain in the ass...

thanks,

greg k-h

Greg KH

2008-10-10 20:20 UTC #6 lore raw

In reply to Stefan Bader

Quoted 5 lines · Stefan Bader

Affected: 2.6.24-2.6.27
Someone from the community found out, that after repeatedly unloading and loading a device driver that uses MSI IRQs, the system eventually assigned the vector initially reserved for IRQ0 to the device driver.

"Someone"? do you have a name or pointer to the person/bugreport so that they can be properly credited?

thanks,

greg k-h

Stefan Bader

author

2008-10-10 20:23 UTC #7 lore raw

Quoted 10 lines · Greg KH

Quoted 5 lines · Stefan Bader
Affected: 2.6.24-2.6.27
Someone from the community found out, that after repeatedly unloading and loading a device driver that uses MSI IRQs, the system eventually assigned the vector initially reserved for IRQ0 to the device driver.
"Someone"? do you have a name or pointer to the person/bugreport so that they can be properly credited?

I had a name in but the person did not want to be mentioned.

Quoted 6 lines

thanks,
greg k-h

Signature · 1 line

When all other means of communication fail, try words!

Greg KH

2008-10-10 20:21 UTC #8 lore raw

In reply to Stefan Bader

Quoted 11 lines · Stefan Bader

Affected: 2.6.24-2.6.27
Someone from the community found out, that after repeatedly unloading and loading a device driver that uses MSI IRQs, the system eventually assigned the vector initially reserved for IRQ0 to the device driver.
The reason for this is, that although IRQ0 is tied to the FIRST_DEVICE_VECTOR when declaring the irq_vector table, the corresponding bit in the used_vectors map is not set. So, if vectors are released and assigned often enough, the vector will get assigned to another interrupt. This happens more often with MSI interrupts as those are exclusively using a vector.

Is there a problem with being assigned to IRQ0 in situations like this?

thanks,

greg k-h

Stefan Bader

author

2008-10-10 20:24 UTC #9 lore raw

Quoted 13 lines · Greg KH

Quoted 11 lines · Stefan Bader
Affected: 2.6.24-2.6.27
Someone from the community found out, that after repeatedly unloading and loading a device driver that uses MSI IRQs, the system eventually assigned the vector initially reserved for IRQ0 to the device driver.
The reason for this is, that although IRQ0 is tied to the FIRST_DEVICE_VECTOR when declaring the irq_vector table, the corresponding bit in the used_vectors map is not set. So, if vectors are released and assigned often enough, the vector will get assigned to another interrupt. This happens more often with MSI interrupts as those are exclusively using a vector.

Quoted 5 lines

Is there a problem with being assigned to IRQ0 in situations like this?
thanks,
greg k-h

Yes, since IRQ0 is still wanted and used by the system timer. After the module get unloaded again, the vector gets freed and no timer interrupt is delivered anymore.

Stefan

Signature · 1 line

When all other means of communication fail, try words!

H. Peter Anvin

reviewer

2008-10-10 20:36 UTC #10 lore raw

In reply to Greg KH

Quoted 3 lines · Greg KH

Is there a problem with being assigned to IRQ0 in situations like this?

On PCs, IRQ 0 is dedicated to use for the PIT.

-hpa

Greg KH

2008-10-10 20:45 UTC #11 lore raw

Quoted 4 lines · H. Peter Anvin

Quoted 1 line · Greg KH
Is there a problem with being assigned to IRQ0 in situations like this?
On PCs, IRQ 0 is dedicated to use for the PIT.

Ah, I didn't realize it couldn't be shared.

thanks,

greg k-h

Alan Cox

2008-10-10 20:51 UTC #12 lore raw

Quoted 7 lines · On Fri, 10 Oct 2008 13:41:33 -0700 Greg KH <[email protected]> wrote:

Quoted 4 lines · H. Peter Anvin
Quoted 1 line · Greg KH
Is there a problem with being assigned to IRQ0 in situations like this?
On PCs, IRQ 0 is dedicated to use for the PIT.
Ah, I didn't realize it couldn't be shared.

And more importantly dev->irq == 0 or IRQ == 0 to anything but arch internal code means "no interrupt line".

Greg KH

2008-10-10 21:23 UTC #13 lore raw

Quoted 5 lines · Alan Cox

Quoted 2 lines · On Fri, 10 Oct 2008 13:41:33 -0700 Greg KH <[email protected]> wrote:
Quoted 1 line · H. Peter Anvin
Greg KH wrote:

Quoted 8 lines

Quoted 5 lines
Quoted 3 lines
Quoted 1 line
Is there a problem with being assigned to IRQ0 in situations like this?
On PCs, IRQ 0 is dedicated to use for the PIT.
Ah, I didn't realize it couldn't be shared.
And more importantly dev->irq == 0 or IRQ == 0 to anything but arch internal code means "no interrupt line".

I thought we had changed that a while ago, as some arches had 0 as a valid irq line.

Anyway, thanks all for the information, I appreciate it.

greg k-h

Alan Cox

2008-10-10 21:49 UTC #14 lore raw

Quoted 5 lines

Quoted 2 lines
And more importantly dev->irq == 0 or IRQ == 0 to anything but arch internal code means "no interrupt line".
I thought we had changed that a while ago, as some arches had 0 as a valid irq line.

No .. Linus decreed (and I think sensibly myself that IRQ = 0 meant no IRQ assigned).. the argument being that in C you naturally write stuff like

		if (!dev->irq)

I can dig up the reference URLS to the list mails if you want

Greg KH

2008-10-10 22:13 UTC #15 lore raw

Quoted 11 lines · Alan Cox

Quoted 5 lines
Quoted 2 lines
And more importantly dev->irq == 0 or IRQ == 0 to anything but arch internal code means "no interrupt line".
I thought we had changed that a while ago, as some arches had 0 as a valid irq line.
No .. Linus decreed (and I think sensibly myself that IRQ = 0 meant no IRQ assigned).. the argument being that in C you naturally write stuff like
		if (!dev->irq)

Ok, I was confused, thanks.

Quoted 1 line

I can dig up the reference URLS to the list mails if you want

Nah, I trust you :)

thanks,

greg k-h