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

!MAILaRCHIVE_VOTE_RePLACE
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
To: Arjan van de Ven <arjan@...>
Cc: Ivan Kokshaysky <ink@...>, Greg KH <greg@...>, Matthew Wilcox <matthew@...>, Linus Torvalds <torvalds@...>, Greg KH <gregkh@...>, <linux-kernel@...>, Jeff Garzik <jeff@...>, <linux-pci@...>, Benjamin Herrenschmidt <benh@...>, Martin Mares <mj@...>, Tony Camuso <tcamuso@...>
Date: Sunday, January 13, 2008 - 4:51 pm

On 1/13/2008 1:41 PM, Arjan van de Ven wrote:





I think I got your point the first time, and I agree it is sound. But in 
my subjective and biased opinion,  I just think ext-conf-space is 
already useful and widespread enough (being used is not the same as 
being strictly required for basic operation) for your proposed tradeoff 
to not be optimal (protecting against "future/non-proven" hardware bugs, 
i.e. bringing non-proven benefits, at the expense of making life harder 
for ext-conf-space users while bringing additional extra API/code).


To take an example from the linux tree: the driver/pci/pcie/aer code 
uses ext-conf-space for every pcie-root (currently several distributions 
enable it by default), does it mean opt-in would be automatically 
activated for most pcie hierarchies (defeating most of the benefits of 
being opt-in), or we just disable that code by default?


Does lspci -v will automatically opt-in all pcie (right now by default 
it tries to list the extended-capabilities for pcie and pcix), or do we 
now require manual explicit sysfs operations to get the whole thing? Is 
is an additional flag to lspci (if so will that flag also apply to pcix, 
possibly causing a crash for lspci -v 
-<opt-in-all-potential-ext-devices> on some machines).






To go one step your direction, I have already argued in a couple of 
emails that I would prefer to not implement ext-conf-space access for 
any PCI-X devices (removing PCI-X2 from pci_ext_cfg_size), because there 
we are trying to support devices that we don't really know exists or 
will ever exists. And protecting against "unproven bugs" makes more 
sense when it only removes "unproven benefits".





FWIW, I have in my tree a patch almost identical to Ivan's dated 
"December 2005". Because of the constant activity on the mmconfig front 
(that I thought would make it obsolete), I never took the effort of 
suggesting it before one month ago (I am not a regular user of 
linux-kernel). I admit nobody else should view it that way, but for me  
rather than the last attempt at fixing mmconfig, it's a patch first used 
two years ago that would have arguably prevented all problems that have 
been reported since then.

Besides, recent mails show that hypothetically, we could even not change 
anything to the existing conf-space code, since the only known bug 
remaining is the one associated with bar probing and could be adressed by:

http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.24-rc6/2.6.24-rc6-m...


[ Thanks to Robert hancok and Grant Grundler for explaining to me the 
history of bar-probing last month  ]


Even if  that bar-probing patch was applied (maybe it needs to be more 
combat-proven), by default, it still seems  better to not use mmconfig 
for legacy-conf-space access, but going two extra precaution steps 
beyond what seems necessary might be excessive.







You can indeed never exclude 100% that possibility, but if they see a 
problem again, it is likely to be a new category of hardware/BIOS bugs 
never seen before.



Loic

