* [PATCH] rtc: efi: restore alarm support with runtime capability probe
@ 2026-08-28 3:15 Johnny.Fan
2026-08-28 3:32 ` sashiko-bot
` (4 more replies)
0 siblings, 5 replies; 13+ messages in thread
From: Johnny.Fan @ 2026-08-28 3:15 UTC (permalink / raw)
To: Alexandre Belloni, linux-rtc
Cc: Ard Biesheuvel, Feng Tang, linux-efi, linux-kernel,
cix-kernel-upstream, bob.zhang, jie.fu
From: "Johnny.Fan" <Johnny.Fan@cixtech.com>
Restore the EFI RTC alarm functionality (read_alarm/set_alarm and
procfs output) that was removed by commit 18a3510bc87d ("rtc: efi:
Remove wakeup functionality").
The removal was motivated by the observation that many EFI platforms
lack RTC wakeup support while also omitting the RT_PROP table, causing
the wakeup capability to be incorrectly advertised. While that is
true, removing the feature entirely also breaks platforms where the
wakeup runtime service actually works.
On CIX SoCs with an I2C-connected RTC, the firmware owns the RTC and
exposes it via EFI runtime services. Using the native I2C RTC driver
under Linux breaks Windows, which relies on EFI runtime services to
access the RTC. rtc-efi is therefore the only viable option for
shared RTC access across operating systems, and it needs working
alarm/wakeup support.
To avoid exposing a broken alarm on firmware that does not implement
the wakeup runtime services, probe the capability at ->probe() time by
actually calling GetWakeupTime() -- if it fails, clear RTC_FEATURE_ALARM
and the device behaves as before the removal.
Also set RTC_FEATURE_ALARM_WAKEUP_ONLY since EFI wakeup time is a
wakeup-only alarm, not a periodic/update interrupt source.
Reviewed-by: Fugang Duan <fugang.duan@cixtech.com>
Tested-by: Johnny.Fan <Johnny.Fan@cixtech.com>
Signed-off-by: Johnny.Fan <Johnny.Fan@cixtech.com>
---
drivers/rtc/rtc-efi.c | 78 +++++++++++++++++++++++++++++++++++++++++--
1 file changed, 76 insertions(+), 2 deletions(-)
diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
index b4f44999ef0f..708cba43435e 100644
--- a/drivers/rtc/rtc-efi.c
+++ b/drivers/rtc/rtc-efi.c
@@ -112,6 +112,34 @@ convert_from_efi_time(efi_time_t *eft, struct rtc_time *wtime)
return true;
}
+static int efi_read_alarm(struct device *dev, struct rtc_wkalrm *wkalrm)
+{
+ efi_time_t eft;
+ efi_status_t status;
+
+ status = efi.get_wakeup_time((efi_bool_t *)&wkalrm->enabled,
+ (efi_bool_t *)&wkalrm->pending, &eft);
+ if (status != EFI_SUCCESS)
+ return -EINVAL;
+
+ if (!convert_from_efi_time(&eft, &wkalrm->time))
+ return -EIO;
+
+ return rtc_valid_tm(&wkalrm->time);
+}
+
+static int efi_set_alarm(struct device *dev, struct rtc_wkalrm *wkalrm)
+{
+ efi_time_t eft;
+ efi_status_t status;
+
+ convert_to_efi_time(&wkalrm->time, &eft);
+
+ status = efi.set_wakeup_time((efi_bool_t)wkalrm->enabled, &eft);
+
+ return status == EFI_SUCCESS ? 0 : -EINVAL;
+}
+
static int efi_read_time(struct device *dev, struct rtc_time *tm)
{
efi_status_t status;
@@ -146,13 +174,21 @@ static int efi_set_time(struct device *dev, struct rtc_time *tm)
static int efi_procfs(struct device *dev, struct seq_file *seq)
{
- efi_time_t eft;
+ efi_time_t eft, alm;
efi_time_cap_t cap;
+ efi_bool_t enabled, pending;
+ struct rtc_device *rtc = dev_get_drvdata(dev);
memset(&eft, 0, sizeof(eft));
+ memset(&alm, 0, sizeof(alm));
memset(&cap, 0, sizeof(cap));
efi.get_time(&eft, &cap);
+ if (test_bit(RTC_FEATURE_ALARM, rtc->features) &&
+ efi.get_wakeup_time(&enabled, &pending, &alm) != EFI_SUCCESS) {
+ enabled = false;
+ pending = false;
+ }
seq_printf(seq,
"Time\t\t: %u:%u:%u.%09u\n"
@@ -168,6 +204,25 @@ static int efi_procfs(struct device *dev, struct seq_file *seq)
/* XXX fixme: convert to string? */
seq_printf(seq, "Timezone\t: %u\n", eft.timezone);
+ if (test_bit(RTC_FEATURE_ALARM, rtc->features)) {
+ seq_printf(seq,
+ "Alarm Time\t: %u:%u:%u.%09u\n"
+ "Alarm Date\t: %u-%u-%u\n"
+ "Alarm Daylight\t: %u\n"
+ "Enabled\t\t: %s\n"
+ "Pending\t\t: %s\n",
+ alm.hour, alm.minute, alm.second, alm.nanosecond,
+ alm.year, alm.month, alm.day,
+ alm.daylight,
+ enabled == 1 ? "yes" : "no",
+ pending == 1 ? "yes" : "no");
+
+ if (alm.timezone == EFI_UNSPECIFIED_TIMEZONE)
+ seq_puts(seq, "Alarm Timezone\t: unspecified\n");
+ else
+ seq_printf(seq, "Alarm Timezone\t: %u\n", alm.timezone);
+ }
+
/*
* now prints the capabilities
*/
@@ -183,6 +238,8 @@ static int efi_procfs(struct device *dev, struct seq_file *seq)
static const struct rtc_class_ops efi_rtc_ops = {
.read_time = efi_read_time,
.set_time = efi_set_time,
+ .read_alarm = efi_read_alarm,
+ .set_alarm = efi_set_alarm,
.proc = efi_procfs,
};
@@ -191,6 +248,7 @@ static int __init efi_rtc_probe(struct platform_device *dev)
struct rtc_device *rtc;
efi_time_t eft;
efi_time_cap_t cap;
+ efi_bool_t enabled, pending;
/* First check if the RTC is usable */
if (efi.get_time(&eft, &cap) != EFI_SUCCESS)
@@ -203,7 +261,23 @@ static int __init efi_rtc_probe(struct platform_device *dev)
platform_set_drvdata(dev, rtc);
rtc->ops = &efi_rtc_ops;
- clear_bit(RTC_FEATURE_ALARM, rtc->features);
+ clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, rtc->features);
+
+ /*
+ * The EFI_RT_SUPPORTED_WAKEUP_SERVICES bit defaults to enabled
+ * and only gets cleared when the RT_PROP table explicitly says
+ * wakeup is unsupported. Many platforms lack an RT_PROP table
+ * even though they don't implement the wakeup runtime service,
+ * so probe by actually calling GetWakeupTime() to avoid exposing
+ * a broken alarm to userspace.
+ */
+ if (efi_rt_services_supported(EFI_RT_SUPPORTED_WAKEUP_SERVICES) &&
+ efi.get_wakeup_time(&enabled, &pending, &eft) == EFI_SUCCESS) {
+ set_bit(RTC_FEATURE_ALARM, rtc->features);
+ set_bit(RTC_FEATURE_ALARM_WAKEUP_ONLY, rtc->features);
+ } else {
+ clear_bit(RTC_FEATURE_ALARM, rtc->features);
+ }
device_init_wakeup(&dev->dev, true);
--
2.55.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] rtc: efi: restore alarm support with runtime capability probe
2026-08-28 3:15 [PATCH] rtc: efi: restore alarm support with runtime capability probe Johnny.Fan
@ 2026-08-28 3:32 ` sashiko-bot
2026-08-28 7:26 ` Ard Biesheuvel
` (3 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-08-28 3:32 UTC (permalink / raw)
To: Johnny.Fan; +Cc: Alexandre Belloni, linux-rtc
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
New issues:
- [High] Uninitialized kernel stack memory (struct padding) is leaked to EFI firmware in efi_set_alarm and efi_set_time.
- [High] efi_read_alarm processes uninitialized stack memory and returns spurious errors when the alarm is disabled.
- [High] wkalrm->enabled is passed to firmware without being normalized to a strict boolean, risking unpredictable firmware behavior.
- [Low] Incorrect format specifier for signed timezone value in procfs output.
Pre-existing issues:
- [High] Lack of year bounds checking allows an extremely large tm_year to be truncated, programming the RTC with an unintended valid year.
--
commit f48bd3a58905ce7e489d515621c84d688e125eec
Author: Johnny.Fan <Johnny.Fan@cixtech.com>
rtc: efi: restore alarm support with runtime capability probe
This commit restores EFI RTC alarm support by probing for the capability
during initialization via GetWakeupTime(). It avoids breaking systems
without a working wakeup capability by clearing RTC_FEATURE_ALARM if the
probe fails, ensuring the alarm functions correctly for platforms that
rely on EFI for shared RTC access.
> diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
> index b4f44999ef0f4..708cba43435e0 100644
> --- a/drivers/rtc/rtc-efi.c
> +++ b/drivers/rtc/rtc-efi.c
> @@ -112,6 +112,34 @@ convert_from_efi_time(efi_time_t *eft, struct rtc_time *wtime)
> return true;
> }
>
> +static int efi_read_alarm(struct device *dev, struct rtc_wkalrm *wkalrm)
> +{
> + efi_time_t eft;
> + efi_status_t status;
> +
> + status = efi.get_wakeup_time((efi_bool_t *)&wkalrm->enabled,
> + (efi_bool_t *)&wkalrm->pending, &eft);
> + if (status != EFI_SUCCESS)
> + return -EINVAL;
> +
> + if (!convert_from_efi_time(&eft, &wkalrm->time))
> + return -EIO;
[Severity: High]
Does this code handle disabled alarms correctly?
According to the UEFI specification, if the alarm is not enabled, the
contents of the Time parameter returned by GetWakeupTime are undefined. Since
eft is allocated on the stack uninitialized, it may contain random stack
garbage if the firmware leaves the buffer unmodified.
Calling convert_from_efi_time() with this stack garbage will fail its strict
bounds checks (like eft->second >= 60) and invariably return -EIO instead of
returning the disabled alarm state.
> +
> + return rtc_valid_tm(&wkalrm->time);
> +}
> +
> +static int efi_set_alarm(struct device *dev, struct rtc_wkalrm *wkalrm)
> +{
> + efi_time_t eft;
> + efi_status_t status;
> +
> + convert_to_efi_time(&wkalrm->time, &eft);
> +
> + status = efi.set_wakeup_time((efi_bool_t)wkalrm->enabled, &eft);
[Severity: High]
Does this code leak uninitialized stack memory to the EFI firmware?
convert_to_efi_time() initializes specific fields but fails to zero out the
explicit pad1 and pad2 fields in efi_time_t. The UEFI specification mandates
that Pad1 and Pad2 must be zero, and passing stack garbage in these fields can
cause the firmware to reject valid parameters with EFI_INVALID_PARAMETER.
[Severity: High]
Does this code safely pass the enabled flag to the firmware?
The wkalrm->enabled field is directly populated from userspace as an
unsigned char (e.g., 255) and cast to efi_bool_t without normalization. The
UEFI specification explicitly defines BOOLEAN as containing exactly 0 for FALSE
or 1 for TRUE.
Passing un-normalized values can cause unpredictable behavior in buggy UEFI
firmware implementations.
> +
> + return status == EFI_SUCCESS ? 0 : -EINVAL;
> +}
[ ... ]
> @@ -168,6 +204,25 @@ static int efi_procfs(struct device *dev, struct seq_file *seq)
> /* XXX fixme: convert to string? */
> seq_printf(seq, "Timezone\t: %u\n", eft.timezone);
>
> + if (test_bit(RTC_FEATURE_ALARM, rtc->features)) {
> + seq_printf(seq,
> + "Alarm Time\t: %u:%u:%u.%09u\n"
> + "Alarm Date\t: %u-%u-%u\n"
> + "Alarm Daylight\t: %u\n"
> + "Enabled\t\t: %s\n"
> + "Pending\t\t: %s\n",
> + alm.hour, alm.minute, alm.second, alm.nanosecond,
> + alm.year, alm.month, alm.day,
> + alm.daylight,
> + enabled == 1 ? "yes" : "no",
> + pending == 1 ? "yes" : "no");
> +
> + if (alm.timezone == EFI_UNSPECIFIED_TIMEZONE)
> + seq_puts(seq, "Alarm Timezone\t: unspecified\n");
> + else
> + seq_printf(seq, "Alarm Timezone\t: %u\n", alm.timezone);
[Severity: Low]
Does this use the correct format specifier for alm.timezone?
alm.timezone is an s16 variable, so using the %u format specifier will cause
a negative timezone value (e.g., -300) to be formatted as a large positive
unsigned integer in the procfs output.
> + }
> +
[ ... ]
> @@ -203,7 +261,23 @@ static int __init efi_rtc_probe(struct platform_device *dev)
> platform_set_drvdata(dev, rtc);
>
> rtc->ops = &efi_rtc_ops;
> - clear_bit(RTC_FEATURE_ALARM, rtc->features);
> + clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, rtc->features);
[Severity: High]
This isn't a bug introduced by this patch, but does this probe function need to
initialize rtc->range_max?
Without setting rtc->range_max = 9999 - 1900, the RTC core's internal bounds
checking is disabled. If userspace provides a maliciously large year, the
calculation wtime->tm_year + 1900 in convert_to_efi_time() overflows the 16-bit
unsigned integer eft->year, silently truncating it into a valid range and
programming the hardware RTC with an unintended year.
This issue affects efi_set_time and propagates to the new efi_set_alarm
functionality.
> +
> + /*
> + * The EFI_RT_SUPPORTED_WAKEUP_SERVICES bit defaults to enabled
> + * and only gets cleared when the RT_PROP table explicitly says
> + * wakeup is unsupported. Many platforms lack an RT_PROP table
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260828031519.2749354-1-johnny.fan@cixtech.com?part=1
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] rtc: efi: restore alarm support with runtime capability probe
2026-08-28 3:15 [PATCH] rtc: efi: restore alarm support with runtime capability probe Johnny.Fan
2026-08-28 3:32 ` sashiko-bot
@ 2026-08-28 7:26 ` Ard Biesheuvel
2026-08-28 8:23 ` Johnny Fan
2026-08-28 8:32 ` Johnny Fan
2026-08-31 1:10 ` Feng Tang
` (2 subsequent siblings)
4 siblings, 2 replies; 13+ messages in thread
From: Ard Biesheuvel @ 2026-08-28 7:26 UTC (permalink / raw)
To: Johnny.Fan, Alexandre Belloni, linux-rtc
Cc: Feng Tang, linux-efi, linux-kernel, cix-kernel-upstream,
bob.zhang, jie.fu
Hello Johnny,
On Fri, 28 Aug 2026, at 05:15, Johnny.Fan wrote:
> From: "Johnny.Fan" <Johnny.Fan@cixtech.com>
>
> Restore the EFI RTC alarm functionality (read_alarm/set_alarm and
> procfs output) that was removed by commit 18a3510bc87d ("rtc: efi:
> Remove wakeup functionality").
>
> The removal was motivated by the observation that many EFI platforms
> lack RTC wakeup support while also omitting the RT_PROP table, causing
> the wakeup capability to be incorrectly advertised. While that is
> true, removing the feature entirely also breaks platforms where the
> wakeup runtime service actually works.
>
> On CIX SoCs with an I2C-connected RTC, the firmware owns the RTC and
> exposes it via EFI runtime services. Using the native I2C RTC driver
> under Linux breaks Windows, which relies on EFI runtime services to
> access the RTC. rtc-efi is therefore the only viable option for
> shared RTC access across operating systems, and it needs working
> alarm/wakeup support.
>
Why does it need working alarm/wakeup support?
I'm not saying we shouldn't bring it back, but this deserves a bit of
motivation, no?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] rtc: efi: restore alarm support with runtime capability probe
2026-08-28 7:26 ` Ard Biesheuvel
@ 2026-08-28 8:23 ` Johnny Fan
2026-08-28 8:32 ` Johnny Fan
1 sibling, 0 replies; 13+ messages in thread
From: Johnny Fan @ 2026-08-28 8:23 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-rtc, Feng Tang, linux-efi, linux-kernel,
cix-kernel-upstream, Bob Zhang, Jie Fu
On Fri, Aug 28, 2026 at 09:26:42AM +0200, Ard Biesheuvel wrote:
> Why does it need working alarm/wakeup support?
>
> I'm not saying we shouldn't bring it back, but this deserves a bit of
> motivation, no?
Hi Ard,
Thanks for the review.
The reason is wakeup: on CIX platforms, the RTC is an I2C device
owned by firmware, and in the ACPI boot configuration the bus is
not exposed to the OS, so the EFI runtime services are the only
way Linux can program the RTC alarm at all. With it removed,
'rtcwake' fails for both suspend-to-RAM and power-off wakeup --
Linux simply has no other path to set an alarm on this platform.
It is also a cross-OS consistency thing: Windows programs the
same alarm through the same runtime services, so a shared
interface that is time-only on the Linux side is asymmetric.
I'll extend the commit message with this motivation in v2.
Thanks,
Johnny
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] rtc: efi: restore alarm support with runtime capability probe
2026-08-28 7:26 ` Ard Biesheuvel
2026-08-28 8:23 ` Johnny Fan
@ 2026-08-28 8:32 ` Johnny Fan
2026-08-28 10:13 ` Ard Biesheuvel
1 sibling, 1 reply; 13+ messages in thread
From: Johnny Fan @ 2026-08-28 8:32 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Alexandre Belloni, linux-rtc, Feng Tang, linux-efi, linux-kernel,
cix-kernel-upstream, bob.zhang, jie.fu
On Fri, Aug 28, 2026 at 09:26:42AM +0200, Ard Biesheuvel wrote:
Hi Ard,
One additional data point: this is not a hypothetical requirement
-- multiple CIX customers have explicitly requested RTC wakeup
support (both suspend-to-RAM and power-off wakeup) on these
platforms, which is part of why we are motivated to restore the
alarm functionality rather than work around it.
Thanks,
Johnny
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] rtc: efi: restore alarm support with runtime capability probe
2026-08-28 8:32 ` Johnny Fan
@ 2026-08-28 10:13 ` Ard Biesheuvel
2026-08-28 12:48 ` Alexandre Belloni
0 siblings, 1 reply; 13+ messages in thread
From: Ard Biesheuvel @ 2026-08-28 10:13 UTC (permalink / raw)
To: Johnny.Fan
Cc: Alexandre Belloni, linux-rtc, Feng Tang, linux-efi, linux-kernel,
cix-kernel-upstream, bob.zhang, jie.fu
On Fri, 28 Aug 2026, at 10:32, Johnny Fan wrote:
> On Fri, Aug 28, 2026 at 09:26:42AM +0200, Ard Biesheuvel wrote:
> Hi Ard,
>
> One additional data point: this is not a hypothetical requirement
> -- multiple CIX customers have explicitly requested RTC wakeup
> support (both suspend-to-RAM and power-off wakeup) on these
> platforms, which is part of why we are motivated to restore the
> alarm functionality rather than work around it.
>
Fair enough - I'll queue this as a fix.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] rtc: efi: restore alarm support with runtime capability probe
2026-08-28 10:13 ` Ard Biesheuvel
@ 2026-08-28 12:48 ` Alexandre Belloni
0 siblings, 0 replies; 13+ messages in thread
From: Alexandre Belloni @ 2026-08-28 12:48 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Johnny.Fan, linux-rtc, Feng Tang, linux-efi, linux-kernel,
cix-kernel-upstream, bob.zhang, jie.fu
Hello Ard,
On 28/08/2026 12:13:10+0200, Ard Biesheuvel wrote:
>
> On Fri, 28 Aug 2026, at 10:32, Johnny Fan wrote:
> > On Fri, Aug 28, 2026 at 09:26:42AM +0200, Ard Biesheuvel wrote:
> > Hi Ard,
> >
> > One additional data point: this is not a hypothetical requirement
> > -- multiple CIX customers have explicitly requested RTC wakeup
> > support (both suspend-to-RAM and power-off wakeup) on these
> > platforms, which is part of why we are motivated to restore the
> > alarm functionality rather than work around it.
> >
>
> Fair enough - I'll queue this as a fix.
I can take care of this if you are happy with the patch.
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] rtc: efi: restore alarm support with runtime capability probe
2026-08-28 3:15 [PATCH] rtc: efi: restore alarm support with runtime capability probe Johnny.Fan
2026-08-28 3:32 ` sashiko-bot
2026-08-28 7:26 ` Ard Biesheuvel
@ 2026-08-31 1:10 ` Feng Tang
2026-08-31 11:38 ` [PATCH v2] " Johnny.Fan
2026-08-31 14:00 ` [PATCH v3] " Johnny.Fan
4 siblings, 0 replies; 13+ messages in thread
From: Feng Tang @ 2026-08-31 1:10 UTC (permalink / raw)
To: Johnny.Fan
Cc: Alexandre Belloni, linux-rtc, Ard Biesheuvel, linux-efi,
linux-kernel, cix-kernel-upstream, bob.zhang, jie.fu
On Fri, Aug 28, 2026 at 11:15:19AM +0800, Johnny.Fan wrote:
> From: "Johnny.Fan" <Johnny.Fan@cixtech.com>
>
> Restore the EFI RTC alarm functionality (read_alarm/set_alarm and
> procfs output) that was removed by commit 18a3510bc87d ("rtc: efi:
> Remove wakeup functionality").
>
> The removal was motivated by the observation that many EFI platforms
> lack RTC wakeup support while also omitting the RT_PROP table, causing
> the wakeup capability to be incorrectly advertised. While that is
> true, removing the feature entirely also breaks platforms where the
> wakeup runtime service actually works.
>
> On CIX SoCs with an I2C-connected RTC, the firmware owns the RTC and
> exposes it via EFI runtime services. Using the native I2C RTC driver
> under Linux breaks Windows, which relies on EFI runtime services to
> access the RTC. rtc-efi is therefore the only viable option for
> shared RTC access across operating systems, and it needs working
> alarm/wakeup support.
>
> To avoid exposing a broken alarm on firmware that does not implement
> the wakeup runtime services, probe the capability at ->probe() time by
> actually calling GetWakeupTime() -- if it fails, clear RTC_FEATURE_ALARM
> and the device behaves as before the removal.
>
> Also set RTC_FEATURE_ALARM_WAKEUP_ONLY since EFI wakeup time is a
> wakeup-only alarm, not a periodic/update interrupt source.
It covers the alarm selftest failure we met before, thanks!
Reviewed-by: Feng Tang <feng.tang@linux.alibaba.com>
>
> Reviewed-by: Fugang Duan <fugang.duan@cixtech.com>
> Tested-by: Johnny.Fan <Johnny.Fan@cixtech.com>
> Signed-off-by: Johnny.Fan <Johnny.Fan@cixtech.com>
> ---
> drivers/rtc/rtc-efi.c | 78 +++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 76 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
> index b4f44999ef0f..708cba43435e 100644
> --- a/drivers/rtc/rtc-efi.c
> +++ b/drivers/rtc/rtc-efi.c
> @@ -112,6 +112,34 @@ convert_from_efi_time(efi_time_t *eft, struct rtc_time *wtime)
> return true;
> }
>
> +static int efi_read_alarm(struct device *dev, struct rtc_wkalrm *wkalrm)
> +{
> + efi_time_t eft;
> + efi_status_t status;
> +
> + status = efi.get_wakeup_time((efi_bool_t *)&wkalrm->enabled,
> + (efi_bool_t *)&wkalrm->pending, &eft);
> + if (status != EFI_SUCCESS)
> + return -EINVAL;
> +
> + if (!convert_from_efi_time(&eft, &wkalrm->time))
> + return -EIO;
> +
> + return rtc_valid_tm(&wkalrm->time);
> +}
> +
> +static int efi_set_alarm(struct device *dev, struct rtc_wkalrm *wkalrm)
> +{
> + efi_time_t eft;
> + efi_status_t status;
> +
> + convert_to_efi_time(&wkalrm->time, &eft);
> +
> + status = efi.set_wakeup_time((efi_bool_t)wkalrm->enabled, &eft);
> +
> + return status == EFI_SUCCESS ? 0 : -EINVAL;
> +}
> +
> static int efi_read_time(struct device *dev, struct rtc_time *tm)
> {
> efi_status_t status;
> @@ -146,13 +174,21 @@ static int efi_set_time(struct device *dev, struct rtc_time *tm)
>
> static int efi_procfs(struct device *dev, struct seq_file *seq)
> {
> - efi_time_t eft;
> + efi_time_t eft, alm;
> efi_time_cap_t cap;
> + efi_bool_t enabled, pending;
> + struct rtc_device *rtc = dev_get_drvdata(dev);
>
> memset(&eft, 0, sizeof(eft));
> + memset(&alm, 0, sizeof(alm));
> memset(&cap, 0, sizeof(cap));
>
> efi.get_time(&eft, &cap);
> + if (test_bit(RTC_FEATURE_ALARM, rtc->features) &&
> + efi.get_wakeup_time(&enabled, &pending, &alm) != EFI_SUCCESS) {
> + enabled = false;
> + pending = false;
> + }
>
> seq_printf(seq,
> "Time\t\t: %u:%u:%u.%09u\n"
> @@ -168,6 +204,25 @@ static int efi_procfs(struct device *dev, struct seq_file *seq)
> /* XXX fixme: convert to string? */
> seq_printf(seq, "Timezone\t: %u\n", eft.timezone);
>
> + if (test_bit(RTC_FEATURE_ALARM, rtc->features)) {
> + seq_printf(seq,
> + "Alarm Time\t: %u:%u:%u.%09u\n"
> + "Alarm Date\t: %u-%u-%u\n"
> + "Alarm Daylight\t: %u\n"
> + "Enabled\t\t: %s\n"
> + "Pending\t\t: %s\n",
> + alm.hour, alm.minute, alm.second, alm.nanosecond,
> + alm.year, alm.month, alm.day,
> + alm.daylight,
> + enabled == 1 ? "yes" : "no",
> + pending == 1 ? "yes" : "no");
> +
> + if (alm.timezone == EFI_UNSPECIFIED_TIMEZONE)
> + seq_puts(seq, "Alarm Timezone\t: unspecified\n");
> + else
> + seq_printf(seq, "Alarm Timezone\t: %u\n", alm.timezone);
> + }
> +
> /*
> * now prints the capabilities
> */
> @@ -183,6 +238,8 @@ static int efi_procfs(struct device *dev, struct seq_file *seq)
> static const struct rtc_class_ops efi_rtc_ops = {
> .read_time = efi_read_time,
> .set_time = efi_set_time,
> + .read_alarm = efi_read_alarm,
> + .set_alarm = efi_set_alarm,
> .proc = efi_procfs,
> };
>
> @@ -191,6 +248,7 @@ static int __init efi_rtc_probe(struct platform_device *dev)
> struct rtc_device *rtc;
> efi_time_t eft;
> efi_time_cap_t cap;
> + efi_bool_t enabled, pending;
>
> /* First check if the RTC is usable */
> if (efi.get_time(&eft, &cap) != EFI_SUCCESS)
> @@ -203,7 +261,23 @@ static int __init efi_rtc_probe(struct platform_device *dev)
> platform_set_drvdata(dev, rtc);
>
> rtc->ops = &efi_rtc_ops;
> - clear_bit(RTC_FEATURE_ALARM, rtc->features);
> + clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, rtc->features);
> +
> + /*
> + * The EFI_RT_SUPPORTED_WAKEUP_SERVICES bit defaults to enabled
> + * and only gets cleared when the RT_PROP table explicitly says
> + * wakeup is unsupported. Many platforms lack an RT_PROP table
> + * even though they don't implement the wakeup runtime service,
> + * so probe by actually calling GetWakeupTime() to avoid exposing
> + * a broken alarm to userspace.
> + */
> + if (efi_rt_services_supported(EFI_RT_SUPPORTED_WAKEUP_SERVICES) &&
> + efi.get_wakeup_time(&enabled, &pending, &eft) == EFI_SUCCESS) {
> + set_bit(RTC_FEATURE_ALARM, rtc->features);
> + set_bit(RTC_FEATURE_ALARM_WAKEUP_ONLY, rtc->features);
> + } else {
> + clear_bit(RTC_FEATURE_ALARM, rtc->features);
> + }
>
> device_init_wakeup(&dev->dev, true);
>
> --
> 2.55.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2] rtc: efi: restore alarm support with runtime capability probe
2026-08-28 3:15 [PATCH] rtc: efi: restore alarm support with runtime capability probe Johnny.Fan
` (2 preceding siblings ...)
2026-08-31 1:10 ` Feng Tang
@ 2026-08-31 11:38 ` Johnny.Fan
2026-08-31 11:56 ` sashiko-bot
2026-08-31 13:34 ` Ard Biesheuvel
2026-08-31 14:00 ` [PATCH v3] " Johnny.Fan
4 siblings, 2 replies; 13+ messages in thread
From: Johnny.Fan @ 2026-08-31 11:38 UTC (permalink / raw)
To: Alexandre Belloni, linux-rtc
Cc: Ard Biesheuvel, Feng Tang, linux-efi, linux-kernel,
cix-kernel-upstream, bob.zhang, jie.fu
From: "Johnny.Fan" <Johnny.Fan@cixtech.com>
Restore the EFI RTC alarm functionality (read_alarm/set_alarm and
procfs output) that was removed by commit 18a3510bc87d ("rtc: efi:
Remove wakeup functionality").
The removal was motivated by the observation that many EFI platforms
lack RTC wakeup support while also omitting the RT_PROP table, causing
the wakeup capability to be incorrectly advertised. While that is
true, removing the feature entirely also breaks platforms where the
wakeup runtime service actually works.
On CIX SoCs, the RTC is an I2C device owned by firmware, and in the
ACPI boot configuration the I2C bus it sits on is not exposed to the
OS, so the EFI runtime services are the only way Linux can access
the RTC at all. Both Linux and Windows use this same interface by
design, so the RTC state is consistent across the operating systems
installed on the machine, and Windows relies on the EFI wakeup
services for its own RTC alarm support.
With the alarm support removed from rtc-efi, Linux loses the
equivalent capability entirely on these platforms: rtcwake fails
for both suspend-to-RAM and power-off wakeup, as there is no other
path to program the RTC alarm. This is a real product requirement
that CIX customers have explicitly requested, not a hypothetical
use case.
To avoid exposing a broken alarm on firmware that does not implement
the wakeup runtime services, probe the capability at ->probe() time by
actually calling GetWakeupTime() -- if it fails, clear RTC_FEATURE_ALARM
and the device behaves as before the removal.
Also set RTC_FEATURE_ALARM_WAKEUP_ONLY since EFI wakeup time is a
wakeup-only alarm, not a periodic/update interrupt source.
Reviewed-by: Feng Tang <feng.tang@linux.alibaba.com>
Reviewed-by: Fugang Duan <fugang.duan@cixtech.com>
Tested-by: Johnny.Fan <Johnny.Fan@cixtech.com>
Signed-off-by: Johnny.Fan <Johnny.Fan@cixtech.com>
---
v2:
- Expand the commit message with the actual product motivation,
as requested by Ard.
- Zero-initialize efi_time_t in convert_to_efi_time() to avoid
leaking stack garbage in pad1/pad2 fields to firmware.
- Zero-initialize the local eft in efi_read_alarm() so that
convert_from_efi_time() does not fail with -EIO on disabled
alarms, where the returned time is undefined per the UEFI spec.
- Normalize wkalrm->enabled to a strict boolean (!!) before
passing it to SetWakeupTime, as UEFI BOOLEAN must be 0 or 1.
- Use %d for the signed alm.timezone value in procfs output.
drivers/rtc/rtc-efi.c | 80 +++++++++++++++++++++++++++++++++++++++++--
1 file changed, 78 insertions(+), 2 deletions(-)
diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
index b4f44999ef0f..5276870b410e 100644
--- a/drivers/rtc/rtc-efi.c
+++ b/drivers/rtc/rtc-efi.c
@@ -52,6 +52,7 @@ compute_wday(efi_time_t *eft, int yday)
static void
convert_to_efi_time(struct rtc_time *wtime, efi_time_t *eft)
{
+ memset(eft, 0, sizeof(*eft));
eft->year = wtime->tm_year + 1900;
eft->month = wtime->tm_mon + 1;
eft->day = wtime->tm_mday;
@@ -112,6 +113,35 @@ convert_from_efi_time(efi_time_t *eft, struct rtc_time *wtime)
return true;
}
+static int efi_read_alarm(struct device *dev, struct rtc_wkalrm *wkalrm)
+{
+ efi_time_t eft;
+ efi_status_t status;
+
+ memset(&eft, 0, sizeof(eft));
+ status = efi.get_wakeup_time((efi_bool_t *)&wkalrm->enabled,
+ (efi_bool_t *)&wkalrm->pending, &eft);
+ if (status != EFI_SUCCESS)
+ return -EINVAL;
+
+ if (!convert_from_efi_time(&eft, &wkalrm->time))
+ return -EIO;
+
+ return rtc_valid_tm(&wkalrm->time);
+}
+
+static int efi_set_alarm(struct device *dev, struct rtc_wkalrm *wkalrm)
+{
+ efi_time_t eft;
+ efi_status_t status;
+
+ convert_to_efi_time(&wkalrm->time, &eft);
+
+ status = efi.set_wakeup_time((efi_bool_t)!!wkalrm->enabled, &eft);
+
+ return status == EFI_SUCCESS ? 0 : -EINVAL;
+}
+
static int efi_read_time(struct device *dev, struct rtc_time *tm)
{
efi_status_t status;
@@ -146,13 +176,21 @@ static int efi_set_time(struct device *dev, struct rtc_time *tm)
static int efi_procfs(struct device *dev, struct seq_file *seq)
{
- efi_time_t eft;
+ efi_time_t eft, alm;
efi_time_cap_t cap;
+ efi_bool_t enabled, pending;
+ struct rtc_device *rtc = dev_get_drvdata(dev);
memset(&eft, 0, sizeof(eft));
+ memset(&alm, 0, sizeof(alm));
memset(&cap, 0, sizeof(cap));
efi.get_time(&eft, &cap);
+ if (test_bit(RTC_FEATURE_ALARM, rtc->features) &&
+ efi.get_wakeup_time(&enabled, &pending, &alm) != EFI_SUCCESS) {
+ enabled = false;
+ pending = false;
+ }
seq_printf(seq,
"Time\t\t: %u:%u:%u.%09u\n"
@@ -168,6 +206,25 @@ static int efi_procfs(struct device *dev, struct seq_file *seq)
/* XXX fixme: convert to string? */
seq_printf(seq, "Timezone\t: %u\n", eft.timezone);
+ if (test_bit(RTC_FEATURE_ALARM, rtc->features)) {
+ seq_printf(seq,
+ "Alarm Time\t: %u:%u:%u.%09u\n"
+ "Alarm Date\t: %u-%u-%u\n"
+ "Alarm Daylight\t: %u\n"
+ "Enabled\t\t: %s\n"
+ "Pending\t\t: %s\n",
+ alm.hour, alm.minute, alm.second, alm.nanosecond,
+ alm.year, alm.month, alm.day,
+ alm.daylight,
+ enabled == 1 ? "yes" : "no",
+ pending == 1 ? "yes" : "no");
+
+ if (alm.timezone == EFI_UNSPECIFIED_TIMEZONE)
+ seq_puts(seq, "Alarm Timezone\t: unspecified\n");
+ else
+ seq_printf(seq, "Alarm Timezone\t: %d\n", alm.timezone);
+ }
+
/*
* now prints the capabilities
*/
@@ -183,6 +240,8 @@ static int efi_procfs(struct device *dev, struct seq_file *seq)
static const struct rtc_class_ops efi_rtc_ops = {
.read_time = efi_read_time,
.set_time = efi_set_time,
+ .read_alarm = efi_read_alarm,
+ .set_alarm = efi_set_alarm,
.proc = efi_procfs,
};
@@ -191,6 +250,7 @@ static int __init efi_rtc_probe(struct platform_device *dev)
struct rtc_device *rtc;
efi_time_t eft;
efi_time_cap_t cap;
+ efi_bool_t enabled, pending;
/* First check if the RTC is usable */
if (efi.get_time(&eft, &cap) != EFI_SUCCESS)
@@ -203,7 +263,23 @@ static int __init efi_rtc_probe(struct platform_device *dev)
platform_set_drvdata(dev, rtc);
rtc->ops = &efi_rtc_ops;
- clear_bit(RTC_FEATURE_ALARM, rtc->features);
+ clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, rtc->features);
+
+ /*
+ * The EFI_RT_SUPPORTED_WAKEUP_SERVICES bit defaults to enabled
+ * and only gets cleared when the RT_PROP table explicitly says
+ * wakeup is unsupported. Many platforms lack an RT_PROP table
+ * even though they don't implement the wakeup runtime service,
+ * so probe by actually calling GetWakeupTime() to avoid exposing
+ * a broken alarm to userspace.
+ */
+ if (efi_rt_services_supported(EFI_RT_SUPPORTED_WAKEUP_SERVICES) &&
+ efi.get_wakeup_time(&enabled, &pending, &eft) == EFI_SUCCESS) {
+ set_bit(RTC_FEATURE_ALARM, rtc->features);
+ set_bit(RTC_FEATURE_ALARM_WAKEUP_ONLY, rtc->features);
+ } else {
+ clear_bit(RTC_FEATURE_ALARM, rtc->features);
+ }
device_init_wakeup(&dev->dev, true);
--
2.55.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2] rtc: efi: restore alarm support with runtime capability probe
2026-08-31 11:38 ` [PATCH v2] " Johnny.Fan
@ 2026-08-31 11:56 ` sashiko-bot
2026-08-31 13:34 ` Ard Biesheuvel
1 sibling, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-08-31 11:56 UTC (permalink / raw)
To: Johnny.Fan; +Cc: linux-rtc, Alexandre Belloni
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Zero-initializing `eft` in `efi_read_alarm()` guarantees an `-EIO` failure when reading disabled alarms, breaking user-space reads.
--
commit f9829a03fcbfff469b82ea3dd308ff1029bfea0e
Author: Johnny.Fan <Johnny.Fan@cixtech.com>
rtc: efi: restore alarm support with runtime capability probe
This commit restores the EFI RTC alarm functionality and procfs output that
was previously removed. It introduces a runtime probe of the wakeup
capability to avoid exposing a broken alarm if the service is unsupported.
> diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
> index b4f44999ef0f4..5276870b410e0 100644
> --- a/drivers/rtc/rtc-efi.c
> +++ b/drivers/rtc/rtc-efi.c
[ ... ]
> @@ -112,6 +113,35 @@ convert_from_efi_time(efi_time_t *eft, struct rtc_time *wtime)
> return true;
> }
>
> +static int efi_read_alarm(struct device *dev, struct rtc_wkalrm *wkalrm)
> +{
> + efi_time_t eft;
> + efi_status_t status;
> +
> + memset(&eft, 0, sizeof(eft));
> + status = efi.get_wakeup_time((efi_bool_t *)&wkalrm->enabled,
> + (efi_bool_t *)&wkalrm->pending, &eft);
> + if (status != EFI_SUCCESS)
> + return -EINVAL;
> +
> + if (!convert_from_efi_time(&eft, &wkalrm->time))
> + return -EIO;
[Severity: High]
Does this return -EIO for disabled alarms?
When a user queries the RTC alarm via the standard ioctl interface,
efi_read_alarm() zero-initializes eft before calling efi.get_wakeup_time().
Per the UEFI specification, if the alarm is disabled, the time output is
undefined and the firmware may leave eft unmodified.
Because eft is all zeroes, its day field is 0. The convert_from_efi_time()
function strictly validates the time, rejecting day == 0, and returns false:
drivers/rtc/rtc-efi.c:convert_from_efi_time() {
...
if (!eft->day || eft->day > 31)
return false;
...
}
Will this cause efi_read_alarm() to return -EIO to the subsystem, propagating
the error to userspace instead of correctly reporting the alarm as disabled,
and potentially breaking expected functionality for standard userspace tools
like rtcwake and hwclock?
> +
> + return rtc_valid_tm(&wkalrm->time);
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260831113858.759210-1-johnny.fan@cixtech.com?part=1
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] rtc: efi: restore alarm support with runtime capability probe
2026-08-31 11:38 ` [PATCH v2] " Johnny.Fan
2026-08-31 11:56 ` sashiko-bot
@ 2026-08-31 13:34 ` Ard Biesheuvel
1 sibling, 0 replies; 13+ messages in thread
From: Ard Biesheuvel @ 2026-08-31 13:34 UTC (permalink / raw)
To: Johnny.Fan, Alexandre Belloni, linux-rtc
Cc: Feng Tang, linux-efi, linux-kernel, cix-kernel-upstream,
bob.zhang, jie.fu
On Mon, 31 Aug 2026, at 13:38, Johnny.Fan wrote:
> From: "Johnny.Fan" <Johnny.Fan@cixtech.com>
>
> Restore the EFI RTC alarm functionality (read_alarm/set_alarm and
> procfs output) that was removed by commit 18a3510bc87d ("rtc: efi:
> Remove wakeup functionality").
>
> The removal was motivated by the observation that many EFI platforms
> lack RTC wakeup support while also omitting the RT_PROP table, causing
> the wakeup capability to be incorrectly advertised. While that is
> true, removing the feature entirely also breaks platforms where the
> wakeup runtime service actually works.
>
> On CIX SoCs, the RTC is an I2C device owned by firmware, and in the
> ACPI boot configuration the I2C bus it sits on is not exposed to the
> OS, so the EFI runtime services are the only way Linux can access
> the RTC at all. Both Linux and Windows use this same interface by
> design, so the RTC state is consistent across the operating systems
> installed on the machine, and Windows relies on the EFI wakeup
> services for its own RTC alarm support.
>
> With the alarm support removed from rtc-efi, Linux loses the
> equivalent capability entirely on these platforms: rtcwake fails
> for both suspend-to-RAM and power-off wakeup, as there is no other
> path to program the RTC alarm. This is a real product requirement
> that CIX customers have explicitly requested, not a hypothetical
> use case.
>
> To avoid exposing a broken alarm on firmware that does not implement
> the wakeup runtime services, probe the capability at ->probe() time by
> actually calling GetWakeupTime() -- if it fails, clear RTC_FEATURE_ALARM
> and the device behaves as before the removal.
>
> Also set RTC_FEATURE_ALARM_WAKEUP_ONLY since EFI wakeup time is a
> wakeup-only alarm, not a periodic/update interrupt source.
>
> Reviewed-by: Feng Tang <feng.tang@linux.alibaba.com>
> Reviewed-by: Fugang Duan <fugang.duan@cixtech.com>
> Tested-by: Johnny.Fan <Johnny.Fan@cixtech.com>
Please drop this tested-by - it is implied.
With that fixed, and the tweak below,
Acked-by: Ard Biesheuvel <ardb@kernel.org>
> Signed-off-by: Johnny.Fan <Johnny.Fan@cixtech.com>
> ---
> v2:
> - Expand the commit message with the actual product motivation,
> as requested by Ard.
> - Zero-initialize efi_time_t in convert_to_efi_time() to avoid
> leaking stack garbage in pad1/pad2 fields to firmware.
> - Zero-initialize the local eft in efi_read_alarm() so that
> convert_from_efi_time() does not fail with -EIO on disabled
> alarms, where the returned time is undefined per the UEFI spec.
> - Normalize wkalrm->enabled to a strict boolean (!!) before
> passing it to SetWakeupTime, as UEFI BOOLEAN must be 0 or 1.
> - Use %d for the signed alm.timezone value in procfs output.
>
> drivers/rtc/rtc-efi.c | 80 +++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 78 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
> index b4f44999ef0f..5276870b410e 100644
> --- a/drivers/rtc/rtc-efi.c
> +++ b/drivers/rtc/rtc-efi.c
> @@ -52,6 +52,7 @@ compute_wday(efi_time_t *eft, int yday)
> static void
> convert_to_efi_time(struct rtc_time *wtime, efi_time_t *eft)
> {
> + memset(eft, 0, sizeof(*eft));
> eft->year = wtime->tm_year + 1900;
> eft->month = wtime->tm_mon + 1;
> eft->day = wtime->tm_mday;
> @@ -112,6 +113,35 @@ convert_from_efi_time(efi_time_t *eft, struct
> rtc_time *wtime)
> return true;
> }
>
> +static int efi_read_alarm(struct device *dev, struct rtc_wkalrm
> *wkalrm)
> +{
> + efi_time_t eft;
> + efi_status_t status;
> +
> + memset(&eft, 0, sizeof(eft));
> + status = efi.get_wakeup_time((efi_bool_t *)&wkalrm->enabled,
> + (efi_bool_t *)&wkalrm->pending, &eft);
> + if (status != EFI_SUCCESS)
> + return -EINVAL;
> +
> + if (!convert_from_efi_time(&eft, &wkalrm->time))
> + return -EIO;
> +
> + return rtc_valid_tm(&wkalrm->time);
> +}
> +
> +static int efi_set_alarm(struct device *dev, struct rtc_wkalrm *wkalrm)
> +{
> + efi_time_t eft;
> + efi_status_t status;
> +
> + convert_to_efi_time(&wkalrm->time, &eft);
> +
> + status = efi.set_wakeup_time((efi_bool_t)!!wkalrm->enabled, &eft);
> +
> + return status == EFI_SUCCESS ? 0 : -EINVAL;
> +}
> +
> static int efi_read_time(struct device *dev, struct rtc_time *tm)
> {
> efi_status_t status;
> @@ -146,13 +176,21 @@ static int efi_set_time(struct device *dev,
> struct rtc_time *tm)
>
> static int efi_procfs(struct device *dev, struct seq_file *seq)
> {
> - efi_time_t eft;
> + efi_time_t eft, alm;
> efi_time_cap_t cap;
> + efi_bool_t enabled, pending;
> + struct rtc_device *rtc = dev_get_drvdata(dev);
>
> memset(&eft, 0, sizeof(eft));
> + memset(&alm, 0, sizeof(alm));
> memset(&cap, 0, sizeof(cap));
>
> efi.get_time(&eft, &cap);
> + if (test_bit(RTC_FEATURE_ALARM, rtc->features) &&
> + efi.get_wakeup_time(&enabled, &pending, &alm) != EFI_SUCCESS) {
> + enabled = false;
> + pending = false;
Assign 0 here not false - efi_bool_t is a u8.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3] rtc: efi: restore alarm support with runtime capability probe
2026-08-28 3:15 [PATCH] rtc: efi: restore alarm support with runtime capability probe Johnny.Fan
` (3 preceding siblings ...)
2026-08-31 11:38 ` [PATCH v2] " Johnny.Fan
@ 2026-08-31 14:00 ` Johnny.Fan
2026-08-31 17:47 ` sashiko-bot
4 siblings, 1 reply; 13+ messages in thread
From: Johnny.Fan @ 2026-08-31 14:00 UTC (permalink / raw)
To: Alexandre Belloni, linux-rtc
Cc: Ard Biesheuvel, Feng Tang, linux-efi, linux-kernel,
cix-kernel-upstream, bob.zhang, jie.fu
From: "Johnny.Fan" <Johnny.Fan@cixtech.com>
Restore the EFI RTC alarm functionality (read_alarm/set_alarm and
procfs output) that was removed by commit 18a3510bc87d ("rtc: efi:
Remove wakeup functionality").
The removal was motivated by the observation that many EFI platforms
lack RTC wakeup support while also omitting the RT_PROP table, causing
the wakeup capability to be incorrectly advertised. While that is
true, removing the feature entirely also breaks platforms where the
wakeup runtime service actually works.
On CIX SoCs, the RTC is an I2C device owned by firmware, and in the
ACPI boot configuration the I2C bus it sits on is not exposed to the
OS, so the EFI runtime services are the only way Linux can access
the RTC at all. Both Linux and Windows use this same interface by
design, so the RTC state is consistent across the operating systems
installed on the machine, and Windows relies on the EFI wakeup
services for its own RTC alarm support.
With the alarm support removed from rtc-efi, Linux loses the
equivalent capability entirely on these platforms: rtcwake fails
for both suspend-to-RAM and power-off wakeup, as there is no other
path to program the RTC alarm. This is a real product requirement
that CIX customers have explicitly requested, not a hypothetical
use case.
To avoid exposing a broken alarm on firmware that does not implement
the wakeup runtime services, probe the capability at ->probe() time by
actually calling GetWakeupTime() -- if it fails, clear RTC_FEATURE_ALARM
and the device behaves as before the removal.
Also set RTC_FEATURE_ALARM_WAKEUP_ONLY since EFI wakeup time is a
wakeup-only alarm, not a periodic/update interrupt source.
Acked-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Feng Tang <feng.tang@linux.alibaba.com>
Reviewed-by: Fugang Duan <fugang.duan@cixtech.com>
Signed-off-by: Johnny.Fan <Johnny.Fan@cixtech.com>
---
v3:
- Assign 0 instead of false to the efi_bool_t variables, as
suggested by Ard.
- Drop the Tested-by tag.
- Collect Ard's Acked-by.
v2:
- Expand the commit message with the actual product motivation,
as requested by Ard.
- Zero-initialize efi_time_t in convert_to_efi_time() to avoid
leaking stack garbage in pad1/pad2 fields to firmware.
- Zero-initialize the local eft in efi_read_alarm() so that
convert_from_efi_time() does not fail with -EIO on disabled
alarms, where the returned time is undefined per the UEFI spec.
- Normalize wkalrm->enabled to a strict boolean (!!) before
passing it to SetWakeupTime, as UEFI BOOLEAN must be 0 or 1.
- Use %d for the signed alm.timezone value in procfs output.
drivers/rtc/rtc-efi.c | 80 +++++++++++++++++++++++++++++++++++++++++--
1 file changed, 78 insertions(+), 2 deletions(-)
diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
index b4f44999ef0f..a8c18ebc5599 100644
--- a/drivers/rtc/rtc-efi.c
+++ b/drivers/rtc/rtc-efi.c
@@ -52,6 +52,7 @@ compute_wday(efi_time_t *eft, int yday)
static void
convert_to_efi_time(struct rtc_time *wtime, efi_time_t *eft)
{
+ memset(eft, 0, sizeof(*eft));
eft->year = wtime->tm_year + 1900;
eft->month = wtime->tm_mon + 1;
eft->day = wtime->tm_mday;
@@ -112,6 +113,35 @@ convert_from_efi_time(efi_time_t *eft, struct rtc_time *wtime)
return true;
}
+static int efi_read_alarm(struct device *dev, struct rtc_wkalrm *wkalrm)
+{
+ efi_time_t eft;
+ efi_status_t status;
+
+ memset(&eft, 0, sizeof(eft));
+ status = efi.get_wakeup_time((efi_bool_t *)&wkalrm->enabled,
+ (efi_bool_t *)&wkalrm->pending, &eft);
+ if (status != EFI_SUCCESS)
+ return -EINVAL;
+
+ if (!convert_from_efi_time(&eft, &wkalrm->time))
+ return -EIO;
+
+ return rtc_valid_tm(&wkalrm->time);
+}
+
+static int efi_set_alarm(struct device *dev, struct rtc_wkalrm *wkalrm)
+{
+ efi_time_t eft;
+ efi_status_t status;
+
+ convert_to_efi_time(&wkalrm->time, &eft);
+
+ status = efi.set_wakeup_time((efi_bool_t)!!wkalrm->enabled, &eft);
+
+ return status == EFI_SUCCESS ? 0 : -EINVAL;
+}
+
static int efi_read_time(struct device *dev, struct rtc_time *tm)
{
efi_status_t status;
@@ -146,13 +176,21 @@ static int efi_set_time(struct device *dev, struct rtc_time *tm)
static int efi_procfs(struct device *dev, struct seq_file *seq)
{
- efi_time_t eft;
+ efi_time_t eft, alm;
efi_time_cap_t cap;
+ efi_bool_t enabled, pending;
+ struct rtc_device *rtc = dev_get_drvdata(dev);
memset(&eft, 0, sizeof(eft));
+ memset(&alm, 0, sizeof(alm));
memset(&cap, 0, sizeof(cap));
efi.get_time(&eft, &cap);
+ if (test_bit(RTC_FEATURE_ALARM, rtc->features) &&
+ efi.get_wakeup_time(&enabled, &pending, &alm) != EFI_SUCCESS) {
+ enabled = 0;
+ pending = 0;
+ }
seq_printf(seq,
"Time\t\t: %u:%u:%u.%09u\n"
@@ -168,6 +206,25 @@ static int efi_procfs(struct device *dev, struct seq_file *seq)
/* XXX fixme: convert to string? */
seq_printf(seq, "Timezone\t: %u\n", eft.timezone);
+ if (test_bit(RTC_FEATURE_ALARM, rtc->features)) {
+ seq_printf(seq,
+ "Alarm Time\t: %u:%u:%u.%09u\n"
+ "Alarm Date\t: %u-%u-%u\n"
+ "Alarm Daylight\t: %u\n"
+ "Enabled\t\t: %s\n"
+ "Pending\t\t: %s\n",
+ alm.hour, alm.minute, alm.second, alm.nanosecond,
+ alm.year, alm.month, alm.day,
+ alm.daylight,
+ enabled == 1 ? "yes" : "no",
+ pending == 1 ? "yes" : "no");
+
+ if (alm.timezone == EFI_UNSPECIFIED_TIMEZONE)
+ seq_puts(seq, "Alarm Timezone\t: unspecified\n");
+ else
+ seq_printf(seq, "Alarm Timezone\t: %d\n", alm.timezone);
+ }
+
/*
* now prints the capabilities
*/
@@ -183,6 +240,8 @@ static int efi_procfs(struct device *dev, struct seq_file *seq)
static const struct rtc_class_ops efi_rtc_ops = {
.read_time = efi_read_time,
.set_time = efi_set_time,
+ .read_alarm = efi_read_alarm,
+ .set_alarm = efi_set_alarm,
.proc = efi_procfs,
};
@@ -191,6 +250,7 @@ static int __init efi_rtc_probe(struct platform_device *dev)
struct rtc_device *rtc;
efi_time_t eft;
efi_time_cap_t cap;
+ efi_bool_t enabled, pending;
/* First check if the RTC is usable */
if (efi.get_time(&eft, &cap) != EFI_SUCCESS)
@@ -203,7 +263,23 @@ static int __init efi_rtc_probe(struct platform_device *dev)
platform_set_drvdata(dev, rtc);
rtc->ops = &efi_rtc_ops;
- clear_bit(RTC_FEATURE_ALARM, rtc->features);
+ clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, rtc->features);
+
+ /*
+ * The EFI_RT_SUPPORTED_WAKEUP_SERVICES bit defaults to enabled
+ * and only gets cleared when the RT_PROP table explicitly says
+ * wakeup is unsupported. Many platforms lack an RT_PROP table
+ * even though they don't implement the wakeup runtime service,
+ * so probe by actually calling GetWakeupTime() to avoid exposing
+ * a broken alarm to userspace.
+ */
+ if (efi_rt_services_supported(EFI_RT_SUPPORTED_WAKEUP_SERVICES) &&
+ efi.get_wakeup_time(&enabled, &pending, &eft) == EFI_SUCCESS) {
+ set_bit(RTC_FEATURE_ALARM, rtc->features);
+ set_bit(RTC_FEATURE_ALARM_WAKEUP_ONLY, rtc->features);
+ } else {
+ clear_bit(RTC_FEATURE_ALARM, rtc->features);
+ }
device_init_wakeup(&dev->dev, true);
--
2.55.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3] rtc: efi: restore alarm support with runtime capability probe
2026-08-31 14:00 ` [PATCH v3] " Johnny.Fan
@ 2026-08-31 17:47 ` sashiko-bot
0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-08-31 17:47 UTC (permalink / raw)
To: Johnny.Fan; +Cc: Alexandre Belloni, linux-rtc
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Zero-initializing `eft` causes `efi_read_alarm()` to always fail with `-EIO` for disabled alarms, completely breaking userspace reads.
--
commit b695e617f63e29889d5f34a76788ecbfdea7f619
Author: Johnny.Fan <Johnny.Fan@cixtech.com>
rtc: efi: restore alarm support with runtime capability probe
This commit restores EFI RTC alarm functionality removed in a previous commit.
It adds a capability probe at ->probe() time to check if GetWakeupTime()
actually works before exposing the alarm feature to the OS.
> diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
> index b4f44999ef0f4..a8c18ebc55996 100644
> --- a/drivers/rtc/rtc-efi.c
> +++ b/drivers/rtc/rtc-efi.c
[ ... ]
> @@ -112,6 +113,35 @@ convert_from_efi_time(efi_time_t *eft, struct rtc_time *wtime)
> return true;
> }
>
> +static int efi_read_alarm(struct device *dev, struct rtc_wkalrm *wkalrm)
> +{
> + efi_time_t eft;
> + efi_status_t status;
> +
> + memset(&eft, 0, sizeof(eft));
> + status = efi.get_wakeup_time((efi_bool_t *)&wkalrm->enabled,
> + (efi_bool_t *)&wkalrm->pending, &eft);
> + if (status != EFI_SUCCESS)
> + return -EINVAL;
> +
> + if (!convert_from_efi_time(&eft, &wkalrm->time))
> + return -EIO;
[Severity: High]
Does this zero-initialization cause efi_read_alarm() to fail unconditionally
with -EIO for disabled alarms?
When the alarm is disabled, firmware can return EFI_SUCCESS but leave the
eft structure untouched, as allowed by the UEFI spec. Because this patch adds
memset(&eft, 0, sizeof(eft)), eft.day and eft.month will remain 0.
Looking at convert_from_efi_time(), there is a preexisting validation check:
drivers/rtc/rtc-efi.c:convert_from_efi_time() {
...
if (!eft->day || eft->day > 31)
return false;
...
}
This check strictly rejects 0 as a valid day, nullifying the attempt to use
an all-zero structure to bypass validation. As a result, reading a disabled
RTC alarm will fail with -EIO rather than correctly reporting the disabled
state, breaking userspace tools like hwclock or sysfs reads that expect to
read the alarm status without a hard I/O error.
> +
> + return rtc_valid_tm(&wkalrm->time);
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260831140014.839222-1-johnny.fan@cixtech.com?part=1
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2026-08-31 17:47 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-28 3:15 [PATCH] rtc: efi: restore alarm support with runtime capability probe Johnny.Fan
2026-08-28 3:32 ` sashiko-bot
2026-08-28 7:26 ` Ard Biesheuvel
2026-08-28 8:23 ` Johnny Fan
2026-08-28 8:32 ` Johnny Fan
2026-08-28 10:13 ` Ard Biesheuvel
2026-08-28 12:48 ` Alexandre Belloni
2026-08-31 1:10 ` Feng Tang
2026-08-31 11:38 ` [PATCH v2] " Johnny.Fan
2026-08-31 11:56 ` sashiko-bot
2026-08-31 13:34 ` Ard Biesheuvel
2026-08-31 14:00 ` [PATCH v3] " Johnny.Fan
2026-08-31 17:47 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox