linux-kernel - Re: [PATCH] pci: fix bridge 64bit flag setting

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: <4B15604D.2090202@kernel.org>
Date:	Tue, 01 Dec 2009 10:28:29 -0800
From:	Yinghai Lu <yinghai@...nel.org>
To:	Bjorn Helgaas <bjorn.helgaas@...com>
CC:	Jesse Barnes <jbarnes@...tuousgeek.org>,
	Alex Williamson <alex.williamson@...com>,
	linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
	Grant Grundler <grundler@...isc-linux.org>,
	Ivan Kokshaysky <ink@...assic.park.msu.ru>,
	Alex Chiang <achiang@...com>
Subject: Re: [PATCH] pci: fix bridge 64bit flag setting

Bjorn Helgaas wrote:
> On Tuesday 01 December 2009 12:03:57 am Yinghai Lu wrote:
>> Alex found one system that one pci bridge pref mmio 64 is not set correctly.
>> aka, the upper32 base/limit is not cleaned.
>> he found that bridge is supporting 64 bit pref mmio, but device under that
>> does not support that. so that IORESOURCE_MEM_64 get cleared in pbus_size_mem()
> 
> I think it's wrong that pbus_size_mem() fiddles with IORESOURCE_MEM_64
> in bus resources based on where BARs of devices on that bus live.  That
> feels fragile.

yes. need more clean up.

> 
> The question of whether the bridge supports 64-bit apertures is
> strictly a hardware property of the bridge.  It has nothing to do
> with where we place downstream devices.
> 
> Is there really a problem with writing to PCI_PREF_BASE_UPPER32
> unconditionally?  As Alex pointed out, per 3.2.5.10 of the bridge
> spec, the UPPER32 registers are read-only if only 32-bit apertures
> are supported.  If you mentioned a problem with doing this
> unconditionally, I missed it.

remember that some x2apic registers say they are reserved, and read them could cause GP error.

and for pci devices, if it is read-only, for good design, then write it should be ok.
but if there is some design problem in devices, and they could say those are read-only.
why kernel write value to it?

> 
> The only place we test IORESOURCE_MEM_64 for a bus resource is when
> we're programming PCI_PREF_BASE_UPPER32.  If we think it's important
> to program it conditionally, why don't we skip IORESOURCE_MEM_64

IORESOURCE_MEM_64 is used to control if we can use MMIO > 4g.

> altogether, and just look at the bits in PCI_PREF_MEMORY_BASE directly?
> E.g., something like this:
> 
> 	pci_read_config_dword(bridge, PCI_PREF_MEMORY_BASE, &l);
> 	if ((l & PCI_PREF_RANGE_TYPE_MASK) == PCI_PREF_RANGE_TYPE_64) {
> 		pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, bu);
> 		pci_write_config_dword(bridge, PCI_PREF_LIMIT_UPPER32, lu);
> 	}
> 
> Then we don't have to maintain flags at all, and it's easy to verify
> that the code corresponds to the spec.

that will have several extra read, and also we already store that bit in pci_read_bridge_bases()

but forget to set that in pci_check_bus_range()

YH

YH

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