Re: [Qemu-devel] [PATCH] mips: Correctly save/restore the FP flush-to-zero state

[Peter Maydell]

On 12 November 2014 16:07, Maciej W. Rozycki <address@hidden> wrote:
> Fix the FP state save/restore operations by saving the `flush_to_zero'
> rather than the `float_detect_tininess' setting.  There is no provision
> for the latter in MIPS hardware, whereas the former is controlled by the
> CP1.FCSR.FS bit.  As a result all the older saved state images are
> invalid as they do not restore the FP state corresponding to the
> CP1.FCSR.FS bit and may execute differently when resumed compared to the
> case where no save/restore operations have ever been made.  Therefore
> reject any such older images too and do not allow them to be loaded.

> @@ -208,12 +206,12 @@ int cpu_load(QEMUFile *f, void *opaque,
>      MIPSCPU *cpu = mips_env_get_cpu(env);
>      int i;
>
> -    if (version_id < 3) {
> +    if (version_id != CPU_SAVE_VERSION) {
>          return -EINVAL;
>      }

Shouldn't this read "if (version_id < 6)" ?
Otherwise next time somebody bumps the CPU_SAVE_VERSION it
will give another migration compatibility break without that
being very obvious.

As a longer-term cleanup I would highly recommend converting
the MIPS machine.c to use VMState structs to define its
migration (this is likely to imply another compatibility
break, so if you have any plans for supporting cross-QEMU-version
migration in future you should definitely do the conversion
before that point). The commit where we did this for ARM was
3cc1d20823 which gives you an idea of how the conversion works.
MIPS, CRIS and SPARC are the only three remaining guest CPUs
still using the old-style by-hand save/restore, so it would
be good to finally complete that transition.

thanks
-- PMM