| Message ID | 20210302022907.1835-1-yunqiang.su@cipunited.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v6] MIPS: force use FR=0 for FPXX binary | expand |
On Tue, 2 Mar 2021, YunQiang Su wrote: > The MIPS FPU may have 2 mode: > FR=0: MIPS I style, odd-FPR can only be single, > and even-FPR can be double. Depending on the ISA level FR=0 may or may not allow single arithmetic with odd-numbered FPRs. Compare the FP64A ABI. > FR=1: all 32 FPR can be double. I think it's best to describe the FR=0 mode as one where the FP registers are 32-bit and the FR=1 mode as one where the FP registers are 64-bit (this mode is also needed for the paired-single format). See: <https://dmz-portal.mips.com/wiki/MIPS_O32_ABI_-_FR0_and_FR1_Interlinking#1._Introduction> > The binary may have 3 mode: > FP32: can only work with FR=0 mode > FPXX: can work with both FR=0 and FR=1 mode. > FP64: can only work with FR=1 mode > > Some binary, for example the output of golang, may be mark as FPXX, > while in fact they are FP32. > > Currently, FR=1 mode is used for all FPXX binary, it makes some wrong > behivour of the binaries. Since FPXX binary can work with both FR=1 and FR=0, > we force it to use FR=0. I think you need to document here what we discussed, that is the linker bug exposed in the context of FPXX annotation by legacy modules that lack FP mode annotation, which is the underlying problem. > v5->v6: > Rollback to V3, aka remove config option. You can't reuse v3 as it stands because it breaks R6 as we previously discussed. You need to tell R6 and earlier ISAs apart and set the FR bit accordingly. It would be more proper I suppose if we actually checked at the boot time whether the bit is writable, just like we handle NAN2008, but I don't see it as a prerequisite for this workaround since we currently don't do this either (it would also be good to check if the FP emulation code gets the read-only FR bit right for R6 for FPU-less operation). Also you need to put the history in the comment section and not the commit description, so that the change can be directly applied and does not have to be hand-edited by the maintainer. You don't want to overload him with mechanical work. Maciej
> On Tue, 2 Mar 2021, YunQiang Su wrote: > > > The MIPS FPU may have 2 mode: > > FR=0: MIPS I style, odd-FPR can only be single, > > and even-FPR can be double. > > Depending on the ISA level FR=0 may or may not allow single arithmetic with > odd-numbered FPRs. Compare the FP64A ABI. > > > FR=1: all 32 FPR can be double. > > I think it's best to describe the FR=0 mode as one where the FP registers are > 32-bit and the FR=1 mode as one where the FP registers are 64-bit (this mode > is also needed for the paired-single format). See: > > <https://dmz-portal.mips.com/wiki/MIPS_O32_ABI_-_FR0_and_FR1_Interlin > king#1._Introduction> > Thank you. I will update it. > > The binary may have 3 mode: > > FP32: can only work with FR=0 mode > > FPXX: can work with both FR=0 and FR=1 mode. > > FP64: can only work with FR=1 mode > > > > Some binary, for example the output of golang, may be mark as FPXX, > > while in fact they are FP32. > > > > Currently, FR=1 mode is used for all FPXX binary, it makes some wrong > > behivour of the binaries. Since FPXX binary can work with both FR=1 > > and FR=0, we force it to use FR=0. > > I think you need to document here what we discussed, that is the linker bug > exposed in the context of FPXX annotation by legacy modules that lack FP > mode annotation, which is the underlying problem. I will do it. > > > v5->v6: > > Rollback to V3, aka remove config option. > > You can't reuse v3 as it stands because it breaks R6 as we previously > discussed. You need to tell R6 and earlier ISAs apart and set the FR bit > accordingly. > It won't break r6, as all of r6 binary is FP64, and on r6 `frdefault' is always false, and `fr1' is always true. It won't trigger this mode switch. Oh, you are right, there may be a case that to run legacy app on r6 CPU. For this case, on r6, we need to set the CPU to FRE mode. > It would be more proper I suppose if we actually checked at the boot time > whether the bit is writable, just like we handle NAN2008, but I don't see it as > a prerequisite for this workaround since we currently don't do this either (it > would also be good to check if the FP emulation code gets the read-only FR bit > right for R6 for FPU-less operation). > Check writable is not needed here. > Also you need to put the history in the comment section and not the commit > description, so that the change can be directly applied and does not have to > be hand-edited by the maintainer. You don't want to overload him with > mechanical work. > Ok, I will add more comment. > Maciej
diff --git a/arch/mips/kernel/elf.c b/arch/mips/kernel/elf.c index 7b045d2a0b51..d1aa907e9864 100644 --- a/arch/mips/kernel/elf.c +++ b/arch/mips/kernel/elf.c @@ -234,9 +234,10 @@ int arch_check_elf(void *_ehdr, bool has_interpreter, void *_interp_ehdr, * fpxx case. This is because, in any-ABI (or no-ABI) we have no FPU * instructions so we don't care about the mode. We will simply use * the one preferred by the hardware. In fpxx case, that ABI can - * handle both FR=1 and FR=0, so, again, we simply choose the one - * preferred by the hardware. Next, if we only use single-precision - * FPU instructions, and the default ABI FPU mode is not good + * handle both FR=1 and FR=0. Here, we may need to use FR=0, because + * some binaries may be mark as FPXX by mistake (ie, output of golang). + * - If we only use single-precision FPU instructions, + * and the default ABI FPU mode is not good * (ie single + any ABI combination), we set again the FPU mode to the * one is preferred by the hardware. Next, if we know that the code * will only use single-precision instructions, shown by single being @@ -248,8 +249,9 @@ int arch_check_elf(void *_ehdr, bool has_interpreter, void *_interp_ehdr, */ if (prog_req.fre && !prog_req.frdefault && !prog_req.fr1) state->overall_fp_mode = FP_FRE; - else if ((prog_req.fr1 && prog_req.frdefault) || - (prog_req.single && !prog_req.frdefault)) + else if (prog_req.fr1 && prog_req.frdefault) + state->overall_fp_mode = FP_FR0; + else if (prog_req.single && !prog_req.frdefault) /* Make sure 64-bit MIPS III/IV/64R1 will not pick FR1 */ state->overall_fp_mode = ((raw_current_cpu_data.fpu_id & MIPS_FPIR_F64) && cpu_has_mips_r2_r6) ?
The MIPS FPU may have 2 mode: FR=0: MIPS I style, odd-FPR can only be single, and even-FPR can be double. FR=1: all 32 FPR can be double. The binary may have 3 mode: FP32: can only work with FR=0 mode FPXX: can work with both FR=0 and FR=1 mode. FP64: can only work with FR=1 mode Some binary, for example the output of golang, may be mark as FPXX, while in fact they are FP32. Currently, FR=1 mode is used for all FPXX binary, it makes some wrong behivour of the binaries. Since FPXX binary can work with both FR=1 and FR=0, we force it to use FR=0. Reference: https://web.archive.org/web/20180828210612/https://dmz-portal.mips.com/wiki/MIPS_O32_ABI_-_FR0_and_FR1_Interlinking https://go-review.googlesource.com/c/go/+/239217 https://go-review.googlesource.com/c/go/+/237058 Signed-off-by: YunQiang Su <yunqiang.su@cipunited.com> Cc: stable@vger.kernel.org # 4.19+ v5->v6: Rollback to V3, aka remove config option. v4->v5: Fix CONFIG_MIPS_O32_FPXX_USE_FR0 usage: if -> ifdef v3->v4: introduce a config option: CONFIG_MIPS_O32_FPXX_USE_FR0 v2->v3: commit message: add Signed-off-by and Cc to stable. v1->v2: Fix bad commit message: in fact, we are switching to FR=0 --- arch/mips/kernel/elf.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)