x86_64: restore mask_bits in msi shutdown

PATCH In review Yinghai Lu 4 participants 8 messages Apr 11, 2008 – Apr 17, 2008

Subsystems (2)

x86 PCI

lore

Share & tools

lore

Yinghai Lu

author

2008-04-11 23:22 UTC #0 lore raw

PATCH 1/1 x86_64: restore mask_bits in msi shutdown

I can not kexec RHEL 5.1 from 2.6.25-rc3 later

caused by:
commit 89d694b9dbe769ca1004e01db0ca43964806a611

Author: Thomas Gleixner <[email protected]>
Date:   Mon Feb 18 18:25:17 2008 +0100

genirq: do not leave interupts enabled on free_irq

The default_disable() function was changed in commit:

76d2160147f43f982dfe881404cfde9fd0a9da21
genirq: do not mask interrupts by default

It removed the mask function in favour of the default delayed interrupt disabling. Unfortunately this also broke the shutdown in free_irq() when the last handler is removed from the interrupt for those architectures which rely on the default implementations. Now we can end up with a enabled interrupt line after the last handler was removed, which can result in spurious interrupts.

Fix this by adding a default_shutdown function, which is only installed, when the irqchip implementation does provide neither a shutdown nor a disable function.

[@stable: affected versions: .21 - .24 ]

for MSI, default_shutdown will call mask_bit for msi device. so all mask bits will left disabled after free_irq.
then if kexec next kernel that only can use msi_enable bit.
all device's MSI can not be used.

So try to restore MSI mask bits that is saved before using msi in first kernel.

Signed-off-by Yinghai Lu <[email protected]>

arch/x86/kernel/io_apic_64.c +9 -0

Index: linux-2.6/arch/x86/kernel/io_apic_64.c
===================================================================
@@ -2003,6 +2003,14 @@ static void set_msi_irq_affinity(unsigne
 }
 #endif /* CONFIG_SMP */
 
+static void msi_shutdown(unsigned int irq)
+{
+	struct irq_desc *desc = irq_desc + irq;
+
+	msi_restore_mask_bits(irq);
+	desc->status |= IRQ_MASKED;
+}
+
 /*
  * IRQ Chip for MSI PCI/PCI-X/PCI-Express Devices,
  * which implement the MSI or MSI-X Capability Structure.

@@ -2012,6 +2020,7 @@ static struct irq_chip msi_chip = {
 	.unmask		= unmask_msi_irq,
 	.mask		= mask_msi_irq,
 	.ack		= ack_apic_edge,
+	.shutdown	= msi_shutdown,
 #ifdef CONFIG_SMP
 	.set_affinity	= set_msi_irq_affinity,
 #endif

drivers/pci/msi.c +26 -0

Index: linux-2.6/drivers/pci/msi.c
===================================================================
@@ -123,6 +123,31 @@ static void msix_flush_writes(unsigned i
 	}
 }
 
+void msi_restore_mask_bits(unsigned int irq)
+{
+	struct msi_desc *entry;
+
+	entry = get_irq_msi(irq);
+	BUG_ON(!entry || !entry->dev);
+	switch (entry->msi_attrib.type) {
+	case PCI_CAP_ID_MSI:
+		if (entry->msi_attrib.maskbit) {
+			int pos;
+			u32 mask_bits;
+
+			pos = (long)entry->mask_base;
+			mask_bits = entry->orig_mask_bits;
+			pci_write_config_dword(entry->dev, pos, mask_bits);
+		}
+		break;
+	case PCI_CAP_ID_MSIX:
+		break;
+	default:
+		BUG();
+		break;
+	}
+}
+
 static void msi_set_mask_bit(unsigned int irq, int flag)
 {
 	struct msi_desc *entry;

@@ -376,6 +401,7 @@ static int msi_capability_init(struct pc
 		pci_read_config_dword(dev,
 			msi_mask_bits_reg(pos, is_64bit_address(control)),
 			&maskbits);
+		entry->orig_mask_bits = maskbits;
 		temp = (1 << multi_msi_capable(control));
 		temp = ((temp - 1) & ~temp);
 		maskbits |= temp;

include/linux/msi.h +2 -0

Index: linux-2.6/include/linux/msi.h
===================================================================
@@ -14,6 +14,7 @@ extern void mask_msi_irq(unsigned int ir
 extern void unmask_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 msi_restore_mask_bits(unsigned int irq);
 
 struct msi_desc {
 	struct {

@@ -30,6 +31,7 @@ struct msi_desc {
 	struct list_head list;
 
 	void __iomem *mask_base;
+	u32 orig_mask_bits;  /* restoring it for freeing */
 	struct pci_dev *dev;
 
 	/* Last set MSI message */

