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

[Janosch Frank <frankja@linux.ibm.com>]

On 3/30/23 18:34, Claudio Imbrenda wrote:
> On Thu, 30 Mar 2023 11:42:41 +0000
> Janosch Frank <frankja@linux.ibm.com> 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>
>> ---
> 
> [...]
> 
>> +
>> +static void test_pgms_pqap(void)
>> +{
>> +	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;
> 
> maybe make a macro out of it, both to avoid magic numbers and to change
> it easily if code 42 will ever become defined in the future.

I don't really see a benefit to that.

> 
>> +	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");
>> +	for (i = 42; i < 6; i++) {
> 
> 42 is not < 6, this whole thing will be skipped?

Right, I've fixed this.

[...]
>> +
>> +static void test_pgms_nqap(void)
>> +{
>> +	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");
> 
> I would say
> "0", "1", "2", "3", "4", "6", "7"
> 
> since there are two ways of doing it wrong when it comes to even-odd
> register pairs (r and r+1, r&~1 and r&~1+1)

R1 & R1 + 1 should never change, same goes for R2.
GR0, GR1, R2 + 1 could potentially change.

But the more interesting question is: Does it make sense to clobber 
anything other than cc (if at all) for the PGM checks? If the PGM fails 
we're in uncharted territory. Seems like I need to look up what the 
other tests do.