Re: [PATCH] i2c: allow ddr5 ram page change with active intel spd write protection

[Jean Delvare <jdelvare@suse.de>]

Hi Florian,

On Sat, 11 Oct 2025 14:31:53 +0200, Blacktempel (Florian K.) wrote:
> SPD of most Intel DDR5 systems is write protected.
> The write protection also includes changing page, via MR11 register, and prevents reading data.
> 
> This patch allows page changing with write protection active via a I2C_SMBUS_READ on PROC_CALL.
> That is a, publicly undocumented, Intel specific way to allow page change.
> 
> Link: https://github.com/Blacktempel/RAMSPDToolkit/issues/4
> Link: https://github.com/Blacktempel/RAMSPDToolkit/blob/9b2aeab9b7637be1874520c74c9873e174fe4947/RAMSPDToolkit/SPD/DDR5AccessorBase.cs#L237
> Link: https://github.com/namazso/PawnIO.Modules/pull/19/files#diff-43cf449eaf4b834c447bc85ad039882fbd4fc883be00447908b94eb9cc8a9c36R481
> Signed-off-by: Blacktempel (Florian K.) <Blacktempel@hotmail.de>
> ---
>  drivers/i2c/busses/i2c-i801.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
> index cba992fa6557..9d8eda8b3287 100644
> --- a/drivers/i2c/busses/i2c-i801.c
> +++ b/drivers/i2c/busses/i2c-i801.c
> @@ -788,7 +788,7 @@ static int i801_simple_transaction(struct i801_priv *priv, union i2c_smbus_data
>  		xact = I801_WORD_DATA;
>  		break;
>  	case I2C_SMBUS_PROC_CALL:
> -		i801_set_hstadd(priv, addr, I2C_SMBUS_WRITE);
> +		i801_set_hstadd(priv, addr, read_write);
>  		iowrite8(data->word & 0xff, SMBHSTDAT0(priv));
>  		iowrite8((data->word & 0xff00) >> 8, SMBHSTDAT1(priv));
>  		iowrite8(hstcmd, SMBHSTCMD(priv));

This looks similar to the workaround we applied for AT24-based SPD
implementations many years ago:

  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ba9ad2af7019956b990ad654c56da5bac1e8b71b

Because all datasheets since the 82801AA explicitly say that "for
process call command, the value written into bit 0 of the Transmit
Slave Address Register (SMB I/O register, offset 04h) needs to be 0", I
would prefer to have a similar implementation for the SMBus Process
Call: check SMBHSTCFG_SPD_WD and only apply the workaround if that bit
is set. This will avoid risking a regression on older chipsets.

I checked the spd5118 driver (which implements DDR5 SPD EEPROM support)
and it is NOT using the SMBus Process Call. So the above change alone
isn't going to allow accessing DDR5 SPD data on Intel systems where SPD
write protection is enabled. And the spd5118 driver is built on top of
regmap which is a generic register map access implementation.
Convincing it to use the SMBus Process Call transfer type for page
selection won't be trivial, if possible at all.

Are you using a modified spd5118 driver? Or are you accessing the SPD
EEPROM from user-space using the i2c-dev driver?

I must confess I'm curious how using the SMBus Process Call for the
purpose of switching the pages could work safely. This transfer type
will write 3 bytes on the bus, while page switching only expects 2. The
third written byte could have unexpected side effects, such as writing
to the following register (MR12). I'm not sure that's a risk I would be
willing to take. Then the process call will attempt to read 2 bytes
from the SPD, I'm not sure how the SPD will react, but that should be
less of a problem.

If Intel messed up their Write Protect implementation for the DDR5 SPD
standard then they should really fix it in future chipsets.

-- 
Jean Delvare
SUSE L3 Support