[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Pine.LNX.4.64.0610051821280.3952@g5.osdl.org>
Date: Thu, 5 Oct 2006 18:29:03 -0700 (PDT)
From: Linus Torvalds <torvalds@...l.org>
To: Jeff Garzik <jeff@...zik.org>
cc: Andi Kleen <ak@...e.de>, discuss@...-64.org,
linux-kernel@...r.kernel.org
Subject: Re: [discuss] Re: Please pull x86-64 bug fixes
On Thu, 5 Oct 2006, Jeff Garzik wrote:
> Linus Torvalds wrote:
> > (And we should probably have the "pci=mmiocfg" kernel command line entry
> > that forces MMIOCFG regardless of any e820 issues, even for normal
> > accesses).
>
> Something like this?
Yes, except I look at the resulting messy conditional:
if ((type == 1) && (!(pci_probe & PCI_NO_CHECKS)) &&
!e820_all_mapped(pci_mmcfg_config[0].base_address,
pci_mmcfg_config[0].base_address + MMCONFIG_APER_MIN,
E820_RESERVED)) {
and it's totally unreadable, so how about making this a helper inline
function like
static inline int validate_mmcfg(int type, unsigned long address)
{
/*
* If type 1 accesses don't work, assume we run on a
* Mac and always use MCFG
*/
if (type != 1)
return 1;
/*
* If the user asked us to not check the MMIOCFG base
* address, just trust it.
*/
if (pci_probe & PCI_NO_CHECKS)
return 1;
/*
* Otherwise require that it's marked reserved in
* the e820 tables
*/
return e820_all_mapped(address,
address + MMCONFIG_APER_MIN,
E820_RESERVED);
}
and then just saying
if (!validate_mmcfg(type, pci_mmcfg_config[0].base_address)) {
.. complain and exit ..
which just seems a hell of a lot prettier to me.
It also means that _if_ we ever figure out other ways to double-check the
address automatically (or we have some automatic way of saying "that can't
be valid"), we can do so in that "validate" function, without adding more
and more horrible code.
What say you? Let's try to keep the code readable (and preferably make it
more so..)
Linus
-
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/
Powered by blists - more mailing lists