Re: [PATCH v1 13/25] hw/tricore: Add testdevice for tests in tests/tcg/

[Alex Bennée]

Philippe Mathieu-Daudé <f4bug@amsat.org> writes:

> On 4/19/21 4:54 PM, Alex Bennée wrote:
>> From: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
>> 
>> this device is used to verify the correctness of regression tests by
>> allowing guests to write their exit status to this device. This is then
>> used by qemu to exit using the written status.
>> 
>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>> Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Message-Id: <20210305170045.869437-4-kbastian@mail.uni-paderborn.de>
>> ---
>>  include/hw/tricore/tricore_testdevice.h | 38 ++++++++++++
>>  hw/tricore/tricore_testboard.c          |  8 +++
>>  hw/tricore/tricore_testdevice.c         | 82 +++++++++++++++++++++++++
>>  hw/tricore/meson.build                  |  1 +
>>  4 files changed, 129 insertions(+)
>>  create mode 100644 include/hw/tricore/tricore_testdevice.h
>>  create mode 100644 hw/tricore/tricore_testdevice.c
>
>> +#include "hw/tricore/tricore_testdevice.h"
>> +
>> +static void tricore_testdevice_write(void *opaque, hwaddr offset,
>> +                                      uint64_t value, unsigned size)
>> +{
>> +    exit(value);
>
>    ->  qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
>
> I'd rather use a 2 steps check of value such watchdog devices do
> (to be sure the guest is still in control and isn't nut).

This isn't any different to what we do for TARGET_SYS_EXIT_EXTENDED or
the various other semihosting exits. Maybe we could do a better job of
flagging that these devices (or features) give the guest an avenue to
cause QEMU to shutdown but none of these are enabled by default.

>
> A general comments, all targets require a such test feature,
> so we should have a generic user-creatable sysbus-testdev for that.

We also have the isa-debug-exit device used by x86. I believe there is
also a PCI device (pci-testdev) used to submit error-exit results for
kvm-unit-tests.

I'm all for modelling a cleaner abstraction that could be used by all
these methods and avoiding multiple exit paths but I don't want to hold
up Bastian's tests to a higher standard without addressing the other
cases. In the meantime given it improves the testing situation for
Tricore I don't think it's a major issue.

>
> Regards,
>
> Phil.


-- 
Alex Bennée