[RESEND PATCH 0/2] rtc: cmos: Fix ACPI RTC alarm handling

[RESEND PATCH 0/2] rtc: cmos: Fix ACPI RTC alarm handling

[Gregory Price]

Resend of the series first posted on September 9.
https://lore.kernel.org/linux-rtc/20260909225132.1392883-1-gourry@gourry.net/

RTC update interrupt emulation can stop delivering events when rtc-cmos
uses the ACPI RTC fixed event. On an affected system, a test that waits
for five one-second update events completed only 40 of 50 runs. The ACPI
ff_rt_clk counter stopped at 218, exactly matching the number of events
delivered to userspace, with the event status set but delivery disabled.

The ACPI SCI handler disables the fixed event after dispatching the RTC
interrupt. Because dispatch queues work that rearms the next alarm, the
handler can disable an event that the worker has already enabled. Move
the clear and disable operations before dispatch so that rearming always
wins the race.

During diagnosis, an explicit rtc_cmos.use_acpi_alarm=0 argument was also
found to be ineffective. The automatic platform selection runs after
parameter parsing and overwrites the explicit false value. Record whether
the parameter was supplied and skip automatic selection in that case.

On the affected system, ACPI mode completed 150 of 150 runs after the
ordering fix and delivered all 750 requested events. With
rtc_cmos.use_acpi_alarm=0, sysfs reported N, HPET emulation was active,
and 100 of 100 runs passed without using the ACPI RTC event.

The in-tree RTC selftest also passed all 8 tests under VNG with
rtc_cmos.use_acpi_alarm=0. VNG does not deliver ACPI RTC fixed events, so
the ACPI ordering fix was tested on hardware.

Gregory Price (2):
  rtc: cmos: Disable ACPI RTC event before handling it
  rtc: cmos: Honor explicit use_acpi_alarm parameter value

 drivers/rtc/rtc-cmos.c | 32 ++++++++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 4 deletions(-)

-- 
2.56.0

[RESEND PATCH 1/2] rtc: cmos: Disable ACPI RTC event before handling it

[Gregory Price]

Users of RTC_UIE_ON can stop receiving update interrupts when rtc-cmos
handles alarms through ACPI. In one 50-run test, 40 runs passed and 10
timed out after receiving between one and four of five expected events.

The ACPI ff_rt_clk counter stopped at 218, exactly matching the number
of events delivered to userspace, and reported:

    218     STS disabled     unmasked

rtc_handler() runs in the threaded ACPI SCI handler and calls
cmos_interrupt(), which schedules rtc->irqwork. The work can run before
rtc_handler() returns:

    ACPI SCI thread          rtc->irqwork

    cmos_interrupt()
      schedule_work()
                             __rtc_set_alarm()
                               rtc_wake_on()
    acpi_clear_event()
    acpi_disable_event()

The SCI thread then disables the event that the worker enabled for the
next alarm. That alarm sets its status while delivery remains disabled,
so no update interrupt reaches userspace.

Clear and disable the current ACPI RTC event before cmos_interrupt()
schedules the work. The worker can then enable the next event without
the handler undoing it. With this ordering, 150 runs delivered all 750
events without a stall.

Fixes: 311ee9c151ad ("rtc: cmos: allow using ACPI for RTC alarm instead of HPET")
Cc: stable@vger.kernel.org
Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>
---
 drivers/rtc/rtc-cmos.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
index fa04ece151b8..fe514b1aa343 100644
--- a/drivers/rtc/rtc-cmos.c
+++ b/drivers/rtc/rtc-cmos.c
@@ -758,7 +758,9 @@ static u32 rtc_handler(void *context)
 	unsigned char rtc_intr;
 	unsigned long flags;
 
-
+	/* Quiesce this event before rtc->irqwork can arm the next one. */
+	acpi_clear_event(ACPI_EVENT_RTC);
+	acpi_disable_event(ACPI_EVENT_RTC, 0);
 	/*
 	 * Always update rtc irq when ACPI is used as RTC Alarm.
 	 * Or else, ACPI SCI is enabled during suspend/resume only,
@@ -781,8 +783,6 @@ static u32 rtc_handler(void *context)
 	}
 
 	pm_wakeup_hard_event(dev);
-	acpi_clear_event(ACPI_EVENT_RTC);
-	acpi_disable_event(ACPI_EVENT_RTC, 0);
 	return ACPI_INTERRUPT_HANDLED;
 }
 
-- 
2.56.0

[RESEND PATCH 2/2] rtc: cmos: Honor explicit use_acpi_alarm parameter value

[Gregory Price]

An explicit rtc_cmos.use_acpi_alarm=0 command-line argument is ignored
on systems where use_acpi_alarm_quirks() enables ACPI RTC alarms. The
argument appears in /proc/cmdline, but the read-only parameter reports Y.

The boolean parameter defaults to false, so the driver cannot distinguish
an omitted argument from an explicit false value. Use a custom setter to
record whether parsing occurred and skip automatic selection when it did.
Preserve the bool parameter type metadata. Automatic selection remains
unchanged when the parameter is omitted.

On the affected system, the explicit false value reports N after this
change. The HPET-emulated path completed 100 test runs without a failure
while the ACPI RTC event counter remained at zero.

Fixes: 36d91a4d401c ("rtc: cmos: introduce quirks to enable use_acpi_alarm mode")
Cc: stable@vger.kernel.org
Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>
---
 drivers/rtc/rtc-cmos.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
index fe514b1aa343..209aa4ec732c 100644
--- a/drivers/rtc/rtc-cmos.c
+++ b/drivers/rtc/rtc-cmos.c
@@ -57,7 +57,28 @@
  */
 
 static bool use_acpi_alarm;
