From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932626AbcGDVph (ORCPT ); Mon, 4 Jul 2016 17:45:37 -0400 Received: from mail-oi0-f66.google.com ([209.85.218.66]:33586 "EHLO mail-oi0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753490AbcGDVpf (ORCPT ); Mon, 4 Jul 2016 17:45:35 -0400 Subject: Re: [RFC] mips: Add MXU context switching support To: PrasannaKumar Muralidharan , linux-mips@linux-mips.org References: <1466856870-17153-1-git-send-email-prasannatsmkumar@gmail.com> Cc: linux-kernel@vger.kernel.org, adobriyan@gmail.com, john.stultz@linaro.org, mguzik@redhat.com, athorlton@sgi.com, mhocko@suse.com, ebiederm@xmission.com, gorcunov@openvz.org, luto@kernel.org, cl@linux.com, serge.hallyn@ubuntu.com, keescook@chromium.org, jslaby@suse.cz, akpm@linux-foundation.org, macro@imgtec.com, mingo@kernel.org, alex.smith@imgtec.com, markos.chandras@imgtec.com, Leonid.Yegoshin@imgtec.com, david.daney@cavium.com, zhaoxiu.zeng@gmail.com, chenhc@lemote.com, Zubair.Kakakhel@imgtec.com, james.hogan@imgtec.com, paul.burton@imgtec.com, ralf@linux-mips.org From: Florian Fainelli Message-ID: <577AD8FB.3040909@gmail.com> Date: Mon, 4 Jul 2016 14:45:31 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-Version: 1.0 In-Reply-To: <1466856870-17153-1-git-send-email-prasannatsmkumar@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Le 25/06/2016 05:14, PrasannaKumar Muralidharan a écrit : > From: PrasannaKumar Muralidharan > > This patch adds support for context switching Xburst MXU registers. The > registers are named xr0 to xr16. xr16 is the control register that can > be used to enable and disable MXU instruction set. Read and write to > these registers can be done without enabling MXU instruction set by user > space. Only when MXU instruction set is enabled any MXU instruction > (other than read or write to xr registers) can be done. xr0 is always 0. > > Kernel does not know when MXU instruction is enabled or disabled. So > during context switch if MXU is enabled in xr16 register then MXU > registers are saved, restored when the task is run. When user space > application enables MXU, it is not reflected in other threads > immediately. So for convenience the applications can use prctl syscall > to let the MXU state propagate across threads running in different CPUs. This is all well and good and seems useful, but you have not stated why this is even useful in the first place? > > Signed-off-by: PrasannaKumar Muralidharan > --- [snip] > diff --git a/arch/mips/include/asm/processor.h b/arch/mips/include/asm/processor.h > index 7e78b62..a4def30 100644 > --- a/arch/mips/include/asm/processor.h > +++ b/arch/mips/include/asm/processor.h > @@ -142,6 +142,11 @@ struct mips_dsp_state { > unsigned int dspcontrol; > }; > > +#define NUM_MXU_REGS 16 > +struct xburst_mxu_state { > + unsigned int xr[NUM_MXU_REGS]; > +}; > + > #define INIT_CPUMASK { \ > {0,} \ > } > @@ -266,6 +271,10 @@ struct thread_struct { > /* Saved state of the DSP ASE, if available. */ > struct mips_dsp_state dsp; > > + 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? > + > /* Saved watch register state, if available. */ > union mips_watch_reg_state watch; > > @@ -330,6 +339,10 @@ struct thread_struct { > .dspr = {0, }, \ > .dspcontrol = 0, \ > }, \ > + .mxu_used = 0, \ > + .mxu = { \ > + .xr = {0, }, \ > + }, \ > /* \ > * saved watch register stuff \ > */ \ > @@ -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? [snip] > 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. -- Florian