Merge branch 'ecam-accessors' into guard

ECAM MMIO accessors (use_ecam, default on): remove the CF8/CFC SMI
window, suspected poison channel of the ws1 poweroff hang.
This commit is contained in:
Simone
2026-08-09 09:48:04 +02:00
2 changed files with 97 additions and 4 deletions
+1 -1
View File
@@ -1 +1 @@
1.0.4-guard1
1.0.5-guard1
+96 -3
View File
@@ -44,6 +44,22 @@
* /sys/module/i2c_imc_skylake/parameters/protect_spd for deliberate
* SPD maintenance.
*
* - use_ecam (default on): reach the engine registers through a private
* ECAM (MMCONFIG) mapping instead of the kernel's legacy config
* cycles. For standard config space the kernel uses conf1: two I/O
* instructions, index on 0xCF8 and data on 0xCFC. pci_config_lock
* serialises that pair against the rest of the kernel but not against
* SMM: an SMI landing between the two, whose handler runs its own
* CF8/CFC dialogue without restoring the index, silently redirects
* the data phase to whatever register the firmware addressed last.
* A misdirected read only perturbs one poll iteration and the CMD
* write is covered by the landing check below, but the DATA-register
* write has no readback, so its payload would land unnoticed in a
* foreign config register. A single MMIO access cannot be split by
* an SMI, so the window does not exist on the ECAM path. Suspected
* (not proven) trigger of the ws1 poweroff hang. Falls back to
* config cycles when MCFG has no entry for the device.
*
* Bus arbitration / concurrency with firmware:
* 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
@@ -63,6 +79,7 @@
* to be arbitrated against.
*/
#include <linux/acpi.h>
#include <linux/bits.h>
#include <linux/delay.h>
#include <linux/i2c.h>
@@ -183,6 +200,16 @@ module_param(protect_spd, bool, 0644);
MODULE_PARM_DESC(protect_spd,
"reject SMBus writes to SPD addresses 0x30-0x35 and 0x50-0x57 (default on)");
/*
* Load-time only: the accessors pick their path from s->ecam, captured at
* probe. Flipping this at runtime would race in-flight transfers for no
* benefit; the A/B lever is a reboot away, like the pre-ECAM baseline.
*/
static bool use_ecam = true;
module_param(use_ecam, bool, 0444);
MODULE_PARM_DESC(use_ecam,
"reach the engine via an ECAM mapping instead of CF8/CFC config cycles (default on)");
struct imc_chan {
unsigned int cmd, data, stat;
int idx; /* channel index 0 or 1 */
@@ -196,6 +223,8 @@ static const struct imc_chan imc_chans[2] = {
/* one driver state object, shared by both per-channel adapters */
struct imc_smbus {
struct pci_dev *pdev; /* config space carrying the engine */
void __iomem *ecam; /* this function's config page, or NULL
to fall back to config cycles */
struct device *dev; /* &pdev->dev, for dev_*() logging */
struct mutex lock; /* serialises all SMBus transactions */
/* needed: both channels share one engine */
@@ -211,9 +240,52 @@ struct imc_xfer_state {
};
/*
* Register accessors. A config-space failure is recorded in s->io_err and
* reported once, at the end of the transfer, so the transaction logic does not
* have to check a return value on every register touch. A failed read yields
* Locate this function's configuration page in the ECAM window described by
* the ACPI MCFG table and map it. The arch code keeps its own MCFG mapping
* for extended config space but exposes no way to route standard config
* space through it, hence the private lookup. Mapping the page a second
* time is harmless: it is device MMIO, not RAM.
*/
static void __iomem *imc_map_ecam(struct pci_dev *pdev)
{
struct acpi_table_mcfg *mcfg;
struct acpi_mcfg_allocation *alloc;
void __iomem *map = NULL;
unsigned int bus = pdev->bus->number;
int i, n;
if (ACPI_FAILURE(acpi_get_table(ACPI_SIG_MCFG, 0,
(struct acpi_table_header **)&mcfg)))
return NULL;
n = (mcfg->header.length - sizeof(*mcfg)) / sizeof(*alloc);
alloc = (struct acpi_mcfg_allocation *)(mcfg + 1);
for (i = 0; i < n; i++, alloc++) {
resource_size_t addr;
if (alloc->pci_segment != pci_domain_nr(pdev->bus) ||
bus < alloc->start_bus_number ||
bus > alloc->end_bus_number)
continue;
addr = alloc->address +
((resource_size_t)(bus - alloc->start_bus_number) << 20) +
((resource_size_t)pdev->devfn << 12);
map = devm_ioremap(&pdev->dev, addr, SZ_4K);
break;
}
acpi_put_table(&mcfg->header);
return map;
}
/*
* Register accessors. On the ECAM path a register touch is a single MMIO
* access; it has no failure channel, and the function sits on the CPU die,
* so it cannot vanish and produce the all-ones of a master abort. On the
* config-cycle fallback a failure is recorded in s->io_err and reported
* once, at the end of the transfer, so the transaction logic does not have
* to check a return value on every register touch. A failed read yields
* all-ones, which no polling loop mistakes for a completed transaction.
*/
static u32 imc_reg(struct imc_smbus *s, unsigned int off)
@@ -221,6 +293,9 @@ static u32 imc_reg(struct imc_smbus *s, unsigned int off)
u32 val;
int ret;
if (s->ecam)
return readl(s->ecam + off);
ret = pci_read_config_dword(s->pdev, off, &val);
if (ret) {
s->io_err = pcibios_err_to_errno(ret);
@@ -234,6 +309,11 @@ static void imc_set(struct imc_smbus *s, unsigned int off, u32 val)
{
int ret;
if (s->ecam) {
writel(val, s->ecam + off);
return;
}
ret = pci_write_config_dword(s->pdev, off, val);
if (ret)
s->io_err = pcibios_err_to_errno(ret);
@@ -837,6 +917,19 @@ static int imc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
if (ret)
return ret;
/*
* Before the first register touch, so that even the pristine-word
* snapshot below goes through the SMI-safe path.
*/
if (use_ecam) {
s->ecam = imc_map_ecam(pdev);
if (!s->ecam)
dev_warn(&pdev->dev,
"no usable MCFG entry; falling back to CF8/CFC config cycles\n");
}
dev_dbg(&pdev->dev, "engine access via %s\n",
s->ecam ? "ECAM MMIO" : "CF8/CFC config cycles");
/*
* Pristine command words: the state the firmware left in the engine at
* boot, captured before this driver's first transfer. They are handed