Andrew Morton

2008-04-15 08:13 UTC #1 lore raw

Quoted 43 lines · On Fri, 11 Apr 2008 16:26:11 -0700 Yinghai Lu <[email protected]>…

I can not kexec RHEL 5.1 from 2.6.25-rc3 later
caused by:
commit 89d694b9dbe769ca1004e01db0ca43964806a611
Author: Thomas Gleixner <[email protected]>
Date:   Mon Feb 18 18:25:17 2008 +0100
genirq: do not leave interupts enabled on free_irq
The default_disable() function was changed in commit:
76d2160147f43f982dfe881404cfde9fd0a9da21
genirq: do not mask interrupts by default
It removed the mask function in favour of the default delayed interrupt disabling. Unfortunately this also broke the shutdown in free_irq() when the last handler is removed from the interrupt for those architectures which rely on the default implementations. Now we can end up with a enabled interrupt line after the last handler was removed, which can result in spurious interrupts.
Fix this by adding a default_shutdown function, which is only installed, when the irqchip implementation does provide neither a shutdown nor a disable function.
[@stable: affected versions: .21 - .24 ]
for MSI, default_shutdown will call mask_bit for msi device. so all mask bits will left disabled after free_irq.
then if kexec next kernel that only can use msi_enable bit.
all device's MSI can not be used.
So try to restore MSI mask bits that is saved before using msi in first kernel.
Signed-off-by Yinghai Lu <[email protected]>
Index: linux-2.6/arch/x86/kernel/io_apic_64.c
===================================================================

arch/x86/kernel/io_apic_64.c static void set_msi_irq_affinity(unsigne · line 2012

Quoted 10 lines

arch/x86/kernel/io_apic_64.c +7 -0

@@ -2003,6 +2003,14 @@ static void set_msi_irq_affinity(unsigne
 }
 #endif /* CONFIG_SMP */
 
+static void msi_shutdown(unsigned int irq)
+{
+	struct irq_desc *desc = irq_desc + irq;
+
+	msi_restore_mask_bits(irq);
+	desc->status |= IRQ_MASKED;
+}

I wonder if only x86_64 needs this treatment.

Quoted 22 lines

Index: linux-2.6/drivers/pci/msi.c
===================================================================

drivers/pci/msi.c +14 -0

@@ -123,6 +123,31 @@ static void msix_flush_writes(unsigned i
 	}
 }
 
