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@xxxxxxxxxxxxx>
> Reviewed-by: Cornelia Huck <cohuck@xxxxxxxxxx>
> ---
> 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 },
>
