Re: [PATCH v2] target/loongarch: Add TCG macro in structure CPUArchState

[maobibo]

On 2024/3/5 上午12:53, Richard Henderson wrote:
> On 3/3/24 16:18, Bibo Mao wrote:
> > @@ -696,11 +700,15 @@ void loongarch_cpu_dump_state(CPUState *cs, FILE 
> > *f, int flags)
> >   {
> >       LoongArchCPU *cpu = LOONGARCH_CPU(cs);
> >       CPULoongArchState *env = &cpu->env;
> > -    int i;
> > +    int i, fp_status;
> > +#ifdef CONFIG_TCG
> > +    fp_status = get_float_exception_flags(&env->fp_status);
> > +#else
> > +    fp_status = 0;
> > +#endif
> >       qemu_fprintf(f, " PC=%016" PRIx64 " ", env->pc);
> > -    qemu_fprintf(f, " FCSR0 0x%08x  fp_status 0x%02x\n", env->fcsr0,
> > -                 get_float_exception_flags(&env->fp_status));
> > +    qemu_fprintf(f, " FCSR0 0x%08x  fp_status 0x%02x\n", env->fcsr0, 
> > fp_status);
>
> fp_status, I think, is unnecessary to print all of the time.
>
> In update_fcsr0_mask, we ensure that fcsr0 is updated and 
> fp_status.exception_flags is 0. So I would expect this field to be 0 all 
> of the time -- anything else is a bug.
yes, fp_status is temporary status during float instruction translation, 
it will clear to 0 when fp instruction translation is done.

Will remove print sentence.

> > +#if defined(CONFIG_TCG) && !defined(CONFIG_USER_ONLY)
> > +static bool tlb_needed(void *opaque)
> > +{
> > +    if (kvm_enabled()) {
> > +        return false;
> > +    }
> > +
> > +    return true;
> > +}
>
> Better as return tcg_enabled();
Will return as tcg_enabled(), it is more simpler.

Regards
Bibo Mao
> r~