Re: [PATCH v22 16/17] i386: gdbstub: only write CR0/CR2/CR3/EFER for SOFTMMU

[Richard Henderson]

On 2/24/21 5:34 AM, Claudio Fontana wrote:
> Signed-off-by: Claudio Fontana <cfontana@suse.de>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  target/i386/gdbstub.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/target/i386/gdbstub.c b/target/i386/gdbstub.c
> index 41e265fc67..9f505d6ee3 100644
> --- a/target/i386/gdbstub.c
> +++ b/target/i386/gdbstub.c
> @@ -383,26 +383,38 @@ int x86_cpu_gdb_write_register(CPUState *cs, uint8_t 
> *mem_buf, int n)
>  
>          case IDX_CTL_CR0_REG:
>              if (env->hflags & HF_CS64_MASK) {
> +#ifdef CONFIG_SOFTMMU
>                  cpu_x86_update_cr0(env, ldq_p(mem_buf));
> +#endif
>                  return 8;
>              }
> +#ifdef CONFIG_SOFTMMU
>              cpu_x86_update_cr0(env, ldl_p(mem_buf));
> +#endif
>              return 4;

It would be nice to do all these with rather less ifdefs.
And let's correctly use !CONFIG_USER_ONLY.

Without adding more stubs, may I suggest a new helper:

static target_ulong read_long_cs64(env, buf, len)
{
#ifdef TARGET_X86_64
    if (env->hflags & HF_CS64_MASK) {
        *len = 8;
        return ldq_p(buf);
    }
#endif
    *len = 4;
    return ldl_p(buf);
}

which, even by itself allows some cleanup in this function.
Then:

    case IDX_CTL_CR2_REG:
       tmp = read_long_cs64(env, mem_buf, &len);
#ifndef CONFIG_USER_ONLY
       env->cr[2] = tmp;
#endif
       return len;

which still has one ifdef, but not 2.


r~