| 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
| ||
|
Date: Tue, 2 Nov 2021 18:03:18 +0100
From: Stefan Roese <sr@...x.de>
To: Pali Rohár <pali@...nel.org>,
"Maciej W. Rozycki" <macro@...am.me.uk>
Cc: Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
Russell King <linux@...linux.org.uk>,
Andrew Lunn <andrew@...n.ch>,
Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>,
Gregory Clement <gregory.clement@...tlin.com>,
Jason Gunthorpe <jgg@...dia.com>,
Marek Behún <kabel@...nel.org>,
linux-arm-kernel@...ts.infradead.org, linux-mips@...r.kernel.org,
linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] PCI: Marvell: Update PCIe fixup
On 02.11.21 16:48, Pali Rohár wrote:
> On Tuesday 02 November 2021 15:49:29 Pali Rohár wrote:
>> On Tuesday 02 November 2021 14:01:41 Maciej W. Rozycki wrote:
>>> On Tue, 2 Nov 2021, Pali Rohár wrote:
>>>
>>>>> None of the Galileo system controllers I came across had the class code
>>>>> set incorrectly.
>>>>
>>>> In kernel there is quirk only for one device with id:
>>>> PCI_VENDOR_ID_MARVELL (0x11ab) PCI_DEVICE_ID_MARVELL_GT64111 (0x4146)
>>>>
>>>> So for some reasons quirk is needed... Anyway, patch for this quirk just
>>>> adds comment as there is no explanation for it. It does not modify quirk
>>>> code.
>>>>
>>>> So it is possible that Marvell (or rather Galileo at that time) included
>>>> some config space fixup in some products and 0x4146 did not have it.
>>>> Just guessing... We can really only guess what could happen at that time
>>>> 20 years ago...
>>>
>>> Ah, there you go! -- sadly I don't seem to have a copy of the datasheet
>>> for the GT-64111, but the GT-64115 has it[1]:
>>>
>>> Table 158: PCI Class Code and Revision ID, Offset: 0x008
>>> Bits Field name Function Initial Value
>>> 7:0 RevID Indicates the GT-64115 PCI Revision 0x01
>>> number.
>>> 15:8 Reserved Read only. 0x0
>>> 23:16 SubClass Indicates the GT-64115 Subclass - Mem- 0x80
>>> ory controller.
>>> 31:24 BaseClass Indicates the GT-64115 Base Class - 0x05
>>> memory controller.
>>>
>>> and then:
>>>
>>> "Device and Vendor ID (0x000), Class Code and Revision ID (0x008), and
>>> Header Type (0x00e) fields are read only from the PCI bus. These fields
>>> can be modified and read via the CPU bus."
>>>
>>> Likewise with the GT-64120[2]:
>>>
>>> Table 208: PCI_0 Class Code and Revision ID, Offset: 0x008 from PCI_0 or CPU; 0x088 from
>>> PCI_1
>>> Bits Field name Function Initial Value
>>> 7:0 RevID Indicates the GT-64120 PCI_0 revision number. 0x02
>>> 15:8 Reserved Read Only 0. 0x0
>>> 23:16 SubClass Indicates the GT-64120 Subclass Depends on value
>>> 0x00 - Host Bridge Device. sampled at reset
>>> 0x80 - Memory Device. on BankSel[0]. See
>>> Table 44 on page
>>> 11-1.
>>> 31:24 BaseClass Indicates the GT-64120 Base Class Depends on value
>>> 0x06 - Bridge Device. sampled at reset
>>> 0x05 - Memory Device. on BankSel[0]. See
>>> Table 44 on page
>>> 11-1.
>>>
>>> Table 209: PCI_1 Class Code and Revision ID, Offset: 0x088 from PCI_0 or CPU; 0x008 from
>>> PCI_1
>>> Bits Field name Function Initial Value
>>> 31:0 Various Same as for PCI_0 Class Code and Revision ID.
>>>
>>> and then also:
>>>
>>> "Device and Vendor ID (0x000), Class Code and Revision ID (0x008), and
>>> Header Type (0x00e) fields are read only from the PCI bus. These fields
>>> can be modified and read via the CPU bus."
>>>
>>> -- so this is system-specific and an intended chip feature rather than an
>>> erratum (or rather it is a system erratum if the reset strap or the boot
>>> firmware has got it wrong).
>>>
>>> The memory device class code is IIUC meant to be typically chosen when
>>> the Galileo/Marvell device is used without the CPU interface, i.e. as a
>>> PCI memory controller device only[3].
>
> I have found on internet some copy of GT64111 datasheet ("GT-64111
> System Controller for RC4640, RM523X and VR4300 CPUs", Galileo
> Technology, Product Preview Revision 1.1, FEB 4, 1999) and in section
> "17.15 PCI Configuration Registers" there is subsection "Class Code and
> Revision ID, Offset: 0x008" with content:
>
> Bits Field name Function Initial Value
> 7:0 RevID Indicates the GT-64111 Revision number. 0x10
> GT-64111-P-0 = 0x10
> 15:8 Reserved 0x0
> 23:16 SubClass Indicates the GT-64111 Subclass (0x80 - other mem- 0x80
> ory controller)
> 31:24 BaseClass Indicates the GT-64111 Base Class (0x5 - memory 0x05
> controller).
>
> And in section "6.5.3 PCI Autoconfiguration at RESET" is following
> interesting information:
>
> Eight PCI registers can be automatically loaded after Rst*.
> Autoconfiguration mode is enabled by asserting the DMAReq[3]* LOW on
> Rst*. Any PCI transactions targeted for the GT-64111 will be retried
> while the loading of the PCI configuration registers is in process.
>
> It is highly recommended that all PC applications utilize the PCI
> Autoconfiguration at RESET feature. The autoload feature can be easily
> implemented with a very low cost EPLD. Galileo provides sample EPLD
> equations upon request. (You can always pull the EPLD off your final
> product if you find there are no issues during testing.)
>
> NOTE: The GT-64111’s default Class Code is 0x0580 (Memory Controller)
> which is a change from the GT-64011.
>
> The GT-64011 used the Class Code 0x0600 which denotes Host Bridge. Some
> PCs refuse to configure host bridges if they are found plugged into a
> PCI slot (ask the BIOS vendors why...). The “Memory Controller” Class
> Code does not cause a problem for these non-compliant BIOSes, so we used
> this as the default in the GT-64111. The Class Code can be reporgrammed
> in both devices via autoload or CPU register writes.
>
> The PCI register values are loaded from the ROM controlled by BootCS*
> are shown in Table 21, below.
>
> TABLE 21. PCI Registers Loaded at RESET
> Register Offset Boot Device Address
> Device and Vendor ID 0x000 0x1fffffe0
> Class Code and Revision ID 0x008 0x1fffffe4
> Subsystem Device and Vendor ID 0x02c 0x1fffffe8
> Interrupt Pin and Line 0x03c 0x1fffffec
> RAS[1:0]* Bank Size 0xc08 0x1ffffff0
> RAS[3:2]* Bank Size 0xc0c 0x1ffffff4
> CS[2:0]* Bank Size 0xc10 0x1ffffff8
> CS[3]* & Boot CS* Bank Size 0xc14 0x1ffffffc
>
> ===
>
> So the conclusion is that there is also some RESET configuration via
> BootCS (I have no idea what it is or was). And default value (when RESET
> configuration is not used) is always "Memory controller" due to
> existence of "broken PC BIOSes" (probably x86).
>
> Hence the quirk for GT64111 in kernel is always needed. And Thomas
> already confirmed in his pci hexdump that PCI Class code is set to
> Memory Controller.
>
> I hope that now this mystery of this GT64111 quirk is solved :-) I will
> update patch with correct comment, why quirk is needed.
>
> So due to the fact that 20 years ago there were broken x86 BIOSes which
> did not like PCI devices with PCI Class code of Host Bridge, Marvell
> changed default PCI Class code to Memory Controller and let it in this
> state also for future PCIe-based ARM and AR64 SoCs for next 20 years.
> Which generally leaded to broken PCIe support in mvebu SoCs. I have no
> more comments about it... :-(
If this is really the case, that all this was "copied" in such a bad
design state into newer SoC's for that many years, which I don't doubt
right now any more, then this is absolutely amazing and pretty sad IMHO.
Pali, many thanks for being persistant enough to dig through this.
Thanks,
Stefan
Powered by blists - more mailing lists