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

From: Cornelia Huck <cohuck@redhat.com>
To: Janosch Frank <frankja@linux.ibm.com>
Cc: Pierre Morel <pmorel@linux.ibm.com>,
	kvm@vger.kernel.org, david@redhat.com, thuth@redhat.com,
	imbrenda@linux.ibm.com
Subject: Re: [kvm-unit-tests PATCH v4 5/6] s390x: css: testing measurement block format 0
Date: Thu, 4 Mar 2021 18:05:11 +0100	[thread overview]
Message-ID: <20210304180511.34afe9fe.cohuck@redhat.com> (raw)
In-Reply-To: <80e25939-239a-8579-ba48-563ca0b2960f@linux.ibm.com>

On Mon, 1 Mar 2021 16:54:57 +0100
Janosch Frank <frankja@linux.ibm.com> wrote:

> On 3/1/21 12:47 PM, Pierre Morel wrote:
> > We test the update of the measurement block format 0, the
> > measurement 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 | 12 +++++++++
> >  s390x/css.c     | 66 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 78 insertions(+)
> > 

(...)

> > diff --git a/s390x/css.c b/s390x/css.c
> > index e8f96f3..3915ed3 100644
> > --- a/s390x/css.c
> > +++ b/s390x/css.c
> > @@ -184,6 +184,71 @@ static void test_schm(void)
> >  	report_prefix_pop();
> >  }
> >  
> > +#define SCHM_UPDATE_CNT 10
> > +static bool start_measuring(uint64_t mbo, uint16_t mbi, bool fmt1)
> > +{
> > +	int i;
> > +
> > +	if (!css_enable_mb(test_device_sid, mbo, mbi, PMCW_MBUE, fmt1)) {
> > +		report(0, "Enabling measurement_block_format");
> > +		return false;
> > +	}
> > +
> > +	for (i = 0; i < SCHM_UPDATE_CNT; i++) {
> > +		if (!do_test_sense()) {
> > +			report(0, "Error during sense");
> > +			return false;  
> Are these hard fails, i.e. would it make sense to stop testing if this
> or the css_enable_mb() above fails?

I think so; if we can't even enable the mb or send a sense, there's
something really broken.

(...)

Otherwise, this looks good to me (same for the next patch.)