Re: [kvm-unit-tests PATCH v2 4/5] s390x: sclp: Add detection of alternate STFLE facilities

["Nico Boehr" <nrb@linux.ibm.com>]

On Fri Apr 17, 2026 at 5:28 PM CEST, Christoph Schlameuss wrote:
> On Wed Apr 15, 2026 at 1:34 PM CEST, Nico Boehr wrote:
>> On Tue Mar 24, 2026 at 4:28 PM CET, Christoph Schlameuss wrote:
>> [...]
>>> diff --git a/lib/s390x/sclp.c b/lib/s390x/sclp.c
>>> index d624872cba608fcbbd0c482a25f091fe19475a43..77b2a5ec00d68ec7ee82da295f2f31c539b5c00c 100644
>>> --- a/lib/s390x/sclp.c
>>> +++ b/lib/s390x/sclp.c
>> [...]
>>>  	if (read_info->offset_cpu <= 134)
>>>  		return;
>>>  	sclp_facilities.has_diag318 = read_info->byte_134_diag318;
>>> +
>>> +	if (read_info->offset_cpu <= 139)
>>> +		return;
>>> +	sclp_facilities.has_astfleie2 = sclp_feat_check(139, SCLP_FEAT_139_BIT_ASTFLEIE2);
>>
>> Help me understand which case is the odd one, the diag318 one where we access
>> read_info directly or this one where we use sclp_feat_check()?
>>
>> Or is there a particular reason to do it this way that I didn't see?
>
> Both methods will read from the location read_info is pointing to. Which
> actually is pointing to _read_info, which is a 2 page buffer. The actual sclp
> info is read into _read_info dependent on facility 140 as either 1 or 2 pages.
>
> So the data will then in either case be there in _read_info, either from the 2
> page sclp read or as zeros behind the 1 page sclp read.
> As for byte_134_diag318 in struct ReadInfo I dont think there is a golden way.
> Without fac140 byte_134_diag318 does not exist but is in the struct. And with
> fac140 and using sclp_feat_check we are reaching behind struct ReadInfo into
> _read_info.
> Both not optimal.
>
> In the end the difference is if we go the extra mile and define all the feature
> bits in struct ReadInfo or just use sclp_feat_check to check the bit directly.
> Simply because I think clp_facilities_setup looks cleaner that way and it is
> easier to add more feature bits.
> I agree that we should use the same method for all high feature bits.
>
> So I propose to add another patch here to use sclp_feat_check directly for
> diag318 as well. And also remove byte_134_diag318 from struct ReadInfo.

Thank you, makes sense to me. Do you want to take this as part of this series or
should I send a patch you can base your series on?