linux-kernel - Re: [RFC] mips: Add MXU context switching support

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: <CANc+2y5mJDv9Vrg2q+2VVNFGr6BYOydhjF_jcJrB6q4_erMecQ@mail.gmail.com>
Date:	Tue, 5 Jul 2016 15:17:40 +0530
From:	PrasannaKumar Muralidharan <prasannatsmkumar@...il.com>
To:	Florian Fainelli <f.fainelli@...il.com>
Cc:	linux-mips@...ux-mips.org, linux-kernel@...r.kernel.org,
	Alexey Dobriyan <adobriyan@...il.com>,
	John Stultz <john.stultz@...aro.org>, mguzik@...hat.com,
	athorlton@....com, mhocko@...e.com, ebiederm@...ssion.com,
	gorcunov@...nvz.org, luto@...nel.org, cl@...ux.com,
	serge.hallyn@...ntu.com, Kees Cook <keescook@...omium.org>,
	jslaby@...e.cz, akpm@...ux-foundation.org, macro@...tec.com,
	mingo@...nel.org, alex.smith@...tec.com,
	markos.chandras@...tec.com, Leonid.Yegoshin@...tec.com,
	david.daney@...ium.com, zhaoxiu.zeng@...il.com, chenhc@...ote.com,
	Zubair.Kakakhel@...tec.com, james.hogan@...tec.com,
	Paul Burton <paul.burton@...tec.com>, ralf@...ux-mips.org
Subject: Re: [RFC] mips: Add MXU context switching support

> This is all well and good and seems useful, but you have not stated why
> this is even useful in the first place?

Sorry, should have done that already. MXU is the name for SIMD
instructions found in Ingenic SoC. Registers xr0 to xr16 is used by
MXU instructions. I will add info when I submit next version of the
patch.

>> +     unsigned int mxu_used;
>> +     /* Saved registers of Xburst MXU, if available. */
>> +     struct xburst_mxu_state mxu;
>
> That's adding about 17 * 4 bytes to a structure that is probably best
> kept as small as possible, might be worth adding an ifdef here specific
> to the Ingenic platforms w/ MXU?

Yes, makes sense. Will do.

>> @@ -410,4 +423,10 @@ extern int mips_set_process_fp_mode(struct task_struct *task,
>>  #define GET_FP_MODE(task)            mips_get_process_fp_mode(task)
>>  #define SET_FP_MODE(task,value)              mips_set_process_fp_mode(task, value)
>>
>> +extern int mips_enable_mxu_other_cpus(void);
>> +extern int mips_disable_mxu_other_cpus(void);
>> +
>> +#define ENABLE_MXU_OTHER_CPUS()         mips_enable_mxu_other_cpus()
>> +#define DISABLE_MXU_OTHER_CPUS()        mips_disable_mxu_other_cpus()
>
> Where is the stub when building for !MIPS? Have you build a kernel with
> your changes for e.g: x86?

I have missed it. Will add it for sure. Did not check for other arch,
will do it before submitting next revision.

>> diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
>> index a8d0759..b193d91 100644
>> --- a/include/uapi/linux/prctl.h
>> +++ b/include/uapi/linux/prctl.h
>> @@ -197,4 +197,7 @@ struct prctl_mm_map {
>>  # define PR_CAP_AMBIENT_LOWER                3
>>  # define PR_CAP_AMBIENT_CLEAR_ALL    4
>>
>> +#define PR_ENABLE_MXU_OTHER_CPUS        48
>> +#define PR_DISABLE_MXU_OTHER_CPUS       49
>> +
>>  #endif /* _LINUX_PRCTL_H */
>> diff --git a/kernel/sys.c b/kernel/sys.c
>> index 89d5be4..fbbc7b2 100644
>> --- a/kernel/sys.c
>> +++ b/kernel/sys.c
>> @@ -2270,6 +2270,12 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
>>       case PR_GET_FP_MODE:
>>               error = GET_FP_MODE(me);
>>               break;
>> +     case PR_ENABLE_MXU_OTHER_CPUS:
>> +             error = ENABLE_MXU_OTHER_CPUS();
>> +             break;
>> +     case PR_DISABLE_MXU_OTHER_CPUS:
>> +             error = DISABLE_MXU_OTHER_CPUS();
>> +             break;
>
> Is not there a way to call into an architecture specific prctl() for the
> unhandled options passed to prctl()? Not everybody will
> want/implement/support that, and, more importantly changing generic code
> here looks fishy and probably the wrong abstraction.

PR_GET_FP_MODE is specific to MIPS and is present in arch independent
code. I followed similar structure. If it makes sense to put into arch
specific prctl() I will do that.

Thanks for your review. Really appreciate it.