[PATCH] update MMConfig patches w/915 support

[PATCH] update MMConfig patches w/915 support

[Jesse Barnes]

This patch updates Oliver's MMConfig bridge detection patches with support
for 915G bridges.  It seems to work ok on my 915GM laptop.

I also tried adding 965 support, but it doesn't work (at least not on my
G965 box).  When I enable MMConfig support when the register value is
0xf00000003 (should be a 256M enabled window at 0xf0000000) the box hangs
at boot, so I'm not sure what I'm doing wrong...

The routines could probably be consolidated into a single probe_intel_9xx
routine or something, but I really looked at that yet (though there are
many similarities between the 91[05], 945 and 965 families, they may not
be enough that the code would actually be simpler if shared.

Thanks,
Jesse

Signed-off-by:  Jesse Barnes <jbarnes@virtuousgeek.org>

diff -Napur -X /home/jbarnes/dontdiff linux-2.6.19-mmconfig.orig/arch/i386/pci/mmconfig-shared.c linux-2.6.19-mmconfig/arch/i386/pci/mmconfig-shared.c
--- linux-2.6.19-mmconfig.orig/arch/i386/pci/mmconfig-shared.c	2007-01-07 10:10:29.000000000 -0800
+++ linux-2.6.19-mmconfig/arch/i386/pci/mmconfig-shared.c	2007-01-07 11:36:24.000000000 -0800
@@ -71,6 +71,25 @@ static __init const char *pci_mmcfg_e752
 	return "Intel Corporation E7520 Memory Controller Hub";
 }
 
