Re: [PATCH] pci: let pci_device_shutdown to call pci_disable_msi

[ebiederm@xmission.com (Eric W. Biederman)]

Michael Ellerman <michael@ellerman.id.au> writes:

> On Tue, 2008-04-22 at 21:48 -0700, Yinghai Lu wrote:
>> this change
>> 
>> | commit 23a274c8a5adafc74a66f16988776fc7dd6f6e51
>> | Author: Prakash, Sathya <sathya.prakash@lsi.com>
>> | 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 <sathya.prakash@lsi.com>
>> |     Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
>> |
>> 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
>
> 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.

Eric