Re: [kvm-unit-tests PATCH v4 2/7] s390x: Add guest 2 AP test

[Anthony Krowiak <akrowiak@linux.ibm.com>]

I made a couple of function name change suggestions, but those are not 
critical:

Acked-by: Anthony Krowiak <akrowiak@linux.ibm.com>

On 2/2/24 9:59 AM, Janosch Frank wrote:
> Add a test that checks the exceptions for the PQAP, NQAP and DQAP
> adjunct processor (AP) crypto instructions.
>
> Since triggering the exceptions doesn't require actual AP hardware,
> this test can run without complicated setup.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>   s390x/Makefile      |   1 +
>   s390x/ap.c          | 309 ++++++++++++++++++++++++++++++++++++++++++++
>   s390x/unittests.cfg |   3 +
>   3 files changed, 313 insertions(+)
>   create mode 100644 s390x/ap.c
>
> diff --git a/s390x/Makefile b/s390x/Makefile
> index 4f6c627d..6d28a5bf 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -42,6 +42,7 @@ tests += $(TEST_DIR)/exittime.elf
>   tests += $(TEST_DIR)/ex.elf
>   tests += $(TEST_DIR)/topology.elf
>   tests += $(TEST_DIR)/sie-dat.elf
> +tests += $(TEST_DIR)/ap.elf
>   
>   pv-tests += $(TEST_DIR)/pv-diags.elf
>   pv-tests += $(TEST_DIR)/pv-icptcode.elf
> diff --git a/s390x/ap.c b/s390x/ap.c
> new file mode 100644
> index 00000000..b3cee37a
> --- /dev/null
> +++ b/s390x/ap.c
> @@ -0,0 +1,309 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * AP instruction G2 tests
> + *
> + * Copyright (c) 2024 IBM Corp
> + *
> + * Authors:
> + *  Janosch Frank <frankja@linux.ibm.com>
> + */
> +
> +#include <libcflat.h>
> +#include <interrupt.h>
> +#include <bitops.h>
> +#include <alloc_page.h>
> +#include <asm/facility.h>
> +#include <asm/time.h>
> +#include <ap.h>
> +
> +/* For PQAP PGM checks where we need full control over the input */
> +static void pqap(unsigned long grs[3])
> +{
> +	asm volatile(
> +		"	lgr	0,%[r0]\n"
> +		"	lgr	1,%[r1]\n"
> +		"	lgr	2,%[r2]\n"
> +		"	.insn	rre,0xb2af0000,0,0\n" /* PQAP */
> +		::  [r0] "d" (grs[0]), [r1] "d" (grs[1]), [r2] "d" (grs[2])
> +		: "cc", "memory", "0", "1", "2");
> +}
> +
> +static void test_pgms_pqap(void)


If I saw this function name without having read the patch description, I 
wouldn't have any idea what is being tested.

Maybe test_pqap_pgm_chk?


> +{
> +	unsigned long grs[3] = {};
> +	struct pqap_r0 *r0 = (struct pqap_r0 *)grs;
> +	uint8_t *data = alloc_page();
> +	uint16_t pgm;
> +	int fails = 0;
> +	int i;
> +
> +	report_prefix_push("pqap");
> +
> +	/* Wrong FC code */
> +	report_prefix_push("invalid fc");
> +	r0->fc = 42;


Just out of curiosity, why 42? Why not some ridiculous number that will 
never be used for a function code, like 4294967295?


> +	expect_pgm_int();
> +	pqap(grs);
> +	check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
> +	memset(grs, 0, sizeof(grs));
> +	report_prefix_pop();
> +
> +	report_prefix_push("invalid gr0 bits");
> +	/*
> +	 * GR0 bits 41 - 47 are defined 0 and result in a
> +	 * specification exception if set to 1.
> +	 */
> +	for (i = 0; i < 48 - 41; i++) {
> +		grs[0] = BIT(63 - 47 + i);
> +
> +		expect_pgm_int();
> +		pqap(grs);
> +		pgm = clear_pgm_int();
> +
> +		if (pgm != PGM_INT_CODE_SPECIFICATION) {
> +			report_fail("fail on bit %d", 42 + i);
> +			fails++;
> +		}
> +	}
> +	report(!fails, "All bits tested");
> +	memset(grs, 0, sizeof(grs));
> +	fails = 0;
> +	report_prefix_pop();
> +
> +	report_prefix_push("alignment");
> +	report_prefix_push("fc=4");
> +	r0->fc = PQAP_QUERY_AP_CONF_INFO;
> +	grs[2] = (unsigned long)data;
> +	for (i = 1; i < 8; i++) {
> +		expect_pgm_int();
> +		grs[2]++;
> +		pqap(grs);
> +		pgm = clear_pgm_int();
> +		if (pgm != PGM_INT_CODE_SPECIFICATION) {
> +			report_fail("fail on bit %d", i);
> +			fails++;
> +		}
> +	}
> +	report(!fails, "All alignments tested");
> +	report_prefix_pop();
> +	report_prefix_push("fc=6");
> +	r0->fc = PQAP_BEST_AP;
> +	grs[2] = (unsigned long)data;
> +	for (i = 1; i < 8; i++) {
> +		expect_pgm_int();
> +		grs[2]++;
> +		pqap(grs);
> +		pgm = clear_pgm_int();
> +		if (pgm != PGM_INT_CODE_SPECIFICATION) {
> +			report_fail("fail on bit %d", i);
> +			fails++;
> +		}
> +	}
> +	report(!fails, "All alignments tested");
> +	report_prefix_pop();
> +	report_prefix_pop();
> +
> +	free_page(data);
> +	report_prefix_pop();
> +}
> +
> +static void test_pgms_nqap(void)


