linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] rtc: efi: Add runtime check for the wakeup service capability
@ 2025-07-10  8:41 Feng Tang
  2025-07-11  1:06 ` Ard Biesheuvel
  0 siblings, 1 reply; 6+ messages in thread
From: Feng Tang @ 2025-07-10  8:41 UTC (permalink / raw)
  To: Alexandre Belloni, Ard Biesheuvel
  Cc: linux-rtc, linux-efi, linux-kernel, Feng Tang

The kernel selftest of rtc reported a error on an ARM server which
use rtc-efi device:

	RUN           rtc.alarm_alm_set ...
	rtctest.c:262:alarm_alm_set:Alarm time now set to 17:31:36.
	rtctest.c:267:alarm_alm_set:Expected -1 (-1) != rc (-1)
	alarm_alm_set: Test terminated by assertion
		 FAIL  rtc.alarm_alm_set
	not ok 5 rtc.alarm_alm_set

The root cause is, the underlying EFI firmware doesn't support wakeup
service (get/set alarm), while it doesn't have the EFI RT_PROP table
either. As Ard Biesheuvel clarified [1], this breaks the UEFI spec,
which requires EFI firmware to provide a 'RT_PROP' table if it doesn't
support all runtime services (Section 4.6.2, UEFI spec 2.10).

This issue was also reproduced on ARM server from another vendor, which
doesn't have RT_PROP table either. This means, in real world, there are
quite some platforms having this issue, that it doesn't support wakeup
service while not providing a correct RT_PROP table, which makes it
wrongly claimed to support it.

Add a runtime check for the wakeup service to detect and correct this
kind of cases.

[1]. https://lore.kernel.org/lkml/CAMj1kXEkzXsjm0dPhzxB+KdtzqADd4NmafKmw2rKw7mAPBrgdA@mail.gmail.com/

Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com>
---
 drivers/rtc/rtc-efi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
index fa8bf82df948..8d1b9bde6f66 100644
--- a/drivers/rtc/rtc-efi.c
+++ b/drivers/rtc/rtc-efi.c
@@ -259,6 +259,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)
@@ -272,7 +273,8 @@ static int __init efi_rtc_probe(struct platform_device *dev)
 
 	rtc->ops = &efi_rtc_ops;
 	clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, rtc->features);
-	if (efi_rt_services_supported(EFI_RT_SUPPORTED_WAKEUP_SERVICES))
+	if (efi_rt_services_supported(EFI_RT_SUPPORTED_WAKEUP_SERVICES) &&
+		efi.get_wakeup_time(&enabled, &pending, &eft) == EFI_SUCCESS)
 		set_bit(RTC_FEATURE_ALARM_WAKEUP_ONLY, rtc->features);
 	else
 		clear_bit(RTC_FEATURE_ALARM, rtc->features);
-- 
2.39.5 (Apple Git-154)


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] rtc: efi: Add runtime check for the wakeup service capability
  2025-07-10  8:41 [PATCH] rtc: efi: Add runtime check for the wakeup service capability Feng Tang
@ 2025-07-11  1:06 ` Ard Biesheuvel
  2025-07-11  1:26   ` Ard Biesheuvel
  0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2025-07-11  1:06 UTC (permalink / raw)
  To: Feng Tang; +Cc: Alexandre Belloni, linux-rtc, linux-efi, linux-kernel

On Thu, 10 Jul 2025 at 18:41, Feng Tang <feng.tang@linux.alibaba.com> wrote:
>
> The kernel selftest of rtc reported a error on an ARM server which
> use rtc-efi device:
>
>         RUN           rtc.alarm_alm_set ...
>         rtctest.c:262:alarm_alm_set:Alarm time now set to 17:31:36.
>         rtctest.c:267:alarm_alm_set:Expected -1 (-1) != rc (-1)
>         alarm_alm_set: Test terminated by assertion
>                  FAIL  rtc.alarm_alm_set
>         not ok 5 rtc.alarm_alm_set
>
> The root cause is, the underlying EFI firmware doesn't support wakeup
> service (get/set alarm), while it doesn't have the EFI RT_PROP table
> either. As Ard Biesheuvel clarified [1], this breaks the UEFI spec,
> which requires EFI firmware to provide a 'RT_PROP' table if it doesn't
> support all runtime services (Section 4.6.2, UEFI spec 2.10).
>
> This issue was also reproduced on ARM server from another vendor, which
> doesn't have RT_PROP table either. This means, in real world, there are
> quite some platforms having this issue, that it doesn't support wakeup
> service while not providing a correct RT_PROP table, which makes it
> wrongly claimed to support it.
>
> Add a runtime check for the wakeup service to detect and correct this
> kind of cases.
>
> [1]. https://lore.kernel.org/lkml/CAMj1kXEkzXsjm0dPhzxB+KdtzqADd4NmafKmw2rKw7mAPBrgdA@mail.gmail.com/
>
> Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com>
> ---
>  drivers/rtc/rtc-efi.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>

Thanks, I've queued this up now.

> diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
> index fa8bf82df948..8d1b9bde6f66 100644
> --- a/drivers/rtc/rtc-efi.c
> +++ b/drivers/rtc/rtc-efi.c
> @@ -259,6 +259,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)
> @@ -272,7 +273,8 @@ static int __init efi_rtc_probe(struct platform_device *dev)
>
>         rtc->ops = &efi_rtc_ops;
>         clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, rtc->features);
> -       if (efi_rt_services_supported(EFI_RT_SUPPORTED_WAKEUP_SERVICES))
> +       if (efi_rt_services_supported(EFI_RT_SUPPORTED_WAKEUP_SERVICES) &&
> +               efi.get_wakeup_time(&enabled, &pending, &eft) == EFI_SUCCESS)
>                 set_bit(RTC_FEATURE_ALARM_WAKEUP_ONLY, rtc->features);
>         else
>                 clear_bit(RTC_FEATURE_ALARM, rtc->features);
> --
> 2.39.5 (Apple Git-154)
>
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] rtc: efi: Add runtime check for the wakeup service capability
  2025-07-11  1:06 ` Ard Biesheuvel
