Re: [PATCH 2.6.20.3] Flush writes to MSI-X table

2007-03-24 Thread Roland Dreier
 > I don't understand your need to try to rush an api change like this in
 > so quickly in an area that has a lot of churn and disagreement lately.
 > _Especially_ so late in the release cycle, and with no hardware publicly
 > availble.

I'm not sure I understood this thread properly, but if I did understand
correctly then this bug affects IRQ balancing on any device with MSI-X
enabled.  In which case, there's plenty of publicly available hardware
with MSI-X support (including drivers in the mainline tree for a long
time).  A quick grep for pci_enable_msix finds plenty of drivers using
MSI-X now: cciss, ib_mthca, cxgb3, forcedeth, s2io, qla2xxx.

 - R.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2.6.20.3] Flush writes to MSI-X table

2007-03-24 Thread Greg KH
On Sat, Mar 24, 2007 at 04:33:41PM -0700, Kok, Auke wrote:
> Greg KH wrote:
> >On Fri, Mar 23, 2007 at 05:28:02PM -0700, Greg KH wrote:
> >>On Fri, Mar 23, 2007 at 05:24:23PM -0700, Williams, Mitch A wrote:
> >>>Greg KH wrote:
> Well, I'm sure you can agree that it is _very_ late in the 2.6.21
> release cycle to expect to get this in for that kernel.  How about
> waiting for 2.6.22 and if it's a big deal, getting it into the
> 2.6.21-stable tree if needed.
> 
> So far I have not seen any bug reports that this patch would fix, have
> you?
> >>>Well, I've seen several bug reports on this issue -- but they're all
> >>>internal to Intel.
> >>>
> >>>However, we do have here a real bug, which shows up on real hardware,
> >>>which will be released soon.  Obviously, I can't discuss release
> >>>schedules, but "soon" is a good word to use.  You might find out more if
> >>>you read The Register (wink, wink).
> >>Ok, but again, as this is something that no one outside of a company can
> >>see, it doesn't really make sense to rush it into the kernel.
> >>
> >>>Given the time frame for release of 2.6.21, I'd be fine with skipping
> >>>2.6.20.x, and putting this in 2.6.21.  But we really don't want to wait
> >>>for 2.6.22.
> >>I think it needs to wait, especially given that there is no public
> >>hardware yet.
> >>
> >>I'll add this to my queue.
> >
> >No, nevermind, I'll wait till it hits linux-pci and gets review from the
> >people there, as there are a _ton_ of other pending MSI patches that you
> >will need to be aware of, as they might conflict with this patch.
> >Please see the linux-pci archives for details of them.
> 
> Actually Mitch and me have been monitoring those and applying them as they 
> came in for the last two months as some of those partially impacted 
> (improved) the issue. The read flush to update the msi-x tables is the only 
> thing missing right now.

Are you including the 21 set MSI patch that want to linux-pci two days
ago?

I don't understand your need to try to rush an api change like this in
so quickly in an area that has a lot of churn and disagreement lately.
_Especially_ so late in the release cycle, and with no hardware publicly
availble.  What is the pressing need here?

thanks,

greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2.6.20.3] Flush writes to MSI-X table

2007-03-24 Thread Kok, Auke

Greg KH wrote:

On Fri, Mar 23, 2007 at 05:28:02PM -0700, Greg KH wrote:

On Fri, Mar 23, 2007 at 05:24:23PM -0700, Williams, Mitch A wrote:

Greg KH wrote:

Well, I'm sure you can agree that it is _very_ late in the 2.6.21
release cycle to expect to get this in for that kernel.  How about
waiting for 2.6.22 and if it's a big deal, getting it into the
2.6.21-stable tree if needed.

So far I have not seen any bug reports that this patch would fix, have
you?

Well, I've seen several bug reports on this issue -- but they're all
internal to Intel.

However, we do have here a real bug, which shows up on real hardware,
which will be released soon.  Obviously, I can't discuss release
schedules, but "soon" is a good word to use.  You might find out more if
you read The Register (wink, wink).

Ok, but again, as this is something that no one outside of a company can
see, it doesn't really make sense to rush it into the kernel.


Given the time frame for release of 2.6.21, I'd be fine with skipping
2.6.20.x, and putting this in 2.6.21.  But we really don't want to wait
for 2.6.22.

I think it needs to wait, especially given that there is no public
hardware yet.

I'll add this to my queue.


No, nevermind, I'll wait till it hits linux-pci and gets review from the
people there, as there are a _ton_ of other pending MSI patches that you
will need to be aware of, as they might conflict with this patch.
Please see the linux-pci archives for details of them.


Actually Mitch and me have been monitoring those and applying them as they came 
in for the last two months as some of those partially impacted (improved) the 
issue. The read flush to update the msi-x tables is the only thing missing right 
now.


Auke'
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2.6.20.3] Flush writes to MSI-X table

2007-03-24 Thread Kok, Auke

Greg KH wrote:

On Fri, Mar 23, 2007 at 05:28:02PM -0700, Greg KH wrote:

