All of lore.kernel.org
 help / color / mirror / Atom feed
* [kvm-unit-tests v2 PATCH] s390x/cpumodel: The missing DFP facility on TCG is expected
@ 2020-07-07 10:42 Thomas Huth
  2020-07-07 10:48 ` Thomas Huth
  2020-07-07 11:44 ` Cornelia Huck
  0 siblings, 2 replies; 6+ messages in thread
From: Thomas Huth @ 2020-07-07 10:42 UTC (permalink / raw)
  To: kvm; +Cc: David Hildenbrand, Janosch Frank, Cornelia Huck

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

diff --git a/s390x/cpumodel.c b/s390x/cpumodel.c
index 5d232c6..190ceb2 100644
--- a/s390x/cpumodel.c
+++ b/s390x/cpumodel.c
@@ -11,14 +11,19 @@
  */
 
 #include <asm/facility.h>
+#include <alloc_page.h>
 
-static int dep[][2] = {
+static struct {
+	int facility;
+	int implied;
+	bool expected_tcg_fail;
+} dep[] = {
 	/* from SA22-7832-11 4-98 facility indications */
 	{   4,   3 },
 	{   5,   3 },
 	{   5,   4 },
 	{  19,  18 },
-	{  37,  42 },
+	{  37,  42, true },  /* TCG does not have DFP and won't get it soon */
 	{  43,  42 },
 	{  73,  49 },
 	{ 134, 129 },
@@ -38,6 +43,36 @@ 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)
+{
+	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;
+	}
+
+	/*
+	 * 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");
 	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();
-- 
2.18.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [kvm-unit-tests v2 PATCH] s390x/cpumodel: The missing DFP facility on TCG is expected
  2020-07-07 10:42 [kvm-unit-tests v2 PATCH] s390x/cpumodel: The missing DFP facility on TCG is expected Thomas Huth
@ 2020-07-07 10:48 ` Thomas Huth
  2020-07-07 11:44 ` Cornelia Huck
  1 sibling, 0 replies; 6+ messages in thread
From: Thomas Huth @ 2020-07-07 10:48 UTC (permalink / raw)
  To: kvm; +Cc: David Hildenbrand, Janosch Frank, Cornelia Huck

On 07/07/2020 12.42, Thomas Huth 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(-)
> 
> diff --git a/s390x/cpumodel.c b/s390x/cpumodel.c
> index 5d232c6..190ceb2 100644
> --- a/s390x/cpumodel.c
> +++ b/s390x/cpumodel.c
> @@ -11,14 +11,19 @@
>   */
>  
>  #include <asm/facility.h>
> +#include <alloc_page.h>
>  
> -static int dep[][2] = {
> +static struct {
> +	int facility;
> +	int implied;
> +	bool expected_tcg_fail;
> +} dep[] = {
>  	/* from SA22-7832-11 4-98 facility indications */
>  	{   4,   3 },
>  	{   5,   3 },
>  	{   5,   4 },
>  	{  19,  18 },
> -	{  37,  42 },
> +	{  37,  42, true },  /* TCG does not have DFP and won't get it soon */
>  	{  43,  42 },
>  	{  73,  49 },
>  	{ 134, 129 },
> @@ -38,6 +43,36 @@ 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")

I forgot to update the comment, it rather should read: "A hack to detect
TCG (instead of KVM): Check the manufacturer string which differs
between TCG and KVM." (or something similar)

 Thomas


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [kvm-unit-tests v2 PATCH] s390x/cpumodel: The missing DFP facility on TCG is expected
  2020-07-07 10:42 [kvm-unit-tests v2 PATCH] s390x/cpumodel: The missing DFP facility on TCG is expected Thomas Huth
  2020-07-07 10:48 ` Thomas Huth
@ 2020-07-07 11:44 ` Cornelia Huck
  2020-07-07 11:45   ` David Hildenbrand
  1 sibling, 1 reply; 6+ messages in thread
From: Cornelia Huck @ 2020-07-07 11:44 UTC (permalink / raw)
  To: Thomas Huth; +Cc: kvm, David Hildenbrand, Janosch Frank

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [kvm-unit-tests v2 PATCH] s390x/cpumodel: The missing DFP facility on TCG is expected
  2020-07-07 11:44 ` Cornelia Huck
@ 2020-07-07 11:45   ` David Hildenbrand
  2020-07-07 15:09     ` Janosch Frank
  0 siblings, 1 reply; 6+ messages in thread
From: David Hildenbrand @ 2020-07-07 11:45 UTC (permalink / raw)
  To: Cornelia Huck, Thomas Huth; +Cc: kvm, Janosch Frank

On 07.07.20 13:44, Cornelia Huck wrote:
> 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?

Maybe move it to common code and do the detection early during boot? The
n provide is_tcg() or sth. like that. Could be helpful in other context
maybe.


-- 
Thanks,

David / dhildenb


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [kvm-unit-tests v2 PATCH] s390x/cpumodel: The missing DFP facility on TCG is expected
  2020-07-07 11:45   ` David Hildenbrand
@ 2020-07-07 15:09     ` Janosch Frank
  2020-07-08 11:18       ` Thomas Huth
  0 siblings, 1 reply; 6+ messages in thread
From: Janosch Frank @ 2020-07-07 15:09 UTC (permalink / raw)
  To: David Hildenbrand, Cornelia Huck, Thomas Huth; +Cc: kvm


[-- Attachment #1.1: Type: text/plain, Size: 2309 bytes --]

On 7/7/20 1:45 PM, David Hildenbrand wrote:
> On 07.07.20 13:44, Cornelia Huck wrote:
>> 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?
> 
> Maybe move it to common code and do the detection early during boot? The
> n provide is_tcg() or sth. like that. Could be helpful in other context
> maybe.
> 

Well we also already have a check for zvm 6 with stsi 3.2.2 in skey.c
I'm not completely convinced that I want to loose two pages and a few
cycles on every startup for two separate test cases.



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [kvm-unit-tests v2 PATCH] s390x/cpumodel: The missing DFP facility on TCG is expected
  2020-07-07 15:09     ` Janosch Frank
@ 2020-07-08 11:18       ` Thomas Huth
  0 siblings, 0 replies; 6+ messages in thread
From: Thomas Huth @ 2020-07-08 11:18 UTC (permalink / raw)
  To: Janosch Frank, David Hildenbrand, Cornelia Huck; +Cc: kvm

On 07/07/2020 17.09, Janosch Frank wrote:
> On 7/7/20 1:45 PM, David Hildenbrand wrote:
>> On 07.07.20 13:44, Cornelia Huck wrote:
>>> 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?
>>
>> Maybe move it to common code and do the detection early during boot? The
>> n provide is_tcg() or sth. like that. Could be helpful in other context
>> maybe.
>>
> 
> Well we also already have a check for zvm 6 with stsi 3.2.2 in skey.c
> I'm not completely convinced that I want to loose two pages and a few
> cycles on every startup for two separate test cases.

It certainly does not make sense to run the stsi calls during each and
every startup ... but I can move the is_tcg() function to the library
and make it a little bit smarter. I'll prepare a v3...

 Thomas


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-07-08 11:18 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-07 10:42 [kvm-unit-tests v2 PATCH] s390x/cpumodel: The missing DFP facility on TCG is expected Thomas Huth
2020-07-07 10:48 ` Thomas Huth
2020-07-07 11:44 ` Cornelia Huck
2020-07-07 11:45   ` David Hildenbrand
2020-07-07 15:09     ` Janosch Frank
2020-07-08 11:18       ` Thomas Huth

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.