@ 2025-07-11  1:26   ` Ard Biesheuvel
  2025-07-11  8:56     ` Alexandre Belloni
  0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2025-07-11  1:26 UTC (permalink / raw)
  To: Feng Tang; +Cc: Alexandre Belloni, linux-rtc, linux-efi, linux-kernel

On Fri, 11 Jul 2025 at 11:06, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Thu, 10 Jul 2025 at 18:41, Feng Tang <feng.tang@linux.alibaba.com> wrote:
> >
> > The kernel selftest of rtc reported a error on an ARM server which
> > use rtc-efi device:
> >
> >         RUN           rtc.alarm_alm_set ...
> >         rtctest.c:262:alarm_alm_set:Alarm time now set to 17:31:36.
> >         rtctest.c:267:alarm_alm_set:Expected -1 (-1) != rc (-1)
> >         alarm_alm_set: Test terminated by assertion
> >                  FAIL  rtc.alarm_alm_set
> >         not ok 5 rtc.alarm_alm_set
> >
> > The root cause is, the underlying EFI firmware doesn't support wakeup
> > service (get/set alarm), while it doesn't have the EFI RT_PROP table
> > either. As Ard Biesheuvel clarified [1], this breaks the UEFI spec,
> > which requires EFI firmware to provide a 'RT_PROP' table if it doesn't
> > support all runtime services (Section 4.6.2, UEFI spec 2.10).
> >
> > This issue was also reproduced on ARM server from another vendor, which
> > doesn't have RT_PROP table either. This means, in real world, there are
> > quite some platforms having this issue, that it doesn't support wakeup
> > service while not providing a correct RT_PROP table, which makes it
> > wrongly claimed to support it.
> >
> > Add a runtime check for the wakeup service to detect and correct this
> > kind of cases.
> >
> > [1]. https://lore.kernel.org/lkml/CAMj1kXEkzXsjm0dPhzxB+KdtzqADd4NmafKmw2rKw7mAPBrgdA@mail.gmail.com/
> >
> > Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com>
> > ---
> >  drivers/rtc/rtc-efi.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
>
> Thanks, I've queued this up now.
>

Actually, we might just remove the EFI get/set wakeup time
functionality altogether, as it seems rather pointless to me to begin
with.

