Re: [PATCH for-8.1 1/1] hw/riscv/virt.c: change 'aclint' TCG check

[Alistair Francis]

On Fri, Aug 11, 2023 at 12:03 PM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> The 'aclint' property is being conditioned with tcg acceleration in
> virt_machine_class_init(). But acceleration code starts later than the
> class init of the board, meaning that tcg_enabled() will be always be
> false during class_init(), and the option is never being declared even
> when declaring TCG accel:
>
> $ ./build/qemu-system-riscv64 -M virt,accel=tcg,aclint=on
> qemu-system-riscv64: Property 'virt-machine.aclint' not found
>
> Fix it by moving the check from class_init() to machine_init(). Tune the
> description to mention that the option is TCG only.
>
> Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
> Fixes: c0716c81b ("hw/riscv/virt: Restrict ACLINT to TCG")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1823
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/riscv/virt.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index d90286dc46..99c4e6314b 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -1350,6 +1350,11 @@ static void virt_machine_init(MachineState *machine)
>          exit(1);
>      }
>
> +    if (!tcg_enabled() && s->have_aclint) {
> +        error_report("'aclint' is only available with TCG acceleration");
> +        exit(1);
> +    }
> +
>      /* Initialize sockets */
>      mmio_irqchip = virtio_irqchip = pcie_irqchip = NULL;
>      for (i = 0; i < socket_count; i++) {
> @@ -1683,13 +1688,14 @@ static void virt_machine_class_init(ObjectClass *oc, 
> void *data)
>      machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
>  #endif
>
> -    if (tcg_enabled()) {
> -        object_class_property_add_bool(oc, "aclint", virt_get_aclint,
> -                                       virt_set_aclint);
> -        object_class_property_set_description(oc, "aclint",
> -                                              "Set on/off to enable/disable "
> -                                              "emulating ACLINT devices");
> -    }
> +
> +    object_class_property_add_bool(oc, "aclint", virt_get_aclint,
> +                                   virt_set_aclint);
> +    object_class_property_set_description(oc, "aclint",
> +                                          "(TCG only) Set on/off to "
> +                                          "enable/disable emulating "
> +                                          "ACLINT devices");
> +
>      object_class_property_add_str(oc, "aia", virt_get_aia,
>                                    virt_set_aia);
>      object_class_property_set_description(oc, "aia",
> --
> 2.41.0
>
>