+void msi_restore_mask_bits(unsigned int irq)
+{
+	struct msi_desc *entry;
+
+	entry = get_irq_msi(irq);
+	BUG_ON(!entry || !entry->dev);
+	switch (entry->msi_attrib.type) {
+	case PCI_CAP_ID_MSI:
+		if (entry->msi_attrib.maskbit) {
+			int pos;
+			u32 mask_bits;
+
+			pos = (long)entry->mask_base;
+			mask_bits = entry->orig_mask_bits;

drivers/pci/msi.c static void msix_flush_writes(unsigned i · line 149

Quoted 10 lines

drivers/pci/msi.c +10 -0

+			pci_write_config_dword(entry->dev, pos, mask_bits);
+		}
+		break;
+	case PCI_CAP_ID_MSIX:
+		break;
+	default:
+		BUG();
+		break;
+	}
+}

If no other architectures will ever need this, perhaps it is in the wrong file.

Yinghai Lu

2008-04-15 08:18 UTC #2 lore raw

Quoted 87 lines · Andrew Morton

Quoted 43 lines · On Fri, 11 Apr 2008 16:26:11 -0700 Yinghai Lu <[email protected]>…
I can not kexec RHEL 5.1 from 2.6.25-rc3 later
caused by:
commit 89d694b9dbe769ca1004e01db0ca43964806a611
Author: Thomas Gleixner <[email protected]>
Date:   Mon Feb 18 18:25:17 2008 +0100
genirq: do not leave interupts enabled on free_irq
The default_disable() function was changed in commit:
76d2160147f43f982dfe881404cfde9fd0a9da21
genirq: do not mask interrupts by default
It removed the mask function in favour of the default delayed interrupt disabling. Unfortunately this also broke the shutdown in free_irq() when the last handler is removed from the interrupt for those architectures which rely on the default implementations. Now we can end up with a enabled interrupt line after the last handler was removed, which can result in spurious interrupts.
Fix this by adding a default_shutdown function, which is only installed, when the irqchip implementation does provide neither a shutdown nor a disable function.
[@stable: affected versions: .21 - .24 ]
for MSI, default_shutdown will call mask_bit for msi device. so all mask bits will left disabled after free_irq.
then if kexec next kernel that only can use msi_enable bit.
all device's MSI can not be used.
So try to restore MSI mask bits that is saved before using msi in first kernel.
Signed-off-by Yinghai Lu <[email protected]>
Index: linux-2.6/arch/x86/kernel/io_apic_64.c
===================================================================
arch/x86/kernel/io_apic_64.c static void set_msi_irq_affinity(unsigne · line 2012
Quoted 10 lines
arch/x86/kernel/io_apic_64.c +7 -0
@@ -2003,6 +2003,14 @@ static void set_msi_irq_affinity(unsigne
 }
 #endif /* CONFIG_SMP */
 
+static void msi_shutdown(unsigned int irq)
+{
+     struct irq_desc *desc = irq_desc + irq;
+
+     msi_restore_mask_bits(irq);
+     desc->status |= IRQ_MASKED;
+}
I wonder if only x86_64 needs this treatment.
Quoted 27 lines
Index: linux-2.6/drivers/pci/msi.c
===================================================================
drivers/pci/msi.c +19 -0
@@ -123,6 +123,31 @@ static void msix_flush_writes(unsigned i
      }
 }
 
+void msi_restore_mask_bits(unsigned int irq)
+{
+     struct msi_desc *entry;
+
+     entry = get_irq_msi(irq);
+     BUG_ON(!entry || !entry->dev);
+     switch (entry->msi_attrib.type) {
+     case PCI_CAP_ID_MSI:
+             if (entry->msi_attrib.maskbit) {
+                     int pos;
+                     u32 mask_bits;
+
+                     pos = (long)entry->mask_base;
+                     mask_bits = entry->orig_mask_bits;
+                     pci_write_config_dword(entry->dev, pos, mask_bits);
+             }
+             break;
+     case PCI_CAP_ID_MSIX:
+             break;

Quoted 8 lines

drivers/pci/msi.c static void msix_flush_writes(unsigned i · line 149
Quoted 5 lines
drivers/pci/msi.c +5 -0
+     default:
+             BUG();
+             break;
+     }
+}
If no other architectures will ever need this, perhaps it is in the wrong file.

need someone has powerpc (64?) etc platform with RHEL 5.1 with PCIe NIC that is using MSI
to test if they have the same problem with kexec.

Eric W. Biederman

2008-04-17 09:32 UTC #3 lore raw

In reply to Yinghai Lu

Quoted 35 lines · Yinghai Lu

I can not kexec RHEL 5.1 from 2.6.25-rc3 later
caused by:
commit 89d694b9dbe769ca1004e01db0ca43964806a611
Author: Thomas Gleixner <[email protected]>
Date:   Mon Feb 18 18:25:17 2008 +0100
genirq: do not leave interupts enabled on free_irq
The default_disable() function was changed in commit:
76d2160147f43f982dfe881404cfde9fd0a9da21
genirq: do not mask interrupts by default
It removed the mask function in favour of the default delayed interrupt disabling. Unfortunately this also broke the shutdown in free_irq() when the last handler is removed from the interrupt for those architectures which rely on the default implementations. Now we can end up with a enabled interrupt line after the last handler was removed, which can result in spurious interrupts.
Fix this by adding a default_shutdown function, which is only installed, when the irqchip implementation does provide neither a shutdown nor a disable function.
[@stable: affected versions: .21 - .24 ]
for MSI, default_shutdown will call mask_bit for msi device. so all mask bits will
left disabled after free_irq.
then if kexec next kernel that only can use msi_enable bit.
all device's MSI can not be used.

Quoted 3 lines

So try to restore MSI mask bits that is saved before using msi in first kernel.
Signed-off-by Yinghai Lu <[email protected]>

Ouch! In the case of MSI-X this is horrible. Reenabling an interrupt line when we are not using it. That is likely to cause even stranger things than kexec to fail.

What happens when someone next comes to use that msi interrupt is a reasonable question.

The PCI standard describes the state the bits in the msi capability are supposed to be in after reset, and if a driver is going to assume some state that is the only reasonable state for a driver to expect the hardware to be in. So we don't need to perform a save/restore cycle.

Could you look at having pci_disable_msi reset the mask bit to it's default state after we have called msi_set_enable(dev, 0)? Once the msi capability is disabled the mask bit has no affect.

You should be able to ignoring pci_disable_msix for now.

Thanks,
Eric

Yinghai Lu

2008-04-17 10:05 UTC #4 lore raw

Quoted 38 lines · Eric W. Biederman

Quoted 35 lines · Yinghai Lu
I can not kexec RHEL 5.1 from 2.6.25-rc3 later
caused by:
commit 89d694b9dbe769ca1004e01db0ca43964806a611
Author: Thomas Gleixner <[email protected]>
Date:   Mon Feb 18 18:25:17 2008 +0100
genirq: do not leave interupts enabled on free_irq
The default_disable() function was changed in commit:
76d2160147f43f982dfe881404cfde9fd0a9da21
genirq: do not mask interrupts by default
It removed the mask function in favour of the default delayed interrupt disabling. Unfortunately this also broke the shutdown in free_irq() when the last handler is removed from the interrupt for those architectures which rely on the default implementations. Now we can end up with a enabled interrupt line after the last handler was removed, which can result in spurious interrupts.
Fix this by adding a default_shutdown function, which is only installed, when the irqchip implementation does provide neither a shutdown nor a disable function.
[@stable: affected versions: .21 - .24 ]
for MSI, default_shutdown will call mask_bit for msi device. so all mask bits will
left disabled after free_irq.
then if kexec next kernel that only can use msi_enable bit.
all device's MSI can not be used.

Quoted 7 lines

Quoted 3 lines
So try to restore MSI mask bits that is saved before using msi in first kernel.
Signed-off-by Yinghai Lu <[email protected]>
Ouch! In the case of MSI-X this is horrible. Reenabling an interrupt line when we are not using it. That is likely to cause even stranger things than kexec to fail.

only does save and restore with msi and doesn't do anything with MSIX

+void msi_restore_mask_bits(unsigned int irq)
+{
+ struct msi_desc *entry;
+

+       entry = get_irq_msi(irq);
+       BUG_ON(!entry || !entry->dev);
+       switch (entry->msi_attrib.type) {
+       case PCI_CAP_ID_MSI:

+ if (entry->msi_attrib.maskbit) {
+ int pos;
+ u32 mask_bits;
+

+                       pos = (long)entry->mask_base;
+                       mask_bits = entry->orig_mask_bits;
+                       pci_write_config_dword(entry->dev, pos, mask_bits);

+ }
+ break;
+ case PCI_CAP_ID_MSIX:
+ break; ======================> does not restore that for MSIX
+ default:
+ BUG();
+ break;
+ }
+}

Yinghai Lu

2008-04-17 10:06 UTC #5 lore raw

In reply to Eric W. Biederman

Quoted 55 lines · Eric W. Biederman

Quoted 35 lines · Yinghai Lu
I can not kexec RHEL 5.1 from 2.6.25-rc3 later
caused by:
commit 89d694b9dbe769ca1004e01db0ca43964806a611
Author: Thomas Gleixner <[email protected]>
Date:   Mon Feb 18 18:25:17 2008 +0100
genirq: do not leave interupts enabled on free_irq
The default_disable() function was changed in commit:
76d2160147f43f982dfe881404cfde9fd0a9da21
genirq: do not mask interrupts by default
It removed the mask function in favour of the default delayed interrupt disabling. Unfortunately this also broke the shutdown in free_irq() when the last handler is removed from the interrupt for those architectures which rely on the default implementations. Now we can end up with a enabled interrupt line after the last handler was removed, which can result in spurious interrupts.
Fix this by adding a default_shutdown function, which is only installed, when the irqchip implementation does provide neither a shutdown nor a disable function.
[@stable: affected versions: .21 - .24 ]
for MSI, default_shutdown will call mask_bit for msi device. so all mask bits will
left disabled after free_irq.
then if kexec next kernel that only can use msi_enable bit.
all device's MSI can not be used.
Quoted 3 lines
So try to restore MSI mask bits that is saved before using msi in first kernel.
Signed-off-by Yinghai Lu <[email protected]>
Ouch! In the case of MSI-X this is horrible. Reenabling an interrupt line when we are not using it. That is likely to cause even stranger things than kexec to fail.
What happens when someone next comes to use that msi interrupt is a reasonable question.
The PCI standard describes the state the bits in the msi capability are supposed to be in after reset, and if a driver is going to assume some state that is the only reasonable state for a driver to expect the hardware to be in. So we don't need to perform a save/restore cycle.

Quoted 3 lines

Could you look at having pci_disable_msi reset the mask bit to it's default state after we have called msi_set_enable(dev, 0)? Once the msi capability is disabled the mask bit has no affect.

but the next kernel (RHEL 5.1) only can enable msi, and it doesn't touch mask_bits (that is leaved as 0xff by first 2.6.25-rc2 later) so device (nvidia mcp55 nic) doesn't work. --- if i manually used setpci to set 0x60 to 0xfe or 0x00. it will work. also if i booted kernel (RHEL 5.1), that 0x60
is always 0x00, even nic works with MSI.

Eric W. Biederman

2008-04-17 10:16 UTC #6 lore raw

In reply to Yinghai Lu

Hmm. Trying this again with what should be yh's proper address.

Quoted 35 lines · Yinghai Lu

I can not kexec RHEL 5.1 from 2.6.25-rc3 later
caused by:
commit 89d694b9dbe769ca1004e01db0ca43964806a611
Author: Thomas Gleixner <[email protected]>
Date:   Mon Feb 18 18:25:17 2008 +0100
genirq: do not leave interupts enabled on free_irq
The default_disable() function was changed in commit:
76d2160147f43f982dfe881404cfde9fd0a9da21
genirq: do not mask interrupts by default
It removed the mask function in favour of the default delayed interrupt disabling. Unfortunately this also broke the shutdown in free_irq() when the last handler is removed from the interrupt for those architectures which rely on the default implementations. Now we can end up with a enabled interrupt line after the last handler was removed, which can result in spurious interrupts.
Fix this by adding a default_shutdown function, which is only installed, when the irqchip implementation does provide neither a shutdown nor a disable function.
[@stable: affected versions: .21 - .24 ]
for MSI, default_shutdown will call mask_bit for msi device. so all mask bits will
left disabled after free_irq.
then if kexec next kernel that only can use msi_enable bit.
all device's MSI can not be used.

Quoted 3 lines

So try to restore MSI mask bits that is saved before using msi in first kernel.
Signed-off-by Yinghai Lu <[email protected]>

Ouch! In the case of MSI-X this is horrible. Reenabling an interrupt line when we are not using it. That is likely to cause even stranger things than kexec to fail.

What happens when someone next comes to use that msi interrupt is a reasonable question.

Could you look at having pci_disable_msi reset the mask bit to it's default state after we have called msi_set_enable(dev, 0)? Once the msi capability is disabled the mask bit has no affect.

You should be able to ignoring pci_disable_msix for now.

Thanks,
Eric

Yinghai Lu

2008-04-17 19:38 UTC #7 lore raw

Quoted 43 lines · Eric W. Biederman

Hmm. Trying this again with what should be yh's proper address.
Quoted 35 lines · Yinghai Lu
I can not kexec RHEL 5.1 from 2.6.25-rc3 later
caused by:
commit 89d694b9dbe769ca1004e01db0ca43964806a611
Author: Thomas Gleixner <[email protected]>
Date:   Mon Feb 18 18:25:17 2008 +0100
genirq: do not leave interupts enabled on free_irq
The default_disable() function was changed in commit:
76d2160147f43f982dfe881404cfde9fd0a9da21
genirq: do not mask interrupts by default
It removed the mask function in favour of the default delayed interrupt disabling. Unfortunately this also broke the shutdown in free_irq() when the last handler is removed from the interrupt for those architectures which rely on the default implementations. Now we can end up with a enabled interrupt line after the last handler was removed, which can result in spurious interrupts.
Fix this by adding a default_shutdown function, which is only installed, when the irqchip implementation does provide neither a shutdown nor a disable function.
[@stable: affected versions: .21 - .24 ]
for MSI, default_shutdown will call mask_bit for msi device. so all mask bits will
left disabled after free_irq.
then if kexec next kernel that only can use msi_enable bit.
all device's MSI can not be used.

Quoted 8 lines

Quoted 3 lines
So try to restore MSI mask bits that is saved before using msi in first kernel.
Signed-off-by Yinghai Lu <[email protected]>
Ouch! In the case of MSI-X this is horrible. Reenabling an interrupt
line when we are not using it. That is likely to cause even stranger things than kexec to fail.

yes It doesn't restore that for MSI-X

Quoted 3 lines

What happens when someone next comes to use that msi interrupt is a reasonable question.

it uses MSI, but doesn't touch mask_bits

Quoted 10 lines

The PCI standard describes the state the bits in the msi capability are supposed to be in after reset, and if a driver is going to assume some state that is the only reasonable state for a driver to expect the hardware to be in. So we don't need to perform a save/restore cycle.
Could you look at having pci_disable_msi reset the mask bit to it's default state after we have called msi_set_enable(dev, 0)? Once the msi capability is disabled the mask bit has no affect.