Re: [kvm-unit-tests PATCH] powerpc: Check whether TM is available before running other tests

[Suraj Jitindar Singh <sjitindarsingh@gmail.com>]

On Wed, 2016-09-28 at 12:18 +0200, Thomas Huth wrote:
> Transactional memory is currently only supported on KVM-HV, and
> not yet on KVM-PR. So it's better to check the device tree first
> and fail gracefully if it's not available.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  powerpc/tm.c | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/powerpc/tm.c b/powerpc/tm.c
> index 6ce750a..83d9d3d 100644
> --- a/powerpc/tm.c
> +++ b/powerpc/tm.c
> @@ -10,6 +10,32 @@
>  #include <asm/processor.h>
>  #include <asm/handlers.h>
>  #include <asm/smp.h>
> +#include <asm/setup.h>
> +#include <devicetree.h>
> +
> +/* Check "ibm,pa-features" property of a CPU node for the TM flag */
> +static void cpu_has_tm(int fdtnode, u32 regval __unused, void *ptr)
> +{
> +	const struct fdt_property *prop;
> +	int plen;
> +
> +	prop = fdt_get_property(dt_fdt(), fdtnode, "ibm,pa-
> features", &plen);
> +	assert(prop != NULL);
> +
> +	if (plen >= 26 && prop->data[1] = 0 && (prop->data[24] &
> 0x80) != 0)
If you're checking prop->data[1] = 0 to verify device tree consistency
then maybe it's worth checking that plen = prop->data[0] as well.

Although in the current state these are hard coded into qemu and
unlikely to ever be wrong it's probably nice to check.

It might also be worth printing some error message if these consistency
checks fail as it's probably an indicator of a larger problem somewhere
else than anything actually wrong with the test.
> +		*(int *)ptr += 1;
> +}
> +
> +/* Check whether all CPU nodes have the TM flag */
> +static bool all_cpus_have_tm(void)
> +{
> +	int ret;
> +	int available = 0;
> +
> +	ret = dt_for_each_cpu_node(cpu_has_tm, &available);
> +
> +	return ret = 0 && available = nr_cpus;
> +}
>  
>  static int h_cede(void)
>  {
> @@ -106,6 +132,11 @@ int main(int argc, char **argv)
>  
>  	report_prefix_push("tm");
>  
> +	i = all_cpus_have_tm();
> +	report_xfail("TM available in 'ibm,pa-features' property",
> !i, i);
> +	if (!i)
> +		return report_summary();
> +
>  	all = argc = 1 || !strcmp(argv[1], "all");
>  
>  	for (i = 0; hctests[i].name != NULL; i++) {
Thanks for adding this check, didn't cross my mind when implementing
the test.

In its current state or with changes:
Reviewed-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>