I'll send out an RFC shortly.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] rtc: efi: Add runtime check for the wakeup service capability
  2025-07-11  1:26   ` Ard Biesheuvel
@ 2025-07-11  8:56     ` Alexandre Belloni
  2025-07-14  2:08       ` Ard Biesheuvel
  0 siblings, 1 reply; 6+ messages in thread
From: Alexandre Belloni @ 2025-07-11  8:56 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: Feng Tang, linux-rtc, linux-efi, linux-kernel

On 11/07/2025 11:26:18+1000, Ard Biesheuvel wrote:
> On Fri, 11 Jul 2025 at 11:06, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Thu, 10 Jul 2025 at 18:41, Feng Tang <feng.tang@linux.alibaba.com> wrote:
> > >
> > > The kernel selftest of rtc reported a error on an ARM server which
> > > use rtc-efi device:
> > >
> > >         RUN           rtc.alarm_alm_set ...
> > >         rtctest.c:262:alarm_alm_set:Alarm time now set to 17:31:36.
> > >         rtctest.c:267:alarm_alm_set:Expected -1 (-1) != rc (-1)
> > >         alarm_alm_set: Test terminated by assertion
> > >                  FAIL  rtc.alarm_alm_set
> > >         not ok 5 rtc.alarm_alm_set
> > >
> > > The root cause is, the underlying EFI firmware doesn't support wakeup
> > > service (get/set alarm), while it doesn't have the EFI RT_PROP table
> > > either. As Ard Biesheuvel clarified [1], this breaks the UEFI spec,
> > > which requires EFI firmware to provide a 'RT_PROP' table if it doesn't
> > > support all runtime services (Section 4.6.2, UEFI spec 2.10).
> > >
> > > This issue was also reproduced on ARM server from another vendor, which
> > > doesn't have RT_PROP table either. This means, in real world, there are
> > > quite some platforms having this issue, that it doesn't support wakeup
> > > service while not providing a correct RT_PROP table, which makes it
> > > wrongly claimed to support it.
> > >
> > > Add a runtime check for the wakeup service to detect and correct this
> > > kind of cases.
> > >
> > > [1]. https://lore.kernel.org/lkml/CAMj1kXEkzXsjm0dPhzxB+KdtzqADd4NmafKmw2rKw7mAPBrgdA@mail.gmail.com/
> > >
> > > Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com>
> > > ---
> > >  drivers/rtc/rtc-efi.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> >
> > Thanks, I've queued this up now.
> >
> 
> Actually, we might just remove the EFI get/set wakeup time
> functionality altogether, as it seems rather pointless to me to begin
> with.
> 
> I'll send out an RFC shortly.

I guess this is going to also solve the issue reported by loongson
https://lore.kernel.org/linux-rtc/20250613061747.4117470-1-wangming01@loongson.cn/

However, please let me take care of patches in my subsystem...


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] rtc: efi: Add runtime check for the wakeup service capability
  2025-07-11  8:56     ` Alexandre Belloni
@ 2025-07-14  2:08       ` Ard Biesheuvel
  2025-07-14  2:36         ` Bibo Mao
  0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2025-07-14  2:08 UTC (permalink / raw)
  To: Alexandre Belloni; +Cc: Feng Tang, linux-rtc, linux-efi, linux-kernel

On Fri, 11 Jul 2025 at 18:56, Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
>
> On 11/07/2025 11:26:18+1000, Ard Biesheuvel wrote:
> > On Fri, 11 Jul 2025 at 11:06, Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > On Thu, 10 Jul 2025 at 18:41, Feng Tang <feng.tang@linux.alibaba.com> wrote:
> > > >
> > > > The kernel selftest of rtc reported a error on an ARM server which
> > > > use rtc-efi device:
> > > >
> > > >         RUN           rtc.alarm_alm_set ...
> > > >         rtctest.c:262:alarm_alm_set:Alarm time now set to 17:31:36.
> > > >         rtctest.c:267:alarm_alm_set:Expected -1 (-1) != rc (-1)
> > > >         alarm_alm_set: Test terminated by assertion
> > > >                  FAIL  rtc.alarm_alm_set
> > > >         not ok 5 rtc.alarm_alm_set
> > > >
> > > > The root cause is, the underlying EFI firmware doesn't support wakeup
> > > > service (get/set alarm), while it doesn't have the EFI RT_PROP table
> > > > either. As Ard Biesheuvel clarified [1], this breaks the UEFI spec,
> > > > which requires EFI firmware to provide a 'RT_PROP' table if it doesn't
> > > > support all runtime services (Section 4.6.2, UEFI spec 2.10).
> > > >
> > > > This issue was also reproduced on ARM server from another vendor, which
> > > > doesn't have RT_PROP table either. This means, in real world, there are
> > > > quite some platforms having this issue, that it doesn't support wakeup
> > > > service while not providing a correct RT_PROP table, which makes it
> > > > wrongly claimed to support it.
> > > >
> > > > Add a runtime check for the wakeup service to detect and correct this
> > > > kind of cases.
> > > >
> > > > [1]. https://lore.kernel.org/lkml/CAMj1kXEkzXsjm0dPhzxB+KdtzqADd4NmafKmw2rKw7mAPBrgdA@mail.gmail.com/
> > > >
> > > > Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com>
> > > > ---
> > > >  drivers/rtc/rtc-efi.c | 4 +++-
> > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > >
> > > Thanks, I've queued this up now.
> > >
> >
> > Actually, we might just remove the EFI get/set wakeup time
> > functionality altogether, as it seems rather pointless to me to begin
> > with.
> >
> > I'll send out an RFC shortly.
>
> I guess this is going to also solve the issue reported by loongson
> https://lore.kernel.org/linux-rtc/20250613061747.4117470-1-wangming01@loongson.cn/
>
> However, please let me take care of patches in my subsystem...
>