--
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
Re: [Patch v2] Make PCI extended config space (MMCONFIG) a d..., Arjan van de Ven, (Thu Dec 27, 10:09 am)
Re: [Patch v2] Make PCI extended config space (MMCONFIG) a d..., Øyvind Vågen Jægtnes, (Tue Jan 15, 8:58 am)
Re: [Patch v2] Make PCI extended config space (MMCONFIG) a d..., Loic Prylli, (Sun Jan 13, 4:51 pm)
Re: [Patch v2] Make PCI extended config space (MMCONFIG) a d..., Arjan van de Ven, (Sun Jan 13, 12:42 am)
Re: [Patch v2] Make PCI extended config space (MMCONFIG) a d..., Arjan van de Ven, (Mon Jan 14, 10:46 am)
Re: [Patch v2] Make PCI extended config space (MMCONFIG) a d..., Arjan van de Ven, (Mon Jan 14, 12:01 pm)
Re: [Patch v2] Make PCI extended config space (MMCONFIG) a d..., Arjan van de Ven, (Sat Jan 12, 11:46 am)
Re: [Patch v2] Make PCI extended config space (MMCONFIG) a d..., Benjamin Herrenschmidt, (Sun Jan 13, 3:08 am)
Re: [Patch v2] Make PCI extended config space (MMCONFIG) a d..., Arjan van de Ven, (Mon Jan 28, 11:05 pm)
Re: [Patch v2] Make PCI extended config space (MMCONFIG) a d..., Arjan van de Ven, (Wed Jan 30, 11:42 am)
Re: [Patch v2] Make PCI extended config space (MMCONFIG) a d..., Arjan van de Ven, (Tue Jan 29, 10:47 am)
Re: [Patch v2] Make PCI extended config space (MMCONFIG) a d..., Arjan van de Ven, (Tue Jan 29, 11:29 am)
[PATCH] Change pci_raw_ops to pci_raw_read/write, Matthew Wilcox, (Mon Jan 28, 11:03 pm)
Re: [PATCH] Change pci_raw_ops to pci_raw_read/write, Tony Camuso, (Thu Feb 7, 11:54 am)
Re: [PATCH] Change pci_raw_ops to pci_raw_read/write, Matthew Wilcox, (Sat Feb 9, 8:41 am)
Re: [PATCH] Change pci_raw_ops to pci_raw_read/write, Yinghai Lu, (Sun Feb 10, 2:25 am)
Re: [PATCH] Change pci_raw_ops to pci_raw_read/write, Matthew Wilcox, (Sun Feb 10, 10:51 am)
Re: [PATCH] Change pci_raw_ops to pci_raw_read/write, Yinghai Lu, (Sun Feb 10, 4:16 pm)
Re: [PATCH] Change pci_raw_ops to pci_raw_read/write, Linus Torvalds, (Sun Feb 10, 4:24 pm)
Re: [PATCH] Change pci_raw_ops to pci_raw_read/write, Matthew Wilcox, (Sun Feb 10, 4:45 pm)
Re: [PATCH] Change pci_raw_ops to pci_raw_read/write, Yinghai Lu, (Sun Feb 10, 9:49 pm)
Re: [PATCH] Change pci_raw_ops to pci_raw_read/write, Andrew Morton, (Mon Feb 11, 6:10 pm)
Re: [PATCH] Change pci_raw_ops to pci_raw_read/write, Ingo Molnar, (Mon Feb 11, 6:38 pm)
Re: [PATCH] Change pci_raw_ops to pci_raw_read/write, Robert Hancock, (Sun Feb 10, 10:53 pm)
Re: [PATCH] Change pci_raw_ops to pci_raw_read/write, Yinghai Lu, (Mon Feb 11, 1:59 am)
raw_pci_read in quirk_intel_irqbalance, Matthew Wilcox, (Sun Feb 10, 7:02 pm)
Re: raw_pci_read in quirk_intel_irqbalance, Matthew Wilcox, (Mon Feb 11, 1:04 am)
Re: raw_pci_read in quirk_intel_irqbalance, Grant Grundler, (Mon Feb 11, 3:49 am)
Re: raw_pci_read in quirk_intel_irqbalance, Matthew Wilcox, (Mon Feb 11, 12:15 pm)
Re: raw_pci_read in quirk_intel_irqbalance, Linus Torvalds, (Mon Feb 11, 1:18 pm)
Re: raw_pci_read in quirk_intel_irqbalance, Grant Grundler, (Mon Feb 11, 3:38 pm)
Re: [PATCH] Change pci_raw_ops to pci_raw_read/write, Matthew Wilcox, (Sun Feb 10, 4:19 pm)
Re: [PATCH] Change pci_raw_ops to pci_raw_read/write, Yinghai Lu, (Sun Feb 10, 4:25 pm)
Re: [PATCH] Change pci_raw_ops to pci_raw_read/write, Matthew Wilcox, (Sun Feb 10, 4:32 pm)
Re: [PATCH] Change pci_raw_ops to pci_raw_read/write, Yinghai Lu, (Sun Feb 10, 4:47 pm)
Re: [PATCH] Change pci_raw_ops to pci_raw_read/write, Grant Grundler, (Sun Feb 10, 3:13 pm)
Re: [PATCH] Change pci_raw_ops to pci_raw_read/write, Matthew Wilcox, (Sun Feb 10, 3:37 pm)
Re: [PATCH] Change pci_raw_ops to pci_raw_read/write, Arjan van de Ven, (Thu Feb 7, 12:28 pm)
Re: [PATCH] Change pci_raw_ops to pci_raw_read/write, Tony Camuso, (Thu Feb 7, 12:36 pm)
Re: [PATCH] Change pci_raw_ops to pci_raw_read/write, Grant Grundler, (Thu Feb 7, 10:28 pm)