linux-kernel - Re: Commit 07f9b61 breaks systems that don't implement a

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAErSpo6aE-vxhOHk_mgyWMYrgkG0Juuy8Bs-S2Hjb_7k8S=n8Q@mail.gmail.com>
Date:	Fri, 4 Oct 2013 16:11:12 -0600
From:	Bjorn Helgaas <bhelgaas@...gle.com>
To:	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Ethan Zhao <ethan.zhao@...cle.com>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Yinghai Lu <yinghai@...nel.org>
Subject: Re: Commit 07f9b61 breaks systems that don't implement a _CBA method

On Thu, Oct 3, 2013 at 7:18 PM, Hedi Berriche <hedi@....com> wrote:
> Chaps,
>
> The following failure was encountered on hardware that does *not*
> implement a _CBA method which is AFAICT (and confirmed to me by BIOS
> chaps) optional.

_CBA is definitely optional.

> [    1.230647] PCI: MMCONFIG for domain 0000 [bus 00-0c] at [mem 0x80000000-0x80cfffff] (base 0x80000000)
> [    1.241046] PCI: MMCONFIG for domain 0001 [bus 00-02] at [mem 0xff84000000-0xff842fffff] (base 0xff84000000)
> [    1.252025] PCI: MMCONFIG for domain 1000 [bus 3f-3f] at [mem 0xff83f00000-0xff83ffffff] (base 0xff80000000)
> [    1.263006] PCI: MMCONFIG for domain 1001 [bus 3f-3f] at [mem 0xff87f00000-0xff87ffffff] (base 0xff84000000)
> [    1.273984] PCI: MMCONFIG at [mem 0x80000000-0x80cfffff] reserved in E820
> [    1.281564] PCI: MMCONFIG at [mem 0xff84000000-0xff842fffff] reserved in E820
> [    1.289535] PCI: MMCONFIG at [mem 0xff83f00000-0xff83ffffff] reserved in E820
> [    1.297505] PCI: MMCONFIG at [mem 0xff87f00000-0xff87ffffff] reserved in E820
>
> <snip>
>
> [    1.427926] ACPI: PCI Root Bridge [I001] (domain 0001 [bus 00-3d])
> [    1.434968] acpi PNP0A08:00: fail to add MMCONFIG information, can't access PCI configuration space under this host bridge.
> [    1.447405] acpi PNP0A08:00: Bus 0001:00 not present in PCI namespace
...

> Bisection points to this commit (included in full given its brevity):
>
> commit 07f9b61c3915e8eb156cb4461b3946736356ad02
> Author: ethan.zhao <ethan.zhao@...cle.com>
> Date:   Fri Jul 26 11:21:24 2013 -0600
>
>     x86/PCI: MMCONFIG: Check earlier for MMCONFIG region at address zero
>
>     We can check for addr being zero earlier and thus avoid the mutex_unlock()
>     cleanup path.
>
>     [bhelgaas: drop warning printk]
>     Signed-off-by: ethan.zhao <ethan.zhao@...cle.com>
>     Signed-off-by: Bjorn Helgaas <bhelgaas@...gle.com>
>     Acked-by: Yinghai Lu <yinghai@...nel.org>
...

> So the question is should commit
>
>         07f9b61 x86/PCI: MMCONFIG: Check earlier for MMCONFIG region at address zero
>
> be reverted? or am I missing something?

I think we should revert it.  As far as I can tell, it was only a
cleanup and it did not fix anything itself.  So since it did break
something, we should revert it.

It's regrettable that the code is so subtle.  The obvious thing to do
would be to check for _CBA, and if it doesn't exist, look for an MCFG
entry.  But for various historical reasons, the code is a mess, and we
haven't figured out how to safely simplify it.

Ethan, Yinghai, any objections to reverting 07f9b61?

Bjorn
--
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/