Re: [Qemu-devel] [PATCH v4 7/7] mips/kvm: Support MSA in MIPS KVM guests

[James Hogan <james.hogan@imgtec.com>]

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

Hi Leon,

On Tue, Feb 02, 2016 at 09:58:46AM +0000, Leon Alrae wrote:
> Hi James,
> 
> On 18/12/15 10:46, James Hogan wrote:
> > @@ -611,17 +664,51 @@ static int kvm_mips_get_fpu_registers(CPUState *cs)
> >              restore_flush_mode(env);
> >          }
> >  
> > -        /* Floating point registers */
> > -        for (i = 0; i < 32; ++i) {
> > -            if (env->CP0_Status & (1 << CP0St_FR)) {
> > -                err = kvm_mips_get_one_ureg64(cs, KVM_REG_MIPS_FPR_64(i),
> > -                                              &env->active_fpu.fpr[i].d);
> > -            } else {
> > -                err = kvm_mips_get_one_ureg(cs, KVM_REG_MIPS_FPR_32(i),
> > -                                      &env->active_fpu.fpr[i].w[FP_ENDIAN_IDX]);
> > +        /*
> > +         * FPU register state is a subset of MSA vector state, so don't save FPU
> > +         * registers if we're emulating a CPU with MSA.
> > +         */
> > +        if (!(env->CP0_Config3 & (1 << CP0C3_MSAP))) {
> > +            /* Floating point registers */
> > +            for (i = 0; i < 32; ++i) {
> > +                if (env->CP0_Status & (1 << CP0St_FR)) {
> > +                    err = kvm_mips_get_one_ureg64(cs, KVM_REG_MIPS_FPR_64(i),
> > +                                                  &env->active_fpu.fpr[i].d);
> > +                } else {
> > +                    err = kvm_mips_get_one_ureg(cs, KVM_REG_MIPS_FPR_32(i),
> > +                                    &env->active_fpu.fpr[i].w[FP_ENDIAN_IDX]);
> > +                }
> > +                if (err < 0) {
> > +                    DPRINTF("%s: Failed to get FPR%u (%d)\n", __func__, i, err);
> > +                    ret = err;
> > +                }
> >              }
> > +        }
> > +    }
> > +
> > +    /* Only get MSA state if we're emulating a CPU with MSA */
> > +    if (env->CP0_Config3 & (1 << CP0C3_MSAP)) {
> > +        /* MSA Control Registers */
> > +        err = kvm_mips_get_one_reg(cs, KVM_REG_MIPS_MSA_IR,
> > +                                   &env->msair);
> > +        if (err < 0) {
> > +            DPRINTF("%s: Failed to get MSA_IR (%d)\n", __func__, err);
> > +            ret = err;
> > +        }
> > +        err = kvm_mips_get_one_reg(cs, KVM_REG_MIPS_MSA_CSR,
> > +                                   &env->active_tc.msacsr);
> > +        if (err < 0) {
> > +            DPRINTF("%s: Failed to get MSA_CSR (%d)\n", __func__, err);
> > +            ret = err;
> > +        }
> 
> Shouldn't MSA's float_status (i.e. msa_fp_status) be restored to reflect
> MSACSR?

Yes, it probably should. Good spot!

I can use restore_msa_fp_status(env); here, and actually the FP one
above (in previous patch) can change to restore_fp_status(env); too,
since it does the same thing.

I've rebased and resolved the uint64 thing too, would you like a resend?

Thanks
James

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]