Re: [kvm-unit-tests PATCH v3 3/6] s390x: sclp: Rework sclp_facilities_setup() for simpler control flow

Christoph Schlameuss
2026-04-22T13:20:38Z

On Wed Apr 22, 2026 at 1:05 PM CEST, Janosch Frank wrote:
> On 4/21/26 13:31, Christoph Schlameuss wrote:
>> Reverse the order of operations to allow for early exits with increasing
>> cpu offsets when reading facility bits in the extended range.
>>
>> This is done to ease future additions like the one in the next patch in
>> this series.
>>
>> Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
>> Reviewed-by: Nico Boehr <nrb@linux.ibm.com>
>> ---
>
> I have a feeling that we start coding ourselves into a corner here.
> Wouldn't something like this be enough to fix the offset checks? With
> that we don't need checks in sclp_facilities_setup() anymore.
>
> diff --git i/lib/s390x/sclp.c w/lib/s390x/sclp.c
> index 2f902e39..1ffcf448 100644
> --- i/lib/s390x/sclp.c
> +++ w/lib/s390x/sclp.c
> @@ -142,6 +142,9 @@ static bool sclp_feat_check(int byte, int bit)
> {
> uint8_t *rib = (uint8_t *)read_info;
>
> + if (read_info->offset_cpu <= byte)
> + return false;
> +
> return !!(rib[byte] & (0x80 >> bit));
> }

That is a good idea. Makes sclp_facilities_setup() nicer and less likely to mess
up if someone wants to add code in the end of that in the future.
Since the number of feature bits here is so limited anyway this should also
not have any real performance impact.

I will revert the reodering and early returns and add this instead.