Re: [PATCH v2 3/3] tests/qtest: Add test for Aspeed HACE

[Thomas Huth]

On 12/03/2021 00.47, Joel Stanley wrote:
> This adds a test for the Aspeed Hash and Crypto (HACE) engine. It tests
> the currently implemented behavior of the hash functionality.
>
> The tests are similar, but are cut/pasted instead of broken out into a
> common function so the assert machinery produces useful output when a
> test fails.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>   tests/qtest/aspeed_hace-test.c | 215 +++++++++++++++++++++++++++++++++
>   MAINTAINERS                    |   1 +
>   tests/qtest/meson.build        |   3 +
>   3 files changed, 219 insertions(+)
>   create mode 100644 tests/qtest/aspeed_hace-test.c
>
> diff --git a/tests/qtest/aspeed_hace-test.c b/tests/qtest/aspeed_hace-test.c
> new file mode 100644
> index 000000000000..52501ee37afb
> --- /dev/null
> +++ b/tests/qtest/aspeed_hace-test.c
> @@ -0,0 +1,215 @@
> +/*
> + * QTest testcase for the ASPEED Hash and Crypto Engine
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + * Copyright 2021 IBM Corp.
> + */
> +
> +#include "qemu/osdep.h"
> +
> +#include "libqtest-single.h"

Bonus points for writing the test without "libqtest-single.h" and using only 
"libqtest.h" instead. We should try to avoid libqtest-single.h in new code 
since such code can not be shared with tests that run multiple instances of 
QEMU. But if it's too cumbersome, I do not insist.

> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index 2688e1bfad7f..2eec06b10094 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -156,12 +156,15 @@ qtests_npcm7xx = \
>      'npcm7xx_timer-test',
>      'npcm7xx_watchdog_timer-test'] + \
>      (slirp.found() ? ['npcm7xx_emc-test'] : [])
> +qtests_aspeed = \
> +  ['aspeed_hace-test']

Do you plan to add more aspeed tests later? If not, I think you could simply 
do it without this variable and use ['aspeed_hace-test'] below.

>   qtests_arm = \
>     (config_all_devices.has_key('CONFIG_MPS2') ? ['sse-timer-test'] : []) + \
>     (config_all_devices.has_key('CONFIG_CMSDK_APB_DUALTIMER') ? 
> ['cmsdk-apb-dualtimer-test'] : []) + \
>     (config_all_devices.has_key('CONFIG_CMSDK_APB_TIMER') ? 
> ['cmsdk-apb-timer-test'] : []) + \
>     (config_all_devices.has_key('CONFIG_CMSDK_APB_WATCHDOG') ? 
> ['cmsdk-apb-watchdog-test'] : []) + \
>     (config_all_devices.has_key('CONFIG_PFLASH_CFI02') ? ['pflash-cfi02-test'] 
> : []) +         \
> +  (config_all_devices.has_key('CONFIG_ASPEED_SOC') ? qtests_aspeed : []) + \
>     (config_all_devices.has_key('CONFIG_NPCM7XX') ? qtests_npcm7xx : []) + \
>     ['arm-cpu-features',
>      'microbit-test',

 Thomas