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

[Pierre Morel <pmorel@xxxxxxxxxxxxx>]

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