On Fri, Mar 23, 2007 at 05:24:23PM -0700, Williams, Mitch A wrote:

Greg KH wrote:

Well, I'm sure you can agree that it is _very_ late in the 2.6.21
release cycle to expect to get this in for that kernel.  How about
waiting for 2.6.22 and if it's a big deal, getting it into the
2.6.21-stable tree if needed.

So far I have not seen any bug reports that this patch would fix, have
you?

Well, I've seen several bug reports on this issue -- but they're all
internal to Intel.

However, we do have here a real bug, which shows up on real hardware,
which will be released soon.  Obviously, I can't discuss release
schedules, but soon is a good word to use.  You might find out more if
you read The Register (wink, wink).

Ok, but again, as this is something that no one outside of a company can
see, it doesn't really make sense to rush it into the kernel.


Given the time frame for release of 2.6.21, I'd be fine with skipping
2.6.20.x, and putting this in 2.6.21.  But we really don't want to wait
for 2.6.22.

I think it needs to wait, especially given that there is no public
hardware yet.

I'll add this to my queue.


No, nevermind, I'll wait till it hits linux-pci and gets review from the
people there, as there are a _ton_ of other pending MSI patches that you
will need to be aware of, as they might conflict with this patch.
Please see the linux-pci archives for details of them.


Actually Mitch and me have been monitoring those and applying them as they came 
in for the last two months as some of those partially impacted (improved) the 
issue. The read flush to update the msi-x tables is the only thing missing right 
now.


Auke'
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2.6.20.3] Flush writes to MSI-X table

2007-03-24 Thread Greg KH
On Sat, Mar 24, 2007 at 04:33:41PM -0700, Kok, Auke wrote:
 Greg KH wrote:
 On Fri, Mar 23, 2007 at 05:28:02PM -0700, Greg KH wrote:
 On Fri, Mar 23, 2007 at 05:24:23PM -0700, Williams, Mitch A wrote:
 Greg KH wrote:
 Well, I'm sure you can agree that it is _very_ late in the 2.6.21
 release cycle to expect to get this in for that kernel.  How about
 waiting for 2.6.22 and if it's a big deal, getting it into the
 2.6.21-stable tree if needed.
 
 So far I have not seen any bug reports that this patch would fix, have
 you?
 Well, I've seen several bug reports on this issue -- but they're all
 internal to Intel.
 
 However, we do have here a real bug, which shows up on real hardware,
 which will be released soon.  Obviously, I can't discuss release
 schedules, but soon is a good word to use.  You might find out more if
 you read The Register (wink, wink).
 Ok, but again, as this is something that no one outside of a company can
 see, it doesn't really make sense to rush it into the kernel.
 
 Given the time frame for release of 2.6.21, I'd be fine with skipping
 2.6.20.x, and putting this in 2.6.21.  But we really don't want to wait
 for 2.6.22.
 I think it needs to wait, especially given that there is no public
 hardware yet.
 
 I'll add this to my queue.
 
 No, nevermind, I'll wait till it hits linux-pci and gets review from the
 people there, as there are a _ton_ of other pending MSI patches that you
 will need to be aware of, as they might conflict with this patch.
 Please see the linux-pci archives for details of them.
 
 Actually Mitch and me have been monitoring those and applying them as they 
 came in for the last two months as some of those partially impacted 
 (improved) the issue. The read flush to update the msi-x tables is the only 
 thing missing right now.

Are you including the 21 set MSI patch that want to linux-pci two days
ago?

I don't understand your need to try to rush an api change like this in
so quickly in an area that has a lot of churn and disagreement lately.
_Especially_ so late in the release cycle, and with no hardware publicly
availble.  What is the pressing need here?

thanks,

greg k-h
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2.6.20.3] Flush writes to MSI-X table

2007-03-24 Thread Roland Dreier
  I don't understand your need to try to rush an api change like this in
  so quickly in an area that has a lot of churn and disagreement lately.
  _Especially_ so late in the release cycle, and with no hardware publicly
  availble.

I'm not sure I understood this thread properly, but if I did understand
correctly then this bug affects IRQ balancing on any device with MSI-X
enabled.  In which case, there's plenty of publicly available hardware
with MSI-X support (including drivers in the mainline tree for a long
time).  A quick grep for pci_enable_msix finds plenty of drivers using
MSI-X now: cciss, ib_mthca, cxgb3, forcedeth, s2io, qla2xxx.

 - R.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2.6.20.3] Flush writes to MSI-X table

