On 22.10.19 12:53, Claudio Imbrenda wrote:
SCLP unit test. Testing the following:
* Correctly ignoring instruction bits that should be ignored
* Privileged instruction check
* Check for addressing exceptions
* Specification exceptions:
- SCCB size less than 8
- SCCB unaligned
- SCCB overlaps prefix or lowcore
- SCCB address higher than 2GB
* Return codes for
- Invalid command
- SCCB too short (but at least 8)
- SCCB page boundary violation
Signed-off-by: Claudio Imbrenda <imbrenda@xxxxxxxxxxxxx>
---
s390x/Makefile | 1 +
s390x/sclp.c | 373 ++++++++++++++++++++++++++++++++++++++++++++++++++++
s390x/unittests.cfg | 3 +
3 files changed, 377 insertions(+)
create mode 100644 s390x/sclp.c
diff --git a/s390x/Makefile b/s390x/Makefile
index 3744372..ddb4b48 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -16,6 +16,7 @@ tests += $(TEST_DIR)/diag288.elf
tests += $(TEST_DIR)/stsi.elf
tests += $(TEST_DIR)/skrf.elf
tests += $(TEST_DIR)/smp.elf
+tests += $(TEST_DIR)/sclp.elf
tests_binary = $(patsubst %.elf,%.bin,$(tests))
all: directories test_cases test_cases_binary
diff --git a/s390x/sclp.c b/s390x/sclp.c
new file mode 100644
index 0000000..d7a9212
--- /dev/null
+++ b/s390x/sclp.c
@@ -0,0 +1,373 @@
+/*
+ * Store System Information tests
+ *
+ * Copyright (c) 2019 IBM Corp
+ *
+ * Authors:
+ * Claudio Imbrenda <imbrenda@xxxxxxxxxxxxx>
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2.
+ */
+
+#include <libcflat.h>
+#include <asm/page.h>
+#include <asm/asm-offsets.h>
+#include <asm/interrupt.h>
+#include <sclp.h>
+
+static uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2)));
+static uint8_t prefix_buf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2)));
+static uint32_t valid_sclp_code;
+static struct lowcore *lc;
+
+static void sclp_setup_int_test(void)
+{
+ uint64_t mask;
+
+ ctl_set_bit(0, 9);
+
+ mask = extract_psw_mask();
+ mask |= PSW_MASK_EXT;
+ load_psw_mask(mask);
+}
+
+static int sclp_service_call_test(unsigned int command, void *sccb)
Wouldn't it be easier to pass an uint8_t*, so you can simply forward
pagebuf through all functions?
+{
+ int cc;
+
+ sclp_mark_busy();
+ sclp_setup_int_test();
+ lc->pgm_int_code = 0;
+ cc = servc(command, __pa(sccb));
+ if (lc->pgm_int_code) {
+ sclp_handle_ext();
You receive a PGM interrupt and handle an external interrupt? That looks
strange. Please elaborate what's going on here.
+ return 0;
+ }
+ if (!cc)
+ sclp_wait_busy();
+ return cc;
+}
+
+static int test_one_sccb(uint32_t cmd, uint64_t addr, uint16_t len,
+ void *data, uint64_t exp_pgm, uint16_t exp_rc)
+{
+ SCCBHeader *h = (SCCBHeader *)addr;
+ int res, pgm;
+
+ if (data && len)
+ memcpy((void *)addr, data, len);
+ if (!exp_pgm)
+ exp_pgm = 1;
+ expect_pgm_int();
+ res = sclp_service_call_test(cmd, h);
+ if (res) {
+ report_info("SCLP not ready (command %#x, address %#lx, cc %d)",
+ cmd, addr, res);
+ return 0;
+ }
+ pgm = lc->pgm_int_code;
+ if (!((1ULL << pgm) & exp_pgm)) {
+ report_info("First failure at addr %#lx, size %d, cmd %#x, pgm code %d",
+ addr, len, cmd, pgm);
+ return 0;
+ }
+ if (exp_rc && (exp_rc != h->response_code)) {
+ report_info("First failure at addr %#lx, size %d, cmd %#x, resp code %#x",
+ addr, len, cmd, h->response_code);
+ return 0;
+ }
+ return 1;
+}
+
+static int test_one_run(uint32_t cmd, uint64_t addr, uint16_t len,
+ uint16_t clear, uint64_t exp_pgm, uint16_t exp_rc)
I somewhat dislike passing in "exp_pgm" and "exp_rc". Why can't you
handle both things in the caller where the tests actually become readable?
+{
+ char sccb[4096];
I prefer uint8_t sccb[PAGE_SIZE]
+ void *p = sccb;
+
+ if (!len && !clear)
+ p = NULL;
+ else
+ memset(sccb, 0, sizeof(sccb));
+ ((SCCBHeader *)sccb)->length = len;
+ if (clear && (clear < 8))
+ clear = 8;
Can you elaborate what "clear" means. It is passed as "length".
/me confused
+ return test_one_sccb(cmd, addr, clear, p, exp_pgm, exp_rc);
+}
+
+#define PGM_BIT_SPEC (1ULL << PGM_INT_CODE_SPECIFICATION)
+#define PGM_BIT_ADDR (1ULL << PGM_INT_CODE_ADDRESSING)
+#define PGM_BIT_PRIV (1ULL << PGM_INT_CODE_PRIVILEGED_OPERATION)
+
+#define PGBUF ((uintptr_t)pagebuf)
+#define VALID (valid_sclp_code)
I dislike both defines, can you get rid of these?
+
+static void test_sccb_too_short(void)
+{
+ int cx;
cx is passed as "len". What does cx stand for? Can we give this a better
name?
[...] (not reviewing the other stuff in detail because I am still confused)
+static void test_resp(void)
+{
+ test_inval();
+ test_short();
+ test_boundary();
+ test_toolong();
+}
Can you just get rid of this function and call all tests from main?
(just separate them in logical blocks eventually with comments)
+
+static void test_priv(void)
+{
+ pagebuf[0] = 0;
+ pagebuf[1] = 8;
+ expect_pgm_int();
+ enter_pstate();
+ servc(valid_sclp_code, __pa(pagebuf));
+ report_prefix_push("Priv check");
+ check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION);
+ report_prefix_pop();
Can we push at the beginning of the function and pop at the end?
report_prefix_push("Privileged Operation");
expect_pgm_int();
enter_pstate();
servc(valid_sclp_code, __pa(pagebuf));
check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION);
report_prefix_pop();
Also shouldn't you better mark sclp busy and wait for interrupts to make
sore you handle it correctly in case the check *fails* and servc
completes (cc==0)?
+}
+
+static void test_addressing(void)
+{
+ unsigned long cx, maxram = get_ram_size();
+
+ if (maxram >= 0x80000000) {
+ report_skip("Invalid SCCB address");
+ return;
+ }
Do we really need maxram here, can't we simply use very high addresses
like in all other tests?
E.g. just user address "-PAGE_SIZE"
+ for (cx = maxram; cx < MIN(maxram + 65536, 0x80000000); cx += 8)
+ if (!test_one_run(VALID, cx, 0, 0, PGM_BIT_ADDR, 0))
+ goto out;
+ for (; cx < MIN((maxram + 0x7fffff) & ~0xfffff, 0x80000000); cx += 4096)
+ if (!test_one_run(VALID, cx, 0, 0, PGM_BIT_ADDR, 0))
+ goto out;
+ for (; cx < 0x80000000; cx += 1048576)
+ if (!test_one_run(VALID, cx, 0, 0, PGM_BIT_ADDR, 0))
+ goto out;
+out:
+ report("Invalid SCCB address", cx == 0x80000000);
+}
+
+static void test_instbits(void)
+{
+ SCCBHeader *h = (SCCBHeader *)pagebuf;
+ unsigned long mask;
+ int cc;
+
+ sclp_mark_busy();
+ h->length = 8;
+
+ ctl_set_bit(0, 9);
+ mask = extract_psw_mask();
+ mask |= PSW_MASK_EXT;
+ load_psw_mask(mask);
+
+ asm volatile(
+ " .insn rre,0xb2204200,%1,%2\n" /* servc %1,%2 */
+ " ipm %0\n"
+ " srl %0,28"
+ : "=&d" (cc) : "d" (valid_sclp_code), "a" (__pa(pagebuf))
+ : "cc", "memory");
+ sclp_wait_busy();
+ report("Instruction format ignored bits", cc == 0);
+}
+
+static void find_valid_sclp_code(void)
+{
+ unsigned int commands[] = { SCLP_CMDW_READ_SCP_INFO_FORCED,
+ SCLP_CMDW_READ_SCP_INFO };
+ SCCBHeader *h = (SCCBHeader *)pagebuf;
+ int i, cc;
+
+ for (i = 0; i < ARRAY_SIZE(commands); i++) {
+ sclp_mark_busy();
+ memset(h, 0, sizeof(pagebuf));
+ h->length = 4096;
+
+ valid_sclp_code = commands[i];
+ cc = sclp_service_call(commands[i], h);
+ if (cc)
+ break;
+ if (h->response_code == SCLP_RC_NORMAL_READ_COMPLETION)
+ return;
+ if (h->response_code != SCLP_RC_INVALID_SCLP_COMMAND)
+ break;
+ }
+ valid_sclp_code = 0;
+ report_abort("READ_SCP_INFO failed");
+}
+
+int main(void)
+{
+ report_prefix_push("sclp");
+ find_valid_sclp_code();
+ test_instbits();
+ test_priv();
+ test_addressing();
+ test_spec();
+ test_resp();
+ return report_summary();
+}
diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
index f1b07cd..75e3d37 100644
--- a/s390x/unittests.cfg
+++ b/s390x/unittests.cfg
@@ -75,3 +75,6 @@ file = stsi.elf
[smp]
file = smp.elf
extra_params =-smp 2
+
+[sclp]
+file = sclp.elf
--
Thanks,
David / dhildenb
