From: Paul Burton <paul.burton@imgtec.com>
To: "Maciej W. Rozycki" <macro@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>, <linux-mips@linux-mips.org>,
<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 3/4] MIPS: Fix ISA I/II FP signal context offsets
Date: Tue, 1 Nov 2016 09:31:29 +0000 [thread overview]
Message-ID: <1814661.4I5xJD6gaP@np-p-burton> (raw)
In-Reply-To: <alpine.DEB.2.00.1610310407150.31859@tp.orcam.me.uk>
[-- Attachment #1: Type: text/plain, Size: 10482 bytes --]
On Monday, 31 October 2016 16:27:01 GMT Maciej W. Rozycki wrote:
> Fix a regression introduced with commit 2db9ca0a3551 ("MIPS: Use struct
> mips_abi offsets to save FP context") for MIPS I/I FP signal contexts,
> by converting save/restore code to the updated internal API. Start FGR
> offsets from 0 rather than SC_FPREGS from $a0 and use $a1 rather than
> the offset of SC_FPC_CSR from $a0 for the Floating Point Control/Status
> Register (FCSR).
>
> Document the new internal API and adjust assembly code formatting for
> consistency.
>
> Signed-off-by: Maciej W. Rozycki <macro@imgtec.com>
> ---
> linux-mips-isa12-sig-fp-context-offsets.patch
> Index: linux-sfr-test/arch/mips/kernel/r2300_fpu.S
> ===================================================================
> --- linux-sfr-test.orig/arch/mips/kernel/r2300_fpu.S 2016-10-22
> 02:28:09.266462000 +0100 +++
> linux-sfr-test/arch/mips/kernel/r2300_fpu.S 2016-10-22 02:29:01.523906000
> +0100 @@ -26,97 +26,104 @@
>
> .set noreorder
> .set mips1
> - /* Save floating point context */
> +
> +/**
> + * _save_fp_context() - save FP context from the FPU
> + * @a0 - pointer to fpregs field of sigcontext
> + * @a1 - pointer to fpc_csr field of sigcontext
> + *
> + * Save FP context, including the 32 FP data registers and the FP
> + * control & status register, from the FPU to signal context.
> + */
> LEAF(_save_fp_context)
> .set push
> SET_HARDFLOAT
> li v0, 0 # assume success
> - cfc1 t1,fcr31
> - EX(swc1 $f0,(SC_FPREGS+0)(a0))
> - EX(swc1 $f1,(SC_FPREGS+8)(a0))
> - EX(swc1 $f2,(SC_FPREGS+16)(a0))
> - EX(swc1 $f3,(SC_FPREGS+24)(a0))
> - EX(swc1 $f4,(SC_FPREGS+32)(a0))
> - EX(swc1 $f5,(SC_FPREGS+40)(a0))
> - EX(swc1 $f6,(SC_FPREGS+48)(a0))
> - EX(swc1 $f7,(SC_FPREGS+56)(a0))
> - EX(swc1 $f8,(SC_FPREGS+64)(a0))
> - EX(swc1 $f9,(SC_FPREGS+72)(a0))
> - EX(swc1 $f10,(SC_FPREGS+80)(a0))
> - EX(swc1 $f11,(SC_FPREGS+88)(a0))
> - EX(swc1 $f12,(SC_FPREGS+96)(a0))
> - EX(swc1 $f13,(SC_FPREGS+104)(a0))
> - EX(swc1 $f14,(SC_FPREGS+112)(a0))
> - EX(swc1 $f15,(SC_FPREGS+120)(a0))
> - EX(swc1 $f16,(SC_FPREGS+128)(a0))
> - EX(swc1 $f17,(SC_FPREGS+136)(a0))
> - EX(swc1 $f18,(SC_FPREGS+144)(a0))
> - EX(swc1 $f19,(SC_FPREGS+152)(a0))
> - EX(swc1 $f20,(SC_FPREGS+160)(a0))
> - EX(swc1 $f21,(SC_FPREGS+168)(a0))
> - EX(swc1 $f22,(SC_FPREGS+176)(a0))
> - EX(swc1 $f23,(SC_FPREGS+184)(a0))
> - EX(swc1 $f24,(SC_FPREGS+192)(a0))
> - EX(swc1 $f25,(SC_FPREGS+200)(a0))
> - EX(swc1 $f26,(SC_FPREGS+208)(a0))
> - EX(swc1 $f27,(SC_FPREGS+216)(a0))
> - EX(swc1 $f28,(SC_FPREGS+224)(a0))
> - EX(swc1 $f29,(SC_FPREGS+232)(a0))
> - EX(swc1 $f30,(SC_FPREGS+240)(a0))
> - EX(swc1 $f31,(SC_FPREGS+248)(a0))
> + cfc1 t1, fcr31
> + EX(swc1 $f0, 0(a0))
> + EX(swc1 $f1, 8(a0))
> + EX(swc1 $f2, 16(a0))
> + EX(swc1 $f3, 24(a0))
> + EX(swc1 $f4, 32(a0))
> + EX(swc1 $f5, 40(a0))
> + EX(swc1 $f6, 48(a0))
> + EX(swc1 $f7, 56(a0))
> + EX(swc1 $f8, 64(a0))
> + EX(swc1 $f9, 72(a0))
> + EX(swc1 $f10, 80(a0))
> + EX(swc1 $f11, 88(a0))
> + EX(swc1 $f12, 96(a0))
> + EX(swc1 $f13, 104(a0))
> + EX(swc1 $f14, 112(a0))
> + EX(swc1 $f15, 120(a0))
> + EX(swc1 $f16, 128(a0))
> + EX(swc1 $f17, 136(a0))
> + EX(swc1 $f18, 144(a0))
> + EX(swc1 $f19, 152(a0))
> + EX(swc1 $f20, 160(a0))
> + EX(swc1 $f21, 168(a0))
> + EX(swc1 $f22, 176(a0))
> + EX(swc1 $f23, 184(a0))
> + EX(swc1 $f24, 192(a0))
> + EX(swc1 $f25, 200(a0))
> + EX(swc1 $f26, 208(a0))
> + EX(swc1 $f27, 216(a0))
> + EX(swc1 $f28, 224(a0))
> + EX(swc1 $f29, 232(a0))
> + EX(swc1 $f30, 240(a0))
> + EX(swc1 $f31, 248(a0))
> jr ra
> - EX(sw t1,(SC_FPC_CSR)(a0))
> + EX(sw t1, (a1))
> .set pop
> END(_save_fp_context)
>
> -/*
> - * Restore FPU state:
> - * - fp gp registers
> - * - cp1 status/control register
> +/**
> + * _restore_fp_context() - restore FP context to the FPU
> + * @a0 - pointer to fpregs field of sigcontext
> + * @a1 - pointer to fpc_csr field of sigcontext
> *
> - * We base the decision which registers to restore from the signal stack
> - * frame on the current content of c0_status, not on the content of the
> - * stack frame which might have been changed by the user.
> + * Restore FP context, including the 32 FP data registers and the FP
> + * control & status register, from signal context to the FPU.
> */
> LEAF(_restore_fp_context)
> .set push
> SET_HARDFLOAT
> li v0, 0 # assume success
> - EX(lw t0,(SC_FPC_CSR)(a0))
> - EX(lwc1 $f0,(SC_FPREGS+0)(a0))
> - EX(lwc1 $f1,(SC_FPREGS+8)(a0))
> - EX(lwc1 $f2,(SC_FPREGS+16)(a0))
> - EX(lwc1 $f3,(SC_FPREGS+24)(a0))
> - EX(lwc1 $f4,(SC_FPREGS+32)(a0))
> - EX(lwc1 $f5,(SC_FPREGS+40)(a0))
> - EX(lwc1 $f6,(SC_FPREGS+48)(a0))
> - EX(lwc1 $f7,(SC_FPREGS+56)(a0))
> - EX(lwc1 $f8,(SC_FPREGS+64)(a0))
> - EX(lwc1 $f9,(SC_FPREGS+72)(a0))
> - EX(lwc1 $f10,(SC_FPREGS+80)(a0))
> - EX(lwc1 $f11,(SC_FPREGS+88)(a0))
> - EX(lwc1 $f12,(SC_FPREGS+96)(a0))
> - EX(lwc1 $f13,(SC_FPREGS+104)(a0))
> - EX(lwc1 $f14,(SC_FPREGS+112)(a0))
> - EX(lwc1 $f15,(SC_FPREGS+120)(a0))
> - EX(lwc1 $f16,(SC_FPREGS+128)(a0))
> - EX(lwc1 $f17,(SC_FPREGS+136)(a0))
> - EX(lwc1 $f18,(SC_FPREGS+144)(a0))
> - EX(lwc1 $f19,(SC_FPREGS+152)(a0))
> - EX(lwc1 $f20,(SC_FPREGS+160)(a0))
> - EX(lwc1 $f21,(SC_FPREGS+168)(a0))
> - EX(lwc1 $f22,(SC_FPREGS+176)(a0))
> - EX(lwc1 $f23,(SC_FPREGS+184)(a0))
> - EX(lwc1 $f24,(SC_FPREGS+192)(a0))
> - EX(lwc1 $f25,(SC_FPREGS+200)(a0))
> - EX(lwc1 $f26,(SC_FPREGS+208)(a0))
> - EX(lwc1 $f27,(SC_FPREGS+216)(a0))
> - EX(lwc1 $f28,(SC_FPREGS+224)(a0))
> - EX(lwc1 $f29,(SC_FPREGS+232)(a0))
> - EX(lwc1 $f30,(SC_FPREGS+240)(a0))
> - EX(lwc1 $f31,(SC_FPREGS+248)(a0))
> + EX(lw t0, (a1))
> + EX(lwc1 $f0, 0(a0))
> + EX(lwc1 $f1, 8(a0))
> + EX(lwc1 $f2, 16(a0))
> + EX(lwc1 $f3, 24(a0))
> + EX(lwc1 $f4, 32(a0))
> + EX(lwc1 $f5, 40(a0))
> + EX(lwc1 $f6, 48(a0))
> + EX(lwc1 $f7, 56(a0))
> + EX(lwc1 $f8, 64(a0))
> + EX(lwc1 $f9, 72(a0))
> + EX(lwc1 $f10, 80(a0))
> + EX(lwc1 $f11, 88(a0))
> + EX(lwc1 $f12, 96(a0))
> + EX(lwc1 $f13, 104(a0))
> + EX(lwc1 $f14, 112(a0))
> + EX(lwc1 $f15, 120(a0))
> + EX(lwc1 $f16, 128(a0))
> + EX(lwc1 $f17, 136(a0))
> + EX(lwc1 $f18, 144(a0))
> + EX(lwc1 $f19, 152(a0))
> + EX(lwc1 $f20, 160(a0))
> + EX(lwc1 $f21, 168(a0))
> + EX(lwc1 $f22, 176(a0))
> + EX(lwc1 $f23, 184(a0))
> + EX(lwc1 $f24, 192(a0))
> + EX(lwc1 $f25, 200(a0))
> + EX(lwc1 $f26, 208(a0))
> + EX(lwc1 $f27, 216(a0))
> + EX(lwc1 $f28, 224(a0))
> + EX(lwc1 $f29, 232(a0))
> + EX(lwc1 $f30, 240(a0))
> + EX(lwc1 $f31, 248(a0))
> jr ra
> - ctc1 t0,fcr31
> + ctc1 t0, fcr31
> .set pop
> END(_restore_fp_context)
> .set reorder
> Index: linux-sfr-test/arch/mips/kernel/r6000_fpu.S
> ===================================================================
> --- linux-sfr-test.orig/arch/mips/kernel/r6000_fpu.S 2016-10-22
> 02:28:57.927876000 +0100 +++
> linux-sfr-test/arch/mips/kernel/r6000_fpu.S 2016-10-22 02:29:12.415002000
> +0100 @@ -21,7 +21,14 @@
> .set push
> SET_HARDFLOAT
>
> - /* Save floating point context */
> +/**
> + * _save_fp_context() - save FP context from the FPU
> + * @a0 - pointer to fpregs field of sigcontext
> + * @a1 - pointer to fpc_csr field of sigcontext
> + *
> + * Save FP context, including the 32 FP data registers and the FP
> + * control & status register, from the FPU to signal context.
> + */
> LEAF(_save_fp_context)
> mfc0 t0,CP0_STATUS
> sll t0,t0,2
> @@ -30,59 +37,59 @@
>
> cfc1 t1,fcr31
> /* Store the 16 double precision registers */
> - sdc1 $f0,(SC_FPREGS+0)(a0)
> - sdc1 $f2,(SC_FPREGS+16)(a0)
> - sdc1 $f4,(SC_FPREGS+32)(a0)
> - sdc1 $f6,(SC_FPREGS+48)(a0)
> - sdc1 $f8,(SC_FPREGS+64)(a0)
> - sdc1 $f10,(SC_FPREGS+80)(a0)
> - sdc1 $f12,(SC_FPREGS+96)(a0)
> - sdc1 $f14,(SC_FPREGS+112)(a0)
> - sdc1 $f16,(SC_FPREGS+128)(a0)
> - sdc1 $f18,(SC_FPREGS+144)(a0)
> - sdc1 $f20,(SC_FPREGS+160)(a0)
> - sdc1 $f22,(SC_FPREGS+176)(a0)
> - sdc1 $f24,(SC_FPREGS+192)(a0)
> - sdc1 $f26,(SC_FPREGS+208)(a0)
> - sdc1 $f28,(SC_FPREGS+224)(a0)
> - sdc1 $f30,(SC_FPREGS+240)(a0)
> + sdc1 $f0,0(a0)
> + sdc1 $f2,16(a0)
> + sdc1 $f4,32(a0)
> + sdc1 $f6,48(a0)
> + sdc1 $f8,64(a0)
> + sdc1 $f10,80(a0)
> + sdc1 $f12,96(a0)
> + sdc1 $f14,112(a0)
> + sdc1 $f16,128(a0)
> + sdc1 $f18,144(a0)
> + sdc1 $f20,160(a0)
> + sdc1 $f22,176(a0)
> + sdc1 $f24,192(a0)
> + sdc1 $f26,208(a0)
> + sdc1 $f28,224(a0)
> + sdc1 $f30,240(a0)
> jr ra
> - sw t0,SC_FPC_CSR(a0)
> + sw t0,(a1)
> 1: jr ra
> nop
> END(_save_fp_context)
>
> -/* Restore FPU state:
> - * - fp gp registers
> - * - cp1 status/control register
> +/**
> + * _restore_fp_context() - restore FP context to the FPU
> + * @a0 - pointer to fpregs field of sigcontext
> + * @a1 - pointer to fpc_csr field of sigcontext
> *
> - * We base the decision which registers to restore from the signal stack
> - * frame on the current content of c0_status, not on the content of the
> - * stack frame which might have been changed by the user.
> + * Restore FP context, including the 32 FP data registers and the FP
> + * control & status register, from signal context to the FPU.
> */
> LEAF(_restore_fp_context)
> mfc0 t0,CP0_STATUS
> sll t0,t0,2
>
> bgez t0,1f
> - lw t0,SC_FPC_CSR(a0)
> + lw t0,(a1)
> /* Restore the 16 double precision registers */
> - ldc1 $f0,(SC_FPREGS+0)(a0)
> - ldc1 $f2,(SC_FPREGS+16)(a0)
> - ldc1 $f4,(SC_FPREGS+32)(a0)
> - ldc1 $f6,(SC_FPREGS+48)(a0)
> - ldc1 $f8,(SC_FPREGS+64)(a0)
> - ldc1 $f10,(SC_FPREGS+80)(a0)
> - ldc1 $f12,(SC_FPREGS+96)(a0)
> - ldc1 $f14,(SC_FPREGS+112)(a0)
> - ldc1 $f16,(SC_FPREGS+128)(a0)
> - ldc1 $f18,(SC_FPREGS+144)(a0)
> - ldc1 $f20,(SC_FPREGS+160)(a0)
> - ldc1 $f22,(SC_FPREGS+176)(a0)
> - ldc1 $f24,(SC_FPREGS+192)(a0)
> - ldc1 $f26,(SC_FPREGS+208)(a0)
> - ldc1 $f28,(SC_FPREGS+224)(a0)
> - ldc1 $f30,(SC_FPREGS+240)(a0)
> + ldc1 $f0,0(a0)
> + ldc1 $f2,16(a0)
> + ldc1 $f4,32(a0)
> + ldc1 $f6,48(a0)
> + ldc1 $f8,64(a0)
> + ldc1 $f10,80(a0)
> + ldc1 $f12,96(a0)
> + ldc1 $f14,112(a0)
> + ldc1 $f16,128(a0)
> + ldc1 $f18,144(a0)
> + ldc1 $f20,160(a0)
> + ldc1 $f22,176(a0)
> + ldc1 $f24,192(a0)
> + ldc1 $f26,208(a0)
> + ldc1 $f28,224(a0)
> + ldc1 $f30,240(a0)
> jr ra
> ctc1 t0,fcr31
> 1: jr ra
Hi Maciej,
Thanks for fixing that up:
Reviewed-by: Paul Burton <paul.burton@imgtec.com>
BTW, do you have a feel for whether there's a good r2k/r3k platform (ideal
would be some software emulator if any are good enough) that we could hook up
to our continuous integration system? That would help us to catch any
regressions like this in future before they hit mainline.
Thanks,
Paul
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
next prev parent reply other threads:[~2016-11-01 9:31 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-31 16:24 [PATCH 0/4] MIPS I/II FP signal context handling fixes Maciej W. Rozycki
2016-10-31 16:25 ` [PATCH 1/4] MIPS: Fix ISA I FP sigcontext access violation handling Maciej W. Rozycki
2016-11-01 9:27 ` Paul Burton
2016-10-31 16:26 ` [PATCH 2/4] MIPS: Remove FIR from ISA I FP signal context Maciej W. Rozycki
2016-10-31 16:27 ` [PATCH 3/4] MIPS: Fix ISA I/II FP signal context offsets Maciej W. Rozycki
2016-11-01 9:31 ` Paul Burton [this message]
2016-11-01 11:49 ` Maciej W. Rozycki
2016-10-31 16:27 ` [PATCH 4/4] MIPS: Correct MIPS I FP sigcontext layout Maciej W. Rozycki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1814661.4I5xJD6gaP@np-p-burton \
--to=paul.burton@imgtec.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mips@linux-mips.org \
--cc=macro@imgtec.com \
--cc=ralf@linux-mips.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®