Skip to contentSkip to conversation

MIPS: SGI-IP30: power off through the RTC

PATCH Superseded Imre Kaloz 2 participants 4 messages Oct 13

Subsystems (2)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Imre Kaloz

author
#0 lore raw

cover MIPS: SGI-IP30: power off through the RTC

The Octane's power supply is switched by its DS1687 RTC. Patch 1 uses it for power-off and halt. Patch 2 turns a power button press into an orderly power-off, and adds the hook to the IOC3 RTC platform data in drivers/mfd/ioc3.c.

Both take effect only with CONFIG_RTC_DRV_DS1685_FAMILY=y, which ip30_defconfig sets with "MIPS: SGI-IP30: fix the defconfig defaults for the Bridge PROM and RTC" from the "MIPS: SGI fixes" series. They build without it, and without the rtc-ds1685 power-off fixes sent to the RTC tree.

Tested on an Octane with poweroff, one and two power button presses, and a stalled shutdown.

Imre Kaloz (2):
MIPS: SGI-IP30: power off through the DS1687 RTC
MIPS: SGI-IP30: shut down cleanly on a power button press

 arch/mips/include/asm/sgi/heart.h |  2 ++
 arch/mips/sgi-ip30/ip30-power.c   | 56 +++++++++++++++++++++++++++++++
 drivers/mfd/ioc3.c                |  6 ++++
 3 files changed, 64 insertions(+)

base-commit: 24a19b953e2a3d0ea5a8c4951a0ab8f32720c907

Signature · 1 line
2.47.3
author
#1 lore raw

PATCH 1/2 MIPS: SGI-IP30: power off through the DS1687 RTC

The Octane's power supply is switched by its DS1687 RTC. Use ds1685_rtc_poweroff() for power-off and halt, with all HEART interrupts masked first, instead of leaving the machine halted.

---
 arch/mips/sgi-ip30/ip30-power.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)
arch/mips/sgi-ip30/ip30-power.c +30 -0
index 66851e17c5a7..2b862b27e114 100644
@@ -15,6 +15,7 @@
 #include <linux/delay.h>
 #include <linux/rtc/ds1685.h>
 #include <linux/interrupt.h>
+#include <linux/platform_device.h>
 #include <linux/pm.h>
 
 #include <asm/reboot.h>
@@ -31,10 +32,39 @@ static void __noreturn ip30_machine_restart(char *cmd)
 	unreachable();
 }
 
+static void ip30_machine_power_off(void)
+{
+	struct device *dev;
+	int cpu;
+
+	if (!IS_REACHABLE(CONFIG_RTC_DRV_DS1685_FAMILY))
+		return;
+
+	dev = bus_find_device_by_name(&platform_bus_type, NULL, "rtc-ds1685");
+	if (!dev || !dev_get_drvdata(dev))
+		return;
+
+	/* Devices losing power may still interrupt; keep their handlers out. */
+	for (cpu = 0; cpu < HEART_MAX_CPUS; cpu++)
+		heart_write(HEART_CLR_ALL_MASK, &heart_regs->imr[cpu]);
+	heart_write(HEART_ACK_ALL_MASK, &heart_regs->clear_isr);
+
+	ds1685_rtc_poweroff(to_platform_device(dev));
+}
+
 static int __init ip30_reboot_setup(void)
 {
 	_machine_restart = ip30_machine_restart;
 
+	/*
+	 * The DS1687 RTC switches the power supply off.  Halt powers off
+	 * too, as the power button needs a running kernel.
+	 */
+	if (IS_REACHABLE(CONFIG_RTC_DRV_DS1685_FAMILY)) {
+		_machine_halt = ip30_machine_power_off;
+		pm_power_off = ip30_machine_power_off;
+	}
+
 	return 0;
 }
 
Signature · 1 line
2.47.3
author
#2 lore raw

PATCH 2/2 MIPS: SGI-IP30: shut down cleanly on a power button press

In reply to Imre Kaloz

The power button raises a kickstart interrupt on the DS1687, and rtc-ds1685 cuts power at once unless the platform supplies a prepare_poweroff hook. Supply one that starts an orderly power-off, cutting power directly on a second press or after 120 seconds.

---
 arch/mips/include/asm/sgi/heart.h |  2 ++
 arch/mips/sgi-ip30/ip30-power.c   | 26 ++++++++++++++++++++++++++
 drivers/mfd/ioc3.c                |  6 ++++++
 3 files changed, 34 insertions(+)
arch/mips/include/asm/sgi/heart.h +2 -0
index c224c2e3575a..736cabb7fdbb 100644
@@ -317,6 +317,8 @@ struct ip30_heart_regs {		/* 0x0ff00000 */
 
 extern struct ip30_heart_regs __iomem *heart_regs;
 
+void ip30_prepare_poweroff(void);
+
 #define heart_read	____raw_readq
 #define heart_write	____raw_writeq
 
arch/mips/sgi-ip30/ip30-power.c +26 -0
index 2b862b27e114..1200fef108b1 100644
@@ -17,6 +17,8 @@
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
 #include <linux/pm.h>
+#include <linux/reboot.h>
+#include <linux/workqueue.h>
 
 #include <asm/reboot.h>
 #include <asm/sgi/heart.h>
@@ -52,6 +54,30 @@ static void ip30_machine_power_off(void)
 	ds1685_rtc_poweroff(to_platform_device(dev));
 }
 
