Re: [kvm-unit-tests PATCH v8 1/2] s390x: topology: Check the Perform Topology Function

[Pierre Morel <pmorel@linux.ibm.com>]

On 4/27/23 08:57, Nico Boehr wrote:
> Quoting Pierre Morel (2023-04-26 10:34:25)
> [...]
>> diff --git a/s390x/topology.c b/s390x/topology.c
>> new file mode 100644
>> index 0000000..07f1650
>> --- /dev/null
>> +++ b/s390x/topology.c
>> @@ -0,0 +1,191 @@
> [...]
>> +#define PTF_INVALID_FUNCTION   0xff
> No longer used?


right


>
> [...]
>> +static void check_specifications(void)
>> +{
>> +       unsigned long wrong_bits = 0;
>> +       unsigned long ptf_bits;
>> +       unsigned long rc;
>> +       int i;
>> +
>> +       report_prefix_push("Specifications");
>> +
>> +       /* Function codes above 3 are undefined */
>> +       for (i = 4; i < 255; i++) {
>> +               expect_pgm_int();
>> +               ptf(i, &rc);
>> +               mb();
>> +               if (lowcore.pgm_int_code != PGM_INT_CODE_SPECIFICATION) {
> Please use clear_pgm_int(), the return value will be the interruption code. You can also get rid of the barrier then.
>
> Also, using wrong_bits is confusing here since it serves a completely different purpose below.
>
> Maybe just:
>
> if (clear_pgm_int() != PGM_INT_CODE_SPECIFICATION)
>      report_fail("FC %d did not yield specification exception", i);

OK, thanks,


>
> [...]
>> +       /* Reserved bits must be 0 */
>> +       for (i = 8, wrong_bits = 0; i < 64; i++) {
>> +               ptf_bits = 0x01UL << i;
>> +               expect_pgm_int();
>> +               ptf(ptf_bits, &rc);
>> +               mb();
>> +               if (lowcore.pgm_int_code != PGM_INT_CODE_SPECIFICATION)
> Also use clear_pgm_int() here.


OK, too, thanks


Regards,

Pierre