Re: [kvm-unit-tests PATCH v1 4/5] s390x: css: SCHM tests format 0

[Thomas Huth <thuth@redhat.com>]

On 29/01/2021 15.34, Pierre Morel wrote:
> We tests the update of the mesurement block format 0, the
> mesurement block origin is calculated from the mbo argument
> used by the SCHM instruction and the offset calculated using
> the measurement block index of the SCHIB.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>   lib/s390x/css.h | 29 ++++++++++++++++++++++++++++
>   s390x/css.c     | 51 +++++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 80 insertions(+)
> 
> diff --git a/lib/s390x/css.h b/lib/s390x/css.h
> index 938f0ab..ba0bc67 100644
> --- a/lib/s390x/css.h
> +++ b/lib/s390x/css.h
> @@ -358,7 +358,36 @@ static inline void schm(void *mbo, unsigned int flags)
>   	asm("schm" : : "d" (__gpr2), "d" (__gpr1));
>   }
>   
> +#define SCHM_DCTM	1 /* activate Device Connection TiMe */
> +#define SCHM_MBU	2 /* activate Measurement Block Update */
> +
>   int css_enable_mb(int schid, uint64_t mb, int format1, uint16_t mbi,
>   		  uint16_t flags);
>   
> +struct measurement_block_format0 {
> +	uint16_t ssch_rsch_count;
> +	uint16_t sample_count;
> +	uint32_t device_connect_time;
> +	uint32_t function_pending_time;
> +	uint32_t device_disconnect_time;
> +	uint32_t cu_queuing_time;
> +	uint32_t device_active_only_time;
> +	uint32_t device_busy_time;
> +	uint32_t initial_cmd_resp_time;
> +};
> +
> +struct measurement_block_format1 {
> +	uint32_t ssch_rsch_count;
> +	uint32_t sample_count;
> +	uint32_t device_connect_time;
> +	uint32_t function_pending_time;
> +	uint32_t device_disconnect_time;
> +	uint32_t cu_queuing_time;
> +	uint32_t device_active_only_time;
> +	uint32_t device_busy_time;
> +	uint32_t initial_cmd_resp_time;
> +	uint32_t irq_delay_time;
> +	uint32_t irq_prio_delay_time;
> +};
> +
>   #endif
> diff --git a/s390x/css.c b/s390x/css.c
> index 000ce04..2e9ea47 100644
> --- a/s390x/css.c
> +++ b/s390x/css.c
> @@ -155,10 +155,61 @@ static void css_init(void)
>   	report(1, "CSS initialized");
>   }
>   
> +#define SCHM_UPDATE_CNT 10
> +static void test_schm_fmt0(struct measurement_block_format0 *mb0)
> +{
> +	int ret;
> +	int i;
> +
> +	report_prefix_push("Format 0");
> +	ret = css_enable_mb(test_device_sid, 0, 0, 0, PMCW_MBUE);
> +	if (ret) {
> +		report(0, "Enabling measurement_block_format0");
> +		goto end;
> +	}
> +
> +	for (i = 0; i < SCHM_UPDATE_CNT; i++) {
> +		if (!do_test_sense()) {
> +			report(0, "Error during sense");
> +			break;
> +		}
> +	}
> +
> +	report_info("ssch_rsch_count: %d", mb0->ssch_rsch_count);
> +	report_info("sample_count: %d", mb0->sample_count);
> +	report_info("device_connect_time: %d", mb0->device_connect_time);
> +	report_info("function_pending_time: %d", mb0->function_pending_time);
> +	report_info("device_disconnect_time: %d", mb0->device_disconnect_time);
> +	report_info("cu_queuing_time: %d", mb0->cu_queuing_time);
> +	report_info("device_active_only_time: %d", mb0->device_active_only_time);
> +	report_info("device_busy_time: %d", mb0->device_busy_time);
> +	report_info("initial_cmd_resp_time: %d", mb0->initial_cmd_resp_time);
> +
> +	report(i == mb0->ssch_rsch_count,
> +	       "SSCH expected %d measured %d", i, mb0->ssch_rsch_count);
> +
> +end:
> +	report_prefix_pop();
> +}
> +
>   static void test_schm(void)
>   {
> +	struct measurement_block_format0 *mb0;
> +
>   	if (css_general_feature(CSSC_EXTENDED_MEASUREMENT_BLOCK))
>   		report_info("Extended measurement block available");
> +
> +	mb0 = alloc_io_mem(sizeof(struct measurement_block_format0), 0);
> +	if (!mb0) {
> +		report(0, "measurement_block_format0 allocation");
> +		goto end_free;

If allocation failed, there is certainly no need to try to free it, so you 
can get rid of the goto and the label here and return directly instead. Or 
maybe
Maybe also simply use report_abort() in this case?

  Thomas


> +	}
> +
> +	schm(mb0, SCHM_MBU);
> +	test_schm_fmt0(mb0);
> +
> +end_free:
> +	free_io_mem(mb0, sizeof(struct measurement_block_format0));
>   }
>   
>   static struct {
>