2007-03-23 Thread Greg KH
On Fri, Mar 23, 2007 at 05:28:02PM -0700, Greg KH wrote:
> On Fri, Mar 23, 2007 at 05:24:23PM -0700, Williams, Mitch A wrote:
> > Greg KH wrote:
> > >Well, I'm sure you can agree that it is _very_ late in the 2.6.21
> > >release cycle to expect to get this in for that kernel.  How about
> > >waiting for 2.6.22 and if it's a big deal, getting it into the
> > >2.6.21-stable tree if needed.
> > >
> > >So far I have not seen any bug reports that this patch would fix, have
> > >you?
> > 
> > Well, I've seen several bug reports on this issue -- but they're all
> > internal to Intel.
> > 
> > However, we do have here a real bug, which shows up on real hardware,
> > which will be released soon.  Obviously, I can't discuss release
> > schedules, but "soon" is a good word to use.  You might find out more if
> > you read The Register (wink, wink).
> 
> Ok, but again, as this is something that no one outside of a company can
> see, it doesn't really make sense to rush it into the kernel.
> 
> > Given the time frame for release of 2.6.21, I'd be fine with skipping
> > 2.6.20.x, and putting this in 2.6.21.  But we really don't want to wait
> > for 2.6.22.
> 
> I think it needs to wait, especially given that there is no public
> hardware yet.
> 
> I'll add this to my queue.

No, nevermind, I'll wait till it hits linux-pci and gets review from the
people there, as there are a _ton_ of other pending MSI patches that you
will need to be aware of, as they might conflict with this patch.
Please see the linux-pci archives for details of them.

thanks,

greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2.6.20.3] Flush writes to MSI-X table

2007-03-23 Thread Greg KH
On Fri, Mar 23, 2007 at 05:24:23PM -0700, Williams, Mitch A wrote:
> Greg KH wrote:
> >Well, I'm sure you can agree that it is _very_ late in the 2.6.21
> >release cycle to expect to get this in for that kernel.  How about
> >waiting for 2.6.22 and if it's a big deal, getting it into the
> >2.6.21-stable tree if needed.
> >
> >So far I have not seen any bug reports that this patch would fix, have
> >you?
> 
> Well, I've seen several bug reports on this issue -- but they're all
> internal to Intel.
> 
> However, we do have here a real bug, which shows up on real hardware,
> which will be released soon.  Obviously, I can't discuss release
> schedules, but "soon" is a good word to use.  You might find out more if
> you read The Register (wink, wink).

Ok, but again, as this is something that no one outside of a company can
see, it doesn't really make sense to rush it into the kernel.

> Given the time frame for release of 2.6.21, I'd be fine with skipping
> 2.6.20.x, and putting this in 2.6.21.  But we really don't want to wait
> for 2.6.22.

I think it needs to wait, especially given that there is no public
hardware yet.

I'll add this to my queue.

thanks,

greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH 2.6.20.3] Flush writes to MSI-X table

2007-03-23 Thread Williams, Mitch A
Greg KH wrote:
>Well, I'm sure you can agree that it is _very_ late in the 2.6.21
>release cycle to expect to get this in for that kernel.  How about
>waiting for 2.6.22 and if it's a big deal, getting it into the
>2.6.21-stable tree if needed.
>
>So far I have not seen any bug reports that this patch would fix, have
>you?

Well, I've seen several bug reports on this issue -- but they're all
internal to Intel.

However, we do have here a real bug, which shows up on real hardware,
which will be released soon.  Obviously, I can't discuss release
schedules, but "soon" is a good word to use.  You might find out more if
you read The Register (wink, wink).

Given the time frame for release of 2.6.21, I'd be fine with skipping
2.6.20.x, and putting this in 2.6.21.  But we really don't want to wait
for 2.6.22.

I'll take Auke's suggestion and repost this Monday and cc linux-pci and
a slew of other people.

-Mitch
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2.6.20.3] Flush writes to MSI-X table

2007-03-23 Thread Greg KH
On Fri, Mar 23, 2007 at 02:50:45PM -0700, Kok, Auke wrote:
> Greg KH wrote:
> >On Thu, Mar 22, 2007 at 02:08:19PM -0700, Mitch Williams wrote:
> >>Because both MSI-X interrupt messages and MSI-X table writes are posted,
> >>it's possible for them to cross while in-flight.  This results in
> >>interrupts being received long after the kernel thinks they're disabled,
> >>and in interrupts being sent to stale vectors after rebalancing.
> >>
> >>This patch performs a read flush after writes to the MSI-X table for
> >>enable/disable and rebalancing operations.  Because this is an expensive
> >>operation, we do not perform the read flush after mask/unmask
> >>operations.  Hardware which supports MSI-X typically also supports some
> >>sort of interrupt moderation, so a read-flush is not necessary for
> >>mask/unmask operations.
> >>
> >>This patch has been validated with (unreleased) network hardware which
> >>uses MSI-X.
> >
> >Is this needed for any hardware that is public today?
> 
> yes. Every msi-x capable piece of hardware in the field will crash if it 
> does any form of interrupt balancing. (okay that is not that much stuff out 
> there... I know, but the patch is not that big at all - all it does is 
> subtly add a few read flushes to make sure that critical changes in the 
> msix vector tables are pushed out at the proper time).
> 
> >Also, it seems a bit too big of a patch for -stable right now,
> >especially as the mainline patch will not make it into 2.6.22 at the
> >earliest.
> 
> I think Mitch was way too sensitive when he worded his e-mail. We should 
> really be trying to get this fix into 2.6.21 at least.
> 
> Mitch, can you re-post this and include Eric Biederman, linux-pci, our 
> intel platform guys and perhaps Linus and Andrew?
> 
> A lot of vendors (not just us) will be pushing msi-x capable hardware out, 
> and this fix is absolutely needed. Getting it in soon is really preferred. 
> Not to mention that Mitch has spent well over 8 weeks I think making sure 
> that this is indeed the issue and the proper fix...

