Re: [PATCH] support PCI MCFG space on Intel i915 bridges

Re: [PATCH] support PCI MCFG space on Intel i915 bridges

[Robert Hancock]

Jesse Barnes wrote:
> Add support for Intel 915 bridge chips to the new PCI MMConfig detection
> code.  Tested and works on my sole 915 based platform (a Toshiba laptop).  I
> added register masking per Oliver's suggestion, and moved the __init
> qualifier to after the 'static const char' to match Ogawa-san's recent
> cleanup patches.
> 
> Signed-off-by:  Jesse Barnes <jesse.barnes@intel.com>
> 
> diff --git a/arch/i386/pci/mmconfig-shared.c b/arch/i386/pci/mmconfig-shared.c
> index 747d8c6..1339d31 100644
> --- a/arch/i386/pci/mmconfig-shared.c
> +++ b/arch/i386/pci/mmconfig-shared.c
> @@ -72,6 +72,26 @@ static const char __init *pci_mmcfg_e7520(void)
>  	return "Intel Corporation E7520 Memory Controller Hub";
>  }
>  
> +static const char __init *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;

Check the 915 spec more carefully, there is an enable bit, it's in the 
DEVEN register offset 54h, the bit is called PCIEXBAREN (bit 31). If 
that is not set you should be setting pci_mmcfg_config_num to 0 and 
bailing out.

The size is indeed always 256MB though.

> +       pci_mmcfg_config_num = 1;
> +       pciexbar &= 0xe0000000; /* mask out potentially bogus bits */
> +
> +       pci_mmcfg_config = kzalloc(sizeof(pci_mmcfg_config[0]), GFP_KERNEL);
> +       pci_mmcfg_config[0].address = pciexbar;
> +       pci_mmcfg_config[0].pci_segment = 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 const char __init *pci_mmcfg_intel_945(void)
>  {
>  	u32 pciexbar, mask = 0, len = 0;
> @@ -129,6 +149,7 @@ struct pci_mmcfg_hostbridge_probe {
>  
>  static struct pci_mmcfg_hostbridge_probe pci_mmcfg_probes[] __initdata = {
>  	{ 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 },
>  };

-- 
Robert Hancock      Saskatoon, SK, Canada
To email, remove "nospam" from hancockr@nospamshaw.ca
Home Page: http://www.roberthancock.com/

Re: [PATCH] support PCI MCFG space on Intel i915 bridges

[Jesse Barnes]

On Sunday, April 29, 2007 7:10 pm Robert Hancock wrote:
> Jesse Barnes wrote:
> > Add support for Intel 915 bridge chips to the new PCI MMConfig
> > detection code.  Tested and works on my sole 915 based platform (a
> > Toshiba laptop).  I added register masking per Oliver's suggestion,
> > and moved the __init qualifier to after the 'static const char' to
> > match Ogawa-san's recent cleanup patches.
> >
> > Signed-off-by:  Jesse Barnes <jesse.barnes@intel.com>
> >
> > diff --git a/arch/i386/pci/mmconfig-shared.c
> > b/arch/i386/pci/mmconfig-shared.c index 747d8c6..1339d31 100644
> > --- a/arch/i386/pci/mmconfig-shared.c
> > +++ b/arch/i386/pci/mmconfig-shared.c
> > @@ -72,6 +72,26 @@ static const char __init *pci_mmcfg_e7520(void)
> >  	return "Intel Corporation E7520 Memory Controller Hub";
> >  }
> >
> > +static const char __init *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;
>
> Check the 915 spec more carefully, there is an enable bit, it's in
> the DEVEN register offset 54h, the bit is called PCIEXBAREN (bit 31).
> If that is not set you should be setting pci_mmcfg_config_num to 0
> and bailing out.

Right, but you patch should obsolete this stuff anyway.  I'll test it 
out in the next few days.

Jesse

Re: [PATCH] support PCI MCFG space on Intel i915 bridges

[Robert Hancock]

Jesse Barnes wrote:
> On Sunday, April 29, 2007 7:10 pm Robert Hancock wrote:
>> Jesse Barnes wrote:
>>> Add support for Intel 915 bridge chips to the new PCI MMConfig
>>> detection code.  Tested and works on my sole 915 based platform (a
>>> Toshiba laptop).  I added register masking per Oliver's suggestion,
>>> and moved the __init qualifier to after the 'static const char' to
>>> match Ogawa-san's recent cleanup patches.
>>>
>>> Signed-off-by:  Jesse Barnes <jesse.barnes@intel.com>
>>>
>>> diff --git a/arch/i386/pci/mmconfig-shared.c
>>> b/arch/i386/pci/mmconfig-shared.c index 747d8c6..1339d31 100644
>>> --- a/arch/i386/pci/mmconfig-shared.c
>>> +++ b/arch/i386/pci/mmconfig-shared.c
>>> @@ -72,6 +72,26 @@ static const char __init *pci_mmcfg_e7520(void)
>>>  	return "Intel Corporation E7520 Memory Controller Hub";
>>>  }
>>>
>>> +static const char __init *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;
>> Check the 915 spec more carefully, there is an enable bit, it's in
>> the DEVEN register offset 54h, the bit is called PCIEXBAREN (bit 31).
>> If that is not set you should be setting pci_mmcfg_config_num to 0
>> and bailing out.
> 
> Right, but you patch should obsolete this stuff anyway.  I'll test it 
> out in the next few days.

