linux-kernel - Re: [PATCH 3/4] MIPS: Fix ISA I/II FP signal context offsets

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1814661.4I5xJD6gaP@np-p-burton>
Date:   Tue, 1 Nov 2016 09:31:29 +0000
From:   Paul Burton <paul.burton@...tec.com>
To:     "Maciej W. Rozycki" <macro@...tec.com>
CC:     Ralf Baechle <ralf@...ux-mips.org>, <linux-mips@...ux-mips.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/4] MIPS: Fix ISA I/II FP signal context offsets

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@...tec.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@...tec.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
Download attachment "signature.asc" of type "application/pgp-signature" (802 bytes)