Apologies - I've dropped it now.

But please don't queue this, I'll send out my RFC shortly.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] rtc: efi: Add runtime check for the wakeup service capability
  2025-07-14  2:08       ` Ard Biesheuvel
@ 2025-07-14  2:36         ` Bibo Mao
  0 siblings, 0 replies; 6+ messages in thread
From: Bibo Mao @ 2025-07-14  2:36 UTC (permalink / raw)
  To: Ard Biesheuvel, Alexandre Belloni
  Cc: Feng Tang, linux-rtc, linux-efi, linux-kernel



On 2025/7/14 上午10:08, Ard Biesheuvel wrote:
> On Fri, 11 Jul 2025 at 18:56, Alexandre Belloni
> <alexandre.belloni@bootlin.com> wrote:
>>
>> On 11/07/2025 11:26:18+1000, Ard Biesheuvel wrote:
>>> On Fri, 11 Jul 2025 at 11:06, Ard Biesheuvel <ardb@kernel.org> wrote:
>>>>
>>>> On Thu, 10 Jul 2025 at 18:41, Feng Tang <feng.tang@linux.alibaba.com> wrote:
>>>>>
>>>>> The kernel selftest of rtc reported a error on an ARM server which
>>>>> use rtc-efi device:
>>>>>
>>>>>          RUN           rtc.alarm_alm_set ...
>>>>>          rtctest.c:262:alarm_alm_set:Alarm time now set to 17:31:36.
>>>>>          rtctest.c:267:alarm_alm_set:Expected -1 (-1) != rc (-1)
>>>>>          alarm_alm_set: Test terminated by assertion
>>>>>                   FAIL  rtc.alarm_alm_set
>>>>>          not ok 5 rtc.alarm_alm_set
>>>>>
>>>>> The root cause is, the underlying EFI firmware doesn't support wakeup
>>>>> service (get/set alarm), while it doesn't have the EFI RT_PROP table
>>>>> either. As Ard Biesheuvel clarified [1], this breaks the UEFI spec,
>>>>> which requires EFI firmware to provide a 'RT_PROP' table if it doesn't
>>>>> support all runtime services (Section 4.6.2, UEFI spec 2.10).
>>>>>
>>>>> This issue was also reproduced on ARM server from another vendor, which
>>>>> doesn't have RT_PROP table either. This means, in real world, there are
>>>>> quite some platforms having this issue, that it doesn't support wakeup
>>>>> service while not providing a correct RT_PROP table, which makes it
>>>>> wrongly claimed to support it.
>>>>>
>>>>> Add a runtime check for the wakeup service to detect and correct this
>>>>> kind of cases.
>>>>>
>>>>> [1]. https://lore.kernel.org/lkml/CAMj1kXEkzXsjm0dPhzxB+KdtzqADd4NmafKmw2rKw7mAPBrgdA@mail.gmail.com/
>>>>>
>>>>> Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com>
>>>>> ---
>>>>>   drivers/rtc/rtc-efi.c | 4 +++-
>>>>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>
>>>>
>>>> Thanks, I've queued this up now.
>>>>
>>>
>>> Actually, we might just remove the EFI get/set wakeup time
>>> functionality altogether, as it seems rather pointless to me to begin
>>> with.
>>>
>>> I'll send out an RFC shortly.
>>
>> I guess this is going to also solve the issue reported by loongson
>> https://lore.kernel.org/linux-rtc/20250613061747.4117470-1-wangming01@loongson.cn/
>>
>> However, please let me take care of patches in my subsystem...
>>
> 
> Apologies - I've dropped it now.
> 
> But please don't queue this, I'll send out my RFC shortly.
> 
This is similar problem on Loongarch VM when running ltp testcase rtc02, 
I do not know whether the root cause is the same, the runtime service is 
hard to debug.

Hope for RFC version :)

Regards
Bibo Mao


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-07-14  2:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-10  8:41 [PATCH] rtc: efi: Add runtime check for the wakeup service capability Feng Tang
2025-07-11  1:06 ` Ard Biesheuvel
2025-07-11  1:26   ` Ard Biesheuvel
2025-07-11  8:56     ` Alexandre Belloni
2025-07-14  2:08       ` Ard Biesheuvel
2025-07-14  2:36         ` Bibo Mao

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