We likely still want this chipset-specific support, it will catch the 
case where the MCFG table lists a location which is reserved in ACPI but 
the chipset was actually programmed to a different location entirely, 
which I seem to remember someone mentioning was actually the case on 
some boards..

-- 
Robert Hancock      Saskatoon, SK, Canada
To email, remove "nospam" from hancockr@nospamshaw.ca
Home Page: http://www.roberthancock.com/

Re: [PATCH] support PCI MCFG space on Intel i915 bridges

[Jesse Barnes]

On Monday, April 30, 2007 4:20 pm Robert Hancock wrote:
> > Right, but you patch should obsolete this stuff anyway.  I'll test
> > it out in the next few days.
>
> We likely still want this chipset-specific support, it will catch the
> case where the MCFG table lists a location which is reserved in ACPI
> but the chipset was actually programmed to a different location
> entirely, which I seem to remember someone mentioning was actually
> the case on some boards..

Yeah, I suppose that's true.  So maybe your new checking code should 
leverage the stuff in mmconfig-shared.c to check against the register 
values like Olivier mentioned?

Jesse

Re: [PATCH] support PCI MCFG space on Intel i915 bridges

[Robert Hancock]

Jesse Barnes wrote:
> On Monday, April 30, 2007 4:20 pm Robert Hancock wrote:
>>> Right, but you patch should obsolete this stuff anyway.  I'll test
>>> it out in the next few days.
>> We likely still want this chipset-specific support, it will catch the
>> case where the MCFG table lists a location which is reserved in ACPI
>> but the chipset was actually programmed to a different location
>> entirely, which I seem to remember someone mentioning was actually
>> the case on some boards..
> 
> Yeah, I suppose that's true.  So maybe your new checking code should 
> leverage the stuff in mmconfig-shared.c to check against the register 
> values like Olivier mentioned?

The patch I sent takes the MCFG address from the chipset registers for 
known chipsets, or the ACPI MCFG table otherwise. Either way, it will 
verify that the memory range is reserved in ACPI.

-- 
Robert Hancock      Saskatoon, SK, Canada
To email, remove "nospam" from hancockr@nospamshaw.ca
Home Page: http://www.roberthancock.com/

[PATCH] support PCI MCFG space on Intel i915 bridges

[Jesse Barnes]

Add support for Intel 915 bridge chips to the new PCI MMConfig detection
code.  Tested and works on my sole 915 based platform (a Toshiba laptop).  I
added register masking per Oliver's suggestion, and moved the __init
qualifier to after the 'static const char' to match Ogawa-san's recent
cleanup patches.

Signed-off-by:  Jesse Barnes <jesse.barnes@intel.com>

diff --git a/arch/i386/pci/mmconfig-shared.c b/arch/i386/pci/mmconfig-shared.c
index 747d8c6..1339d31 100644
--- a/arch/i386/pci/mmconfig-shared.c
+++ b/arch/i386/pci/mmconfig-shared.c
@@ -72,6 +72,26 @@ static const char __init *pci_mmcfg_e7520(void)
 	return "Intel Corporation E7520 Memory Controller Hub";
 }
 
+static const char __init *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;
+       pciexbar &= 0xe0000000; /* mask out potentially bogus bits */
+
+       pci_mmcfg_config = kzalloc(sizeof(pci_mmcfg_config[0]), GFP_KERNEL);
+       pci_mmcfg_config[0].address = pciexbar;
+       pci_mmcfg_config[0].pci_segment = 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 const char __init *pci_mmcfg_intel_945(void)
 {
 	u32 pciexbar, mask = 0, len = 0;
@@ -129,6 +149,7 @@ struct pci_mmcfg_hostbridge_probe {
 
 static struct pci_mmcfg_hostbridge_probe pci_mmcfg_probes[] __initdata = {
 	{ 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 },
 };