Date: Tue, 30 Oct 2007 17:41:26 -0600
From: Robert Hancock <hancockr@shaw.ca>
Subject: Re: pci-disable-decode-of-io-memory-during-bar-sizing.patch
In-reply-to: <alpine.LFD.0.999.0710300957260.30120@woody.linux-foundation.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Arjan van de Ven <arjan@infradead.org>, Greg KH <greg@kroah.com>,
       Jesse Barnes <jbarnes@virtuousgeek.org>, akpm@linux-foundation.org,
       ak@suse.de, rajesh.shah@intel.com,
       linux-kernel <linux-kernel@vger.kernel.org>
Message-id: <4727C126.8070000@shaw.ca>
MIME-version: 1.0
Content-type: text/plain; charset=ISO-8859-1; format=flowed
Content-transfer-encoding: 7bit
References: <200708151919.l7FJJfUE010966@imap1.linux-foundation.org>
 <200710251622.36773.jbarnes@virtuousgeek.org>
 <20071026025407.GA21408@kroah.com>
 <200710260959.46811.jbarnes@virtuousgeek.org>
 <20071027024140.GC29039@kroah.com> <47267232.3020506@shaw.ca>
 <alpine.LFD.0.999.0710300811230.30120@woody.linux-foundation.org>
 <20071030094756.779ac5c0@laptopd505.fenrus.org>
 <alpine.LFD.0.999.0710300957260.30120@woody.linux-foundation.org>
User-Agent: Thunderbird 2.0.0.6 (Windows/20070728)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 4462
Lines: 105

Linus Torvalds wrote:
> 
> On Tue, 30 Oct 2007, Arjan van de Ven wrote:
>> the problem is... you're not supposed to mix both types of accesses.
> 
> You have to, anyway. Even now the MMCONFIG stuff uses CONF1 cycles for 
> startup.

If it does, it's not by necessity. As soon as you read the table 
location out of the ACPI tables you can start using it, and that 
shouldn't require any config space accesses.

> 
> Also, there's reason to believe that mixing things up _has_ to work 
> anyway, and if the issue is between "works in practice" and "theory says 
> that you shouldn't mix", I'll take practice every time.
> 
> Especially since we *know* that the theory is broken. Right now MMCONFIG 
> is effectively disabled very aggressively because it's simply unusably 
> flaky. So the choice is between:
> 
>  - don't use MMCONFIG at all, because it has so many problems
>  - use MMCONFIG sparingly enough to hide the problems

Fact is, we don't really know how many of these systems with supposedly 
"broken" MMCONFIG were really just suffering from the overlapping 
PCI/MMCONFIG address space problem, which is entirely the fault of the 
way we do PCI probing. I would bet quite a few of them.

> 
> and what "you're supposed to do" is simply trumped by Real Life(tm). 
> Because Intel screwed up so badly when they designed that piece of shit.
> 
> (Where "screwed up badly" is the usual "left it to firmware people" thing, 
> of course. Dammit, Intel *could* have just made it a real PCI BAR in the 
> Northbridge, and specified it as such, and we wouldn't have these 
> problems! But no, it had to be another idiotic "firmware tells where it 
> is" thing)

This wouldn't have helped anything with the problem in question.

> 
>>> The fact is, CONF1 style accesses are just safer, and *work*. 
>> I would suggest a slight twist then: use CONF1 *until* you're using
>> something above 256, and then and only then switch to MMCONFIG from
>> then on for all accesses.
> 
> No.
> 
> Maybe if you do it per-device, and only *after* probing (ie we have seen 
> multiple, and successful, accesses), but globally, absolutely not. That 
> would be useless. The bugs we have had in this area have been exactly the 
> kinds of things like "we don't know the real size of the MMCONFIG areas" 
> etc.
> 
> I could easily see device driver writers probing to see if something 
> works, and I absolutely don't think we should just automatically enable 
> MMCONFIG from then on.

Why per device? It's not like the MSI case where both the platform and 
the device are potentially busted. Whether or not MMCONFIG works has 
nothing to do with the device, all that matters is whether it works on 
the platform. It shouldn't be the driver's responsibility to know this.

> 
> But maybe we could have a per-device flag that a driver *can* set. Ie have 
> the logic be:
> 
>  - use MMCONFIG if we have to (reg >= 256)
> 
> OR
> 
>  - use MMCONFIG if the driver specifically asked us to
> 
> and then drivers that absolutely need it, and know they do, can set that 
> flag. Preferably after they actually verified that it works.

How will they verify that it works? If it works, then verifying it works 
is all well and good. If it doesn't work, trying to verify if it does 
could very well blow up the machine.

I've made the point before that if we're going to allow using it at all, 
we'd better find out if it works or not early on, not after we've been 
running and somebody decides it's a good idea to try using it and 
causing a lockup or something.

> 
> That way you _can_ get the "this is how you're supposed to do it" 
> behaviour, but you get it when there is a reasonable chance that it 
> actually works.
> 
> And quite frankly, if you're not supposed to mix these things even across 
> devices, then I think we are better off just doing what we effectively do 
> now: mostly ignore the damn thing because it's too broken to use.
> 
> Maybe somebody inside Intel could just clarify the documentation, and 
> change it from "you're not supposed to mix" to "mix all you want". 

Intel could say what they want on the subject.. but that doesn't 
necessarily reflect what happens with anyone else's chipset implementations.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/