* [dovetail 6.1.y-cip][PATCH 0/3] fpsimd fixes and cleanups
@ 2025-09-19 6:55 Jan Kiszka
2025-09-19 6:55 ` [dovetail 6.1.y-cip][PATCH 1/3] arm64: fpsimd: dovetail: Hard-disable interrupts inside fpsimd_save_and_flush_cpu_state Jan Kiszka
` (2 more replies)
0 siblings, 3 replies; 23+ messages in thread
From: Jan Kiszka @ 2025-09-19 6:55 UTC (permalink / raw)
To: Philippe Gerum, xenomai
See patches.
Jan
Jan Kiszka (3):
arm64: fpsimd: dovetail: Hard-disable interrupts inside
fpsimd_save_and_flush_cpu_state
arm64: fpsimd: dovetail: drop redundant hard irqs disabling
arm64: fpsimd: dovetail: Drop extra condition from WARN_ON
arch/arm64/kernel/fpsimd.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
--
2.51.0
^ permalink raw reply [flat|nested] 23+ messages in thread* [dovetail 6.1.y-cip][PATCH 1/3] arm64: fpsimd: dovetail: Hard-disable interrupts inside fpsimd_save_and_flush_cpu_state 2025-09-19 6:55 [dovetail 6.1.y-cip][PATCH 0/3] fpsimd fixes and cleanups Jan Kiszka @ 2025-09-19 6:55 ` Jan Kiszka 2025-09-19 7:18 ` Jan Kiszka 2025-09-23 18:51 ` Philippe Gerum 2025-09-19 6:55 ` [dovetail 6.1.y-cip][PATCH 2/3] arm64: fpsimd: dovetail: drop redundant hard irqs disabling Jan Kiszka 2025-09-19 6:55 ` [dovetail 6.1.y-cip][PATCH 3/3] arm64: fpsimd: dovetail: Drop extra condition from WARN_ON Jan Kiszka 2 siblings, 2 replies; 23+ messages in thread From: Jan Kiszka @ 2025-09-19 6:55 UTC (permalink / raw) To: Philippe Gerum, xenomai From: Jan Kiszka <jan.kiszka@siemens.com> Not all callers of this function do that themselves. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- arch/arm64/kernel/fpsimd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c index 83b8492984982..ba804fcdfdb3f 100644 --- a/arch/arm64/kernel/fpsimd.c +++ b/arch/arm64/kernel/fpsimd.c @@ -1951,10 +1951,10 @@ void fpsimd_save_and_flush_cpu_state(void) if (!system_supports_fpsimd()) return; WARN_ON(!hard_irqs_disabled() && preemptible()); - __get_cpu_fpsimd_context(); + get_cpu_fpsimd_context(); __fpsimd_save(); fpsimd_flush_cpu_state(); - __put_cpu_fpsimd_context(); + put_cpu_fpsimd_context(); } #ifdef CONFIG_KERNEL_MODE_NEON -- 2.51.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [dovetail 6.1.y-cip][PATCH 1/3] arm64: fpsimd: dovetail: Hard-disable interrupts inside fpsimd_save_and_flush_cpu_state 2025-09-19 6:55 ` [dovetail 6.1.y-cip][PATCH 1/3] arm64: fpsimd: dovetail: Hard-disable interrupts inside fpsimd_save_and_flush_cpu_state Jan Kiszka @ 2025-09-19 7:18 ` Jan Kiszka 2025-09-19 9:20 ` Jan Kiszka 2025-09-23 18:51 ` Philippe Gerum 1 sibling, 1 reply; 23+ messages in thread From: Jan Kiszka @ 2025-09-19 7:18 UTC (permalink / raw) To: Philippe Gerum, xenomai On 19.09.25 08:55, Jan Kiszka wrote: > From: Jan Kiszka <jan.kiszka@siemens.com> > > Not all callers of this function do that themselves. > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > arch/arm64/kernel/fpsimd.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c > index 83b8492984982..ba804fcdfdb3f 100644 > --- a/arch/arm64/kernel/fpsimd.c > +++ b/arch/arm64/kernel/fpsimd.c > @@ -1951,10 +1951,10 @@ void fpsimd_save_and_flush_cpu_state(void) > if (!system_supports_fpsimd()) > return; > WARN_ON(!hard_irqs_disabled() && preemptible()); > - __get_cpu_fpsimd_context(); > + get_cpu_fpsimd_context(); > __fpsimd_save(); > fpsimd_flush_cpu_state(); > - __put_cpu_fpsimd_context(); > + put_cpu_fpsimd_context(); > } > > #ifdef CONFIG_KERNEL_MODE_NEON This one is actually vanishing when rebasing over v6.1.151-cip46 which has https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=8f4dc4e54eed4bebb18390305eb1f721c00457e1. Jan -- Siemens AG, Foundational Technologies Linux Expert Center ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dovetail 6.1.y-cip][PATCH 1/3] arm64: fpsimd: dovetail: Hard-disable interrupts inside fpsimd_save_and_flush_cpu_state 2025-09-19 7:18 ` Jan Kiszka @ 2025-09-19 9:20 ` Jan Kiszka 0 siblings, 0 replies; 23+ messages in thread From: Jan Kiszka @ 2025-09-19 9:20 UTC (permalink / raw) To: Philippe Gerum, xenomai On 19.09.25 09:18, Jan Kiszka wrote: > On 19.09.25 08:55, Jan Kiszka wrote: >> From: Jan Kiszka <jan.kiszka@siemens.com> >> >> Not all callers of this function do that themselves. >> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >> --- >> arch/arm64/kernel/fpsimd.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c >> index 83b8492984982..ba804fcdfdb3f 100644 >> --- a/arch/arm64/kernel/fpsimd.c >> +++ b/arch/arm64/kernel/fpsimd.c >> @@ -1951,10 +1951,10 @@ void fpsimd_save_and_flush_cpu_state(void) >> if (!system_supports_fpsimd()) >> return; >> WARN_ON(!hard_irqs_disabled() && preemptible()); >> - __get_cpu_fpsimd_context(); >> + get_cpu_fpsimd_context(); >> __fpsimd_save(); >> fpsimd_flush_cpu_state(); >> - __put_cpu_fpsimd_context(); >> + put_cpu_fpsimd_context(); >> } >> >> #ifdef CONFIG_KERNEL_MODE_NEON > > This one is actually vanishing when rebasing over v6.1.151-cip46 which > has > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=8f4dc4e54eed4bebb18390305eb1f721c00457e1. > ...provided some "flags" argument is provided to get/put_cpu_fpsimd_context which I didn't do above, and which should be done while resolving the merge conflict with v6.1.151-cip46. Jan -- Siemens AG, Foundational Technologies Linux Expert Center ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dovetail 6.1.y-cip][PATCH 1/3] arm64: fpsimd: dovetail: Hard-disable interrupts inside fpsimd_save_and_flush_cpu_state 2025-09-19 6:55 ` [dovetail 6.1.y-cip][PATCH 1/3] arm64: fpsimd: dovetail: Hard-disable interrupts inside fpsimd_save_and_flush_cpu_state Jan Kiszka 2025-09-19 7:18 ` Jan Kiszka @ 2025-09-23 18:51 ` Philippe Gerum 2025-09-25 12:40 ` Jan Kiszka 1 sibling, 1 reply; 23+ messages in thread From: Philippe Gerum @ 2025-09-23 18:51 UTC (permalink / raw) To: Jan Kiszka; +Cc: xenomai Jan Kiszka <jan.kiszka@siemens.com> writes: > From: Jan Kiszka <jan.kiszka@siemens.com> > > Not all callers of this function do that themselves. > Are we 100% sure that kvm_arch_vcpu_load_fp() and kvm_arch_vcpu_put_fp() would not benefit from doing that themselves precisely, to create an atomic section around their internal state change _and_ the fpsimd flush op? > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > arch/arm64/kernel/fpsimd.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c > index 83b8492984982..ba804fcdfdb3f 100644 > --- a/arch/arm64/kernel/fpsimd.c > +++ b/arch/arm64/kernel/fpsimd.c > @@ -1951,10 +1951,10 @@ void fpsimd_save_and_flush_cpu_state(void) > if (!system_supports_fpsimd()) > return; > WARN_ON(!hard_irqs_disabled() && preemptible()); > - __get_cpu_fpsimd_context(); > + get_cpu_fpsimd_context(); > __fpsimd_save(); > fpsimd_flush_cpu_state(); > - __put_cpu_fpsimd_context(); > + put_cpu_fpsimd_context(); > } > > #ifdef CONFIG_KERNEL_MODE_NEON -- Philippe. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dovetail 6.1.y-cip][PATCH 1/3] arm64: fpsimd: dovetail: Hard-disable interrupts inside fpsimd_save_and_flush_cpu_state 2025-09-23 18:51 ` Philippe Gerum @ 2025-09-25 12:40 ` Jan Kiszka 2025-09-25 12:47 ` Florian Bezdeka 0 siblings, 1 reply; 23+ messages in thread From: Jan Kiszka @ 2025-09-25 12:40 UTC (permalink / raw) To: Philippe Gerum; +Cc: xenomai On 23.09.25 20:51, Philippe Gerum wrote: > Jan Kiszka <jan.kiszka@siemens.com> writes: > >> From: Jan Kiszka <jan.kiszka@siemens.com> >> >> Not all callers of this function do that themselves. >> > > Are we 100% sure that kvm_arch_vcpu_load_fp() and kvm_arch_vcpu_put_fp() > would not benefit from doing that themselves precisely, to create an > atomic section around their internal state change _and_ the fpsimd flush > op? Is kvm on arm64 officially enabled for oob (-preemption), or are we patching blindly here? If no one was looking at kvm yet, then you might be right. If it was reviewed already, then this here is just cleaning up. Jan > >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >> --- >> arch/arm64/kernel/fpsimd.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c >> index 83b8492984982..ba804fcdfdb3f 100644 >> --- a/arch/arm64/kernel/fpsimd.c >> +++ b/arch/arm64/kernel/fpsimd.c >> @@ -1951,10 +1951,10 @@ void fpsimd_save_and_flush_cpu_state(void) >> if (!system_supports_fpsimd()) >> return; >> WARN_ON(!hard_irqs_disabled() && preemptible()); >> - __get_cpu_fpsimd_context(); >> + get_cpu_fpsimd_context(); >> __fpsimd_save(); >> fpsimd_flush_cpu_state(); >> - __put_cpu_fpsimd_context(); >> + put_cpu_fpsimd_context(); >> } >> >> #ifdef CONFIG_KERNEL_MODE_NEON > -- Siemens AG, Foundational Technologies Linux Expert Center ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dovetail 6.1.y-cip][PATCH 1/3] arm64: fpsimd: dovetail: Hard-disable interrupts inside fpsimd_save_and_flush_cpu_state 2025-09-25 12:40 ` Jan Kiszka @ 2025-09-25 12:47 ` Florian Bezdeka 2025-09-25 12:49 ` Jan Kiszka 0 siblings, 1 reply; 23+ messages in thread From: Florian Bezdeka @ 2025-09-25 12:47 UTC (permalink / raw) To: Jan Kiszka, Philippe Gerum; +Cc: xenomai On Thu, 2025-09-25 at 14:40 +0200, Jan Kiszka wrote: > On 23.09.25 20:51, Philippe Gerum wrote: > > Jan Kiszka <jan.kiszka@siemens.com> writes: > > > > > From: Jan Kiszka <jan.kiszka@siemens.com> > > > > > > Not all callers of this function do that themselves. > > > > > > > Are we 100% sure that kvm_arch_vcpu_load_fp() and kvm_arch_vcpu_put_fp() > > would not benefit from doing that themselves precisely, to create an > > atomic section around their internal state change _and_ the fpsimd flush > > op? > > Is kvm on arm64 officially enabled for oob (-preemption), or are we > patching blindly here? If no one was looking at kvm yet, then you might > be right. If it was reviewed already, then this here is just cleaning up. We have https://gitlab.com/Xenomai/linux-dovetail/-/commit/028ad3b2c19ca8a3655577fb34cac7996f166d2a since 6.14 (AFAICT). At the time of writing this was the first kernel release that used in-kernel FPU. Maybe that changed (thanks to backports)? Btw: Which version are we talking about right now? 6.1? 6.12? > > Jan > > > > > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > > > --- > > > arch/arm64/kernel/fpsimd.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c > > > index 83b8492984982..ba804fcdfdb3f 100644 > > > --- a/arch/arm64/kernel/fpsimd.c > > > +++ b/arch/arm64/kernel/fpsimd.c > > > @@ -1951,10 +1951,10 @@ void fpsimd_save_and_flush_cpu_state(void) > > > if (!system_supports_fpsimd()) > > > return; > > > WARN_ON(!hard_irqs_disabled() && preemptible()); > > > - __get_cpu_fpsimd_context(); > > > + get_cpu_fpsimd_context(); > > > __fpsimd_save(); > > > fpsimd_flush_cpu_state(); > > > - __put_cpu_fpsimd_context(); > > > + put_cpu_fpsimd_context(); > > > } > > > > > > #ifdef CONFIG_KERNEL_MODE_NEON > > > > > -- > Siemens AG, Foundational Technologies > Linux Expert Center ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dovetail 6.1.y-cip][PATCH 1/3] arm64: fpsimd: dovetail: Hard-disable interrupts inside fpsimd_save_and_flush_cpu_state 2025-09-25 12:47 ` Florian Bezdeka @ 2025-09-25 12:49 ` Jan Kiszka 2025-09-25 12:57 ` Florian Bezdeka 0 siblings, 1 reply; 23+ messages in thread From: Jan Kiszka @ 2025-09-25 12:49 UTC (permalink / raw) To: Florian Bezdeka, Philippe Gerum; +Cc: xenomai On 25.09.25 14:47, Florian Bezdeka wrote: > On Thu, 2025-09-25 at 14:40 +0200, Jan Kiszka wrote: >> On 23.09.25 20:51, Philippe Gerum wrote: >>> Jan Kiszka <jan.kiszka@siemens.com> writes: >>> >>>> From: Jan Kiszka <jan.kiszka@siemens.com> >>>> >>>> Not all callers of this function do that themselves. >>>> >>> >>> Are we 100% sure that kvm_arch_vcpu_load_fp() and kvm_arch_vcpu_put_fp() >>> would not benefit from doing that themselves precisely, to create an >>> atomic section around their internal state change _and_ the fpsimd flush >>> op? >> >> Is kvm on arm64 officially enabled for oob (-preemption), or are we >> patching blindly here? If no one was looking at kvm yet, then you might >> be right. If it was reviewed already, then this here is just cleaning up. > > We have > https://gitlab.com/Xenomai/linux-dovetail/-/commit/028ad3b2c19ca8a3655577fb34cac7996f166d2a > > since 6.14 (AFAICT). At the time of writing this was the first kernel > release that used in-kernel FPU. Maybe that changed (thanks to > backports)? Btw: Which version are we talking about right now? 6.1? > 6.12? All of them. Jan -- Siemens AG, Foundational Technologies Linux Expert Center ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dovetail 6.1.y-cip][PATCH 1/3] arm64: fpsimd: dovetail: Hard-disable interrupts inside fpsimd_save_and_flush_cpu_state 2025-09-25 12:49 ` Jan Kiszka @ 2025-09-25 12:57 ` Florian Bezdeka 2025-09-25 13:00 ` Jan Kiszka 2025-09-27 17:53 ` Philippe Gerum 0 siblings, 2 replies; 23+ messages in thread From: Florian Bezdeka @ 2025-09-25 12:57 UTC (permalink / raw) To: Jan Kiszka, Philippe Gerum; +Cc: xenomai On Thu, 2025-09-25 at 14:49 +0200, Jan Kiszka wrote: > On 25.09.25 14:47, Florian Bezdeka wrote: > > On Thu, 2025-09-25 at 14:40 +0200, Jan Kiszka wrote: > > > On 23.09.25 20:51, Philippe Gerum wrote: > > > > Jan Kiszka <jan.kiszka@siemens.com> writes: > > > > > > > > > From: Jan Kiszka <jan.kiszka@siemens.com> > > > > > > > > > > Not all callers of this function do that themselves. > > > > > > > > > > > > > Are we 100% sure that kvm_arch_vcpu_load_fp() and kvm_arch_vcpu_put_fp() > > > > would not benefit from doing that themselves precisely, to create an > > > > atomic section around their internal state change _and_ the fpsimd flush > > > > op? > > > > > > Is kvm on arm64 officially enabled for oob (-preemption), or are we > > > patching blindly here? If no one was looking at kvm yet, then you might > > > be right. If it was reviewed already, then this here is just cleaning up. > > > > We have > > https://gitlab.com/Xenomai/linux-dovetail/-/commit/028ad3b2c19ca8a3655577fb34cac7996f166d2a > > > > since 6.14 (AFAICT). At the time of writing this was the first kernel > > release that used in-kernel FPU. Maybe that changed (thanks to > > backports)? Btw: Which version are we talking about right now? 6.1? > > 6.12? > > All of them. > In case backporting is considered: Please also take 97cb63af01de ("arm64: fpsimd: dovetail: Align with x86 implementation") into account. Just a "couple of weeks" old and brings some optimizations. > > -- > Siemens AG, Foundational Technologies > Linux Expert Center ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dovetail 6.1.y-cip][PATCH 1/3] arm64: fpsimd: dovetail: Hard-disable interrupts inside fpsimd_save_and_flush_cpu_state 2025-09-25 12:57 ` Florian Bezdeka @ 2025-09-25 13:00 ` Jan Kiszka 2025-09-25 13:03 ` Florian Bezdeka 2025-09-27 17:53 ` Philippe Gerum 1 sibling, 1 reply; 23+ messages in thread From: Jan Kiszka @ 2025-09-25 13:00 UTC (permalink / raw) To: Florian Bezdeka, Philippe Gerum; +Cc: xenomai On 25.09.25 14:57, Florian Bezdeka wrote: > On Thu, 2025-09-25 at 14:49 +0200, Jan Kiszka wrote: >> On 25.09.25 14:47, Florian Bezdeka wrote: >>> On Thu, 2025-09-25 at 14:40 +0200, Jan Kiszka wrote: >>>> On 23.09.25 20:51, Philippe Gerum wrote: >>>>> Jan Kiszka <jan.kiszka@siemens.com> writes: >>>>> >>>>>> From: Jan Kiszka <jan.kiszka@siemens.com> >>>>>> >>>>>> Not all callers of this function do that themselves. >>>>>> >>>>> >>>>> Are we 100% sure that kvm_arch_vcpu_load_fp() and kvm_arch_vcpu_put_fp() >>>>> would not benefit from doing that themselves precisely, to create an >>>>> atomic section around their internal state change _and_ the fpsimd flush >>>>> op? >>>> >>>> Is kvm on arm64 officially enabled for oob (-preemption), or are we >>>> patching blindly here? If no one was looking at kvm yet, then you might >>>> be right. If it was reviewed already, then this here is just cleaning up. >>> >>> We have >>> https://gitlab.com/Xenomai/linux-dovetail/-/commit/028ad3b2c19ca8a3655577fb34cac7996f166d2a >>> >>> since 6.14 (AFAICT). At the time of writing this was the first kernel >>> release that used in-kernel FPU. Maybe that changed (thanks to >>> backports)? Btw: Which version are we talking about right now? 6.1? >>> 6.12? >> >> All of them. >> > > In case backporting is considered: > > Please also take > > 97cb63af01de ("arm64: fpsimd: dovetail: Align with x86 implementation") > No signed-off - still testing? > into account. Just a "couple of weeks" old and brings some > optimizations. > That goes to Philippe then. Jan -- Siemens AG, Foundational Technologies Linux Expert Center ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dovetail 6.1.y-cip][PATCH 1/3] arm64: fpsimd: dovetail: Hard-disable interrupts inside fpsimd_save_and_flush_cpu_state 2025-09-25 13:00 ` Jan Kiszka @ 2025-09-25 13:03 ` Florian Bezdeka 2025-09-25 13:05 ` Jan Kiszka 0 siblings, 1 reply; 23+ messages in thread From: Florian Bezdeka @ 2025-09-25 13:03 UTC (permalink / raw) To: Jan Kiszka, Philippe Gerum; +Cc: xenomai On Thu, 2025-09-25 at 15:00 +0200, Jan Kiszka wrote: > On 25.09.25 14:57, Florian Bezdeka wrote: > > On Thu, 2025-09-25 at 14:49 +0200, Jan Kiszka wrote: > > > On 25.09.25 14:47, Florian Bezdeka wrote: > > > > On Thu, 2025-09-25 at 14:40 +0200, Jan Kiszka wrote: > > > > > On 23.09.25 20:51, Philippe Gerum wrote: > > > > > > Jan Kiszka <jan.kiszka@siemens.com> writes: > > > > > > > > > > > > > From: Jan Kiszka <jan.kiszka@siemens.com> > > > > > > > > > > > > > > Not all callers of this function do that themselves. > > > > > > > > > > > > > > > > > > > Are we 100% sure that kvm_arch_vcpu_load_fp() and kvm_arch_vcpu_put_fp() > > > > > > would not benefit from doing that themselves precisely, to create an > > > > > > atomic section around their internal state change _and_ the fpsimd flush > > > > > > op? > > > > > > > > > > Is kvm on arm64 officially enabled for oob (-preemption), or are we > > > > > patching blindly here? If no one was looking at kvm yet, then you might > > > > > be right. If it was reviewed already, then this here is just cleaning up. > > > > > > > > We have > > > > https://gitlab.com/Xenomai/linux-dovetail/-/commit/028ad3b2c19ca8a3655577fb34cac7996f166d2a > > > > > > > > since 6.14 (AFAICT). At the time of writing this was the first kernel > > > > release that used in-kernel FPU. Maybe that changed (thanks to > > > > backports)? Btw: Which version are we talking about right now? 6.1? > > > > 6.12? > > > > > > All of them. > > > > > > > In case backporting is considered: > > > > Please also take > > > > 97cb63af01de ("arm64: fpsimd: dovetail: Align with x86 implementation") > > > > No signed-off - still testing? No, just realized that as well. Must be a glitch somewhere in my workflow. No idea how that happened. All other patches in the same series were fine... > > > into account. Just a "couple of weeks" old and brings some > > optimizations. > > > > That goes to Philippe then. Would be nice to know upfront if that would fix the actual problem... > > Jan > > -- > Siemens AG, Foundational Technologies > Linux Expert Center ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dovetail 6.1.y-cip][PATCH 1/3] arm64: fpsimd: dovetail: Hard-disable interrupts inside fpsimd_save_and_flush_cpu_state 2025-09-25 13:03 ` Florian Bezdeka @ 2025-09-25 13:05 ` Jan Kiszka 0 siblings, 0 replies; 23+ messages in thread From: Jan Kiszka @ 2025-09-25 13:05 UTC (permalink / raw) To: Florian Bezdeka, Philippe Gerum; +Cc: xenomai On 25.09.25 15:03, Florian Bezdeka wrote: > On Thu, 2025-09-25 at 15:00 +0200, Jan Kiszka wrote: >> On 25.09.25 14:57, Florian Bezdeka wrote: >>> On Thu, 2025-09-25 at 14:49 +0200, Jan Kiszka wrote: >>>> On 25.09.25 14:47, Florian Bezdeka wrote: >>>>> On Thu, 2025-09-25 at 14:40 +0200, Jan Kiszka wrote: >>>>>> On 23.09.25 20:51, Philippe Gerum wrote: >>>>>>> Jan Kiszka <jan.kiszka@siemens.com> writes: >>>>>>> >>>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com> >>>>>>>> >>>>>>>> Not all callers of this function do that themselves. >>>>>>>> >>>>>>> >>>>>>> Are we 100% sure that kvm_arch_vcpu_load_fp() and kvm_arch_vcpu_put_fp() >>>>>>> would not benefit from doing that themselves precisely, to create an >>>>>>> atomic section around their internal state change _and_ the fpsimd flush >>>>>>> op? >>>>>> >>>>>> Is kvm on arm64 officially enabled for oob (-preemption), or are we >>>>>> patching blindly here? If no one was looking at kvm yet, then you might >>>>>> be right. If it was reviewed already, then this here is just cleaning up. >>>>> >>>>> We have >>>>> https://gitlab.com/Xenomai/linux-dovetail/-/commit/028ad3b2c19ca8a3655577fb34cac7996f166d2a >>>>> >>>>> since 6.14 (AFAICT). At the time of writing this was the first kernel >>>>> release that used in-kernel FPU. Maybe that changed (thanks to >>>>> backports)? Btw: Which version are we talking about right now? 6.1? >>>>> 6.12? >>>> >>>> All of them. >>>> >>> >>> In case backporting is considered: >>> >>> Please also take >>> >>> 97cb63af01de ("arm64: fpsimd: dovetail: Align with x86 implementation") >>> >> >> No signed-off - still testing? > > No, just realized that as well. Must be a glitch somewhere in my > workflow. No idea how that happened. All other patches in the same > series were fine... > >> >>> into account. Just a "couple of weeks" old and brings some >>> optimizations. >>> >> >> That goes to Philippe then. > > Would be nice to know upfront if that would fix the actual problem... > Which problem exactly now? I'm currently juggling so many... Jan -- Siemens AG, Foundational Technologies Linux Expert Center ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dovetail 6.1.y-cip][PATCH 1/3] arm64: fpsimd: dovetail: Hard-disable interrupts inside fpsimd_save_and_flush_cpu_state 2025-09-25 12:57 ` Florian Bezdeka 2025-09-25 13:00 ` Jan Kiszka @ 2025-09-27 17:53 ` Philippe Gerum 2025-09-27 18:54 ` Florian Bezdeka 1 sibling, 1 reply; 23+ messages in thread From: Philippe Gerum @ 2025-09-27 17:53 UTC (permalink / raw) To: Florian Bezdeka; +Cc: Jan Kiszka, xenomai Florian Bezdeka <florian.bezdeka@siemens.com> writes: > On Thu, 2025-09-25 at 14:49 +0200, Jan Kiszka wrote: >> On 25.09.25 14:47, Florian Bezdeka wrote: >> > On Thu, 2025-09-25 at 14:40 +0200, Jan Kiszka wrote: >> > > On 23.09.25 20:51, Philippe Gerum wrote: >> > > > Jan Kiszka <jan.kiszka@siemens.com> writes: >> > > > >> > > > > From: Jan Kiszka <jan.kiszka@siemens.com> >> > > > > >> > > > > Not all callers of this function do that themselves. >> > > > > >> > > > >> > > > Are we 100% sure that kvm_arch_vcpu_load_fp() and kvm_arch_vcpu_put_fp() >> > > > would not benefit from doing that themselves precisely, to create an >> > > > atomic section around their internal state change _and_ the fpsimd flush >> > > > op? >> > > >> > > Is kvm on arm64 officially enabled for oob (-preemption), or are we >> > > patching blindly here? If no one was looking at kvm yet, then you might >> > > be right. If it was reviewed already, then this here is just cleaning up. >> > >> > We have >> > https://gitlab.com/Xenomai/linux-dovetail/-/commit/028ad3b2c19ca8a3655577fb34cac7996f166d2a >> > >> > since 6.14 (AFAICT). At the time of writing this was the first kernel >> > release that used in-kernel FPU. Maybe that changed (thanks to >> > backports)? Btw: Which version are we talking about right now? 6.1? >> > 6.12? >> >> All of them. >> > > In case backporting is considered: > We already have 028ad3b2 in [rebase/]6.1.y-cip, I'll pick 97cb63af01de as well. > Please also take > > 97cb63af01de ("arm64: fpsimd: dovetail: Align with x86 implementation") > > into account. Just a "couple of weeks" old and brings some > optimizations. > > We also need to enable the oob preemption test in switchtest, which was disabled so far due to lack of support. Now that 028ad3b2 enables safe preemption of kernel_neon_begin/end sections by oob thread, we should allow testing this feature in v3 as well with something along these lines: diff --git a/kernel/cobalt/arch/arm64/include/asm/xenomai/fptest.h b/kernel/cobalt/arch/arm64/include/asm/xenomai/fptest.h index 0958d5e2a..b8c0c39c5 100644 --- a/kernel/cobalt/arch/arm64/include/asm/xenomai/fptest.h +++ b/kernel/cobalt/arch/arm64/include/asm/xenomai/fptest.h @@ -10,16 +10,20 @@ #include <linux/errno.h> #include <asm/xenomai/uapi/fptest.h> #include <asm/hwcap.h> +#include <asm/neon.h> #define have_fp (ELF_HWCAP & HWCAP_FP) static inline int fp_linux_begin(void) { - return -ENOSYS; + kernel_neon_begin(); + + return 0; } static inline void fp_linux_end(void) { + kernel_neon_end(); } static inline int fp_detect(void) This was just done in v4 by [1], no issue detected afterwards by the 'hectic' test (switchtest equivalent). [1] https://gitlab.com/Xenomai/xenomai4/linux-evl/-/commit/0396a52a9b3dc352ef10e6a9d2ed096718ef5d80 -- Philippe. ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [dovetail 6.1.y-cip][PATCH 1/3] arm64: fpsimd: dovetail: Hard-disable interrupts inside fpsimd_save_and_flush_cpu_state 2025-09-27 17:53 ` Philippe Gerum @ 2025-09-27 18:54 ` Florian Bezdeka 2025-10-08 13:35 ` Bezdeka, Florian 0 siblings, 1 reply; 23+ messages in thread From: Florian Bezdeka @ 2025-09-27 18:54 UTC (permalink / raw) To: Philippe Gerum; +Cc: Jan Kiszka, xenomai On Sat, 2025-09-27 at 19:53 +0200, Philippe Gerum wrote: > Florian Bezdeka <florian.bezdeka@siemens.com> writes: > > > On Thu, 2025-09-25 at 14:49 +0200, Jan Kiszka wrote: > > > On 25.09.25 14:47, Florian Bezdeka wrote: > > > > On Thu, 2025-09-25 at 14:40 +0200, Jan Kiszka wrote: > > > > > On 23.09.25 20:51, Philippe Gerum wrote: > > > > > > Jan Kiszka <jan.kiszka@siemens.com> writes: > > > > > > > > > > > > > From: Jan Kiszka <jan.kiszka@siemens.com> > > > > > > > > > > > > > > Not all callers of this function do that themselves. > > > > > > > > > > > > > > > > > > > Are we 100% sure that kvm_arch_vcpu_load_fp() and kvm_arch_vcpu_put_fp() > > > > > > would not benefit from doing that themselves precisely, to create an > > > > > > atomic section around their internal state change _and_ the fpsimd flush > > > > > > op? > > > > > > > > > > Is kvm on arm64 officially enabled for oob (-preemption), or are we > > > > > patching blindly here? If no one was looking at kvm yet, then you might > > > > > be right. If it was reviewed already, then this here is just cleaning up. > > > > > > > > We have > > > > https://gitlab.com/Xenomai/linux-dovetail/-/commit/028ad3b2c19ca8a3655577fb34cac7996f166d2a > > > > > > > > since 6.14 (AFAICT). At the time of writing this was the first kernel > > > > release that used in-kernel FPU. Maybe that changed (thanks to > > > > backports)? Btw: Which version are we talking about right now? 6.1? > > > > 6.12? > > > > > > All of them. > > > > > > > In case backporting is considered: > > > > We already have 028ad3b2 in [rebase/]6.1.y-cip, I'll pick 97cb63af01de > as well. > > > Please also take > > > > 97cb63af01de ("arm64: fpsimd: dovetail: Align with x86 implementation") > > > > into account. Just a "couple of weeks" old and brings some > > optimizations. > > > > > > We also need to enable the oob preemption test in switchtest, which was > disabled so far due to lack of support. Now that 028ad3b2 enables safe > preemption of kernel_neon_begin/end sections by oob thread, we should > allow testing this feature in v3 as well with something along these > lines: > > diff --git a/kernel/cobalt/arch/arm64/include/asm/xenomai/fptest.h b/kernel/cobalt/arch/arm64/include/asm/xenomai/fptest.h > index 0958d5e2a..b8c0c39c5 100644 > --- a/kernel/cobalt/arch/arm64/include/asm/xenomai/fptest.h > +++ b/kernel/cobalt/arch/arm64/include/asm/xenomai/fptest.h > @@ -10,16 +10,20 @@ > #include <linux/errno.h> > #include <asm/xenomai/uapi/fptest.h> > #include <asm/hwcap.h> > +#include <asm/neon.h> > > #define have_fp (ELF_HWCAP & HWCAP_FP) > > static inline int fp_linux_begin(void) > { > - return -ENOSYS; > + kernel_neon_begin(); > + > + return 0; > } > > static inline void fp_linux_end(void) > { > + kernel_neon_end(); > } > > static inline int fp_detect(void) > > This was just done in v4 by [1], no issue detected afterwards by the > 'hectic' test (switchtest equivalent). > > [1] https://gitlab.com/Xenomai/xenomai4/linux-evl/-/commit/0396a52a9b3dc352ef10e6a9d2ed096718ef5d80 Thanks for the reminder... I'm quite sure I had sent that to the list. Might have got forgotten or it's hiding successfully in my xenomai queue. I will check and take care of the test enablement. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dovetail 6.1.y-cip][PATCH 1/3] arm64: fpsimd: dovetail: Hard-disable interrupts inside fpsimd_save_and_flush_cpu_state 2025-09-27 18:54 ` Florian Bezdeka @ 2025-10-08 13:35 ` Bezdeka, Florian 2025-11-18 10:55 ` Florian Bezdeka 0 siblings, 1 reply; 23+ messages in thread From: Bezdeka, Florian @ 2025-10-08 13:35 UTC (permalink / raw) To: rpm@xenomai.org; +Cc: xenomai@xenomai.org, Kiszka, Jan On Sat, 2025-09-27 at 20:54 +0200, Florian Bezdeka wrote: > On Sat, 2025-09-27 at 19:53 +0200, Philippe Gerum wrote: > > Florian Bezdeka <florian.bezdeka@siemens.com> writes: > > > > > On Thu, 2025-09-25 at 14:49 +0200, Jan Kiszka wrote: > > > > On 25.09.25 14:47, Florian Bezdeka wrote: > > > > > On Thu, 2025-09-25 at 14:40 +0200, Jan Kiszka wrote: > > > > > > On 23.09.25 20:51, Philippe Gerum wrote: > > > > > > > Jan Kiszka <jan.kiszka@siemens.com> writes: > > > > > > > > > > > > > > > From: Jan Kiszka <jan.kiszka@siemens.com> > > > > > > > > > > > > > > > > Not all callers of this function do that themselves. > > > > > > > > > > > > > > > > > > > > > > Are we 100% sure that kvm_arch_vcpu_load_fp() and kvm_arch_vcpu_put_fp() > > > > > > > would not benefit from doing that themselves precisely, to create an > > > > > > > atomic section around their internal state change _and_ the fpsimd flush > > > > > > > op? > > > > > > > > > > > > Is kvm on arm64 officially enabled for oob (-preemption), or are we > > > > > > patching blindly here? If no one was looking at kvm yet, then you might > > > > > > be right. If it was reviewed already, then this here is just cleaning up. > > > > > > > > > > We have > > > > > https://gitlab.com/Xenomai/linux-dovetail/-/commit/028ad3b2c19ca8a3655577fb34cac7996f166d2a > > > > > > > > > > since 6.14 (AFAICT). At the time of writing this was the first kernel > > > > > release that used in-kernel FPU. Maybe that changed (thanks to > > > > > backports)? Btw: Which version are we talking about right now? 6.1? > > > > > 6.12? > > > > > > > > All of them. > > > > > > > > > > In case backporting is considered: > > > > > > > We already have 028ad3b2 in [rebase/]6.1.y-cip, I'll pick 97cb63af01de > > as well. > > > > > Please also take > > > > > > 97cb63af01de ("arm64: fpsimd: dovetail: Align with x86 implementation") > > > > > > into account. Just a "couple of weeks" old and brings some > > > optimizations. > > > > > > > > > > We also need to enable the oob preemption test in switchtest, which was > > disabled so far due to lack of support. Now that 028ad3b2 enables safe > > preemption of kernel_neon_begin/end sections by oob thread, we should > > allow testing this feature in v3 as well with something along these > > lines: > > > > diff --git a/kernel/cobalt/arch/arm64/include/asm/xenomai/fptest.h b/kernel/cobalt/arch/arm64/include/asm/xenomai/fptest.h > > index 0958d5e2a..b8c0c39c5 100644 > > --- a/kernel/cobalt/arch/arm64/include/asm/xenomai/fptest.h > > +++ b/kernel/cobalt/arch/arm64/include/asm/xenomai/fptest.h > > @@ -10,16 +10,20 @@ > > #include <linux/errno.h> > > #include <asm/xenomai/uapi/fptest.h> > > #include <asm/hwcap.h> > > +#include <asm/neon.h> > > > > #define have_fp (ELF_HWCAP & HWCAP_FP) > > > > static inline int fp_linux_begin(void) > > { > > - return -ENOSYS; > > + kernel_neon_begin(); > > + > > + return 0; > > } > > > > static inline void fp_linux_end(void) > > { > > + kernel_neon_end(); > > } > > > > static inline int fp_detect(void) > > > > This was just done in v4 by [1], no issue detected afterwards by the > > 'hectic' test (switchtest equivalent). > > > > [1] https://gitlab.com/Xenomai/xenomai4/linux-evl/-/commit/0396a52a9b3dc352ef10e6a9d2ed096718ef5d80 > > Thanks for the reminder... > > I'm quite sure I had sent that to the list. Might have got forgotten or > it's hiding successfully in my xenomai queue. I will check and take > care of the test enablement. I did some tests with that in-kernel FPU usage enabled. My tests were based on xenomai/next with Kernels down to 5.10. Results: - 5.10: Missing backport - 5.15: Missing backport - 6.1 : Has the backport but that looks slightly different compared to 6.12. At least the following is needed to get the kernel into a "stable" state, but still no test success: diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c index 7ee823a6a5e45..be3d37554de0f 100644 --- a/arch/arm64/kernel/fpsimd.c +++ b/arch/arm64/kernel/fpsimd.c @@ -2216,6 +2216,7 @@ void fpsimd_suspend_inband(void) */ if (__this_cpu_read(fpsimd_context_busy)) { fpsimd_save_state(kfpu); + __put_cpu_fpsimd_context(); set_thread_flag(TIF_KERNEL_FP_PREEMPTED); } } @@ -2224,8 +2225,11 @@ void fpsimd_resume_inband(void) { struct user_fpsimd_state *kfpu = this_cpu_ptr(&in_kernel_fpstate); - if (test_and_clear_thread_flag(TIF_KERNEL_FP_PREEMPTED)) + if (test_and_clear_thread_flag(TIF_KERNEL_FP_PREEMPTED)) { fpsimd_load_state(kfpu); + __get_cpu_fpsimd_context(); + } + } #endif - 6.12 and newer: OK Btw: I'm on vacation for ~2 weeks now. So please don't expect any progress on that topic ;-) Florian ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [dovetail 6.1.y-cip][PATCH 1/3] arm64: fpsimd: dovetail: Hard-disable interrupts inside fpsimd_save_and_flush_cpu_state 2025-10-08 13:35 ` Bezdeka, Florian @ 2025-11-18 10:55 ` Florian Bezdeka 2025-11-18 12:29 ` Jan Kiszka 0 siblings, 1 reply; 23+ messages in thread From: Florian Bezdeka @ 2025-11-18 10:55 UTC (permalink / raw) To: rpm@xenomai.org; +Cc: xenomai@xenomai.org, Kiszka, Jan On Wed, 2025-10-08 at 13:35 +0000, Bezdeka, Florian wrote: > > > lines: > > > > > > diff --git a/kernel/cobalt/arch/arm64/include/asm/xenomai/fptest.h b/kernel/cobalt/arch/arm64/include/asm/xenomai/fptest.h > > > index 0958d5e2a..b8c0c39c5 100644 > > > --- a/kernel/cobalt/arch/arm64/include/asm/xenomai/fptest.h > > > +++ b/kernel/cobalt/arch/arm64/include/asm/xenomai/fptest.h > > > @@ -10,16 +10,20 @@ > > > #include <linux/errno.h> > > > #include <asm/xenomai/uapi/fptest.h> > > > #include <asm/hwcap.h> > > > +#include <asm/neon.h> > > > > > > #define have_fp (ELF_HWCAP & HWCAP_FP) > > > > > > static inline int fp_linux_begin(void) > > > { > > > - return -ENOSYS; > > > + kernel_neon_begin(); > > > + > > > + return 0; > > > } > > > > > > static inline void fp_linux_end(void) > > > { > > > + kernel_neon_end(); > > > } > > > > > > static inline int fp_detect(void) > > > > > > This was just done in v4 by [1], no issue detected afterwards by the > > > 'hectic' test (switchtest equivalent). > > > > > > [1] https://gitlab.com/Xenomai/xenomai4/linux-evl/-/commit/0396a52a9b3dc352ef10e6a9d2ed096718ef5d80 > > > > Thanks for the reminder... > > > > I'm quite sure I had sent that to the list. Might have got forgotten or > > it's hiding successfully in my xenomai queue. I will check and take > > care of the test enablement. > > I did some tests with that in-kernel FPU usage enabled. My tests were > based on xenomai/next with Kernels down to 5.10. > > Results: > - 5.10: Missing backport > - 5.15: Missing backport > - 6.1 : Has the backport but that looks slightly different compared to > 6.12. At least the following is needed to get the kernel into > a "stable" state, but still no test success: > > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c > index 7ee823a6a5e45..be3d37554de0f 100644 > --- a/arch/arm64/kernel/fpsimd.c > +++ b/arch/arm64/kernel/fpsimd.c > @@ -2216,6 +2216,7 @@ void fpsimd_suspend_inband(void) > */ > if (__this_cpu_read(fpsimd_context_busy)) { > fpsimd_save_state(kfpu); > + __put_cpu_fpsimd_context(); > set_thread_flag(TIF_KERNEL_FP_PREEMPTED); > } > } > @@ -2224,8 +2225,11 @@ void fpsimd_resume_inband(void) > { > struct user_fpsimd_state *kfpu = > this_cpu_ptr(&in_kernel_fpstate); > > - if (test_and_clear_thread_flag(TIF_KERNEL_FP_PREEMPTED)) > + if (test_and_clear_thread_flag(TIF_KERNEL_FP_PREEMPTED)) { > fpsimd_load_state(kfpu); > + __get_cpu_fpsimd_context(); > + } > + > } > > #endif > > - 6.12 and newer: OK > > > Btw: I'm on vacation for ~2 weeks now. So please don't expect any > progress on that topic ;-) > I finally found some time to debug that issue again. To recap: As soon as I enable the in-kernel oob FPU usage (see patch at the beginning of this mail) the xenomai3 switchtest utility will fail when running ontop of Dovetail < 6.12. The winner is aefbab8e77eb ("arm64: fpsimd: Preserve/restore kernel mode NEON at context switch") which is part of 6.8 and newer. That explains why Dovetail versions below 6.12 show this problem. It allows to preempt in-kernel FPU usage sections by implementing the necessary context save/restore. My summary for now is that the hard_preempt_disable() in kernel_neon_begin() - as part of get_cpu_fpsimd_context() - does not work as it should. Seems we need a fix for Dovetail < 6.12. Could someone please have a look and share some thoughts? Confirming my assumption / theory would be a nice starting point for me. Possible workaround: I could implement a version specific test enablement, but I'm not sure if that is papering over a real problem. The FPU usage in such a context looks valid to me. Best regards, Florian ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dovetail 6.1.y-cip][PATCH 1/3] arm64: fpsimd: dovetail: Hard-disable interrupts inside fpsimd_save_and_flush_cpu_state 2025-11-18 10:55 ` Florian Bezdeka @ 2025-11-18 12:29 ` Jan Kiszka 0 siblings, 0 replies; 23+ messages in thread From: Jan Kiszka @ 2025-11-18 12:29 UTC (permalink / raw) To: Florian Bezdeka, rpm@xenomai.org; +Cc: xenomai@xenomai.org On 18.11.25 11:55, Florian Bezdeka wrote: > On Wed, 2025-10-08 at 13:35 +0000, Bezdeka, Florian wrote: >>>> lines: >>>> >>>> diff --git a/kernel/cobalt/arch/arm64/include/asm/xenomai/fptest.h b/kernel/cobalt/arch/arm64/include/asm/xenomai/fptest.h >>>> index 0958d5e2a..b8c0c39c5 100644 >>>> --- a/kernel/cobalt/arch/arm64/include/asm/xenomai/fptest.h >>>> +++ b/kernel/cobalt/arch/arm64/include/asm/xenomai/fptest.h >>>> @@ -10,16 +10,20 @@ >>>> #include <linux/errno.h> >>>> #include <asm/xenomai/uapi/fptest.h> >>>> #include <asm/hwcap.h> >>>> +#include <asm/neon.h> >>>> >>>> #define have_fp (ELF_HWCAP & HWCAP_FP) >>>> >>>> static inline int fp_linux_begin(void) >>>> { >>>> - return -ENOSYS; >>>> + kernel_neon_begin(); >>>> + >>>> + return 0; >>>> } >>>> >>>> static inline void fp_linux_end(void) >>>> { >>>> + kernel_neon_end(); >>>> } >>>> >>>> static inline int fp_detect(void) >>>> >>>> This was just done in v4 by [1], no issue detected afterwards by the >>>> 'hectic' test (switchtest equivalent). >>>> >>>> [1] https://gitlab.com/Xenomai/xenomai4/linux-evl/-/commit/0396a52a9b3dc352ef10e6a9d2ed096718ef5d80 >>> >>> Thanks for the reminder... >>> >>> I'm quite sure I had sent that to the list. Might have got forgotten or >>> it's hiding successfully in my xenomai queue. I will check and take >>> care of the test enablement. >> >> I did some tests with that in-kernel FPU usage enabled. My tests were >> based on xenomai/next with Kernels down to 5.10. >> >> Results: >> - 5.10: Missing backport >> - 5.15: Missing backport >> - 6.1 : Has the backport but that looks slightly different compared to >> 6.12. At least the following is needed to get the kernel into >> a "stable" state, but still no test success: >> >> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c >> index 7ee823a6a5e45..be3d37554de0f 100644 >> --- a/arch/arm64/kernel/fpsimd.c >> +++ b/arch/arm64/kernel/fpsimd.c >> @@ -2216,6 +2216,7 @@ void fpsimd_suspend_inband(void) >> */ >> if (__this_cpu_read(fpsimd_context_busy)) { >> fpsimd_save_state(kfpu); >> + __put_cpu_fpsimd_context(); >> set_thread_flag(TIF_KERNEL_FP_PREEMPTED); >> } >> } >> @@ -2224,8 +2225,11 @@ void fpsimd_resume_inband(void) >> { >> struct user_fpsimd_state *kfpu = >> this_cpu_ptr(&in_kernel_fpstate); >> >> - if (test_and_clear_thread_flag(TIF_KERNEL_FP_PREEMPTED)) >> + if (test_and_clear_thread_flag(TIF_KERNEL_FP_PREEMPTED)) { >> fpsimd_load_state(kfpu); >> + __get_cpu_fpsimd_context(); >> + } >> + >> } >> >> #endif >> >> - 6.12 and newer: OK >> >> >> Btw: I'm on vacation for ~2 weeks now. So please don't expect any >> progress on that topic ;-) >> > > I finally found some time to debug that issue again. To recap: As soon > as I enable the in-kernel oob FPU usage (see patch at the beginning of > this mail) the xenomai3 switchtest utility will fail when running ontop > of Dovetail < 6.12. > > The winner is > > aefbab8e77eb ("arm64: fpsimd: Preserve/restore kernel mode NEON at context switch") > > which is part of 6.8 and newer. That explains why Dovetail versions > below 6.12 show this problem. It allows to preempt in-kernel FPU usage > sections by implementing the necessary context save/restore. > > My summary for now is that the hard_preempt_disable() in > kernel_neon_begin() - as part of get_cpu_fpsimd_context() - does not > work as it should. Seems we need a fix for Dovetail < 6.12. It is not hard_preempt as there is a hard_cond_local_irq_restore at its end, and that lifts all the preempt disabling we have in oob from dovetail perspective: irqs off. > > Could someone please have a look and share some thoughts? Confirming my > assumption / theory would be a nice starting point for me. > > Possible workaround: I could implement a version specific test > enablement, but I'm not sure if that is papering over a real problem. > The FPU usage in such a context looks valid to me. > We should rather look into backporting that upstream commit, I would say. Or does it have tons of dependencies? Jan -- Siemens AG, Foundational Technologies Linux Expert Center ^ permalink raw reply [flat|nested] 23+ messages in thread
* [dovetail 6.1.y-cip][PATCH 2/3] arm64: fpsimd: dovetail: drop redundant hard irqs disabling 2025-09-19 6:55 [dovetail 6.1.y-cip][PATCH 0/3] fpsimd fixes and cleanups Jan Kiszka 2025-09-19 6:55 ` [dovetail 6.1.y-cip][PATCH 1/3] arm64: fpsimd: dovetail: Hard-disable interrupts inside fpsimd_save_and_flush_cpu_state Jan Kiszka @ 2025-09-19 6:55 ` Jan Kiszka 2025-10-27 16:01 ` Jan Kiszka 2025-09-19 6:55 ` [dovetail 6.1.y-cip][PATCH 3/3] arm64: fpsimd: dovetail: Drop extra condition from WARN_ON Jan Kiszka 2 siblings, 1 reply; 23+ messages in thread From: Jan Kiszka @ 2025-09-19 6:55 UTC (permalink / raw) To: Philippe Gerum, xenomai From: Jan Kiszka <jan.kiszka@siemens.com> fpsimd_save_and_flush_cpu_state() itself does take care of this. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- arch/arm64/kernel/fpsimd.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c index ba804fcdfdb3f..f915706332191 100644 --- a/arch/arm64/kernel/fpsimd.c +++ b/arch/arm64/kernel/fpsimd.c @@ -2148,13 +2148,9 @@ void __efi_fpsimd_end(void) static int fpsimd_cpu_pm_notifier(struct notifier_block *self, unsigned long cmd, void *v) { - unsigned long flags; - switch (cmd) { case CPU_PM_ENTER: - flags = hard_cond_local_irq_save(); fpsimd_save_and_flush_cpu_state(); - hard_cond_local_irq_restore(flags); break; case CPU_PM_EXIT: break; -- 2.51.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [dovetail 6.1.y-cip][PATCH 2/3] arm64: fpsimd: dovetail: drop redundant hard irqs disabling 2025-09-19 6:55 ` [dovetail 6.1.y-cip][PATCH 2/3] arm64: fpsimd: dovetail: drop redundant hard irqs disabling Jan Kiszka @ 2025-10-27 16:01 ` Jan Kiszka 2025-10-27 16:07 ` Philippe Gerum 0 siblings, 1 reply; 23+ messages in thread From: Jan Kiszka @ 2025-10-27 16:01 UTC (permalink / raw) To: Philippe Gerum, xenomai On 19.09.25 08:55, Jan Kiszka wrote: > From: Jan Kiszka <jan.kiszka@siemens.com> > > fpsimd_save_and_flush_cpu_state() itself does take care of this. > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > arch/arm64/kernel/fpsimd.c | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c > index ba804fcdfdb3f..f915706332191 100644 > --- a/arch/arm64/kernel/fpsimd.c > +++ b/arch/arm64/kernel/fpsimd.c > @@ -2148,13 +2148,9 @@ void __efi_fpsimd_end(void) > static int fpsimd_cpu_pm_notifier(struct notifier_block *self, > unsigned long cmd, void *v) > { > - unsigned long flags; > - > switch (cmd) { > case CPU_PM_ENTER: > - flags = hard_cond_local_irq_save(); > fpsimd_save_and_flush_cpu_state(); > - hard_cond_local_irq_restore(flags); > break; > case CPU_PM_EXIT: > break; And this one for 6.1 should also be merged. The changes in patch 1 and 3 found their way into v6.1.y-cip-dovetail-rebase in the meantime, as it looks like. BTW, a new release for 6.1 would be good as well then, to get all of this tagged. Thanks Jan -- Siemens AG, Foundational Technologies Linux Expert Center ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dovetail 6.1.y-cip][PATCH 2/3] arm64: fpsimd: dovetail: drop redundant hard irqs disabling 2025-10-27 16:01 ` Jan Kiszka @ 2025-10-27 16:07 ` Philippe Gerum 0 siblings, 0 replies; 23+ messages in thread From: Philippe Gerum @ 2025-10-27 16:07 UTC (permalink / raw) To: Jan Kiszka; +Cc: xenomai Jan Kiszka <jan.kiszka@siemens.com> writes: > On 19.09.25 08:55, Jan Kiszka wrote: >> From: Jan Kiszka <jan.kiszka@siemens.com> >> >> fpsimd_save_and_flush_cpu_state() itself does take care of this. >> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >> --- >> arch/arm64/kernel/fpsimd.c | 4 ---- >> 1 file changed, 4 deletions(-) >> >> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c >> index ba804fcdfdb3f..f915706332191 100644 >> --- a/arch/arm64/kernel/fpsimd.c >> +++ b/arch/arm64/kernel/fpsimd.c >> @@ -2148,13 +2148,9 @@ void __efi_fpsimd_end(void) >> static int fpsimd_cpu_pm_notifier(struct notifier_block *self, >> unsigned long cmd, void *v) >> { >> - unsigned long flags; >> - >> switch (cmd) { >> case CPU_PM_ENTER: >> - flags = hard_cond_local_irq_save(); >> fpsimd_save_and_flush_cpu_state(); >> - hard_cond_local_irq_restore(flags); >> break; >> case CPU_PM_EXIT: >> break; > > And this one for 6.1 should also be merged. The changes in patch 1 and 3 > found their way into v6.1.y-cip-dovetail-rebase in the meantime, as it > looks like. > > BTW, a new release for 6.1 would be good as well then, to get all of > this tagged. > There is a set of fixes for the oob network stack coming shortly, rebase over stable is planned right after (Wednesday). -- Philippe. ^ permalink raw reply [flat|nested] 23+ messages in thread
* [dovetail 6.1.y-cip][PATCH 3/3] arm64: fpsimd: dovetail: Drop extra condition from WARN_ON 2025-09-19 6:55 [dovetail 6.1.y-cip][PATCH 0/3] fpsimd fixes and cleanups Jan Kiszka 2025-09-19 6:55 ` [dovetail 6.1.y-cip][PATCH 1/3] arm64: fpsimd: dovetail: Hard-disable interrupts inside fpsimd_save_and_flush_cpu_state Jan Kiszka 2025-09-19 6:55 ` [dovetail 6.1.y-cip][PATCH 2/3] arm64: fpsimd: dovetail: drop redundant hard irqs disabling Jan Kiszka @ 2025-09-19 6:55 ` Jan Kiszka 2025-09-23 18:52 ` Philippe Gerum 2 siblings, 1 reply; 23+ messages in thread From: Jan Kiszka @ 2025-09-19 6:55 UTC (permalink / raw) To: Philippe Gerum, xenomai From: Jan Kiszka <jan.kiszka@siemens.com> There is no indication that this function is called over oob context as well, and newer dovetail versions do not carry this extra condition anymore. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- arch/arm64/kernel/fpsimd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c index f915706332191..ed269edea7907 100644 --- a/arch/arm64/kernel/fpsimd.c +++ b/arch/arm64/kernel/fpsimd.c @@ -1950,7 +1950,7 @@ void fpsimd_save_and_flush_cpu_state(void) { if (!system_supports_fpsimd()) return; - WARN_ON(!hard_irqs_disabled() && preemptible()); + WARN_ON(preemptible()); get_cpu_fpsimd_context(); __fpsimd_save(); fpsimd_flush_cpu_state(); -- 2.51.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [dovetail 6.1.y-cip][PATCH 3/3] arm64: fpsimd: dovetail: Drop extra condition from WARN_ON 2025-09-19 6:55 ` [dovetail 6.1.y-cip][PATCH 3/3] arm64: fpsimd: dovetail: Drop extra condition from WARN_ON Jan Kiszka @ 2025-09-23 18:52 ` Philippe Gerum 2025-09-23 19:02 ` Philippe Gerum 0 siblings, 1 reply; 23+ messages in thread From: Philippe Gerum @ 2025-09-23 18:52 UTC (permalink / raw) To: Jan Kiszka; +Cc: xenomai Jan Kiszka <jan.kiszka@siemens.com> writes: > From: Jan Kiszka <jan.kiszka@siemens.com> > > There is no indication that this function is called over oob context as > well, and newer dovetail versions do not carry this extra condition > anymore. > Depends on whether patch 2/3 is actually correct. > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > arch/arm64/kernel/fpsimd.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c > index f915706332191..ed269edea7907 100644 > --- a/arch/arm64/kernel/fpsimd.c > +++ b/arch/arm64/kernel/fpsimd.c > @@ -1950,7 +1950,7 @@ void fpsimd_save_and_flush_cpu_state(void) > { > if (!system_supports_fpsimd()) > return; > - WARN_ON(!hard_irqs_disabled() && preemptible()); > + WARN_ON(preemptible()); > get_cpu_fpsimd_context(); > __fpsimd_save(); > fpsimd_flush_cpu_state(); -- Philippe. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dovetail 6.1.y-cip][PATCH 3/3] arm64: fpsimd: dovetail: Drop extra condition from WARN_ON 2025-09-23 18:52 ` Philippe Gerum @ 2025-09-23 19:02 ` Philippe Gerum 0 siblings, 0 replies; 23+ messages in thread From: Philippe Gerum @ 2025-09-23 19:02 UTC (permalink / raw) To: Jan Kiszka; +Cc: xenomai Philippe Gerum <rpm@xenomai.org> writes: > Jan Kiszka <jan.kiszka@siemens.com> writes: > >> From: Jan Kiszka <jan.kiszka@siemens.com> >> >> There is no indication that this function is called over oob context as >> well, and newer dovetail versions do not carry this extra condition >> anymore. >> > > Depends on whether patch 2/3 is actually correct. 1/3, not 2/3. > >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >> --- >> arch/arm64/kernel/fpsimd.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c >> index f915706332191..ed269edea7907 100644 >> --- a/arch/arm64/kernel/fpsimd.c >> +++ b/arch/arm64/kernel/fpsimd.c >> @@ -1950,7 +1950,7 @@ void fpsimd_save_and_flush_cpu_state(void) >> { >> if (!system_supports_fpsimd()) >> return; >> - WARN_ON(!hard_irqs_disabled() && preemptible()); >> + WARN_ON(preemptible()); >> get_cpu_fpsimd_context(); >> __fpsimd_save(); >> fpsimd_flush_cpu_state(); -- Philippe. ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2025-11-18 12:29 UTC | newest] Thread overview: 23+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-09-19 6:55 [dovetail 6.1.y-cip][PATCH 0/3] fpsimd fixes and cleanups Jan Kiszka 2025-09-19 6:55 ` [dovetail 6.1.y-cip][PATCH 1/3] arm64: fpsimd: dovetail: Hard-disable interrupts inside fpsimd_save_and_flush_cpu_state Jan Kiszka 2025-09-19 7:18 ` Jan Kiszka 2025-09-19 9:20 ` Jan Kiszka 2025-09-23 18:51 ` Philippe Gerum 2025-09-25 12:40 ` Jan Kiszka 2025-09-25 12:47 ` Florian Bezdeka 2025-09-25 12:49 ` Jan Kiszka 2025-09-25 12:57 ` Florian Bezdeka 2025-09-25 13:00 ` Jan Kiszka 2025-09-25 13:03 ` Florian Bezdeka 2025-09-25 13:05 ` Jan Kiszka 2025-09-27 17:53 ` Philippe Gerum 2025-09-27 18:54 ` Florian Bezdeka 2025-10-08 13:35 ` Bezdeka, Florian 2025-11-18 10:55 ` Florian Bezdeka 2025-11-18 12:29 ` Jan Kiszka 2025-09-19 6:55 ` [dovetail 6.1.y-cip][PATCH 2/3] arm64: fpsimd: dovetail: drop redundant hard irqs disabling Jan Kiszka 2025-10-27 16:01 ` Jan Kiszka 2025-10-27 16:07 ` Philippe Gerum 2025-09-19 6:55 ` [dovetail 6.1.y-cip][PATCH 3/3] arm64: fpsimd: dovetail: Drop extra condition from WARN_ON Jan Kiszka 2025-09-23 18:52 ` Philippe Gerum 2025-09-23 19:02 ` Philippe Gerum
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.