From: "Christoph Schlameuss" <schlameuss@linux.ibm.com>
To: "Janosch Frank" <frankja@linux.ibm.com>,
"Christoph Schlameuss" <schlameuss@linux.ibm.com>,
<linux-s390@vger.kernel.org>
Cc: "Claudio Imbrenda" <imbrenda@linux.ibm.com>,
"Nico Böhr" <nrb@linux.ibm.com>,
"David Hildenbrand" <david@kernel.org>,
"Thomas Huth" <thuth@redhat.com>,
kvm@vger.kernel.org
Subject: Re: [kvm-unit-tests PATCH v3 3/6] s390x: sclp: Rework sclp_facilities_setup() for simpler control flow
Date: Wed, 22 Apr 2026 15:20:23 +0200 [thread overview]
Message-ID: <DHZPQ7DONRP5.1IL4NP46DFHCD@linux.ibm.com> (raw)
In-Reply-To: <f82ad19a-5576-4a10-82f4-1c1577af2fef@linux.ibm.com>
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.
next prev parent reply other threads:[~2026-04-22 13:20 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-21 11:31 [kvm-unit-tests PATCH v3 0/6] s390x: Add test for STFLE interpretive execution (format-2) Christoph Schlameuss
2026-04-21 11:31 ` [kvm-unit-tests PATCH v3 1/6] s390x: snippets: Add reset_guest() to lib Christoph Schlameuss
2026-04-21 11:31 ` [kvm-unit-tests PATCH v3 2/6] s390x: sclp: Remove unnecessary padding from struct sclp_facilities Christoph Schlameuss
2026-04-21 11:31 ` [kvm-unit-tests PATCH v3 3/6] s390x: sclp: Rework sclp_facilities_setup() for simpler control flow Christoph Schlameuss
2026-04-22 11:05 ` Janosch Frank
2026-04-22 13:20 ` Christoph Schlameuss [this message]
2026-04-21 11:31 ` [kvm-unit-tests PATCH v3 4/6] s390x: sclp: Use sclp_feat_check directly to read DIAG318 feature bit Christoph Schlameuss
2026-04-21 11:31 ` [kvm-unit-tests PATCH v3 5/6] s390x: sclp: Add detection of alternate STFLE facilities Christoph Schlameuss
2026-04-22 12:38 ` Janosch Frank
2026-04-21 11:31 ` [kvm-unit-tests PATCH v3 6/6] s390x: Add test for STFLE interpretive execution (format-2) Christoph Schlameuss
2026-04-22 12:39 ` Janosch Frank
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DHZPQ7DONRP5.1IL4NP46DFHCD@linux.ibm.com \
--to=schlameuss@linux.ibm.com \
--cc=david@kernel.org \
--cc=frankja@linux.ibm.com \
--cc=imbrenda@linux.ibm.com \
--cc=kvm@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=nrb@linux.ibm.com \
--cc=thuth@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox