Re: [PATCH V3 1/2] MIPS: Loongson-3: Enable COP2 usage in kernel

[Huacai Chen <chenhc@lemote.com>]

Hi, Thomas,

On Tue, Aug 11, 2020 at 8:08 PM Thomas Bogendoerfer
<tsbogend@alpha.franken.de> wrote:
>
> On Tue, Aug 11, 2020 at 02:45:05PM +0800, Huacai Chen wrote:
> > Hi, Thomas and Jiaxun,
> >
> > On Tue, Aug 11, 2020 at 10:18 AM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote:
> > >
> > >
> > >
> > > 在 2020/8/10 22:12, Thomas Bogendoerfer 写道:
> > > > On Sun, Aug 09, 2020 at 10:53:13PM +0800, Jiaxun Yang wrote:
> > > >> Thus we still need to enable CU2 with exception for user space, and we can
> > > >> always enable CU2 in
> > > >> kernel since kernel won't be compiled with hard-float. :-)
> > > > I see, how about the patch below
> > > That looks fine for me.
> > > Is it good with you, Huacai?
> >
> > There are two problems:
> > 1, zboot (arch/mips/boot/compressed/head.S) should be considered,
> > because the initial value of Status may or may not contain CU2.
>
> this comes with it's own memcpy/memset and stuff, I don't see a reason why
> COP2 needs to be enabled there,
gslq/gssq can also be generated by toolchains.

>
> > 2, r4k_switch.S should set CU2 for the new process, otherwise it
> > cannot use gslq/gssq while it in kernel (Because the new process
> > doesn't contain CU2 in THERAD_STATUS.
>
> which is correct for all user space process, otherwise the whole
> cop2 exception thing wouldn't work. And if cop2 exception handling
> has been run it's set in THREAD_STATUS.
>
THREAD_STATUS means thread_struct.cp0_status, which is the cp0_status
when a process runs in kernel-space. KSTK_STATUS (what you have seen
in copy_thread_tls() below) means cp0_status in a process's kernel
stack, which saves the cp0_status when a process runs in user-space.
Whether COP2 exception can work depends on that KSTK_STATUS (but not
THREAD_STATUS) should not contain CU2 at the first time. So, whether
or not THREAD_STATUS contains CU2, it won't break COP2 handling.

Huacai
> > Though a process sets CU2 when it enters kernel, but it
> > only sets CU2 in hardware, not in THREAD_STATUS).
>
> A kernel thread will get THREAD_STATUS from current running kernel code,
> at least that's how I read this code:
>
>        if (unlikely(p->flags & PF_KTHREAD)) {
>                 /* kernel thread */
>                 unsigned long status = p->thread.cp0_status;
>                 memset(childregs, 0, sizeof(struct pt_regs));
>                 ti->addr_limit = KERNEL_DS;
>                 p->thread.reg16 = usp; /* fn */
>                 p->thread.reg17 = kthread_arg;
>                 p->thread.reg29 = childksp;
>                 p->thread.reg31 = (unsigned long) ret_from_kernel_thread;
> #if defined(CONFIG_CPU_R3000) || defined(CONFIG_CPU_TX39XX)
>                 status = (status & ~(ST0_KUP | ST0_IEP | ST0_IEC)) |
>                          ((status & (ST0_KUC | ST0_IEC)) << 2);
> #else
>                 status |= ST0_EXL;
> #endif
>                 childregs->cp0_status = status;
>                 return 0;
>         }
>
> If there is still something missing, I want to find the root cause
> and not paper over it in r4k_switch.S and IMHO break COP2 handling for
> loongsoon completely.
>
> Thomas.
>
> --
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea.                                                [ RFC1925, 2.3 ]