linux-efi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] efi: Disable runtime services on RT
@ 2021-09-24 13:49 Sebastian Andrzej Siewior
  2021-09-24 13:49 ` [PATCH 1/2] " Sebastian Andrzej Siewior
  2021-09-24 13:49 ` [PATCH 2/2] efi: Allow efi=runtime Sebastian Andrzej Siewior
  0 siblings, 2 replies; 13+ messages in thread
From: Sebastian Andrzej Siewior @ 2021-09-24 13:49 UTC (permalink / raw)
  To: linux-efi; +Cc: Ard Biesheuvel, Thomas Gleixner

These two patches change the default of EFI's runtime service to off on
PREEMPT_RT and allow it to be overwritten in case something needs to be
done like changing the boot-order or so.
The reasoning is to avoid unforeseen large latencies, like the one
observed while using efi-rtc, by default.

Sebastian


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

* [PATCH 1/2] efi: Disable runtime services on RT
  2021-09-24 13:49 [PATCH 0/2] efi: Disable runtime services on RT Sebastian Andrzej Siewior
@ 2021-09-24 13:49 ` Sebastian Andrzej Siewior
  2021-09-28 13:30   ` Ard Biesheuvel
  2021-09-24 13:49 ` [PATCH 2/2] efi: Allow efi=runtime Sebastian Andrzej Siewior
  1 sibling, 1 reply; 13+ messages in thread
From: Sebastian Andrzej Siewior @ 2021-09-24 13:49 UTC (permalink / raw)
  To: linux-efi
  Cc: Ard Biesheuvel, Thomas Gleixner, Sebastian Andrzej Siewior,
	Ard Biesheuvel

Based on measurements the EFI functions get_variable /
get_next_variable take up to 2us which looks okay.
The functions get_time, set_time take around 10ms. These 10ms are too
much. Even one ms would be too much.
Ard mentioned that SetVariable might even trigger larger latencies if
the firmware will erase flash blocks on NOR.

The time-functions are used by efi-rtc and can be triggered during
run-time (either via explicit read/write or ntp sync).

The variable write could be used by pstore.
These functions can be disabled without much of a loss. The poweroff /
reboot hooks may be provided by PSCI.

Disable EFI's runtime wrappers on PREEMPT_RT.

This was observed on "EFI v2.60 by SoftIron Overdrive 1000".

Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/firmware/efi/efi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 847f33ffc4aed..39031cfcb6b92 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -66,7 +66,7 @@ struct mm_struct efi_mm = {
 
 struct workqueue_struct *efi_rts_wq;
 
-static bool disable_runtime;
+static bool disable_runtime = IS_ENABLED(CONFIG_PREEMPT_RT);
 static int __init setup_noefi(char *arg)
 {
 	disable_runtime = true;
-- 
2.33.0


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

* [PATCH 2/2] efi: Allow efi=runtime
  2021-09-24 13:49 [PATCH 0/2] efi: Disable runtime services on RT Sebastian Andrzej Siewior
  2021-09-24 13:49 ` [PATCH 1/2] " Sebastian Andrzej Siewior
@ 2021-09-24 13:49 ` Sebastian Andrzej Siewior
  1 sibling, 0 replies; 13+ messages in thread
From: Sebastian Andrzej Siewior @ 2021-09-24 13:49 UTC (permalink / raw)
  To: linux-efi
  Cc: Ard Biesheuvel, Thomas Gleixner, Sebastian Andrzej Siewior,
	Ard Biesheuvel

In case the command line option "efi=noruntime" is default at built-time, the user
could overwrite its state by `efi=runtime' and allow it again.

This is useful on PREEMPT_RT where "efi=noruntime" is default and the
user might need to alter the boot order for instance.

Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/firmware/efi/efi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 39031cfcb6b92..ae79c33001297 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -97,6 +97,9 @@ static int __init parse_efi_cmdline(char *str)
 	if (parse_option_str(str, "noruntime"))
 		disable_runtime = true;
 
+	if (parse_option_str(str, "runtime"))
+		disable_runtime = false;
+
 	if (parse_option_str(str, "nosoftreserve"))
 		set_bit(EFI_MEM_NO_SOFT_RESERVE, &efi.flags);
 
-- 
2.33.0


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

