Subject: Re: [PATCH] e1000e: test MSI interrupts

From: "Kok, Auke" <[email-masked]>
Date: Thu, 27 Mar 2008 15:16:41 -0700

Andy Gospodarek wrote:
> On Thu, Mar 27, 2008 at 10:53:43AM -0700, Brandeburg, Jesse wrote:
>> Kok, Auke wrote:
>>> Jeff Garzik wrote:
>>>> Auke Kok wrote:
>>>>> From: Jesse Brandeburg <[email-masked]>
>>>>>
>>>>> Test the MSI interrupt physically once before assuming that it
>>>>> actually works. Several platforms have already come across that
>>>>> have non-functional MSI interrupts and this code will attempt
>>>>> to detect those safely. Once the test succeeds MSI interrupts
>>>>> will be enabled.
>>>>>
>>>>> Signed-off-by: Jesse Brandeburg <[email-masked]>
>>>>> Signed-off-by: Auke Kok <[email-masked]>
>>>> Ah, the perennial add-same-test-to-every-driver conundrum.
>>>>
>>>> I think we are far enough along with MSI to _not_ do this anymore in
>>>> drivers. 
>> Actually, I'm hoping you'll allow this Jeff, we have a production system
>> (see below) we know about that doesn't like the way 82571 formats MSI
>> interrupt messages.  All other systems seem to be okay with this format
>> of MSI messages, but this system implemented a stricter interpretation
>> of the spec, and so even though that system doesn't need a quirk for MSI
>> because MSI works in general, we still MUST test the MSI vector to make
>> sure it works *for us*  In this case it comes down to being an errata
>> workaround.
>>
>> Since there is no way to "test" generation of an interrupt from any
>> specific hardware device without internal knowledge of said device,
>> there isn't a way for us to help the kernel by writing a generic "test
>> MSI" routine.
>>
>> I would prefer this "generic test" code be in the driver rather than
>> having to identify all the chipsets that fail and have the driver do
>> *specific chipset* detection ala bnx2.c's 8132 bridge workaround.
>>
>>>> The platforms with MSI problems should be discovered, made public,
>>>> and worked around. 
>> This is our workaround, it is our fault, the incompatible chipset is in
>> an x3850 IBM system.
>>
> 
> I've seem similar problems on other systems, so this would be a nice bit
> of code to have somewhere.  I can see Jeff's argument for having this
> outside of drivers, but to make my life easier I'd like to see this in
> e1000e (and e1000!).  :-)

e1000 will soon no longer support PCI Express devices, so this is unneeded.

> I suppose an more general alternative would be to make this code run as
> an ethtool test (current or new one) and then have a module option to
> disable/enable MSI so all could be done in userspace, but something
> automatic like this sure would be great.

a lot of work ...