+static __init const char *pci_mmcfg_intel_915(void)
+{
+	u32 pciexbar, len = 0;
+
+	pci_conf1_read(0, 0, PCI_DEVFN(0,0), 0x48, 4, &pciexbar);
+
+	/* No enable bit or size field, so assume 256M range is enabled. */
+	len = 0x10000000U;
+	pci_mmcfg_config_num = 1;
+
+	pci_mmcfg_config = kzalloc(sizeof(pci_mmcfg_config[0]), GFP_KERNEL);
+	pci_mmcfg_config[0].base_address = pciexbar;
+	pci_mmcfg_config[0].pci_segment_group_number = 0;
+	pci_mmcfg_config[0].start_bus_number = 0;
+	pci_mmcfg_config[0].end_bus_number = (len >> 20) - 1;
+
+	return "Intel Corporation 915PM/GM/GMS Express Memory Controller Hub";
+}
+
 static __init const char *pci_mmcfg_intel_945(void)
 {
 	u32 pciexbar, mask = 0, len = 0;
@@ -126,6 +145,7 @@ struct pci_mmcfg_hostbridge_probe {
 
 static __initdata struct pci_mmcfg_hostbridge_probe pci_mmcfg_probes[] = {
 	{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_E7520_MCH, pci_mmcfg_e7520 },
+	{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82915GM_HB, pci_mmcfg_intel_915 },
 	{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82945G_HB, pci_mmcfg_intel_945 },
 };
 

Re: [PATCH] update MMConfig patches w/915 support

[Jesse Barnes]

On Sunday, January 07, 2007 11:42 am, Jesse Barnes wrote:
> This patch updates Oliver's MMConfig bridge detection patches with
> support for 915G bridges.  It seems to work ok on my 915GM laptop.
>
> I also tried adding 965 support, but it doesn't work (at least not on my
> G965 box).  When I enable MMConfig support when the register value is
> 0xf00000003 (should be a 256M enabled window at 0xf0000000) the box
> hangs at boot, so I'm not sure what I'm doing wrong...

For reference, here's the probe routine I tried for 965, probably something 
dumb wrong with it that I'm not seeing atm.

static __init const char *pci_mmcfg_intel_965(void)
{
	u64 pciexbar, mask = 0, len = 0;
	u32 lo, hi;

	pci_mmcfg_config_num = 1;

	pci_conf1_read(0, 0, PCI_DEVFN(0,0), 0x60, 4, &lo);
	pci_conf1_read(0, 0, PCI_DEVFN(0,0), 0x64, 4, &hi);
	pciexbar = ((u64)hi << 32) | lo;

	/* Enable bit */
	if (!(pciexbar & 1))
		pci_mmcfg_config_num = 0;

	/* Size bits */
	switch ((pciexbar >> 1) & 3) {
	case 0:
		mask = 0xff0000000UL;
		len  = 0x10000000U;
		break;
	case 1:
		mask = 0xff8000000UL;
		len  = 0x08000000U;
		break;
	case 2:
		mask = 0xffc000000UL;
		len  = 0x04000000U;
		break;
	default:
		pci_mmcfg_config_num = 0;
	}

	if (pci_mmcfg_config_num) {
		pci_mmcfg_config = kzalloc(sizeof(pci_mmcfg_config[0]),
					   GFP_KERNEL);
		pci_mmcfg_config[0].base_address = pciexbar & mask;
		pci_mmcfg_config[0].pci_segment_group_number = 0;
		pci_mmcfg_config[0].start_bus_number = 0;
		pci_mmcfg_config[0].end_bus_number = (len >> 20) - 1;
	}

	return "Intel Corporation G965 Express Memory Controller Hub";
}

Thanks,
Jesse

P.S.  Hooray for Intel for publishing their bridge specs!  Makes stuff like 
this a bit easier.

Re: [PATCH] update MMConfig patches w/915 support

[Olivier Galibert]

On Sun, Jan 07, 2007 at 11:44:16AM -0800, Jesse Barnes wrote:
> For reference, here's the probe routine I tried for 965, probably something 
> dumb wrong with it that I'm not seeing atm.

It shouldn't have mattered in your case, but base_address is limited
to 32bits.  There is a 32 bits reserved zone after it so hope is not
to be lost, but in any case the current code can't handle over-4G base
addresses at that point.

Does the bios or your '965 give a correct acpi mmconfig entry?

  OG.

> P.S.  Hooray for Intel for publishing their bridge specs!  Makes stuff like 
> this a bit easier.

Ohhh yes.

Re: [PATCH] update MMConfig patches w/915 support

[Jesse Barnes]

On Monday, January 8, 2007 12:45 pm, Olivier Galibert wrote:
> On Sun, Jan 07, 2007 at 11:44:16AM -0800, Jesse Barnes wrote:
> > For reference, here's the probe routine I tried for 965, probably
> > something dumb wrong with it that I'm not seeing atm.
>
> It shouldn't have mattered in your case, but base_address is limited
> to 32bits.  There is a 32 bits reserved zone after it so hope is not
> to be lost, but in any case the current code can't handle over-4G
> base addresses at that point.
>
> Does the bios or your '965 give a correct acpi mmconfig entry?

I should have captured the debug printk I put in while testing.  I think 
the base address specified in the register was 0xf0000000, with a size 
of 256M marked enabled.  Arjan points out that this likely conflicts 
with other BIOS mappings, which is probably why I saw my machine hang 
when I tried to use it.

As for ACPI, I assume you mean the MCFG table?  I haven't looked at it, 
but the stock kernel complains about a lack of the MCFG range in the 
e820 table and subsequently disables mmconfig.

But won't the bridge register value control what actually gets decoded?  
If so, it sounds like this BIOS is buggy wrt mmconfig mapping in 
general; good thing I'm not using any PCIe devices I guess...

Thanks,
Jesse

Re: [PATCH] update MMConfig patches w/915 support

[Olivier Galibert]

On Mon, Jan 08, 2007 at 01:27:15PM -0800, Jesse Barnes wrote:
> On Monday, January 8, 2007 12:45 pm, Olivier Galibert wrote:
> > On Sun, Jan 07, 2007 at 11:44:16AM -0800, Jesse Barnes wrote:
> > > For reference, here's the probe routine I tried for 965, probably
> > > something dumb wrong with it that I'm not seeing atm.
> >
> > It shouldn't have mattered in your case, but base_address is limited
> > to 32bits.  There is a 32 bits reserved zone after it so hope is not
> > to be lost, but in any case the current code can't handle over-4G
> > base addresses at that point.
> >
> > Does the bios or your '965 give a correct acpi mmconfig entry?
> 
> I should have captured the debug printk I put in while testing.  I think 
> the base address specified in the register was 0xf0000000, with a size 
> of 256M marked enabled.  Arjan points out that this likely conflicts 
> with other BIOS mappings, which is probably why I saw my machine hang 
> when I tried to use it.
> 
> As for ACPI, I assume you mean the MCFG table?  I haven't looked at it, 
> but the stock kernel complains about a lack of the MCFG range in the 
> e820 table and subsequently disables mmconfig.
> 
> But won't the bridge register value control what actually gets decoded?  
> If so, it sounds like this BIOS is buggy wrt mmconfig mapping in 
> general; good thing I'm not using any PCIe devices I guess...

Yeah.  I've checked the docs, I think I know what's going on.  On one
hand, if the chipset is configured to have the range somewhere, it is
decoded before anything external to the chipset, be it ram or mmaped
i/o.  So the information you get from the chipset should not be able
to conflict with anything by definition, it's the anything that
wouldn't be visible.

But in your case of a f0000000-ffffffff mapping, something else
interesting is going on: it's conflicting with other internal
registers of the chipset, which, being fixed address, probably have
priority.  So you probably have to either reduce the range so that the
chipset registers aren't touched, or drop mmconfig if the address is
f0000000.

Technically, we can have the exact same problem with the other
chipsets.  BIOSen suck.

  OG.

Re: [PATCH] update MMConfig patches w/915 support

[Jesse Barnes]

On Tuesday, January 16, 2007 11:28 am, Olivier Galibert wrote:
> > But won't the bridge register value control what actually gets
> > decoded? If so, it sounds like this BIOS is buggy wrt mmconfig
> > mapping in general; good thing I'm not using any PCIe devices I
> > guess...
>
> Yeah.  I've checked the docs, I think I know what's going on.  On one
> hand, if the chipset is configured to have the range somewhere, it is
> decoded before anything external to the chipset, be it ram or mmaped
> i/o.  So the information you get from the chipset should not be able
> to conflict with anything by definition, it's the anything that
> wouldn't be visible.
>
> But in your case of a f0000000-ffffffff mapping, something else
> interesting is going on: it's conflicting with other internal
> registers of the chipset, which, being fixed address, probably have
> priority.  So you probably have to either reduce the range so that
> the chipset registers aren't touched, or drop mmconfig if the address
> is f0000000.
>
> Technically, we can have the exact same problem with the other
> chipsets.  BIOSen suck.

Ouch.  It doesn't seem like there's a sane general way to fix this up, 
aside from some sort of blacklist.  Maybe it's best to leave this 
particular bridge unsupported for now...

Thanks,
Jesse

Re: [PATCH] update MMConfig patches w/915 support

[Olivier Galibert]

On Sun, Jan 07, 2007 at 11:42:09AM -0800, Jesse Barnes wrote:
> This patch updates Oliver's MMConfig bridge detection patches with support
> for 915G bridges.  It seems to work ok on my 915GM laptop.

Looks ok to me.


> I also tried adding 965 support, but it doesn't work (at least not on my
> G965 box).  When I enable MMConfig support when the register value is
> 0xf00000003 (should be a 256M enabled window at 0xf0000000) the box hangs
> at boot, so I'm not sure what I'm doing wrong...
> 
> The routines could probably be consolidated into a single probe_intel_9xx
> routine or something, but I really looked at that yet (though there are
> many similarities between the 91[05], 945 and 965 families, they may not
> be enough that the code would actually be simpler if shared.

The individual functions are so simple, it's probably way better for
maintainance simplicity to keep them separate, at least for now.


> +	pci_conf1_read(0, 0, PCI_DEVFN(0,0), 0x48, 4, &pciexbar);
> +
> +	/* No enable bit or size field, so assume 256M range is enabled. */
> +	len = 0x10000000U;
> +	pci_mmcfg_config_num = 1;
> +
> +	pci_mmcfg_config = kzalloc(sizeof(pci_mmcfg_config[0]), GFP_KERNEL);
> +	pci_mmcfg_config[0].base_address = pciexbar;

Hmmm, I'd mask out the reserved bits if I were you.  Paranoia :-)

  OG.

Re: [PATCH] update MMConfig patches w/915 support

[Jesse Barnes]

On Monday, January 8, 2007 12:32 pm, Olivier Galibert wrote:
> > The routines could probably be consolidated into a single
> > probe_intel_9xx routine or something, but I really looked at that
> > yet (though there are many similarities between the 91[05], 945 and
> > 965 families, they may not be enough that the code would actually
> > be simpler if shared.
>
> The individual functions are so simple, it's probably way better for
> maintainance simplicity to keep them separate, at least for now.

Yeah, sounds good.

> > +	pci_conf1_read(0, 0, PCI_DEVFN(0,0), 0x48, 4, &pciexbar);
> > +
> > +	/* No enable bit or size field, so assume 256M range is enabled.
> > */ +	len = 0x10000000U;
> > +	pci_mmcfg_config_num = 1;
> > +
> > +	pci_mmcfg_config = kzalloc(sizeof(pci_mmcfg_config[0]),
> > GFP_KERNEL); +	pci_mmcfg_config[0].base_address = pciexbar;
>
> Hmmm, I'd mask out the reserved bits if I were you.  Paranoia :-)

Wouldn't hurt I suppose, want me to post a new patch?

Thanks,
Jesse