[PATCH 0/2] hw/arm/virt, qtests: Fix make check-qtest-aarch64 when CONFIG_ARM_GIC_TCG is unset

[PATCH 0/2] hw/arm/virt, qtests: Fix make check-qtest-aarch64 when CONFIG_ARM_GIC_TCG is unset

[Eric Auger]

When CONFIG_ARM_GIC_TCG is unset, qtests fail with
ERROR:../qom/object.c:715:object_new_with_type: assertion failed: (type != NULL)

This is due to the fact a bunch of tests currently depend on GICv3
availability. qom-test and test-hmp try to launch sbsa-ref
and xlnx-versal-virt which only support GICv3. Also a bunch of
tests use gic-version=max which currectly selects GICv3,
ignoring the fact this latter has been disabled.

This series proposes to ignore sbsa-ref and xlnx-versal-virt during
qos-test and test-hmp execution if CONFIG_ARM_GIC_TCG is unset.
Also it selects GICv2 if gic-version=max and CONFIG_ARM_GIC_TCG is
unset, in TCG mode. With those fixes make check passes.

Best Regards

Eric

This series can be found at:
https://github.com/eauger/qemu/tree/config_arm_gic_tcg_v1

Eric Auger (2):
  hw/arm/virt: Fix gic-version=max when CONFIG_ARM_GIC_TCG is unset
  tests/qtest: Special case sbsa-ref and xlnx-versal-virt if
    !CONFIG_ARM_GIC_TCG

 hw/arm/virt.c          | 4 ++++
 tests/qtest/libqtest.c | 6 ++++++
 2 files changed, 10 insertions(+)

-- 
2.26.3

[PATCH 1/2] hw/arm/virt: Fix gic-version=max when CONFIG_ARM_GIC_TCG is unset

[Eric Auger]

In TCG mode, if gic-version=max we always select GICv3 even if
CONFIG_ARM_GIC_TCG is unset. We shall rather select GICv2.
This also brings the benefit of fixing qos tests errors for tests
using gic-version=max with CONFIG_ARM_GIC_TCG unset.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Fixes: a8a5546798c3 ("hw/intc/arm_gicv3: Introduce CONFIG_ARM_GIC_TCG Kconfig selector")
---
 hw/arm/virt.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 141350bf21..2f1d4d0230 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1852,7 +1852,11 @@ static void finalize_gic_version(VirtMachineState *vms)
         vms->gic_version = VIRT_GIC_VERSION_2;
         break;
     case VIRT_GIC_VERSION_MAX:
+#ifdef CONFIG_ARM_GIC_TCG
         vms->gic_version = VIRT_GIC_VERSION_3;
+#else
+        vms->gic_version = VIRT_GIC_VERSION_2;
+#endif
         break;
     case VIRT_GIC_VERSION_HOST:
         error_report("gic-version=host requires KVM");
-- 
2.26.3

Re: [PATCH 1/2] hw/arm/virt: Fix gic-version=max when CONFIG_ARM_GIC_TCG is unset

[Andrew Jones]

On Mon, Jan 31, 2022 at 04:45:30PM +0100, Eric Auger wrote:
> In TCG mode, if gic-version=max we always select GICv3 even if
> CONFIG_ARM_GIC_TCG is unset. We shall rather select GICv2.
> This also brings the benefit of fixing qos tests errors for tests
> using gic-version=max with CONFIG_ARM_GIC_TCG unset.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Fixes: a8a5546798c3 ("hw/intc/arm_gicv3: Introduce CONFIG_ARM_GIC_TCG Kconfig selector")
> ---
>  hw/arm/virt.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 141350bf21..2f1d4d0230 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1852,7 +1852,11 @@ static void finalize_gic_version(VirtMachineState *vms)
>          vms->gic_version = VIRT_GIC_VERSION_2;
>          break;
>      case VIRT_GIC_VERSION_MAX:
> +#ifdef CONFIG_ARM_GIC_TCG
>          vms->gic_version = VIRT_GIC_VERSION_3;
> +#else
> +        vms->gic_version = VIRT_GIC_VERSION_2;
> +#endif
>          break;
>      case VIRT_GIC_VERSION_HOST:
>          error_report("gic-version=host requires KVM");
> -- 
> 2.26.3
>

Why is the config generically named "ARM_GIC_TCG" when it only controls
Arm GICV3 TCG?

Otherwise,

Reviewed-by: Andrew Jones <drjones@redhat.com>

Thanks,
drew

[PATCH 2/2] tests/qtest: Special case sbsa-ref and xlnx-versal-virt if !CONFIG_ARM_GIC_TCG

[Eric Auger]

