Re: [kvm-unit-tests PATCH v3 5/5] s390x: css: testing measurement block format 1

[Cornelia Huck <cohuck@redhat.com>]

On Thu, 18 Feb 2021 18:26:44 +0100
Pierre Morel <pmorel@linux.ibm.com> wrote:

> Measurement block format 1 is made available by the extended
> measurement block facility and is indicated in the SCHIB by
> the bit in the PMCW.
> 
> The MBO is specified in the SCHIB of each channel and the MBO
> defined by the SCHM instruction is ignored.
> 
> The test of the MB format 1 is just skipped if the feature is
> not available.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>  lib/s390x/css.h     | 16 ++++++++++++++
>  lib/s390x/css_lib.c | 25 ++++++++++++++++++++-
>  s390x/css.c         | 53 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 93 insertions(+), 1 deletion(-)
> 

(...)

> diff --git a/lib/s390x/css_lib.c b/lib/s390x/css_lib.c
> index 4c8a6ae..1f09f93 100644
> --- a/lib/s390x/css_lib.c
> +++ b/lib/s390x/css_lib.c
> @@ -298,7 +298,7 @@ static bool schib_update_mb(int schid, uint64_t mb, uint16_t mbi,
>  			pmcw->flags2 &= ~PMCW_MBF1;
>  
>  		pmcw->mbi = mbi;
> -		schib.mbo = mb;
> +		schib.mbo = mb & ~0x3f;

Merge this into the patch introducing the function?

>  	} else {
>  		pmcw->flags &= ~(PMCW_MBUE | PMCW_DCTME);
>  	}
> @@ -527,3 +527,26 @@ void enable_io_isc(uint8_t isc)
>  	value = (uint64_t)isc << 24;
>  	lctlg(6, value);
>  }
> +
> +void msch_with_wrong_fmt1_mbo(unsigned int schid, uint64_t mb)
> +{
> +	struct pmcw *pmcw = &schib.pmcw;
> +	int cc;
> +
> +	/* Read the SCHIB for this subchannel */
> +	cc = stsch(schid, &schib);
> +	if (cc) {
> +		report(0, "stsch: sch %08x failed with cc=%d", schid, cc);
> +		return;
> +	}
> +
> +	/* Update the SCHIB to enable the measurement block */
> +	pmcw->flags |= PMCW_MBUE;
> +	pmcw->flags2 |= PMCW_MBF1;
> +	schib.mbo = mb;
> +
> +	/* Tell the CSS we want to modify the subchannel */
> +	expect_pgm_int();
> +	cc = msch(schid, &schib);
> +	check_pgm_int_code(PGM_INT_CODE_OPERAND);
> +}
> diff --git a/s390x/css.c b/s390x/css.c
> index b65aa89..576df48 100644
> --- a/s390x/css.c
> +++ b/s390x/css.c
> @@ -257,6 +257,58 @@ end:
>  	report_prefix_pop();
>  }
>  
> +/*
> + * test_schm_fmt1:
> + * With measurement block format 1 the mesurement block is
> + * dedicated to a subchannel.
> + */
> +static void test_schm_fmt1(void)
> +{
> +	struct measurement_block_format1 *mb1;
> +
> +	report_prefix_push("Format 1");
> +
> +	if (!test_device_sid) {
> +		report_skip("No device");
> +		goto end;
> +	}
> +
> +	if (!css_general_feature(CSSC_EXTENDED_MEASUREMENT_BLOCK)) {
> +		report_skip("Extended measurement block not available");
> +		goto end;
> +	}
> +
> +	/* Allocate zeroed Measurement block */
> +	mb1 = alloc_io_mem(sizeof(struct measurement_block_format1), 0);
> +	if (!mb1) {
> +		report_abort("measurement_block_format1 allocation failed");
> +		goto end;
> +	}
> +
> +	schm(NULL, 0); /* Stop any previous measurement */
> +	schm(0, SCHM_MBU);
> +
> +	/* Expect error for non aligned MB */
> +	report_prefix_push("Unaligned MB origin");
> +	msch_with_wrong_fmt1_mbo(test_device_sid, (uint64_t)mb1 + 1);
> +	report_prefix_pop();
> +
> +	/* Clear the measurement block for the next test */
> +	memset(mb1, 0, sizeof(*mb1));
> +
> +	/* Expect success */
> +	report_prefix_push("Valid MB address and index");
> +	report(start_measure((u64)mb1, 0, true) &&
> +	       mb1->ssch_rsch_count == SCHM_UPDATE_CNT,
> +	       "SSCH measured %d", mb1->ssch_rsch_count);
> +	report_prefix_pop();
> +
> +	schm(NULL, 0); /* Stop the measurement */

Same here, I think you should call css_disable_mb().

> +	free_io_mem(mb1, sizeof(struct measurement_block_format1));
> +end:
> +	report_prefix_pop();
> +}
> +
>  static struct {
>  	const char *name;
>  	void (*func)(void);
> @@ -268,6 +320,7 @@ static struct {
>  	{ "sense (ssch/tsch)", test_sense },
>  	{ "measurement block (schm)", test_schm },
>  	{ "measurement block format0", test_schm_fmt0 },
> +	{ "measurement block format1", test_schm_fmt1 },
>  	{ NULL, NULL }
>  };
>