[PATCH] watchdog: sp5100_tco: add reversible legacy MMIO relocation

[PATCH] watchdog: sp5100_tco: add reversible legacy MMIO relocation

[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

[PATCH v2] watchdog: sp5100_tco: add reversible legacy MMIO relocation

[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.

When the firmware-provided watchdog address cannot be reserved, search a
bounded 4 KiB range above it for a free, naturally aligned 8-byte MMIO
window. Use devm_request_mem_region() for each candidate so resource-tree
traversal, conflict checking and IORESOURCE_BUSY registration are handled
by the resource core under its lock.

Before reprogramming the watchdog, save the original PM control and base
registers as well as the PCI watchdog decode register. Disable PCI MMIO
decode and the watchdog timer before changing the byte-wise base
registers, avoiding transient decoded addresses while BASE0..3 are being
updated.

Restore the original firmware PM and PCI state on probe failure or device
removal and release the relocated MMIO resource through devres.

On the tested system the complete load/unload lifecycle is:

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

The relocated range appears as an exclusive SP5100 TCO resource in
/proc/iomem, /dev/watchdog0 registers successfully, and unloading the
module restores the original firmware state and removes the resource.

A warm reboot with the relocated watchdog loaded was also tested
successfully; the machine booted normally and the firmware watchdog state
after reboot was again 0xfec000f0 with PM control 0x03.

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>
---
Changes in v2:
- replace direct iomem resource-tree traversal with
  devm_request_mem_region(), providing locked conflict handling and
  IORESOURCE_BUSY semantics
- bound the relocation search to a 4 KiB window
- disable PCI watchdog MMIO decode while rewriting BASE0..3
- save and restore the original PCI watchdog decode register
- retain reversible restoration on probe failure and device removal
- hardware-test the updated load/unload lifecycle and a warm reboot

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

diff --git a/drivers/watchdog/sp5100_tco.c b/drivers/watchdog/sp5100_tco.c
index 7e99c3b1f367..dbe6946d8f95 100644
--- a/drivers/watchdog/sp5100_tco.c
+++ b/drivers/watchdog/sp5100_tco.c
@@ -229,6 +229,215 @@ 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);
+
+#define SP5100_WDT_RELOCATION_RANGE	0x1000
+
+/*
+ * 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 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 {
+	u32 saved_pci_misc;
+	u8 saved_control;
+	u8 saved_base[4];
+	bool programmed;
+	bool restored;
+};
+
+static void sp5100_tco_restore_relocation(struct sp5100_tco_relocation *reloc)
+{
+	u32 val;
+	int i;
+
+	if (!reloc || !reloc->programmed || reloc->restored)
+		return;
+
+	if (!request_muxed_region(SP5100_IO_PM_INDEX_REG,
+				  SP5100_PM_IOPORTS_SIZE,
+				  "sp5100_tco restore"))
+		return;
+
+	/*
+	 * MMIO decode is controlled independently of the PM watchdog-disable
+	 * bit. Disable decode before changing the byte-wise base registers so
+	 * that no intermediate address can become active.
+	 */
+	pci_read_config_dword(sp5100_tco_pci,
+			      SP5100_PCI_WATCHDOG_MISC_REG, &val);
+	pci_write_config_dword(sp5100_tco_pci,
+			       SP5100_PCI_WATCHDOG_MISC_REG,
+			       val & ~SP5100_PCI_WATCHDOG_DECODE_EN);
+
+	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);
+
+	/*
+	 * Restore the firmware-programmed PCI state only after the original
+	 * watchdog base and PM control register have been restored.
+	 */
+	pci_write_config_dword(sp5100_tco_pci,
+			       SP5100_PCI_WATCHDOG_MISC_REG,
+			       reloc->saved_pci_misc);
+
+	reloc->restored = true;
+
+	release_region(SP5100_IO_PM_INDEX_REG,
+		       SP5100_PM_IOPORTS_SIZE);
+}
+
+static void sp5100_tco_release_relocation(void *data)
+{
+	struct sp5100_tco_relocation *reloc = data;
+
+	sp5100_tco_restore_relocation(reloc);
+}
+
+/*
+ * Older SP5100/SB7x0 firmware may place the watchdog MMIO window inside
+ * another reserved resource, historically most notably the IOAPIC area.
+ *
+ * E820 reserved device address ranges are represented as non-busy
+ * containers in the iomem tree. devm_request_mem_region() performs the
+ * resource-tree traversal and conflict checking under resource_lock and
+ * creates an IORESOURCE_BUSY child for a successful reservation.
+ *
+ * Keep the search deliberately local to the firmware-provided address
+ * instead of walking an arbitrarily large reserved address range.
+ */
+static int sp5100_tco_reprogram_base(struct sp5100_tco *tco,
+				     u32 conflict_addr,
+				     const char *dev_name)
+{
+	struct device *dev = tco->wdd.parent;
+	struct sp5100_tco_relocation *reloc;
+	struct resource *res = NULL;
+	resource_size_t candidate;
+	resource_size_t search_end;
+	u32 mmio_addr;
+	u8 base0_reserved;
+	int ret;
+	int i;
+
+	reloc = devm_kzalloc(dev, sizeof(*reloc), GFP_KERNEL);
+	if (!reloc)
+		return -ENOMEM;
+
+	search_end = min_t(resource_size_t, U32_MAX,
+			   (resource_size_t)conflict_addr +
+			   SP5100_WDT_RELOCATION_RANGE);
+
+	candidate = ALIGN((resource_size_t)conflict_addr +
+			  SP5100_WDT_MEM_MAP_SIZE,
+			  SP5100_WDT_MEM_MAP_SIZE);
+
+	for (; candidate <= search_end &&
+	     search_end - candidate + 1 >= SP5100_WDT_MEM_MAP_SIZE;
+	     candidate += SP5100_WDT_MEM_MAP_SIZE) {
+		res = devm_request_mem_region(dev, candidate,
+					      SP5100_WDT_MEM_MAP_SIZE,
+					     dev_name);
+		if (res)
+			break;
+	}
+
+	if (!res) {
+		dev_err(dev,
+			"No free watchdog MMIO slot found within 0x%x bytes of 0x%08x\n",
+			SP5100_WDT_RELOCATION_RANGE, conflict_addr);
+		return -EBUSY;
+	}
+
+	mmio_addr = (u32)res->start;
+
+	/*
+	 * Save all firmware-programmed watchdog and decode state before
+	 * changing anything.
+	 */
+	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);
+
+	pci_read_config_dword(sp5100_tco_pci,
+			      SP5100_PCI_WATCHDOG_MISC_REG,
+			      &reloc->saved_pci_misc);
+
+	ret = devm_add_action_or_reset(dev,
+				       sp5100_tco_release_relocation,
+				       reloc);
+	if (ret)
+		return ret;
+
+	/*
+	 * Disable MMIO decode before byte-wise modification of BASE0..3.
+	 */
+	pci_write_config_dword(sp5100_tco_pci,
+			       SP5100_PCI_WATCHDOG_MISC_REG,
+			       reloc->saved_pci_misc &
+			       ~SP5100_PCI_WATCHDOG_DECODE_EN);
+
+	/* Stop the watchdog timer as well before changing its base. */
+	sp5100_tco_update_pm_reg8(SP5100_PM_WATCHDOG_CONTROL,
+				  0xff,
+				  SP5100_PM_WATCHDOG_DISABLE);
+
+	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 +465,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 +483,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 +617,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 +640,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 +705,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

