From: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>
To: Ralf Baechle <ralf@linux-mips.org>
Cc: <linux-mips@linux-mips.org>, <rusty@rustcorp.com.au>,
<alexinbeijing@gmail.com>, <paul.burton@imgtec.com>,
<david.daney@cavium.com>, <alex@alex-smith.me.uk>,
<linux-kernel@vger.kernel.org>, <james.hogan@imgtec.com>,
<markos.chandras@imgtec.com>, <macro@linux-mips.org>,
<eunb.song@samsung.com>, <manuel.lauss@gmail.com>,
<andreas.herrmann@caviumnetworks.com>
Subject: Re: [PATCH 1/2] MIPS: MSA: bugfix - disable MSA during thread switch correctly
Date: Fri, 22 May 2015 17:00:42 -0700 [thread overview]
Message-ID: <555FC32A.4020308@imgtec.com> (raw)
In-Reply-To: <20150522232019.GA10556@linux-mips.org>
On 05/22/2015 04:20 PM, Ralf Baechle wrote:
> On Fri, May 22, 2015 at 11:37:34AM -0700, Leonid Yegoshin wrote:
>
>> On 05/22/2015 02:38 AM, Ralf Baechle wrote:
>>> Just move the call to finish_arch_switch().
>> It might be a problem later, then a correct MSA partiton starts working. It
>> should be tight to saving MSA registers in that case.
>>
>>> Your rewrite also dropped the if (cpu_has_msa) condition from
>>> disable_msa() probably causing havoc on lots of CPUs which will likely not
>>> decode the set bits of the MFC0/MTC0 instructions thus end up accessing
>>> Config0. Ralf
>> Right before this chunk of code there is a saving MSA registers. Does it
>> causing a havoc or else?
>>
>> May I ask you to look into switch_to macro to figure out how "if
>> (cpu_has_msa)" check works in this case?
> Ah sorry I now see that your added code is not executed for all CPUs but
> only those having MSA. So then it's safe.
>
> Still I don't stylistically like defining the register t4 in the middle
> of the code.
>
> Below my suggested patch. It's advantage is that for non-MSA platforms
> the call to disable_msa() will be removed entirely.
>
> Something like Paul's http://patchwork.linux-mips.org/patch/10111/ (assuming
> it's correct and tested) seems like a full cleanup but it's way too
> complex for 4.1 or the stable kernels.
>
> Ralf
>
>
All 3 patches seems working (I tested), but if you don't like mine then
I prefer Paul's patch more - it concentrates stuff more closely and
removes some assembly stuff.
Besides that, it introduces lose_fpu_inatomic() which is needed for me :)
- Leonid.
WARNING: multiple messages have this Message-ID (diff)
From: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>
To: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org, rusty@rustcorp.com.au,
alexinbeijing@gmail.com, paul.burton@imgtec.com,
david.daney@cavium.com, alex@alex-smith.me.uk,
linux-kernel@vger.kernel.org, james.hogan@imgtec.com,
markos.chandras@imgtec.com, macro@linux-mips.org,
eunb.song@samsung.com, manuel.lauss@gmail.com,
andreas.herrmann@caviumnetworks.com
Subject: Re: [PATCH 1/2] MIPS: MSA: bugfix - disable MSA during thread switch correctly
Date: Fri, 22 May 2015 17:00:42 -0700 [thread overview]
Message-ID: <555FC32A.4020308@imgtec.com> (raw)
Message-ID: <20150523000042.gFN5f85Ke0Rffz2GPOqWvUGqHgIN7wa0x5c0XdfwhwE@z> (raw)
In-Reply-To: <20150522232019.GA10556@linux-mips.org>
On 05/22/2015 04:20 PM, Ralf Baechle wrote:
> On Fri, May 22, 2015 at 11:37:34AM -0700, Leonid Yegoshin wrote:
>
>> On 05/22/2015 02:38 AM, Ralf Baechle wrote:
>>> Just move the call to finish_arch_switch().
>> It might be a problem later, then a correct MSA partiton starts working. It
>> should be tight to saving MSA registers in that case.
>>
>>> Your rewrite also dropped the if (cpu_has_msa) condition from
>>> disable_msa() probably causing havoc on lots of CPUs which will likely not
>>> decode the set bits of the MFC0/MTC0 instructions thus end up accessing
>>> Config0. Ralf
>> Right before this chunk of code there is a saving MSA registers. Does it
>> causing a havoc or else?
>>
>> May I ask you to look into switch_to macro to figure out how "if
>> (cpu_has_msa)" check works in this case?
> Ah sorry I now see that your added code is not executed for all CPUs but
> only those having MSA. So then it's safe.
>
> Still I don't stylistically like defining the register t4 in the middle
> of the code.
>
> Below my suggested patch. It's advantage is that for non-MSA platforms
> the call to disable_msa() will be removed entirely.
>
> Something like Paul's http://patchwork.linux-mips.org/patch/10111/ (assuming
> it's correct and tested) seems like a full cleanup but it's way too
> complex for 4.1 or the stable kernels.
>
> Ralf
>
>
All 3 patches seems working (I tested), but if you don't like mine then
I prefer Paul's patch more - it concentrates stuff more closely and
removes some assembly stuff.
Besides that, it introduces lose_fpu_inatomic() which is needed for me :)
- Leonid.
next prev parent reply other threads:[~2015-05-23 0:02 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-19 21:13 [PATCH 0/2] MIPS: MSA: bugfixes of context switch Leonid Yegoshin
2015-05-19 21:13 ` Leonid Yegoshin
2015-05-19 21:13 ` [PATCH 1/2] MIPS: MSA: bugfix - disable MSA during thread switch correctly Leonid Yegoshin
2015-05-19 21:13 ` Leonid Yegoshin
2015-05-21 9:12 ` Paul Burton
2015-05-21 9:12 ` Paul Burton
2015-05-21 16:11 ` Leonid Yegoshin
2015-05-21 16:20 ` [PATCH] MIPS: tidy up FPU context switching Paul Burton
2015-05-21 16:20 ` Paul Burton
2015-05-22 10:42 ` [PATCH v2] " Paul Burton
2015-05-22 10:42 ` Paul Burton
2015-05-22 9:38 ` [PATCH 1/2] MIPS: MSA: bugfix - disable MSA during thread switch correctly Ralf Baechle
2015-05-22 18:37 ` Leonid Yegoshin
2015-05-22 18:37 ` Leonid Yegoshin
2015-05-22 19:06 ` Leonid Yegoshin
2015-05-22 19:06 ` Leonid Yegoshin
2015-05-22 23:20 ` Ralf Baechle
2015-05-23 0:00 ` Leonid Yegoshin [this message]
2015-05-23 0:00 ` Leonid Yegoshin
2015-05-19 21:13 ` [PATCH 2/2] MIPS: MSA: bugfix of keeping MSA live context through clone or fork Leonid Yegoshin
2015-05-19 21:13 ` Leonid Yegoshin
2015-05-20 19:23 ` Leonid Yegoshin
2015-05-20 19:23 ` Leonid Yegoshin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=555FC32A.4020308@imgtec.com \
--to=leonid.yegoshin@imgtec.com \
--cc=alex@alex-smith.me.uk \
--cc=alexinbeijing@gmail.com \
--cc=andreas.herrmann@caviumnetworks.com \
--cc=david.daney@cavium.com \
--cc=eunb.song@samsung.com \
--cc=james.hogan@imgtec.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mips@linux-mips.org \
--cc=macro@linux-mips.org \
--cc=manuel.lauss@gmail.com \
--cc=markos.chandras@imgtec.com \
--cc=paul.burton@imgtec.com \
--cc=ralf@linux-mips.org \
--cc=rusty@rustcorp.com.au \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.