Re: [PATCH] target/arm: Store FPSR cumulative exception bits in env->vfp.fpsr

[Richard Henderson <richard.henderson@linaro.org>]

On 10/11/24 09:24, Peter Maydell wrote:
> Currently we store the FPSR cumulative exception bits in the
> float_status fields, and use env->vfp.fpsr only for the NZCV bits.
> (The QC bit is stored in env->vfp.qc[].)
> 
> This works for TCG, but if QEMU was built without CONFIG_TCG (i.e.
> with KVM support only) then we use the stub versions of
> vfp_get_fpsr_from_host() and vfp_set_fpsr_to_host() which do nothing,
> throwing away the cumulative exception bit state.  The effect is that
> if the FPSR state is round-tripped from KVM to QEMU then we lose the
> cumulative exception bits.  In particular, this will happen if the VM
> is migrated.  There is no user-visible bug when using KVM with a QEMU
> binary that was built with CONFIG_TCG.

Ok, noted.

> -    int i = vfp_exceptbits_to_host(val);
> -    set_float_exception_flags(i, &env->vfp.fp_status);
> +    set_float_exception_flags(0, &env->vfp.fp_status);
>       set_float_exception_flags(0, &env->vfp.fp_status_f16);
>       set_float_exception_flags(0, &env->vfp.standard_fp_status);
>       set_float_exception_flags(0, &env->vfp.standard_fp_status_f16);

I will note that this affects can_use_fpu() in softfpu.c, at least for the first operation 
after setting FPSR: without float_flag_inexact set, we will always use the slow path.

In glibc, when manipulating FPSR, it tends to come in pairs: feholdsetround + 
feresetround.  With this, half of the time we'll be setting the exception flags to 0, and 
half of the time we'll be restoring old values.  The latter set is reasonably like to have 
inexact set.

It might be worthwhile to set float_flag_inexact in all of these when IXC is set.  We will 
still sum to the same result when reading later.

That said, this fixes a bug and is not wrong so
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~