diff mbox series

[kvm-unit-tests,v3,1/5] s390x: css: Store CSS Characteristics

Message ID 1613669204-6464-2-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 
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(-)

Comments

Janosch Frank Feb. 26, 2021, 9:50 a.m. UTC | #1 
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 },
>
Pierre Morel Feb. 26, 2021, 1:38 p.m. UTC | #2 
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 },
>>
>
diff mbox series

Patch

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 },