Re: [Patch v2] Make PCI extended config space (MMCONFIG) a driver opt-in

[Loic Prylli <loic@myri.com>]

On 1/14/2008 6:04 PM, Adrian Bunk wrote:
>> I thought so, but due to the way that things are initialised, mmconfig
>> happens before conf1.  conf1 is known to be usable, but hasn't set
>> raw_pci_ops at this point.  Confusing, and not ideal, but fixing this
>> isn't in scope for 2.6.24.
>> ...
>>     
>
> *ahem*
>
> I don't think anything of what was discussed in this thread would be in 
> scope for 2.6.24 (unless Linus wants to let the bunny that brings eggs 
> release 2.6.24).
>
> cu
> Adrian
>   


Why not put in 2.6.24 a simple fix for the last known remaining mmconfig 
problems in 2.6.24?  There has mostly been three bugs related to mmconfig:
- BIOS/hardware: exaggerated MCFG claims: solved long ago
- hardware: buggy CRS+mmconfig chipset: fix included last month
- Linux code: mmconfig incompatible with live BAR-probing: *not fixed*

It would be ironic to not fix the only one that is really confined to 
the Linux code.

Everybody more or less agrees *any* patches submitted so far does solve 
the known problems, and will not cause regressions. The only long 
discussion is about how to best prevent the effect of an "imaginary" 
fourth bug, and by nature that's a controversial topic.

For 2.6.24, if nothing more than a few lines can be done, either make 
pci=nommconf the default and add a pci=mmconf option, or/and apply one 
of the easiest patch to review i.e.Tony's one, so small I copy it again 
below (using 0x40 or 0x100 for the comparison does not really matter, 
personally I would change it to 0x100 to be like Ivan's patch, but 
either is much better than nothing). Replacing some mmconfig access by 
conf1 cannot cause any regression.


Loic


P.S.: with that patch, conf1-less x86 systems requiring mmconfig would 
not be supported. But they are like UFOs. They are plenty of them in the 
galaxy, but earth sightings are not convincing enough for 2.6.24 
support, they can wait 2.6.25.


diff --git a/arch/x86/pci/mmconfig_32.c b/arch/x86/pci/mmconfig_32.c
index 1bf5816..4474979 100644
--- a/arch/x86/pci/mmconfig_32.c
+++ b/arch/x86/pci/mmconfig_32.c
@@ -73,7 +73,7 @@ static int pci_mmcfg_read(unsigned int seg, unsigned 
int bus,
     }
 
     base = get_base_addr(seg, bus, devfn);
-    if (!base)
+    if ((!base) || (reg < 0x40))
         return pci_conf1_read(seg,bus,devfn,reg,len,value);
 
     spin_lock_irqsave(&pci_config_lock, flags);
@@ -106,7 +106,7 @@ static int pci_mmcfg_write(unsigned int seg, 
unsigned int bus,
         return -EINVAL;
 
     base = get_base_addr(seg, bus, devfn);
-    if (!base)
+    if ((!base) || (reg < 0x40))
         return pci_conf1_write(seg,bus,devfn,reg,len,value);
 
     spin_lock_irqsave(&pci_config_lock, flags);
diff --git a/arch/x86/pci/mmconfig_64.c b/arch/x86/pci/mmconfig_64.c
index 4095e4d..4ad1fcb 100644
--- a/arch/x86/pci/mmconfig_64.c
+++ b/arch/x86/pci/mmconfig_64.c
@@ -61,7 +61,7 @@ static int pci_mmcfg_read(unsigned int seg, unsigned 
int bus,
     }
 
     addr = pci_dev_base(seg, bus, devfn);
-    if (!addr)
+    if ((!addr) || (reg < 0x40))
         return pci_conf1_read(seg,bus,devfn,reg,len,value);
 
     switch (len) {
@@ -89,7 +89,7 @@ static int pci_mmcfg_write(unsigned int seg, unsigned 
int bus,
         return -EINVAL;
 
     addr = pci_dev_base(seg, bus, devfn);
-    if (!addr)
+    if ((!addr) || (reg < 0x40))
         return pci_conf1_write(seg,bus,devfn,reg,len,value);
 
     switch (len) {