Re: [Qemu-devel] [PATCH v2 for-4.1] target/arm: Set VFP-related MVFR0 fields for arm926 and arm1026

From: Christophe Lyon <christophe.lyon@linaro.org>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: "Alex Bennée" <alex.bennee@linaro.org>,
	qemu-arm@nongnu.org,
	"Richard Henderson" <richard.henderson@linaro.org>,
	"QEMU Developers" <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] [PATCH v2 for-4.1] target/arm: Set VFP-related MVFR0 fields for arm926 and arm1026
Date: Fri, 12 Jul 2019 16:13:38 +0200	[thread overview]
Message-ID: <CAKdteOYTgv1OenmRhbVMTjC4vtc+8B7w_=ZtaOD3q8Y8zNeqKA@mail.gmail.com> (raw)
In-Reply-To: <20190711131241.22231-1-peter.maydell@linaro.org>

On Thu, 11 Jul 2019 at 15:12, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> The ARMv5 architecture didn't specify detailed per-feature ID
> registers. Now that we're using the MVFR0 register fields to
> gate the existence of VFP instructions, we need to set up
> the correct values in the cpu->isar structure so that we still
> provide an FPU to the guest.
>
> This fixes a regression in the arm926 and arm1026 CPUs, which
> are the only ones that both have VFP and are ARMv5 or earlier.
> This regression was introduced by the VFP refactoring, and more
> specifically by commits 1120827fa182f0e76 and 266bd25c485597c,
> which accidentally disabled VFP short-vector support and
> double-precision support on these CPUs.
>
> Reported-by: Christophe Lyon <christophe.lyon@linaro.org>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> Fixes: 1120827fa182f0e
> Fixes: 266bd25c485597c
> Fixes: https://bugs.launchpad.net/qemu/+bug/1836192
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> v1->v2: just move the arm1026_initfn() part up a few lines;
> this seems trivial so I've retained the reviewed-by tags.
>

For v1:
Tested-by: Christophe Lyon <christophe.lyon@linaro.org>

Works for me, thanks!

>  target/arm/cpu.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index e75a64a25a4..05b78ba2662 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1666,6 +1666,12 @@ static void arm926_initfn(Object *obj)
>       * set the field to indicate Jazelle support within QEMU.
>       */
>      cpu->isar.id_isar1 = FIELD_DP32(cpu->isar.id_isar1, ID_ISAR1, JAZELLE, 1);
> +    /*
> +     * Similarly, we need to set MVFR0 fields to enable double precision
> +     * and short vector support even though ARMv5 doesn't have this register.
> +     */
> +    cpu->isar.mvfr0 = FIELD_DP32(cpu->isar.mvfr0, MVFR0, FPSHVEC, 1);
> +    cpu->isar.mvfr0 = FIELD_DP32(cpu->isar.mvfr0, MVFR0, FPDP, 1);
>  }
>
>  static void arm946_initfn(Object *obj)
> @@ -1702,6 +1708,12 @@ static void arm1026_initfn(Object *obj)
>       * set the field to indicate Jazelle support within QEMU.
>       */
>      cpu->isar.id_isar1 = FIELD_DP32(cpu->isar.id_isar1, ID_ISAR1, JAZELLE, 1);
> +    /*
> +     * Similarly, we need to set MVFR0 fields to enable double precision
> +     * and short vector support even though ARMv5 doesn't have this register.
> +     */
> +    cpu->isar.mvfr0 = FIELD_DP32(cpu->isar.mvfr0, MVFR0, FPSHVEC, 1);
> +    cpu->isar.mvfr0 = FIELD_DP32(cpu->isar.mvfr0, MVFR0, FPDP, 1);
>
>      {
>          /* The 1026 had an IFAR at c6,c0,0,1 rather than the ARMv6 c6,c0,0,2 */
> --
> 2.20.1
>