On 08/01/2020 17.13, 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 | 462 ++++++++++++++++++++++++++++++++++++++++++++
> s390x/unittests.cfg | 8 +
> 3 files changed, 471 insertions(+)
> create mode 100644 s390x/sclp.c
[...]
> +/**
> + * Test SCCBs whose address is in the lowcore or prefix area.
> + */
> +static void test_sccb_prefix(void)
> +{
> + uint8_t scratch[2 * PAGE_SIZE];
> + uint32_t prefix, new_prefix;
> + int offset;
> +
> + /*
> + * copy the current lowcore to the future new location, otherwise we
> + * will not have a valid lowcore after setting the new prefix.
> + */
> + memcpy(prefix_buf, 0, 2 * PAGE_SIZE);
> + /* save the current prefix (it's probably going to be 0) */
> + prefix = stpx();
> + /*
> + * save the current content of absolute pages 0 and 1, so we can
> + * restore them after we trash them later on
> + */
> + memcpy(scratch, (void *)(intptr_t)prefix, 2 * PAGE_SIZE);
> + /* set the new prefix to prefix_buf */
> + new_prefix = (uint32_t)(intptr_t)prefix_buf;
> + spx(new_prefix);
> +
> + /*
> + * testing with SCCB addresses in the lowcore; since we can't
> + * actually trash the lowcore (unsurprisingly, things break if we
> + * do), this will be a read-only test.
> + */
> + for (offset = 0; offset < 2 * PAGE_SIZE; offset += 8)
> + if (!test_one_sccb(valid_code, MKPTR(offset), 0, PGM_BIT_SPEC, 0))
> + break;
> + report(offset == 2 * PAGE_SIZE, "SCCB low pages");
> +
> + /*
> + * this will trash the contents of the two pages at absolute
> + * address 0; we will need to restore them later.
> + */
I'm still a bit confused by this comment - will SCLP really trash the
contents here, or will there be a specification exception (since
PGM_BIT_SPEC is given below)? ... maybe you could clarify the comment in
case you respin again (or it could be fixed when picking up the patch)?
> + for (offset = 0; offset < 2 * PAGE_SIZE; offset += 8)
> + if (!test_one_simple(valid_code, MKPTR(new_prefix + offset), 8, 8, PGM_BIT_SPEC, 0))
> + break;
> + report(offset == 2 * PAGE_SIZE, "SCCB prefix pages");
> +
> + /* restore the previous contents of absolute pages 0 and 1 */
> + memcpy(prefix_buf, 0, 2 * PAGE_SIZE);
> + /* restore the prefix to the original value */
> + spx(prefix);
> +}
Remaining parts look ok to me now, so with the comment clarified:
Reviewed-by: Thomas Huth <thuth@xxxxxxxxxx>