Well, I'm sure you can agree that it is _very_ late in the 2.6.21
release cycle to expect to get this in for that kernel.  How about
waiting for 2.6.22 and if it's a big deal, getting it into the
2.6.21-stable tree if needed.

So far I have not seen any bug reports that this patch would fix, have
you?

thanks,

greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2.6.20.3] Flush writes to MSI-X table

2007-03-23 Thread Kok, Auke

Greg KH wrote:

On Thu, Mar 22, 2007 at 02:08:19PM -0700, Mitch Williams wrote:

Because both MSI-X interrupt messages and MSI-X table writes are posted,
it's possible for them to cross while in-flight.  This results in
interrupts being received long after the kernel thinks they're disabled,
and in interrupts being sent to stale vectors after rebalancing.

This patch performs a read flush after writes to the MSI-X table for
enable/disable and rebalancing operations.  Because this is an expensive
operation, we do not perform the read flush after mask/unmask
operations.  Hardware which supports MSI-X typically also supports some
sort of interrupt moderation, so a read-flush is not necessary for
mask/unmask operations.

This patch has been validated with (unreleased) network hardware which
uses MSI-X.


Is this needed for any hardware that is public today?


yes. Every msi-x capable piece of hardware in the field will crash if it does 
any form of interrupt balancing. (okay that is not that much stuff out there... 
I know, but the patch is not that big at all - all it does is subtly add a few 
read flushes to make sure that critical changes in the msix vector tables are 
pushed out at the proper time).



Also, it seems a bit too big of a patch for -stable right now,
especially as the mainline patch will not make it into 2.6.22 at the
earliest.


I think Mitch was way too sensitive when he worded his e-mail. We should really 
be trying to get this fix into 2.6.21 at least.


Mitch, can you re-post this and include Eric Biederman, linux-pci, our intel 
platform guys and perhaps Linus and Andrew?


A lot of vendors (not just us) will be pushing msi-x capable hardware out, and 
this fix is absolutely needed. Getting it in soon is really preferred. Not to 
mention that Mitch has spent well over 8 weeks I think making sure that this is 
indeed the issue and the proper fix...


Auke
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2.6.20.3] Flush writes to MSI-X table

2007-03-23 Thread Greg KH
On Thu, Mar 22, 2007 at 02:08:19PM -0700, Mitch Williams wrote:
> Because both MSI-X interrupt messages and MSI-X table writes are posted,
> it's possible for them to cross while in-flight.  This results in
> interrupts being received long after the kernel thinks they're disabled,
> and in interrupts being sent to stale vectors after rebalancing.
> 
> This patch performs a read flush after writes to the MSI-X table for
> enable/disable and rebalancing operations.  Because this is an expensive
> operation, we do not perform the read flush after mask/unmask
> operations.  Hardware which supports MSI-X typically also supports some
> sort of interrupt moderation, so a read-flush is not necessary for
> mask/unmask operations.
> 
> This patch has been validated with (unreleased) network hardware which
> uses MSI-X.

Is this needed for any hardware that is public today?

Also, it seems a bit too big of a patch for -stable right now,
especially as the mainline patch will not make it into 2.6.22 at the
earliest.

thanks,

greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2.6.20.3] Flush writes to MSI-X table

2007-03-23 Thread Greg KH
On Thu, Mar 22, 2007 at 02:08:19PM -0700, Mitch Williams wrote:
 Because both MSI-X interrupt messages and MSI-X table writes are posted,
 it's possible for them to cross while in-flight.  This results in
 interrupts being received long after the kernel thinks they're disabled,
 and in interrupts being sent to stale vectors after rebalancing.
 
 This patch performs a read flush after writes to the MSI-X table for
 enable/disable and rebalancing operations.  Because this is an expensive
 operation, we do not perform the read flush after mask/unmask
 operations.  Hardware which supports MSI-X typically also supports some
 sort of interrupt moderation, so a read-flush is not necessary for
 mask/unmask operations.
 
 This patch has been validated with (unreleased) network hardware which
 uses MSI-X.

Is this needed for any hardware that is public today?

Also, it seems a bit too big of a patch for -stable right now,
especially as the mainline patch will not make it into 2.6.22 at the
earliest.

thanks,

greg k-h
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2.6.20.3] Flush writes to MSI-X table

2007-03-23 Thread Kok, Auke

Greg KH wrote:

On Thu, Mar 22, 2007 at 02:08:19PM -0700, Mitch Williams wrote:

