Re: [kvm-unit-tests PATCH 2/3] s390x: sclp: Add detection of alternate STFLE facilities

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

On Fri Feb 20, 2026 at 11:20 AM CET, Janosch Frank wrote:
> On 2/11/26 15:57, Christoph Schlameuss wrote:
>> From: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
>> 
>> Detect availability of alternate STFLE interpretive execution facilities
>> 1 and 2.
>> Also fix number of unassigned bits in sclp_facilities which wasn't
>> adjusted in a prior commit adding entries to sclp_facilities.
>> 
>> Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
>> Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
>> ---
>>   lib/s390x/sclp.c | 2 ++
>>   lib/s390x/sclp.h | 6 +++++-
>>   2 files changed, 7 insertions(+), 1 deletion(-)
>> 
>> diff --git a/lib/s390x/sclp.c b/lib/s390x/sclp.c
>> index 2f902e39e785ff4e139a39be2ffe11b5fa01edc0..7408b813b6396d572d740c19c15175e173fed596 100644
>> --- a/lib/s390x/sclp.c
>> +++ b/lib/s390x/sclp.c
>> @@ -163,8 +163,10 @@ void sclp_facilities_setup(void)
>>   	sclp_facilities.has_cmma = sclp_feat_check(116, SCLP_FEAT_116_BIT_CMMA);
>>   	sclp_facilities.has_64bscao = sclp_feat_check(116, SCLP_FEAT_116_BIT_64BSCAO);
>>   	sclp_facilities.has_esca = sclp_feat_check(116, SCLP_FEAT_116_BIT_ESCA);
>> +	sclp_facilities.has_astfleie1 = sclp_feat_check(116, SCLP_FEAT_116_BIT_ASTFLEIE1);
>>   	sclp_facilities.has_ibs = sclp_feat_check(117, SCLP_FEAT_117_BIT_IBS);
>>   	sclp_facilities.has_pfmfi = sclp_feat_check(117, SCLP_FEAT_117_BIT_PFMFI);
>> +	sclp_facilities.has_astfleie2 = sclp_feat_check(139, SCLP_FEAT_139_BIT_ASTFLEIE2);
>>   
>>   	for (i = 0; i < read_info->entries_cpu; i++, cpu++) {
>>   		/*
>> diff --git a/lib/s390x/sclp.h b/lib/s390x/sclp.h
>> index 22f120d1b7ea7d1c3fe822385d0c689e5b3459fe..91a81c902eaa8ee6b999184aeb8a33633efd1065 100644
>> --- a/lib/s390x/sclp.h
>> +++ b/lib/s390x/sclp.h
>> @@ -129,10 +129,12 @@ struct sclp_facilities {
>>   	uint64_t has_cmma : 1;
>>   	uint64_t has_64bscao : 1;
>>   	uint64_t has_esca : 1;
>> +	uint64_t has_astfleie1 : 1;
>>   	uint64_t has_kss : 1;
>>   	uint64_t has_pfmfi : 1;
>>   	uint64_t has_ibs : 1;
>> -	uint64_t : 64 - 15;
>> +	uint64_t has_astfleie2 : 1;
>> +	uint64_t : 64 - 19;
>>   };
>
> 64 - 17?

Absolutely

> I was wondering why the static assert didn't trigger here. Turns out I 
> only added it to a feature branch that didn't go upstream yet...
>
> Could you do me a favor and add a patch that introduces the static assert:
> _Static_assert(sizeof(struct sclp_facilities) == sizeof(uint64_t));

Sure thing, I can add that to the next version. If we want to keep the padding.

> Then again, why do we pad that at all, it's not a FW struct and I hope 
> that we never cast it to u64 and copy it somewhere.

I would assume nobody ever questioned that since you introduced the struct with
6dff7c9a123c ("s390x: SCLP feature checking")

As the ordering of these bits is nothing like anything anywhere else I agree
that the manual padding is not necessary at all.
So I would rather opt to just remove the padding within its own commit and not
add the assertion.

>>   
>>   /* bit number within a certain byte */
>> @@ -143,8 +145,10 @@ struct sclp_facilities {
>>   #define SCLP_FEAT_116_BIT_64BSCAO	0
>>   #define SCLP_FEAT_116_BIT_CMMA		1
>>   #define SCLP_FEAT_116_BIT_ESCA		4
>> +#define SCLP_FEAT_116_BIT_ASTFLEIE1	7
>>   #define SCLP_FEAT_117_BIT_PFMFI		1
>>   #define SCLP_FEAT_117_BIT_IBS		2
>> +#define SCLP_FEAT_139_BIT_ASTFLEIE2	1
>>   
>>   typedef struct ReadInfo {
>>   	SCCBHeader h;
>>