Re: [PATCH v2] watchdog: sp5100_tco: add reversible legacy MMIO relocation

[Guenter Roeck]

On 8/20/26 16:14, Christoph Berliner wrote:
> 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.
> 
> When the firmware-provided watchdog address cannot be reserved, search a
> bounded 4 KiB range above it for a free, naturally aligned 8-byte MMIO
> window. Use devm_request_mem_region() for each candidate so resource-tree
> traversal, conflict checking and IORESOURCE_BUSY registration are handled
> by the resource core under its lock.
> 
> Before reprogramming the watchdog, save the original PM control and base
> registers as well as the PCI watchdog decode register. Disable PCI MMIO
> decode and the watchdog timer before changing the byte-wise base
> registers, avoiding transient decoded addresses while BASE0..3 are being
> updated.
> 
> Restore the original firmware PM and PCI state on probe failure or device
> removal and release the relocated MMIO resource through devres.
> 
> On the tested system the complete load/unload lifecycle is:
> 
>    firmware:  0xfec000f0, PM control 0x03
>    relocated: 0xfec00400, PM control 0x06
>    restored:  0xfec000f0, PM control 0x03
> 
> The relocated range appears as an exclusive SP5100 TCO resource in
> /proc/iomem, /dev/watchdog0 registers successfully, and unloading the
> module restores the original firmware state and removes the resource.
> 
> A warm reboot with the relocated watchdog loaded was also tested
> successfully; the machine booted normally and the firmware watchdog state
> after reboot was again 0xfec000f0 with PM control 0x03.
> 
> 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>

