Linux Watchdog driver development
 help / color / mirror / Atom feed
* [PATCH] watchdog: sp5100_tco: add reversible legacy MMIO relocation
@ 2026-08-20 22:25 Christoph Berliner
  2026-08-20 22:39 ` sashiko-bot
  2026-08-20 23:14 ` [PATCH v2] " Christoph Berliner
  0 siblings, 2 replies; 6+ messages in thread
From: Christoph Berliner @ 2026-08-20 22:25 UTC (permalink / raw)
  To: Wim Van Sebroeck, Guenter Roeck
  Cc: linux-watchdog, linux-kernel, Christoph Berliner

Older SP5100/SB7x0 systems may provide a watchdog MMIO base that
overlaps another reserved resource. On the tested system firmware
programs the watchdog at 0xfec000f0, which falls inside the IOAPIC
resource and causes sp5100_tco to fail probing with -EBUSY.

A relocation mechanism for this class of conflict was introduced by
commit 740fbddf5c3f ("watchdog: sp5100_tco: Add SB8x0 chipset support")
for Linux 3.8.

It was deliberately removed shortly afterwards by commit 18e4321276fc
("watchdog: sp5100_tco: Remove code that may cause a boot failure")
after an SB700 machine failed to load BIOS after running a kernel
containing the relocation path until power was completely removed.

The exact cause of that failure is not documented. One relevant
difference is that the old relocation path reprogrammed the watchdog
base but did not restore the firmware-programmed PM register state when
the driver was removed.

Add a reversible relocation path for the legacy SP5100 register layout.
Locate the top-level firmware-reserved MMIO resource containing the
conflicting address and search it for a free, naturally aligned 8-byte
child resource.

Use request_resource() for conflict checking instead of
allocate_resource(), since x86 dynamic resource allocation excludes
E820-reserved address ranges.

Before reprogramming the watchdog, save the original PM control and base
registers. Restore them on probe failure or device removal and release
the relocated resource.

On the tested system the complete lifecycle is:

  firmware:  0xfec000f0, PM control 0x03
  relocated: 0xfec00400, PM control 0x06
  restored:  0xfec000f0, PM control 0x03

The relocated resource is registered correctly in /proc/iomem,
the watchdog registers successfully as /dev/watchdog0, and unloading
the module restores both the original firmware address and control
register.

Tested on Linux 7.1.8 with an AMD SBx00 SMBus controller
(PCI 1002:4385, revision 0x3c). The sp5100_tco.c driver in current
mainline is identical to the tested 7.1.8 version.

Signed-off-by: Christoph Berliner <caberliner@gmail.com>
---
Hi Wim, Guenter,

A note on context: I am a physician, not a software developer. This
watchdog issue has annoyed me for quite some time on an older machine
that I still use. Current AI-assisted programming tools finally gave me
a practical way to investigate it in enough depth to trace the firmware
configuration, historical driver behavior and kernel resource handling,
and to build and test a possible fix on the actual hardware.

While investigating the history I found the 2013 SB700 regression that
led to removal of the old relocation path. I do not have the machine
affected by that regression and therefore cannot establish its exact
root cause. The reversible restoration of the firmware-programmed PM
state in this patch is intended to address one important difference
from the old implementation.

I consider this a hardware-tested proposal rather than the work of an
experienced kernel developer. I would particularly appreciate review
of whether restoring the original PM state sufficiently addresses the
risk that led to commit 18e4321276fc ("watchdog: sp5100_tco: Remove
code that may cause a boot failure").

 drivers/watchdog/sp5100_tco.c | 255 +++++++++++++++++++++++++++++++++-
 1 file changed, 251 insertions(+), 4 deletions(-)

diff --git a/drivers/watchdog/sp5100_tco.c b/drivers/watchdog/sp5100_tco.c
index 7e99c3b1f367..d21339d07942 100644
--- a/drivers/watchdog/sp5100_tco.c
+++ b/drivers/watchdog/sp5100_tco.c
@@ -229,6 +229,231 @@ static void tco_timer_enable(struct sp5100_tco *tco)
 	}
 }
 
