Re: [PATCH 3/4] x86, pci: Add interface to force mmconfig

From: Thomas Gleixner <tglx@linutronix.de>
To: Andi Kleen <andi@firstfloor.org>
Cc: bhelgaas@google.com, x86@kernel.org, linux-pci@vger.kernel.org,
	eranian@google.com, Peter Zijlstra <peterz@infradead.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Andi Kleen <ak@linux.intel.com>
Subject: Re: [PATCH 3/4] x86, pci: Add interface to force mmconfig
Date: Tue, 14 Mar 2017 14:55:30 +0100 (CET)	[thread overview]
Message-ID: <alpine.DEB.2.20.1703141407090.3619@nanos> (raw)
In-Reply-To: <20170302232104.10136-3-andi@firstfloor.org>

On Thu, 2 Mar 2017, Andi Kleen wrote:
> +struct pci_ops pci_mmconfig_ops = {
> +	.read = pci_mmconfig_read,
> +	.write = pci_mmconfig_write,
> +};
> +
> +/* Force all config accesses to go through mmconfig. */
> +int pci_bus_force_mmconfig(struct pci_bus *bus)
> +{
> +	if (!raw_pci_ext_ops)
> +		return -1;

We have error defines. That aside, the weak version of this returns 0,
i.e. success, but here you return fail. Consistency is overrated, right?

> +	bus->ops = &pci_mmconfig_ops;

What guarantees that raw_pci_ext_ops == pci_mmcfg? Nothing as far as I can
tell. So that function name is nonsensical. It does not force anything.

And the way how this function is used is a horrible hack. It's called from
a random driver at some random point in time.

The proper solution is to identify the bus at the point where the bus is
discovered and switch it to mmconfig if possible.

Thanks,

	tglx