Your solution is way too risky. Try to find one that lets the driver use the
region without reserving it, specifically suited for the system affected
by the problem. You should be able to identify that system using DMI.

Thanks,
Guenter

[PATCH v3] watchdog: sp5100_tco: allow unreserved MMIO on GA-78LMT-USB3

[Christoph Berliner]

The Gigabyte GA-78LMT-USB3 firmware programs the legacy SP5100
watchdog MMIO window at 0xfec000f0. This address falls inside the
IOAPIC resource, so sp5100_tco fails to reserve it and aborts probing.

Do not relocate or reprogram the watchdog. Instead, add a narrowly
scoped DMI quirk for this board which permits use of the
firmware-provided MMIO address without reserving it.

The exception is limited to the legacy SP5100 register layout, the
GA-78LMT-USB3 DMI identity, and the firmware address 0xfec000f0.
All other systems retain the existing resource reservation behavior.

On the affected system the watchdog initializes successfully and
/dev/watchdog0 is registered while the firmware-programmed watchdog
base remains unchanged at 0xfec000f0 during load and unload.

Tested on a Gigabyte GA-78LMT-USB3 with AMD SBx00 SMBus controller
(PCI 1002:4385, revision 0x3c).

Signed-off-by: Christoph Berliner <caberliner@gmail.com>
---
Thanks Guenter. I reworked the patch along the lines you suggested.

v3 is a complete replacement of the v2 relocation approach. None of the
relocation code from v2 is retained.

Instead, v3 adds a narrowly scoped DMI quirk for the affected
GA-78LMT-USB3. On that system, and only for the legacy SP5100 register
layout with the firmware-programmed watchdog address 0xfec000f0, the
driver is allowed to map and use the existing watchdog MMIO window
without reserving it.

The watchdog base is not relocated or reprogrammed. All other systems
retain the existing resource reservation behavior.

I tested the new path on the affected machine and again from a fresh
clone of the published GitHub repository. The watchdog initializes
successfully and /dev/watchdog0 is registered, while the firmware MMIO
base remains unchanged at 0xfec000f0 before, during and after driver use.
No separate SP5100 TCO resource is added to /proc/iomem.

Changes in v3:
- replace the v2 relocation approach entirely; none of the relocation
  code from v2 is retained
- add a DMI quirk for the affected Gigabyte GA-78LMT-USB3
- allow the firmware-programmed 0xfec000f0 watchdog window to be used
  without reserving it
- restrict the exception to the legacy SP5100 register layout and the
  known firmware address
- leave all other systems on the existing resource reservation path
- hardware-test the new path and verify that the watchdog MMIO base
  remains unchanged

 drivers/watchdog/sp5100_tco.c | 61 ++++++++++++++++++++++++++++++++---
 1 file changed, 56 insertions(+), 5 deletions(-)

diff --git a/drivers/watchdog/sp5100_tco.c b/drivers/watchdog/sp5100_tco.c
index 7e99c3b1f367..6c85f11cff24 100644
--- a/drivers/watchdog/sp5100_tco.c
+++ b/drivers/watchdog/sp5100_tco.c
@@ -33,6 +33,7 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/init.h>
+#include <linux/dmi.h>
 #include <linux/io.h>
 #include <linux/ioport.h>
 #include <linux/module.h>
@@ -240,6 +241,35 @@ static u32 sp5100_tco_read_pm_reg32(u8 index)
 	return val;
 }
 
+/*
+ * The Gigabyte GA-78LMT-USB3 firmware programs the legacy SP5100 watchdog
+ * MMIO window at 0xfec000f0. This address lies inside the IOAPIC resource,
+ * so the generic resource reservation fails even though firmware explicitly
+ * assigns the watchdog to this address.
+ *
+ * Keep this exception narrowly scoped to the affected system and firmware
+ * address. Do not relocate or otherwise reprogram the watchdog.
+ */
+#define SP5100_WDT_GA78LMT_MMIO	0xfec000f0
+
+static const struct dmi_system_id sp5100_tco_unreserved_mmio_dmi[] = {
+	{
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Gigabyte Technology Co., Ltd."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "GA-78LMT-USB3"),
+		},
+	},
+	{}
+};
+
+static bool sp5100_tco_allow_unreserved_mmio(struct sp5100_tco *tco,
+					     u32 mmio_addr)
+{
+	return tco->tco_reg_layout == sp5100 &&
+	       mmio_addr == SP5100_WDT_GA78LMT_MMIO &&
+	       dmi_check_system(sp5100_tco_unreserved_mmio_dmi);
+}
+
 static u32 sp5100_tco_request_region(struct device *dev,
 				     u32 mmio_addr,
 				     const char *dev_name)
