Re: [discuss] Re: Please pull x86-64 bug fixes

Linus Torvalds
2006-10-06T01:29:14Z

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