On Thu, 12 May 2022 17:05:41 +0100 Peter Maydell <peter.mayd...@linaro.org> wrote:
> On Thu, 12 May 2022 at 16:59, Eric Auger <eric.au...@redhat.com> wrote: > > > > Hi Peter, > > > > On 5/12/22 15:08, Peter Maydell wrote: > > > On Thu, 5 Mar 2020 at 16:52, Eric Auger <eric.au...@redhat.com> wrote: > > >> The tests themselves are the same as the ISA device ones. > > >> Only the main() changes as the "tpm-tis-device" device gets > > >> instantiated. Also the base address of the device is not > > >> 0xFED40000 anymore but matches the base address of the > > >> ARM virt platform bus. > > >> > > >> Signed-off-by: Eric Auger <eric.au...@redhat.com> > > >> Reviewed-by: Stefan Berger <stef...@linux.ibm.com> > > > Hi Eric; the commit adding this test is from back in 2020, but I've > > > just noticed something a bit odd about it: > > > > > >> + args = g_strdup_printf( > > >> + "-machine virt,gic-version=max -accel tcg " > > >> + "-chardev socket,id=chr,path=%s " > > >> + "-tpmdev emulator,id=dev,chardev=chr " > > >> + "-device tpm-tis-device,tpmdev=dev", > > >> + test.addr->u.q_unix.path); > > > This 'virt' command line doesn't specify a CPU type, so it > > > will end up running with a Cortex-A15 (32-bit). Was > > > that intended? Also, it will get a GICv3, which is a > > > definitely odd combination with an A15, which was a GICv2 CPU... > > no it is not intended. I guess it should include "-cpu max" too > > as arm-cpu-features.c does? > > Seems like a reasonable thing to set, yes. > > > > I noticed this because I have some recent GICv3 patches which > > > end up asserting if the GICv3 and a non-GICv3 CPU are used together, > > > and this test case triggers them. Since the user can also cause > > > an assert with that kind of command line I'm going to rework them > > > (either to make the virt board fail cleanly or else to make the > > > GICv3 code do something plausible even if the real hardware CPU > > > nominally didn't have a GICv3). But maybe we should make this > > > test case not use a non-standard combination anyway? (The meson > > > conversion seems to have resulted in this test being run under > > > qemu-system-arm as well, incidentally, so I guess we would want > > > it to specify either 'a 64 bit CPU and GICv3' or 'a 32 bit > > > CPU and GICv2' accordingly. Or limit the test to aarch64...) > > limiting the test to aarch64 may be enough? > > Mmm, if running the test under 'qemu-system-arm' isn't giving > us interesting extra coverage we might as well save the CI cycles. agreed, we probably should limit all ARM tests in bios-tables-test to aarch64 'qemu-system-arm' doesn't give us anything extra on top of what aarch64 already does. > > -- PMM >