2008-10-10 20:02:19

by Stefan Bader

[permalink] [raw]
Subject: [PATCH] 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]>
--

When all other means of communication fail, try words!



Attachments:
0001-x86-Reserve-FIRST_DEVICE_VECTOR-in-used_vectors-bit.patch (1.49 kB)

2008-10-10 20:19:53

by Greg KH

[permalink] [raw]
Subject: Re: [stable] [PATCH] x86: Reserve FIRST_DEVICE_VECTOR in used_vectors bitmap

On Fri, Oct 10, 2008 at 04:01:54PM -0400, Stefan Bader wrote:
> 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]>

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

2008-10-10 20:20:13

by Greg KH

[permalink] [raw]
Subject: Re: [stable] [PATCH] x86: Reserve FIRST_DEVICE_VECTOR in used_vectors bitmap

On Fri, Oct 10, 2008 at 04:01:54PM -0400, Stefan Bader wrote:
> 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

2008-10-10 20:21:22

by Greg KH

[permalink] [raw]
Subject: Re: [stable] [PATCH] x86: Reserve FIRST_DEVICE_VECTOR in used_vectors bitmap

On Fri, Oct 10, 2008 at 04:01:54PM -0400, Stefan Bader wrote:
> 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

2008-10-10 20:23:20

by Stefan Bader

[permalink] [raw]
Subject: Re: [stable] [PATCH] x86: Reserve FIRST_DEVICE_VECTOR in used_vectors bitmap

Greg KH wrote:
> On Fri, Oct 10, 2008 at 04:01:54PM -0400, Stefan Bader wrote:
>> 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.

> thanks,
>



> greg k-h


--

When all other means of communication fail, try words!

2008-10-10 20:24:18

by Ingo Molnar

[permalink] [raw]
Subject: Re: [stable] [PATCH] x86: Reserve FIRST_DEVICE_VECTOR in used_vectors bitmap


* Greg KH <[email protected]> wrote:

> On Fri, Oct 10, 2008 at 04:01:54PM -0400, Stefan Bader wrote:
> > 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]>
>
> 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

2008-10-10 20:24:50

by Stefan Bader

[permalink] [raw]
Subject: Re: [stable] [PATCH] x86: Reserve FIRST_DEVICE_VECTOR in used_vectors bitmap

Greg KH wrote:
> On Fri, Oct 10, 2008 at 04:01:54PM -0400, Stefan Bader wrote:
>> 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
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

--

When all other means of communication fail, try words!

2008-10-10 20:32:35

by Greg KH

[permalink] [raw]
Subject: Re: [stable] [PATCH] x86: Reserve FIRST_DEVICE_VECTOR in used_vectors bitmap

On Fri, Oct 10, 2008 at 10:23:50PM +0200, Ingo Molnar wrote:
>
> * Greg KH <[email protected]> wrote:
>
> > On Fri, Oct 10, 2008 at 04:01:54PM -0400, Stefan Bader wrote:
> > > 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]>
> >
> > 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]>

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

thanks,

greg k-h

2008-10-10 20:36:25

by Stefan Bader

[permalink] [raw]
Subject: Re: [stable] [PATCH] x86: Reserve FIRST_DEVICE_VECTOR in used_vectors bitmap

Greg KH wrote:
> On Fri, Oct 10, 2008 at 10:23:50PM +0200, Ingo Molnar wrote:
>> * Greg KH <[email protected]> wrote:
>>
>>> On Fri, Oct 10, 2008 at 04:01:54PM -0400, Stefan Bader wrote:
>>>> 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]>
>>> 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]>
>
> 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
--

When all other means of communication fail, try words!

2008-10-10 20:36:40

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [stable] [PATCH] x86: Reserve FIRST_DEVICE_VECTOR in used_vectors bitmap

Greg KH wrote:
>
> 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

2008-10-10 20:45:24

by Greg KH

[permalink] [raw]
Subject: Re: [stable] [PATCH] x86: Reserve FIRST_DEVICE_VECTOR in used_vectors bitmap

On Fri, Oct 10, 2008 at 04:36:11PM -0400, Stefan Bader wrote:
> Greg KH wrote:
> > On Fri, Oct 10, 2008 at 10:23:50PM +0200, Ingo Molnar wrote:
> >> * Greg KH <[email protected]> wrote:
> >>
> >>> On Fri, Oct 10, 2008 at 04:01:54PM -0400, Stefan Bader wrote:
> >>>> 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]>
> >>> 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]>
> >
> > 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

2008-10-10 20:45:42

by Greg KH

[permalink] [raw]
Subject: Re: [stable] [PATCH] x86: Reserve FIRST_DEVICE_VECTOR in used_vectors bitmap

On Fri, Oct 10, 2008 at 01:35:48PM -0700, H. Peter Anvin wrote:
> Greg KH wrote:
>> 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

2008-10-10 20:51:25

by Alan

[permalink] [raw]
Subject: Re: [stable] [PATCH] x86: Reserve FIRST_DEVICE_VECTOR in used_vectors bitmap

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

> On Fri, Oct 10, 2008 at 01:35:48PM -0700, H. Peter Anvin wrote:
> > Greg KH wrote:
> >> 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".

2008-10-10 21:23:43

by Greg KH

[permalink] [raw]
Subject: Re: [stable] [PATCH] x86: Reserve FIRST_DEVICE_VECTOR in used_vectors bitmap

On Fri, Oct 10, 2008 at 09:50:38PM +0100, Alan Cox wrote:
> On Fri, 10 Oct 2008 13:41:33 -0700
> Greg KH <[email protected]> wrote:
>
> > On Fri, Oct 10, 2008 at 01:35:48PM -0700, H. Peter Anvin wrote:
> > > Greg KH wrote:
> > >> 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

2008-10-10 21:49:39

by Alan

[permalink] [raw]
Subject: Re: [stable] [PATCH] x86: Reserve FIRST_DEVICE_VECTOR in used_vectors bitmap

> > 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

2008-10-10 22:13:15

by Greg KH

[permalink] [raw]
Subject: Re: [stable] [PATCH] x86: Reserve FIRST_DEVICE_VECTOR in used_vectors bitmap

On Fri, Oct 10, 2008 at 10:49:07PM +0100, Alan Cox wrote:
> > > 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.

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

Nah, I trust you :)

thanks,

greg k-h