Re: [PATCH-for-10.0? v2 13/14] hw/arm/virt-acpi: Do not advertise disabled GIC ITS

[Igor Mammedov <imammedo@redhat.com>]

On Fri, 4 Apr 2025 14:49:45 +0200
Philippe Mathieu-Daudé <philmd@linaro.org> wrote:

> On 4/4/25 12:41, Igor Mammedov wrote:
> > On Thu,  3 Apr 2025 17:18:28 +0200
> > Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> >   
> >> GIC ITS can be disabled at runtime using '-M its=off',
> >> which sets VirtMachineState::its = false. Check this
> >> field to avoid advertising the ITS in the MADT table.
> >>
> >> Reported-by: Udo Steinberg <udo@hypervisor.org>
> >> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2886
> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> >> Reviewed-by: Gustavo Romero <gustavo.romero@linaro.org>
> >> ---
> >>   hw/arm/virt-acpi-build.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> >> index e7e27951cb9..38a9e6fe0c5 100644
> >> --- a/hw/arm/virt-acpi-build.c
> >> +++ b/hw/arm/virt-acpi-build.c
> >> @@ -212,7 +212,7 @@ static bool its_enabled(VirtMachineState *vms)
> >>   {
> >>       VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
> >>   
> >> -    return !vmc->no_its;
> >> +    return !vmc->no_its && vms->its;  
> > 
> > It's confusing have both no_its and its,
> > it would be better to lean this mess up (i.e dedup or rename if something is poorly named)  
> 
> What about:
> 
> class: s/no_its/!its_createable/ or !its_usable
> state: s/its/its_created/ or its_used or its_in_use

looking at the code what we are essentially doing with variables in class,
is selecting its device type. And then playing scattered if/then game to decide
which 'its' to create or not create.
can we make something like this instead:

 machine_foo_class_init_vX():
  vmc->its_type = NULL;

 machine_foo_class_init_vX+1():
  vmc->its_type = accel_get_its_type()
 
and then elsewhere:

if(vmc->its_type && vms->its_enabled)
    create_its(vmc->its_type)