Re: [PATCH] x86/pci: intel ioh need to subtract mmconf range

[Bjorn Helgaas]

On Tuesday 19 January 2010 12:57:39 pm Yinghai Lu wrote:
> On 01/19/2010 11:42 AM, Jeff Garrett wrote:
> > On Fri, Jan 15, 2010 at 10:14:17AM -0800, Jesse Barnes wrote:
> >> On Thu, 14 Jan 2010 16:39:13 -0800
> >> Yinghai Lu <yinghai@xxxxxxxxxx> wrote:
> >>
> >>> On 01/14/2010 03:49 PM, Bjorn Helgaas wrote:
> >>>> On Thursday 14 January 2010 04:38:08 pm Yinghai Lu wrote:
> >>>>> On 01/14/2010 03:09 PM, Bjorn Helgaas wrote:
> >>>>>> On Thursday 14 January 2010 03:46:35 pm Yinghai Lu wrote:
> >>>>>>>
> >>>>>>> Bjorn pointed out we need to remove mmconf range
> >>>>>>>
> >>>>>>> Signed-off-by: Yinghai Lu <yinghai@xxxxxxxxxx>
> >>>>>>>
> >>>>>>> ---
> > ...
> >>>>>>
> >>>>>> This can't be right, can it?  Let's say the kernel was built with
> >>>>>> CONFIG_PCI_MMCONFIG turned off, or the user used "pci=nommconf",
> >>>>>> or the kernel decides not to use MMCONFIG for some other reason.
> >>>>>>
> >>>>>> In that case, the hardware may still be configured to support
> >>>>>> MMCONFIG, but the pci_mmcfg_list will be empty, so your code will
> >>>>>> leave the window alone.  We might assign some of that MMCONFIG
> >>>>>> space to a device, but the hardware will route it to MMCONFIG,
> >>>>>> not to the device.
> >>>>>
> >>>>> so if there is mmconf specified, we just skip the whole function?
> >>>>
> >>>> No, I'm saying that intel-bus.c must ALWAYS remove the MMCONFIG
> >>>> region from the host bridge apertures, even if Linux isn't using
> >>>> MMCONFIG.
> >>>>
> >>>> That means intel-bus.c has to be smart enough to figure out on its
> >>>> own what the MMCONFIG area is.  It can't depend on mmconfig-shared.c
> >>>> to do it, because mmconfig-shared.c might not be there.
> >>>
> >>> that seems go too far away...
> >>>
> >>> Subject: [PATCH -v2] x86/pci: intel ioh need to subtrac mmconf range
> >>>
> >>> Bjorn pointed out we need to remove mmconf range
> >>>
> >>> -v2: if mmconf is not there, get out early.
> >>>
> >>> Signed-off-by: Yinghai Lu <yinghai@xxxxxxxxxx>
> >>>
> >>> ---
> > ...
> >>
> >> This goes against the real intent of intel_bus.c doesn't it?  When we
> >> first added it, the thought was that it would be a purely native way of
> >> getting at bridge window information and not rely on firmware.  If
> >> you're going to make it dependent on MMCONFIG now, why not trust other
> >> firmware tables as well, like _CRS?
> >>
> >> The MMCONFIG ranges are pretty easy to get at, the public docs have
> >> info about the registers that control the MMCONFIG decode ranges, so
> >> you should be able to read them out and add them to this file,
> >> preserving the original intent.
> > 
> > I did attempt a bisection last week, but my pared down config kept
> > hitting a sysfs_create_file panic.  I didn't succeed.

I don't think there's any need to bisect this; sorry I didn't
mention this earlier.

2.6.32 didn't include intel-bus.c, so the kernel just assumed that
all non-RAM addresses got routed to the PCI bus.  This would have
included the [mem 0xd0000000-0xdfffffff] used by your Radeon device,
which explains why it would work there.

After 2.6.32, we added intel-bus.c, which reads some of the host
bridge aperture information from the chipset.  This is apparently
missing something, because intel-bus.c didn't find that region,
so Linux thought the Radeon resource was wrong and disabled it,
which broke it.

> > Should I try the v2 patch above?  What tree is it against?
> 
> maybe later with -tip tree + pci/linux-next.

Yinghai, did you figure out how to discover the [mem 0xd0000000-0xdfffffff]
region in intel-bus.c?  Jeff's video isn't going to work without that.

I don't think we have anything that's worth testing yet.

Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/