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

Christoph Schlameuss
2026-04-17T15:28:17Z

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.

Cheers,

Christoph