Flush writes to MSI-X table
PATCH 1/1 Flush writes to MSI-X table
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.
arch/i386/kernel/io_apic.c +2 -0
@@ -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,
arch/ia64/kernel/msi_ia64.c +2 -0
@@ -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,
arch/ia64/sn/kernel/msi_sn.c +2 -0
@@ -216,6 +216,8 @@ static int sn_msi_retrigger_irq(unsignedstatic 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,
arch/x86_64/kernel/io_apic.c +4 -0
@@ -1923,6 +1923,7 @@ static void set_msi_irq_affinity(unsignecpus_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,
drivers/pci/msi.c +34 -0
@@ -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 disable_msi_irq (unsigned int irq) +{ +msi_set_mask_bit (irq,1 ); +msix_flush_writes (irq); +} + +void enable_msi_irq (unsigned int irq) +{ +msi_set_mask_bit (irq,0 ); +msix_flush_writes (irq); +}static int msi_free_irq (struct pci_dev * dev,int irq);static int msi_init (void ) {
include/linux/msi.h +3 -0
@@ -10,9 +10,12 @@ struct msi_msg {/* Heper functions */ extern void mask_msi_irq (unsigned int irq);extern void unmask_msi_irq (unsigned int irq); +extern void disable_msi_irq (unsigned int irq); +extern void enable_msi_irq (unsigned int irq);extern void read_msi_msg (unsigned int irq,struct msi_msg *msg);extern void write_msi_msg (unsigned int irq,struct msi_msg *msg); +extern void msix_flush_writes (unsigned int irq); struct msi_desc { struct {
Quoted 12 lines · 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.
Quoted 2 lines
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
Quoted 13 lines · Greg KH
Quoted 12 lines · 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.
Quoted 4 lines
Quoted 2 lines
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).
Quoted 3 lines
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
Quoted 32 lines · Auke
Quoted 13 lines · Greg KH
Quoted 12 lines · 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.
Quoted 4 lines
Quoted 2 lines
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).
Quoted 3 lines
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.
Quoted 7 lines
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
Quoted 7 lines · Greg KH
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
Quoted 9 lines · Mitch A
Quoted 7 lines · Greg KH
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?
Quoted 7 lines
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.
Quoted 3 lines
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
Quoted 21 lines · Greg KH
Quoted 9 lines · Mitch A
Quoted 7 lines · Greg KH
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?
Quoted 7 lines
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.
Quoted 8 lines
Quoted 3 lines
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
Quoted 26 lines · Greg KH
Quoted 25 lines · Greg KH
Quoted 8 lines · Mitch A
Quoted 7 lines · Greg KH
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?
Quoted 7 lines
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.
Quoted 3 lines
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.
Quoted 6 lines
Quoted 1 line
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'
Quoted 34 lines · Auke
Quoted 26 lines · Greg KH
Quoted 25 lines · Greg KH
Quoted 8 lines · Mitch A
Quoted 7 lines · Greg KH
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?
Quoted 7 lines
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.
Quoted 3 lines
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.
Quoted 6 lines
Quoted 1 line
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.
Quoted 4 lines
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
Quoted 4 lines
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.