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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 15 Jan 2008 11:00:37 -0500
From:	Loic Prylli <loic@...i.com>
To:	Adrian Bunk <bunk@...nel.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>
CC:	Matthew Wilcox <matthew@....cx>,
	Arjan van de Ven <arjan@...radead.org>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Ivan Kokshaysky <ink@...assic.park.msu.ru>,
	Greg KH <greg@...ah.com>, Greg KH <gregkh@...e.de>,
	linux-kernel@...r.kernel.org, Jeff Garzik <jeff@...zik.org>,
	linux-pci@...ey.karlin.mff.cuni.cz, Martin Mares <mj@....cz>,
	Tony Camuso <tcamuso@...hat.com>
Subject: Re: [Patch v2] Make PCI extended config space (MMCONFIG) a driver
 opt-in



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) {





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