Re: [PATCH v6 30/39] accel: Propagate AccelState to AccelClass::init_machine()

[Zhao Liu <zhao1.liu@xxxxxxxxx>]

On Thu, Jul 03, 2025 at 07:32:36PM +0200, Philippe Mathieu-Daudé wrote:
> Date: Thu,  3 Jul 2025 19:32:36 +0200
> From: Philippe Mathieu-Daudé <philmd@xxxxxxxxxx>
> Subject: [PATCH v6 30/39] accel: Propagate AccelState to
>  AccelClass::init_machine()
> X-Mailer: git-send-email 2.49.0
> 
> In order to avoid init_machine() to call current_accel(),
> pass AccelState along.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@xxxxxxxxxx>
> Reviewed-by: Richard Henderson <richard.henderson@xxxxxxxxxx>
> Reviewed-by: Alex Bennée <alex.bennee@xxxxxxxxxx>
> ---
>  include/qemu/accel.h        | 2 +-
>  accel/accel-system.c        | 2 +-
>  accel/hvf/hvf-all.c         | 2 +-
>  accel/kvm/kvm-all.c         | 2 +-
>  accel/qtest/qtest.c         | 2 +-
>  accel/tcg/tcg-all.c         | 2 +-
>  accel/xen/xen-all.c         | 2 +-
>  bsd-user/main.c             | 2 +-
>  linux-user/main.c           | 2 +-
>  target/i386/nvmm/nvmm-all.c | 2 +-
>  target/i386/whpx/whpx-all.c | 2 +-
>  11 files changed, 11 insertions(+), 11 deletions(-)

...

> diff --git a/accel/accel-system.c b/accel/accel-system.c
> index b5b368c6a9c..fb8abe38594 100644
> --- a/accel/accel-system.c
> +++ b/accel/accel-system.c
> @@ -37,7 +37,7 @@ int accel_init_machine(AccelState *accel, MachineState *ms)
>      int ret;
>      ms->accelerator = accel;
>      *(acc->allowed) = true;
> -    ret = acc->init_machine(ms);
> +    ret = acc->init_machine(accel, ms);

Now we've already set "ms->accelerator", so that we could get @accel
by ms->accelerator.

But considerring the user emulation, where the @ms is NULL, and for
these cases, it needs to bring current_accel() back in patch 32.

Anyway, this solution is also fine for me, so,

Reviewed-by: Zhao Liu <zhao1.liu@xxxxxxxxx>


...But there're still more comments/questions about user emulation:

> --- a/bsd-user/main.c
> +++ b/bsd-user/main.c
> @@ -474,7 +474,7 @@ int main(int argc, char **argv)
>                                   opt_one_insn_per_tb, &error_abort);
>          object_property_set_int(OBJECT(accel), "tb-size",
>                                  opt_tb_size, &error_abort);
> -        ac->init_machine(NULL);
> +        ac->init_machine(accel, NULL);

Not the issue about this patch though,

it seems user emulation doesn't set acc->allowed. At least TCG enabled
is necessary, I guess?

>      }
>  
>      /*
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 5ac5b55dc65..a9142ee7268 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -820,7 +820,7 @@ int main(int argc, char **argv, char **envp)
>                                   opt_one_insn_per_tb, &error_abort);
>          object_property_set_int(OBJECT(accel), "tb-size",
>                                  opt_tb_size, &error_abort);
> -        ac->init_machine(NULL);
> +        ac->init_machine(accel, NULL);

Ditto.

>      }
>  
>      /*

Thanks,
Zhao