+#define IP30_POWERDOWN_TIMEOUT	120	/* seconds */
+
+static bool ip30_shutting_down;
+
+static void ip30_power_timeout(struct work_struct *work)
+{
+	ip30_machine_power_off();
+}
+
+static DECLARE_DELAYED_WORK(ip30_power_work, ip30_power_timeout);
+
+/* Called by the RTC driver when the power button is pressed. */
+void ip30_prepare_poweroff(void)
+{
+	/* A second press powers off at once. */
+	if (ip30_shutting_down)
+		ip30_machine_power_off();
+
+	/* Shut down through userspace, powering off anyway if it stalls. */
+	ip30_shutting_down = true;
+	orderly_poweroff(true);
+	schedule_delayed_work(&ip30_power_work, IP30_POWERDOWN_TIMEOUT * HZ);
+}
+
 static int __init ip30_reboot_setup(void)
 {
 	_machine_restart = ip30_machine_restart;
drivers/mfd/ioc3.c +6 -0
index 5f8ac364b610..8f603651cc8e 100644
@@ -24,6 +24,9 @@
 
 #include <asm/pci/bridge.h>
 #include <asm/sn/ioc3.h>
+#ifdef CONFIG_SGI_IP30
+#include <asm/sgi/heart.h>
+#endif
 
 #define IOC3_IRQ_SERIAL_A	6
 #define IOC3_IRQ_SERIAL_B	15
@@ -321,6 +324,9 @@ static struct ds1685_rtc_platform_data ip30_rtc_platform_data = {
 	.no_irq = false,
 	.uie_unsupported = true,
 	.access_type = ds1685_reg_indirect,
+#ifdef CONFIG_SGI_IP30
+	.plat_prepare_poweroff = ip30_prepare_poweroff,
+#endif
 };
 
 static const struct resource ioc3_rtc_ds1685_resources[] = {
Signature · 1 line
2.47.3
#3 lore raw

Re: [PATCH 2/2] MIPS: SGI-IP30: shut down cleanly on a power button press

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] mips: ip30: concurrent ip30_machine_power_off() scrambles RTC I/O - [High] mips: TOCTOU race with…

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mips: ip30: concurrent ip30_machine_power_off() scrambles RTC I/O
- [High] mips: TOCTOU race with driver unbind leading to UAF in ip30_machine_power_off --

commit 01338bb2869999b0a3fdb7a5d240c7d0c419fb37
Author: Imre Kaloz <[email protected]>

MIPS: SGI-IP30: shut down cleanly on a power button press

The power button raises a kickstart interrupt on the DS1687, and rtc-ds1685 cuts power at once unless the platform supplies a prepare_poweroff hook. Supply one that starts an orderly power-off, cutting power directly on a second press or after 120 seconds.

Quoted 14 lines
arch/mips/sgi-ip30/ip30-power.c +7 -0
@@ -52,6 +54,30 @@ static void ip30_machine_power_off(void)
 	ds1685_rtc_poweroff(to_platform_device(dev));
 }
 
+#define IP30_POWERDOWN_TIMEOUT	120	/* seconds */
+
+static bool ip30_shutting_down;
+
+static void ip30_power_timeout(struct work_struct *work)
+{
+	ip30_machine_power_off();

arch/mips/sgi-ip30/ip30-power.c static void ip30_machine_power_off(void) · line 73

Quoted 10 lines
arch/mips/sgi-ip30/ip30-power.c +10 -0
+}
+
+static DECLARE_DELAYED_WORK(ip30_power_work, ip30_power_timeout);
+
+/* Called by the RTC driver when the power button is pressed. */
+void ip30_prepare_poweroff(void)
+{
+	/* A second press powers off at once. */
+	if (ip30_shutting_down)
+		ip30_machine_power_off();

[Severity: High]
Can concurrent execution of ip30_machine_power_off() happen here?

If ip30_prepare_poweroff() is called from a threaded IRQ handler on a second button press concurrently with ip30_power_work firing or pm_power_off() running on another CPU, could ip30_machine_power_off() execute on multiple CPUs at once?

Since ip30_machine_power_off() and ds1685_rtc_poweroff() don't appear to have atomic guards or spinlocks to prevent concurrent execution, would this race scramble the address and data writes to the RTC's indirect I/O registers and cause the system to hang indefinitely instead of powering off?

arch/mips/sgi-ip30/ip30-power.c static void ip30_machine_power_off(void) · line 78

Quoted 5 lines
arch/mips/sgi-ip30/ip30-power.c +5 -0
+
+	/* Shut down through userspace, powering off anyway if it stalls. */
+	ip30_shutting_down = true;
+	orderly_poweroff(true);
+	schedule_delayed_work(&ip30_power_work, IP30_POWERDOWN_TIMEOUT * HZ);

[Severity: High]
Does this 120-second delayed work introduce a use-after-free window?

If the rtc-ds1685 driver is unbound via sysfs or module unload during this delay, ip30_power_timeout() will race against device_unbind_cleanup().

Because ip30_machine_power_off() checks dev_get_drvdata(dev) without holding device_lock(dev):

arch/mips/sgi-ip30/ip30-power.c:ip30_machine_power_off() {
	...
	dev = bus_find_device_by_name(&platform_bus_type, NULL, "rtc-ds1685");
	if (!dev || !dev_get_drvdata(dev))
		return;
	...
	ds1685_rtc_poweroff(to_platform_device(dev));
}

Could a concurrent unbind trigger devres_release_all() and free the RTC structure before ds1685_rtc_poweroff() runs, causing it to unconditionally dereference a freed pointer or NULL pointer?

Quoted 1 line
arch/mips/sgi-ip30/ip30-power.c +1 -0
+}
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2