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

[Thomas Huth <thuth@redhat.com>]

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