On Jul 5, 2016 5:35 AM, "Paul Burton" <paul.burton@xxxxxxxxxx> wrote: > > Hi PrasannaKumar, > > > On 25/06/16 13:14, PrasannaKumar Muralidharan wrote: >> >> From: PrasannaKumar Muralidharan <prasannatsmkumar@xxxxxxxxx> >> >> 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. > > > Do you have any examples of userland programs making use of MXU? They would be useful in allowing people to test this patch. > > How have you tested this? > > >> 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. > > > I'm not convinced this is the right way to go. It seems complex & fragile vs the alternatives, the simplest of which could be to just always save & restore MXU context in kernels with MXU support. Is there a significant performance cost to just unconditionally saving & restoring the MXU context? That is after all what Ingenic's vendor kernel, which it looks like large parts of your patch are taken from, does. > > >> 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. > > > Surely it wouldn't be reflected at all, since each thread has its own MXU context? Would you expect applications to actually want to enable MXU on one thread & make use of it from other already running threads? Off the top of my head I can't think of why that would be useful, so I'm wondering whether it would be better to just let each thread handle enabling MXU if it wants & leave the kernel out of it. If we just save & restore unconditionally then this becomes a non-issue anyway. > I don't know much about MIPS, but switching save/restore off depending on a bit of *user* state sounds like a gaping security hole.
