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

[Cornelia Huck <cohuck@redhat.com>]

On Tue,  7 Jul 2020 12:42:05 +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.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  v2:
>  - Rewrote the logic, introduced expected_tcg_fail flag
>  - Use manufacturer string instead of VM name to detect TCG
> 
>  s390x/cpumodel.c | 49 ++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 43 insertions(+), 6 deletions(-)

(...)

> +static bool is_tcg(void)
> +{
> +	const char qemu_ebcdic[] = { 0xd8, 0xc5, 0xd4, 0xe4 };
> +	bool ret = false;
> +	uint8_t *buf;
> +
> +	buf = alloc_page();
> +	if (!buf)
> +		return false;
> +
> +	if (stsi(buf, 1, 1, 1)) {
> +		goto out;
> +	}

This does an alloc_page() and a stsi() every time you call it...

> +
> +	/*
> +	 * If the manufacturer string is "QEMU" in EBCDIC, then we are on TCG
> +	 * (otherwise the string is "IBM" in EBCDIC)
> +	 */
> +	if (!memcmp(&buf[32], qemu_ebcdic, sizeof(qemu_ebcdic)))
> +		ret =  true;
> +out:
> +	free_page(buf);
> +	return ret;
> +}
> +
> +
>  int main(void)
>  {
>  	int i;
> @@ -46,11 +81,13 @@ int main(void)
>  
>  	report_prefix_push("dependency");

...so maybe cache the value for is_tcg() here instead of checking
multiple times in the loop?

(Or cache the value in is_tcg().)

>  	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]);
> +		if (test_facility(dep[i].facility)) {
> +			report_xfail(dep[i].expected_tcg_fail && is_tcg(),
> +				     test_facility(dep[i].implied),
> +				     "%d implies %d",
> +				     dep[i].facility, dep[i].implied);
>  		} else {
> -			report_skip("facility %d not present", dep[i][0]);
> +			report_skip("facility %d not present", dep[i].facility);
>  		}
>  	}
>  	report_prefix_pop();