Linux MIPS Architecture development
 help / color / mirror / Atom feed
From: Paul Burton <paul.burton@imgtec.com>
To: James Hogan <james.hogan@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>, <linux-mips@linux-mips.org>,
	<stable@vger.kernel.org>
Subject: Re: [PATCH] MIPS: Force o32 fp64 support on 32bit MIPS64r6 kernels
Date: Fri, 17 Feb 2017 10:29:11 -0800	[thread overview]
Message-ID: <1648905.7UokBy1sI6@np-p-burton> (raw)
In-Reply-To: <01a9d2344224e76ea17ff62ffa7b75717f6f1100.1487248664.git-series.james.hogan@imgtec.com>

[-- Attachment #1: Type: text/plain, Size: 1935 bytes --]

Hi James,

On Thursday, 16 February 2017 04:39:01 PST James Hogan wrote:
> When a 32-bit kernel is configured to support MIPS64r6 (CPU_MIPS64_R6),
> MIPS_O32_FP64_SUPPORT won't be selected as it should be because
> MIPS32_O32 is disabled (o32 is already the default ABI available on
> 32-bit kernels).

Nice catch!

> This results in userland FP breakage as CP0_Status.FR is read-only 1
> since r6 (when an FPU is present) but CP0_Config5.FRE won't be set to
> emulate FR=0.

Perhaps it would be worth clarifying that what it breaks is FPU emulation or 
pre-r6 FP code running atop MIPS32r6 kernels. Since FR=1 context switching 
should work fine for r6 user code, and it would only be impacted if it 
requires emulation for some reason (which is probably why we haven't hit this 
earlier in our CI testing).

> Force o32 fp64 support in this case by also selecting
> MIPS_O32_FP64_SUPPORT from CPU_MIPS64_R6 if 32BIT.
> 
> Fixes: 4e9d324d4288 ("MIPS: Require O32 FP64 support for MIPS64 with O32
> compat") Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Paul Burton <paul.burton@imgtec.com>
> Cc: linux-mips@linux-mips.org
> Cc: <stable@vger.kernel.org> # 4.0.x-
> ---
>  arch/mips/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index b969522feb97..e2890002d6d1 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -1531,7 +1531,7 @@ config CPU_MIPS64_R6
>  	select CPU_SUPPORTS_HIGHMEM
>  	select CPU_SUPPORTS_MSA
>  	select GENERIC_CSUM
> -	select MIPS_O32_FP64_SUPPORT if MIPS32_O32
> +	select MIPS_O32_FP64_SUPPORT if 32BIT || MIPS32_O32
>  	select HAVE_KVM
>  	help
>  	  Choose this option to build a kernel for release 6 or later of the

Besides possibly clarifying the commit message above this looks good to me so:

    Reviewed-by: Paul Burton <paul.burton@imgtec.com>

Thanks,
    Paul

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Paul Burton <paul.burton@imgtec.com>
To: James Hogan <james.hogan@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>,
	linux-mips@linux-mips.org, stable@vger.kernel.org
Subject: Re: [PATCH] MIPS: Force o32 fp64 support on 32bit MIPS64r6 kernels
Date: Fri, 17 Feb 2017 10:29:11 -0800	[thread overview]
Message-ID: <1648905.7UokBy1sI6@np-p-burton> (raw)
Message-ID: <20170217182911.uCK1zMaouJEJgILGl-yyK19x5k6Cv1P79T2BbhvYbqA@z> (raw)
In-Reply-To: <01a9d2344224e76ea17ff62ffa7b75717f6f1100.1487248664.git-series.james.hogan@imgtec.com>

[-- Attachment #1: Type: text/plain, Size: 1935 bytes --]

Hi James,

On Thursday, 16 February 2017 04:39:01 PST James Hogan wrote:
> When a 32-bit kernel is configured to support MIPS64r6 (CPU_MIPS64_R6),
> MIPS_O32_FP64_SUPPORT won't be selected as it should be because
> MIPS32_O32 is disabled (o32 is already the default ABI available on
> 32-bit kernels).

Nice catch!

> This results in userland FP breakage as CP0_Status.FR is read-only 1
> since r6 (when an FPU is present) but CP0_Config5.FRE won't be set to
> emulate FR=0.

Perhaps it would be worth clarifying that what it breaks is FPU emulation or 
pre-r6 FP code running atop MIPS32r6 kernels. Since FR=1 context switching 
should work fine for r6 user code, and it would only be impacted if it 
requires emulation for some reason (which is probably why we haven't hit this 
earlier in our CI testing).

> Force o32 fp64 support in this case by also selecting
> MIPS_O32_FP64_SUPPORT from CPU_MIPS64_R6 if 32BIT.
> 
> Fixes: 4e9d324d4288 ("MIPS: Require O32 FP64 support for MIPS64 with O32
> compat") Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Paul Burton <paul.burton@imgtec.com>
> Cc: linux-mips@linux-mips.org
> Cc: <stable@vger.kernel.org> # 4.0.x-
> ---
>  arch/mips/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index b969522feb97..e2890002d6d1 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -1531,7 +1531,7 @@ config CPU_MIPS64_R6
>  	select CPU_SUPPORTS_HIGHMEM
>  	select CPU_SUPPORTS_MSA
>  	select GENERIC_CSUM
> -	select MIPS_O32_FP64_SUPPORT if MIPS32_O32
> +	select MIPS_O32_FP64_SUPPORT if 32BIT || MIPS32_O32
>  	select HAVE_KVM
>  	help
>  	  Choose this option to build a kernel for release 6 or later of the

Besides possibly clarifying the commit message above this looks good to me so:

    Reviewed-by: Paul Burton <paul.burton@imgtec.com>

Thanks,
    Paul

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  parent reply	other threads:[~2017-02-17 18:29 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-16 12:39 [PATCH] MIPS: Force o32 fp64 support on 32bit MIPS64r6 kernels James Hogan
2017-02-16 12:39 ` James Hogan
2017-02-17 18:29 ` Paul Burton [this message]
2017-02-17 18:29   ` Paul Burton
2017-02-23 12:52   ` James Hogan
2017-02-23 12:52     ` James Hogan

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=1648905.7UokBy1sI6@np-p-burton \
    --to=paul.burton@imgtec.com \
    --cc=james.hogan@imgtec.com \
    --cc=linux-mips@linux-mips.org \
    --cc=ralf@linux-mips.org \
    --cc=stable@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox