[PATCH] target/s390x: query-cpu-definitions filter deprecated features

[PATCH] target/s390x: query-cpu-definitions filter deprecated features

[Collin Walling]

Part of the query-cpu-definitions return payload includes an array of
of "unavailable-features" which helps determine if the CPU model is
compatible with the system's configuration.  When a KVM guest is
using a CPU model with deprecated features disabled,
query-cpu-definitions will claim that certain models are incompatible
due to these features missing.

Since deprecated features are preferred to be disabled for s390 CPU
models, lets filter them out from the "missing" features list.

Signed-off-by: Collin Walling <walling@linux.ibm.com>
---

Examples output of query-cpu-definitions ran on a gen17a host:

WithOUT patch:

```
{
  "return": [
    {
      "name": "z13",
      "typename": "z13-s390x-cpu",
      "unavailable-features": [
        "csske"
      ],
      "static": false,
      "migration-safe": true,
      "deprecated": false
    },
    ...
```

WITH patch:

```
{
  "return": [
    {
      "name": "z13",
      "typename": "z13-s390x-cpu",
      "unavailable-features": [],
      "static": false,
      "migration-safe": true,
      "deprecated": false
    },
    ...
```

---

 target/s390x/cpu_models_system.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/target/s390x/cpu_models_system.c b/target/s390x/cpu_models_system.c
index 5b846048675..f2f6b00b547 100644
--- a/target/s390x/cpu_models_system.c
+++ b/target/s390x/cpu_models_system.c
@@ -28,6 +28,7 @@ static void check_unavailable_features(const S390CPUModel *max_model,
                                        strList **unavailable)
 {
     S390FeatBitmap missing;
+    S390FeatBitmap deprecated_feats;
 
     /* check general model compatibility */
     if (max_model->def->gen < model->def->gen ||
@@ -39,6 +40,13 @@ static void check_unavailable_features(const S390CPUModel *max_model,
     /* detect missing features if any to properly report them */
     bitmap_andnot(missing, model->features, max_model->features,
                   S390_FEAT_MAX);
+
+    /* deprecated features are optional so do not report them as missing */
+    bitmap_zero(deprecated_feats, S390_FEAT_MAX);
+    s390_get_deprecated_features(deprecated_feats);
+
+    bitmap_andnot(missing, missing, deprecated_feats, S390_FEAT_MAX);
+
     if (!bitmap_empty(missing, S390_FEAT_MAX)) {
         s390_feat_bitmap_to_ascii(missing, unavailable, list_add_feat);
     }
-- 
2.54.0

Re: [PATCH] target/s390x: query-cpu-definitions filter deprecated features

[Markus Armbruster]

Collin Walling <walling@linux.ibm.com> writes:

> Part of the query-cpu-definitions return payload includes an array of
> of "unavailable-features" which helps determine if the CPU model is
> compatible with the system's configuration.  When a KVM guest is
> using a CPU model with deprecated features disabled,
> query-cpu-definitions will claim that certain models are incompatible
> due to these features missing.
>
> Since deprecated features are preferred to be disabled for s390 CPU
> models, lets filter them out from the "missing" features list.
>
> Signed-off-by: Collin Walling <walling@linux.ibm.com>
> ---
>
> Examples output of query-cpu-definitions ran on a gen17a host:
>
> WithOUT patch:
>
> ```
> {
>   "return": [
>     {
>       "name": "z13",
>       "typename": "z13-s390x-cpu",
>       "unavailable-features": [
>         "csske"
>       ],
>       "static": false,
>       "migration-safe": true,
>       "deprecated": false
>     },
>     ...
> ```
>
> WITH patch:
>
> ```
> {
>   "return": [
>     {
>       "name": "z13",
>       "typename": "z13-s390x-cpu",
>       "unavailable-features": [],
>       "static": false,
>       "migration-safe": true,
>       "deprecated": false
>     },
>     ...
> ```

I'd put this right into the commit message.

Possibly stupid question (I know nothing about the subject matter): any
compatibility ramifications?  Does the change need to be tied to machine
type version?

Re: [PATCH] target/s390x: query-cpu-definitions filter deprecated features

[Collin Walling]

On 7/9/26 01:13, Markus Armbruster wrote:
> Collin Walling <walling@linux.ibm.com> writes:
> 
>> Part of the query-cpu-definitions return payload includes an array of
>> of "unavailable-features" which helps determine if the CPU model is
>> compatible with the system's configuration.  When a KVM guest is
>> using a CPU model with deprecated features disabled,
>> query-cpu-definitions will claim that certain models are incompatible
>> due to these features missing.
>>
>> Since deprecated features are preferred to be disabled for s390 CPU
>> models, lets filter them out from the "missing" features list.
>>
>> Signed-off-by: Collin Walling <walling@linux.ibm.com>
>> ---
>>
>> Examples output of query-cpu-definitions ran on a gen17a host:
>>
>> WithOUT patch:
>>
>> ```
>> {
>>   "return": [
>>     {
>>       "name": "z13",
>>       "typename": "z13-s390x-cpu",
>>       "unavailable-features": [
>>         "csske"
>>       ],
>>       "static": false,
>>       "migration-safe": true,
>>       "deprecated": false
>>     },
>>     ...
>> ```
>>
>> WITH patch:
>>
>> ```
>> {
>>   "return": [
>>     {
>>       "name": "z13",
>>       "typename": "z13-s390x-cpu",
>>       "unavailable-features": [],
>>       "static": false,
>>       "migration-safe": true,
>>       "deprecated": false
>>     },
>>     ...
>> ```
> 
> I'd put this right into the commit message.
> 
> Possibly stupid question (I know nothing about the subject matter): any
> compatibility ramifications?  Does the change need to be tied to machine
> type version?
> 
> 

This is a valid question, and one we raised internally when performing
some regression tests (specifically about differing QEMU and libvirt
versions).  Functionally, there should be no compatibility ramifications.

However, you got me to investigating some other behaviors, specifically
regarding how libvirt utilizes this command.

This patch introduces a possible discrepancy between libvirt's domain
capabilities report and what the user may expect when defining a domain
with a CPU model within that report.

Here's a rundown of how libvirt utilizes the query-cpu-definition data
command WITHOUT the patch:

Let's say I'm running on a z17 machine.  On this hardware generation,
some CPU features have been dropped completely (e.g. csske). Now, being
on a newer generation machine, I would assume that I could start a guest
with an older model (e.g. z14). However, running virsh domcapabilities,
libvirt tells me otherwise:

      <model usable='no' vendor='IBM'>z14</model>
      <blockers model='z14'>
        <feature name='csske'/>
      </blockers>

But this is not entirely true.  I could create a domain with a z14 CPU
model and either explicitly disable csske, or append the
deprecated_features='off' attribute to the <cpu> element and the domain
will start successfully.

Now, let's run through this scenario WITH the patch:

Running virsh domcapabilities, I see:

      <model usable='yes' vendor='IBM'>z14</model>

But this is _also_ not entirely true: a user cannot simply define a
domain using a z14 CPU model without further adjustments.  This invokes
the following error:

      qemu-system-s390x: Some features requested in the CPU model are
      not available in the current configuration: csske
      Consider a different accelerator, QEMU, or kernel version

Again, the user can either explicitly disable this feature or use the
deprecated_features attribute and everything is fine, but that isn't
immediately clear.

I am now wondering if this filtering should be handled via libvirt
instead of QEMU?  Right now, it is possible to run virsh domcapabilities
and provide a --disable-deprecated-features flag, which currently only
presents the host-model with deprecated features disabled.  I wonder if
this functionality should be extended to the custom models in this
output as well?  This could be done by simply checking the blocking
features against the deprecated features and filtering them out when the
command flag is provided.

I've added Christian, Hendrik, and Boris to CC to gather their input.

Thank you for posing your question.

-- 
Regards,
  Collin