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

[Cornelia Huck <cohuck@redhat.com>]

On Tue, 23 Feb 2021 16:49:56 +0100
Pierre Morel <pmorel@linux.ibm.com> wrote:

> On 2/23/21 2:27 PM, Cornelia Huck wrote:
> > On Thu, 18 Feb 2021 18:26:43 +0100
> > Pierre Morel <pmorel@linux.ibm.com> wrote:

> >> + */
> >> +static void test_schm_fmt0(void)
> >> +{
> >> +	struct measurement_block_format0 *mb0;
> >> +	int shared_mb_size = 2 * sizeof(struct measurement_block_format0);
> >> +
> >> +	report_prefix_push("Format 0");
> >> +
> >> +	/* Allocate zeroed Measurement block */
> >> +	mb0 = alloc_io_mem(shared_mb_size, 0);
> >> +	if (!mb0) {
> >> +		report_abort("measurement_block_format0 allocation failed");
> >> +		goto end;
> >> +	}
> >> +
> >> +	schm(NULL, 0); /* Stop any previous measurement */  
> > 
> > Probably not strictly needed, but cannot hurt.  
> yes
> 
> >   
> >> +	schm(mb0, SCHM_MBU);
> >> +
> >> +	/* Expect success */
> >> +	report_prefix_push("Valid MB address and index 0");
> >> +	report(start_measure(0, 0, false) &&
> >> +	       mb0->ssch_rsch_count == SCHM_UPDATE_CNT,
> >> +	       "SSCH measured %d", mb0->ssch_rsch_count);
> >> +	report_prefix_pop();
> >> +
> >> +	/* Clear the measurement block for the next test */
> >> +	memset(mb0, 0, shared_mb_size);
> >> +
> >> +	/* Expect success */
> >> +	report_prefix_push("Valid MB address and index 1");
> >> +	report(start_measure(0, 1, false) &&
> >> +	       mb0[1].ssch_rsch_count == SCHM_UPDATE_CNT,
> >> +	       "SSCH measured %d", mb0[1].ssch_rsch_count);
> >> +	report_prefix_pop();
> >> +
> >> +	schm(NULL, 0); /* Stop the measurement */  
> > 
> > Shouldn't you call css_disable_mb() here as well?  
> 
> I do not think it is obligatory, measurements are stopped but it may be 
> indeed better so we get a clean SCHIB.
> So yes,
> 
>      css_disable_mb();
>      schm(NULL, 0);
> 
> seems the right thing to do.

Yes, keeping a reference to something you free seems just wrong.