@@ -259,6 +289,7 @@ static u32 sp5100_tco_prepare_base(struct sp5100_tco *tco,
 				   const char *dev_name)
 {
 	struct device *dev = tco->wdd.parent;
+	bool reserved = false;
 
 	dev_dbg(dev, "Got 0x%08x from SBResource_MMIO register\n", mmio_addr);
 
@@ -266,11 +297,29 @@ static u32 sp5100_tco_prepare_base(struct sp5100_tco *tco,
 		return -ENODEV;
 
 	/* Check for MMIO address and alternate MMIO address conflicts */
-	if (mmio_addr)
-		mmio_addr = sp5100_tco_request_region(dev, mmio_addr, dev_name);
+	if (mmio_addr) {
+		u32 requested_addr;
+
+		requested_addr = sp5100_tco_request_region(dev, mmio_addr,
+							   dev_name);
+		if (requested_addr) {
+			mmio_addr = requested_addr;
+			reserved = true;
+		} else if (sp5100_tco_allow_unreserved_mmio(tco, mmio_addr)) {
+			dev_info(dev,
+				 "Using firmware watchdog MMIO 0x%08x without reserving it\n",
+				 mmio_addr);
+		} else {
+			mmio_addr = 0;
+		}
+	}
 
-	if (!mmio_addr && alt_mmio_addr)
-		mmio_addr = sp5100_tco_request_region(dev, alt_mmio_addr, dev_name);
+	if (!mmio_addr && alt_mmio_addr) {
+		mmio_addr = sp5100_tco_request_region(dev, alt_mmio_addr,
+						      dev_name);
+		if (mmio_addr)
+			reserved = true;
+	}
 
 	if (!mmio_addr) {
 		dev_err(dev, "Failed to reserve MMIO or alternate MMIO region\n");
@@ -280,7 +329,9 @@ static u32 sp5100_tco_prepare_base(struct sp5100_tco *tco,
 	tco->tcobase = devm_ioremap(dev, mmio_addr, SP5100_WDT_MEM_MAP_SIZE);
 	if (!tco->tcobase) {
 		dev_err(dev, "MMIO address 0x%08x failed mapping\n", mmio_addr);
-		devm_release_mem_region(dev, mmio_addr, SP5100_WDT_MEM_MAP_SIZE);
+		if (reserved)
+			devm_release_mem_region(dev, mmio_addr,
+						SP5100_WDT_MEM_MAP_SIZE);
 		return -ENOMEM;
 	}
 
-- 
2.55.0

Re: [PATCH v3] watchdog: sp5100_tco: allow unreserved MMIO on GA-78LMT-USB3

[Guenter Roeck]

On Fri, Aug 21, 2026 at 04:25:11PM +0200, Christoph Berliner wrote:
> The Gigabyte GA-78LMT-USB3 firmware programs the legacy SP5100
> watchdog MMIO window at 0xfec000f0. This address falls inside the
> IOAPIC resource, so sp5100_tco fails to reserve it and aborts probing.
> 
> Do not relocate or reprogram the watchdog. Instead, add a narrowly
> scoped DMI quirk for this board which permits use of the
> firmware-provided MMIO address without reserving it.
> 
> The exception is limited to the legacy SP5100 register layout, the
> GA-78LMT-USB3 DMI identity, and the firmware address 0xfec000f0.
> All other systems retain the existing resource reservation behavior.
> 
> On the affected system the watchdog initializes successfully and
> /dev/watchdog0 is registered while the firmware-programmed watchdog
> base remains unchanged at 0xfec000f0 during load and unload.
> 
> Tested on a Gigabyte GA-78LMT-USB3 with AMD SBx00 SMBus controller
> (PCI 1002:4385, revision 0x3c).
> 
> Signed-off-by: Christoph Berliner <caberliner@gmail.com>

Applied.

Thanks,
Guenter