* [kvm-unit-tests PATCH v3 1/5] s390x: css: Store CSS Characteristics
2021-02-18 17:26 [kvm-unit-tests PATCH v3 0/5] CSS Mesurement Block Pierre Morel
@ 2021-02-18 17:26 ` Pierre Morel
2021-02-26 9:50 ` Janosch Frank
2021-02-18 17:26 ` [kvm-unit-tests PATCH v3 2/5] s390x: css: simplifications of the tests Pierre Morel
` (3 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Pierre Morel @ 2021-02-18 17:26 UTC (permalink / raw)
To: kvm; +Cc: frankja, david, thuth, cohuck, imbrenda
CSS characteristics exposes the features of the Channel SubSystem.
Let's use Store Channel Subsystem Characteristics to retrieve
the features of the CSS.
Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
---
lib/s390x/css.h | 67 +++++++++++++++++++++++++++
lib/s390x/css_lib.c | 110 +++++++++++++++++++++++++++++++++++++++++++-
s390x/css.c | 12 +++++
3 files changed, 188 insertions(+), 1 deletion(-)
diff --git a/lib/s390x/css.h b/lib/s390x/css.h
index 3e57445..49daecd 100644
--- a/lib/s390x/css.h
+++ b/lib/s390x/css.h
@@ -288,4 +288,71 @@ int css_residual_count(unsigned int schid);
void enable_io_isc(uint8_t isc);
int wait_and_check_io_completion(int schid);
+/*
+ * CHSC definitions
+ */
+struct chsc_header {
+ u16 len;
+ u16 code;
+};
+
+/* Store Channel Subsystem Characteristics */
+struct chsc_scsc {
+ struct chsc_header req;
+ u16 req_fmt;
+ u8 cssid;
+ u8 res_03;
+ u32 res_04[2];
+ struct chsc_header res;
+ u32 res_fmt;
+ u64 general_char[255];
+ u64 chsc_char[254];
+};
+
+extern struct chsc_scsc *chsc_scsc;
+#define CHSC_SCSC 0x0010
+#define CHSC_SCSC_LEN 0x0010
+
+int get_chsc_scsc(void);
+
+#define CSS_GENERAL_FEAT_BITLEN (255 * 64)
+#define CSS_CHSC_FEAT_BITLEN (254 * 64)
+
+#define CHSC_SCSC 0x0010
+#define CHSC_SCSC_LEN 0x0010
+
+#define CHSC_ERROR 0x0000
+#define CHSC_RSP_OK 0x0001
+#define CHSC_RSP_INVAL 0x0002
+#define CHSC_RSP_REQERR 0x0003
+#define CHSC_RSP_ENOCMD 0x0004
+#define CHSC_RSP_NODATA 0x0005
+#define CHSC_RSP_SUP31B 0x0006
+#define CHSC_RSP_EFRMT 0x0007
+#define CHSC_RSP_ECSSID 0x0008
+#define CHSC_RSP_ERFRMT 0x0009
+#define CHSC_RSP_ESSID 0x000A
+#define CHSC_RSP_EBUSY 0x000B
+#define CHSC_RSP_MAX 0x000B
+
+static inline int _chsc(void *p)
+{
+ int cc;
+
+ asm volatile(" .insn rre,0xb25f0000,%2,0\n"
+ " ipm %0\n"
+ " srl %0,28\n"
+ : "=d" (cc), "=m" (p)
+ : "d" (p), "m" (p)
+ : "cc");
+
+ return cc;
+}
+
+int chsc(void *p, uint16_t code, uint16_t len);
+
+#include <bitops.h>
+#define css_general_feature(bit) test_bit_inv(bit, chsc_scsc->general_char)
+#define css_chsc_feature(bit) test_bit_inv(bit, chsc_scsc->chsc_char)
+
#endif
diff --git a/lib/s390x/css_lib.c b/lib/s390x/css_lib.c
index 3c24480..64560a2 100644
--- a/lib/s390x/css_lib.c
+++ b/lib/s390x/css_lib.c
@@ -15,11 +15,119 @@
#include <asm/arch_def.h>
#include <asm/time.h>
#include <asm/arch_def.h>
-
+#include <alloc_page.h>
#include <malloc_io.h>
#include <css.h>
static struct schib schib;
+struct chsc_scsc *chsc_scsc;
+
+static const char * const chsc_rsp_description[] = {
+ "CHSC unknown error",
+ "Command executed",
+ "Invalid command",
+ "Request-block error",
+ "Command not installed",
+ "Data not available",
+ "Absolute address of channel-subsystem communication block exceeds 2G - 1.",
+ "Invalid command format",
+ "Invalid channel-subsystem identification (CSSID)",
+ "The command-request block specified an invalid format for the command response block.",
+ "Invalid subchannel-set identification (SSID)",
+ "A busy condition precludes execution.",
+};
+
+static int check_response(void *p)
+{
+ struct chsc_header *h = p;
+
+ if (h->code == CHSC_RSP_OK) {
+ report(1, "CHSC command completed.");
+ return 0;
+ }
+ if (h->code > CHSC_RSP_MAX)
+ h->code = 0;
+ report(0, "Response code %04x: %s", h->code, chsc_rsp_description[h->code]);
+ return -1;
+}
+
+int chsc(void *p, uint16_t code, uint16_t len)
+{
+ struct chsc_header *h = p;
+ int cc;
+
+ report_prefix_push("Channel Subsystem Call");
+ h->code = code;
+ h->len = len;
+ cc = _chsc(p);
+ switch (cc) {
+ case 3:
+ report(0, "Subchannel invalid or not enabled.");
+ break;
+ case 2:
+ report(0, "CHSC subchannel busy.");
+ break;
+ case 1:
+ report(0, "Subchannel invalid or not enabled.");
+ break;
+ case 0:
+ cc = check_response(p + len);
+ break;
+ }
+
+ report_prefix_pop();
+ return cc;
+}
+
+int get_chsc_scsc(void)
+{
+ int i, n;
+ int ret = 0;
+ char buffer[510];
+ char *p;
+
+ report_prefix_push("Channel Subsystem Call");
+
+ if (chsc_scsc) {
+ report_info("chsc_scsc already initialized");
+ goto end;
+ }
+
+ chsc_scsc = alloc_page();
+ report_info("scsc_scsc at: %016lx", (u64)chsc_scsc);
+ if (!chsc_scsc) {
+ ret = -1;
+ report(0, "could not allocate chsc_scsc page!");
+ goto end;
+ }
+
+ report_info("scsc format %x\n", chsc_scsc->req_fmt);
+ ret = chsc(chsc_scsc, CHSC_SCSC, CHSC_SCSC_LEN);
+ if (ret) {
+ report(0, "chsc: CC %d", ret);
+ goto end;
+ }
+
+ for (i = 0, p = buffer; i < CSS_GENERAL_FEAT_BITLEN; i++) {
+ if (css_general_feature(i)) {
+ n = snprintf(p, sizeof(buffer) - ret, "%d,", i);
+ p += n;
+ }
+ }
+ report_info("General features: %s", buffer);
+
+ for (i = 0, p = buffer, ret = 0; i < CSS_CHSC_FEAT_BITLEN; i++) {
+ if (css_chsc_feature(i)) {
+ n = snprintf(p, sizeof(buffer) - ret, "%d,", i);
+ p += n;
+ }
+ }
+ report_info("CHSC features: %s", buffer);
+
+end:
+ report_prefix_pop();
+ return ret;
+}
/*
* css_enumerate:
diff --git a/s390x/css.c b/s390x/css.c
index 1a61a5c..18dbf01 100644
--- a/s390x/css.c
+++ b/s390x/css.c
@@ -14,6 +14,7 @@
#include <string.h>
#include <interrupt.h>
#include <asm/arch_def.h>
+#include <alloc_page.h>
#include <malloc_io.h>
#include <css.h>
@@ -140,10 +141,21 @@ error_senseid:
unregister_io_int_func(css_irq_io);
}
+static void css_init(void)
+{
+ int ret;
+
+ ret = get_chsc_scsc();
+ if (!ret)
+ report(1, " ");
+}
+
static struct {
const char *name;
void (*func)(void);
} tests[] = {
+ /* The css_init test is needed to initialize the CSS Characteristics */
+ { "initialize CSS (chsc)", css_init },
{ "enumerate (stsch)", test_enumerate },
{ "enable (msch)", test_enable },
{ "sense (ssch/tsch)", test_sense },
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [kvm-unit-tests PATCH v3 1/5] s390x: css: Store CSS Characteristics
2021-02-18 17:26 ` [kvm-unit-tests PATCH v3 1/5] s390x: css: Store CSS Characteristics Pierre Morel
@ 2021-02-26 9:50 ` Janosch Frank
2021-02-26 13:38 ` Pierre Morel
0 siblings, 1 reply; 17+ messages in thread
From: Janosch Frank @ 2021-02-26 9:50 UTC (permalink / raw)
To: Pierre Morel, kvm; +Cc: david, thuth, cohuck, imbrenda
On 2/18/21 6:26 PM, Pierre Morel wrote:
> CSS characteristics exposes the features of the Channel SubSystem.
> Let's use Store Channel Subsystem Characteristics to retrieve
> the features of the CSS.
>
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> ---
> lib/s390x/css.h | 67 +++++++++++++++++++++++++++
> lib/s390x/css_lib.c | 110 +++++++++++++++++++++++++++++++++++++++++++-
> s390x/css.c | 12 +++++
> 3 files changed, 188 insertions(+), 1 deletion(-)
>
> diff --git a/lib/s390x/css.h b/lib/s390x/css.h
> index 3e57445..49daecd 100644
> --- a/lib/s390x/css.h
> +++ b/lib/s390x/css.h
> @@ -288,4 +288,71 @@ int css_residual_count(unsigned int schid);
> void enable_io_isc(uint8_t isc);
> int wait_and_check_io_completion(int schid);
>
> +/*
> + * CHSC definitions
> + */
> +struct chsc_header {
> + u16 len;
> + u16 code;
uint*_t types please
> +};
> +
> +/* Store Channel Subsystem Characteristics */
> +struct chsc_scsc {
> + struct chsc_header req;
> + u16 req_fmt;
> + u8 cssid;
> + u8 res_03;
> + u32 res_04[2];
> + struct chsc_header res;
> + u32 res_fmt;
> + u64 general_char[255];
> + u64 chsc_char[254];
> +};
> +
> +extern struct chsc_scsc *chsc_scsc;
> +#define CHSC_SCSC 0x0010
> +#define CHSC_SCSC_LEN 0x0010
> +
> +int get_chsc_scsc(void);
> +
> +#define CSS_GENERAL_FEAT_BITLEN (255 * 64)
> +#define CSS_CHSC_FEAT_BITLEN (254 * 64)
> +
> +#define CHSC_SCSC 0x0010
> +#define CHSC_SCSC_LEN 0x0010
> +
> +#define CHSC_ERROR 0x0000
> +#define CHSC_RSP_OK 0x0001
> +#define CHSC_RSP_INVAL 0x0002
> +#define CHSC_RSP_REQERR 0x0003
> +#define CHSC_RSP_ENOCMD 0x0004
> +#define CHSC_RSP_NODATA 0x0005
> +#define CHSC_RSP_SUP31B 0x0006
> +#define CHSC_RSP_EFRMT 0x0007
> +#define CHSC_RSP_ECSSID 0x0008
> +#define CHSC_RSP_ERFRMT 0x0009
> +#define CHSC_RSP_ESSID 0x000A
> +#define CHSC_RSP_EBUSY 0x000B
> +#define CHSC_RSP_MAX 0x000B
> +
> +static inline int _chsc(void *p)
> +{
> + int cc;
> +
> + asm volatile(" .insn rre,0xb25f0000,%2,0\n"
> + " ipm %0\n"
> + " srl %0,28\n"
> + : "=d" (cc), "=m" (p)
> + : "d" (p), "m" (p)
> + : "cc");
> +
> + return cc;
> +}
> +
> +int chsc(void *p, uint16_t code, uint16_t len);
> +
> +#include <bitops.h>
> +#define css_general_feature(bit) test_bit_inv(bit, chsc_scsc->general_char)
> +#define css_chsc_feature(bit) test_bit_inv(bit, chsc_scsc->chsc_char)
> +
> #endif
> diff --git a/lib/s390x/css_lib.c b/lib/s390x/css_lib.c
> index 3c24480..64560a2 100644
> --- a/lib/s390x/css_lib.c
> +++ b/lib/s390x/css_lib.c
> @@ -15,11 +15,119 @@
> #include <asm/arch_def.h>
> #include <asm/time.h>
> #include <asm/arch_def.h>
> -
> +#include <alloc_page.h>
> #include <malloc_io.h>
> #include <css.h>
>
> static struct schib schib;
> +struct chsc_scsc *chsc_scsc;
> +
> +static const char * const chsc_rsp_description[] = {
> + "CHSC unknown error",
> + "Command executed",
> + "Invalid command",
> + "Request-block error",
> + "Command not installed",
> + "Data not available",
> + "Absolute address of channel-subsystem communication block exceeds 2G - 1.",
> + "Invalid command format",
> + "Invalid channel-subsystem identification (CSSID)",
> + "The command-request block specified an invalid format for the command response block.",
> + "Invalid subchannel-set identification (SSID)",
> + "A busy condition precludes execution.",
> +};
> +
> +static int check_response(void *p)
> +{
> + struct chsc_header *h = p;
> +
> + if (h->code == CHSC_RSP_OK) {
> + report(1, "CHSC command completed.");
I'm not a big fan of using integer constants for boolean type arguments.
> + return 0;
> + }
> + if (h->code > CHSC_RSP_MAX)
> + h->code = 0;
> + report(0, "Response code %04x: %s", h->code, chsc_rsp_description[h->code]);
> + return -1;
> +}
> +
> +int chsc(void *p, uint16_t code, uint16_t len)
> +{
> + struct chsc_header *h = p;
> + int cc;
> +
> + report_prefix_push("Channel Subsystem Call");
> + h->code = code;
> + h->len = len;
> + cc = _chsc(p);
> + switch (cc) {
> + case 3:
> + report(0, "Subchannel invalid or not enabled.");
> + break;
> + case 2:
> + report(0, "CHSC subchannel busy.");
> + break;
> + case 1:
> + report(0, "Subchannel invalid or not enabled.");
> + break;
I don't think that this is how we want to handle error reporting in lib
files.
Please don't use report for library error reporting if it's not needed.
Most of the times you should return an error code or simply
abort()/assert() if for instance a library init function fails and you
can assume that most of the test code is dependent on that librarie's
initialization. Sometimes report_abort() is also ok.
Test code should not be part of the library if possible!
> + case 0:
> + cc = check_response(p + len);
> + break;
> + }
> +
> + report_prefix_pop();
> + return cc;
> +}
> +
> +int get_chsc_scsc(void)
> +{
> + int i, n;
> + int ret = 0;
> + char buffer[510];
> + char *p;
> +
> + report_prefix_push("Channel Subsystem Call");
> +
> + if (chsc_scsc) {
> + report_info("chsc_scsc already initialized");
> + goto end;
> + }
> +
> + chsc_scsc = alloc_page();
> + report_info("scsc_scsc at: %016lx", (u64)chsc_scsc);
> + if (!chsc_scsc) {
> + ret = -1;
> + report(0, "could not allocate chsc_scsc page!");
> + goto end;
> + }
> +
> + report_info("scsc format %x\n", chsc_scsc->req_fmt);
> + ret = chsc(chsc_scsc, CHSC_SCSC, CHSC_SCSC_LEN);
> + if (ret) {
> + report(0, "chsc: CC %d", ret);
> + goto end;
> + }
> +
> + for (i = 0, p = buffer; i < CSS_GENERAL_FEAT_BITLEN; i++) {
> + if (css_general_feature(i)) {
> + n = snprintf(p, sizeof(buffer) - ret, "%d,", i);
> + p += n;
> + }
> + }
> + report_info("General features: %s", buffer);
> +
> + for (i = 0, p = buffer, ret = 0; i < CSS_CHSC_FEAT_BITLEN; i++) {
> + if (css_chsc_feature(i)) {
> + n = snprintf(p, sizeof(buffer) - ret, "%d,", i);
> + p += n;
> + }
> + }
> + report_info("CHSC features: %s", buffer);
> +
> +end:
> + report_prefix_pop();
> + return ret;
> +}
>
> /*
> * css_enumerate:
> diff --git a/s390x/css.c b/s390x/css.c
> index 1a61a5c..18dbf01 100644
> --- a/s390x/css.c
> +++ b/s390x/css.c
> @@ -14,6 +14,7 @@
> #include <string.h>
> #include <interrupt.h>
> #include <asm/arch_def.h>
> +#include <alloc_page.h>
>
> #include <malloc_io.h>
> #include <css.h>
> @@ -140,10 +141,21 @@ error_senseid:
> unregister_io_int_func(css_irq_io);
> }
>
> +static void css_init(void)
> +{
> + int ret;
> +
> + ret = get_chsc_scsc();
> + if (!ret)
> + report(1, " ");
> +}
> +
> static struct {
> const char *name;
> void (*func)(void);
> } tests[] = {
> + /* The css_init test is needed to initialize the CSS Characteristics */
> + { "initialize CSS (chsc)", css_init },
Is css_init() really a test or does it only setup state for further tests?
> { "enumerate (stsch)", test_enumerate },
> { "enable (msch)", test_enable },
> { "sense (ssch/tsch)", test_sense },
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [kvm-unit-tests PATCH v3 1/5] s390x: css: Store CSS Characteristics
2021-02-26 9:50 ` Janosch Frank
@ 2021-02-26 13:38 ` Pierre Morel
0 siblings, 0 replies; 17+ messages in thread
From: Pierre Morel @ 2021-02-26 13:38 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: david, thuth, cohuck, imbrenda
On 2/26/21 10:50 AM, Janosch Frank wrote:
> On 2/18/21 6:26 PM, Pierre Morel wrote:
>>
>> +/*
>> + * CHSC definitions
>> + */
>> +struct chsc_header {
>> + u16 len;
>> + u16 code;
>
> uint*_t types please
OK
>
>> +};
>> +static int check_response(void *p)
>> +{
>> + struct chsc_header *h = p;
>> +
>> + if (h->code == CHSC_RSP_OK) {
>> + report(1, "CHSC command completed.");
>
> I'm not a big fan of using integer constants for boolean type arguments.
hum, right, I will rework all these reports that should not appear here
anyway
>
>> + return 0;
>> + }
>> + if (h->code > CHSC_RSP_MAX)
>> + h->code = 0;
>> + report(0, "Response code %04x: %s", h->code, chsc_rsp_description[h->code]);
>> + return -1;
>> +}
>> +
>> +int chsc(void *p, uint16_t code, uint16_t len)
>> +{
>> + struct chsc_header *h = p;
>> + int cc;
>> +
>> + report_prefix_push("Channel Subsystem Call");
>> + h->code = code;
>> + h->len = len;
>> + cc = _chsc(p);
>> + switch (cc) {
>> + case 3:
>> + report(0, "Subchannel invalid or not enabled.");
>> + break;
>> + case 2:
>> + report(0, "CHSC subchannel busy.");
>> + break;
>> + case 1:
>> + report(0, "Subchannel invalid or not enabled.");
>> + break;
>
> I don't think that this is how we want to handle error reporting in lib
> files.
>
> Please don't use report for library error reporting if it's not needed.
>
> Most of the times you should return an error code or simply
> abort()/assert() if for instance a library init function fails and you
> can assume that most of the test code is dependent on that librarie's
> initialization. Sometimes report_abort() is also ok.
>
> Test code should not be part of the library if possible!
Yes, will take care of that.
>> static struct {
>> const char *name;
>> void (*func)(void);
>> } tests[] = {
>> + /* The css_init test is needed to initialize the CSS Characteristics */
>> + { "initialize CSS (chsc)", css_init },
>
> Is css_init() really a test or does it only setup state for further tests?
Yes it is a test too
I rework that.
>
>> { "enumerate (stsch)", test_enumerate },
>> { "enable (msch)", test_enable },
>> { "sense (ssch/tsch)", test_sense },
>>
>
--
Pierre Morel
IBM Lab Boeblingen
^ permalink raw reply [flat|nested] 17+ messages in thread
* [kvm-unit-tests PATCH v3 2/5] s390x: css: simplifications of the tests
2021-02-18 17:26 [kvm-unit-tests PATCH v3 0/5] CSS Mesurement Block Pierre Morel
2021-02-18 17:26 ` [kvm-unit-tests PATCH v3 1/5] s390x: css: Store CSS Characteristics Pierre Morel
@ 2021-02-18 17:26 ` Pierre Morel
2021-02-18 17:26 ` [kvm-unit-tests PATCH v3 3/5] s390x: css: implementing Set CHannel Monitor Pierre Morel
` (2 subsequent siblings)
4 siblings, 0 replies; 17+ messages in thread
From: Pierre Morel @ 2021-02-18 17:26 UTC (permalink / raw)
To: kvm; +Cc: frankja, david, thuth, cohuck, imbrenda
In order to ease the writing of tests based on:
- interrupt
- enabling a subchannel
- using multiple I/O on a channel without disabling it
We do the following simplifications:
- the I/O interrupt handler is registered on CSS initialization
- We do not enable again a subchannel in senseid if it is already
enabled
- we add a css_enabled() function to test if a subchannel is enabled
Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
---
lib/s390x/css.h | 1 +
lib/s390x/css_lib.c | 37 ++++++++++++++++++++++----------
s390x/css.c | 51 ++++++++++++++++++++++++++-------------------
3 files changed, 56 insertions(+), 33 deletions(-)
diff --git a/lib/s390x/css.h b/lib/s390x/css.h
index 49daecd..794321d 100644
--- a/lib/s390x/css.h
+++ b/lib/s390x/css.h
@@ -278,6 +278,7 @@ int css_enumerate(void);
#define IO_SCH_ISC 3
int css_enable(int schid, int isc);
+bool css_enabled(int schid);
/* Library functions */
int start_ccw1_chain(unsigned int sid, struct ccw1 *ccw);
diff --git a/lib/s390x/css_lib.c b/lib/s390x/css_lib.c
index 64560a2..65b58ff 100644
--- a/lib/s390x/css_lib.c
+++ b/lib/s390x/css_lib.c
@@ -178,6 +178,31 @@ out:
return schid;
}
+/*
+ * css_enabled: report if the subchannel is enabled
+ * @schid: Subchannel Identifier
+ * Return value:
+ * true if the subchannel is enabled
+ * false otherwise
+ */
+bool css_enabled(int schid)
+{
+ struct pmcw *pmcw = &schib.pmcw;
+ int cc;
+
+ cc = stsch(schid, &schib);
+ if (cc) {
+ report_info("stsch: updating sch %08x failed with cc=%d",
+ schid, cc);
+ return false;
+ }
+
+ if (!(pmcw->flags & PMCW_ENABLE)) {
+ report_info("stsch: sch %08x not enabled", schid);
+ return false;
+ }
+ return true;
+}
/*
* css_enable: enable the subchannel with the specified ISC
* @schid: Subchannel Identifier
@@ -227,18 +252,8 @@ retry:
/*
* Read the SCHIB again to verify the enablement
*/
- cc = stsch(schid, &schib);
- if (cc) {
- report_info("stsch: updating sch %08x failed with cc=%d",
- schid, cc);
- return cc;
- }
-
- if ((pmcw->flags & flags) == flags) {
- report_info("stsch: sch %08x successfully modified after %d retries",
- schid, retry_count);
+ if (css_enabled(schid))
return 0;
- }
if (retry_count++ < MAX_ENABLE_RETRIES) {
mdelay(10); /* the hardware was not ready, give it some time */
diff --git a/s390x/css.c b/s390x/css.c
index 18dbf01..d4b3cc8 100644
--- a/s390x/css.c
+++ b/s390x/css.c
@@ -56,36 +56,27 @@ static void test_enable(void)
* - We need the test device as the first recognized
* device by the enumeration.
*/
-static void test_sense(void)
+static bool do_test_sense(void)
{
struct ccw1 *ccw;
+ bool success = false;
int ret;
int len;
if (!test_device_sid) {
report_skip("No device");
- return;
+ return success;
}
- ret = css_enable(test_device_sid, IO_SCH_ISC);
- if (ret) {
- report(0, "Could not enable the subchannel: %08x",
- test_device_sid);
- return;
- }
-
- ret = register_io_int_func(css_irq_io);
- if (ret) {
- report(0, "Could not register IRQ handler");
- return;
+ if (!css_enabled(test_device_sid)) {
+ report(0, "enabling subchannel %08x", test_device_sid);
+ return success;
}
- lowcore_ptr->io_int_param = 0;
-
senseid = alloc_io_mem(sizeof(*senseid), 0);
if (!senseid) {
report(0, "Allocation of senseid");
- goto error_senseid;
+ return success;
}
ccw = ccw_alloc(CCW_CMD_SENSE_ID, senseid, sizeof(*senseid), CCW_F_SLI);
@@ -129,16 +120,21 @@ static void test_sense(void)
report_info("reserved 0x%02x cu_type 0x%04x cu_model 0x%02x dev_type 0x%04x dev_model 0x%02x",
senseid->reserved, senseid->cu_type, senseid->cu_model,
senseid->dev_type, senseid->dev_model);
+ report_info("cu_type expected 0x%04x got 0x%04x", (uint16_t)cu_type,
+ senseid->cu_type);
- report(senseid->cu_type == cu_type, "cu_type expected 0x%04x got 0x%04x",
- (uint16_t)cu_type, senseid->cu_type);
+ success = senseid->cu_type == cu_type;
error:
free_io_mem(ccw, sizeof(*ccw));
error_ccw:
free_io_mem(senseid, sizeof(*senseid));
-error_senseid:
- unregister_io_int_func(css_irq_io);
+ return success;
+}
+
+static void test_sense(void)
+{
+ report(do_test_sense(), "Got CU type expected");
}
static void css_init(void)
@@ -146,8 +142,19 @@ static void css_init(void)
int ret;
ret = get_chsc_scsc();
- if (!ret)
- report(1, " ");
+ if (ret) {
+ report(0, "Could not get CHSC Characteristics");
+ return;
+ }
+
+ ret = register_io_int_func(css_irq_io);
+ if (ret) {
+ report(0, "Could not register IRQ handler");
+ return;
+ }
+ lowcore_ptr->io_int_param = 0;
+
+ report(1, "CSS initialized");
}
static struct {
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [kvm-unit-tests PATCH v3 3/5] s390x: css: implementing Set CHannel Monitor
2021-02-18 17:26 [kvm-unit-tests PATCH v3 0/5] CSS Mesurement Block Pierre Morel
2021-02-18 17:26 ` [kvm-unit-tests PATCH v3 1/5] s390x: css: Store CSS Characteristics Pierre Morel
2021-02-18 17:26 ` [kvm-unit-tests PATCH v3 2/5] s390x: css: simplifications of the tests Pierre Morel
@ 2021-02-18 17:26 ` Pierre Morel
2021-02-23 13:22 ` Cornelia Huck
2021-02-18 17:26 ` [kvm-unit-tests PATCH v3 4/5] s390x: css: testing measurement block format 0 Pierre Morel
2021-02-18 17:26 ` [kvm-unit-tests PATCH v3 5/5] s390x: css: testing measurement block format 1 Pierre Morel
4 siblings, 1 reply; 17+ messages in thread
From: Pierre Morel @ 2021-02-18 17:26 UTC (permalink / raw)
To: kvm; +Cc: frankja, david, thuth, cohuck, imbrenda
We implement the call of the Set CHannel Monitor instruction,
starting the monitoring of the all Channel Sub System, and
initializing channel subsystem monitoring.
Initial tests report the presence of the extended measurement block
feature, and verify the error reporting of the hypervisor for SCHM.
Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
lib/s390x/css.h | 21 +++++++++-
lib/s390x/css_lib.c | 100 ++++++++++++++++++++++++++++++++++++++++++++
s390x/css.c | 35 ++++++++++++++++
3 files changed, 155 insertions(+), 1 deletion(-)
diff --git a/lib/s390x/css.h b/lib/s390x/css.h
index 794321d..17b917f 100644
--- a/lib/s390x/css.h
+++ b/lib/s390x/css.h
@@ -82,6 +82,8 @@ struct pmcw {
uint32_t intparm;
#define PMCW_DNV 0x0001
#define PMCW_ENABLE 0x0080
+#define PMCW_MBUE 0x0010
+#define PMCW_DCTME 0x0008
#define PMCW_ISC_MASK 0x3800
#define PMCW_ISC_SHIFT 11
uint16_t flags;
@@ -94,6 +96,7 @@ struct pmcw {
uint8_t pom;
uint8_t pam;
uint8_t chpid[8];
+#define PMCW_MBF1 0x0004
uint32_t flags2;
};
#define PMCW_CHANNEL_TYPE(pmcw) (pmcw->flags2 >> 21)
@@ -101,7 +104,8 @@ struct pmcw {
struct schib {
struct pmcw pmcw;
struct scsw scsw;
- uint8_t md[12];
+ uint64_t mbo;
+ uint8_t md[4];
} __attribute__ ((aligned(4)));
struct irb {
@@ -306,6 +310,7 @@ struct chsc_scsc {
u32 res_04[2];
struct chsc_header res;
u32 res_fmt;
+#define CSSC_EXTENDED_MEASUREMENT_BLOCK 48
u64 general_char[255];
u64 chsc_char[254];
};
@@ -356,4 +361,18 @@ int chsc(void *p, uint16_t code, uint16_t len);
#define css_general_feature(bit) test_bit_inv(bit, chsc_scsc->general_char)
#define css_chsc_feature(bit) test_bit_inv(bit, chsc_scsc->chsc_char)
+#define SCHM_DCTM 1 /* activate Device Connection TiMe */
+#define SCHM_MBU 2 /* activate Measurement Block Update */
+
+static inline void schm(void *mbo, unsigned int flags)
+{
+ register void *__gpr2 asm("2") = mbo;
+ register long __gpr1 asm("1") = flags;
+
+ asm("schm" : : "d" (__gpr2), "d" (__gpr1));
+}
+
+bool css_enable_mb(int sid, uint64_t mb, uint16_t mbi, uint16_t flg, bool fmt1);
+bool css_disable_mb(int schid);
+
#endif
diff --git a/lib/s390x/css_lib.c b/lib/s390x/css_lib.c
index 65b58ff..4c8a6ae 100644
--- a/lib/s390x/css_lib.c
+++ b/lib/s390x/css_lib.c
@@ -265,6 +265,106 @@ retry:
return -1;
}
+/*
+ * schib_update_mb: update the subchannel Mesurement Block
+ * @schid: Subchannel Identifier
+ * @mb : 64bit address of the measurement block
+ * @mbi : the measurement block offset
+ * @flags : PMCW_MBUE to enable measurement block update
+ * PMCW_DCTME to enable device connect time
+ * 0 to disable measurement
+ * @format1: set if format 1 is to be used
+ */
+static bool schib_update_mb(int schid, uint64_t mb, uint16_t mbi,
+ uint16_t flags, bool format1)
+{
+ struct pmcw *pmcw = &schib.pmcw;
+ int cc;
+
+ /* Read the SCHIB for this subchannel */
+ cc = stsch(schid, &schib);
+ if (cc) {
+ report_info("stsch: sch %08x failed with cc=%d", schid, cc);
+ return false;
+ }
+
+ /* Update the SCHIB to enable the measurement block */
+ if (flags) {
+ pmcw->flags |= flags;
+
+ if (format1)
+ pmcw->flags2 |= PMCW_MBF1;
+ else
+ pmcw->flags2 &= ~PMCW_MBF1;
+
+ pmcw->mbi = mbi;
+ schib.mbo = mb;
+ } else {
+ pmcw->flags &= ~(PMCW_MBUE | PMCW_DCTME);
+ }
+
+ /* Tell the CSS we want to modify the subchannel */
+ cc = msch(schid, &schib);
+ if (cc) {
+ /*
+ * If the subchannel is status pending or
+ * if a function is in progress,
+ * we consider both cases as errors.
+ */
+ report_info("msch: sch %08x failed with cc=%d", schid, cc);
+ return false;
+ }
+
+ /*
+ * Read the SCHIB again
+ */
+ cc = stsch(schid, &schib);
+ if (cc) {
+ report_info("stsch: updating sch %08x failed with cc=%d",
+ schid, cc);
+ return false;
+ }
+
+ return true;
+}
+
+/*
+ * css_enable_mb: enable the subchannel Mesurement Block
+ * @schid: Subchannel Identifier
+ * @mb : 64bit address of the measurement block
+ * @format1: set if format 1 is to be used
+ * @mbi : the measurement block offset
+ * @flags : PMCW_MBUE to enable measurement block update
+ * PMCW_DCTME to enable device connect time
+ */
+bool css_enable_mb(int schid, uint64_t mb, uint16_t mbi, uint16_t flags,
+ bool format1)
+{
+ int retry_count = MAX_ENABLE_RETRIES;
+ struct pmcw *pmcw = &schib.pmcw;
+
+ while (retry_count-- &&
+ !schib_update_mb(schid, mb, mbi, flags, format1))
+ mdelay(10); /* the hardware was not ready, give it some time */
+
+ return schib.mbo == mb && pmcw->mbi == mbi;
+}
+
+/*
+ * css_disable_mb: enable the subchannel Mesurement Block
+ * @schid: Subchannel Identifier
+ */
+bool css_disable_mb(int schid)
+{
+ int retry_count = MAX_ENABLE_RETRIES;
+
+ while (retry_count-- &&
+ !schib_update_mb(schid, 0, 0, 0, 0))
+ mdelay(10); /* the hardware was not ready, give it some time */
+
+ return retry_count > 0;
+}
+
static struct irb irb;
void css_irq_io(void)
diff --git a/s390x/css.c b/s390x/css.c
index d4b3cc8..fc693f3 100644
--- a/s390x/css.c
+++ b/s390x/css.c
@@ -157,6 +157,40 @@ static void css_init(void)
report(1, "CSS initialized");
}
+static void test_schm(void)
+{
+ if (css_general_feature(CSSC_EXTENDED_MEASUREMENT_BLOCK))
+ report_info("Extended measurement block available");
+
+ /* bits 59-63 of MB address must be 0 if MBU is defined */
+ report_prefix_push("Unaligned operand");
+ expect_pgm_int();
+ schm((void *)0x01, SCHM_MBU);
+ check_pgm_int_code(PGM_INT_CODE_OPERAND);
+ report_prefix_pop();
+
+ /* bits 36-61 of register 1 (flags) must be 0 */
+ report_prefix_push("Bad flags");
+ expect_pgm_int();
+ schm(NULL, 0xfffffffc);
+ check_pgm_int_code(PGM_INT_CODE_OPERAND);
+ report_prefix_pop();
+
+ /* SCHM is a privilege operation */
+ report_prefix_push("Privilege");
+ enter_pstate();
+ expect_pgm_int();
+ schm(NULL, SCHM_MBU);
+ check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION);
+ report_prefix_pop();
+
+ /* Normal operation */
+ report_prefix_push("Normal operation");
+ schm(NULL, SCHM_MBU);
+ report(1, "SCHM call without address");
+ report_prefix_pop();
+}
+
static struct {
const char *name;
void (*func)(void);
@@ -166,6 +200,7 @@ static struct {
{ "enumerate (stsch)", test_enumerate },
{ "enable (msch)", test_enable },
{ "sense (ssch/tsch)", test_sense },
+ { "measurement block (schm)", test_schm },
{ NULL, NULL }
};
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [kvm-unit-tests PATCH v3 3/5] s390x: css: implementing Set CHannel Monitor
2021-02-18 17:26 ` [kvm-unit-tests PATCH v3 3/5] s390x: css: implementing Set CHannel Monitor Pierre Morel
@ 2021-02-23 13:22 ` Cornelia Huck
2021-02-23 15:43 ` Pierre Morel
0 siblings, 1 reply; 17+ messages in thread
From: Cornelia Huck @ 2021-02-23 13:22 UTC (permalink / raw)
To: Pierre Morel; +Cc: kvm, frankja, david, thuth, imbrenda
On Thu, 18 Feb 2021 18:26:42 +0100
Pierre Morel <pmorel@linux.ibm.com> wrote:
> We implement the call of the Set CHannel Monitor instruction,
> starting the monitoring of the all Channel Sub System, and
> initializing channel subsystem monitoring.
>
> Initial tests report the presence of the extended measurement block
> feature, and verify the error reporting of the hypervisor for SCHM.
>
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
> lib/s390x/css.h | 21 +++++++++-
> lib/s390x/css_lib.c | 100 ++++++++++++++++++++++++++++++++++++++++++++
> s390x/css.c | 35 ++++++++++++++++
> 3 files changed, 155 insertions(+), 1 deletion(-)
(...)
> diff --git a/lib/s390x/css_lib.c b/lib/s390x/css_lib.c
> index 65b58ff..4c8a6ae 100644
> --- a/lib/s390x/css_lib.c
> +++ b/lib/s390x/css_lib.c
> @@ -265,6 +265,106 @@ retry:
> return -1;
> }
>
> +/*
> + * schib_update_mb: update the subchannel Mesurement Block
s/Mesurement/Measurement/
I guess that one is hard to get out of one's fingers :)
> + * @schid: Subchannel Identifier
> + * @mb : 64bit address of the measurement block
> + * @mbi : the measurement block offset
> + * @flags : PMCW_MBUE to enable measurement block update
> + * PMCW_DCTME to enable device connect time
> + * 0 to disable measurement
> + * @format1: set if format 1 is to be used
> + */
> +static bool schib_update_mb(int schid, uint64_t mb, uint16_t mbi,
> + uint16_t flags, bool format1)
> +{
> + struct pmcw *pmcw = &schib.pmcw;
> + int cc;
> +
> + /* Read the SCHIB for this subchannel */
> + cc = stsch(schid, &schib);
> + if (cc) {
> + report_info("stsch: sch %08x failed with cc=%d", schid, cc);
> + return false;
> + }
> +
> + /* Update the SCHIB to enable the measurement block */
> + if (flags) {
> + pmcw->flags |= flags;
> +
> + if (format1)
> + pmcw->flags2 |= PMCW_MBF1;
> + else
> + pmcw->flags2 &= ~PMCW_MBF1;
> +
> + pmcw->mbi = mbi;
> + schib.mbo = mb;
> + } else {
> + pmcw->flags &= ~(PMCW_MBUE | PMCW_DCTME);
> + }
> +
> + /* Tell the CSS we want to modify the subchannel */
> + cc = msch(schid, &schib);
> + if (cc) {
> + /*
> + * If the subchannel is status pending or
> + * if a function is in progress,
> + * we consider both cases as errors.
> + */
> + report_info("msch: sch %08x failed with cc=%d", schid, cc);
> + return false;
> + }
> +
> + /*
> + * Read the SCHIB again
> + */
> + cc = stsch(schid, &schib);
> + if (cc) {
> + report_info("stsch: updating sch %08x failed with cc=%d",
> + schid, cc);
> + return false;
> + }
> +
> + return true;
> +}
> +
> +/*
> + * css_enable_mb: enable the subchannel Mesurement Block
s/Mesurement/Measurement/
> + * @schid: Subchannel Identifier
> + * @mb : 64bit address of the measurement block
> + * @format1: set if format 1 is to be used
> + * @mbi : the measurement block offset
> + * @flags : PMCW_MBUE to enable measurement block update
> + * PMCW_DCTME to enable device connect time
> + */
> +bool css_enable_mb(int schid, uint64_t mb, uint16_t mbi, uint16_t flags,
> + bool format1)
> +{
> + int retry_count = MAX_ENABLE_RETRIES;
> + struct pmcw *pmcw = &schib.pmcw;
> +
> + while (retry_count-- &&
> + !schib_update_mb(schid, mb, mbi, flags, format1))
> + mdelay(10); /* the hardware was not ready, give it some time */
> +
> + return schib.mbo == mb && pmcw->mbi == mbi;
> +}
> +
> +/*
> + * css_disable_mb: enable the subchannel Mesurement Block
s/enable/disable/
s/Mesurement/Measurement/
> + * @schid: Subchannel Identifier
> + */
> +bool css_disable_mb(int schid)
> +{
> + int retry_count = MAX_ENABLE_RETRIES;
> +
> + while (retry_count-- &&
> + !schib_update_mb(schid, 0, 0, 0, 0))
> + mdelay(10); /* the hardware was not ready, give it some time */
> +
> + return retry_count > 0;
> +}
> +
> static struct irb irb;
>
> void css_irq_io(void)
(...)
I'd still have split out the subchannel-modifying functions into a
separate patch, but no strong opinion.
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [kvm-unit-tests PATCH v3 3/5] s390x: css: implementing Set CHannel Monitor
2021-02-23 13:22 ` Cornelia Huck
@ 2021-02-23 15:43 ` Pierre Morel
0 siblings, 0 replies; 17+ messages in thread
From: Pierre Morel @ 2021-02-23 15:43 UTC (permalink / raw)
To: Cornelia Huck; +Cc: kvm, frankja, david, thuth, imbrenda
On 2/23/21 2:22 PM, Cornelia Huck wrote:
> On Thu, 18 Feb 2021 18:26:42 +0100
> Pierre Morel <pmorel@linux.ibm.com> wrote:
>
>> We implement the call of the Set CHannel Monitor instruction,
>> starting the monitoring of the all Channel Sub System, and
>> initializing channel subsystem monitoring.
>>
>> Initial tests report the presence of the extended measurement block
>> feature, and verify the error reporting of the hypervisor for SCHM.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>> lib/s390x/css.h | 21 +++++++++-
>> lib/s390x/css_lib.c | 100 ++++++++++++++++++++++++++++++++++++++++++++
>> s390x/css.c | 35 ++++++++++++++++
>> 3 files changed, 155 insertions(+), 1 deletion(-)
>
> (...)
>
>> diff --git a/lib/s390x/css_lib.c b/lib/s390x/css_lib.c
>> index 65b58ff..4c8a6ae 100644
>> --- a/lib/s390x/css_lib.c
>> +++ b/lib/s390x/css_lib.c
>> @@ -265,6 +265,106 @@ retry:
>> return -1;
>> }
>>
>> +/*
>> + * schib_update_mb: update the subchannel Mesurement Block
>
> s/Mesurement/Measurement/
>
> I guess that one is hard to get out of one's fingers :)
grrr yes, thanks
>
>> + * @schid: Subchannel Identifier
>> + * @mb : 64bit address of the measurement block
>> + * @mbi : the measurement block offset
>> + * @flags : PMCW_MBUE to enable measurement block update
>> + * PMCW_DCTME to enable device connect time
>> + * 0 to disable measurement
>> + * @format1: set if format 1 is to be used
>> + */
>> +static bool schib_update_mb(int schid, uint64_t mb, uint16_t mbi,
>> + uint16_t flags, bool format1)
>> +{
>> + struct pmcw *pmcw = &schib.pmcw;
>> + int cc;
>> +
>> + /* Read the SCHIB for this subchannel */
>> + cc = stsch(schid, &schib);
>> + if (cc) {
>> + report_info("stsch: sch %08x failed with cc=%d", schid, cc);
>> + return false;
>> + }
>> +
>> + /* Update the SCHIB to enable the measurement block */
>> + if (flags) {
>> + pmcw->flags |= flags;
>> +
>> + if (format1)
>> + pmcw->flags2 |= PMCW_MBF1;
>> + else
>> + pmcw->flags2 &= ~PMCW_MBF1;
>> +
>> + pmcw->mbi = mbi;
>> + schib.mbo = mb;
>> + } else {
>> + pmcw->flags &= ~(PMCW_MBUE | PMCW_DCTME);
>> + }
>> +
>> + /* Tell the CSS we want to modify the subchannel */
>> + cc = msch(schid, &schib);
>> + if (cc) {
>> + /*
>> + * If the subchannel is status pending or
>> + * if a function is in progress,
>> + * we consider both cases as errors.
>> + */
>> + report_info("msch: sch %08x failed with cc=%d", schid, cc);
>> + return false;
>> + }
>> +
>> + /*
>> + * Read the SCHIB again
>> + */
>> + cc = stsch(schid, &schib);
>> + if (cc) {
>> + report_info("stsch: updating sch %08x failed with cc=%d",
>> + schid, cc);
>> + return false;
>> + }
>> +
>> + return true;
>> +}
>> +
>> +/*
>> + * css_enable_mb: enable the subchannel Mesurement Block
>
> s/Mesurement/Measurement/
>
>> + * @schid: Subchannel Identifier
>> + * @mb : 64bit address of the measurement block
>> + * @format1: set if format 1 is to be used
>> + * @mbi : the measurement block offset
>> + * @flags : PMCW_MBUE to enable measurement block update
>> + * PMCW_DCTME to enable device connect time
>> + */
>> +bool css_enable_mb(int schid, uint64_t mb, uint16_t mbi, uint16_t flags,
>> + bool format1)
>> +{
>> + int retry_count = MAX_ENABLE_RETRIES;
>> + struct pmcw *pmcw = &schib.pmcw;
>> +
>> + while (retry_count-- &&
>> + !schib_update_mb(schid, mb, mbi, flags, format1))
>> + mdelay(10); /* the hardware was not ready, give it some time */
>> +
>> + return schib.mbo == mb && pmcw->mbi == mbi;
>> +}
>> +
>> +/*
>> + * css_disable_mb: enable the subchannel Mesurement Block
>
> s/enable/disable/
hum yes,
> s/Mesurement/Measurement/
... /o\
>
>> + * @schid: Subchannel Identifier
>> + */
>> +bool css_disable_mb(int schid)
>> +{
>> + int retry_count = MAX_ENABLE_RETRIES;
>> +
>> + while (retry_count-- &&
>> + !schib_update_mb(schid, 0, 0, 0, 0))
>> + mdelay(10); /* the hardware was not ready, give it some time */
>> +
>> + return retry_count > 0;
>> +}
>> +
>> static struct irb irb;
>>
>> void css_irq_io(void)
>
> (...)
>
> I'd still have split out the subchannel-modifying functions into a
> separate patch, but no strong opinion.
Will do it since I need a respin due to all the Me"a"surement errors!
>
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
>
Thanks,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
^ permalink raw reply [flat|nested] 17+ messages in thread
* [kvm-unit-tests PATCH v3 4/5] s390x: css: testing measurement block format 0
2021-02-18 17:26 [kvm-unit-tests PATCH v3 0/5] CSS Mesurement Block Pierre Morel
` (2 preceding siblings ...)
2021-02-18 17:26 ` [kvm-unit-tests PATCH v3 3/5] s390x: css: implementing Set CHannel Monitor Pierre Morel
@ 2021-02-18 17:26 ` Pierre Morel
2021-02-23 13:27 ` Cornelia Huck
2021-02-18 17:26 ` [kvm-unit-tests PATCH v3 5/5] s390x: css: testing measurement block format 1 Pierre Morel
4 siblings, 1 reply; 17+ messages in thread
From: Pierre Morel @ 2021-02-18 17:26 UTC (permalink / raw)
To: kvm; +Cc: frankja, david, thuth, cohuck, imbrenda
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/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 }
};
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [kvm-unit-tests PATCH v3 4/5] s390x: css: testing measurement block format 0
2021-02-18 17:26 ` [kvm-unit-tests PATCH v3 4/5] s390x: css: testing measurement block format 0 Pierre Morel
@ 2021-02-23 13:27 ` Cornelia Huck
2021-02-23 15:49 ` Pierre Morel
0 siblings, 1 reply; 17+ messages in thread
From: Cornelia Huck @ 2021-02-23 13:27 UTC (permalink / raw)
To: Pierre Morel; +Cc: kvm, frankja, david, thuth, imbrenda
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 }
> };
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [kvm-unit-tests PATCH v3 4/5] s390x: css: testing measurement block format 0
2021-02-23 13:27 ` Cornelia Huck
@ 2021-02-23 15:49 ` Pierre Morel
2021-02-23 16:05 ` Cornelia Huck
0 siblings, 1 reply; 17+ messages in thread
From: Pierre Morel @ 2021-02-23 15:49 UTC (permalink / raw)
To: Cornelia Huck; +Cc: kvm, frankja, david, thuth, imbrenda
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
--
Pierre Morel
IBM Lab Boeblingen
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [kvm-unit-tests PATCH v3 4/5] s390x: css: testing measurement block format 0
2021-02-23 15:49 ` Pierre Morel
@ 2021-02-23 16:05 ` Cornelia Huck
0 siblings, 0 replies; 17+ messages in thread
From: Cornelia Huck @ 2021-02-23 16:05 UTC (permalink / raw)
To: Pierre Morel; +Cc: kvm, frankja, david, thuth, imbrenda
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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [kvm-unit-tests PATCH v3 5/5] s390x: css: testing measurement block format 1
2021-02-18 17:26 [kvm-unit-tests PATCH v3 0/5] CSS Mesurement Block Pierre Morel
` (3 preceding siblings ...)
2021-02-18 17:26 ` [kvm-unit-tests PATCH v3 4/5] s390x: css: testing measurement block format 0 Pierre Morel
@ 2021-02-18 17:26 ` Pierre Morel
2021-02-23 13:29 ` Cornelia Huck
2021-02-26 10:02 ` Janosch Frank
4 siblings, 2 replies; 17+ messages in thread
From: Pierre Morel @ 2021-02-18 17:26 UTC (permalink / raw)
To: kvm; +Cc: frankja, david, thuth, cohuck, imbrenda
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.h b/lib/s390x/css.h
index dabe54a..1e5e4b5 100644
--- a/lib/s390x/css.h
+++ b/lib/s390x/css.h
@@ -387,4 +387,20 @@ struct measurement_block_format0 {
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;
+};
+
+void msch_with_wrong_fmt1_mbo(unsigned int schid, uint64_t mb);
+
#endif
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;
} 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 */
+ 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 }
};
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [kvm-unit-tests PATCH v3 5/5] s390x: css: testing measurement block format 1
2021-02-18 17:26 ` [kvm-unit-tests PATCH v3 5/5] s390x: css: testing measurement block format 1 Pierre Morel
@ 2021-02-23 13:29 ` Cornelia Huck
2021-02-23 15:52 ` Pierre Morel
2021-02-26 10:02 ` Janosch Frank
1 sibling, 1 reply; 17+ messages in thread
From: Cornelia Huck @ 2021-02-23 13:29 UTC (permalink / raw)
To: Pierre Morel; +Cc: kvm, frankja, david, thuth, imbrenda
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 }
> };
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [kvm-unit-tests PATCH v3 5/5] s390x: css: testing measurement block format 1
2021-02-23 13:29 ` Cornelia Huck
@ 2021-02-23 15:52 ` Pierre Morel
0 siblings, 0 replies; 17+ messages in thread
From: Pierre Morel @ 2021-02-23 15:52 UTC (permalink / raw)
To: Cornelia Huck; +Cc: kvm, frankja, david, thuth, imbrenda
On 2/23/21 2:29 PM, Cornelia Huck wrote:
> 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;
indeed, looks like a later change.
...snip...
>> + */
>> +static void test_schm_fmt1(void)
>> +{
>> +
...snip...
>> + /* 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().
I agree.
Thanks,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [kvm-unit-tests PATCH v3 5/5] s390x: css: testing measurement block format 1
2021-02-18 17:26 ` [kvm-unit-tests PATCH v3 5/5] s390x: css: testing measurement block format 1 Pierre Morel
2021-02-23 13:29 ` Cornelia Huck
@ 2021-02-26 10:02 ` Janosch Frank
2021-02-26 13:34 ` Pierre Morel
1 sibling, 1 reply; 17+ messages in thread
From: Janosch Frank @ 2021-02-26 10:02 UTC (permalink / raw)
To: Pierre Morel, kvm; +Cc: david, thuth, cohuck, imbrenda
On 2/18/21 6:26 PM, Pierre Morel 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.h b/lib/s390x/css.h
> index dabe54a..1e5e4b5 100644
> --- a/lib/s390x/css.h
> +++ b/lib/s390x/css.h
> @@ -387,4 +387,20 @@ struct measurement_block_format0 {
> 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;
> +};
> +
> +void msch_with_wrong_fmt1_mbo(unsigned int schid, uint64_t mb);
> +
> #endif
> 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;
> } 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);
Why would you expect a PGM in a library function are PGMs normal for IO
instructions? oO
Is this a test function which should be part of your test file in
s390x/*.c or is it part of the IO library which should:
- Abort if an initialization failed and we can assume that future tests
are now useless
- Return an error so the test can report an error
- Return success
> +}
> 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 */
> + 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 },
Output will then be:
"measurement block format1: Format 1: Report message"
Wouldn't it make more sense to put the format 0 and 1 tests into
test_schm() so we'd have:
"measurement block (schm): Format 0: Report message" ?
> { NULL, NULL }
> };
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [kvm-unit-tests PATCH v3 5/5] s390x: css: testing measurement block format 1
2021-02-26 10:02 ` Janosch Frank
@ 2021-02-26 13:34 ` Pierre Morel
0 siblings, 0 replies; 17+ messages in thread
From: Pierre Morel @ 2021-02-26 13:34 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: david, thuth, cohuck, imbrenda
On 2/26/21 11:02 AM, Janosch Frank wrote:
> On 2/18/21 6:26 PM, Pierre Morel 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.
...
>> +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);
>
> Why would you expect a PGM in a library function are PGMs normal for IO
> instructions? oO
>
> Is this a test function which should be part of your test file in
> s390x/*.c or is it part of the IO library which should:
>
> - Abort if an initialization failed and we can assume that future tests
> are now useless
> - Return an error so the test can report an error
> - Return success
Now it looks clear to me that this test belongs to the tests and not the
lib.
I put it there to avoid exporting the SCHIB, but after all, why not
exporting the SCHIB, we may need access to it from other tests again in
the future.
>
>> +}
>> 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;
>> + }
...
>> + 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 */
>> + 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 },
>
> Output will then be:
> "measurement block format1: Format 1: Report message"
>
> Wouldn't it make more sense to put the format 0 and 1 tests into
> test_schm() so we'd have:
> "measurement block (schm): Format 0: Report message" ?
too much prefix push...
rationalizing will make the goto disapear...
Thanks for review
Regards,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
^ permalink raw reply [flat|nested] 17+ messages in thread