| Message ID | 20210322015902.18451-1-yunqiang.su@cipunited.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v9] MIPS: force use FR=0 for FPXX binaries | expand |
YunQiang Su <yunqiang.su@cipunited.com> 于2021年3月22日周一 上午10:00写道： > > The MIPS FPU may have 3 mode: > FR=0: MIPS I style, all of the FPR are single. > FR=1: all 32 FPR can be double. > FRE: redirecting the rw of odd-FPR to the upper 32bit of even-double FPR. > > The binary may have 3 mode: > FP32: can only work with FR=0 and FRE mode > FPXX: can work with all of FR=0/FR=1/FRE 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. It is caused by the bug of design and linker: > Object produced by pure Go has no FP annotation while in fact they are FP32; > if we link them with the C module which marked as FPXX, > the result will be marked as FPXX. If these fake-FPXX binaries is executed > in FR=1 mode, some problem will happen. > > In Golang, now we add the FP32 annotation, so the future golang programs > won't have this problem. While for the existing binaries, we need a > kernel workaround. > We meet a new problem in Debian: with the O32_FP64 enabled kernel, mips64el may also be affected. https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=983583 > Currently, FR=1 mode is used for all FPXX binary if O32_FP64 supported is enabled, > 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+ > --- > v7->v8->v9: > Rollback to use FR=1 for FPXX on R6 CPU. > > v6->v7: > Use FRE mode for pre-R6 binaries on R6 CPU. > > 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 | 20 +++++++++++++------- > 1 file changed, 13 insertions(+), 7 deletions(-) > > diff --git a/arch/mips/kernel/elf.c b/arch/mips/kernel/elf.c > index 7b045d2a0b51..554561d5c1f8 100644 > --- a/arch/mips/kernel/elf.c > +++ b/arch/mips/kernel/elf.c > @@ -232,11 +232,16 @@ int arch_check_elf(void *_ehdr, bool has_interpreter, void *_interp_ehdr, > * that inherently require the hybrid FP mode. > * - If FR1 and FRDEFAULT is true, that means we hit the any-abi or > * 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 > + * instructions so we don't care about the mode. > + * In fpxx case, that ABI can handle all of FR=1/FR=0/FRE mode. > + * Here, we need to use FR=0 mode instead of FR=1, because some binaries > + * may be mark as FPXX by mistake due to bugs of design and linker: > + * The object produced by pure Go has no FP annotation, > + * then is treated as any-ABI by linker, although in fact they are FP32; > + * if any-ABI object is linked with FPXX object, the result will be mark as FPXX. > + * Then the problem happens: run FP32 binaries in FR=1 mode. > + * - 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 +253,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 = cpu_has_mips_r6 ? FP_FR1 : 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) ? > -- > 2.20.1 > -- YunQiang Su
YunQiang Su <wzssyqa@gmail.com> 于2021年3月29日周一 下午1:09写道： > > YunQiang Su <yunqiang.su@cipunited.com> 于2021年3月22日周一 上午10:00写道： > > > > The MIPS FPU may have 3 mode: > > FR=0: MIPS I style, all of the FPR are single. > > FR=1: all 32 FPR can be double. > > FRE: redirecting the rw of odd-FPR to the upper 32bit of even-double FPR. > > > > The binary may have 3 mode: > > FP32: can only work with FR=0 and FRE mode > > FPXX: can work with all of FR=0/FR=1/FRE 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. It is caused by the bug of design and linker: > > Object produced by pure Go has no FP annotation while in fact they are FP32; > > if we link them with the C module which marked as FPXX, > > the result will be marked as FPXX. If these fake-FPXX binaries is executed > > in FR=1 mode, some problem will happen. > > > > In Golang, now we add the FP32 annotation, so the future golang programs > > won't have this problem. While for the existing binaries, we need a > > kernel workaround. > > > > We meet a new problem in Debian: with the O32_FP64 enabled kernel, > mips64el may also be affected. > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=983583 > Sorry, it is not about O32_FP64, it is about memory segment. Loongson 3A2000+ supports RI/XI, which stop the access of some memory region. The problem is about Go itself: why it map the datafile writeonly. > > > > Currently, FR=1 mode is used for all FPXX binary if O32_FP64 supported is enabled, > > 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+ > > --- > > v7->v8->v9: > > Rollback to use FR=1 for FPXX on R6 CPU. > > > > v6->v7: > > Use FRE mode for pre-R6 binaries on R6 CPU. > > > > 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 | 20 +++++++++++++------- > > 1 file changed, 13 insertions(+), 7 deletions(-) > > > > diff --git a/arch/mips/kernel/elf.c b/arch/mips/kernel/elf.c > > index 7b045d2a0b51..554561d5c1f8 100644 > > --- a/arch/mips/kernel/elf.c > > +++ b/arch/mips/kernel/elf.c > > @@ -232,11 +232,16 @@ int arch_check_elf(void *_ehdr, bool has_interpreter, void *_interp_ehdr, > > * that inherently require the hybrid FP mode. > > * - If FR1 and FRDEFAULT is true, that means we hit the any-abi or > > * 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 > > + * instructions so we don't care about the mode. > > + * In fpxx case, that ABI can handle all of FR=1/FR=0/FRE mode. > > + * Here, we need to use FR=0 mode instead of FR=1, because some binaries > > + * may be mark as FPXX by mistake due to bugs of design and linker: > > + * The object produced by pure Go has no FP annotation, > > + * then is treated as any-ABI by linker, although in fact they are FP32; > > + * if any-ABI object is linked with FPXX object, the result will be mark as FPXX. > > + * Then the problem happens: run FP32 binaries in FR=1 mode. > > + * - 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 +253,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 = cpu_has_mips_r6 ? FP_FR1 : 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) ? > > -- > > 2.20.1 > > > > > -- > YunQiang Su
On Mon, Mar 29, 2021 at 01:09:18PM +0800, YunQiang Su wrote: > YunQiang Su <yunqiang.su@cipunited.com> 于2021年3月22日周一 上午10:00写道： > > > > The MIPS FPU may have 3 mode: > > FR=0: MIPS I style, all of the FPR are single. > > FR=1: all 32 FPR can be double. > > FRE: redirecting the rw of odd-FPR to the upper 32bit of even-double FPR. > > > > The binary may have 3 mode: > > FP32: can only work with FR=0 and FRE mode > > FPXX: can work with all of FR=0/FR=1/FRE 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. It is caused by the bug of design and linker: > > Object produced by pure Go has no FP annotation while in fact they are FP32; > > if we link them with the C module which marked as FPXX, > > the result will be marked as FPXX. If these fake-FPXX binaries is executed > > in FR=1 mode, some problem will happen. > > > > In Golang, now we add the FP32 annotation, so the future golang programs > > won't have this problem. While for the existing binaries, we need a > > kernel workaround. > > > > We meet a new problem in Debian: with the O32_FP64 enabled kernel, > mips64el may also be affected. > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=983583 hmm, raising this issue in this context before knowing more details, feels very trigger happy to me and this doesn't help accepting anything, jfyi... Could you please provide a link for downloading a golang binary, which would need this patch to run ? Thomas.
Thomas Bogendoerfer <tsbogend@alpha.franken.de> 于2021年3月29日周一 下午5:30写道： > > On Mon, Mar 29, 2021 at 01:09:18PM +0800, YunQiang Su wrote: > > YunQiang Su <yunqiang.su@cipunited.com> 于2021年3月22日周一 上午10:00写道： > > > > > > The MIPS FPU may have 3 mode: > > > FR=0: MIPS I style, all of the FPR are single. > > > FR=1: all 32 FPR can be double. > > > FRE: redirecting the rw of odd-FPR to the upper 32bit of even-double FPR. > > > > > > The binary may have 3 mode: > > > FP32: can only work with FR=0 and FRE mode > > > FPXX: can work with all of FR=0/FR=1/FRE 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. It is caused by the bug of design and linker: > > > Object produced by pure Go has no FP annotation while in fact they are FP32; > > > if we link them with the C module which marked as FPXX, > > > the result will be marked as FPXX. If these fake-FPXX binaries is executed > > > in FR=1 mode, some problem will happen. > > > > > > In Golang, now we add the FP32 annotation, so the future golang programs > > > won't have this problem. While for the existing binaries, we need a > > > kernel workaround. > > > > > > > We meet a new problem in Debian: with the O32_FP64 enabled kernel, > > mips64el may also be affected. > > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=983583 > > hmm, raising this issue in this context before knowing more details, > feels very trigger happy to me and this doesn't help accepting anything, > jfyi... > > Could you please provide a link for downloading a golang binary, which > would need this patch to run ? > For rootfs, you can download http://58.246.137.130:20180/debian-from/rootfs/buster-mipsel.tar.xz or create by: sudo debootstrap --arch mipsel --include golang-1.11-go \ buster buster-mipsel http://deb.debian.org/debian For binary packages, you can download: https://packages.debian.org/buster/mipsel/golang-1.11-go/download just chroot the rootfs and run: /usr/lib/go-1.11/bin/go It will crash if kernel's O32_FP64 option is enabled. > Thomas. > > -- > Crap can work. Given enough thrust pigs will fly, but it's not necessarily a > good idea. [ RFC1925, 2.3 ]
On Mon, Mar 29, 2021 at 06:28:40PM +0800, YunQiang Su wrote: > Thomas Bogendoerfer <tsbogend@alpha.franken.de> 于2021年3月29日周一 下午5:30写道： > > > > On Mon, Mar 29, 2021 at 01:09:18PM +0800, YunQiang Su wrote: > > > YunQiang Su <yunqiang.su@cipunited.com> 于2021年3月22日周一 上午10:00写道： > > > > > > > > The MIPS FPU may have 3 mode: > > > > FR=0: MIPS I style, all of the FPR are single. > > > > FR=1: all 32 FPR can be double. > > > > FRE: redirecting the rw of odd-FPR to the upper 32bit of even-double FPR. > > > > > > > > The binary may have 3 mode: > > > > FP32: can only work with FR=0 and FRE mode > > > > FPXX: can work with all of FR=0/FR=1/FRE 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. It is caused by the bug of design and linker: > > > > Object produced by pure Go has no FP annotation while in fact they are FP32; > > > > if we link them with the C module which marked as FPXX, > > > > the result will be marked as FPXX. If these fake-FPXX binaries is executed > > > > in FR=1 mode, some problem will happen. > > > > > > > > In Golang, now we add the FP32 annotation, so the future golang programs > > > > won't have this problem. While for the existing binaries, we need a > > > > kernel workaround. > > > > > > > > > > We meet a new problem in Debian: with the O32_FP64 enabled kernel, > > > mips64el may also be affected. > > > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=983583 > > > > hmm, raising this issue in this context before knowing more details, > > feels very trigger happy to me and this doesn't help accepting anything, > > jfyi... > > > > Could you please provide a link for downloading a golang binary, which > > would need this patch to run ? > > > > For rootfs, you can download > http://58.246.137.130:20180/debian-from/rootfs/buster-mipsel.tar.xz > or create by: > sudo debootstrap --arch mipsel --include golang-1.11-go \ > buster buster-mipsel http://deb.debian.org/debian > > For binary packages, you can download: > https://packages.debian.org/buster/mipsel/golang-1.11-go/download > > just chroot the rootfs and run: > /usr/lib/go-1.11/bin/go > It will crash if kernel's O32_FP64 option is enabled. now I'm confused. Do go binaries crash the kernel ? Or is this just the issue _not_ related to this patch ? I want a single go binary, which I can inspect about the FPXX thing and see how easy it would be to just patch the binary and make it run without this patch. Thomas.
On Tue, 30 Mar 2021, Thomas Bogendoerfer wrote: > I want a single go binary, which I can inspect about the FPXX thing and > see how easy it would be to just patch the binary and make it run without > this patch. I have experimented with this a bit and unfortunately a command like: $ objcopy --remove-section=.MIPS.abiflags go go-noabiflags will indeed remove the MIPS ABI Flags section, but will leave an empty ABIFLAGS segment behind: ABIFLAGS 0x000000 0x00400248 0x00000000 0x00000 0x00000 R 0x4 which the kernel will choke on in `arch_elf_pt_proc': if (phdr32->p_filesz < sizeof(abiflags)) return -EINVAL; or: if (phdr64->p_filesz < sizeof(abiflags)) return -EINVAL; which I think is another implementation bug; I think we should silently ignore an empty segment like say `readelf -A' does, that is: if (!phdr32->p_filesz) return 0; etc. Sadly there's no option for `objcopy' to explicitly handle segments anyhow including to remove empty ones. There's the `--update-section' section option for `objcopy' that should work, but it requires more effort as an image of a patched .MIPS.abiflags section is required (still doable). Maciej
On Mon, 22 Mar 2021, YunQiang Su wrote: > Currently, FR=1 mode is used for all FPXX binary if O32_FP64 supported is enabled, > 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. Even if it is given a go-ahead from the semantic point of view, your change still suffers from style issues. I find the change description inaccurate or unclear in some places (I can help with that, but not before code itself has been accepted), and the change has overlong lines both in the body (the limit is 80 columns) and the description (the limit is 75 columns). It's not clear to me why `scripts/checkpatch.pl' only complains about one place, but you need to fix them all. If you haven't so far, please familiarise yourself with the documents describing patch submission: Documentation/process/coding-style.rst and Documentation/process/submitting-patches.rst available in the kernel tree. They document the line length limits given above and all the other aspects of code preparation for submission. Maciej
diff --git a/arch/mips/kernel/elf.c b/arch/mips/kernel/elf.c index 7b045d2a0b51..554561d5c1f8 100644 --- a/arch/mips/kernel/elf.c +++ b/arch/mips/kernel/elf.c @@ -232,11 +232,16 @@ int arch_check_elf(void *_ehdr, bool has_interpreter, void *_interp_ehdr, * that inherently require the hybrid FP mode. * - If FR1 and FRDEFAULT is true, that means we hit the any-abi or * 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 + * instructions so we don't care about the mode. + * In fpxx case, that ABI can handle all of FR=1/FR=0/FRE mode. + * Here, we need to use FR=0 mode instead of FR=1, because some binaries + * may be mark as FPXX by mistake due to bugs of design and linker: + * The object produced by pure Go has no FP annotation, + * then is treated as any-ABI by linker, although in fact they are FP32; + * if any-ABI object is linked with FPXX object, the result will be mark as FPXX. + * Then the problem happens: run FP32 binaries in FR=1 mode. + * - 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 +253,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 = cpu_has_mips_r6 ? FP_FR1 : 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 3 mode: FR=0: MIPS I style, all of the FPR are single. FR=1: all 32 FPR can be double. FRE: redirecting the rw of odd-FPR to the upper 32bit of even-double FPR. The binary may have 3 mode: FP32: can only work with FR=0 and FRE mode FPXX: can work with all of FR=0/FR=1/FRE 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. It is caused by the bug of design and linker: Object produced by pure Go has no FP annotation while in fact they are FP32; if we link them with the C module which marked as FPXX, the result will be marked as FPXX. If these fake-FPXX binaries is executed in FR=1 mode, some problem will happen. In Golang, now we add the FP32 annotation, so the future golang programs won't have this problem. While for the existing binaries, we need a kernel workaround. Currently, FR=1 mode is used for all FPXX binary if O32_FP64 supported is enabled, 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+ --- v7->v8->v9: Rollback to use FR=1 for FPXX on R6 CPU. v6->v7: Use FRE mode for pre-R6 binaries on R6 CPU. 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 | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-)