Re: [kvm-unit-tests PATCH v3] s390x: Add stsi 3.2.2 tests

[Janosch Frank]

On 3/31/20 12:16 PM, Cornelia Huck wrote:
> On Tue, 31 Mar 2020 11:46:53 +0200
> Janosch Frank <address@hidden> wrote:
> 
>> On 3/31/20 11:35 AM, Cornelia Huck wrote:
>>> On Tue, 31 Mar 2020 03:14:56 -0400
>>> Janosch Frank <address@hidden> wrote:
>>>   
>>>> Subcode 3.2.2 is handled by KVM/QEMU and should therefore be tested
>>>> a bit more thorough.  
>>>
>>> s/thorough/thoroughly/ ?
>>>   
>>>>
>>>> In this test we set a custom name and uuid through the QEMU command
>>>> line. Both parameters will be passed to the guest on a stsi subcode
>>>> 3.2.2 call and will then be checked.
>>>>
>>>> We also compare the configured cpu numbers against the smp reported
>>>> numbers and if the reserved + configured add up to the total number
>>>> reported.
>>>>
>>>> Signed-off-by: Janosch Frank <address@hidden>
>>>> ---
>>>>
>>>> * Tabify on struct
>>>> * Moved prefix_push up a bit
>>>> * Replaced returns with goto out to pop prefix
>>>>
>>>> ---
>>>>  s390x/stsi.c        | 73 +++++++++++++++++++++++++++++++++++++++++++++
>>>>  s390x/unittests.cfg |  1 +
>>>>  2 files changed, 74 insertions(+)
>>>>  
>>>
>>> (...)
>>>   
>>>> +static void test_3_2_2(void)
>>>> +{
>>>> +  int rc;
>>>> +  /* EBCDIC for "kvm-unit" */
>>>> +  const uint8_t vm_name[] = { 0x92, 0xa5, 0x94, 0x60, 0xa4, 0x95, 0x89,
>>>> +                              0xa3 };
>>>> +  const uint8_t uuid[] = { 0x0f, 0xb8, 0x4a, 0x86, 0x72, 0x7c,
>>>> +                           0x11, 0xea, 0xbc, 0x55, 0x02, 0x42, 0xac, 0x13,
>>>> +                           0x00, 0x03 };
>>>> +  /* EBCDIC for "KVM/" */
>>>> +  const uint8_t cpi_kvm[] = { 0xd2, 0xe5, 0xd4, 0x61 };
>>>> +  const char *vm_name_ext = "kvm-unit-test";
>>>> +  struct stsi_322 *data = (void *)pagebuf;
>>>> +
>>>> +  report_prefix_push("3.2.2");
>>>> +
>>>> +  /* Is the function code available at all? */
>>>> +  if (stsi_get_fc(pagebuf) < 3) {
>>>> +          report_skip("Running under lpar, no level 3 to test.");
>>>> +          goto out;
>>>> +  }
>>>> +
>>>> +  rc = stsi(pagebuf, 3, 2, 2);
>>>> +  report(!rc, "call");
>>>> +
>>>> +  /* For now we concentrate on KVM/QEMU */
>>>> +  if (memcmp(&data->vm[0].cpi, cpi_kvm, sizeof(cpi_kvm))) {
>>>> +          report_skip("Not running under KVM/QEMU.");
>>>> +          goto out;
>>>> +  }
>>>> +
>>>> +  report(!memcmp(data->vm[0].uuid, uuid, sizeof(uuid)), "uuid");
>>>> +  report(data->vm[0].conf_cpus == smp_query_num_cpus(), "cpu # 
>>>> configured");
>>>> +  report(data->vm[0].total_cpus ==
>>>> +         data->vm[0].reserved_cpus + data->vm[0].conf_cpus,
>>>> +         "cpu # total == conf + reserved");
>>>> +  report(data->vm[0].standby_cpus == 0, "cpu # standby");
>>>> +  report(!memcmp(data->vm[0].name, vm_name, sizeof(data->vm[0].name)),
>>>> +         "VM name == kvm-unit-test");
>>>> +
>>>> +  if (data->vm[0].ext_name_encoding != 2) {
>>>> +          report_skip("Extended VM names are not UTF-8.");  
>>>
>>> Do we expect this to be anything other than UTF-8?  
>>
>> With the current QEMU no.
> 
> I don't really see a reason to change this in QEMU, though; and as you
> check already whether we're running under QEMU, maybe make this a
> failure?

David has requested it and already queued the patch.
If it bothers you, you can provide a fixpatch. :-)

> 
>> When I find time to test this under z/VM (as a guest 2, no KVM) maybe.
> 
> Would it make sense to check (different) expected values for z/VM and
> QEMU, then?

I'm not sure what z/VM reports here, so I can't answer that.
But testing firmware, lpar and z/VM in addition to KVM has been proven
increasingly useful over time.

I'll add it to my low prio todo list.

> 
>>
>>>   
>>>> +          goto out;
>>>> +  }
>>>> +  report(!memcmp(data->ext_names[0], vm_name_ext, sizeof(vm_name_ext)),
>>>> +                 "ext VM name == kvm-unit-test");
>>>> +
>>>> +out:
>>>> +  report_prefix_pop();
>>>> +}
>>>> +
>>>>  int main(void)
>>>>  {
>>>>    report_prefix_push("stsi");
>>>>    test_priv();
>>>>    test_specs();
>>>>    test_fc();
>>>> +  test_3_2_2();
>>>>    return report_summary();
>>>>  }  
>>>
>>> (...)
>>>   
>>
>>
>

signature.asc
Description: OpenPGP digital signature