+static u32 sp5100_tco_prepare_base(struct sp5100_tco *tco,
+				   u32 mmio_addr,
+				   u32 alt_mmio_addr,
+				   const char *dev_name,
+				   bool report_error);
+
+/*
+ * Legacy SP5100/SB7x0 fallback.
+ *
+ * Some firmware uses 0xfec000f0 for the watchdog MMIO window.
+ * This may overlap the IOAPIC resource. Historical Linux versions
+ * relocated the watchdog to a free 8-byte MMIO window and
+ * reprogrammed PM registers 0x6c..0x6f.
+ */
+static void sp5100_tco_write_pm_reg8(u8 index, u8 val)
+{
+	outb(index, SP5100_IO_PM_INDEX_REG);
+	outb(val, SP5100_IO_PM_DATA_REG);
+}
+
+struct sp5100_tco_relocation {
+	struct resource res;
+	u8 saved_control;
+	u8 saved_base[4];
+	bool programmed;
+};
+
+static void sp5100_tco_release_relocation(void *data)
+{
+	struct sp5100_tco_relocation *reloc = data;
+	int i;
+
+	if (reloc->programmed &&
+	    request_muxed_region(SP5100_IO_PM_INDEX_REG,
+				 SP5100_PM_IOPORTS_SIZE,
+				 "sp5100_tco restore")) {
+		/*
+		 * Disable decoding while restoring the original base.
+		 */
+		sp5100_tco_update_pm_reg8(SP5100_PM_WATCHDOG_CONTROL,
+					  0xff,
+					  SP5100_PM_WATCHDOG_DISABLE);
+
+		for (i = 0; i < 4; i++)
+			sp5100_tco_write_pm_reg8(SP5100_PM_WATCHDOG_BASE + i,
+						 reloc->saved_base[i]);
+
+		sp5100_tco_write_pm_reg8(SP5100_PM_WATCHDOG_CONTROL,
+					 reloc->saved_control);
+
+		release_region(SP5100_IO_PM_INDEX_REG,
+			       SP5100_PM_IOPORTS_SIZE);
+	}
+
+	if (reloc->res.parent)
+		release_resource(&reloc->res);
+}
+
+/*
+ * Find the top-level firmware resource containing the conflicting
+ * watchdog address.  The watchdog can then be allocated as a sibling
+ * of resources such as the IOAPIC rather than as a child of them.
+ */
+static struct resource *
+sp5100_tco_find_parent_resource(resource_size_t addr)
+{
+	struct resource *res;
+
+	for (res = iomem_resource.child; res; res = res->sibling) {
+		if (addr >= res->start && addr <= res->end)
+			return res;
+	}
+
+	return NULL;
+}
+
+/*
+ * Older SP5100/SB7x0 firmware may place the watchdog MMIO window inside
+ * another reserved resource, historically most notably the IOAPIC area.
+ *
+ * The old Linux driver could relocate the watchdog when its firmware
+ * supplied address was unusable.  Modern resource trees may expose one
+ * large firmware-reserved parent with individual resources below it.
+ *
+ * Allocate an unused, naturally aligned watchdog-sized child resource
+ * inside that parent and reprogram the SP5100 watchdog base registers.
+ */
+static int sp5100_tco_reprogram_base(struct sp5100_tco *tco,
+				     u32 conflict_addr,
+				     const char *dev_name)
+{
+	struct device *dev = tco->wdd.parent;
+	struct resource *parent;
+	struct sp5100_tco_relocation *reloc;
+	struct resource *res;
+	resource_size_t candidate;
+	resource_size_t max_addr;
+	u32 mmio_addr;
+	u8 base0_reserved;
+	int ret;
+	int i;
+
+	parent = sp5100_tco_find_parent_resource(conflict_addr);
+	if (!parent) {
+		dev_err(dev,
+			"No parent MMIO resource contains conflicting watchdog address 0x%08x\n",
+			conflict_addr);
+		return -ENODEV;
+	}
+
+	/*
+	 * The SP5100 watchdog base is a 32-bit physical address.
+	 */
+	if (parent->start > U32_MAX)
+		return -ERANGE;
+
+	max_addr = min_t(resource_size_t, parent->end, U32_MAX);
+
+	reloc = devm_kzalloc(dev, sizeof(*reloc), GFP_KERNEL);
+	if (!reloc)
+		return -ENOMEM;
+
+	res = &reloc->res;
+	res->name = dev_name;
+	res->flags = IORESOURCE_MEM;
+
+	/*
+	 * Do not use allocate_resource() here.
+	 *
+	 * On x86, dynamic resource allocation excludes E820-reserved
+	 * address ranges. Legacy SP5100 firmware may deliberately place
+	 * the watchdog inside such a reserved parent resource.
+	 *
+	 * Try naturally aligned watchdog-sized slots inside the existing
+	 * parent. request_resource() performs the actual conflict check
+	 * while holding the resource lock.
+	 */
+	candidate = ALIGN(parent->start, SP5100_WDT_MEM_MAP_SIZE);
+
+	for (;;) {
+		if (candidate > max_addr ||
+		    max_addr - candidate + 1 < SP5100_WDT_MEM_MAP_SIZE) {
+			dev_err(dev,
+				"No free watchdog MMIO slot found inside %pR\n",
+				parent);
+			return -EBUSY;
+		}
+
+		res->start = candidate;
+		res->end = candidate + SP5100_WDT_MEM_MAP_SIZE - 1;
+
+		ret = request_resource(parent, res);
+		if (!ret)
+			break;
+
+		if (ret != -EBUSY)
+			return ret;
+
+		candidate += SP5100_WDT_MEM_MAP_SIZE;
+	}
+
+	/*
+	 * Save the firmware-provided watchdog state before changing it.
+	 * The devm cleanup action restores it on probe failure or device
+	 * removal and releases the relocated resource.
+	 */
+	reloc->saved_control =
+		sp5100_tco_read_pm_reg8(SP5100_PM_WATCHDOG_CONTROL);
+
+	for (i = 0; i < 4; i++)
+		reloc->saved_base[i] =
+			sp5100_tco_read_pm_reg8(SP5100_PM_WATCHDOG_BASE + i);
+
+	ret = devm_add_action_or_reset(dev,
+				       sp5100_tco_release_relocation,
+				       reloc);
+	if (ret)
+		return ret;
+
+	mmio_addr = (u32)res->start;
+
+	/*
+	 * Disable the watchdog before changing its decode address.
+	 */
+	sp5100_tco_update_pm_reg8(SP5100_PM_WATCHDOG_CONTROL,
+				  0xff,
+				  SP5100_PM_WATCHDOG_DISABLE);
+
+	/*
+	 * Preserve the reserved low three bits of BASE0 while
+	 * reprogramming the aligned watchdog address.
+	 */
+	base0_reserved = reloc->saved_base[0] & 0x07;
+
+	for (i = 0; i < 4; i++) {
+		u8 val = (mmio_addr >> (8 * i)) & 0xff;
+
+		if (!i)
+			val |= base0_reserved;
+
+		sp5100_tco_write_pm_reg8(SP5100_PM_WATCHDOG_BASE + i, val);
+	}
+
+	reloc->programmed = true;
+
+	dev_info(dev,
+		 "Relocated legacy SP5100 watchdog MMIO from 0x%08x to 0x%08x\n",
+		 conflict_addr, mmio_addr);
+
+	tco->tcobase = devm_ioremap(dev, mmio_addr,
+				    SP5100_WDT_MEM_MAP_SIZE);
+	if (!tco->tcobase) {
+		dev_err(dev,
+			"Relocated watchdog MMIO address 0x%08x failed mapping\n",
+			mmio_addr);
+		return -ENOMEM;
+	}
+
+	dev_info(dev,
+		 "Using relocated 0x%08x for watchdog MMIO address\n",
+		 mmio_addr);
+
+	return 0;
+}
+
 static u32 sp5100_tco_read_pm_reg32(u8 index)
 {
 	u32 val = 0;
@@ -256,7 +481,8 @@ static u32 sp5100_tco_request_region(struct device *dev,
 static u32 sp5100_tco_prepare_base(struct sp5100_tco *tco,
 				   u32 mmio_addr,
 				   u32 alt_mmio_addr,
-				   const char *dev_name)
+				   const char *dev_name,
+				   bool report_error)
 {
 	struct device *dev = tco->wdd.parent;
 
@@ -273,7 +499,9 @@ static u32 sp5100_tco_prepare_base(struct sp5100_tco *tco,
 		mmio_addr = sp5100_tco_request_region(dev, alt_mmio_addr, dev_name);
 
 	if (!mmio_addr) {
-		dev_err(dev, "Failed to reserve MMIO or alternate MMIO region\n");
+		if (report_error)
+			dev_err(dev,
+				"Failed to reserve MMIO or alternate MMIO region\n");
 		return -EBUSY;
 	}
 
@@ -405,7 +633,7 @@ static int sp5100_tco_setupdevice_mmio(struct device *dev,
 		alt_mmio_addr = EFCH_PM_ACPI_MMIO_ADDR +
 			EFCH_PM_ACPI_MMIO_WDT_OFFSET;
 
-	ret = sp5100_tco_prepare_base(tco, mmio_addr, alt_mmio_addr, dev_name);
+	ret = sp5100_tco_prepare_base(tco, mmio_addr, alt_mmio_addr, dev_name, true);
 	if (!ret) {
 		tco_timer_enable_mmio(addr);
 		ret = sp5100_tco_timer_init(tco);
@@ -428,6 +656,8 @@ static int sp5100_tco_setupdevice(struct device *dev,
 	const char *dev_name;
 	u32 mmio_addr = 0, val;
 	u32 alt_mmio_addr = 0;
+	u32 conflict_addr;
+	bool report_error;
 	int ret;
 
 	if (tco->tco_reg_layout == efch_mmio)
@@ -491,7 +721,24 @@ static int sp5100_tco_setupdevice(struct device *dev,
 		return -ENODEV;
 	}
 
-	ret = sp5100_tco_prepare_base(tco, mmio_addr, alt_mmio_addr, dev_name);
+	report_error = tco->tco_reg_layout != sp5100;
+	ret = sp5100_tco_prepare_base(tco, mmio_addr, alt_mmio_addr, dev_name, report_error);
+
+	/*
+	 * Legacy SP5100/SB7x0 firmware commonly uses 0xfec000f0,
+	 * which can overlap the IOAPIC resource. If neither firmware
+	 * MMIO location can be reserved, restore the historical
+	 * relocation fallback.
+	 */
+	if (ret == -EBUSY && tco->tco_reg_layout == sp5100) {
+		conflict_addr = mmio_addr ? mmio_addr : alt_mmio_addr;
+		ret = sp5100_tco_reprogram_base(tco, conflict_addr, dev_name);
+		if (ret)
+			dev_err(dev,
+				"Failed to reserve or relocate watchdog MMIO region: %d\n",
+				ret);
+	}
+
 	if (!ret) {
 		/* Setup the watchdog timer */
 		tco_timer_enable(tco);
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-08-21 14:25 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-20 22:25 [PATCH] watchdog: sp5100_tco: add reversible legacy MMIO relocation Christoph Berliner
2026-08-20 22:39 ` sashiko-bot
2026-08-20 23:14 ` [PATCH v2] " Christoph Berliner
2026-08-20 23:26   ` sashiko-bot
2026-08-21  0:59   ` Guenter Roeck
2026-08-21 14:25   ` [PATCH v3] watchdog: sp5100_tco: allow unreserved MMIO on GA-78LMT-USB3 Christoph Berliner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox