[PATCH 0/2] efi: Disable runtime services on RT

[PATCH 0/2] efi: Disable runtime services on RT

[Sebastian Andrzej Siewior]

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

[PATCH 1/2] efi: Disable runtime services on RT

[Sebastian Andrzej Siewior]

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

Re: [PATCH 1/2] efi: Disable runtime services on RT

[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?

Re: [PATCH 1/2] efi: Disable runtime services on RT

[Sebastian Andrzej Siewior]

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

Re: [PATCH 1/2] efi: Disable runtime services on RT

[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?

Re: [PATCH 1/2] efi: Disable runtime services on RT

[Sebastian Andrzej Siewior]

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

Re: [PATCH 1/2] efi: Disable runtime services on RT

[joeyli]

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

Re: [PATCH 1/2] efi: Disable runtime services on RT

[Sebastian Andrzej Siewior]

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

Re: [PATCH 1/2] efi: Disable runtime services on RT

[joeyli]

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

Re: [PATCH 1/2] efi: Disable runtime services on RT

[Sebastian Andrzej Siewior]

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

Re: [PATCH 1/2] efi: Disable runtime services on RT

[Thomas Gleixner]

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

Re: [PATCH 1/2] efi: Disable runtime services on RT

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

[PATCH 2/2] efi: Allow efi=runtime

[Sebastian Andrzej Siewior]

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