* Re: [PATCH 1/2] efi: Disable runtime services on RT
  2021-09-24 13:49 ` [PATCH 1/2] " Sebastian Andrzej Siewior
@ 2021-09-28 13:30   ` Ard Biesheuvel
  2021-09-28 13:33     ` Sebastian Andrzej Siewior
  0 siblings, 1 reply; 13+ messages in thread
From: Ard Biesheuvel @ 2021-09-28 13:30 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior; +Cc: linux-efi, Thomas Gleixner, Ard Biesheuvel

On Fri, 24 Sept 2021 at 15:49, Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
>
> Based on measurements the EFI functions get_variable /
> get_next_variable take up to 2us which looks okay.
> The functions get_time, set_time take around 10ms. These 10ms are too
> much. Even one ms would be too much.
> Ard mentioned that SetVariable might even trigger larger latencies if
> the firmware will erase flash blocks on NOR.
>
> The time-functions are used by efi-rtc and can be triggered during
> run-time (either via explicit read/write or ntp sync).
>
> The variable write could be used by pstore.
> These functions can be disabled without much of a loss. The poweroff /
> reboot hooks may be provided by PSCI.
>
> Disable EFI's runtime wrappers on PREEMPT_RT.
>
> This was observed on "EFI v2.60 by SoftIron Overdrive 1000".
>
> Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  drivers/firmware/efi/efi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index 847f33ffc4aed..39031cfcb6b92 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -66,7 +66,7 @@ struct mm_struct efi_mm = {
>
>  struct workqueue_struct *efi_rts_wq;
>
> -static bool disable_runtime;
> +static bool disable_runtime = IS_ENABLED(CONFIG_PREEMPT_RT);
>  static int __init setup_noefi(char *arg)
>  {
>         disable_runtime = true;



This is generic code and the commit log only talks about arm64. How
about other architectures?

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

* Re: [PATCH 1/2] efi: Disable runtime services on RT
  2021-09-28 13:30   ` Ard Biesheuvel
@ 2021-09-28 13:33     ` Sebastian Andrzej Siewior
  2021-09-28 13:34       ` Ard Biesheuvel
  0 siblings, 1 reply; 13+ messages in thread
From: Sebastian Andrzej Siewior @ 2021-09-28 13:33 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: linux-efi, Thomas Gleixner, Ard Biesheuvel

On 2021-09-28 15:30:32 [+0200], Ard Biesheuvel wrote:
> This is generic code and the commit log only talks about arm64. How
> about other architectures?

They also invoke the EFI services with disables interrupts. If they
provide a RTC behind spi/i2c then we end up in the same situation right?
Or did I misunderstand your point?

Sebastian

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

* Re: [PATCH 1/2] efi: Disable runtime services on RT
  2021-09-28 13:33     ` Sebastian Andrzej Siewior
@ 2021-09-28 13:34       ` Ard Biesheuvel
  2021-09-28 14:24         ` Sebastian Andrzej Siewior
  2021-09-28 20:28         ` Thomas Gleixner
  0 siblings, 2 replies; 13+ messages in thread
From: Ard Biesheuvel @ 2021-09-28 13:34 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior; +Cc: linux-efi, Thomas Gleixner, Ard Biesheuvel

On Tue, 28 Sept 2021 at 15:33, Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
>
> On 2021-09-28 15:30:32 [+0200], Ard Biesheuvel wrote:
> > This is generic code and the commit log only talks about arm64. How
> > about other architectures?
>
> They also invoke the EFI services with disables interrupts. If they
> provide a RTC behind spi/i2c then we end up in the same situation right?
> Or did I misunderstand your point?
>

Are you sure you want to disable EFI runtime services on all x86
systems with PREEMPT_RT as well?

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

* Re: [PATCH 1/2] efi: Disable runtime services on RT
  2021-09-28 13:34       ` Ard Biesheuvel
@ 2021-09-28 14:24         ` Sebastian Andrzej Siewior
  2021-10-20  6:09           ` joeyli
  2021-09-28 20:28         ` Thomas Gleixner
  1 sibling, 1 reply; 13+ messages in thread
From: Sebastian Andrzej Siewior @ 2021-09-28 14:24 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: linux-efi, Thomas Gleixner

On 2021-09-28 15:34:47 [+0200], Ard Biesheuvel wrote:
> Are you sure you want to disable EFI runtime services on all x86
> systems with PREEMPT_RT as well?

