i2c-imc-skylake: guard branch - QUICK emulation and protect_spd
Downstream-only branch, not for submission. Two additions on top of the v4 driver: Approximate SMBus Quick with a byte-data read of register 0 so that i2cdetect and OpenRGB's DDR4 probe see the devices. The engine cannot do a zero-length transaction, so this is a real read against every probed address - acceptable on a known bus, not as a generic facility, which is why mainline does not get it. Add protect_spd (bool, 0644, default on): reject SMBus writes to the SPD EEPROMs (0x50-0x57) and to the DDR4 protection command addresses SWPn/CWP (0x30-0x35) with -EPERM, and log the attempt ratelimited. RSWP cannot be set in-system on this platform (SWPn/CWP require VHV on SA0), so a driver-level refusal is the strongest available protection against stray SPD writes from any i2c-dev client. The page-select addresses 0x36/0x37 stay writable because ee1004 needs them (Send Byte page select); ee1004 itself never writes the EEPROM contents. Toggle at runtime via /sys/module/i2c_imc_skylake/parameters/protect_spd for deliberate SPD maintenance, e.g. the documented repair recipe. The old compat branch predates 1.0.1 (no native Receive Byte, no ee1004 instantiation, no I2C_FUNC_SMBUS_BYTE, so ee1004 cannot even bind) and is renamed to attic/compat-stale-pre-1.0.1 so nobody builds on it.
This commit is contained in:
+61
-2
@@ -25,6 +25,25 @@
|
|||||||
* The two channels are independent SMBus buses (DIMMs 1,2 on ch0; DIMMs 3,4 on
|
* The two channels are independent SMBus buses (DIMMs 1,2 on ch0; DIMMs 3,4 on
|
||||||
* ch1), hence two i2c_adapter instances exposed as separate /dev/i2c-* nodes.
|
* ch1), hence two i2c_adapter instances exposed as separate /dev/i2c-* nodes.
|
||||||
*
|
*
|
||||||
|
* DOWNSTREAM BRANCH (guard). Not what is submitted upstream. Two local
|
||||||
|
* additions:
|
||||||
|
*
|
||||||
|
* - SMBus Quick is approximated with a byte-data read of register 0 so
|
||||||
|
* that i2cdetect can scan these buses. The engine always puts a
|
||||||
|
* register byte on the bus, so Quick cannot be implemented faithfully;
|
||||||
|
* a read against a known bus is acceptable here, not as a generic
|
||||||
|
* kernel facility.
|
||||||
|
*
|
||||||
|
* - protect_spd (default on) rejects SMBus writes to the SPD EEPROMs
|
||||||
|
* (0x50-0x57) and to the DDR4 protection command addresses SWPn/CWP
|
||||||
|
* (0x30-0x35) with -EPERM. RSWP cannot be set in-system on this
|
||||||
|
* platform (it needs VHV on SA0), so this is the strongest available
|
||||||
|
* protection against stray SPD writes. The page-select addresses
|
||||||
|
* 0x36/0x37 stay writable: ee1004 needs them for its Send Byte page
|
||||||
|
* select. Toggle at runtime via
|
||||||
|
* /sys/module/i2c_imc_skylake/parameters/protect_spd for deliberate
|
||||||
|
* SPD maintenance.
|
||||||
|
*
|
||||||
* Bus arbitration / concurrency with firmware:
|
* Bus arbitration / concurrency with firmware:
|
||||||
* The driver cannot coordinate with SMM, a BMC, or the iMC's own closed-loop
|
* The driver cannot coordinate with SMM, a BMC, or the iMC's own closed-loop
|
||||||
* thermal throttling (CLTT), and the PCI ID alone does not prove that none of
|
* thermal throttling (CLTT), and the PCI ID alone does not prove that none of
|
||||||
@@ -159,6 +178,11 @@ module_param(quiesce_on_shutdown, bool, 0644);
|
|||||||
MODULE_PARM_DESC(quiesce_on_shutdown,
|
MODULE_PARM_DESC(quiesce_on_shutdown,
|
||||||
"hand the engine back to the firmware quiesced at shutdown");
|
"hand the engine back to the firmware quiesced at shutdown");
|
||||||
|
|
||||||
|
static bool protect_spd = true;
|
||||||
|
module_param(protect_spd, bool, 0644);
|
||||||
|
MODULE_PARM_DESC(protect_spd,
|
||||||
|
"reject SMBus writes to SPD addresses 0x30-0x35 and 0x50-0x57 (default on)");
|
||||||
|
|
||||||
struct imc_chan {
|
struct imc_chan {
|
||||||
unsigned int cmd, data, stat;
|
unsigned int cmd, data, stat;
|
||||||
int idx; /* channel index 0 or 1 */
|
int idx; /* channel index 0 or 1 */
|
||||||
@@ -514,6 +538,25 @@ static s32 imc_smbus_xfer(struct i2c_adapter *adap, u16 addr,
|
|||||||
if (addr > 0x7f)
|
if (addr > 0x7f)
|
||||||
return -EINVAL;
|
return -EINVAL;
|
||||||
|
|
||||||
|
/*
|
||||||
|
* Downstream-only, see the note at the top of the file: approximate
|
||||||
|
* Quick with a byte-data read of register 0 so that i2cdetect can
|
||||||
|
* scan. Read rather than write, so a probe of an unknown address
|
||||||
|
* cannot store anything.
|
||||||
|
*/
|
||||||
|
if (size == I2C_SMBUS_QUICK) {
|
||||||
|
mutex_lock(&s->lock);
|
||||||
|
s->io_err = 0;
|
||||||
|
ret = imc_access(s, c, addr, 0, I2C_SMBUS_READ,
|
||||||
|
I2C_SMBUS_BYTE_DATA, &val);
|
||||||
|
if (s->io_err)
|
||||||
|
ret = s->io_err;
|
||||||
|
mutex_unlock(&s->lock);
|
||||||
|
dev_dbg(s->dev, "ch%d QUICK addr=%02x (ret %d)\n",
|
||||||
|
c->idx, addr, ret);
|
||||||
|
return ret;
|
||||||
|
}
|
||||||
|
|
||||||
if (size != I2C_SMBUS_BYTE && size != I2C_SMBUS_BYTE_DATA &&
|
if (size != I2C_SMBUS_BYTE && size != I2C_SMBUS_BYTE_DATA &&
|
||||||
size != I2C_SMBUS_WORD_DATA)
|
size != I2C_SMBUS_WORD_DATA)
|
||||||
return -EOPNOTSUPP;
|
return -EOPNOTSUPP;
|
||||||
@@ -526,6 +569,22 @@ static s32 imc_smbus_xfer(struct i2c_adapter *adap, u16 addr,
|
|||||||
*/
|
*/
|
||||||
reg = size == I2C_SMBUS_BYTE ? 0 : command;
|
reg = size == I2C_SMBUS_BYTE ? 0 : command;
|
||||||
|
|
||||||
|
/*
|
||||||
|
* Downstream-only: refuse writes to the SPD EEPROMs and to the DDR4
|
||||||
|
* protection command addresses. Send Byte writes to 0x50-0x57 are
|
||||||
|
* caught too; on an EE1004 they would only move the address pointer,
|
||||||
|
* but there is no caller that needs them. 0x36/0x37 (page select)
|
||||||
|
* stay writable.
|
||||||
|
*/
|
||||||
|
if (protect_spd && read_write == I2C_SMBUS_WRITE &&
|
||||||
|
((addr >= 0x50 && addr <= 0x57) ||
|
||||||
|
(addr >= 0x30 && addr <= 0x35))) {
|
||||||
|
dev_warn_ratelimited(s->dev,
|
||||||
|
"ch%d write to addr 0x%02x reg 0x%02x blocked by protect_spd\n",
|
||||||
|
c->idx, addr, reg);
|
||||||
|
return -EPERM;
|
||||||
|
}
|
||||||
|
|
||||||
mutex_lock(&s->lock);
|
mutex_lock(&s->lock);
|
||||||
s->io_err = 0;
|
s->io_err = 0;
|
||||||
|
|
||||||
@@ -588,8 +647,8 @@ static s32 imc_smbus_xfer(struct i2c_adapter *adap, u16 addr,
|
|||||||
|
|
||||||
static u32 imc_func(struct i2c_adapter *adap)
|
static u32 imc_func(struct i2c_adapter *adap)
|
||||||
{
|
{
|
||||||
return I2C_FUNC_SMBUS_BYTE | I2C_FUNC_SMBUS_BYTE_DATA |
|
return I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_BYTE |
|
||||||
I2C_FUNC_SMBUS_WORD_DATA;
|
I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA;
|
||||||
}
|
}
|
||||||
|
|
||||||
static const struct i2c_algorithm imc_algo = {
|
static const struct i2c_algorithm imc_algo = {
|
||||||
|
|||||||
Reference in New Issue
Block a user