RE: [PATCH RFC 0/8] Add efi page fault handler to fix/recover from
> >> The main problem is that we have just merged Sai's code to use a work > >> queue for invoking EFI services, and killing kworker threads is > >> obviously not going to make EFI any new friends. > >> > >> So I guess we should probably rework that code to use a dedicated > >> kthread, and just freeze it when it performs an illegal memory > >> access, and signal the completion in a way that makes the calling > >> thread understand that a) the call failed and b) runtime services are no > >> longer > available. > > > > Yes, this makes sense to me. > > Initially I did use a dedicated kthread for efi but moved to work queues > > later so > that the synchronization is well handled. I am ok to rework on the patches, > could > we ask Ingo to hold onto efi_workqueue patches? > > > > I am fine with keeping them. We will have a different approach in > v4.19 than in subsequent kernels, but the workqueue approach is still better > than > nothing at all. Makes sense to me.
Re: [PATCH RFC 0/8] Add efi page fault handler to fix/recover from
On 26 July 2018 at 20:51, Prakhya, Sai Praneeth wrote: >> > The basic idea is that, when you get an exception from a context that >> > is busted (i.e. it wasn't user code with a signal handler or kernel >> > code with a fixup), you can't return from the exception handler. That >> > leaves two choices: >> > >> > 1. Kill the task without returning. You could call do_exit(), or, >> > roughly equivalently, let yourself OOPS (fall through to die(), etc). >> > Then you have to make sure that the code that called into the thread >> > can handle it by signaling some completion first or whatever you need >> > to do. >> > >> > 2. Sleep forever. For example, set the task state to >> > TASK_UNINTERRUPTIBLE and reschedule. >> > >> > If the thread is a real kthread, (1) may be a bad idea. I’m not sure a >> > kthread can tolerate do_exit(), since the kthread core plays awful >> > games involving storing important data structures on the stack. Also, >> > with (2), it might be possible to enable some after-the-fact >> > debugging, since the full crashing state is still there. >> > >> >> The main problem is that we have just merged Sai's code to use a work queue >> for invoking EFI services, and killing kworker threads is obviously not >> going to >> make EFI any new friends. >> >> So I guess we should probably rework that code to use a dedicated kthread, >> and >> just freeze it when it performs an illegal memory access, and signal the >> completion in a way that makes the calling thread understand that a) the call >> failed and b) runtime services are no longer available. > > Yes, this makes sense to me. > Initially I did use a dedicated kthread for efi but moved to work queues > later so that the synchronization is well handled. I am ok to rework on the > patches, could we ask Ingo to hold onto efi_workqueue patches? > I am fine with keeping them. We will have a different approach in v4.19 than in subsequent kernels, but the workqueue approach is still better than nothing at all. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH RFC 0/8] Add efi page fault handler to fix/recover from
> > The basic idea is that, when you get an exception from a context that
> > is busted (i.e. it wasn't user code with a signal handler or kernel
> > code with a fixup), you can't return from the exception handler. That
> > leaves two choices:
> >
> > 1. Kill the task without returning. You could call do_exit(), or,
> > roughly equivalently, let yourself OOPS (fall through to die(), etc).
> > Then you have to make sure that the code that called into the thread
> > can handle it by signaling some completion first or whatever you need
> > to do.
> >
> > 2. Sleep forever. For example, set the task state to
> > TASK_UNINTERRUPTIBLE and reschedule.
> >
> > If the thread is a real kthread, (1) may be a bad idea. I’m not sure a
> > kthread can tolerate do_exit(), since the kthread core plays awful
> > games involving storing important data structures on the stack. Also,
> > with (2), it might be possible to enable some after-the-fact
> > debugging, since the full crashing state is still there.
> >
>
> The main problem is that we have just merged Sai's code to use a work queue
> for invoking EFI services, and killing kworker threads is obviously not going
> to
> make EFI any new friends.
>
> So I guess we should probably rework that code to use a dedicated kthread, and
> just freeze it when it performs an illegal memory access, and signal the
> completion in a way that makes the calling thread understand that a) the call
> failed and b) runtime services are no longer available.
Yes, this makes sense to me.
Initially I did use a dedicated kthread for efi but moved to work queues later
so that the synchronization is well handled. I am ok to rework on the patches,
could we ask Ingo to hold onto efi_workqueue patches?
Regards,
Sai
N�r��y���b�X��ǧv�^�){.n�+{�y��^n�r��z���h&�� �G���h�(�階�ݢj"���m�z�ޖ���f���h���~�m�
Re: [PATCH RFC 0/8] Add efi page fault handler to fix/recover from
On 26 July 2018 at 20:09, Andy Lutomirski wrote: > [sorry for totally busted formatting -- your email client and Gmail > don't get along] > > On Jul 26, 2018, at 10:23 AM, Prakhya, Sai Praneeth > wrote: > >> I have added some x86/intel folks to cc. >> >> I am fine with these patches, and I think it is useful to be able to >> detect and recover from buggy UEFI implementations that use boot time >> regions at runtime. >> >> However, I need help from the x86 maintainers/developers to review >> this so please cc them on these patches. >> >> >> >> I'm okay with the general concept, but I'm not really thrilled by the >> longjmp-like approach. >> >> >> >> Wasn't there a bunch of talk of having an actual kernel thread (kefid?) that >> makes runtime services calls? Did that actually get implemented? IMO a >> much nicer approach would be to handle the page fault by killing the thread, >> more or less like how we kill unruly user threads. (And it's yet another >> step toward calling EFI runtime services at CPL 3!) >> >> >> >> Hi Andy, >> >> >> >> Thanks for the feedback J. >> >> >> >> We have efi_kthread implemented and I did briefly think about killing the >> efi_kthread approach, but I thought it might not be possible (I might be >> wrong) because, we are in page fault handler and if we kill efi_kthread, the >> page fault handler still returns to firmware (because a firmware instruction >> caused page fault) and firmware will try to perform illegal access again >> thinking that the page fault handler might have fixed the fault. So, I took >> this approach of jumping out of firmware. >> >> >> >> Please let me know If you think I missed something. > > The basic idea is that, when you get an exception from a context that > is busted (i.e. it wasn't user code with a signal handler or kernel > code with a fixup), you can't return from the exception handler. That > leaves two choices: > > 1. Kill the task without returning. You could call do_exit(), or, > roughly equivalently, let yourself OOPS (fall through to die(), etc). > Then you have to make sure that the code that called into the thread > can handle it by signaling some completion first or whatever you need > to do. > > 2. Sleep forever. For example, set the task state to > TASK_UNINTERRUPTIBLE and reschedule. > > If the thread is a real kthread, (1) may be a bad idea. I’m not sure a > kthread can tolerate do_exit(), since the kthread core plays awful > games involving storing important data structures on the stack. Also, > with (2), it might be possible to enable some after-the-fact > debugging, since the full crashing state is still there. > The main problem is that we have just merged Sai's code to use a work queue for invoking EFI services, and killing kworker threads is obviously not going to make EFI any new friends. So I guess we should probably rework that code to use a dedicated kthread, and just freeze it when it performs an illegal memory access, and signal the completion in a way that makes the calling thread understand that a) the call failed and b) runtime services are no longer available. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC 0/8] Add efi page fault handler to fix/recover from
[sorry for totally busted formatting -- your email client and Gmail don't get along] On Jul 26, 2018, at 10:23 AM, Prakhya, Sai Praneeth wrote: > I have added some x86/intel folks to cc. > > I am fine with these patches, and I think it is useful to be able to > detect and recover from buggy UEFI implementations that use boot time > regions at runtime. > > However, I need help from the x86 maintainers/developers to review > this so please cc them on these patches. > > > > I'm okay with the general concept, but I'm not really thrilled by the > longjmp-like approach. > > > > Wasn't there a bunch of talk of having an actual kernel thread (kefid?) that > makes runtime services calls? Did that actually get implemented? IMO a much > nicer approach would be to handle the page fault by killing the thread, more > or less like how we kill unruly user threads. (And it's yet another step > toward calling EFI runtime services at CPL 3!) > > > > Hi Andy, > > > > Thanks for the feedback J. > > > > We have efi_kthread implemented and I did briefly think about killing the > efi_kthread approach, but I thought it might not be possible (I might be > wrong) because, we are in page fault handler and if we kill efi_kthread, the > page fault handler still returns to firmware (because a firmware instruction > caused page fault) and firmware will try to perform illegal access again > thinking that the page fault handler might have fixed the fault. So, I took > this approach of jumping out of firmware. > > > > Please let me know If you think I missed something. The basic idea is that, when you get an exception from a context that is busted (i.e. it wasn't user code with a signal handler or kernel code with a fixup), you can't return from the exception handler. That leaves two choices: 1. Kill the task without returning. You could call do_exit(), or, roughly equivalently, let yourself OOPS (fall through to die(), etc). Then you have to make sure that the code that called into the thread can handle it by signaling some completion first or whatever you need to do. 2. Sleep forever. For example, set the task state to TASK_UNINTERRUPTIBLE and reschedule. If the thread is a real kthread, (1) may be a bad idea. I’m not sure a kthread can tolerate do_exit(), since the kthread core plays awful games involving storing important data structures on the stack. Also, with (2), it might be possible to enable some after-the-fact debugging, since the full crashing state is still there. --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH RFC 0/8] Add efi page fault handler to fix/recover from
I have added some x86/intel folks to cc. I am fine with these patches, and I think it is useful to be able to detect and recover from buggy UEFI implementations that use boot time regions at runtime. However, I need help from the x86 maintainers/developers to review this so please cc them on these patches. I'm okay with the general concept, but I'm not really thrilled by the longjmp-like approach. Wasn't there a bunch of talk of having an actual kernel thread (kefid?) that makes runtime services calls? Did that actually get implemented? IMO a much nicer approach would be to handle the page fault by killing the thread, more or less like how we kill unruly user threads. (And it's yet another step toward calling EFI runtime services at CPL 3!) Hi Andy, Thanks for the feedback ☺. We have efi_kthread implemented and I did briefly think about killing the efi_kthread approach, but I thought it might not be possible (I might be wrong) because, we are in page fault handler and if we kill efi_kthread, the page fault handler still returns to firmware (because a firmware instruction caused page fault) and firmware will try to perform illegal access again thinking that the page fault handler might have fixed the fault. So, I took this approach of jumping out of firmware. Please let me know If you think I missed something. Regards, Sai
Re: [PATCH RFC 0/8] Add efi page fault handler to fix/recover from
On Wed, Jul 25, 2018 at 2:39 AM, Ard Biesheuvel wrote: > (+ Ingo, Andy, Peter) > > Hello Sai, > > I have added some x86/intel folks to cc. > > I am fine with these patches, and I think it is useful to be able to > detect and recover from buggy UEFI implementations that use boot time > regions at runtime. > > However, I need help from the x86 maintainers/developers to review > this so please cc them on these patches. > > I'm okay with the general concept, but I'm not really thrilled by the longjmp-like approach. Wasn't there a bunch of talk of having an actual kernel thread (kefid?) that makes runtime services calls? Did that actually get implemented? IMO a much nicer approach would be to handle the page fault by killing the thread, more or less like how we kill unruly user threads. (And it's yet another step toward calling EFI runtime services at CPL 3!) --Andy
Re: [PATCH RT] rtc: Disable RTC_DRV_EFI on RT
On 2018-07-26 18:01:47 [+0200], Ard Biesheuvel wrote: > Yes, that's what I was thinking. This way, you can still reboot into > the same kernel occasionally with EFI runtime services enabled to, > e.g., use efibootmgr. > > Acked-by: Ard Biesheuvel > > for both patches if you queue them in the -rt tree. If you want them > in mainline, please resend them as proper patches once > CONFIG_PREEMPT_RT_BASE has been declared in mainline as well. Okay, thank you. Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RT] rtc: Disable RTC_DRV_EFI on RT
On 26 July 2018 at 15:14, Sebastian Andrzej Siewior
wrote:
> On 2018-07-26 15:13:23 [+0200], To Ard Biesheuvel wrote:
>> On 2018-07-26 14:52:21 [+0200], Ard Biesheuvel wrote:
>> > We could also make it the default on -rt, but not disable it entirely, so
>> > that efi=runtime can be used to re-enable it.
>>
>> Oh. I like that. We have something similar for RCU. So I would need
>> that:
> and then I could make it default off:
>
> - >8
>
> Subject: [PATCH] efi: Disable runtime services on RT
>
> Based on meassurements the EFI functions get_variable /
> get_next_variable take up to 2us which looks okay.
> The functions get_time, set_time take around 10ms. Those 10ms are too
> much. Even one ms would be too much.
> Ard mentioned that SetVariable might even trigger larger latencies if
> the firware will erase flash blocks on NOR.
>
> The time-functions are used by efi-rtc and can be triggered during
> runtimed (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.
>
> This was observed on "EFI v2.60 by SoftIron Overdrive 1000".
>
> Signed-off-by: Sebastian Andrzej Siewior
> ---
> 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 232f4915223b..62c6e4b6ce3e 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -84,7 +84,7 @@ struct mm_struct efi_mm = {
> .mmlist = LIST_HEAD_INIT(efi_mm.mmlist),
> };
>
> -static bool disable_runtime;
> +static bool disable_runtime = IS_ENABLED(CONFIG_PREEMPT_RT_BASE);
> static int __init setup_noefi(char *arg)
> {
> disable_runtime = true;
> --
> 2.18.0
>
Yes, that's what I was thinking. This way, you can still reboot into
the same kernel occasionally with EFI runtime services enabled to,
e.g., use efibootmgr.
Acked-by: Ard Biesheuvel
for both patches if you queue them in the -rt tree. If you want them
in mainline, please resend them as proper patches once
CONFIG_PREEMPT_RT_BASE has been declared in mainline as well.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH RFC 0/8] Add efi page fault handler to fix/recover from
> > AFAIK, efi runtime services are not reentrant. With this in mind, if > > something > like above happens, I have completely turned off EFI runtime services in > kernel. > Is that OK? Or should we keep them enabled hoping to catch further illegal > accesses (assuming that this feature is not used in production kernels). > > > > I think it is reasonable to turn off services after that. The only problem is > that > distros will never be able to enable this, given that it may break systems > that are > working fine today. Actually these patches shouldn't break any existing behavior. Below are the possible illegal accesses. 1. If the illegal access was to boot time region, presently, it works during kernel boot but not after kernel boot, because we free boot time regions after set_virtual_address_map() is called. Please see efi_reserve/free_boot_services(). With the patches, we save boot time regions forever and hence illegal access could be fixed even after kernel boot. So, distros shouldn't see anything different here. 2. If the illegal access was to any other region except boot time region, presently, kernel panics both during and after kernel boot (this is the case reported by Al Stone). With these patches, we exit firmware context and hence fixup page fault handler. So, distros here, instead of seeing a kernel panic would see EFI Runtime Services disabled. Regards, Sai
Re: [PATCH RT] rtc: Disable RTC_DRV_EFI on RT
On 2018-07-26 15:13:23 [+0200], To Ard Biesheuvel wrote:
> On 2018-07-26 14:52:21 [+0200], Ard Biesheuvel wrote:
> > We could also make it the default on -rt, but not disable it entirely, so
> > that efi=runtime can be used to re-enable it.
>
> Oh. I like that. We have something similar for RCU. So I would need
> that:
and then I could make it default off:
- >8
Subject: [PATCH] efi: Disable runtime services on RT
Based on meassurements the EFI functions get_variable /
get_next_variable take up to 2us which looks okay.
The functions get_time, set_time take around 10ms. Those 10ms are too
much. Even one ms would be too much.
Ard mentioned that SetVariable might even trigger larger latencies if
the firware will erase flash blocks on NOR.
The time-functions are used by efi-rtc and can be triggered during
runtimed (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.
This was observed on "EFI v2.60 by SoftIron Overdrive 1000".
Signed-off-by: Sebastian Andrzej Siewior
---
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 232f4915223b..62c6e4b6ce3e 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -84,7 +84,7 @@ struct mm_struct efi_mm = {
.mmlist = LIST_HEAD_INIT(efi_mm.mmlist),
};
-static bool disable_runtime;
+static bool disable_runtime = IS_ENABLED(CONFIG_PREEMPT_RT_BASE);
static int __init setup_noefi(char *arg)
{
disable_runtime = true;
--
2.18.0
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RT] rtc: Disable RTC_DRV_EFI on RT
On 2018-07-26 14:52:21 [+0200], Ard Biesheuvel wrote:
> We could also make it the default on -rt, but not disable it entirely, so
> that efi=runtime can be used to re-enable it.
Oh. I like that. We have something similar for RCU. So I would need
that:
--- >8
Subject: [PATCH] efi: Allow efi=runtime
In case the option "efi=noruntime" is default at built-time, the user
could overwrite its sate by `efi=runtime' and allow it again.
Signed-off-by: Sebastian Andrzej Siewior
---
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 62c6e4b6ce3e..d6176ce50b45 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -110,6 +110,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;
+
return 0;
}
early_param("efi", parse_efi_cmdline);
--
2.18.0
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RT] rtc: Disable RTC_DRV_EFI on RT
> On 26 Jul 2018, at 14:46, Sebastian Andrzej Siewior > wrote: > >> On 2018-07-26 14:26:25 [+0200], Ard Biesheuvel wrote: >> What i mean is whatever you end up with if you pass efi=noruntime to the >> kernel. > ah. So I wouldn't have to patch this, just document it. That might work. > We could also make it the default on -rt, but not disable it entirely, so that efi=runtime can be used to re-enable it. >> But as i mentioned before, you may also lose the ability to reboot/shut down >> your system this way. Can you please double check? > > I checked before posting the patch. psci is used instead (on softiron > here, have nothing else). psci_sys_poweroff() works. > > Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RT] rtc: Disable RTC_DRV_EFI on RT
> On 26 Jul 2018, at 14:15, Sebastian Andrzej Siewior > wrote: > >> On 2018-07-26 11:15:52 [+0200], Ard Biesheuvel wrote: >> On 26 July 2018 at 11:04, Sebastian Andrzej Siewior >> wrote: >>> Based on measurements the EFI functions get_variable / >>> get_next_variable take up to 2us. The functions get_time, set_time take >>> around 10ms. Those 10ms are too much. Even one ms would be too much. >> >> You cannot extrapolate from these numbers. If the lack of worst case >> guarantees on an EFI system is a problem for -rt, the only meaningful >> course of action is to disable EFI runtime services entirely. > > Oh boy. I disable EFI entirely because then the bootloader won't boot > due to missing efi-stub. But I could disable the runtime-wrappers if > that is what you meant. Patch below. > What i mean is whatever you end up with if you pass efi=noruntime to the kernel. But as i mentioned before, you may also lose the ability to reboot/shut down your system this way. Can you please double check? >> I think SetVariable() may be even worse than GetTime(), given that NOR >> flash updates may involve erasing blocks. > > Here we go. "Read" can be triggered from userland via sysfs / efivars. > "Write" is (currently) limited to pstore? > No you can also write to variables via efivarfs and sysfs. >>> The funny part is that EFI is invoked with enabled interrupts. >>> According to my tracing I see the interrupt almost 10ms later which >>> indicates that EFI is disabling interrupts while doing its thing. >> >> Yes, and this is also highly implementation specific. Basing this kind >> of policy on a single implementation is not very wise imo. > > even if interrupts were not disabled then there would be no context > switch on exit from interrupt path due to the preempt-disable part. So > no improvement. > >>> This was observed on "EFI v2.60 by SoftIron Overdrive 1000". I don't say >>> that every EFI implementation does this but given that it has to access a >>> slow bus like I2C/SPI it is expected. >>> >>> Disable EFI's RTC driver on RT. >>> >> >> Other calls to GetTime() would still be permitted in this case, so >> this seems like a partial solution (although no other calls seem to >> exist atm) > > I *assumed* pstore would use the SetVariable during warning/bug. The > pstore thingy might be useful. Not sure. > > -- >8 > > Subject: [PATCH RT] arm64: Disable EFI runtime services on RT > > Based on meassurements the EFI functions get_variable / > get_next_variable take up to 2us which looks okay. > The functions get_time, set_time take around 10ms. Those 10ms are too > much. Even one ms would be too much. > Ard mentioned that SetVariable might even trigger larger latencies if > the firware will erase flash blocks on NOR. > > The time-functions are used by efi-rtc and can be triggered during > runtimed (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 will be provided by PSCI. > > Disable EFI's runtime wrappers. > > This was observed on "EFI v2.60 by SoftIron Overdrive 1000". > > Signed-off-by: Sebastian Andrzej Siewior > --- > arch/arm64/Kconfig | 2 +- > drivers/firmware/efi/arm-runtime.c | 2 ++ > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index 111db4e44fcf..68adcb0f4de6 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -1250,7 +1250,7 @@ config EFI >select LIBFDT >select UCS2_STRING >select EFI_PARAMS_FROM_FDT > -select EFI_RUNTIME_WRAPPERS > +select EFI_RUNTIME_WRAPPERS if !PREEMPT_RT_BASE >select EFI_STUB >select EFI_ARMSTUB >default y > diff --git a/drivers/firmware/efi/arm-runtime.c > b/drivers/firmware/efi/arm-runtime.c > index 5889cbea60b8..3e96db10c034 100644 > --- a/drivers/firmware/efi/arm-runtime.c > +++ b/drivers/firmware/efi/arm-runtime.c > @@ -140,8 +140,10 @@ static int __init arm_enable_runtime_services(void) >} > >/* Set up runtime services function pointers */ > +#ifdef CONFIG_EFI_RUNTIME_WRAPPERS >efi_native_runtime_setup(); >set_bit(EFI_RUNTIME_SERVICES, ); > +#endif > >return 0; > } > -- > 2.18.0 > -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RT] rtc: Disable RTC_DRV_EFI on RT
On 2018-07-26 11:15:52 [+0200], Ard Biesheuvel wrote: > On 26 July 2018 at 11:04, Sebastian Andrzej Siewior > wrote: > > Based on measurements the EFI functions get_variable / > > get_next_variable take up to 2us. The functions get_time, set_time take > > around 10ms. Those 10ms are too much. Even one ms would be too much. > > You cannot extrapolate from these numbers. If the lack of worst case > guarantees on an EFI system is a problem for -rt, the only meaningful > course of action is to disable EFI runtime services entirely. Oh boy. I disable EFI entirely because then the bootloader won't boot due to missing efi-stub. But I could disable the runtime-wrappers if that is what you meant. Patch below. > I think SetVariable() may be even worse than GetTime(), given that NOR > flash updates may involve erasing blocks. Here we go. "Read" can be triggered from userland via sysfs / efivars. "Write" is (currently) limited to pstore? > > The funny part is that EFI is invoked with enabled interrupts. > > According to my tracing I see the interrupt almost 10ms later which > > indicates that EFI is disabling interrupts while doing its thing. > > Yes, and this is also highly implementation specific. Basing this kind > of policy on a single implementation is not very wise imo. even if interrupts were not disabled then there would be no context switch on exit from interrupt path due to the preempt-disable part. So no improvement. > > This was observed on "EFI v2.60 by SoftIron Overdrive 1000". I don't say > > that every EFI implementation does this but given that it has to access a > > slow bus like I2C/SPI it is expected. > > > > Disable EFI's RTC driver on RT. > > > > Other calls to GetTime() would still be permitted in this case, so > this seems like a partial solution (although no other calls seem to > exist atm) I *assumed* pstore would use the SetVariable during warning/bug. The pstore thingy might be useful. Not sure. -- >8 Subject: [PATCH RT] arm64: Disable EFI runtime services on RT Based on meassurements the EFI functions get_variable / get_next_variable take up to 2us which looks okay. The functions get_time, set_time take around 10ms. Those 10ms are too much. Even one ms would be too much. Ard mentioned that SetVariable might even trigger larger latencies if the firware will erase flash blocks on NOR. The time-functions are used by efi-rtc and can be triggered during runtimed (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 will be provided by PSCI. Disable EFI's runtime wrappers. This was observed on "EFI v2.60 by SoftIron Overdrive 1000". Signed-off-by: Sebastian Andrzej Siewior --- arch/arm64/Kconfig | 2 +- drivers/firmware/efi/arm-runtime.c | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 111db4e44fcf..68adcb0f4de6 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -1250,7 +1250,7 @@ config EFI select LIBFDT select UCS2_STRING select EFI_PARAMS_FROM_FDT - select EFI_RUNTIME_WRAPPERS + select EFI_RUNTIME_WRAPPERS if !PREEMPT_RT_BASE select EFI_STUB select EFI_ARMSTUB default y diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c index 5889cbea60b8..3e96db10c034 100644 --- a/drivers/firmware/efi/arm-runtime.c +++ b/drivers/firmware/efi/arm-runtime.c @@ -140,8 +140,10 @@ static int __init arm_enable_runtime_services(void) } /* Set up runtime services function pointers */ +#ifdef CONFIG_EFI_RUNTIME_WRAPPERS efi_native_runtime_setup(); set_bit(EFI_RUNTIME_SERVICES, ); +#endif return 0; } -- 2.18.0 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RT] rtc: Disable RTC_DRV_EFI on RT
On 26 July 2018 at 11:04, Sebastian Andrzej Siewior wrote: > Based on measurements the EFI functions get_variable / > get_next_variable take up to 2us. The functions get_time, set_time take > around 10ms. Those 10ms are too much. Even one ms would be too much. You cannot extrapolate from these numbers. If the lack of worst case guarantees on an EFI system is a problem for -rt, the only meaningful course of action is to disable EFI runtime services entirely. I think SetVariable() may be even worse than GetTime(), given that NOR flash updates may involve erasing blocks. > The funny part is that EFI is invoked with enabled interrupts. > According to my tracing I see the interrupt almost 10ms later which > indicates that EFI is disabling interrupts while doing its thing. > Yes, and this is also highly implementation specific. Basing this kind of policy on a single implementation is not very wise imo. > This was observed on "EFI v2.60 by SoftIron Overdrive 1000". I don't say > that every EFI implementation does this but given that it has to access a > slow bus like I2C/SPI it is expected. > > Disable EFI's RTC driver on RT. > Other calls to GetTime() would still be permitted in this case, so this seems like a partial solution (although no other calls seem to exist atm) > Signed-off-by: Sebastian Andrzej Siewior > --- > drivers/rtc/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig > index a2ba5db36145..248d72711650 100644 > --- a/drivers/rtc/Kconfig > +++ b/drivers/rtc/Kconfig > @@ -1087,7 +1087,7 @@ config RTC_DRV_DA9063 > > config RTC_DRV_EFI > tristate "EFI RTC" > - depends on EFI && !X86 > + depends on EFI && !X86 && !PREEMPT_RT_BASE > help > If you say yes here you will get support for the EFI > Real Time Clock. > -- > 2.18.0 > -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RT] rtc: Disable RTC_DRV_EFI on RT
Based on measurements the EFI functions get_variable / get_next_variable take up to 2us. The functions get_time, set_time take around 10ms. Those 10ms are too much. Even one ms would be too much. The funny part is that EFI is invoked with enabled interrupts. According to my tracing I see the interrupt almost 10ms later which indicates that EFI is disabling interrupts while doing its thing. This was observed on "EFI v2.60 by SoftIron Overdrive 1000". I don't say that every EFI implementation does this but given that it has to access a slow bus like I2C/SPI it is expected. Disable EFI's RTC driver on RT. Signed-off-by: Sebastian Andrzej Siewior --- drivers/rtc/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig index a2ba5db36145..248d72711650 100644 --- a/drivers/rtc/Kconfig +++ b/drivers/rtc/Kconfig @@ -1087,7 +1087,7 @@ config RTC_DRV_DA9063 config RTC_DRV_EFI tristate "EFI RTC" - depends on EFI && !X86 + depends on EFI && !X86 && !PREEMPT_RT_BASE help If you say yes here you will get support for the EFI Real Time Clock. -- 2.18.0 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/18] xfrm: Add compat layer
Dmitry Safonov wrote: > So, here I add a compatible layer to xfrm. > As xfrm uses netlink notifications, kernel should send them in ABI > format that an application will parse. The proposed solution is > to save the ABI of bind() syscall. The realization detail is > to create kernel-hidden, non visible to userspace netlink groups > for compat applications. Why not use exisiting netlink support? Just add the 32bit skb to skb64->frag_list and let netlink find if tasks needs 64 or 32 one. It only needs this small fix to properly signal the end of a dump: https://marc.info/?l=linux-netdev=126625240303351=2 I had started a second attempt to make xfrm compat work, but its still in early stage. One link that might still have some value: https://git.breakpoint.cc/cgit/fw/net-next.git/commit/?h=xfrm_config_compat_07=f64430e6d9e297f3990f485a4832e273751b9869 (compat structure definitions with BUILD_BUG_ON checking) My plan was to make xfrm compat work strictly as shrinker (64->32) and expander (32->64), i.e. no/little changes to exisiting code and pass all "expanded" skbs through existing xfrm rcv functions. Example to illustrate idea: https://git.breakpoint.cc/cgit/fw/net-next.git/commit/?h=xfrm_config_compat_07=c622f067849b02170127b69471cb3481e4bc9e49 ... its supposed to take 64bit skb and create a 32bit one from it. Just for reference; I currently don't plan to work on this again. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC 0/8] Add efi page fault handler to fix/recover from
On 25 July 2018 at 19:32, Prakhya, Sai Praneeth
wrote:
>> I have added some x86/intel folks to cc.
>>
>> I am fine with these patches, and I think it is useful to be able to detect
>> and
>> recover from buggy UEFI implementations that use boot time regions at
>> runtime.
>>
>> However, I need help from the x86 maintainers/developers to review this so
>> please cc them on these patches.
>
> Hi Ard,
>
> Sure! I will keep them cc'ed.
>
> Could you also please let me know you thoughts on this approach
>
> If the illegal access occurs to any EFI region other than EFI boot time
> regions (Eg: EFI conventional memory or EFI loader code/data), these patches
> will exit firmware context and return to kernel i.e. we are adjusting RIP and
> RSP in efi page fault handler and leaving runtime service execution abruptly.
> Is that OK?
>
I need the x86 guys to tell me if that is OK. This is essentially an
open coded longjmp(), which smells dodgy to me, but this is an x86
question not an EFI question.
> This code in "[PATCH RFC 4/8] x86/efi: Add page fault handler to
> fixup/recover from page faults caused by firmware"
> + regs->sp = xmm_regs_rsp;
> + regs->ip = exit_fw_ctx_rip;
> + exited_fw_ctx = true;
> + clear_bit(EFI_RUNTIME_SERVICES, );
> + pr_info("Exited Firmware context and disabled EFI Runtime
> Services\n");
>
> AFAIK, efi runtime services are not reentrant. With this in mind, if
> something like above happens, I have completely turned off EFI runtime
> services in kernel. Is that OK? Or should we keep them enabled hoping to
> catch further illegal accesses (assuming that this feature is not used in
> production kernels).
>
I think it is reasonable to turn off services after that. The only
problem is that distros will never be able to enable this, given that
it may break systems that are working fine today.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
