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

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

[Thomas Huth]

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>
---
 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")
+ */
+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)
+{
+	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();
 
-- 
2.18.1

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

[Cornelia Huck]

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();
>  

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

[Thomas Huth]

On 07/07/2020 09.22, Cornelia Huck wrote:
> 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).

OK, I'll have a try.

>> + */
>> +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>

That makes sense, indeed.

 Thanks,
  Thomas