All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff Garzik <jeff@garzik.org>
To: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Loic Prylli <loic@myri.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Arjan van de Ven <arjan@infradead.org>,
	linux-kernel@vger.kernel.org, gregkh@suse.de,
	linux-pci@atrey.karlin.mff.cuni.cz,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>
Subject: Re: [patch] Make MMCONFIG space (extended PCI config space) a	driver opt-in issue
Date: Mon, 24 Dec 2007 02:22:10 -0500	[thread overview]
Message-ID: <476F5E22.3060007@garzik.org> (raw)
In-Reply-To: <20071223110337.GA25441@jurassic.park.msu.ru>

Ivan Kokshaysky wrote:
> On Sun, Dec 23, 2007 at 12:44:30AM -0500, Jeff Garzik wrote:
>> Failures are more predictable and more consistent with an all-or-none 
>> scenario.  The all-or-none solutions are the least complex on the software 
>> side, and far more widely tested than any mixed config access scheme.
> 
> Nope. The vast majority of mmconfig problems that happen at boot time
> actually have nothing to do with "broken" or "working" mmconfig.
> Typically these are
> - mmconf area overlapped during BAR sizing;
> - BIOS (or kernel) placed some MMIO in the middle of mmconfig area,
>   which looks like some random device "doesn't like mmconfig".
> 
> This is a result of all-or-none approach, as mmconfig is fundamentally
> unsafe until after PCI init is done.

The phrase "all or none" specifically describes the current practice in 
arch/x86/pci/mmconfig_{32,64}.c whereby a PCI bus always has one, and 
only one, access method.

So the problems you describe are unrelated to "all or none" as I was 
describing it.


> The mixed access that Loic proposes allows to avoid these boot problems
> just for free.

False.  If you have overlapping areas, then clearly it is 
board-dependent (undefined) what happens -- you might trigger an 
mmconfig access by writel() to your PCI device's MMIO BAR space.

Consider, too, what the current code in arch/x86/pci/mmconfig_{32,64}.c 
does:  it uses the range BIOS told to use for mmconfig, no more no less.

Clearly many early mmconfig BIOSes have buggy resource allocation... 
Thus if mmconfig is not known working on a system, turn it off 100% for 
all buses.  End of story.


> Systems that have both non-mmconf PCI(X) and mmconf PCI-E
> domains could be handled almost for free as well.

The existing code in arch/x86/pci/mmconfig_{32,64}.c today handles 
mixing of PCI-E and PCI-X just fine.  As noted above, its done on a 
per-bus and per-segment basis today.


> And probably most important thing is that the x86 pci_conf implementation
> would be so much simpler and cleaner that I honestly don't understand
> why are we still discussing it ;-)

The proposed API adds code, so I don't see how it simplifies things.

The current approach is quite clean anyway:

1) "raw_pci_ops" points to a single set of PCI config accessors.
2) For mmconfig, if the BIOS did not tell us to use mmconfig with a 
given PCI bus, fall back to type1 accesses.


> At the same time, making drivers to request extended config space
> still makes sense. I think of pci_request_ext_conf(dev, drv_name) which
> doesn't set any per-device flag, but
> - returns success or failure depending on mmconf availability;
> - can be logged by kernel to make it easier to debug if something
>   goes wrong.

I agree with the function as noted in response to Linus's "Sound ok with 
this plan?" email.  But note -- users and developers also need to access 
extended config space, even if driver did not request it.  Even if there 
is no driver at all.