Same as above:
test_nqap_pgm_chk


> +{
> +	uint8_t gr0_zeroes_bits[] = {
> +		32, 34, 35, 40
> +	};
> +	uint64_t gr0;
> +	bool fail;
> +	int i;
> +
> +	report_prefix_push("nqap");
> +
> +	/* Registers 0 and 1 are always used, the others are even/odd pairs */
> +	report_prefix_push("spec");
> +	report_prefix_push("r1");
> +	expect_pgm_int();
> +	asm volatile (
> +		".insn	rre,0xb2ad0000,3,6\n"
> +		: : : "cc", "memory", "0", "1", "2", "3", "4", "6", "7");
> +	check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
> +	report_prefix_pop();
> +
> +	report_prefix_push("r2");
> +	expect_pgm_int();
> +	asm volatile (
> +		".insn	rre,0xb2ad0000,2,7\n"
> +		: : : "cc", "memory", "0", "1", "2", "3", "4", "6", "7");
> +	check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
> +	report_prefix_pop();
> +
> +	report_prefix_push("both");
> +	expect_pgm_int();
> +	asm volatile (
> +		".insn	rre,0xb2ad0000,3,7\n"
> +		: : : "cc", "memory", "0", "1", "2", "3", "4", "6", "7");
> +	check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
> +	report_prefix_pop();
> +
> +	report_prefix_push("len==0");
> +	expect_pgm_int();
> +	asm volatile (
> +		"xgr	0,0\n"
> +		"xgr	5,5\n"
> +		".insn	rre,0xb2ad0000,2,4\n"
> +		: : : "cc", "memory", "0", "1", "2", "3", "4", "5", "6", "7");
> +	check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
> +	report_prefix_pop();
> +
> +	report_prefix_push("gr0_zero_bits");
> +	fail = false;
> +	for (i = 0; i < ARRAY_SIZE(gr0_zeroes_bits); i++) {
> +		expect_pgm_int();
> +		gr0 = BIT_ULL(63 - gr0_zeroes_bits[i]);
> +		asm volatile (
> +			"xgr	5,5\n"
> +			"lghi	5, 128\n"
> +			"lg	0, 0(%[val])\n"
> +			".insn	rre,0xb2ad0000,2,4\n"
> +			: : [val] "a" (&gr0)
> +			: "cc", "memory", "0", "1", "2", "3", "4", "5", "6", "7");
> +		if (clear_pgm_int() != PGM_INT_CODE_SPECIFICATION) {
> +			report_fail("setting gr0 bit %d did not result in a spec exception",
> +				    gr0_zeroes_bits[i]);
> +			fail = true;
> +		}
> +	}
> +	report(!fail, "set bit gr0 pgms");
> +	report_prefix_pop();
> +
> +	report_prefix_pop();
> +	report_prefix_pop();
> +}
> +
> +static void test_pgms_dqap(void)


Same as above:
test_dqap_pgm_chk