The only problem that I am aware of is that you need to reboot with
enabled runtime service (via bootargs, #2) in order to alter boot loader
settings.
I'm not aware of any other shortcomings. There are no guarantees how
long an EFI service routine may take.
That patch is in the RT queue since v4.18-rc8-rt1.

Sebastian

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

* Re: [PATCH 1/2] efi: Disable runtime services on RT
  2021-09-28 13:34       ` Ard Biesheuvel
  2021-09-28 14:24         ` Sebastian Andrzej Siewior
@ 2021-09-28 20:28         ` Thomas Gleixner
  2021-09-28 20:41           ` Ard Biesheuvel
  1 sibling, 1 reply; 13+ messages in thread
From: Thomas Gleixner @ 2021-09-28 20:28 UTC (permalink / raw)
  To: Ard Biesheuvel, Sebastian Andrzej Siewior; +Cc: linux-efi, Ard Biesheuvel

Ard,

On Tue, Sep 28 2021 at 15:34, Ard Biesheuvel wrote:
> On Tue, 28 Sept 2021 at 15:33, Sebastian Andrzej Siewior
> <bigeasy@linutronix.de> wrote:
>>
>> On 2021-09-28 15:30:32 [+0200], Ard Biesheuvel wrote:
>> > This is generic code and the commit log only talks about arm64. How
>> > about other architectures?
>>
>> They also invoke the EFI services with disables interrupts. If they
>> provide a RTC behind spi/i2c then we end up in the same situation right?
>> Or did I misunderstand your point?
>>
> Are you sure you want to disable EFI runtime services on all x86
> systems with PREEMPT_RT as well?

I'm pretty sure because we've ran into inacceptable latencies with EFI
runtime services often enough.

Since we disabled them these complaints have gone down to 0 and nobody
so far complained about their non-availability.

We might revisit that and make them default disabled on RT and offer a
command line option to enable them for those who really want them.

Thanks,

        tglx

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

* Re: [PATCH 1/2] efi: Disable runtime services on RT
  2021-09-28 20:28         ` Thomas Gleixner
@ 2021-09-28 20:41           ` Ard Biesheuvel
  0 siblings, 0 replies; 13+ messages in thread
From: Ard Biesheuvel @ 2021-09-28 20:41 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: Sebastian Andrzej Siewior, linux-efi, Ard Biesheuvel

On Tue, 28 Sept 2021 at 22:28, Thomas Gleixner <tglx@linutronix.de> wrote:
>
> Ard,
>
> On Tue, Sep 28 2021 at 15:34, Ard Biesheuvel wrote:
> > On Tue, 28 Sept 2021 at 15:33, Sebastian Andrzej Siewior
> > <bigeasy@linutronix.de> wrote:
> >>
> >> On 2021-09-28 15:30:32 [+0200], Ard Biesheuvel wrote:
> >> > This is generic code and the commit log only talks about arm64. How
> >> > about other architectures?
> >>
> >> They also invoke the EFI services with disables interrupts. If they
> >> provide a RTC behind spi/i2c then we end up in the same situation right?
> >> Or did I misunderstand your point?
> >>
> > Are you sure you want to disable EFI runtime services on all x86
> > systems with PREEMPT_RT as well?
>
> I'm pretty sure because we've ran into inacceptable latencies with EFI
> runtime services often enough.
>
> Since we disabled them these complaints have gone down to 0 and nobody
> so far complained about their non-availability.
>

Excellent.

> We might revisit that and make them default disabled on RT and offer a
> command line option to enable them for those who really want them.
>

Yes this is what Sebastian's second patch provides.

> Thanks,
>
>         tglx

I'll take this as an ack from you, Thomas, and queue these up for v5.16

Thanks,
Ard.

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

* Re: [PATCH 1/2] efi: Disable runtime services on RT
  2021-09-28 14:24         ` Sebastian Andrzej Siewior
@ 2021-10-20  6:09           ` joeyli
  2021-10-20  6:28             ` Sebastian Andrzej Siewior
  0 siblings, 1 reply; 13+ messages in thread
From: joeyli @ 2021-10-20  6:09 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior; +Cc: Ard Biesheuvel, linux-efi, Thomas Gleixner

On Tue, Sep 28, 2021 at 04:24:34PM +0200, Sebastian Andrzej Siewior wrote:
> On 2021-09-28 15:34:47 [+0200], Ard Biesheuvel wrote:
> > Are you sure you want to disable EFI runtime services on all x86
> > systems with PREEMPT_RT as well?
> 
> The only problem that I am aware of is that you need to reboot with
> enabled runtime service (via bootargs, #2) in order to alter boot loader
> settings.

Just provide another case:
Anyone who uses mokutil for enrolling MOK will also need reboot with
efi=runtime first.

> I'm not aware of any other shortcomings. There are no guarantees how
> long an EFI service routine may take.
> That patch is in the RT queue since v4.18-rc8-rt1.
> 
> Sebastian

Joey Lee


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

* Re: [PATCH 1/2] efi: Disable runtime services on RT
  2021-10-20  6:09           ` joeyli
@ 2021-10-20  6:28             ` Sebastian Andrzej Siewior
  2021-10-20  6:47               ` joeyli
  0 siblings, 1 reply; 13+ messages in thread
From: Sebastian Andrzej Siewior @ 2021-10-20  6:28 UTC (permalink / raw)
  To: joeyli; +Cc: Ard Biesheuvel, linux-efi, Thomas Gleixner

On 2021-10-20 14:09:55 [+0800], joeyli wrote:
> On Tue, Sep 28, 2021 at 04:24:34PM +0200, Sebastian Andrzej Siewior wrote:
> > On 2021-09-28 15:34:47 [+0200], Ard Biesheuvel wrote:
> > > Are you sure you want to disable EFI runtime services on all x86
> > > systems with PREEMPT_RT as well?
> > 
> > The only problem that I am aware of is that you need to reboot with
> > enabled runtime service (via bootargs, #2) in order to alter boot loader
> > settings.
> 
> Just provide another case:
> Anyone who uses mokutil for enrolling MOK will also need reboot with
> efi=runtime first.

I have no idea what it does. This enrolling is only required once
per-lifetime and not on each system boot, right?

> Joey Lee

Sebastian

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

* Re: [PATCH 1/2] efi: Disable runtime services on RT
  2021-10-20  6:28             ` Sebastian Andrzej Siewior
@ 2021-10-20  6:47               ` joeyli
  2021-10-20  7:20                 ` Sebastian Andrzej Siewior
  0 siblings, 1 reply; 13+ messages in thread
From: joeyli @ 2021-10-20  6:47 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior; +Cc: Ard Biesheuvel, linux-efi, Thomas Gleixner

On Wed, Oct 20, 2021 at 08:28:50AM +0200, Sebastian Andrzej Siewior wrote:
> On 2021-10-20 14:09:55 [+0800], joeyli wrote:
> > On Tue, Sep 28, 2021 at 04:24:34PM +0200, Sebastian Andrzej Siewior wrote:
> > > On 2021-09-28 15:34:47 [+0200], Ard Biesheuvel wrote:
> > > > Are you sure you want to disable EFI runtime services on all x86
> > > > systems with PREEMPT_RT as well?
> > > 
> > > The only problem that I am aware of is that you need to reboot with
> > > enabled runtime service (via bootargs, #2) in order to alter boot loader
> > > settings.
> > 
> > Just provide another case:
> > Anyone who uses mokutil for enrolling MOK will also need reboot with
> > efi=runtime first.
> 
> I have no idea what it does. This enrolling is only required once
> per-lifetime and not on each system boot, right?

Yes, no each system boot.

But when boot loader or kernel be updated or user wants to install a
self-signed kernel or module. Then they need to reboot with efi=runtime
in the future.

On the other hand, any RT distro that suppors MOK needs to modify their
installation program/process to add efi=runtime in first boot. Otherwise
the installation will be failed. Honestly this patch changed the kernel
behavior and it may causes that old installation get problem.

Joey Lee


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

* Re: [PATCH 1/2] efi: Disable runtime services on RT
  2021-10-20  6:47               ` joeyli
@ 2021-10-20  7:20                 ` Sebastian Andrzej Siewior
  0 siblings, 0 replies; 13+ messages in thread
From: Sebastian Andrzej Siewior @ 2021-10-20  7:20 UTC (permalink / raw)
  To: joeyli; +Cc: Ard Biesheuvel, linux-efi, Thomas Gleixner

On 2021-10-20 14:47:18 [+0800], joeyli wrote:
> > I have no idea what it does. This enrolling is only required once
> > per-lifetime and not on each system boot, right?
> 
> Yes, no each system boot.
> 
> But when boot loader or kernel be updated or user wants to install a
> self-signed kernel or module. Then they need to reboot with efi=runtime
> in the future.

I see.

> On the other hand, any RT distro that suppors MOK needs to modify their
> installation program/process to add efi=runtime in first boot. Otherwise
> the installation will be failed. Honestly this patch changed the kernel
> behavior and it may causes that old installation get problem.

Nope. It was in the -RT queue since v4.18-RT, and I see it backported
into latest v4.14.244-rt121 probably earlier kernels, too.
So unless you ship something pre v4.18-RT without that patch and you
update to post v4.18-RT you need to pay attention - independently of
this being now merged upstream.

This is also in the wiki
   https://wiki.linuxfoundation.org/realtime/documentation/known_limitations#efi

> Joey Lee

Sebastian

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

end of thread, other threads:[~2021-10-20  7:20 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-24 13:49 [PATCH 0/2] efi: Disable runtime services on RT Sebastian Andrzej Siewior
2021-09-24 13:49 ` [PATCH 1/2] " Sebastian Andrzej Siewior
2021-09-28 13:30   ` Ard Biesheuvel
2021-09-28 13:33     ` Sebastian Andrzej Siewior
2021-09-28 13:34       ` Ard Biesheuvel
2021-09-28 14:24         ` Sebastian Andrzej Siewior
2021-10-20  6:09           ` joeyli
2021-10-20  6:28             ` Sebastian Andrzej Siewior
2021-10-20  6:47               ` joeyli
2021-10-20  7:20                 ` Sebastian Andrzej Siewior
2021-09-28 20:28         ` Thomas Gleixner
2021-09-28 20:41           ` Ard Biesheuvel
2021-09-24 13:49 ` [PATCH 2/2] efi: Allow efi=runtime Sebastian Andrzej Siewior

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