* [RFC PATCH 0/3] Remove unused EFI runtime APIs
@ 2025-07-14 6:08 Ard Biesheuvel
2025-07-14 6:08 ` [RFC PATCH 1/3] efi-rtc: Remove wakeup functionality Ard Biesheuvel
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Ard Biesheuvel @ 2025-07-14 6:08 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arm-kernel, Ard Biesheuvel, Heinrich Schuchardt, Feng Tang,
Alexandre Belloni, Juergen Gross, Stefano Stabellini,
Oleksandr Tyshchenko, Sunil V L, Bibo Mao, linux-rtc, linux-efi,
xen-devel, x86, linux-riscv, loongarch
From: Ard Biesheuvel <ardb@kernel.org>
Using EFI runtime services to program the RTC to wake up the system is
supported in theory, but rarely works in practice. Fortunately, this
functionality is rarely [if ever] used to begin with so we can just drop
it. (Note that the EFI rtc driver is not used by x86, which programs the
CMOS rtc directly)
The same applies to GetNextHighMonoCount(), which, if implemented,
usually relies on SetVariable() under the hood *, which is often not
supported at runtime by non-x86 platforms. But it has no known users
either so let's drop support for it as well.
This means we need to drop the slightly pointless tests for it too.
* EDK2 based EFI implementations usually have a MTC variable carrying
the monotonic counter variable, which is therefore not truly
monotonic, given that SetVariable() will happily overwrite it.
Cc: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Cc: Feng Tang <feng.tang@linux.alibaba.com>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
Cc: Sunil V L <sunilvl@ventanamicro.com>
Cc: Bibo Mao <maobibo@loongson.cn>
Cc: linux-rtc@vger.kernel.org
Cc: linux-efi@vger.kernel.org
Cc: xen-devel@lists.xenproject.org
Cc: x86@kernel.org
Cc: linux-riscv@lists.infradead.org
Cc: loongarch@lists.linux.dev
Ard Biesheuvel (3):
efi-rtc: Remove wakeup functionality
efi/test: Don't bother pseudo-testing unused EFI services
efi: Remove support for pointless, unused EFI services
arch/x86/platform/efi/efi_64.c | 22 ----
drivers/firmware/efi/runtime-wrappers.c | 68 ------------
drivers/firmware/efi/test/efi_test.c | 108 +-------------------
drivers/rtc/rtc-efi.c | 76 +-------------
drivers/xen/efi.c | 56 ----------
include/linux/efi.h | 6 --
6 files changed, 4 insertions(+), 332 deletions(-)
--
2.50.0.727.gbf7dc18ff4-goog
^ permalink raw reply [flat|nested] 16+ messages in thread
* [RFC PATCH 1/3] efi-rtc: Remove wakeup functionality
2025-07-14 6:08 [RFC PATCH 0/3] Remove unused EFI runtime APIs Ard Biesheuvel
@ 2025-07-14 6:08 ` Ard Biesheuvel
2025-07-14 6:13 ` Demi Marie Obenour
` (2 more replies)
2025-07-14 6:08 ` [RFC PATCH 2/3] efi/test: Don't bother pseudo-testing unused EFI services Ard Biesheuvel
` (2 subsequent siblings)
3 siblings, 3 replies; 16+ messages in thread
From: Ard Biesheuvel @ 2025-07-14 6:08 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arm-kernel, Ard Biesheuvel, Heinrich Schuchardt, Feng Tang,
Alexandre Belloni, Juergen Gross, Stefano Stabellini,
Oleksandr Tyshchenko, Sunil V L, Bibo Mao, linux-rtc, linux-efi,
xen-devel, x86, linux-riscv, loongarch
From: Ard Biesheuvel <ardb@kernel.org>
The EFI rtc driver is used by non-x86 architectures only, and exposes
the get/set wakeup time functionality provided by the underlying
platform. This is usually broken on most platforms, and not widely used
to begin with [if at all], so let's just remove it.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
drivers/rtc/rtc-efi.c | 76 +-------------------
1 file changed, 2 insertions(+), 74 deletions(-)
diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
index fa8bf82df948..b4f44999ef0f 100644
--- a/drivers/rtc/rtc-efi.c
+++ b/drivers/rtc/rtc-efi.c
@@ -112,48 +112,6 @@ 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;
-
- /*
- * As of EFI v1.10, this call always returns an unsupported 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);
-
- /*
- * XXX Fixme:
- * As of EFI 0.92 with the firmware I have on my
- * machine this call does not seem to work quite
- * right
- *
- * As of v1.10, this call always returns an unsupported status
- */
- status = efi.set_wakeup_time((efi_bool_t)wkalrm->enabled, &eft);
-
- dev_warn(dev, "write status is %d\n", (int)status);
-
- return status == EFI_SUCCESS ? 0 : -EINVAL;
-}
-
static int efi_read_time(struct device *dev, struct rtc_time *tm)
{
efi_status_t status;
@@ -188,17 +146,13 @@ 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, alm;
+ efi_time_t eft;
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);
- efi.get_wakeup_time(&enabled, &pending, &alm);
seq_printf(seq,
"Time\t\t: %u:%u:%u.%09u\n"
@@ -214,26 +168,6 @@ 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, "Timezone\t: unspecified\n");
- else
- /* XXX fixme: convert to string? */
- seq_printf(seq, "Timezone\t: %u\n", alm.timezone);
- }
-
/*
* now prints the capabilities
*/
@@ -249,8 +183,6 @@ 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,
};
@@ -271,11 +203,7 @@ static int __init efi_rtc_probe(struct platform_device *dev)
platform_set_drvdata(dev, rtc);
rtc->ops = &efi_rtc_ops;
- clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, rtc->features);
- if (efi_rt_services_supported(EFI_RT_SUPPORTED_WAKEUP_SERVICES))
- set_bit(RTC_FEATURE_ALARM_WAKEUP_ONLY, rtc->features);
- else
- clear_bit(RTC_FEATURE_ALARM, rtc->features);
+ clear_bit(RTC_FEATURE_ALARM, rtc->features);
device_init_wakeup(&dev->dev, true);
--
2.50.0.727.gbf7dc18ff4-goog
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [RFC PATCH 2/3] efi/test: Don't bother pseudo-testing unused EFI services
2025-07-14 6:08 [RFC PATCH 0/3] Remove unused EFI runtime APIs Ard Biesheuvel
2025-07-14 6:08 ` [RFC PATCH 1/3] efi-rtc: Remove wakeup functionality Ard Biesheuvel
@ 2025-07-14 6:08 ` Ard Biesheuvel
2025-07-14 6:08 ` [RFC PATCH 3/3] efi: Remove support for pointless, " Ard Biesheuvel
2025-07-14 8:10 ` [RFC PATCH 0/3] Remove unused EFI runtime APIs Heinrich Schuchardt
3 siblings, 0 replies; 16+ messages in thread
From: Ard Biesheuvel @ 2025-07-14 6:08 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arm-kernel, Ard Biesheuvel, Heinrich Schuchardt, Feng Tang,
Alexandre Belloni, Juergen Gross, Stefano Stabellini,
Oleksandr Tyshchenko, Sunil V L, Bibo Mao, linux-rtc, linux-efi,
xen-devel, x86, linux-riscv, loongarch
From: Ard Biesheuvel <ardb@kernel.org>
The EFI test module covers the get/set wakeup time EFI runtime
services, as well as GetNextHighMonoCount(). In both cases, though, it
just mindlessly exercises the API, without any functional testing.
In case of the get/set wakeup time services, this would involve setting
the wakeup time, and subsequently checking whether the system actually
wakes up at the configured time, which is difficult for obvious reasons.
In case of GetNextHighMonoCount(), this would involve performing some
kind of verification that the returned number increases monotonically
across reboots.
Given that these APIs are not used in Linux to begin with, let's not
pretend that testing them in this manner has any value, and just drop
these tests entirely, so that we can drop the APIs themselves from the
Linux EFI runtime layer.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
drivers/firmware/efi/test/efi_test.c | 108 +-------------------
1 file changed, 2 insertions(+), 106 deletions(-)
diff --git a/drivers/firmware/efi/test/efi_test.c b/drivers/firmware/efi/test/efi_test.c
index 77b5f7ac3e20..bb2ace902346 100644
--- a/drivers/firmware/efi/test/efi_test.c
+++ b/drivers/firmware/efi/test/efi_test.c
@@ -333,77 +333,6 @@ static long efi_runtime_set_time(unsigned long arg)
return status == EFI_SUCCESS ? 0 : -EINVAL;
}
-static long efi_runtime_get_waketime(unsigned long arg)
-{
- struct efi_getwakeuptime __user *getwakeuptime_user;
- struct efi_getwakeuptime getwakeuptime;
- efi_bool_t enabled, pending;
- efi_status_t status;
- efi_time_t efi_time;
-
- getwakeuptime_user = (struct efi_getwakeuptime __user *)arg;
- if (copy_from_user(&getwakeuptime, getwakeuptime_user,
- sizeof(getwakeuptime)))
- return -EFAULT;
-
- status = efi.get_wakeup_time(
- getwakeuptime.enabled ? (efi_bool_t *)&enabled : NULL,
- getwakeuptime.pending ? (efi_bool_t *)&pending : NULL,
- getwakeuptime.time ? &efi_time : NULL);
-
- if (put_user(status, getwakeuptime.status))
- return -EFAULT;
-
- if (status != EFI_SUCCESS)
- return -EINVAL;
-
- if (getwakeuptime.enabled && put_user(enabled,
- getwakeuptime.enabled))
- return -EFAULT;
-
- if (getwakeuptime.pending && put_user(pending,
- getwakeuptime.pending))
- return -EFAULT;
-
- if (getwakeuptime.time) {
- if (copy_to_user(getwakeuptime.time, &efi_time,
- sizeof(efi_time_t)))
- return -EFAULT;
- }
-
- return 0;
-}
-
-static long efi_runtime_set_waketime(unsigned long arg)
-{
- struct efi_setwakeuptime __user *setwakeuptime_user;
- struct efi_setwakeuptime setwakeuptime;
- efi_bool_t enabled;
- efi_status_t status;
- efi_time_t efi_time;
-
- setwakeuptime_user = (struct efi_setwakeuptime __user *)arg;
-
- if (copy_from_user(&setwakeuptime, setwakeuptime_user,
- sizeof(setwakeuptime)))
- return -EFAULT;
-
- enabled = setwakeuptime.enabled;
- if (setwakeuptime.time) {
- if (copy_from_user(&efi_time, setwakeuptime.time,
- sizeof(efi_time_t)))
- return -EFAULT;
-
- status = efi.set_wakeup_time(enabled, &efi_time);
- } else
- status = efi.set_wakeup_time(enabled, NULL);
-
- if (put_user(status, setwakeuptime.status))
- return -EFAULT;
-
- return status == EFI_SUCCESS ? 0 : -EINVAL;
-}
-
static long efi_runtime_get_nextvariablename(unsigned long arg)
{
struct efi_getnextvariablename __user *getnextvariablename_user;
@@ -505,37 +434,6 @@ static long efi_runtime_get_nextvariablename(unsigned long arg)
return rv;
}
-static long efi_runtime_get_nexthighmonocount(unsigned long arg)
-{
- struct efi_getnexthighmonotoniccount __user *getnexthighmonocount_user;
- struct efi_getnexthighmonotoniccount getnexthighmonocount;
- efi_status_t status;
- u32 count;
-
- getnexthighmonocount_user = (struct
- efi_getnexthighmonotoniccount __user *)arg;
-
- if (copy_from_user(&getnexthighmonocount,
- getnexthighmonocount_user,
- sizeof(getnexthighmonocount)))
- return -EFAULT;
-
- status = efi.get_next_high_mono_count(
- getnexthighmonocount.high_count ? &count : NULL);
-
- if (put_user(status, getnexthighmonocount.status))
- return -EFAULT;
-
- if (status != EFI_SUCCESS)
- return -EINVAL;
-
- if (getnexthighmonocount.high_count &&
- put_user(count, getnexthighmonocount.high_count))
- return -EFAULT;
-
- return 0;
-}
-
static long efi_runtime_reset_system(unsigned long arg)
{
struct efi_resetsystem __user *resetsystem_user;
@@ -697,16 +595,14 @@ static long efi_test_ioctl(struct file *file, unsigned int cmd,
return efi_runtime_set_time(arg);
case EFI_RUNTIME_GET_WAKETIME:
- return efi_runtime_get_waketime(arg);
-
case EFI_RUNTIME_SET_WAKETIME:
- return efi_runtime_set_waketime(arg);
+ return -EINVAL;
case EFI_RUNTIME_GET_NEXTVARIABLENAME:
return efi_runtime_get_nextvariablename(arg);
case EFI_RUNTIME_GET_NEXTHIGHMONOTONICCOUNT:
- return efi_runtime_get_nexthighmonocount(arg);
+ return -EINVAL;
case EFI_RUNTIME_QUERY_VARIABLEINFO:
return efi_runtime_query_variableinfo(arg);
--
2.50.0.727.gbf7dc18ff4-goog
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [RFC PATCH 3/3] efi: Remove support for pointless, unused EFI services
2025-07-14 6:08 [RFC PATCH 0/3] Remove unused EFI runtime APIs Ard Biesheuvel
2025-07-14 6:08 ` [RFC PATCH 1/3] efi-rtc: Remove wakeup functionality Ard Biesheuvel
2025-07-14 6:08 ` [RFC PATCH 2/3] efi/test: Don't bother pseudo-testing unused EFI services Ard Biesheuvel
@ 2025-07-14 6:08 ` Ard Biesheuvel
2025-07-17 0:49 ` Stefano Stabellini
2025-07-14 8:10 ` [RFC PATCH 0/3] Remove unused EFI runtime APIs Heinrich Schuchardt
3 siblings, 1 reply; 16+ messages in thread
From: Ard Biesheuvel @ 2025-07-14 6:08 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arm-kernel, Ard Biesheuvel, Heinrich Schuchardt, Feng Tang,
Alexandre Belloni, Juergen Gross, Stefano Stabellini,
Oleksandr Tyshchenko, Sunil V L, Bibo Mao, linux-rtc, linux-efi,
xen-devel, x86, linux-riscv, loongarch
From: Ard Biesheuvel <ardb@kernel.org>
The get/set wakeup time EFI runtime services are often broken, and
rarely if ever used in practice. But the GetNextHighMonoCount() EFI
runtime services really takes the cake for most pointless API in the
history of computing.
So let's stop exposing them in Linux, hopefully removing the urge some
folks seem to feel to test these APIs, and send emails around when they
don't work.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/x86/platform/efi/efi_64.c | 22 -------
drivers/firmware/efi/runtime-wrappers.c | 68 --------------------
drivers/xen/efi.c | 56 ----------------
include/linux/efi.h | 6 --
4 files changed, 152 deletions(-)
diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
index e7e8f77f77f8..0207937ab39d 100644
--- a/arch/x86/platform/efi/efi_64.c
+++ b/arch/x86/platform/efi/efi_64.c
@@ -540,19 +540,6 @@ static efi_status_t efi_thunk_set_time(efi_time_t *tm)
return EFI_UNSUPPORTED;
}
-static efi_status_t
-efi_thunk_get_wakeup_time(efi_bool_t *enabled, efi_bool_t *pending,
- efi_time_t *tm)
-{
- return EFI_UNSUPPORTED;
-}
-
-static efi_status_t
-efi_thunk_set_wakeup_time(efi_bool_t enabled, efi_time_t *tm)
-{
- return EFI_UNSUPPORTED;
-}
-
static unsigned long efi_name_size(efi_char16_t *name)
{
return ucs2_strsize(name, EFI_VAR_NAME_LEN) + 1;
@@ -681,12 +668,6 @@ efi_thunk_get_next_variable(unsigned long *name_size,
return status;
}
-static efi_status_t
-efi_thunk_get_next_high_mono_count(u32 *count)
-{
- return EFI_UNSUPPORTED;
-}
-
static void
efi_thunk_reset_system(int reset_type, efi_status_t status,
unsigned long data_size, efi_char16_t *data)
@@ -788,13 +769,10 @@ void __init efi_thunk_runtime_setup(void)
efi.get_time = efi_thunk_get_time;
efi.set_time = efi_thunk_set_time;
- efi.get_wakeup_time = efi_thunk_get_wakeup_time;
- efi.set_wakeup_time = efi_thunk_set_wakeup_time;
efi.get_variable = efi_thunk_get_variable;
efi.get_next_variable = efi_thunk_get_next_variable;
efi.set_variable = efi_thunk_set_variable;
efi.set_variable_nonblocking = efi_thunk_set_variable_nonblocking;
- efi.get_next_high_mono_count = efi_thunk_get_next_high_mono_count;
efi.reset_system = efi_thunk_reset_system;
efi.query_variable_info = efi_thunk_query_variable_info;
efi.query_variable_info_nonblocking = efi_thunk_query_variable_info_nonblocking;
diff --git a/drivers/firmware/efi/runtime-wrappers.c b/drivers/firmware/efi/runtime-wrappers.c
index 708b777857d3..2b66efb5ffef 100644
--- a/drivers/firmware/efi/runtime-wrappers.c
+++ b/drivers/firmware/efi/runtime-wrappers.c
@@ -52,17 +52,6 @@ union efi_rts_args {
efi_time_t *time;
} SET_TIME;
- struct {
- efi_bool_t *enabled;
- efi_bool_t *pending;
- efi_time_t *time;
- } GET_WAKEUP_TIME;
-
- struct {
- efi_bool_t enable;
- efi_time_t *time;
- } SET_WAKEUP_TIME;
-
struct {
efi_char16_t *name;
efi_guid_t *vendor;
@@ -92,10 +81,6 @@ union efi_rts_args {
u64 *max_variable_size;
} QUERY_VARIABLE_INFO;
- struct {
- u32 *high_count;
- } GET_NEXT_HIGH_MONO_COUNT;
-
struct {
efi_capsule_header_t **capsules;
unsigned long count;
@@ -232,17 +217,6 @@ static void __nocfi efi_call_rts(struct work_struct *work)
status = efi_call_virt(set_time,
args->SET_TIME.time);
break;
- case EFI_GET_WAKEUP_TIME:
- status = efi_call_virt(get_wakeup_time,
- args->GET_WAKEUP_TIME.enabled,
- args->GET_WAKEUP_TIME.pending,
- args->GET_WAKEUP_TIME.time);
- break;
- case EFI_SET_WAKEUP_TIME:
- status = efi_call_virt(set_wakeup_time,
- args->SET_WAKEUP_TIME.enable,
- args->SET_WAKEUP_TIME.time);
- break;
case EFI_GET_VARIABLE:
status = efi_call_virt(get_variable,
args->GET_VARIABLE.name,
@@ -272,10 +246,6 @@ static void __nocfi efi_call_rts(struct work_struct *work)
args->QUERY_VARIABLE_INFO.remaining_space,
args->QUERY_VARIABLE_INFO.max_variable_size);
break;
- case EFI_GET_NEXT_HIGH_MONO_COUNT:
- status = efi_call_virt(get_next_high_mono_count,
- args->GET_NEXT_HIGH_MONO_COUNT.high_count);
- break;
case EFI_UPDATE_CAPSULE:
status = efi_call_virt(update_capsule,
args->UPDATE_CAPSULE.capsules,
@@ -366,30 +336,6 @@ static efi_status_t virt_efi_set_time(efi_time_t *tm)
return status;
}
-static efi_status_t virt_efi_get_wakeup_time(efi_bool_t *enabled,
- efi_bool_t *pending,
- efi_time_t *tm)
-{
- efi_status_t status;
-
- if (down_interruptible(&efi_runtime_lock))
- return EFI_ABORTED;
- status = efi_queue_work(GET_WAKEUP_TIME, enabled, pending, tm);
- up(&efi_runtime_lock);
- return status;
-}
-
-static efi_status_t virt_efi_set_wakeup_time(efi_bool_t enabled, efi_time_t *tm)
-{
- efi_status_t status;
-
- if (down_interruptible(&efi_runtime_lock))
- return EFI_ABORTED;
- status = efi_queue_work(SET_WAKEUP_TIME, enabled, tm);
- up(&efi_runtime_lock);
- return status;
-}
-
static efi_status_t virt_efi_get_variable(efi_char16_t *name,
efi_guid_t *vendor,
u32 *attr,
@@ -488,17 +434,6 @@ virt_efi_query_variable_info_nb(u32 attr, u64 *storage_space,
return status;
}
-static efi_status_t virt_efi_get_next_high_mono_count(u32 *count)
-{
- efi_status_t status;
-
- if (down_interruptible(&efi_runtime_lock))
- return EFI_ABORTED;
- status = efi_queue_work(GET_NEXT_HIGH_MONO_COUNT, count);
- up(&efi_runtime_lock);
- return status;
-}
-
static void __nocfi
virt_efi_reset_system(int reset_type, efi_status_t status,
unsigned long data_size, efi_char16_t *data)
@@ -556,13 +491,10 @@ void __init efi_native_runtime_setup(void)
{
efi.get_time = virt_efi_get_time;
efi.set_time = virt_efi_set_time;
- efi.get_wakeup_time = virt_efi_get_wakeup_time;
- efi.set_wakeup_time = virt_efi_set_wakeup_time;
efi.get_variable = virt_efi_get_variable;
efi.get_next_variable = virt_efi_get_next_variable;
efi.set_variable = virt_efi_set_variable;
efi.set_variable_nonblocking = virt_efi_set_variable_nb;
- efi.get_next_high_mono_count = virt_efi_get_next_high_mono_count;
efi.reset_system = virt_efi_reset_system;
efi.query_variable_info = virt_efi_query_variable_info;
efi.query_variable_info_nonblocking = virt_efi_query_variable_info_nb;
diff --git a/drivers/xen/efi.c b/drivers/xen/efi.c
index fb321cd6415a..baccf2d90af0 100644
--- a/drivers/xen/efi.c
+++ b/drivers/xen/efi.c
@@ -76,47 +76,6 @@ static efi_status_t xen_efi_set_time(efi_time_t *tm)
return efi_data(op).status;
}
-static efi_status_t xen_efi_get_wakeup_time(efi_bool_t *enabled,
- efi_bool_t *pending,
- efi_time_t *tm)
-{
- struct xen_platform_op op = INIT_EFI_OP(get_wakeup_time);
-
- if (HYPERVISOR_platform_op(&op) < 0)
- return EFI_UNSUPPORTED;
-
- if (tm) {
- BUILD_BUG_ON(sizeof(*tm) != sizeof(efi_data(op).u.get_wakeup_time));
- memcpy(tm, &efi_data(op).u.get_wakeup_time, sizeof(*tm));
- }
-
- if (enabled)
- *enabled = !!(efi_data(op).misc & XEN_EFI_GET_WAKEUP_TIME_ENABLED);
-
- if (pending)
- *pending = !!(efi_data(op).misc & XEN_EFI_GET_WAKEUP_TIME_PENDING);
-
- return efi_data(op).status;
-}
-
-static efi_status_t xen_efi_set_wakeup_time(efi_bool_t enabled, efi_time_t *tm)
-{
- struct xen_platform_op op = INIT_EFI_OP(set_wakeup_time);
-
- BUILD_BUG_ON(sizeof(*tm) != sizeof(efi_data(op).u.set_wakeup_time));
- if (enabled)
- efi_data(op).misc = XEN_EFI_SET_WAKEUP_TIME_ENABLE;
- if (tm)
- memcpy(&efi_data(op).u.set_wakeup_time, tm, sizeof(*tm));
- else
- efi_data(op).misc |= XEN_EFI_SET_WAKEUP_TIME_ENABLE_ONLY;
-
- if (HYPERVISOR_platform_op(&op) < 0)
- return EFI_UNSUPPORTED;
-
- return efi_data(op).status;
-}
-
static efi_status_t xen_efi_get_variable(efi_char16_t *name, efi_guid_t *vendor,
u32 *attr, unsigned long *data_size,
void *data)
@@ -204,18 +163,6 @@ static efi_status_t xen_efi_query_variable_info(u32 attr, u64 *storage_space,
return efi_data(op).status;
}
-static efi_status_t xen_efi_get_next_high_mono_count(u32 *count)
-{
- struct xen_platform_op op = INIT_EFI_OP(get_next_high_monotonic_count);
-
- if (HYPERVISOR_platform_op(&op) < 0)
- return EFI_UNSUPPORTED;
-
- *count = efi_data(op).misc;
-
- return efi_data(op).status;
-}
-
static efi_status_t xen_efi_update_capsule(efi_capsule_header_t **capsules,
unsigned long count, unsigned long sg_list)
{
@@ -280,8 +227,6 @@ void __init xen_efi_runtime_setup(void)
{
efi.get_time = xen_efi_get_time;
efi.set_time = xen_efi_set_time;
- efi.get_wakeup_time = xen_efi_get_wakeup_time;
- efi.set_wakeup_time = xen_efi_set_wakeup_time;
efi.get_variable = xen_efi_get_variable;
efi.get_next_variable = xen_efi_get_next_variable;
efi.set_variable = xen_efi_set_variable;
@@ -290,7 +235,6 @@ void __init xen_efi_runtime_setup(void)
efi.query_variable_info_nonblocking = xen_efi_query_variable_info;
efi.update_capsule = xen_efi_update_capsule;
efi.query_capsule_caps = xen_efi_query_capsule_caps;
- efi.get_next_high_mono_count = xen_efi_get_next_high_mono_count;
efi.reset_system = xen_efi_reset_system;
}
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 50db7df0efab..516afdc8a49d 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -650,8 +650,6 @@ extern struct efi {
efi_get_time_t *get_time;
efi_set_time_t *set_time;
- efi_get_wakeup_time_t *get_wakeup_time;
- efi_set_wakeup_time_t *set_wakeup_time;
efi_get_variable_t *get_variable;
efi_get_next_variable_t *get_next_variable;
efi_set_variable_t *set_variable;
@@ -660,7 +658,6 @@ extern struct efi {
efi_query_variable_info_t *query_variable_info_nonblocking;
efi_update_capsule_t *update_capsule;
efi_query_capsule_caps_t *query_capsule_caps;
- efi_get_next_high_mono_count_t *get_next_high_mono_count;
efi_reset_system_t *reset_system;
struct efi_memory_map memmap;
@@ -1235,13 +1232,10 @@ enum efi_rts_ids {
EFI_NONE,
EFI_GET_TIME,
EFI_SET_TIME,
- EFI_GET_WAKEUP_TIME,
- EFI_SET_WAKEUP_TIME,
EFI_GET_VARIABLE,
EFI_GET_NEXT_VARIABLE,
EFI_SET_VARIABLE,
EFI_QUERY_VARIABLE_INFO,
- EFI_GET_NEXT_HIGH_MONO_COUNT,
EFI_RESET_SYSTEM,
EFI_UPDATE_CAPSULE,
EFI_QUERY_CAPSULE_CAPS,
--
2.50.0.727.gbf7dc18ff4-goog
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 1/3] efi-rtc: Remove wakeup functionality
2025-07-14 6:08 ` [RFC PATCH 1/3] efi-rtc: Remove wakeup functionality Ard Biesheuvel
@ 2025-07-14 6:13 ` Demi Marie Obenour
2025-07-14 6:19 ` Ard Biesheuvel
2025-07-14 7:16 ` Feng Tang
2025-08-03 1:04 ` Alexandre Belloni
2 siblings, 1 reply; 16+ messages in thread
From: Demi Marie Obenour @ 2025-07-14 6:13 UTC (permalink / raw)
To: Ard Biesheuvel, linux-kernel
Cc: linux-arm-kernel, Ard Biesheuvel, Heinrich Schuchardt, Feng Tang,
Alexandre Belloni, Juergen Gross, Stefano Stabellini,
Oleksandr Tyshchenko, Sunil V L, Bibo Mao, linux-rtc, linux-efi,
xen-devel, x86, linux-riscv, loongarch
[-- Attachment #1.1.1: Type: text/plain, Size: 558 bytes --]
On 7/14/25 02:08, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
>
> The EFI rtc driver is used by non-x86 architectures only, and exposes
> the get/set wakeup time functionality provided by the underlying
> platform. This is usually broken on most platforms, and not widely used
> to begin with [if at all], so let's just remove it.
systemd uses the underlying functionality: a timer can wake the system up.
I have no idea if that is implemented in terms of this function, though.
--
Sincerely,
Demi Marie Obenour (she/her/hers)
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 1/3] efi-rtc: Remove wakeup functionality
2025-07-14 6:13 ` Demi Marie Obenour
@ 2025-07-14 6:19 ` Ard Biesheuvel
2025-07-14 6:22 ` Demi Marie Obenour
0 siblings, 1 reply; 16+ messages in thread
From: Ard Biesheuvel @ 2025-07-14 6:19 UTC (permalink / raw)
To: Demi Marie Obenour
Cc: Ard Biesheuvel, linux-kernel, linux-arm-kernel,
Heinrich Schuchardt, Feng Tang, Alexandre Belloni, Juergen Gross,
Stefano Stabellini, Oleksandr Tyshchenko, Sunil V L, Bibo Mao,
linux-rtc, linux-efi, xen-devel, x86, linux-riscv, loongarch
On Mon, 14 Jul 2025 at 16:13, Demi Marie Obenour <demiobenour@gmail.com> wrote:
>
> On 7/14/25 02:08, Ard Biesheuvel wrote:
> > From: Ard Biesheuvel <ardb@kernel.org>
> >
> > The EFI rtc driver is used by non-x86 architectures only, and exposes
> > the get/set wakeup time functionality provided by the underlying
> > platform. This is usually broken on most platforms, and not widely used
> > to begin with [if at all], so let's just remove it.
> systemd uses the underlying functionality: a timer can wake the system up.
> I have no idea if that is implemented in terms of this function, though.
To be clear, you are referring to wake from poweroff at some date/time
in the future, right?
This change does not remove this functionality from the RTC subsystem,
it just ceases to expose it on non-x86 EFI platforms that claim to
support it.
For reference (which I should have included in the cover letter) [0],
there are arm64 server systems which always return an error when
calling this API, and most non-server arm64 systems do not implement
it to begin with.
The patch in question implements one of the workarounds that was
considered, which is to invoke GetWakeupTime() when registering the
RTC, and disable the wakeup functionality if that fails. However, that
call by itself could easily regress other platforms, where
GetWakeupTime() was simply never called before, and where calling it
may tickle other bugs.
Hence this RFC: if nobody uses this API on non-x86 EFI platforms, then
I'd rather not support it to begin with.
[0] https://lore.kernel.org/all/20250710084151.55003-1-feng.tang@linux.alibaba.com/T/#u
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 1/3] efi-rtc: Remove wakeup functionality
2025-07-14 6:19 ` Ard Biesheuvel
@ 2025-07-14 6:22 ` Demi Marie Obenour
2025-07-14 6:34 ` Ard Biesheuvel
0 siblings, 1 reply; 16+ messages in thread
From: Demi Marie Obenour @ 2025-07-14 6:22 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Ard Biesheuvel, linux-kernel, linux-arm-kernel,
Heinrich Schuchardt, Feng Tang, Alexandre Belloni, Juergen Gross,
Stefano Stabellini, Oleksandr Tyshchenko, Sunil V L, Bibo Mao,
linux-rtc, linux-efi, xen-devel, x86, linux-riscv, loongarch
[-- Attachment #1.1.1: Type: text/plain, Size: 1168 bytes --]
On 7/14/25 02:19, Ard Biesheuvel wrote:
> On Mon, 14 Jul 2025 at 16:13, Demi Marie Obenour <demiobenour@gmail.com> wrote:
>>
>> On 7/14/25 02:08, Ard Biesheuvel wrote:
>>> From: Ard Biesheuvel <ardb@kernel.org>
>>>
>>> The EFI rtc driver is used by non-x86 architectures only, and exposes
>>> the get/set wakeup time functionality provided by the underlying
>>> platform. This is usually broken on most platforms, and not widely used
>>> to begin with [if at all], so let's just remove it.
>> systemd uses the underlying functionality: a timer can wake the system up.
>> I have no idea if that is implemented in terms of this function, though.
>
> To be clear, you are referring to wake from poweroff at some date/time
> in the future, right?
Yes.
> This change does not remove this functionality from the RTC subsystem,
> it just ceases to expose it on non-x86 EFI platforms that claim to
> support it.
Do these platforms generally expose the functionality in a different way?
If not, systemd should probably document that the functionality is
non-portable if it doesn't do that already.
--
Sincerely,
Demi Marie Obenour (she/her/hers)
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 1/3] efi-rtc: Remove wakeup functionality
2025-07-14 6:22 ` Demi Marie Obenour
@ 2025-07-14 6:34 ` Ard Biesheuvel
0 siblings, 0 replies; 16+ messages in thread
From: Ard Biesheuvel @ 2025-07-14 6:34 UTC (permalink / raw)
To: Demi Marie Obenour
Cc: Ard Biesheuvel, linux-kernel, linux-arm-kernel,
Heinrich Schuchardt, Feng Tang, Alexandre Belloni, Juergen Gross,
Stefano Stabellini, Oleksandr Tyshchenko, Sunil V L, Bibo Mao,
linux-rtc, linux-efi, xen-devel, x86, linux-riscv, loongarch
On Mon, 14 Jul 2025 at 16:22, Demi Marie Obenour <demiobenour@gmail.com> wrote:
>
> On 7/14/25 02:19, Ard Biesheuvel wrote:
> > On Mon, 14 Jul 2025 at 16:13, Demi Marie Obenour <demiobenour@gmail.com> wrote:
> >>
> >> On 7/14/25 02:08, Ard Biesheuvel wrote:
> >>> From: Ard Biesheuvel <ardb@kernel.org>
> >>>
> >>> The EFI rtc driver is used by non-x86 architectures only, and exposes
> >>> the get/set wakeup time functionality provided by the underlying
> >>> platform. This is usually broken on most platforms, and not widely used
> >>> to begin with [if at all], so let's just remove it.
> >> systemd uses the underlying functionality: a timer can wake the system up.
> >> I have no idea if that is implemented in terms of this function, though.
> >
> > To be clear, you are referring to wake from poweroff at some date/time
> > in the future, right?
>
> Yes.
>
> > This change does not remove this functionality from the RTC subsystem,
> > it just ceases to expose it on non-x86 EFI platforms that claim to
> > support it.
>
> Do these platforms generally expose the functionality in a different way?
On x86, the CMOS rtc is manipulated directly (and this is officially
condoned by the EFI spec).
On non-x86, this functionality rarely works, which is really the point
of this series.
> If not, systemd should probably document that the functionality is
> non-portable if it doesn't do that already.
Not sure what you mean by non-portable. This functionality should be
exposed in a generic manner (using the RTC subsystem interfaces), but
only if it can be relied upon. On x86, the RTC subsystem will use the
rtc-cmos driver, which implements the wakeup routines in terms of port
I/O.
If removing this functionality altogether from the EFI rtc driver is a
problem, perhaps it would be better to implement an allowlist based
solution that does not attempt to access the runtime services by
default.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 1/3] efi-rtc: Remove wakeup functionality
2025-07-14 6:08 ` [RFC PATCH 1/3] efi-rtc: Remove wakeup functionality Ard Biesheuvel
2025-07-14 6:13 ` Demi Marie Obenour
@ 2025-07-14 7:16 ` Feng Tang
2025-08-03 1:04 ` Alexandre Belloni
2 siblings, 0 replies; 16+ messages in thread
From: Feng Tang @ 2025-07-14 7:16 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-kernel, linux-arm-kernel, Ard Biesheuvel,
Heinrich Schuchardt, Alexandre Belloni, Juergen Gross,
Stefano Stabellini, Oleksandr Tyshchenko, Sunil V L, Bibo Mao,
linux-rtc, linux-efi, xen-devel, x86, linux-riscv, loongarch
On Mon, Jul 14, 2025 at 08:08:45AM +0200, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
>
> The EFI rtc driver is used by non-x86 architectures only, and exposes
> the get/set wakeup time functionality provided by the underlying
> platform. This is usually broken on most platforms, and not widely used
> to begin with [if at all], so let's just remove it.
This solves the problem reported in
https://lore.kernel.org/all/20250710084151.55003-1-feng.tang@linux.alibaba.com/T/#u
Tested-by: Feng Tang <feng.tang@linux.alibaba.com>
Thanks!
- Feng
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> drivers/rtc/rtc-efi.c | 76 +-------------------
> 1 file changed, 2 insertions(+), 74 deletions(-)
>
> diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
> index fa8bf82df948..b4f44999ef0f 100644
> --- a/drivers/rtc/rtc-efi.c
> +++ b/drivers/rtc/rtc-efi.c
> @@ -112,48 +112,6 @@ 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;
> -
> - /*
> - * As of EFI v1.10, this call always returns an unsupported 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);
> -
> - /*
> - * XXX Fixme:
> - * As of EFI 0.92 with the firmware I have on my
> - * machine this call does not seem to work quite
> - * right
> - *
> - * As of v1.10, this call always returns an unsupported status
> - */
> - status = efi.set_wakeup_time((efi_bool_t)wkalrm->enabled, &eft);
> -
> - dev_warn(dev, "write status is %d\n", (int)status);
> -
> - return status == EFI_SUCCESS ? 0 : -EINVAL;
> -}
> -
> static int efi_read_time(struct device *dev, struct rtc_time *tm)
> {
> efi_status_t status;
> @@ -188,17 +146,13 @@ 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, alm;
> + efi_time_t eft;
> 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);
> - efi.get_wakeup_time(&enabled, &pending, &alm);
>
> seq_printf(seq,
> "Time\t\t: %u:%u:%u.%09u\n"
> @@ -214,26 +168,6 @@ 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, "Timezone\t: unspecified\n");
> - else
> - /* XXX fixme: convert to string? */
> - seq_printf(seq, "Timezone\t: %u\n", alm.timezone);
> - }
> -
> /*
> * now prints the capabilities
> */
> @@ -249,8 +183,6 @@ 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,
> };
>
> @@ -271,11 +203,7 @@ static int __init efi_rtc_probe(struct platform_device *dev)
> platform_set_drvdata(dev, rtc);
>
> rtc->ops = &efi_rtc_ops;
> - clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, rtc->features);
> - if (efi_rt_services_supported(EFI_RT_SUPPORTED_WAKEUP_SERVICES))
> - set_bit(RTC_FEATURE_ALARM_WAKEUP_ONLY, rtc->features);
> - else
> - clear_bit(RTC_FEATURE_ALARM, rtc->features);
> + clear_bit(RTC_FEATURE_ALARM, rtc->features);
>
> device_init_wakeup(&dev->dev, true);
>
> --
> 2.50.0.727.gbf7dc18ff4-goog
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 0/3] Remove unused EFI runtime APIs
2025-07-14 6:08 [RFC PATCH 0/3] Remove unused EFI runtime APIs Ard Biesheuvel
` (2 preceding siblings ...)
2025-07-14 6:08 ` [RFC PATCH 3/3] efi: Remove support for pointless, " Ard Biesheuvel
@ 2025-07-14 8:10 ` Heinrich Schuchardt
2025-07-15 3:29 ` Ard Biesheuvel
3 siblings, 1 reply; 16+ messages in thread
From: Heinrich Schuchardt @ 2025-07-14 8:10 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-arm-kernel, Ard Biesheuvel, Feng Tang, Alexandre Belloni,
Juergen Gross, Stefano Stabellini, Oleksandr Tyshchenko,
Sunil V L, Bibo Mao, linux-rtc, linux-efi, xen-devel, x86,
linux-riscv, loongarch, linux-kernel
On 7/14/25 08:08, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
>
> Using EFI runtime services to program the RTC to wake up the system is
> supported in theory, but rarely works in practice. Fortunately, this
> functionality is rarely [if ever] used to begin with so we can just drop
> it. (Note that the EFI rtc driver is not used by x86, which programs the
> CMOS rtc directly)
The main problem I see with firmware offering access to the RTC via UEFI
services is that two different drivers, the firmware one and the Linux
one might be trying to access the same busses or registers which might
lead to unexpected results.
Recently there was a discussion in the RISC-V technical group for the
server platform specification where the same issue was discussed
concerning SetTime().
As a UEFI firmware should not care which operating system is booted, it
should be up to the OS to disable EFI access to the RTC if it has native
access.
Could we disable all the EFI services for the RTC in Linux dynamically
when an RTC driver is successfully probed?
Best regards
Heinrich
>
> The same applies to GetNextHighMonoCount(), which, if implemented,
> usually relies on SetVariable() under the hood *, which is often not
> supported at runtime by non-x86 platforms. But it has no known users
> either so let's drop support for it as well.
>
> This means we need to drop the slightly pointless tests for it too.
>
> * EDK2 based EFI implementations usually have a MTC variable carrying
> the monotonic counter variable, which is therefore not truly
> monotonic, given that SetVariable() will happily overwrite it.
>
> Cc: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> Cc: Feng Tang <feng.tang@linux.alibaba.com>
> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> Cc: Sunil V L <sunilvl@ventanamicro.com>
> Cc: Bibo Mao <maobibo@loongson.cn>
> Cc: linux-rtc@vger.kernel.org
> Cc: linux-efi@vger.kernel.org
> Cc: xen-devel@lists.xenproject.org
> Cc: x86@kernel.org
> Cc: linux-riscv@lists.infradead.org
> Cc: loongarch@lists.linux.dev
>
> Ard Biesheuvel (3):
> efi-rtc: Remove wakeup functionality
> efi/test: Don't bother pseudo-testing unused EFI services
> efi: Remove support for pointless, unused EFI services
>
> arch/x86/platform/efi/efi_64.c | 22 ----
> drivers/firmware/efi/runtime-wrappers.c | 68 ------------
> drivers/firmware/efi/test/efi_test.c | 108 +-------------------
> drivers/rtc/rtc-efi.c | 76 +-------------
> drivers/xen/efi.c | 56 ----------
> include/linux/efi.h | 6 --
> 6 files changed, 4 insertions(+), 332 deletions(-)
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 0/3] Remove unused EFI runtime APIs
2025-07-14 8:10 ` [RFC PATCH 0/3] Remove unused EFI runtime APIs Heinrich Schuchardt
@ 2025-07-15 3:29 ` Ard Biesheuvel
2025-07-15 14:58 ` Sunil V L
0 siblings, 1 reply; 16+ messages in thread
From: Ard Biesheuvel @ 2025-07-15 3:29 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Ard Biesheuvel, linux-arm-kernel, Feng Tang, Alexandre Belloni,
Juergen Gross, Stefano Stabellini, Oleksandr Tyshchenko,
Sunil V L, Bibo Mao, linux-rtc, linux-efi, xen-devel, x86,
linux-riscv, loongarch, linux-kernel
On Mon, 14 Jul 2025 at 18:11, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> On 7/14/25 08:08, Ard Biesheuvel wrote:
> > From: Ard Biesheuvel <ardb@kernel.org>
> >
> > Using EFI runtime services to program the RTC to wake up the system is
> > supported in theory, but rarely works in practice. Fortunately, this
> > functionality is rarely [if ever] used to begin with so we can just drop
> > it. (Note that the EFI rtc driver is not used by x86, which programs the
> > CMOS rtc directly)
>
> The main problem I see with firmware offering access to the RTC via UEFI
> services is that two different drivers, the firmware one and the Linux
> one might be trying to access the same busses or registers which might
> lead to unexpected results.
>
> Recently there was a discussion in the RISC-V technical group for the
> server platform specification where the same issue was discussed
> concerning SetTime().
>
> As a UEFI firmware should not care which operating system is booted, it
> should be up to the OS to disable EFI access to the RTC if it has native
> access.
>
> Could we disable all the EFI services for the RTC in Linux dynamically
> when an RTC driver is successfully probed?
>
I don't think this would be the right way to do it.
It also depends on whether ACPI or DT is being used to describe the
platform to the OS.
ACPI does not support describing the RTC device, so it should provide
the EFI services.
DT can describe the RTC device directly, so I think it is acceptable
for such firmware to mark all RTC routines unsupported in the RT_PROP
table, and just expose the RTC device directly.
The OS shouldn't have to reason about these things: it is up to the
platform to describe itself unambiguously.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 0/3] Remove unused EFI runtime APIs
2025-07-15 3:29 ` Ard Biesheuvel
@ 2025-07-15 14:58 ` Sunil V L
2025-07-16 3:52 ` Ard Biesheuvel
0 siblings, 1 reply; 16+ messages in thread
From: Sunil V L @ 2025-07-15 14:58 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Heinrich Schuchardt, Ard Biesheuvel, linux-arm-kernel, Feng Tang,
Alexandre Belloni, Juergen Gross, Stefano Stabellini,
Oleksandr Tyshchenko, Bibo Mao, linux-rtc, linux-efi, xen-devel,
x86, linux-riscv, loongarch, linux-kernel
On Tue, Jul 15, 2025 at 01:29:15PM +1000, Ard Biesheuvel wrote:
> On Mon, 14 Jul 2025 at 18:11, Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
> >
> > On 7/14/25 08:08, Ard Biesheuvel wrote:
> > > From: Ard Biesheuvel <ardb@kernel.org>
> > >
> > > Using EFI runtime services to program the RTC to wake up the system is
> > > supported in theory, but rarely works in practice. Fortunately, this
> > > functionality is rarely [if ever] used to begin with so we can just drop
> > > it. (Note that the EFI rtc driver is not used by x86, which programs the
> > > CMOS rtc directly)
> >
> > The main problem I see with firmware offering access to the RTC via UEFI
> > services is that two different drivers, the firmware one and the Linux
> > one might be trying to access the same busses or registers which might
> > lead to unexpected results.
> >
> > Recently there was a discussion in the RISC-V technical group for the
> > server platform specification where the same issue was discussed
> > concerning SetTime().
> >
> > As a UEFI firmware should not care which operating system is booted, it
> > should be up to the OS to disable EFI access to the RTC if it has native
> > access.
> >
> > Could we disable all the EFI services for the RTC in Linux dynamically
> > when an RTC driver is successfully probed?
> >
>
> I don't think this would be the right way to do it.
>
> It also depends on whether ACPI or DT is being used to describe the
> platform to the OS.
>
> ACPI does not support describing the RTC device, so it should provide
> the EFI services.
>
Hi Ard,
IIUC, TAD is defined for this purpose, right?
https://uefi.org/specs/ACPI/6.6/09_ACPI_Defined_Devices_and_Device_Specific_Objects.html#time-and-alarm-device
> DT can describe the RTC device directly, so I think it is acceptable
> for such firmware to mark all RTC routines unsupported in the RT_PROP
> table, and just expose the RTC device directly.
>
> The OS shouldn't have to reason about these things: it is up to the
> platform to describe itself unambiguously.
I agree. But I think even with ACPI, EFI GetTime/SetTime can return
unsupported if there is a TAD exposed with proper _GRT/_SRT and _GCP.
Thanks,
Sunil
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 0/3] Remove unused EFI runtime APIs
2025-07-15 14:58 ` Sunil V L
@ 2025-07-16 3:52 ` Ard Biesheuvel
0 siblings, 0 replies; 16+ messages in thread
From: Ard Biesheuvel @ 2025-07-16 3:52 UTC (permalink / raw)
To: Sunil V L
Cc: Heinrich Schuchardt, Ard Biesheuvel, linux-arm-kernel, Feng Tang,
Alexandre Belloni, Juergen Gross, Stefano Stabellini,
Oleksandr Tyshchenko, Bibo Mao, linux-rtc, linux-efi, xen-devel,
x86, linux-riscv, loongarch, linux-kernel
On Wed, 16 Jul 2025 at 00:58, Sunil V L <sunilvl@ventanamicro.com> wrote:
>
> On Tue, Jul 15, 2025 at 01:29:15PM +1000, Ard Biesheuvel wrote:
> > On Mon, 14 Jul 2025 at 18:11, Heinrich Schuchardt
> > <heinrich.schuchardt@canonical.com> wrote:
> > >
> > > On 7/14/25 08:08, Ard Biesheuvel wrote:
> > > > From: Ard Biesheuvel <ardb@kernel.org>
> > > >
> > > > Using EFI runtime services to program the RTC to wake up the system is
> > > > supported in theory, but rarely works in practice. Fortunately, this
> > > > functionality is rarely [if ever] used to begin with so we can just drop
> > > > it. (Note that the EFI rtc driver is not used by x86, which programs the
> > > > CMOS rtc directly)
> > >
> > > The main problem I see with firmware offering access to the RTC via UEFI
> > > services is that two different drivers, the firmware one and the Linux
> > > one might be trying to access the same busses or registers which might
> > > lead to unexpected results.
> > >
> > > Recently there was a discussion in the RISC-V technical group for the
> > > server platform specification where the same issue was discussed
> > > concerning SetTime().
> > >
> > > As a UEFI firmware should not care which operating system is booted, it
> > > should be up to the OS to disable EFI access to the RTC if it has native
> > > access.
> > >
> > > Could we disable all the EFI services for the RTC in Linux dynamically
> > > when an RTC driver is successfully probed?
> > >
> >
> > I don't think this would be the right way to do it.
> >
> > It also depends on whether ACPI or DT is being used to describe the
> > platform to the OS.
> >
> > ACPI does not support describing the RTC device, so it should provide
> > the EFI services.
> >
> Hi Ard,
> IIUC, TAD is defined for this purpose, right?
> https://uefi.org/specs/ACPI/6.6/09_ACPI_Defined_Devices_and_Device_Specific_Objects.html#time-and-alarm-device
>
> > DT can describe the RTC device directly, so I think it is acceptable
> > for such firmware to mark all RTC routines unsupported in the RT_PROP
> > table, and just expose the RTC device directly.
> >
> > The OS shouldn't have to reason about these things: it is up to the
> > platform to describe itself unambiguously.
>
> I agree. But I think even with ACPI, EFI GetTime/SetTime can return
> unsupported if there is a TAD exposed with proper _GRT/_SRT and _GCP.
>
Thanks for the pointer. This device did not exist yet when I last
looked at this stuff.
So it seems like TAD is a suitable way for exposing a RTC to the OS
without the need for a hardware specific driver. However, the existing
Linux driver does not appear to support get/set time, and is not
hooked up to the RTC subsystem [yet].
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 3/3] efi: Remove support for pointless, unused EFI services
2025-07-14 6:08 ` [RFC PATCH 3/3] efi: Remove support for pointless, " Ard Biesheuvel
@ 2025-07-17 0:49 ` Stefano Stabellini
0 siblings, 0 replies; 16+ messages in thread
From: Stefano Stabellini @ 2025-07-17 0:49 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-kernel, linux-arm-kernel, Ard Biesheuvel,
Heinrich Schuchardt, Feng Tang, Alexandre Belloni, Juergen Gross,
Stefano Stabellini, Oleksandr Tyshchenko, Sunil V L, Bibo Mao,
linux-rtc, linux-efi, xen-devel, x86, linux-riscv, loongarch
On Mon, 14 Jul 2025, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
>
> The get/set wakeup time EFI runtime services are often broken, and
> rarely if ever used in practice. But the GetNextHighMonoCount() EFI
> runtime services really takes the cake for most pointless API in the
> history of computing.
>
> So let's stop exposing them in Linux, hopefully removing the urge some
> folks seem to feel to test these APIs, and send emails around when they
> don't work.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
For drivers/xen/efi.c:
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
> arch/x86/platform/efi/efi_64.c | 22 -------
> drivers/firmware/efi/runtime-wrappers.c | 68 --------------------
> drivers/xen/efi.c | 56 ----------------
> include/linux/efi.h | 6 --
> 4 files changed, 152 deletions(-)
>
> diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
> index e7e8f77f77f8..0207937ab39d 100644
> --- a/arch/x86/platform/efi/efi_64.c
> +++ b/arch/x86/platform/efi/efi_64.c
> @@ -540,19 +540,6 @@ static efi_status_t efi_thunk_set_time(efi_time_t *tm)
> return EFI_UNSUPPORTED;
> }
>
> -static efi_status_t
> -efi_thunk_get_wakeup_time(efi_bool_t *enabled, efi_bool_t *pending,
> - efi_time_t *tm)
> -{
> - return EFI_UNSUPPORTED;
> -}
> -
> -static efi_status_t
> -efi_thunk_set_wakeup_time(efi_bool_t enabled, efi_time_t *tm)
> -{
> - return EFI_UNSUPPORTED;
> -}
> -
> static unsigned long efi_name_size(efi_char16_t *name)
> {
> return ucs2_strsize(name, EFI_VAR_NAME_LEN) + 1;
> @@ -681,12 +668,6 @@ efi_thunk_get_next_variable(unsigned long *name_size,
> return status;
> }
>
> -static efi_status_t
> -efi_thunk_get_next_high_mono_count(u32 *count)
> -{
> - return EFI_UNSUPPORTED;
> -}
> -
> static void
> efi_thunk_reset_system(int reset_type, efi_status_t status,
> unsigned long data_size, efi_char16_t *data)
> @@ -788,13 +769,10 @@ void __init efi_thunk_runtime_setup(void)
>
> efi.get_time = efi_thunk_get_time;
> efi.set_time = efi_thunk_set_time;
> - efi.get_wakeup_time = efi_thunk_get_wakeup_time;
> - efi.set_wakeup_time = efi_thunk_set_wakeup_time;
> efi.get_variable = efi_thunk_get_variable;
> efi.get_next_variable = efi_thunk_get_next_variable;
> efi.set_variable = efi_thunk_set_variable;
> efi.set_variable_nonblocking = efi_thunk_set_variable_nonblocking;
> - efi.get_next_high_mono_count = efi_thunk_get_next_high_mono_count;
> efi.reset_system = efi_thunk_reset_system;
> efi.query_variable_info = efi_thunk_query_variable_info;
> efi.query_variable_info_nonblocking = efi_thunk_query_variable_info_nonblocking;
> diff --git a/drivers/firmware/efi/runtime-wrappers.c b/drivers/firmware/efi/runtime-wrappers.c
> index 708b777857d3..2b66efb5ffef 100644
> --- a/drivers/firmware/efi/runtime-wrappers.c
> +++ b/drivers/firmware/efi/runtime-wrappers.c
> @@ -52,17 +52,6 @@ union efi_rts_args {
> efi_time_t *time;
> } SET_TIME;
>
> - struct {
> - efi_bool_t *enabled;
> - efi_bool_t *pending;
> - efi_time_t *time;
> - } GET_WAKEUP_TIME;
> -
> - struct {
> - efi_bool_t enable;
> - efi_time_t *time;
> - } SET_WAKEUP_TIME;
> -
> struct {
> efi_char16_t *name;
> efi_guid_t *vendor;
> @@ -92,10 +81,6 @@ union efi_rts_args {
> u64 *max_variable_size;
> } QUERY_VARIABLE_INFO;
>
> - struct {
> - u32 *high_count;
> - } GET_NEXT_HIGH_MONO_COUNT;
> -
> struct {
> efi_capsule_header_t **capsules;
> unsigned long count;
> @@ -232,17 +217,6 @@ static void __nocfi efi_call_rts(struct work_struct *work)
> status = efi_call_virt(set_time,
> args->SET_TIME.time);
> break;
> - case EFI_GET_WAKEUP_TIME:
> - status = efi_call_virt(get_wakeup_time,
> - args->GET_WAKEUP_TIME.enabled,
> - args->GET_WAKEUP_TIME.pending,
> - args->GET_WAKEUP_TIME.time);
> - break;
> - case EFI_SET_WAKEUP_TIME:
> - status = efi_call_virt(set_wakeup_time,
> - args->SET_WAKEUP_TIME.enable,
> - args->SET_WAKEUP_TIME.time);
> - break;
> case EFI_GET_VARIABLE:
> status = efi_call_virt(get_variable,
> args->GET_VARIABLE.name,
> @@ -272,10 +246,6 @@ static void __nocfi efi_call_rts(struct work_struct *work)
> args->QUERY_VARIABLE_INFO.remaining_space,
> args->QUERY_VARIABLE_INFO.max_variable_size);
> break;
> - case EFI_GET_NEXT_HIGH_MONO_COUNT:
> - status = efi_call_virt(get_next_high_mono_count,
> - args->GET_NEXT_HIGH_MONO_COUNT.high_count);
> - break;
> case EFI_UPDATE_CAPSULE:
> status = efi_call_virt(update_capsule,
> args->UPDATE_CAPSULE.capsules,
> @@ -366,30 +336,6 @@ static efi_status_t virt_efi_set_time(efi_time_t *tm)
> return status;
> }
>
> -static efi_status_t virt_efi_get_wakeup_time(efi_bool_t *enabled,
> - efi_bool_t *pending,
> - efi_time_t *tm)
> -{
> - efi_status_t status;
> -
> - if (down_interruptible(&efi_runtime_lock))
> - return EFI_ABORTED;
> - status = efi_queue_work(GET_WAKEUP_TIME, enabled, pending, tm);
> - up(&efi_runtime_lock);
> - return status;
> -}
> -
> -static efi_status_t virt_efi_set_wakeup_time(efi_bool_t enabled, efi_time_t *tm)
> -{
> - efi_status_t status;
> -
> - if (down_interruptible(&efi_runtime_lock))
> - return EFI_ABORTED;
> - status = efi_queue_work(SET_WAKEUP_TIME, enabled, tm);
> - up(&efi_runtime_lock);
> - return status;
> -}
> -
> static efi_status_t virt_efi_get_variable(efi_char16_t *name,
> efi_guid_t *vendor,
> u32 *attr,
> @@ -488,17 +434,6 @@ virt_efi_query_variable_info_nb(u32 attr, u64 *storage_space,
> return status;
> }
>
> -static efi_status_t virt_efi_get_next_high_mono_count(u32 *count)
> -{
> - efi_status_t status;
> -
> - if (down_interruptible(&efi_runtime_lock))
> - return EFI_ABORTED;
> - status = efi_queue_work(GET_NEXT_HIGH_MONO_COUNT, count);
> - up(&efi_runtime_lock);
> - return status;
> -}
> -
> static void __nocfi
> virt_efi_reset_system(int reset_type, efi_status_t status,
> unsigned long data_size, efi_char16_t *data)
> @@ -556,13 +491,10 @@ void __init efi_native_runtime_setup(void)
> {
> efi.get_time = virt_efi_get_time;
> efi.set_time = virt_efi_set_time;
> - efi.get_wakeup_time = virt_efi_get_wakeup_time;
> - efi.set_wakeup_time = virt_efi_set_wakeup_time;
> efi.get_variable = virt_efi_get_variable;
> efi.get_next_variable = virt_efi_get_next_variable;
> efi.set_variable = virt_efi_set_variable;
> efi.set_variable_nonblocking = virt_efi_set_variable_nb;
> - efi.get_next_high_mono_count = virt_efi_get_next_high_mono_count;
> efi.reset_system = virt_efi_reset_system;
> efi.query_variable_info = virt_efi_query_variable_info;
> efi.query_variable_info_nonblocking = virt_efi_query_variable_info_nb;
> diff --git a/drivers/xen/efi.c b/drivers/xen/efi.c
> index fb321cd6415a..baccf2d90af0 100644
> --- a/drivers/xen/efi.c
> +++ b/drivers/xen/efi.c
> @@ -76,47 +76,6 @@ static efi_status_t xen_efi_set_time(efi_time_t *tm)
> return efi_data(op).status;
> }
>
> -static efi_status_t xen_efi_get_wakeup_time(efi_bool_t *enabled,
> - efi_bool_t *pending,
> - efi_time_t *tm)
> -{
> - struct xen_platform_op op = INIT_EFI_OP(get_wakeup_time);
> -
> - if (HYPERVISOR_platform_op(&op) < 0)
> - return EFI_UNSUPPORTED;
> -
> - if (tm) {
> - BUILD_BUG_ON(sizeof(*tm) != sizeof(efi_data(op).u.get_wakeup_time));
> - memcpy(tm, &efi_data(op).u.get_wakeup_time, sizeof(*tm));
> - }
> -
> - if (enabled)
> - *enabled = !!(efi_data(op).misc & XEN_EFI_GET_WAKEUP_TIME_ENABLED);
> -
> - if (pending)
> - *pending = !!(efi_data(op).misc & XEN_EFI_GET_WAKEUP_TIME_PENDING);
> -
> - return efi_data(op).status;
> -}
> -
> -static efi_status_t xen_efi_set_wakeup_time(efi_bool_t enabled, efi_time_t *tm)
> -{
> - struct xen_platform_op op = INIT_EFI_OP(set_wakeup_time);
> -
> - BUILD_BUG_ON(sizeof(*tm) != sizeof(efi_data(op).u.set_wakeup_time));
> - if (enabled)
> - efi_data(op).misc = XEN_EFI_SET_WAKEUP_TIME_ENABLE;
> - if (tm)
> - memcpy(&efi_data(op).u.set_wakeup_time, tm, sizeof(*tm));
> - else
> - efi_data(op).misc |= XEN_EFI_SET_WAKEUP_TIME_ENABLE_ONLY;
> -
> - if (HYPERVISOR_platform_op(&op) < 0)
> - return EFI_UNSUPPORTED;
> -
> - return efi_data(op).status;
> -}
> -
> static efi_status_t xen_efi_get_variable(efi_char16_t *name, efi_guid_t *vendor,
> u32 *attr, unsigned long *data_size,
> void *data)
> @@ -204,18 +163,6 @@ static efi_status_t xen_efi_query_variable_info(u32 attr, u64 *storage_space,
> return efi_data(op).status;
> }
>
> -static efi_status_t xen_efi_get_next_high_mono_count(u32 *count)
> -{
> - struct xen_platform_op op = INIT_EFI_OP(get_next_high_monotonic_count);
> -
> - if (HYPERVISOR_platform_op(&op) < 0)
> - return EFI_UNSUPPORTED;
> -
> - *count = efi_data(op).misc;
> -
> - return efi_data(op).status;
> -}
> -
> static efi_status_t xen_efi_update_capsule(efi_capsule_header_t **capsules,
> unsigned long count, unsigned long sg_list)
> {
> @@ -280,8 +227,6 @@ void __init xen_efi_runtime_setup(void)
> {
> efi.get_time = xen_efi_get_time;
> efi.set_time = xen_efi_set_time;
> - efi.get_wakeup_time = xen_efi_get_wakeup_time;
> - efi.set_wakeup_time = xen_efi_set_wakeup_time;
> efi.get_variable = xen_efi_get_variable;
> efi.get_next_variable = xen_efi_get_next_variable;
> efi.set_variable = xen_efi_set_variable;
> @@ -290,7 +235,6 @@ void __init xen_efi_runtime_setup(void)
> efi.query_variable_info_nonblocking = xen_efi_query_variable_info;
> efi.update_capsule = xen_efi_update_capsule;
> efi.query_capsule_caps = xen_efi_query_capsule_caps;
> - efi.get_next_high_mono_count = xen_efi_get_next_high_mono_count;
> efi.reset_system = xen_efi_reset_system;
> }
>
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index 50db7df0efab..516afdc8a49d 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -650,8 +650,6 @@ extern struct efi {
>
> efi_get_time_t *get_time;
> efi_set_time_t *set_time;
> - efi_get_wakeup_time_t *get_wakeup_time;
> - efi_set_wakeup_time_t *set_wakeup_time;
> efi_get_variable_t *get_variable;
> efi_get_next_variable_t *get_next_variable;
> efi_set_variable_t *set_variable;
> @@ -660,7 +658,6 @@ extern struct efi {
> efi_query_variable_info_t *query_variable_info_nonblocking;
> efi_update_capsule_t *update_capsule;
> efi_query_capsule_caps_t *query_capsule_caps;
> - efi_get_next_high_mono_count_t *get_next_high_mono_count;
> efi_reset_system_t *reset_system;
>
> struct efi_memory_map memmap;
> @@ -1235,13 +1232,10 @@ enum efi_rts_ids {
> EFI_NONE,
> EFI_GET_TIME,
> EFI_SET_TIME,
> - EFI_GET_WAKEUP_TIME,
> - EFI_SET_WAKEUP_TIME,
> EFI_GET_VARIABLE,
> EFI_GET_NEXT_VARIABLE,
> EFI_SET_VARIABLE,
> EFI_QUERY_VARIABLE_INFO,
> - EFI_GET_NEXT_HIGH_MONO_COUNT,
> EFI_RESET_SYSTEM,
> EFI_UPDATE_CAPSULE,
> EFI_QUERY_CAPSULE_CAPS,
> --
> 2.50.0.727.gbf7dc18ff4-goog
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 1/3] efi-rtc: Remove wakeup functionality
2025-07-14 6:08 ` [RFC PATCH 1/3] efi-rtc: Remove wakeup functionality Ard Biesheuvel
2025-07-14 6:13 ` Demi Marie Obenour
2025-07-14 7:16 ` Feng Tang
@ 2025-08-03 1:04 ` Alexandre Belloni
2025-08-09 23:02 ` Ard Biesheuvel
2 siblings, 1 reply; 16+ messages in thread
From: Alexandre Belloni @ 2025-08-03 1:04 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-kernel, linux-arm-kernel, Ard Biesheuvel,
Heinrich Schuchardt, Feng Tang, Juergen Gross, Stefano Stabellini,
Oleksandr Tyshchenko, Sunil V L, Bibo Mao, linux-rtc, linux-efi,
xen-devel, x86, linux-riscv, loongarch
Hello,
Apart from the topic that should be "rtc: efi:...", I'm ready to apply
this patch.
On 14/07/2025 08:08:45+0200, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
>
> The EFI rtc driver is used by non-x86 architectures only, and exposes
> the get/set wakeup time functionality provided by the underlying
> platform. This is usually broken on most platforms, and not widely used
> to begin with [if at all], so let's just remove it.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> drivers/rtc/rtc-efi.c | 76 +-------------------
> 1 file changed, 2 insertions(+), 74 deletions(-)
>
> diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
> index fa8bf82df948..b4f44999ef0f 100644
> --- a/drivers/rtc/rtc-efi.c
> +++ b/drivers/rtc/rtc-efi.c
> @@ -112,48 +112,6 @@ 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;
> -
> - /*
> - * As of EFI v1.10, this call always returns an unsupported 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);
> -
> - /*
> - * XXX Fixme:
> - * As of EFI 0.92 with the firmware I have on my
> - * machine this call does not seem to work quite
> - * right
> - *
> - * As of v1.10, this call always returns an unsupported status
> - */
> - status = efi.set_wakeup_time((efi_bool_t)wkalrm->enabled, &eft);
> -
> - dev_warn(dev, "write status is %d\n", (int)status);
> -
> - return status == EFI_SUCCESS ? 0 : -EINVAL;
> -}
> -
> static int efi_read_time(struct device *dev, struct rtc_time *tm)
> {
> efi_status_t status;
> @@ -188,17 +146,13 @@ 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, alm;
> + efi_time_t eft;
> 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);
> - efi.get_wakeup_time(&enabled, &pending, &alm);
>
> seq_printf(seq,
> "Time\t\t: %u:%u:%u.%09u\n"
> @@ -214,26 +168,6 @@ 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, "Timezone\t: unspecified\n");
> - else
> - /* XXX fixme: convert to string? */
> - seq_printf(seq, "Timezone\t: %u\n", alm.timezone);
> - }
> -
> /*
> * now prints the capabilities
> */
> @@ -249,8 +183,6 @@ 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,
> };
>
> @@ -271,11 +203,7 @@ static int __init efi_rtc_probe(struct platform_device *dev)
> platform_set_drvdata(dev, rtc);
>
> rtc->ops = &efi_rtc_ops;
> - clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, rtc->features);
> - if (efi_rt_services_supported(EFI_RT_SUPPORTED_WAKEUP_SERVICES))
> - set_bit(RTC_FEATURE_ALARM_WAKEUP_ONLY, rtc->features);
> - else
> - clear_bit(RTC_FEATURE_ALARM, rtc->features);
> + clear_bit(RTC_FEATURE_ALARM, rtc->features);
>
> device_init_wakeup(&dev->dev, true);
>
> --
> 2.50.0.727.gbf7dc18ff4-goog
>
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 1/3] efi-rtc: Remove wakeup functionality
2025-08-03 1:04 ` Alexandre Belloni
@ 2025-08-09 23:02 ` Ard Biesheuvel
0 siblings, 0 replies; 16+ messages in thread
From: Ard Biesheuvel @ 2025-08-09 23:02 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Ard Biesheuvel, linux-kernel, linux-arm-kernel,
Heinrich Schuchardt, Feng Tang, Juergen Gross, Stefano Stabellini,
Oleksandr Tyshchenko, Sunil V L, Bibo Mao, linux-rtc, linux-efi,
xen-devel, x86, linux-riscv, loongarch
On Sun, 3 Aug 2025 at 11:05, Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
>
> Hello,
>
> Apart from the topic that should be "rtc: efi:...", I'm ready to apply
> this patch.
>
Thanks, please go ahead.
> On 14/07/2025 08:08:45+0200, Ard Biesheuvel wrote:
> > From: Ard Biesheuvel <ardb@kernel.org>
> >
> > The EFI rtc driver is used by non-x86 architectures only, and exposes
> > the get/set wakeup time functionality provided by the underlying
> > platform. This is usually broken on most platforms, and not widely used
> > to begin with [if at all], so let's just remove it.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> > drivers/rtc/rtc-efi.c | 76 +-------------------
> > 1 file changed, 2 insertions(+), 74 deletions(-)
> >
> > diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
> > index fa8bf82df948..b4f44999ef0f 100644
> > --- a/drivers/rtc/rtc-efi.c
> > +++ b/drivers/rtc/rtc-efi.c
> > @@ -112,48 +112,6 @@ 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;
> > -
> > - /*
> > - * As of EFI v1.10, this call always returns an unsupported 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);
> > -
> > - /*
> > - * XXX Fixme:
> > - * As of EFI 0.92 with the firmware I have on my
> > - * machine this call does not seem to work quite
> > - * right
> > - *
> > - * As of v1.10, this call always returns an unsupported status
> > - */
> > - status = efi.set_wakeup_time((efi_bool_t)wkalrm->enabled, &eft);
> > -
> > - dev_warn(dev, "write status is %d\n", (int)status);
> > -
> > - return status == EFI_SUCCESS ? 0 : -EINVAL;
> > -}
> > -
> > static int efi_read_time(struct device *dev, struct rtc_time *tm)
> > {
> > efi_status_t status;
> > @@ -188,17 +146,13 @@ 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, alm;
> > + efi_time_t eft;
> > 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);
> > - efi.get_wakeup_time(&enabled, &pending, &alm);
> >
> > seq_printf(seq,
> > "Time\t\t: %u:%u:%u.%09u\n"
> > @@ -214,26 +168,6 @@ 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, "Timezone\t: unspecified\n");
> > - else
> > - /* XXX fixme: convert to string? */
> > - seq_printf(seq, "Timezone\t: %u\n", alm.timezone);
> > - }
> > -
> > /*
> > * now prints the capabilities
> > */
> > @@ -249,8 +183,6 @@ 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,
> > };
> >
> > @@ -271,11 +203,7 @@ static int __init efi_rtc_probe(struct platform_device *dev)
> > platform_set_drvdata(dev, rtc);
> >
> > rtc->ops = &efi_rtc_ops;
> > - clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, rtc->features);
> > - if (efi_rt_services_supported(EFI_RT_SUPPORTED_WAKEUP_SERVICES))
> > - set_bit(RTC_FEATURE_ALARM_WAKEUP_ONLY, rtc->features);
> > - else
> > - clear_bit(RTC_FEATURE_ALARM, rtc->features);
> > + clear_bit(RTC_FEATURE_ALARM, rtc->features);
> >
> > device_init_wakeup(&dev->dev, true);
> >
> > --
> > 2.50.0.727.gbf7dc18ff4-goog
> >
>
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
>
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2025-08-09 23:05 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-14 6:08 [RFC PATCH 0/3] Remove unused EFI runtime APIs Ard Biesheuvel
2025-07-14 6:08 ` [RFC PATCH 1/3] efi-rtc: Remove wakeup functionality Ard Biesheuvel
2025-07-14 6:13 ` Demi Marie Obenour
2025-07-14 6:19 ` Ard Biesheuvel
2025-07-14 6:22 ` Demi Marie Obenour
2025-07-14 6:34 ` Ard Biesheuvel
2025-07-14 7:16 ` Feng Tang
2025-08-03 1:04 ` Alexandre Belloni
2025-08-09 23:02 ` Ard Biesheuvel
2025-07-14 6:08 ` [RFC PATCH 2/3] efi/test: Don't bother pseudo-testing unused EFI services Ard Biesheuvel
2025-07-14 6:08 ` [RFC PATCH 3/3] efi: Remove support for pointless, " Ard Biesheuvel
2025-07-17 0:49 ` Stefano Stabellini
2025-07-14 8:10 ` [RFC PATCH 0/3] Remove unused EFI runtime APIs Heinrich Schuchardt
2025-07-15 3:29 ` Ard Biesheuvel
2025-07-15 14:58 ` Sunil V L
2025-07-16 3:52 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).