pci: let pci_device_shutdown to call pci_disable_msi

this change

| commit 23a274c8a5adafc74a66f16988776fc7dd6f6e51
| Author: Prakash, Sathya <[email protected]>
| Date:   Fri Mar 7 15:53:21 2008 +0530
|
|     [SCSI] mpt fusion: Enable MSI by default for SAS controllers
|
|     This patch modifies the driver to enable MSI by default for all SAS chips.
|
|     Signed-off-by: Sathya Prakash <[email protected]>
|     Signed-off-by: James Bottomley <[email protected]>
|

cause kexec RHEL 5.1 kernel fail.

root casue: the rhel 5.1 kernel still use INTx emulation.
and mptscsih_shutdown doesn't call pci_disable_msi to reenable INTx on kexec path

so try to call pci_disable_msi in shutdown patch

Quoted 15 lines · Yinghai Lu

this change

| commit 23a274c8a5adafc74a66f16988776fc7dd6f6e51
| Author: Prakash, Sathya <[email protected]>
| Date:   Fri Mar 7 15:53:21 2008 +0530
|
|     [SCSI] mpt fusion: Enable MSI by default for SAS controllers
|
|     This patch modifies the driver to enable MSI by default for all SAS chips.
|
|     Signed-off-by: Sathya Prakash <[email protected]>
|     Signed-off-by: James Bottomley <[email protected]>
|

cause kexec RHEL 5.1 kernel fail.

Quoted 4 lines

root casue: the rhel 5.1 kernel still use INTx emulation.
and mptscsih_shutdown doesn't call pci_disable_msi to reenable INTx on kexec path

so try to call pci_disable_msi in shutdown patch

How is kdump going to work? Your shutdown routine won't be called and you'll have the same problem in the 2nd kernel, won't you?

this change

| commit 23a274c8a5adafc74a66f16988776fc7dd6f6e51
| Author: Prakash, Sathya <[email protected]>
| Date:   Fri Mar 7 15:53:21 2008 +0530
|
|     [SCSI] mpt fusion: Enable MSI by default for SAS controllers
|
| This patch modifies the driver to enable MSI by default for all SAS chips.
|
|     Signed-off-by: Sathya Prakash <[email protected]>
|     Signed-off-by: James Bottomley <[email protected]>
|

cause kexec RHEL 5.1 kernel fail.

root casue: the rhel 5.1 kernel still use INTx emulation.
and mptscsih_shutdown doesn't call pci_disable_msi to reenable INTx on kexec path

so try to call pci_disable_msi in shutdown patch

Quoted 15 lines · Yinghai Lu

this change

| commit 23a274c8a5adafc74a66f16988776fc7dd6f6e51
| Author: Prakash, Sathya <[email protected]>
| Date:   Fri Mar 7 15:53:21 2008 +0530
|
|     [SCSI] mpt fusion: Enable MSI by default for SAS controllers
|
| This patch modifies the driver to enable MSI by default for all SAS chips.
|
|     Signed-off-by: Sathya Prakash <[email protected]>
|     Signed-off-by: James Bottomley <[email protected]>
|

cause kexec RHEL 5.1 kernel fail.

Quoted 2 lines

root casue: the rhel 5.1 kernel still use INTx emulation.
and mptscsih_shutdown doesn't call pci_disable_msi to reenable INTx on kexec

path

Quoted 2 lines

so try to call pci_disable_msi in shutdown patch

How is kdump going to work? Your shutdown routine won't be called and you'll have the same problem in the 2nd kernel, won't you?

Quoted 16 lines · Michael Ellerman

Quoted 15 lines · Yinghai Lu

this change

| commit 23a274c8a5adafc74a66f16988776fc7dd6f6e51
| Author: Prakash, Sathya <[email protected]>
| Date:   Fri Mar 7 15:53:21 2008 +0530
|
|     [SCSI] mpt fusion: Enable MSI by default for SAS controllers
|
| This patch modifies the driver to enable MSI by default for all SAS chips.
|
|     Signed-off-by: Sathya Prakash <[email protected]>
|     Signed-off-by: James Bottomley <[email protected]>
|

cause kexec RHEL 5.1 kernel fail.

Quoted 8 lines

Quoted 2 lines

root casue: the rhel 5.1 kernel still use INTx emulation.
and mptscsih_shutdown doesn't call pci_disable_msi to reenable INTx on kexec

