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

[Linus Torvalds]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/