Re: [kvm-unit-tests PATCH] s390x/cpumodel: The missing DFP facility on TCG is expected

[Cornelia Huck <cohuck@redhat.com>]

On Tue,  7 Jul 2020 07:56:19 +0200
Thomas Huth <thuth@redhat.com> wrote:

> When running the kvm-unit-tests with TCG on s390x, the cpumodel test
> always reports the error about the missing DFP (decimal floating point)
> facility. This is kind of expected, since DFP is not required for
> running Linux and thus nobody is really interested in implementing
> this facility in TCG. Thus let's mark this as an expected error instead,
> so that we can run the kvm-unit-tests also with TCG without getting
> test failures that we do not care about.

Checking for tcg seems reasonable.

> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  s390x/cpumodel.c | 51 ++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 45 insertions(+), 6 deletions(-)
> 
> diff --git a/s390x/cpumodel.c b/s390x/cpumodel.c
> index 5d232c6..4310b92 100644
> --- a/s390x/cpumodel.c
> +++ b/s390x/cpumodel.c
> @@ -11,6 +11,7 @@
>   */
>  
>  #include <asm/facility.h>
> +#include <alloc_page.h>
>  
>  static int dep[][2] = {
>  	/* from SA22-7832-11 4-98 facility indications */
> @@ -38,6 +39,49 @@ static int dep[][2] = {
>  	{ 155,  77 },
>  };
>  
> +/*
> + * A hack to detect TCG (instead of KVM): QEMU uses "TCGguest" as guest
> + * name by default when we are running with TCG (otherwise it's "KVMguest")

The guest name can be overwritten; I think it would be better to check
for something hardcoded.

Maybe the manufacturer name in SYSIB 1.1.1? When running under tcg,
it's always 'QEMU' (it's 'IBM' when running under KVM).

> + */
> +static bool is_tcg(void)
> +{
> +	bool ret = false;
> +	uint8_t *buf;
> +
> +	buf = alloc_page();
> +	if (!buf)
> +		return false;
> +
> +	if (stsi(buf, 3, 2, 2)) {
> +		goto out;
> +	}
> +
> +	/* Does the name start with "TCG" in EBCDIC? */
> +	if (buf[2048] == 0x54 && buf[2049] == 0x43 && buf[2050] == 0x47)
> +		ret = true;
> +
> +out:
> +	free_page(buf);
> +	return ret;
> +}
> +
> +static void check_dependency(int dep1, int dep2)

<bikeshed>
Can we find more speaking parameter names? facility/implied?
</bikeshed>

> +{
> +	if (test_facility(dep1)) {
> +		if (dep1 == 37) {
> +			/* TCG does not have DFP and is unlikely to
> +			 * get it implemented soon. */
> +			report_xfail(is_tcg(), test_facility(dep2),
> +				     "%d implies %d", dep1, dep2);
> +		} else {
> +			report(test_facility(dep2), "%d implies %d",
> +			       dep1, dep2);
> +		}
> +	} else {
> +		report_skip("facility %d not present", dep1);
> +	}
> +}
> +
>  int main(void)
>  {
>  	int i;
> @@ -46,12 +90,7 @@ int main(void)
>  
>  	report_prefix_push("dependency");
>  	for (i = 0; i < ARRAY_SIZE(dep); i++) {
> -		if (test_facility(dep[i][0])) {
> -			report(test_facility(dep[i][1]), "%d implies %d",
> -				dep[i][0], dep[i][1]);
> -		} else {
> -			report_skip("facility %d not present", dep[i][0]);
> -		}
> +		check_dependency(dep[i][0], dep[i][1]);
>  	}
>  	report_prefix_pop();
>