[PATCH] [Intrepid SRU, Jaunty] x86: only scan the root bus in early PCI quirks

[Tim Gardner]

Stefan Bader wrote:
> Tim Gardner wrote:
>> Stefan Bader wrote:
>>> Tim Gardner wrote:
>>>> Yingying Zhao wrote:
>>>>> SRU Justification:
>>>>> We found a problem that on Tylersburg-HEDT with Nvidia Graphics
>>>>> card, system may result in kernel panic. Andi Kleen provided the
>>>>> patch to fix this problem. Please consider to include this patch in
>>>>> Jaunty and Intrepid.
>>>>>
>>>>> Patch (also attached):
>>>>> -- 
>>>>> commit 8659c406ade32f47da2c95889094801921d6330a
>>>>> Author: Andi Kleen <andi at firstfloor.org>
>>>>> Date:   Fri Jan 9 12:17:39 2009 -0800
>>>>>
>>>>>     x86: only scan the root bus in early PCI quirks
>>>>>         We found a situation on Linus' machine that the Nvidia
>>>>> timer quirk hit on
>>>>>     a Intel chipset system.  The problem is that the system has a
>>>>> fancy Nvidia
>>>>>     card with an own PCI bridge, and the early-quirks code looking
>>>>> for any
>>>>>     NVidia bridge triggered on it incorrectly.  This didn't lead a
>>>>> boot
>>>>>     failure by luck, but the timer routing code selecting the wrong
>>>>> timer
>>>>>     first and some ugly messages.  It might lead to real problems
>>>>> on other
>>>>>     systems.
>>>>>         I checked all the devices which are currently checked for
>>>>> by early_quirks
>>>>>     and it turns out they are all located in the root bus zero.
>>>>>         So change the early-quirks loop to only scan bus 0.  This
>>>>> incidently also
>>>>>     saves quite some unnecessary scanning work, because
>>>>> early_quirks doesn't
>>>>>     go through all the non root busses.
>>>>>         The graphics card is not on bus 0, so it is not matched
>>>>> anymore.
>>>>>         Signed-off-by: Andi Kleen <ak at linux.intel.com>
>>>>>     Cc: Ingo Molnar <mingo at elte.hu>
>>>>>     Cc: Thomas Gleixner <tglx at linutronix.de>
>>>>>     Cc: "H. Peter Anvin" <hpa at zytor.com>
>>>>>     Cc: Jesse Barnes <jbarnes at virtuousgeek.org>
>>>>>     Signed-off-by: Andrew Morton <akpm at linux-foundation.org>
>>>>>     Signed-off-by: Linus Torvalds <torvalds at linux-foundation.org>
>>>>>
>>>>> diff --git a/arch/x86/kernel/early-quirks.c
>>>>> b/arch/x86/kernel/early-quirks.c
>>>>> index 744aa7f..76b8cd9 100644
>>>>> --- a/arch/x86/kernel/early-quirks.c
>>>>> +++ b/arch/x86/kernel/early-quirks.c
>>>>> @@ -201,6 +201,12 @@ struct chipset {
>>>>>      void (*f)(int num, int slot, int func);
>>>>>  };
>>>>>  
>>>>> +/*
>>>>> + * Only works for devices on the root bus. If you add any devices
>>>>> + * not on bus 0 readd another loop level in early_quirks(). But
>>>>> + * be careful because at least the Nvidia quirk here relies on
>>>>> + * only matching on bus 0.
>>>>> + */
>>>>>  static struct chipset early_qrk[] __initdata = {
>>>>>      { PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID,
>>>>>        PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, QFLAG_APPLY_ONCE,
>>>>> nvidia_bugs },
>>>>> @@ -267,17 +273,17 @@ static int __init check_dev_quirk(int num,
>>>>> int slot, int func)
>>>>>  
>>>>>  void __init early_quirks(void)
>>>>>  {
>>>>> -    int num, slot, func;
>>>>> +    int slot, func;
>>>>>  
>>>>>      if (!early_pci_allowed())
>>>>>          return;
>>>>>  
>>>>>      /* Poor man's PCI discovery */
>>>>> -    for (num = 0; num < 32; num++)
>>>>> -        for (slot = 0; slot < 32; slot++)
>>>>> -            for (func = 0; func < 8; func++) {
>>>>> -                /* Only probe function 0 on single fn devices */
>>>>> -                if (check_dev_quirk(num, slot, func))
>>>>> -                    break;
>>>>> -            }
>>>>> +    /* Only scan the root bus */
>>>>> +    for (slot = 0; slot < 32; slot++)
>>>>> +        for (func = 0; func < 8; func++) {
>>>>> +            /* Only probe function 0 on single fn devices */
>>>>> +            if (check_dev_quirk(0, slot, func))
>>>>> +                break;
>>>>> +        }
>>>>>  }
>>>>>
>>>>>
>>>>>
>>>> applied to Jaunty
>>>>
>>> This is sort of nitpick, but I think formally I'd need at least one
>>> ack for Intrepid...
>>>
>>
>> Unless someone has this specific problem (which would satisfy SRU
>> requirements), then I'd say apply it to the stable branch only.
>>
> There actually has been one. See
> 
>> it seems that I was looking for that patch without knowing that I was
>> looking for that.  ;-)  This
>> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/267295 report
>> really sounds like the exact problem that is described, so I'll use
>> that for the SRU.
> 
> 
> 

Well then, make it so.
-- 
Tim Gardner tim.gardner at canonical.com