Because both MSI-X interrupt messages and MSI-X table writes are posted,
it's possible for them to cross while in-flight.  This results in
interrupts being received long after the kernel thinks they're disabled,
and in interrupts being sent to stale vectors after rebalancing.

This patch performs a read flush after writes to the MSI-X table for
enable/disable and rebalancing operations.  Because this is an expensive
operation, we do not perform the read flush after mask/unmask
operations.  Hardware which supports MSI-X typically also supports some
sort of interrupt moderation, so a read-flush is not necessary for
mask/unmask operations.

This patch has been validated with (unreleased) network hardware which
uses MSI-X.


Is this needed for any hardware that is public today?


yes. Every msi-x capable piece of hardware in the field will crash if it does 
any form of interrupt balancing. (okay that is not that much stuff out there... 
I know, but the patch is not that big at all - all it does is subtly add a few 
read flushes to make sure that critical changes in the msix vector tables are 
pushed out at the proper time).



Also, it seems a bit too big of a patch for -stable right now,
especially as the mainline patch will not make it into 2.6.22 at the
earliest.


I think Mitch was way too sensitive when he worded his e-mail. We should really 
be trying to get this fix into 2.6.21 at least.


Mitch, can you re-post this and include Eric Biederman, linux-pci, our intel 
platform guys and perhaps Linus and Andrew?


A lot of vendors (not just us) will be pushing msi-x capable hardware out, and 
this fix is absolutely needed. Getting it in soon is really preferred. Not to 
mention that Mitch has spent well over 8 weeks I think making sure that this is 
indeed the issue and the proper fix...


Auke
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2.6.20.3] Flush writes to MSI-X table

2007-03-23 Thread Greg KH
On Fri, Mar 23, 2007 at 02:50:45PM -0700, Kok, Auke wrote:
 Greg KH wrote:
 On Thu, Mar 22, 2007 at 02:08:19PM -0700, Mitch Williams wrote:
 Because both MSI-X interrupt messages and MSI-X table writes are posted,
 it's possible for them to cross while in-flight.  This results in
 interrupts being received long after the kernel thinks they're disabled,
 and in interrupts being sent to stale vectors after rebalancing.
 
 This patch performs a read flush after writes to the MSI-X table for
 enable/disable and rebalancing operations.  Because this is an expensive
 operation, we do not perform the read flush after mask/unmask
 operations.  Hardware which supports MSI-X typically also supports some
 sort of interrupt moderation, so a read-flush is not necessary for
 mask/unmask operations.
 
 This patch has been validated with (unreleased) network hardware which
 uses MSI-X.
 
 Is this needed for any hardware that is public today?
 
 yes. Every msi-x capable piece of hardware in the field will crash if it 
 does any form of interrupt balancing. (okay that is not that much stuff out 
 there... I know, but the patch is not that big at all - all it does is 
 subtly add a few read flushes to make sure that critical changes in the 
 msix vector tables are pushed out at the proper time).
 
 Also, it seems a bit too big of a patch for -stable right now,
 especially as the mainline patch will not make it into 2.6.22 at the
 earliest.
 
 I think Mitch was way too sensitive when he worded his e-mail. We should 
 really be trying to get this fix into 2.6.21 at least.
 
 Mitch, can you re-post this and include Eric Biederman, linux-pci, our 
 intel platform guys and perhaps Linus and Andrew?
 
 A lot of vendors (not just us) will be pushing msi-x capable hardware out, 
 and this fix is absolutely needed. Getting it in soon is really preferred. 
 Not to mention that Mitch has spent well over 8 weeks I think making sure 
 that this is indeed the issue and the proper fix...

Well, I'm sure you can agree that it is _very_ late in the 2.6.21
release cycle to expect to get this in for that kernel.  How about
waiting for 2.6.22 and if it's a big deal, getting it into the
2.6.21-stable tree if needed.

So far I have not seen any bug reports that this patch would fix, have
you?

thanks,

greg k-h
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH 2.6.20.3] Flush writes to MSI-X table

2007-03-23 Thread Williams, Mitch A
Greg KH wrote:
Well, I'm sure you can agree that it is _very_ late in the 2.6.21
release cycle to expect to get this in for that kernel.  How about
waiting for 2.6.22 and if it's a big deal, getting it into the
2.6.21-stable tree if needed.

So far I have not seen any bug reports that this patch would fix, have
you?

Well, I've seen several bug reports on this issue -- but they're all
internal to Intel.

However, we do have here a real bug, which shows up on real hardware,
which will be released soon.  Obviously, I can't discuss release
schedules, but soon is a good word to use.  You might find out more if
you read The Register (wink, wink).

Given the time frame for release of 2.6.21, I'd be fine with skipping
2.6.20.x, and putting this in 2.6.21.  But we really don't want to wait
for 2.6.22.

I'll take Auke's suggestion and repost this Monday and cc linux-pci and
a slew of other people.

-Mitch
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2.6.20.3] Flush writes to MSI-X table

