| Message ID | 20220131154531.429533-3-eric.auger@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/arm/virt, qtests: Fix make check-qtest-aarch64 when CONFIG_ARM_GIC_TCG is unset | expand |
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 >
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
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
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 >>
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
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 >
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);
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(+)