Re: [PATCH v5 10/10] test: tpm-tis: Add Sysbus TPM-TIS device test

[Eric Auger]

Hi Peter,

On 5/12/22 15:08, Peter Maydell wrote:
> On Thu, 5 Mar 2020 at 16:52, Eric Auger <eric.auger@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.auger@redhat.com>
>> Reviewed-by: Stefan Berger <stefanb@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?
>
> 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?

Eric
>
> thanks
> -- PMM
>