Re: [kvm-unit-tests PATCH v2 5/5] s390x: Add test for STFLE interpretive execution (format-2)

["Christoph Schlameuss" <schlameuss@linux.ibm.com>]

On Wed Mar 25, 2026 at 11:18 AM CET, Janosch Frank wrote:
> On 3/24/26 16:28, Christoph Schlameuss wrote:
>> From: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
>> 
>> The STFLE instruction indicates installed facilities.
>> SIE has facilities for the interpretive execution of STFLE.
>> There are multiple possible formats for the control block.
>> Use a snippet guest executing STFLE to get the result of
>> interpretive execution and check the result.
>> With the addition of the format-2 control block invalid format
>> specifiers are now possible.
>> Test for the occurrence of optional validity intercepts.
>> 
>> Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
>> Co-developed-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
>> Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
>> ---
>>   lib/s390x/sie.c   | 11 +++++++
>>   lib/s390x/sie.h   |  1 +
>>   s390x/stfle-sie.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++-------
>>   3 files changed, 92 insertions(+), 11 deletions(-)
>> 
>> diff --git a/lib/s390x/sie.c b/lib/s390x/sie.c
>> index 0fa915cf028a1b35a76aa316dfd97308094a4682..17f0ef7eff427002dd6d74d98f58ed493457a7ce 100644
>> --- a/lib/s390x/sie.c
>> +++ b/lib/s390x/sie.c

[...]

>> @@ -55,18 +56,73 @@ static struct guest_stfle_res run_guest(void)
>>   static void test_stfle_format_0(void)
>>   {
>>   	struct guest_stfle_res res;
>> +	int format_mask;
>>   
>>   	report_prefix_push("format-0");
>> -	for (int j = 0; j < stfle_size(); j++)
>> -		WRITE_ONCE((*fac)[j], prng64(&prng_s));
>> -	vm.sblk->fac = (uint32_t)(uint64_t)fac;
>> -	res = run_guest();
>> -	report(res.len == stfle_size(), "stfle len correct");
>> -	report(!memcmp(*fac, res.mem, res.len * sizeof(uint64_t)),
>> -	       "Guest facility list as specified");
>> +	/*
>> +	 * There are multiple valid two bit format control values depending on
>> +	 * the available facilities.
>> +	 * The facility introduced last defines the validity of control bits.
>> +	 */
>> +	format_mask = sclp_facilities.has_astfleie2 ? 3 : sclp_facilities.has_astfleie1;
>
> Without the KVM patches format_mask is 0.
>
>> +	for (int i = 0; i < 4; i++) {
>
> Why?
> This test is only for format 0, no?
>
>> +		if (i & format_mask)
>> +			continue;
>
> i & 0 is always false.
>
>> +		report_prefix_pushf("format control %d", i);
>> +		for (int j = 0; j < stfle_size(); j++)
>> +			WRITE_ONCE((*fac)[j], prng64(&prng_s));
>> +		vm.sblk->fac = (uint32_t)(uint64_t)fac | i;
>
> Since my mask is 0 and i can be 0 - 3 where values >0 can lead to 
> validities (optional) this test can run into a validity at any point.
>
>> +		res = run_guest();
>> +		report(res.len == stfle_size(), "stfle len correct");
>> +		report(!memcmp(*fac, res.mem, res.len * sizeof(uint64_t)),
>> +		       "Guest facility list as specified");
>> +		report_prefix_pop();
>> +	}
>>   	report_prefix_pop();
>>   }

[...]

>> +
>> +static void test_no_stfle_format(int format)
>> +{
>> +	reset_guest(&vm);
>> +	vm.sblk->fac = (uint32_t)(uint64_t)fac | format;
>> +	sie_expect_validity(&vm);
>> +	sie(&vm);
>> +	sie_check_optional_validity(&vm, 0x1330);
>> +}
>
> This needs a prefix, right now I see three of these skip reports and I 
> don't know which format was tested:
> PASS: optional VALIDITY: no
>
> But looking at the code above I wonder if that should be folded into the 
> test above if the logic is fixed.
>

Yes, I added some prefixes and reverted the additional loop from
test_stfle_format_0() for v3. That makes the log nice and readable now.


>> +
>>   struct args {
>>   	uint64_t seed;
>>   };
>> @@ -119,20 +175,33 @@ static struct args parse_args(int argc, char **argv)
>>   int main(int argc, char **argv)
>>   {
>>   	struct args args = parse_args(argc, argv);
>> -	bool run_format_0 = test_facility(7);
>>   
>>   	if (!sclp_facilities.has_sief2) {
>>   		report_skip("SIEF2 facility unavailable");
>>   		goto out;
>>   	}
>> -	if (!run_format_0)
>> +	if (!test_facility(7)) {
>>   		report_skip("STFLE facility not available");
>> +		goto out;
>> +	}
>>   
>>   	report_info("PRNG seed: 0x%lx", args.seed);
>>   	prng_s = prng_init(args.seed);
>>   	setup_guest();
>> -	if (run_format_0)
>> -		test_stfle_format_0();
>> +	test_stfle_format_0();
>> +
>> +	if (!sclp_facilities.has_astfleie1)
>> +		test_no_stfle_format(1);
>> +
>> +	if (!sclp_facilities.has_astfleie2) {
>> +		test_no_stfle_format(2);
>> +		report_skip("alternate STFLE interpretive-execution facility 2 not available");
>> +	} else {
>> +		test_stfle_format_2();
>> +	}
>> +
>
> Does this test work on LPAR and zVM?
>

Yes, it does. I did test explicitly on zVM now as well.

>> +	test_no_stfle_format(3);
>> +
>>   out:
>>   	return report_summary();
>>   }
>>