-module_param(use_acpi_alarm, bool, 0444);
+static bool use_acpi_alarm_explicit;
+
+static int set_use_acpi_alarm(const char *val, const struct kernel_param *kp)
+{
+	int ret;
+
+	ret = param_set_bool(val, kp);
+	if (!ret)
+		use_acpi_alarm_explicit = true;
+
+	return ret;
+}
+
+static const struct kernel_param_ops use_acpi_alarm_ops = {
+	.flags = KERNEL_PARAM_OPS_FL_NOARG,
+	.set = set_use_acpi_alarm,
+	.get = param_get_bool,
+};
+
+param_check_bool(use_acpi_alarm, &use_acpi_alarm);
+module_param_cb(use_acpi_alarm, &use_acpi_alarm_ops, &use_acpi_alarm, 0444);
+__MODULE_PARM_TYPE(use_acpi_alarm, "bool");
 
 static inline int cmos_use_acpi_alarm(void)
 {
@@ -822,6 +843,9 @@ static void rtc_wake_off(struct device *dev)
 #ifdef CONFIG_X86
 static void use_acpi_alarm_quirks(void)
 {
+	if (use_acpi_alarm_explicit)
+		return;
+
 	if (acpi_gbl_FADT.flags & ACPI_FADT_FIXED_RTC)
 		return;
 
-- 
2.56.0

Re: [RESEND PATCH 2/2] rtc: cmos: Honor explicit use_acpi_alarm parameter value

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] rtc: cmos: unused-but-set variable use_acpi_alarm_explicit on non-x86
--

commit db7f65d5eb781cb5e973c2ace56bd5a4865e78bb
Author: Gregory Price <gourry@gourry.net>

rtc: cmos: Honor explicit use_acpi_alarm parameter value

An explicit rtc_cmos.use_acpi_alarm=0 command-line argument is ignored on
systems where use_acpi_alarm_quirks() enables ACPI RTC alarms. This patch uses
a custom setter to record whether parsing occurred and skips automatic
selection when it did.

> diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
> index fe514b1aa3433..209aa4ec732c8 100644
> --- a/drivers/rtc/rtc-cmos.c
> +++ b/drivers/rtc/rtc-cmos.c
> @@ -57,7 +57,28 @@
>   */
>  
>  static bool use_acpi_alarm;
> -module_param(use_acpi_alarm, bool, 0444);
> +static bool use_acpi_alarm_explicit;
> +
> +static int set_use_acpi_alarm(const char *val, const struct kernel_param *kp)
> +{
> +	int ret;
> +
> +	ret = param_set_bool(val, kp);
> +	if (!ret)
> +		use_acpi_alarm_explicit = true;

[Severity: Low]
Will this trigger a -Wunused-but-set-variable compiler warning on non-x86
architectures?

The file-scoped variable use_acpi_alarm_explicit is declared and unconditionally
assigned here, but it appears to only be read inside use_acpi_alarm_quirks(),
which is wrapped in an #ifdef CONFIG_X86 block.

On architectures like ARM64 or PowerPC, the variable is written to but never
read.

> +
> +	return ret;
> +}

[ ... ]

> @@ -822,6 +843,9 @@ static void rtc_wake_off(struct device *dev)
>  #ifdef CONFIG_X86
>  static void use_acpi_alarm_quirks(void)
>  {
> +	if (use_acpi_alarm_explicit)
> +		return;
> +
>  	if (acpi_gbl_FADT.flags & ACPI_FADT_FIXED_RTC)
>  		return;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015173029.3091872-1-gourry@gourry.net?part=2

Re: [RESEND PATCH 1/2] rtc: cmos: Disable ACPI RTC event before handling it

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] rtc: cmos: TOCTOU race in ACPI RTC event toggle leads to lost alarms
--

commit 73bcc5b0dfcf900941896af4381bb8051c9589e0
Author: Gregory Price <gourry@gourry.net>

rtc: cmos: Disable ACPI RTC event before handling it

Users of RTC_UIE_ON can stop receiving update interrupts when rtc-cmos
handles alarms through ACPI. Clear and disable the current ACPI RTC event
before cmos_interrupt() schedules the work to avoid a race where the handler
undoes the worker's event enablement.

> diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
> index fa04ece151b8b..fe514b1aa3433 100644
> --- a/drivers/rtc/rtc-cmos.c
> +++ b/drivers/rtc/rtc-cmos.c
> @@ -758,7 +758,9 @@ static u32 rtc_handler(void *context)
>  	unsigned char rtc_intr;
>  	unsigned long flags;
>  
> -
> +	/* Quiesce this event before rtc->irqwork can arm the next one. */
> +	acpi_clear_event(ACPI_EVENT_RTC);
> +	acpi_disable_event(ACPI_EVENT_RTC, 0);

[Severity: Medium]
Could this unconditional disablement create a race with concurrent userspace
threads setting a new alarm?

If a concurrent userspace thread calls ioctl(RTC_ALM_SET), it acquires the
necessary locks and enables the ACPI event for the newly configured alarm:

drivers/rtc/rtc-cmos.c:rtc_wake_on() {
        acpi_clear_event(ACPI_EVENT_RTC);
        acpi_enable_event(ACPI_EVENT_RTC, 0);
}

While the patch successfully resolves a similar race with the worker thread
by moving the disablement before cmos_interrupt(), it seems to leave the
asynchronous userspace race unhandled.

If the userspace thread enables the event after the SCI thread has been
triggered, but before the SCI thread executes acpi_disable_event() here in
rtc_handler(), will the SCI thread mistakenly disable the newly set alarm?

This could cause the newly configured alarm to be dropped, as the disablement
happens without holding locks such as rtc_lock or rtc->ops_lock.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015173029.3091872-1-gourry@gourry.net?part=1

Re: [RESEND PATCH 1/2] rtc: cmos: Disable ACPI RTC event before handling it

[Gregory Price]

On Thu, Oct 15, 2026 at 05:42:19PM +0000, sashiko-bot@kernel.org wrote:
> > diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
> > index fa04ece151b8b..fe514b1aa3433 100644
> > --- a/drivers/rtc/rtc-cmos.c
> > +++ b/drivers/rtc/rtc-cmos.c
> > @@ -758,7 +758,9 @@ static u32 rtc_handler(void *context)
> >  	unsigned char rtc_intr;
> >  	unsigned long flags;
> >  
> > -
> > +	/* Quiesce this event before rtc->irqwork can arm the next one. */
> > +	acpi_clear_event(ACPI_EVENT_RTC);
> > +	acpi_disable_event(ACPI_EVENT_RTC, 0);
> 
> [Severity: Medium]
> Could this unconditional disablement create a race with concurrent userspace
> threads setting a new alarm?
> 
> If a concurrent userspace thread calls ioctl(RTC_ALM_SET), it acquires the
> necessary locks and enables the ACPI event for the newly configured alarm:
> 
> drivers/rtc/rtc-cmos.c:rtc_wake_on() {
>         acpi_clear_event(ACPI_EVENT_RTC);
>         acpi_enable_event(ACPI_EVENT_RTC, 0);
> }
>
> While the patch successfully resolves a similar race with the worker thread
> by moving the disablement before cmos_interrupt(), it seems to leave the
> asynchronous userspace race unhandled.
> 

This is a pre-existing issue.

~Gregory

Re: [RESEND PATCH 2/2] rtc: cmos: Honor explicit use_acpi_alarm parameter value

[Gregory Price]

On Thu, Oct 15, 2026 at 05:36:16PM +0000, sashiko-bot@kernel.org wrote:
> > +static bool use_acpi_alarm_explicit;
> > +
> > +static int set_use_acpi_alarm(const char *val, const struct kernel_param *kp)
> > +{
> > +	int ret;
> > +
> > +	ret = param_set_bool(val, kp);
> > +	if (!ret)
> > +		use_acpi_alarm_explicit = true;
> 
> [Severity: Low]
> Will this trigger a -Wunused-but-set-variable compiler warning on non-x86
> architectures?
> 
> The file-scoped variable use_acpi_alarm_explicit is declared and unconditionally
> assigned here, but it appears to only be read inside use_acpi_alarm_quirks(),
> which is wrapped in an #ifdef CONFIG_X86 block.
> 
> On architectures like ARM64 or PowerPC, the variable is written to but never
> read.
> 

I could not get this to trigger. Neither gcc nor clang would fire this
warning for a file-scoped static.

~Gregory