linux-kernel - Re: [RFC PATCH v1] power: don't manage floating point regs when no FPU

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: <87o8nh9yjd.fsf@mpe.ellerman.id.au>
Date:   Tue, 11 Aug 2020 22:07:02 +1000
From:   Michael Ellerman <mpe@...erman.id.au>
To:     Christophe Leroy <christophe.leroy@...roup.eu>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Paul Mackerras <paulus@...ba.org>
Cc:     linux-kernel@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org
Subject: Re: [RFC PATCH v1] power: don't manage floating point regs when no FPU

Christophe Leroy <christophe.leroy@...roup.eu> writes:
> There is no point in copying floating point regs when there
> is no FPU and MATH_EMULATION is not selected.

Yeah I guess you're right. I've never touched a system with neither, but
if such a thing exists then it does seem silly to copy regs around that
can't be used.

> Create a new CONFIG_PPC_FPU_REGS bool that is selected by
> CONFIG_MATH_EMULATION and CONFIG_PPC_FPU, and use it to
> opt out everything related to fp_state in thread_struct.
>
> The following app runs in approx 10.50 seconds on an 8xx without
> the patch, and in 9.45 seconds with the patch.
>
> 	void sigusr1(int sig) { }
>
> 	int main(int argc, char **argv)
> 	{
> 		int i = 100000;
>
> 		signal(SIGUSR1, sigusr1);
> 		for (;i--;)
> 			raise(SIGUSR1);
> 		exit(0);
> 	}
>
> Signed-off-by: Christophe Leroy <christophe.leroy@...roup.eu>
> ---
>  arch/powerpc/Kconfig                      |  1 +
>  arch/powerpc/include/asm/processor.h      |  2 ++
>  arch/powerpc/kernel/asm-offsets.c         |  2 ++
>  arch/powerpc/kernel/process.c             |  4 ++++
>  arch/powerpc/kernel/ptrace/ptrace-novsx.c |  8 ++++++++
>  arch/powerpc/kernel/ptrace/ptrace.c       |  4 ++++
>  arch/powerpc/kernel/signal.c              | 12 +++++++++++-
>  arch/powerpc/kernel/signal_32.c           |  4 ++++
>  arch/powerpc/kernel/traps.c               |  4 ++++
>  arch/powerpc/platforms/Kconfig.cputype    |  4 ++++
>  10 files changed, 44 insertions(+), 1 deletion(-)

In general this looks fine.

It's a bit #ifdef heavy. Maybe some of those can be cleaned up a bit
with some wrapper inlines?

> diff --git a/arch/powerpc/kernel/ptrace/ptrace-novsx.c b/arch/powerpc/kernel/ptrace/ptrace-novsx.c
> index b2dc4e92d11a..8f87a11f3f8c 100644
> --- a/arch/powerpc/kernel/ptrace/ptrace-novsx.c
> +++ b/arch/powerpc/kernel/ptrace/ptrace-novsx.c
> @@ -28,6 +29,9 @@ int fpr_get(struct task_struct *target, const struct user_regset *regset,
>  
>  	return user_regset_copyout(&pos, &count, &kbuf, &ubuf,
>  				   &target->thread.fp_state, 0, -1);
> +#else
> +	return 0;
> +#endif

Should we return -ENODEV/EIO here? Wonder if another arch can give us a clue.

cheers