diff mbox series

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

Message ID 1613669204-6464-5-git-send-email-pmorel@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series CSS Mesurement Block | expand

Commit Message

Pierre Morel Feb. 18, 2021, 5:26 p.m. UTC 
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     | 67 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+)

Comments

Cornelia Huck Feb. 23, 2021, 1:27 p.m. UTC | #1 
On Thu, 18 Feb 2021 18:26:43 +0100
Pierre Morel <pmorel@linux.ibm.com> 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     | 67 +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 79 insertions(+)
> 

(...)

> diff --git a/s390x/css.c b/s390x/css.c
> index fc693f3..b65aa89 100644
> --- a/s390x/css.c
> +++ b/s390x/css.c
> @@ -191,6 +191,72 @@ static void test_schm(void)
>  	report_prefix_pop();
>  }
>  
> +#define SCHM_UPDATE_CNT 10
> +static bool start_measure(uint64_t mbo, uint16_t mbi, bool fmt1)

Maybe "start_measuring"? Or "start_measurements"?

> +{
> +	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;
> +		}
> +	}
> +
> +	return true;
> +}
> +
> +/*
> + * test_schm_fmt0:
> + * With measurement block format 0 a memory space is shared
> + * by all subchannels, each subchannel can provide an index
> + * for the measurement block facility to store the measures.

s/measures/measurements/

> + */
> +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.

> +	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?

> +	free_io_mem(mb0, shared_mb_size);
> +end:
> +	report_prefix_pop();
> +}
> +
>  static struct {
>  	const char *name;
>  	void (*func)(void);
> @@ -201,6 +267,7 @@ static struct {
>  	{ "enable (msch)", test_enable },
>  	{ "sense (ssch/tsch)", test_sense },
>  	{ "measurement block (schm)", test_schm },
> +	{ "measurement block format0", test_schm_fmt0 },
>  	{ NULL, NULL }
>  };
>
Pierre Morel Feb. 23, 2021, 3:49 p.m. UTC | #2 
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:
> 
>> 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     | 67 +++++++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 79 insertions(+)
>>
> 
> (...)
> 
>> diff --git a/s390x/css.c b/s390x/css.c
>> index fc693f3..b65aa89 100644
>> --- a/s390x/css.c
>> +++ b/s390x/css.c
>> @@ -191,6 +191,72 @@ static void test_schm(void)
>>   	report_prefix_pop();
>>   }
>>   
>> +#define SCHM_UPDATE_CNT 10
>> +static bool start_measure(uint64_t mbo, uint16_t mbi, bool fmt1)
> 
> Maybe "start_measuring"? Or "start_measurements"?

OK

> 
>> +{
>> +	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;
>> +		}
>> +	}
>> +
>> +	return true;
>> +}
>> +
>> +/*
>> + * test_schm_fmt0:
>> + * With measurement block format 0 a memory space is shared
>> + * by all subchannels, each subchannel can provide an index
>> + * for the measurement block facility to store the measures.
> 
> s/measures/measurements/

yes

> 
>> + */
>> +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.


Thanks,
Pierre
Cornelia Huck Feb. 23, 2021, 4:05 p.m. UTC | #3 
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.
diff mbox series

Patch

diff --git a/lib/s390x/css.h b/lib/s390x/css.h
index 17b917f..dabe54a 100644
--- a/lib/s390x/css.h
+++ b/lib/s390x/css.h
@@ -375,4 +375,16 @@  static inline void schm(void *mbo, unsigned int flags)
 bool css_enable_mb(int sid, uint64_t mb, uint16_t mbi, uint16_t flg, bool fmt1);
 bool css_disable_mb(int schid);
 
+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;
+};
+
 #endif
diff --git a/s390x/css.c b/s390x/css.c
index fc693f3..b65aa89 100644
--- a/s390x/css.c
+++ b/s390x/css.c
@@ -191,6 +191,72 @@  static void test_schm(void)
 	report_prefix_pop();
 }
 
+#define SCHM_UPDATE_CNT 10
+static bool start_measure(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;
+		}
+	}
+
+	return true;
+}
+
+/*
+ * test_schm_fmt0:
+ * With measurement block format 0 a memory space is shared
+ * by all subchannels, each subchannel can provide an index
+ * for the measurement block facility to store the measures.
+ */
+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 */
+	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 */
+	free_io_mem(mb0, shared_mb_size);
+end:
+	report_prefix_pop();
+}
+
 static struct {
 	const char *name;
 	void (*func)(void);
@@ -201,6 +267,7 @@  static struct {
 	{ "enable (msch)", test_enable },
 	{ "sense (ssch/tsch)", test_sense },
 	{ "measurement block (schm)", test_schm },
+	{ "measurement block format0", test_schm_fmt0 },
 	{ NULL, NULL }
 };