linux-kernel - Re: [PATCH v4 5/6] i2c: smbus: Support DDR5 SPD EEPROMs

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: <79f406ae-cfc9-48bb-9c80-20f998c40b69@roeck-us.net>
Date: Mon, 10 Jun 2024 08:55:26 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Wolfram Sang <wsa+renesas@...g-engineering.com>,
 Armin Wolf <W_Armin@....de>, linux-hwmon@...r.kernel.org,
 linux-i2c@...r.kernel.org, linux-kernel@...r.kernel.org,
 devicetree@...r.kernel.org, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 René Rebe <rene@...ctcode.de>,
 Thomas Weißschuh <linux@...ssschuh.net>,
 Stephen Horvath <s.horvath@...look.com.au>
Subject: Re: [PATCH v4 5/6] i2c: smbus: Support DDR5 SPD EEPROMs

On 6/10/24 07:52, Wolfram Sang wrote:
> 
>>> Yes, maybe this could be simplified to "(LP)DDR memory types"
>>>
>>
>> I rephrased it to "Only works for (LP)DDR memory types up to DDR5".
> 
> Thanks!
> 
>> How about "Only works on systems with 1 to 8 memory slots" ?
> 
> This is a question for Heiner. I'd think it is is still correct, but I
> don't know exactly.
> 

My interpretation was that it should work if the DIMMs are connected to
multiplexed I2C busses, but probably not if they are connected to
different adapters. The error message in that case is a bit misleading,
though, because it claims that "More than 8 memory slots on a single bus",
which isn't necessarily the case. For example, it should be perfectly valid
to have up to 24 DIMMs in this system.

i2c-0/name:SMBus PIIX4 adapter port 0 at 0b00
i2c-1/name:SMBus PIIX4 adapter port 2 at 0b00
i2c-2/name:SMBus PIIX4 adapter port 1 at 0b20

... but I guess that is a question for someone with such a system to answer.

Ultimately the handling of systems with more than 8 memory slots will need
to be updated at some point. On my systems, with 'i2c: piix4: Register SPDs'
applied, I see

i2c i2c-0: 4/4 memory slots populated (from DMI)
     [my system is running 6.6.y which still generates that message]
i2c i2c-0: Successfully instantiated SPD at 0x50
i2c i2c-0: Successfully instantiated SPD at 0x51
i2c i2c-0: Successfully instantiated SPD at 0x52
i2c i2c-0: Successfully instantiated SPD at 0x53
i2c i2c-1: 4/4 memory slots populated (from DMI)
i2c i2c-2: 4/4 memory slots populated (from DMI)

meaning the function is called for each adapter (which makes sense).
However, the code counting the DIMMs doesn't really take the adapter
into account, meaning adapters 1 and 2 are still probed even though
all DIMMs were already instantiated from adapter 0.

On a system with more than 8 DIMMs connected to different piix4 adapters
(without mux) we'd probably see something like

i2c i2c-0: More than 8 memory slots on a single bus, contact i801 maintainer ...
i2c i2c-1: More than 8 memory slots on a single bus, contact i801 maintainer ...
i2c i2c-2: More than 8 memory slots on a single bus, contact i801 maintainer ...

which wouldn't be very helpful. I think the main problem may be that
the i801 driver implements sub-adapters as muxes, but the piix4 driver
doesn't do (or need) that. The message is also i801 centric which doesn't
apply anymore after 'i2c: piix4: Register SPDs' is applied.

However, I would not want to even try changing that code without access
to a system using piix4 and supporting more than 8 memory slots.

Thanks,
Guenter