2007-03-23 Thread Greg KH
On Fri, Mar 23, 2007 at 05:24:23PM -0700, Williams, Mitch A wrote:
 Greg KH wrote:
 Well, I'm sure you can agree that it is _very_ late in the 2.6.21
 release cycle to expect to get this in for that kernel.  How about
 waiting for 2.6.22 and if it's a big deal, getting it into the
 2.6.21-stable tree if needed.
 
 So far I have not seen any bug reports that this patch would fix, have
 you?
 
 Well, I've seen several bug reports on this issue -- but they're all
 internal to Intel.
 
 However, we do have here a real bug, which shows up on real hardware,
 which will be released soon.  Obviously, I can't discuss release
 schedules, but soon is a good word to use.  You might find out more if
 you read The Register (wink, wink).

Ok, but again, as this is something that no one outside of a company can
see, it doesn't really make sense to rush it into the kernel.

 Given the time frame for release of 2.6.21, I'd be fine with skipping
 2.6.20.x, and putting this in 2.6.21.  But we really don't want to wait
 for 2.6.22.

I think it needs to wait, especially given that there is no public
hardware yet.

I'll add this to my queue.

thanks,

greg k-h
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2.6.20.3] Flush writes to MSI-X table

2007-03-23 Thread Greg KH
On Fri, Mar 23, 2007 at 05:28:02PM -0700, Greg KH wrote:
 On Fri, Mar 23, 2007 at 05:24:23PM -0700, Williams, Mitch A wrote:
  Greg KH wrote:
  Well, I'm sure you can agree that it is _very_ late in the 2.6.21
  release cycle to expect to get this in for that kernel.  How about
  waiting for 2.6.22 and if it's a big deal, getting it into the
  2.6.21-stable tree if needed.
  
  So far I have not seen any bug reports that this patch would fix, have
  you?
  
  Well, I've seen several bug reports on this issue -- but they're all
  internal to Intel.
  
  However, we do have here a real bug, which shows up on real hardware,
  which will be released soon.  Obviously, I can't discuss release
  schedules, but soon is a good word to use.  You might find out more if
  you read The Register (wink, wink).
 
 Ok, but again, as this is something that no one outside of a company can
 see, it doesn't really make sense to rush it into the kernel.
 
  Given the time frame for release of 2.6.21, I'd be fine with skipping
  2.6.20.x, and putting this in 2.6.21.  But we really don't want to wait
  for 2.6.22.
 
 I think it needs to wait, especially given that there is no public
 hardware yet.
 
 I'll add this to my queue.

No, nevermind, I'll wait till it hits linux-pci and gets review from the
people there, as there are a _ton_ of other pending MSI patches that you
will need to be aware of, as they might conflict with this patch.
Please see the linux-pci archives for details of them.

thanks,

greg k-h
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 2.6.20.3] Flush writes to MSI-X table

2007-03-22 Thread Mitch Williams
Because both MSI-X interrupt messages and MSI-X table writes are posted,
it's possible for them to cross while in-flight.  This results in
interrupts being received long after the kernel thinks they're disabled,
and in interrupts being sent to stale vectors after rebalancing.

This patch performs a read flush after writes to the MSI-X table for
enable/disable and rebalancing operations.  Because this is an expensive
operation, we do not perform the read flush after mask/unmask
operations.  Hardware which supports MSI-X typically also supports some
sort of interrupt moderation, so a read-flush is not necessary for
mask/unmask operations.

This patch has been validated with (unreleased) network hardware which
uses MSI-X.

Signed-off-by: Mitch Williams <[EMAIL PROTECTED]>

