Re: [PATCH-for-10.0 4/6] include: Expose QemuArch in 'qemu/arch_id.h'

[Richard Henderson]

On 11/27/24 06:25, Philippe Mathieu-Daudé wrote:
> Alternatively we can restrict TCGCPUOps::arch_id to
> system emulation, using in the next patch:
>
> -- >8 --
> diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h
> index ec3d2b50a9e..6fe0e1a7e97 100644
> --- a/include/hw/core/tcg-cpu-ops.h
> +++ b/include/hw/core/tcg-cpu-ops.h
> @@ -19,8 +19,6 @@
>   #include "exec/vaddr.h"
>
>   struct TCGCPUOps {
> -    QemuArch arch_id;
> -
>       /**
>        * @initialize_once: Initialize TCG state
>        *
> @@ -58,6 +56,7 @@ struct TCGCPUOps {
>       void (*debug_excp_handler)(CPUState *cpu);
>
>   #ifdef CONFIG_USER_ONLY
> +    QemuArch arch_id;
>       /**
>        * @fake_user_interrupt: Callback for 'fake exception' handling.
>        *
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index b37995f7d0c..31a2ab18e7c 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -1072,15 +1072,20 @@ bool tcg_exec_realizefn(CPUState *cpu, Error **errp)
>   {
>       static unsigned initialized_targets;
>       const TCGCPUOps *tcg_ops = cpu->cc->tcg_ops;
> +#ifndef CONFIG_USER_ONLY
> +    unsigned arch_id = tcg_ops->arch_id;
> +#else
> +    unsigned arch_id = 1;
> +#endif
>
> -    if (!(initialized_targets & tcg_ops->arch_id)) {
> +    if (!(initialized_targets & arch_id)) {
>           /* Check mandatory TCGCPUOps handlers */
>   #ifndef CONFIG_USER_ONLY
>           assert(tcg_ops->cpu_exec_halt);
>           assert(tcg_ops->cpu_exec_interrupt);
>   #endif /* !CONFIG_USER_ONLY */
>           tcg_ops->initialize_once();
> -        initialized_targets |= tcg_ops->arch_id;
> +        initialized_targets |= arch_id;
>       }
>
> ---
>
> But it add more #ifdef'ry and doesn't seem worthwhile IMHO.

I agree, not worthwhile.

r~