> +{
> +	uint8_t gr0_zeroes_bits[] = {
> +		33, 34, 35, 40, 41
> +	};
> +	uint64_t gr0;
> +	bool fail;
> +	int i;
> +
> +	report_prefix_push("dqap");
> +
> +	/* Registers 0 and 1 are always used, the others are even/odd pairs */
> +	report_prefix_push("spec");
> +	report_prefix_push("r1");
> +	expect_pgm_int();
> +	asm volatile (
> +		".insn	rre,0xb2ae0000,3,6\n"
> +		: : : "cc", "memory", "0", "1", "2", "3", "4", "6", "7");
> +	check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
> +	report_prefix_pop();
> +
> +	report_prefix_push("r2");
> +	expect_pgm_int();
> +	asm volatile (
> +		".insn	rre,0xb2ae0000,2,7\n"
> +		: : : "cc", "memory", "0", "1", "2", "3", "4", "6", "7");
> +	check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
> +	report_prefix_pop();
> +
> +	report_prefix_push("both");
> +	expect_pgm_int();
> +	asm volatile (
> +		".insn	rre,0xb2ae0000,3,7\n"
> +		: : : "cc", "memory", "0", "1", "2", "3", "4", "6", "7");
> +	check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
> +	report_prefix_pop();
> +
> +	report_prefix_push("len==0");
> +	expect_pgm_int();
> +	asm volatile (
> +		"xgr	0,0\n"
> +		"xgr	5,5\n"
> +		".insn	rre,0xb2ae0000,2,4\n"
> +		: : : "cc", "memory", "0", "1", "2", "3", "4", "5", "6", "7");
> +	check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
> +	report_prefix_pop();
> +
> +	report_prefix_push("gr0_zero_bits");
> +	fail = false;
> +	for (i = 0; i < ARRAY_SIZE(gr0_zeroes_bits); i++) {
> +		expect_pgm_int();
> +		gr0 = BIT_ULL(63 - gr0_zeroes_bits[i]);
> +		asm volatile (
> +			"xgr	5,5\n"
> +			"lghi	5, 128\n"
> +			"lg	0, 0(%[val])\n"
> +			".insn	rre,0xb2ae0000,2,4\n"
> +			: : [val] "a" (&gr0)
> +			: "cc", "memory", "0", "1", "2", "3", "4", "5", "6", "7");
> +		if (clear_pgm_int() != PGM_INT_CODE_SPECIFICATION) {
> +			report_info("setting gr0 bit %d did not result in a spec exception",
> +				    gr0_zeroes_bits[i]);
> +			fail = true;
> +		}
> +	}
> +	report(!fail, "set bit pgms");
> +	report_prefix_pop();
> +
> +	report_prefix_pop();
> +	report_prefix_pop();
> +}
> +
> +static void test_priv(void)
> +{
> +	struct ap_config_info info = {};
> +
> +	report_prefix_push("privileged");
> +
> +	report_prefix_push("pqap");
> +	expect_pgm_int();
> +	enter_pstate();
> +	ap_pqap_qci(&info);
> +	check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION);
> +	report_prefix_pop();
> +
> +	/*
> +	 * Enqueue and dequeue take too many registers so a simple
> +	 * inline assembly makes more sense than using the library
> +	 * functions.
> +	 */
> +	report_prefix_push("nqap");
> +	expect_pgm_int();
> +	enter_pstate();
> +	asm volatile (
> +		".insn	rre,0xb2ad0000,0,2\n"
> +		: : : "cc", "memory", "0", "1", "2", "3");
> +	check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION);
> +	report_prefix_pop();
> +
> +	report_prefix_push("dqap");
> +	expect_pgm_int();
> +	enter_pstate();
> +	asm volatile (
> +		".insn	rre,0xb2ae0000,0,2\n"
> +		: : : "cc", "memory", "0", "1", "2", "3");
> +	check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION);
> +	report_prefix_pop();
> +
> +	report_prefix_pop();
> +}
> +
> +int main(void)
> +{
> +	report_prefix_push("ap");
> +	if (!ap_check()) {
> +		report_skip("AP instructions not available");
> +		goto done;
> +	}
> +
> +	test_priv();
> +	test_pgms_pqap();
> +	test_pgms_nqap();
> +	test_pgms_dqap();
> +
> +done:
> +	report_prefix_pop();
> +	return report_summary();
> +}
> diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
> index 018e4129..578375e4 100644
> --- a/s390x/unittests.cfg
> +++ b/s390x/unittests.cfg
> @@ -386,3 +386,6 @@ file = sie-dat.elf
>   
>   [pv-attest]
>   file = pv-attest.elf
> +
> +[ap]
> +file = ap.elf