On Wed, 23 Oct 2019 08:48:33 -0400 (EDT)
Thomas Huth <thuth@xxxxxxxxxx> wrote:
> ----- Original Message -----
> > From: "Claudio Imbrenda" <imbrenda@xxxxxxxxxxxxx>
> > Sent: Tuesday, October 22, 2019 12:53:04 PM
> >
> > 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
>
> Copy-n-paste from stsi.c ?
Oops
>
> > + * 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 int test_one_run(uint32_t cmd, uint64_t addr, uint16_t len,
> > + uint16_t clear, uint64_t exp_pgm, uint16_t
> > exp_rc) +{
> > + char sccb[4096];
> > + void *p = sccb;
> > +
> > + if (!len && !clear)
> > + p = NULL;
> > + else
> > + memset(sccb, 0, sizeof(sccb));
> > + ((SCCBHeader *)sccb)->length = len;
> > + if (clear && (clear < 8))
>
> Please remove the parentheses around "clear < 8".
I usually prefer to have more parentheses than necessary
than fewer, but I'll fix it
>
> > + clear = 8;
> > + 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)
> > +
> > +static void test_sccb_too_short(void)
> > +{
> > + int cx;
> > +
> > + for (cx = 0; cx < 8; cx++)
> > + if (!test_one_run(VALID, PGBUF, cx, 8,
> > PGM_BIT_SPEC, 0))
> > + break;
> > +
> > + report("SCCB too short", cx == 8);
> > +}
> > +
> > +static void test_sccb_unaligned(void)
> > +{
> > + int cx;
> > +
> > + for (cx = 1; cx < 8; cx++)
> > + if (!test_one_run(VALID, cx + PGBUF, 8, 8,
> > PGM_BIT_SPEC, 0))
> > + break;
> > + report("SCCB unaligned", cx == 8);
> > +}
> > +
> > +static void test_sccb_prefix(void)
> > +{
> > + uint32_t prefix, new_prefix;
> > + int cx;
> > +
> > + for (cx = 0; cx < 8192; cx += 8)
> > + if (!test_one_run(VALID, cx, 0, 0, PGM_BIT_SPEC,
> > 0))
> > + break;
> > + report("SCCB low pages", cx == 8192);
> > +
> > + new_prefix = (uint32_t)(intptr_t)prefix_buf;
> > + memcpy(prefix_buf, 0, 8192);
> > + asm volatile("stpx %0": "+Q"(prefix));
>
> Isn't "=Q" sufficient enough here?
Ooops. think I copypasted this from somewhere else. Will fix.
>
> > + asm volatile("spx %0": "+Q"(new_prefix));
>
> Shouldn't that be just an input parameter instead? ... and maybe also
> better add "memory" to the clobber list, since the memory layout has
> changed.
same
>
> > + for (cx = 0; cx < 8192; cx += 8)
> > + if (!test_one_run(VALID, new_prefix + cx, 8, 8,
> > PGM_BIT_SPEC, 0))
> > + break;
> > + report("SCCB prefix pages", cx == 8192);
> > +
> > + memcpy(prefix_buf, 0, 8192);
> > + asm volatile("spx %0": "+Q"(prefix));
>
> dito?
same
>
> > +}
>
> Thomas
>
