diff --git a/VERSION b/VERSION index a0b1e44..421092d 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.0.4-guard1 +1.0.5-guard1 diff --git a/i2c-imc-skylake.c b/i2c-imc-skylake.c index 2598c69..9f533ec 100644 --- a/i2c-imc-skylake.c +++ b/i2c-imc-skylake.c @@ -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 #include #include #include @@ -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