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

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

On Tue Apr 21, 2026 at 2:52 PM CEST, Nico Boehr wrote:
> 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?

I did include my proposal in the v3 I sent earlier today.