qom-test and test-hmp shall not run tests on sbsa-ref and
xlnx-versal-virt if CONFIG_ARM_GIC_TCG is unset as those machines
always instantiate GICv3.

Otherwise the tests fail with
ERROR:../qom/object.c:715:object_new_with_type: assertion failed: (type != NULL)

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Fixes: a8a5546798c3 ("hw/intc/arm_gicv3: Introduce CONFIG_ARM_GIC_TCG Kconfig selector")
---
 tests/qtest/libqtest.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 41f4da4e54..f53983a28e 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -1394,6 +1394,12 @@ void qtest_cb_for_every_machine(void (*cb)(const char *machine),
             g_str_equal("xenpv", machines[i].name)) {
             continue;
         }
+#ifndef CONFIG_ARM_GIC_TCG
+        if (!strncmp("sbsa-ref", machines[i].name, 8) ||
+            !strncmp("xlnx-versal-virt", machines[i].name, 16)) {
+            continue;
+        }
+#endif
         if (!skip_old_versioned ||
             !qtest_is_old_versioned_machine(machines[i].name)) {
             cb(machines[i].name);
-- 
2.26.3

Re: [PATCH 2/2] tests/qtest: Special case sbsa-ref and xlnx-versal-virt if !CONFIG_ARM_GIC_TCG

[Andrew Jones]

On Mon, Jan 31, 2022 at 04:45:31PM +0100, Eric Auger wrote:
> qom-test and test-hmp shall not run tests on sbsa-ref and
> xlnx-versal-virt if CONFIG_ARM_GIC_TCG is unset as those machines
> always instantiate GICv3.
> 
> Otherwise the tests fail with
> ERROR:../qom/object.c:715:object_new_with_type: assertion failed: (type != NULL)
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Fixes: a8a5546798c3 ("hw/intc/arm_gicv3: Introduce CONFIG_ARM_GIC_TCG Kconfig selector")
> ---
>  tests/qtest/libqtest.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index 41f4da4e54..f53983a28e 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -1394,6 +1394,12 @@ void qtest_cb_for_every_machine(void (*cb)(const char *machine),
>              g_str_equal("xenpv", machines[i].name)) {
>              continue;
>          }
> +#ifndef CONFIG_ARM_GIC_TCG
> +        if (!strncmp("sbsa-ref", machines[i].name, 8) ||
> +            !strncmp("xlnx-versal-virt", machines[i].name, 16)) {
> +            continue;
> +        }
> +#endif

Hmm, if these machine types completely depend on userspace gicv3
emulation, i.e. no way to use in-kernel gic or another tcg gic
model, then I guess they shouldn't be built at all when ARM_GIC_TCG
isn't configured. I.e.

diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 2e0049196d6c..d7cc028b049d 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -209,6 +209,7 @@ config REALVIEW
 
 config SBSA_REF
     bool
+    depends on ARM_GIC_TCG
     imply PCI_DEVICES
     select AHCI
     select ARM_SMMUV3
@@ -378,6 +379,7 @@ config XLNX_ZYNQMP_ARM
 
 config XLNX_VERSAL
     bool
+    depends on ARM_GIC_TCG
     select ARM_GIC
     select PL011
     select CADENCE


Thanks,
drew

>          if (!skip_old_versioned ||
>              !qtest_is_old_versioned_machine(machines[i].name)) {
>              cb(machines[i].name);
> -- 
> 2.26.3
> 

Re: [PATCH 2/2] tests/qtest: Special case sbsa-ref and xlnx-versal-virt if !CONFIG_ARM_GIC_TCG

[Peter Maydell]

On Mon, 31 Jan 2022 at 15:59, Andrew Jones <drjones@redhat.com> wrote:
> Hmm, if these machine types completely depend on userspace gicv3
> emulation, i.e. no way to use in-kernel gic or another tcg gic
> model, then I guess they shouldn't be built at all when ARM_GIC_TCG
> isn't configured. I.e.
>
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 2e0049196d6c..d7cc028b049d 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -209,6 +209,7 @@ config REALVIEW
>
>  config SBSA_REF
>      bool
> +    depends on ARM_GIC_TCG
>      imply PCI_DEVICES
>      select AHCI
>      select ARM_SMMUV3
> @@ -378,6 +379,7 @@ config XLNX_ZYNQMP_ARM
>
>  config XLNX_VERSAL
>      bool
> +    depends on ARM_GIC_TCG
>      select ARM_GIC
>      select PL011
>      select CADENCE

I kind of agree, but isn't this kind of mixing two things?

(1) Both these machines require a GICv3 and a GICv2 won't do,
so they should do something that says "if you want this
machine type, you need a GICv3 device"

(2) Both these machines don't work with KVM or hvf, so if we're
not building TCG then there's no point configuring in these
machine models (a property they share with every other arm
machine type except "virt", currently)

thanks
-- PMM

Re: [PATCH 2/2] tests/qtest: Special case sbsa-ref and xlnx-versal-virt if !CONFIG_ARM_GIC_TCG

[Andrew Jones]

On Mon, Jan 31, 2022 at 04:05:06PM +0000, Peter Maydell wrote:
> On Mon, 31 Jan 2022 at 15:59, Andrew Jones <drjones@redhat.com> wrote:
> > Hmm, if these machine types completely depend on userspace gicv3
> > emulation, i.e. no way to use in-kernel gic or another tcg gic
> > model, then I guess they shouldn't be built at all when ARM_GIC_TCG
> > isn't configured. I.e.
> >
> > diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> > index 2e0049196d6c..d7cc028b049d 100644
> > --- a/hw/arm/Kconfig
> > +++ b/hw/arm/Kconfig
> > @@ -209,6 +209,7 @@ config REALVIEW
> >
> >  config SBSA_REF
> >      bool
> > +    depends on ARM_GIC_TCG
> >      imply PCI_DEVICES
> >      select AHCI
> >      select ARM_SMMUV3
> > @@ -378,6 +379,7 @@ config XLNX_ZYNQMP_ARM
> >
> >  config XLNX_VERSAL
> >      bool
> > +    depends on ARM_GIC_TCG
> >      select ARM_GIC
> >      select PL011
> >      select CADENCE
> 
> I kind of agree, but isn't this kind of mixing two things?

How about two dependencies?

> 
> (1) Both these machines require a GICv3 and a GICv2 won't do,
> so they should do something that says "if you want this
> machine type, you need a GICv3 device"

depends on ARM_GIC_TCG   (IMO, could use a rename to be gicv3 specific)

> 
> (2) Both these machines don't work with KVM or hvf, so if we're
> not building TCG then there's no point configuring in these
> machine models (a property they share with every other arm
> machine type except "virt", currently)

depends on TCG

Thanks,
drew

Re: [PATCH 2/2] tests/qtest: Special case sbsa-ref and xlnx-versal-virt if !CONFIG_ARM_GIC_TCG

[Peter Maydell]

On Mon, 31 Jan 2022 at 16:14, Andrew Jones <drjones@redhat.com> wrote:
>
> On Mon, Jan 31, 2022 at 04:05:06PM +0000, Peter Maydell wrote:
> > On Mon, 31 Jan 2022 at 15:59, Andrew Jones <drjones@redhat.com> wrote:
> > > Hmm, if these machine types completely depend on userspace gicv3
> > > emulation, i.e. no way to use in-kernel gic or another tcg gic
> > > model, then I guess they shouldn't be built at all when ARM_GIC_TCG
> > > isn't configured. I.e.
> > >
> > > diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> > > index 2e0049196d6c..d7cc028b049d 100644
> > > --- a/hw/arm/Kconfig
> > > +++ b/hw/arm/Kconfig
> > > @@ -209,6 +209,7 @@ config REALVIEW
> > >
> > >  config SBSA_REF
> > >      bool
> > > +    depends on ARM_GIC_TCG
> > >      imply PCI_DEVICES
> > >      select AHCI
> > >      select ARM_SMMUV3
> > > @@ -378,6 +379,7 @@ config XLNX_ZYNQMP_ARM
> > >
> > >  config XLNX_VERSAL
> > >      bool
> > > +    depends on ARM_GIC_TCG
> > >      select ARM_GIC
> > >      select PL011
> > >      select CADENCE
> >
> > I kind of agree, but isn't this kind of mixing two things?
>
> How about two dependencies?
>
> >
> > (1) Both these machines require a GICv3 and a GICv2 won't do,
> > so they should do something that says "if you want this
> > machine type, you need a GICv3 device"
>
> depends on ARM_GIC_TCG   (IMO, could use a rename to be gicv3 specific)

ARM_GIC_TCG has a "depends on ARM_GIC && TCG", though.
"I need a GICv3" ought in principle to be satisfiable by
the KVM GICv3.

Part of the problem here is that we've let ARM_GIC mean both
GICv2 and GICv3.

> > (2) Both these machines don't work with KVM or hvf, so if we're
> > not building TCG then there's no point configuring in these
> > machine models (a property they share with every other arm
> > machine type except "virt", currently)
>
> depends on TCG

I would prefer a way of phrasing this that only required
us to say it in the one machine that can handle not-TCG
(virt) rather than in the large number of machines that cannot...

-- PMM

Re: [PATCH 2/2] tests/qtest: Special case sbsa-ref and xlnx-versal-virt if !CONFIG_ARM_GIC_TCG

[Eric Auger]

Hi Drew,

On 1/31/22 5:14 PM, Andrew Jones wrote:
> On Mon, Jan 31, 2022 at 04:05:06PM +0000, Peter Maydell wrote:
>> On Mon, 31 Jan 2022 at 15:59, Andrew Jones <drjones@redhat.com> wrote:
>>> Hmm, if these machine types completely depend on userspace gicv3
>>> emulation, i.e. no way to use in-kernel gic or another tcg gic
>>> model, then I guess they shouldn't be built at all when ARM_GIC_TCG
>>> isn't configured. I.e.
>>>
>>> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
>>> index 2e0049196d6c..d7cc028b049d 100644
>>> --- a/hw/arm/Kconfig
>>> +++ b/hw/arm/Kconfig
>>> @@ -209,6 +209,7 @@ config REALVIEW
>>>
>>>  config SBSA_REF
>>>      bool
>>> +    depends on ARM_GIC_TCG
>>>      imply PCI_DEVICES
>>>      select AHCI
>>>      select ARM_SMMUV3
>>> @@ -378,6 +379,7 @@ config XLNX_ZYNQMP_ARM
>>>
>>>  config XLNX_VERSAL
>>>      bool
>>> +    depends on ARM_GIC_TCG
>>>      select ARM_GIC
>>>      select PL011
>>>      select CADENCE
>> I kind of agree, but isn't this kind of mixing two things?
> How about two dependencies?
>
>> (1) Both these machines require a GICv3 and a GICv2 won't do,
>> so they should do something that says "if you want this
>> machine type, you need a GICv3 device"
> depends on ARM_GIC_TCG   (IMO, could use a rename to be gicv3 specific)
Yep I think it would be clearer to rename the CONFIG.
>
>> (2) Both these machines don't work with KVM or hvf, so if we're
>> not building TCG then there's no point configuring in these
>> machine models (a property they share with every other arm
>> machine type except "virt", currently)
> depends on TCG

That's what I would be inclined to do as well

Thanks

Eric
>
> Thanks,
> drew
>

Re: [PATCH 2/2] tests/qtest: Special case sbsa-ref and xlnx-versal-virt if !CONFIG_ARM_GIC_TCG

[Eric Auger]

Hi Drew,

On 1/31/22 4:59 PM, Andrew Jones wrote:
> On Mon, Jan 31, 2022 at 04:45:31PM +0100, Eric Auger wrote:
>> qom-test and test-hmp shall not run tests on sbsa-ref and
>> xlnx-versal-virt if CONFIG_ARM_GIC_TCG is unset as those machines
>> always instantiate GICv3.
>>
>> Otherwise the tests fail with
>> ERROR:../qom/object.c:715:object_new_with_type: assertion failed: (type != NULL)
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> Fixes: a8a5546798c3 ("hw/intc/arm_gicv3: Introduce CONFIG_ARM_GIC_TCG Kconfig selector")
>> ---
>>  tests/qtest/libqtest.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
>> index 41f4da4e54..f53983a28e 100644
>> --- a/tests/qtest/libqtest.c
>> +++ b/tests/qtest/libqtest.c
>> @@ -1394,6 +1394,12 @@ void qtest_cb_for_every_machine(void (*cb)(const char *machine),
>>              g_str_equal("xenpv", machines[i].name)) {
>>              continue;
>>          }
>> +#ifndef CONFIG_ARM_GIC_TCG
>> +        if (!strncmp("sbsa-ref", machines[i].name, 8) ||
>> +            !strncmp("xlnx-versal-virt", machines[i].name, 16)) {
>> +            continue;
>> +        }
>> +#endif
> Hmm, if these machine types completely depend on userspace gicv3
> emulation, i.e. no way to use in-kernel gic or another tcg gic
> model, then I guess they shouldn't be built at all when ARM_GIC_TCG
> isn't configured. I.e.

Yes your suggestion make sense to me, thatw would be a better fix indeed.

Eric
>
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 2e0049196d6c..d7cc028b049d 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -209,6 +209,7 @@ config REALVIEW
>  
>  config SBSA_REF
>      bool
> +    depends on ARM_GIC_TCG
>      imply PCI_DEVICES
>      select AHCI
>      select ARM_SMMUV3
> @@ -378,6 +379,7 @@ config XLNX_ZYNQMP_ARM
>  
>  config XLNX_VERSAL
>      bool
> +    depends on ARM_GIC_TCG
>      select ARM_GIC
>      select PL011
>      select CADENCE
>
>
> Thanks,
> drew
>
>>          if (!skip_old_versioned ||
>>              !qtest_is_old_versioned_machine(machines[i].name)) {
>>              cb(machines[i].name);
>> -- 
>> 2.26.3
>>