path

Quoted 2 lines

so try to call pci_disable_msi in shutdown patch

How is kdump going to work? Your shutdown routine won't be called and you'll have the same problem in the 2nd kernel, won't you?

Taking a quick look our current msi initialization appears robust in not assuming the state of the msi config bits.

So the only remaining problem is running older software that assumes the msi config bits are in the state they should be in out of reset.

YH on that score it appears I goofed a little when I gave you my suggestion on how to fix this in pci_disable_msi.

If we have crazy hardware that supports multi irqs in with a plain msi capability. During initialization we mask all of the irqs.

from msi_capability_init:

        if (entry->msi_attrib.maskbit) {
                unsigned int maskbits, temp;
                /* All MSIs are unmasked by default, Mask them all */
                pci_read_config_dword(dev,
                        msi_mask_bits_reg(pos, is_64bit_address(control)),
                        &maskbits);
                temp = (1 << multi_msi_capable(control));
                temp = ((temp - 1) & ~temp);
                maskbits |= temp;
                pci_write_config_dword(dev,
                        msi_mask_bits_reg(pos, is_64bit_address(control)),
                        maskbits);
        }

So it appears to truly return to the reset state we should unmask them all, instead of just that one. Not that it matters in practice, but handling that corner case would be polite.

Quoted 13 lines · Yinghai Lu

this change

| commit 23a274c8a5adafc74a66f16988776fc7dd6f6e51
| Author: Prakash, Sathya <[email protected]>
| Date:   Fri Mar 7 15:53:21 2008 +0530
|
|     [SCSI] mpt fusion: Enable MSI by default for SAS controllers
|
| This patch modifies the driver to enable MSI by default for all SAS chips.
|
|     Signed-off-by: Sathya Prakash <[email protected]>
|     Signed-off-by: James Bottomley <[email protected]>
|

Quoted 7 lines

cause kexec RHEL 5.1 kernel fail.

root casue: the rhel 5.1 kernel still use INTx emulation.
and mptscsih_shutdown doesn't call pci_disable_msi to reenable INTx on kexec path

so try to call pci_disable_msi in shutdown patch

Ok this looks like a reasonable approach.

Could you please change how this is factored.
And implement a pci_shutdown_msi and a pci_shutdown_msix that just performs the hardware state change.

Then have pci_disable_msi and pci_disable_msix call them?

That should be much easier to maintain then a adding a function that takes a magic flag.

That is the design of the shutdown interface and it seems to work well.

Quoted 20 lines · Michael Ellerman

Quoted 13 lines · Yinghai Lu

this change

| commit 23a274c8a5adafc74a66f16988776fc7dd6f6e51
| Author: Prakash, Sathya <[email protected]>
| Date:   Fri Mar 7 15:53:21 2008 +0530
|
|     [SCSI] mpt fusion: Enable MSI by default for SAS controllers
|
| This patch modifies the driver to enable MSI by default for all SAS chips.
|
|     Signed-off-by: Sathya Prakash <[email protected]>
|     Signed-off-by: James Bottomley <[email protected]>
|

Quoted 4 lines

cause kexec RHEL 5.1 kernel fail.

root casue: the rhel 5.1 kernel still use INTx emulation.
and mptscsih_shutdown doesn't call pci_disable_msi to reenable INTx on kexec

path

Quoted 1 line

Quoted 4 lines

Quoted 1 line

so try to call pci_disable_msi in shutdown patch

How is kdump going to work? Your shutdown routine won't be called and you'll have the same problem in the 2nd kernel, won't you?

Taking a quick look our current msi initialization appears robust in not assuming the state of the msi config bits.

Quoted 27 lines · Eric W. Biederman

Quoted 24 lines · Michael Ellerman

Quoted 13 lines · Yinghai Lu

this change

| commit 23a274c8a5adafc74a66f16988776fc7dd6f6e51
| Author: Prakash, Sathya <[email protected]>
| Date:   Fri Mar 7 15:53:21 2008 +0530
|
|     [SCSI] mpt fusion: Enable MSI by default for SAS controllers
|
| This patch modifies the driver to enable MSI by default for all SAS chips.
|
|     Signed-off-by: Sathya Prakash <[email protected]>
|     Signed-off-by: James Bottomley <[email protected]>
|

Quoted 4 lines