diff -urpN -X dontdiff linux-2.6.20.3-clean/arch/i386/kernel/io_apic.c 
linux-2.6.20.3/arch/i386/kernel/io_apic.c
--- linux-2.6.20.3-clean/arch/i386/kernel/io_apic.c 2007-02-04 
10:44:54.0 -0800
+++ linux-2.6.20.3/arch/i386/kernel/io_apic.c   2007-03-22 10:33:47.0 
-0700
@@ -2597,6 +2597,8 @@ static void set_msi_irq_affinity(unsigne
  */
 static struct irq_chip msi_chip = {
.name   = "PCI-MSI",
+   .enable = enable_msi_irq,
+   .disable= disable_msi_irq,
.unmask = unmask_msi_irq,
.mask   = mask_msi_irq,
.ack= ack_ioapic_irq,
diff -urpN -X dontdiff linux-2.6.20.3-clean/arch/ia64/kernel/msi_ia64.c 
linux-2.6.20.3/arch/ia64/kernel/msi_ia64.c
--- linux-2.6.20.3-clean/arch/ia64/kernel/msi_ia64.c2007-02-04 
10:44:54.0 -0800
+++ linux-2.6.20.3/arch/ia64/kernel/msi_ia64.c  2007-03-22 10:33:47.0 
-0700
@@ -116,6 +116,8 @@ static int ia64_msi_retrigger_irq(unsign
  */
 static struct irq_chip ia64_msi_chip = {
.name   = "PCI-MSI",
+   .enable = enable_msi_irq,
+   .disable= disable_msi_irq,
.mask   = mask_msi_irq,
.unmask = unmask_msi_irq,
.ack= ia64_ack_msi_irq,
diff -urpN -X dontdiff linux-2.6.20.3-clean/arch/ia64/sn/kernel/msi_sn.c 
linux-2.6.20.3/arch/ia64/sn/kernel/msi_sn.c
--- linux-2.6.20.3-clean/arch/ia64/sn/kernel/msi_sn.c   2007-02-04 
10:44:54.0 -0800
+++ linux-2.6.20.3/arch/ia64/sn/kernel/msi_sn.c 2007-03-22 10:33:47.0 
-0700
@@ -216,6 +216,8 @@ static int sn_msi_retrigger_irq(unsigned
 
 static struct irq_chip sn_msi_chip = {
.name   = "PCI-MSI",
+   .enable = enable_msi_irq,
+   .disable= disable_msi_irq,
.mask   = mask_msi_irq,
.unmask = unmask_msi_irq,
.ack= sn_ack_msi_irq,
diff -urpN -X dontdiff linux-2.6.20.3-clean/arch/x86_64/kernel/io_apic.c 
linux-2.6.20.3/arch/x86_64/kernel/io_apic.c
--- linux-2.6.20.3-clean/arch/x86_64/kernel/io_apic.c   2007-02-04 
10:44:54.0 -0800
+++ linux-2.6.20.3/arch/x86_64/kernel/io_apic.c 2007-03-22 10:36:03.0 
-0700
@@ -1923,6 +1923,7 @@ static void set_msi_irq_affinity(unsigne
 
cpus_and(mask, tmp, CPU_MASK_ALL);
 
+   msix_flush_writes(irq);
vector = assign_irq_vector(irq, mask, );
if (vector < 0)
return;
@@ -1937,6 +1938,7 @@ static void set_msi_irq_affinity(unsigne
msg.address_lo |= MSI_ADDR_DEST_ID(dest);
 
write_msi_msg(irq, );
+   msix_flush_writes(irq);
set_native_irq_info(irq, mask);
 }
 #endif /* CONFIG_SMP */
@@ -1947,6 +1949,8 @@ static void set_msi_irq_affinity(unsigne
  */
 static struct irq_chip msi_chip = {
.name   = "PCI-MSI",
+   .enable = enable_msi_irq,
+   .disable= disable_msi_irq,
.unmask = unmask_msi_irq,
.mask   = mask_msi_irq,
.ack= ack_apic_edge,
diff -urpN -X dontdiff linux-2.6.20.3-clean/drivers/pci/msi.c 
linux-2.6.20.3/drivers/pci/msi.c
--- linux-2.6.20.3-clean/drivers/pci/msi.c  2007-02-04 10:44:54.0 
-0800
+++ linux-2.6.20.3/drivers/pci/msi.c2007-03-22 10:33:47.0 -0700
@@ -40,6 +40,29 @@ static int msi_cache_init(void)
return 0;
 }
 
+void msix_flush_writes(unsigned int irq)
+{
+   struct msi_desc *entry;
+
+   entry = msi_desc[irq];
+   BUG_ON(!entry || !entry->dev);
+   switch (entry->msi_attrib.type) {
+   case PCI_CAP_ID_MSI:
+   /* nothing to do */
+   break;
+   case PCI_CAP_ID_MSIX:
+   {
+   int offset = entry->msi_attrib.entry_nr * PCI_MSIX_ENTRY_SIZE +
+   PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET;
+   readl(entry->mask_base + offset);
+   break;
+   }
+   default:
+   BUG();
+   break;
+   }
+}
+
 static void msi_set_mask_bit(unsigned int irq, int flag)
 {
struct msi_desc *entry;
@@ -161,6 +184,17 @@ void unmask_msi_irq(unsigned int irq)
msi_set_mask_bit(irq, 0);
 }
 
+void 

[PATCH 2.6.20.3] Flush writes to MSI-X table

2007-03-22 Thread Mitch Williams
Because both MSI-X interrupt messages and MSI-X table writes are posted,
it's possible for them to cross while in-flight.  This results in
interrupts being received long after the kernel thinks they're disabled,
and in interrupts being sent to stale vectors after rebalancing.

This patch performs a read flush after writes to the MSI-X table for
enable/disable and rebalancing operations.  Because this is an expensive
operation, we do not perform the read flush after mask/unmask
operations.  Hardware which supports MSI-X typically also supports some
sort of interrupt moderation, so a read-flush is not necessary for
mask/unmask operations.

This patch has been validated with (unreleased) network hardware which
uses MSI-X.

Signed-off-by: Mitch Williams [EMAIL PROTECTED]

diff -urpN -X dontdiff linux-2.6.20.3-clean/arch/i386/kernel/io_apic.c 
linux-2.6.20.3/arch/i386/kernel/io_apic.c
--- linux-2.6.20.3-clean/arch/i386/kernel/io_apic.c 2007-02-04 
10:44:54.0 -0800
+++ linux-2.6.20.3/arch/i386/kernel/io_apic.c   2007-03-22 10:33:47.0 
-0700
@@ -2597,6 +2597,8 @@ static void set_msi_irq_affinity(unsigne
  */
 static struct irq_chip msi_chip = {
.name   = PCI-MSI,
+   .enable = enable_msi_irq,
+   .disable= disable_msi_irq,
.unmask = unmask_msi_irq,
.mask   = mask_msi_irq,
.ack= ack_ioapic_irq,
diff -urpN -X dontdiff linux-2.6.20.3-clean/arch/ia64/kernel/msi_ia64.c 
linux-2.6.20.3/arch/ia64/kernel/msi_ia64.c
--- linux-2.6.20.3-clean/arch/ia64/kernel/msi_ia64.c2007-02-04 
10:44:54.0 -0800
+++ linux-2.6.20.3/arch/ia64/kernel/msi_ia64.c  2007-03-22 10:33:47.0 
-0700
@@ -116,6 +116,8 @@ static int ia64_msi_retrigger_irq(unsign
  */
 static struct irq_chip ia64_msi_chip = {
.name   = PCI-MSI,
+   .enable = enable_msi_irq,
+   .disable= disable_msi_irq,
.mask   = mask_msi_irq,
.unmask = unmask_msi_irq,
.ack= ia64_ack_msi_irq,
diff -urpN -X dontdiff linux-2.6.20.3-clean/arch/ia64/sn/kernel/msi_sn.c 
linux-2.6.20.3/arch/ia64/sn/kernel/msi_sn.c
--- linux-2.6.20.3-clean/arch/ia64/sn/kernel/msi_sn.c   2007-02-04 
10:44:54.0 -0800
+++ linux-2.6.20.3/arch/ia64/sn/kernel/msi_sn.c 2007-03-22 10:33:47.0 
-0700
@@ -216,6 +216,8 @@ static int sn_msi_retrigger_irq(unsigned
 
 static struct irq_chip sn_msi_chip = {
.name   = PCI-MSI,
+   .enable = enable_msi_irq,
+   .disable= disable_msi_irq,
.mask   = mask_msi_irq,
.unmask = unmask_msi_irq,
.ack= sn_ack_msi_irq,
diff -urpN -X dontdiff linux-2.6.20.3-clean/arch/x86_64/kernel/io_apic.c 
linux-2.6.20.3/arch/x86_64/kernel/io_apic.c
--- linux-2.6.20.3-clean/arch/x86_64/kernel/io_apic.c   2007-02-04 
10:44:54.0 -0800
+++ linux-2.6.20.3/arch/x86_64/kernel/io_apic.c 2007-03-22 10:36:03.0 
-0700
@@ -1923,6 +1923,7 @@ static void set_msi_irq_affinity(unsigne
 
cpus_and(mask, tmp, CPU_MASK_ALL);
 
+   msix_flush_writes(irq);
vector = assign_irq_vector(irq, mask, tmp);
if (vector  0)
return;
@@ -1937,6 +1938,7 @@ static void set_msi_irq_affinity(unsigne
msg.address_lo |= MSI_ADDR_DEST_ID(dest);
 
write_msi_msg(irq, msg);
+   msix_flush_writes(irq);
set_native_irq_info(irq, mask);
 }
 #endif /* CONFIG_SMP */
@@ -1947,6 +1949,8 @@ static void set_msi_irq_affinity(unsigne
  */
 static struct irq_chip msi_chip = {
.name   = PCI-MSI,
+   .enable = enable_msi_irq,
+   .disable= disable_msi_irq,
.unmask = unmask_msi_irq,
.mask   = mask_msi_irq,
.ack= ack_apic_edge,
diff -urpN -X dontdiff linux-2.6.20.3-clean/drivers/pci/msi.c 
linux-2.6.20.3/drivers/pci/msi.c
--- linux-2.6.20.3-clean/drivers/pci/msi.c  2007-02-04 10:44:54.0 
-0800
+++ linux-2.6.20.3/drivers/pci/msi.c2007-03-22 10:33:47.0 -0700
@@ -40,6 +40,29 @@ static int msi_cache_init(void)
return 0;
 }
 
+void msix_flush_writes(unsigned int irq)
+{
+   struct msi_desc *entry;
+
+   entry = msi_desc[irq];
+   BUG_ON(!entry || !entry-dev);
+   switch (entry-msi_attrib.type) {
+   case PCI_CAP_ID_MSI:
+   /* nothing to do */
+   break;
+   case PCI_CAP_ID_MSIX:
+   {
+   int offset = entry-msi_attrib.entry_nr * PCI_MSIX_ENTRY_SIZE +
+   PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET;
+   readl(entry-mask_base + offset);
+   break;
+   }
+   default:
+   BUG();
+   break;
+   }
+}
+
 static void msi_set_mask_bit(unsigned int irq, int flag)
 {
struct msi_desc *entry;
@@ -161,6 +184,17 @@ void unmask_msi_irq(unsigned int irq)
msi_set_mask_bit(irq, 0);
 }
 
+void