Otherwise the value of "lspci -vvvxxx" debugging reports from users is 
diminished.

	Jeff



  parent reply	other threads:[~2007-12-24  7:23 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-12-22 12:31 [patch] Make MMCONFIG space (extended PCI config space) a driver opt-in issue Arjan van de Ven
2007-12-22 12:35 ` [patch] opt the sky2 driver into using extended config space Arjan van de Ven
2007-12-22 20:28   ` Stephen Hemminger
2007-12-22 14:20 ` [patch] Make MMCONFIG space (extended PCI config space) a driver opt-in issue Jeff Garzik
2007-12-22 14:47   ` Arjan van de Ven
2007-12-22 15:54     ` Matthew Wilcox
2007-12-22 16:02       ` Arjan van de Ven
2007-12-22 21:10         ` Matthew Wilcox
2007-12-23 23:33           ` Benjamin Herrenschmidt
2007-12-22 18:06     ` Linus Torvalds
2007-12-22 19:30       ` Martin Mares
2007-12-22 20:15         ` Arjan van de Ven
2007-12-22 20:36           ` Martin Mares
2007-12-23  1:29           ` Jeff Garzik
2007-12-23  1:26       ` Jeff Garzik
2007-12-23  3:46         ` Linus Torvalds
2007-12-23  4:11           ` Jeff Garzik
2007-12-23  4:35             ` Linus Torvalds
2007-12-23  4:52               ` Jeff Garzik
2007-12-23  5:00                 ` Linus Torvalds
2007-12-23  5:09                   ` Jeff Garzik
2007-12-23 21:09                   ` Benjamin Herrenschmidt
2007-12-23 21:15                     ` Martin Mares
2007-12-23 22:32                       ` Ivan Kokshaysky
2007-12-23 23:06                       ` Benjamin Herrenschmidt
2007-12-23 23:19                         ` Benjamin Herrenschmidt
2007-12-23  5:27                 ` Loic Prylli
2007-12-23  5:44                   ` Jeff Garzik
2007-12-23  8:33                     ` Loic Prylli
2007-12-23 11:03                     ` Ivan Kokshaysky
2007-12-23 17:32                       ` Linus Torvalds
2007-12-24  7:31                         ` Jeff Garzik
2007-12-24 18:51                           ` Linus Torvalds
2007-12-24 21:22                             ` Matthew Wilcox
2007-12-27 11:46                             ` Jeff Garzik
2007-12-27 14:09                               ` Arjan van de Ven
2007-12-24  7:22                       ` Jeff Garzik [this message]
2007-12-24 15:47                         ` Ivan Kokshaysky
2007-12-23 21:13                     ` Benjamin Herrenschmidt
2007-12-23 10:33                 ` Arjan van de Ven
2007-12-24  7:04                   ` Jeff Garzik
2007-12-24 11:49                     ` Arjan van de Ven
2007-12-24 11:54                       ` Jeff Garzik
2007-12-24 12:00                         ` Arjan van de Ven
2007-12-25  9:22                           ` Martin Mares
2007-12-25  9:40                             ` Martin Mares
2007-12-23  4:13           ` Jeff Garzik
2007-12-23  4:18             ` Jeff Garzik
2007-12-23  4:44               ` Linus Torvalds
2007-12-23  5:04                 ` Jeff Garzik
2007-12-23  5:18                   ` Linus Torvalds
2007-12-23  5:22                     ` Jeff Garzik
2007-12-23  4:40             ` Linus Torvalds
2007-12-23 10:18               ` Martin Mares
2007-12-23  5:09             ` Loic Prylli
2007-12-23  5:57             ` H. Peter Anvin
2007-12-23  1:34     ` Jeff Garzik
2007-12-22 20:43 ` Benjamin Herrenschmidt
     [not found] <fa.pFsY/52FEkQYqrDPnPMxmcUOsRY@ifi.uio.no>
2007-12-22 16:22 ` Robert Hancock
2007-12-22 19:25   ` Greg KH
2007-12-23  0:56     ` Jeff Garzik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=476F5E22.3060007@garzik.org \
    --to=jeff@garzik.org \
    --cc=arjan@infradead.org \
    --cc=benh@kernel.crashing.org \
    --cc=gregkh@suse.de \
    --cc=ink@jurassic.park.msu.ru \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@atrey.karlin.mff.cuni.cz \
    --cc=loic@myri.com \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.