cause kexec RHEL 5.1 kernel fail.

root casue: the rhel 5.1 kernel still use INTx emulation.
and mptscsih_shutdown doesn't call pci_disable_msi to reenable INTx on kexec

path

Quoted 2 lines

so try to call pci_disable_msi in shutdown patch

How is kdump going to work? Your shutdown routine won't be called and you'll have the same problem in the 2nd kernel, won't you?

Quoted 2 lines

Taking a quick look our current msi initialization appears robust in not assuming the state of the msi config bits.

But does that help us? What if the device driver in the 2nd kernel assumes it's using INTX, when in fact MSI is enabled on the device. In that case none of the MSI code will even be called AFAIK.

Quoted 3 lines · Michael Ellerman

But does that help us? What if the device driver in the 2nd kernel assumes it's using INTX, when in fact MSI is enabled on the device. In that case none of the MSI code will even be called AFAIK.

ok. for kdump case, if driver can not use msi (?), with pci=nomsi we need to call pci_intx_for_msi(dev, 1) at beginning of second kernel.

will produce another patch about that case.

Yinghai found after using 2.6.25-rc3 later to kexec RHEL 5.1, NIC can not be used.

bisected to

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

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

Eric said:
This is over complicated and for hardware that erroneously triggers a msi irq after free_irq may have potential problems.

So lets do the much simpler, much safer, and more general method of restoring the mask bit to it's pci reset defined value (enabled) when we disable the kernels use of msi.

it will work, because pci_diable_msi is called after free_irq is called.

v3: extend msi_set_mask_bit to msi_set_mask_bits to take mask, so we can fully restore that to 0x00 instead of 0xfe.

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

Yinghai found after using 2.6.25-rc3 later to kexec RHEL 5.1, NIC can not be used.

bisected to

Quoted 3 lines · Yinghai Lu

Yinghai found after using 2.6.25-rc3 later to kexec RHEL 5.1, NIC can not be used.

Quoted 1 line

bisected to

Hi Yinghai, I've been thinking about these patches a bit... They seem like an important bug fix (making sure kexec'd kernels work), but I'm a bit worried that the kexec'd kernel can't handle potentially broken MSI/INTx setups. Shouldn't the kexec'd kernel be a bit more robust? I guess in this case you're kexec'ing an old kernel, so there's not much we can do, but it still makes me a little uneasy.

I guess for this particular set it doesn't matter much, since we should be restoring things in pci_msi*_shutdown and pci_shutdown_device either way. Can you clean up the changelog a bit and maybe make it more concise? E.g. we probably don't need the whole commit message for the bisect, and we want to be clearer about what the failure mode is w/o the changes...

On Fri, Apr 25, 2008 at 2:48 PM, Jesse Barnes <[email protected]>

Quoted 12 lines · Yinghai Lu

Quoted 4 lines

Yinghai found after using 2.6.25-rc3 later to kexec RHEL 5.1, NIC can not be used.

bisected to

Hi Yinghai, I've been thinking about these patches a bit... They seem
like an important bug fix (making sure kexec'd kernels work), but I'm a bit worried that the kexec'd kernel can't handle potentially broken MSI/INTx setups. Shouldn't the kexec'd kernel be a bit more robust? I guess in this case you're kexec'ing an old kernel, so there's not much we can do, but it still makes me a little uneasy.

Yes, it is important, and should be in 2.6.25 stable too.

the maskbits always 0x00 (enabled) from BIOS post..., so we should restore that.

Quoted 5 lines

I guess for this particular set it doesn't matter much, since we should
be restoring things in pci_msi*_shutdown and pci_shutdown_device either way. Can you clean up the changelog a bit and maybe make it more concise?
E.g. we probably don't need the whole commit message for the bisect, and
we want to be clearer about what the failure mode is w/o the changes...

without it, the second kernel can not use the devices, if second kernel could use MSI but doesn't touch mask_bit.

---
[PATCH 1/2] pci/irq: restore mask_bits in msi shutdown -v3

Yinghai found after using 2.6.25-rc3 later to kexec RHEL 5.1, NIC can not be used.

bisected to

| 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

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 lets to restore the mask bit to it's pci reset defined value (enabled) when we disable the kernels use of msi.

extend msi_set_mask_bit to msi_set_mask_bits to take mask, so we can fully restore that to 0x00 instead of 0xfe.

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