Re: [RFC] mips: Add MXU context switching support

[Andy Lutomirski]

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.