* [PATCH v2] MIPS: Fix a preemption issue with thread's FPU defaults
@ 2015-05-12 14:20 Maciej W. Rozycki
2015-05-12 17:28 ` Ezequiel Garcia
2015-05-12 21:14 ` Ralf Baechle
0 siblings, 2 replies; 6+ messages in thread
From: Maciej W. Rozycki @ 2015-05-12 14:20 UTC (permalink / raw
To: Ralf Baechle, Paul Martin, Ezequiel Garcia
Cc: Markos Chandras, James Hogan, linux-mips
Fix "BUG: using smp_processor_id() in preemptible" reported in accesses
to thread's FPU defaults: the value to initialise FSCR to at program
startup, the FCSR r/w mask and the contents of FIR in full FPU
emulation, removing a regression introduced with 9b26616c [MIPS: Respect
the ISA level in FCSR handling] and f6843626 [MIPS: math-emu: Set FIR
feature flags for full emulation].
Use `boot_cpu_data' to obtain the data from, following the approach that
`cpu_has_*' macros take and avoiding the call to `smp_processor_id' made
in the reference to `current_cpu_data'. The contents of FSCR have to be
consistent across processors in an SMP system, the settings there must
not change as a thread is migrated across processors. And the contents
of FIR are guaranteed to be consistent in FPU emulation, by definition.
Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
---
Changes from v1:
- also fix PTRACE_SETFPREGS (thanks, Paul!).
Paul, Ezequiel --
Can you verify this change addresses the problems you saw?
[Ralf, this is another 4.1 material, please push it to Linus ASAP, once
this has been confirmed to DTRT.]
Thanks,
Maciej
linux-mips-emu-fcsr-isa-fix.diff
Index: linux-org-test/arch/mips/include/asm/elf.h
===================================================================
--- linux-org-test.orig/arch/mips/include/asm/elf.h 2015-05-06 23:20:51.946503000 +0100
+++ linux-org-test/arch/mips/include/asm/elf.h 2015-05-12 14:37:14.169350000 +0100
@@ -304,7 +304,7 @@ do { \
\
current->thread.abi = &mips_abi; \
\
- current->thread.fpu.fcr31 = current_cpu_data.fpu_csr31; \
+ current->thread.fpu.fcr31 = boot_cpu_data.fpu_csr31; \
} while (0)
#endif /* CONFIG_32BIT */
@@ -366,7 +366,7 @@ do { \
else \
current->thread.abi = &mips_abi; \
\
- current->thread.fpu.fcr31 = current_cpu_data.fpu_csr31; \
+ current->thread.fpu.fcr31 = boot_cpu_data.fpu_csr31; \
\
p = personality(current->personality); \
if (p != PER_LINUX32 && p != PER_LINUX) \
Index: linux-org-test/arch/mips/kernel/ptrace.c
===================================================================
--- linux-org-test.orig/arch/mips/kernel/ptrace.c 2015-04-28 14:52:04.000000000 +0100
+++ linux-org-test/arch/mips/kernel/ptrace.c 2015-05-12 15:12:03.511002000 +0100
@@ -176,7 +176,7 @@ int ptrace_setfpregs(struct task_struct
__get_user(value, data + 64);
fcr31 = child->thread.fpu.fcr31;
- mask = current_cpu_data.fpu_msk31;
+ mask = boot_cpu_data.fpu_msk31;
child->thread.fpu.fcr31 = (value & ~mask) | (fcr31 & mask);
/* FIR may not be written. */
Index: linux-org-test/arch/mips/math-emu/cp1emu.c
===================================================================
--- linux-org-test.orig/arch/mips/math-emu/cp1emu.c 2015-04-28 14:52:04.000000000 +0100
+++ linux-org-test/arch/mips/math-emu/cp1emu.c 2015-05-12 14:41:06.308256000 +0100
@@ -889,7 +889,7 @@ static inline void cop1_cfc(struct pt_re
break;
case FPCREG_RID:
- value = current_cpu_data.fpu_id;
+ value = boot_cpu_data.fpu_id;
break;
default:
@@ -921,7 +921,7 @@ static inline void cop1_ctc(struct pt_re
(void *)xcp->cp0_epc, MIPSInst_RT(ir), value);
/* Preserve read-only bits. */
- mask = current_cpu_data.fpu_msk31;
+ mask = boot_cpu_data.fpu_msk31;
fcr31 = (value & ~mask) | (fcr31 & mask);
break;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] MIPS: Fix a preemption issue with thread's FPU defaults
@ 2015-05-12 17:28 ` Ezequiel Garcia
0 siblings, 0 replies; 6+ messages in thread
From: Ezequiel Garcia @ 2015-05-12 17:28 UTC (permalink / raw
To: Maciej W. Rozycki, Ralf Baechle, Paul Martin
Cc: Markos Chandras, James Hogan, linux-mips
On 05/12/2015 11:20 AM, Maciej W. Rozycki wrote:
> Fix "BUG: using smp_processor_id() in preemptible" reported in accesses
> to thread's FPU defaults: the value to initialise FSCR to at program
> startup, the FCSR r/w mask and the contents of FIR in full FPU
> emulation, removing a regression introduced with 9b26616c [MIPS: Respect
> the ISA level in FCSR handling] and f6843626 [MIPS: math-emu: Set FIR
> feature flags for full emulation].
>
> Use `boot_cpu_data' to obtain the data from, following the approach that
> `cpu_has_*' macros take and avoiding the call to `smp_processor_id' made
> in the reference to `current_cpu_data'. The contents of FSCR have to be
> consistent across processors in an SMP system, the settings there must
> not change as a thread is migrated across processors. And the contents
> of FIR are guaranteed to be consistent in FPU emulation, by definition.
>
> Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
> ---
> Changes from v1:
>
> - also fix PTRACE_SETFPREGS (thanks, Paul!).
>
> Paul, Ezequiel --
>
> Can you verify this change addresses the problems you saw?
>
Tested-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
> [Ralf, this is another 4.1 material, please push it to Linus ASAP, once
> this has been confirmed to DTRT.]
>
> Thanks,
>
> Maciej
>
> linux-mips-emu-fcsr-isa-fix.diff
> Index: linux-org-test/arch/mips/include/asm/elf.h
> ===================================================================
> --- linux-org-test.orig/arch/mips/include/asm/elf.h 2015-05-06 23:20:51.946503000 +0100
> +++ linux-org-test/arch/mips/include/asm/elf.h 2015-05-12 14:37:14.169350000 +0100
> @@ -304,7 +304,7 @@ do { \
> \
> current->thread.abi = &mips_abi; \
> \
> - current->thread.fpu.fcr31 = current_cpu_data.fpu_csr31; \
> + current->thread.fpu.fcr31 = boot_cpu_data.fpu_csr31; \
> } while (0)
>
> #endif /* CONFIG_32BIT */
> @@ -366,7 +366,7 @@ do { \
> else \
> current->thread.abi = &mips_abi; \
> \
> - current->thread.fpu.fcr31 = current_cpu_data.fpu_csr31; \
> + current->thread.fpu.fcr31 = boot_cpu_data.fpu_csr31; \
> \
> p = personality(current->personality); \
> if (p != PER_LINUX32 && p != PER_LINUX) \
> Index: linux-org-test/arch/mips/kernel/ptrace.c
> ===================================================================
> --- linux-org-test.orig/arch/mips/kernel/ptrace.c 2015-04-28 14:52:04.000000000 +0100
> +++ linux-org-test/arch/mips/kernel/ptrace.c 2015-05-12 15:12:03.511002000 +0100
> @@ -176,7 +176,7 @@ int ptrace_setfpregs(struct task_struct
>
> __get_user(value, data + 64);
> fcr31 = child->thread.fpu.fcr31;
> - mask = current_cpu_data.fpu_msk31;
> + mask = boot_cpu_data.fpu_msk31;
> child->thread.fpu.fcr31 = (value & ~mask) | (fcr31 & mask);
>
> /* FIR may not be written. */
> Index: linux-org-test/arch/mips/math-emu/cp1emu.c
> ===================================================================
> --- linux-org-test.orig/arch/mips/math-emu/cp1emu.c 2015-04-28 14:52:04.000000000 +0100
> +++ linux-org-test/arch/mips/math-emu/cp1emu.c 2015-05-12 14:41:06.308256000 +0100
> @@ -889,7 +889,7 @@ static inline void cop1_cfc(struct pt_re
> break;
>
> case FPCREG_RID:
> - value = current_cpu_data.fpu_id;
> + value = boot_cpu_data.fpu_id;
> break;
>
> default:
> @@ -921,7 +921,7 @@ static inline void cop1_ctc(struct pt_re
> (void *)xcp->cp0_epc, MIPSInst_RT(ir), value);
>
> /* Preserve read-only bits. */
> - mask = current_cpu_data.fpu_msk31;
> + mask = boot_cpu_data.fpu_msk31;
> fcr31 = (value & ~mask) | (fcr31 & mask);
> break;
>
>
--
Ezequiel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] MIPS: Fix a preemption issue with thread's FPU defaults
@ 2015-05-12 17:28 ` Ezequiel Garcia
0 siblings, 0 replies; 6+ messages in thread
From: Ezequiel Garcia @ 2015-05-12 17:28 UTC (permalink / raw
To: Maciej W. Rozycki, Ralf Baechle, Paul Martin
Cc: Markos Chandras, James Hogan, linux-mips
On 05/12/2015 11:20 AM, Maciej W. Rozycki wrote:
> Fix "BUG: using smp_processor_id() in preemptible" reported in accesses
> to thread's FPU defaults: the value to initialise FSCR to at program
> startup, the FCSR r/w mask and the contents of FIR in full FPU
> emulation, removing a regression introduced with 9b26616c [MIPS: Respect
> the ISA level in FCSR handling] and f6843626 [MIPS: math-emu: Set FIR
> feature flags for full emulation].
>
> Use `boot_cpu_data' to obtain the data from, following the approach that
> `cpu_has_*' macros take and avoiding the call to `smp_processor_id' made
> in the reference to `current_cpu_data'. The contents of FSCR have to be
> consistent across processors in an SMP system, the settings there must
> not change as a thread is migrated across processors. And the contents
> of FIR are guaranteed to be consistent in FPU emulation, by definition.
>
> Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
> ---
> Changes from v1:
>
> - also fix PTRACE_SETFPREGS (thanks, Paul!).
>
> Paul, Ezequiel --
>
> Can you verify this change addresses the problems you saw?
>
Tested-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
> [Ralf, this is another 4.1 material, please push it to Linus ASAP, once
> this has been confirmed to DTRT.]
>
> Thanks,
>
> Maciej
>
> linux-mips-emu-fcsr-isa-fix.diff
> Index: linux-org-test/arch/mips/include/asm/elf.h
> ===================================================================
> --- linux-org-test.orig/arch/mips/include/asm/elf.h 2015-05-06 23:20:51.946503000 +0100
> +++ linux-org-test/arch/mips/include/asm/elf.h 2015-05-12 14:37:14.169350000 +0100
> @@ -304,7 +304,7 @@ do { \
> \
> current->thread.abi = &mips_abi; \
> \
> - current->thread.fpu.fcr31 = current_cpu_data.fpu_csr31; \
> + current->thread.fpu.fcr31 = boot_cpu_data.fpu_csr31; \
> } while (0)
>
> #endif /* CONFIG_32BIT */
> @@ -366,7 +366,7 @@ do { \
> else \
> current->thread.abi = &mips_abi; \
> \
> - current->thread.fpu.fcr31 = current_cpu_data.fpu_csr31; \
> + current->thread.fpu.fcr31 = boot_cpu_data.fpu_csr31; \
> \
> p = personality(current->personality); \
> if (p != PER_LINUX32 && p != PER_LINUX) \
> Index: linux-org-test/arch/mips/kernel/ptrace.c
> ===================================================================
> --- linux-org-test.orig/arch/mips/kernel/ptrace.c 2015-04-28 14:52:04.000000000 +0100
> +++ linux-org-test/arch/mips/kernel/ptrace.c 2015-05-12 15:12:03.511002000 +0100
> @@ -176,7 +176,7 @@ int ptrace_setfpregs(struct task_struct
>
> __get_user(value, data + 64);
> fcr31 = child->thread.fpu.fcr31;
> - mask = current_cpu_data.fpu_msk31;
> + mask = boot_cpu_data.fpu_msk31;
> child->thread.fpu.fcr31 = (value & ~mask) | (fcr31 & mask);
>
> /* FIR may not be written. */
> Index: linux-org-test/arch/mips/math-emu/cp1emu.c
> ===================================================================
> --- linux-org-test.orig/arch/mips/math-emu/cp1emu.c 2015-04-28 14:52:04.000000000 +0100
> +++ linux-org-test/arch/mips/math-emu/cp1emu.c 2015-05-12 14:41:06.308256000 +0100
> @@ -889,7 +889,7 @@ static inline void cop1_cfc(struct pt_re
> break;
>
> case FPCREG_RID:
> - value = current_cpu_data.fpu_id;
> + value = boot_cpu_data.fpu_id;
> break;
>
> default:
> @@ -921,7 +921,7 @@ static inline void cop1_ctc(struct pt_re
> (void *)xcp->cp0_epc, MIPSInst_RT(ir), value);
>
> /* Preserve read-only bits. */
> - mask = current_cpu_data.fpu_msk31;
> + mask = boot_cpu_data.fpu_msk31;
> fcr31 = (value & ~mask) | (fcr31 & mask);
> break;
>
>
--
Ezequiel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] MIPS: Fix a preemption issue with thread's FPU defaults
2015-05-12 17:28 ` Ezequiel Garcia
(?)
@ 2015-05-12 17:34 ` Paul Martin
-1 siblings, 0 replies; 6+ messages in thread
From: Paul Martin @ 2015-05-12 17:34 UTC (permalink / raw
To: Ezequiel Garcia
Cc: Maciej W. Rozycki, Ralf Baechle, Paul Martin, Markos Chandras,
James Hogan, linux-mips
On Tue, May 12, 2015 at 02:28:23PM -0300, Ezequiel Garcia wrote:
>
>
> On 05/12/2015 11:20 AM, Maciej W. Rozycki wrote:
> > Fix "BUG: using smp_processor_id() in preemptible" reported in accesses
> > to thread's FPU defaults: the value to initialise FSCR to at program
> > startup, the FCSR r/w mask and the contents of FIR in full FPU
> > emulation, removing a regression introduced with 9b26616c [MIPS: Respect
> > the ISA level in FCSR handling] and f6843626 [MIPS: math-emu: Set FIR
> > feature flags for full emulation].
> >
> > Use `boot_cpu_data' to obtain the data from, following the approach that
> > `cpu_has_*' macros take and avoiding the call to `smp_processor_id' made
> > in the reference to `current_cpu_data'. The contents of FSCR have to be
> > consistent across processors in an SMP system, the settings there must
> > not change as a thread is migrated across processors. And the contents
> > of FIR are guaranteed to be consistent in FPU emulation, by definition.
> >
> > Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
> > ---
> > Changes from v1:
> >
> > - also fix PTRACE_SETFPREGS (thanks, Paul!).
> >
> > Paul, Ezequiel --
> >
> > Can you verify this change addresses the problems you saw?
> >
>
> Tested-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
Tested-by: Paul Martin <paul.martin@codethink.co.uk>
>
> > [Ralf, this is another 4.1 material, please push it to Linus ASAP, once
> > this has been confirmed to DTRT.]
> >
> > Thanks,
> >
> > Maciej
> >
> > linux-mips-emu-fcsr-isa-fix.diff
> > Index: linux-org-test/arch/mips/include/asm/elf.h
> > ===================================================================
> > --- linux-org-test.orig/arch/mips/include/asm/elf.h 2015-05-06 23:20:51.946503000 +0100
> > +++ linux-org-test/arch/mips/include/asm/elf.h 2015-05-12 14:37:14.169350000 +0100
> > @@ -304,7 +304,7 @@ do { \
> > \
> > current->thread.abi = &mips_abi; \
> > \
> > - current->thread.fpu.fcr31 = current_cpu_data.fpu_csr31; \
> > + current->thread.fpu.fcr31 = boot_cpu_data.fpu_csr31; \
> > } while (0)
> >
> > #endif /* CONFIG_32BIT */
> > @@ -366,7 +366,7 @@ do { \
> > else \
> > current->thread.abi = &mips_abi; \
> > \
> > - current->thread.fpu.fcr31 = current_cpu_data.fpu_csr31; \
> > + current->thread.fpu.fcr31 = boot_cpu_data.fpu_csr31; \
> > \
> > p = personality(current->personality); \
> > if (p != PER_LINUX32 && p != PER_LINUX) \
> > Index: linux-org-test/arch/mips/kernel/ptrace.c
> > ===================================================================
> > --- linux-org-test.orig/arch/mips/kernel/ptrace.c 2015-04-28 14:52:04.000000000 +0100
> > +++ linux-org-test/arch/mips/kernel/ptrace.c 2015-05-12 15:12:03.511002000 +0100
> > @@ -176,7 +176,7 @@ int ptrace_setfpregs(struct task_struct
> >
> > __get_user(value, data + 64);
> > fcr31 = child->thread.fpu.fcr31;
> > - mask = current_cpu_data.fpu_msk31;
> > + mask = boot_cpu_data.fpu_msk31;
> > child->thread.fpu.fcr31 = (value & ~mask) | (fcr31 & mask);
> >
> > /* FIR may not be written. */
> > Index: linux-org-test/arch/mips/math-emu/cp1emu.c
> > ===================================================================
> > --- linux-org-test.orig/arch/mips/math-emu/cp1emu.c 2015-04-28 14:52:04.000000000 +0100
> > +++ linux-org-test/arch/mips/math-emu/cp1emu.c 2015-05-12 14:41:06.308256000 +0100
> > @@ -889,7 +889,7 @@ static inline void cop1_cfc(struct pt_re
> > break;
> >
> > case FPCREG_RID:
> > - value = current_cpu_data.fpu_id;
> > + value = boot_cpu_data.fpu_id;
> > break;
> >
> > default:
> > @@ -921,7 +921,7 @@ static inline void cop1_ctc(struct pt_re
> > (void *)xcp->cp0_epc, MIPSInst_RT(ir), value);
> >
> > /* Preserve read-only bits. */
> > - mask = current_cpu_data.fpu_msk31;
> > + mask = boot_cpu_data.fpu_msk31;
> > fcr31 = (value & ~mask) | (fcr31 & mask);
> > break;
> >
> >
>
--
Paul Martin http://www.codethink.co.uk/
Senior Software Developer, Codethink Ltd.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] MIPS: Fix a preemption issue with thread's FPU defaults
2015-05-12 14:20 [PATCH v2] MIPS: Fix a preemption issue with thread's FPU defaults Maciej W. Rozycki
2015-05-12 17:28 ` Ezequiel Garcia
@ 2015-05-12 21:14 ` Ralf Baechle
2015-05-12 22:35 ` Maciej W. Rozycki
1 sibling, 1 reply; 6+ messages in thread
From: Ralf Baechle @ 2015-05-12 21:14 UTC (permalink / raw
To: Maciej W. Rozycki
Cc: Paul Martin, Ezequiel Garcia, Markos Chandras, James Hogan,
linux-mips
On Tue, May 12, 2015 at 03:20:57PM +0100, Maciej W. Rozycki wrote:
On systems with multiple types of FPUs this would also result in a
more consistent behaviour when a process is scheduled between different
CPUs.
Ralf
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] MIPS: Fix a preemption issue with thread's FPU defaults
2015-05-12 21:14 ` Ralf Baechle
@ 2015-05-12 22:35 ` Maciej W. Rozycki
0 siblings, 0 replies; 6+ messages in thread
From: Maciej W. Rozycki @ 2015-05-12 22:35 UTC (permalink / raw
To: Ralf Baechle
Cc: Paul Martin, Ezequiel Garcia, Markos Chandras, James Hogan,
linux-mips
On Tue, 12 May 2015, Ralf Baechle wrote:
> On systems with multiple types of FPUs this would also result in a
> more consistent behaviour when a process is scheduled between different
> CPUs.
Hmm, do we support SMP systems where individual FPUs (or CPUs, for that
matter) are different significantly enough to matter? E.g. anyhow
beyond FIR[15:0], that the relevant machine instructions read directly
from hardware where implemented anyway (and likewise FCSR).
I think eventually we should migrate properties that have to be uniform
across all the CPUs in a SMP system outside the per-CPU `cpu_data' array
and have a single global copy only. This will include the ISA level,
some options like the exception and cache model (not the particular
topology though), VM size, etc.
I think this FPU (and also MSA, specifically `msa_id') stuff will
belong there as well, unless proven otherwise. That is unless we have a
mixed system really available in the first place (QEMU does not count,
sorry) or one is at least is being considered, and then we actually want
to support it beyond finding the common set of features across all the
CPUs and limiting userland to using them only (well, if limiting would
at all be possible, that is, via appropriate knobs requiring privilege
to control).
Maciej
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-05-12 22:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-12 14:20 [PATCH v2] MIPS: Fix a preemption issue with thread's FPU defaults Maciej W. Rozycki
2015-05-12 17:28 ` Ezequiel Garcia
2015-05-12 17:28 ` Ezequiel Garcia
2015-05-12 17:34 ` Paul Martin
2015-05-12 21:14 ` Ralf Baechle
2015-05-12 22:35 ` Maciej W. Rozycki
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.