Re: [PATCH 08/17] s390x/cpumodel: Fix UI to CPU features pcc-cmac-{aes, eaes}-256
David Hildenbrand writes: > [...] > >> 1 "msa4-base": true, >> 1 "pcc-cmac-aes-256": false, >> 1 "pcc-cmac-eaes-256": false, >> >> The grouping and masking you described appears to apply to >> query-cpu-model-expansion with type "static". With type "full", I can >> see the grouping, but not the masking. With query-cpu-definitions, I >> can't see either. >> >> I haven't played with query-cpu-model-comparison and >> query-cpu-model-baseline. > > Grouping will be done whenever all features part of a group are to be > reported. Please note that "msa4-base" is part of "msa4". > > You chose the only model where we do have msa4-base but none of the > other msa4 features - the qemu model. Ah, "lucky" random pick. > So when you do a "query-cpu-definitions" under TCG (which basically maps > to the qemu model and only has the msa4-base feature), then you do have > msa4-base of all relevant models, but none of the other sub-features > they define. That's why you don't see msa4 explicitly, but instead the > subfeatures. > > query-cpu-model-expansion and the others behave similar the same way > with the "qemu" model. Note that the qemu model is not really used under > KVM on s390x. There, we use "host" as default. Next random pick: z13.2 shows msa4 before and after. But "'-cpu' setup" doesn't seem to be about reporting features. Am I confused? >>> >>> Let me clarify. Any user input would be broken if the two sub-features >>> would be specified explicitly, instead of the whole "msa4" group. This >>> applies to any user input, also the user input for query-cpu-model-. >>> >>> In the usual cases, libvirt will expand a cpu model (e.g., "host", >>> "z15") and start QEMU with that (-cpu ...). We will only have the >>> complete msa4 group here in practice. >>> >>> Yes, if some user would pick and chose such features manually, it would >>> be broken - it's just not the common on s390x with the huge amount of >>> features. But that's why I thing stable backports still make sense. >> >> The commit message should be accurate and sufficiently precise. The >> "sufficiently" gives me some wiggle room to avoid inaccuracy due to my >> ignorance. Would the following be good enough? >> >> Impact: >> >> * s390_feat_bitmap_to_ascii() misidentifies S390_FEAT_PCC_CMAC_AES_256 >> as "pcc-cmac-eaes-256". Affects QMP commands query-cpu-definitions, >> query-cpu-model-expansion, query-cpu-model-baseline, >> query-cpu-model-comparison, and the error message when >> s390_realize_cpu_model() fails in check_compatibility(). >> >> * s390_cpu_list() also misidentifies it. Affects -cpu help. >> >> * s390_cpu_model_register_props() creates CPU property >> "pcc-cmac-eaes-256" twice. The second one fails, but the error is >> ignored (a later commit will change that). Results in a single >> property "pcc-cmac-eaes-256" with the description for >> S390_FEAT_PCC_CMAC_AES_256, and no property for >> S390_FEAT_PCC_CMAC_EAES_256. CPU properties are visible in CLI -cpu >> and -device, QMP & HMP device_add, QMP device-list-properties, and >> QOM introspection. >> >> The two features are almost always used via their group msa4. Such >> use is not affected by this bug. > > Yeah, sounds good, thanks. Excellent.
Re: [PATCH 08/17] s390x/cpumodel: Fix UI to CPU features pcc-cmac-{aes,eaes}-256
[...] > 1 "msa4-base": true, > 1 "pcc-cmac-aes-256": false, > 1 "pcc-cmac-eaes-256": false, > > The grouping and masking you described appears to apply to > query-cpu-model-expansion with type "static". With type "full", I can > see the grouping, but not the masking. With query-cpu-definitions, I > can't see either. > > I haven't played with query-cpu-model-comparison and > query-cpu-model-baseline. Grouping will be done whenever all features part of a group are to be reported. Please note that "msa4-base" is part of "msa4". You chose the only model where we do have msa4-base but none of the other msa4 features - the qemu model. So when you do a "query-cpu-definitions" under TCG (which basically maps to the qemu model and only has the msa4-base feature), then you do have msa4-base of all relevant models, but none of the other sub-features they define. That's why you don't see msa4 explicitly, but instead the subfeatures. query-cpu-model-expansion and the others behave similar the same way with the "qemu" model. Note that the qemu model is not really used under KVM on s390x. There, we use "host" as default. > >>> But "'-cpu' setup" doesn't seem to be about reporting features. Am I >>> confused? >>> >> >> Let me clarify. Any user input would be broken if the two sub-features >> would be specified explicitly, instead of the whole "msa4" group. This >> applies to any user input, also the user input for query-cpu-model-. >> >> In the usual cases, libvirt will expand a cpu model (e.g., "host", >> "z15") and start QEMU with that (-cpu ...). We will only have the >> complete msa4 group here in practice. >> >> Yes, if some user would pick and chose such features manually, it would >> be broken - it's just not the common on s390x with the huge amount of >> features. But that's why I thing stable backports still make sense. > > The commit message should be accurate and sufficiently precise. The > "sufficiently" gives me some wiggle room to avoid inaccuracy due to my > ignorance. Would the following be good enough? > > Impact: > > * s390_feat_bitmap_to_ascii() misidentifies S390_FEAT_PCC_CMAC_AES_256 > as "pcc-cmac-eaes-256". Affects QMP commands query-cpu-definitions, > query-cpu-model-expansion, query-cpu-model-baseline, > query-cpu-model-comparison, and the error message when > s390_realize_cpu_model() fails in check_compatibility(). > > * s390_cpu_list() also misidentifies it. Affects -cpu help. > > * s390_cpu_model_register_props() creates CPU property > "pcc-cmac-eaes-256" twice. The second one fails, but the error is > ignored (a later commit will change that). Results in a single > property "pcc-cmac-eaes-256" with the description for > S390_FEAT_PCC_CMAC_AES_256, and no property for > S390_FEAT_PCC_CMAC_EAES_256. CPU properties are visible in CLI -cpu > and -device, QMP & HMP device_add, QMP device-list-properties, and > QOM introspection. > > The two features are almost always used via their group msa4. Such > use is not affected by this bug. Yeah, sounds good, thanks. -- Thanks, David / dhildenb
Re: [PATCH 08/17] s390x/cpumodel: Fix UI to CPU features pcc-cmac-{aes, eaes}-256
David Hildenbrand writes:
> On 02.05.20 08:26, Markus Armbruster wrote:
>> David Hildenbrand writes:
>>
>>> On 30.04.20 20:22, Markus Armbruster wrote:
David Hildenbrand writes:
> On 28.04.20 18:34, Markus Armbruster wrote:
>> Both s390_features[S390_FEAT_PCC_CMAC_AES_256].name and
>> s390_features[S390_FEAT_PCC_CMAC_EAES_256].name is
>> "pcc-cmac-eaes-256". The former is obviously a pasto.
>>
>> Impact:
>>
>> * s390_feat_bitmap_to_ascii() misidentifies S390_FEAT_PCC_CMAC_AES_256
>> as "pcc-cmac-eaes-256". Affects QMP commands query-cpu-definitions,
>> query-cpu-model-expansion, query-cpu-model-baseline,
>> query-cpu-model-comparison, and the error message when
>> s390_realize_cpu_model() fails in check_compatibility().
>>
>> * s390_realize_cpu_model() misidentifies it in check_consistency()
>> warnings.
Actually not, because the feature isn't in check_consistency()'s dep[].
I'll drop this item.
>> * s390_cpu_list() likewise. Affects -cpu help.
>>
>> * s390_cpu_model_register_props() creates CPU property
>> "pcc-cmac-eaes-256" twice. The second one fails, but the error is
>> ignored (a later commit will change that). Results in a single
>> property "pcc-cmac-eaes-256" with the description for
>> S390_FEAT_PCC_CMAC_AES_256, and no property for
>> S390_FEAT_PCC_CMAC_EAES_256. CPU properties are visible in CLI -cpu
>> and -device, QMP & HMP device_add, QMP device-list-properties, and
>> QOM introspection.
>>
>> Fix by deleting the wayward 'e'.
>
> Very nice catch - thanks!
:)
> While this sounds very bad, it's luckily not that bad in practice
> (currently).
>
> The feature (or rather, both features) is part of the feature group
> "msa4". As long as we have all sub-features part of that group (which is
> usually the case), we will always indicate "msa4" to the user, instead
> of all the separate sub-features. So, expansion, baseline, comparison
> will usually only work with "msa4".
>
> (in addition, current KVM is not capable of actually masking off these
> sub-features, so it will still, always see the feature, even if not
> explicitly specified via "-cpu X,pcc-cmac-aes-256=on)
Would you like to propose an commit message improvements?
>>>
>>> Maybe something like
>>>
>>> "Both affected features are part of the feature group msa4. In current
>>> setups, we will always see the msa4 feature instead of the separate
>>> contained sub-features (because all sub-features are around). Therefore,
>>> both features are currently never passed from/to the user explicitly
>>> (e.g., via cpu model expansion, comparison, baseline and '-cpu' setup)."
>>>
>>> Thanks!
>>
>> I think I can guess how this could work for reporting features (I
>> haven't checked my guess against the code), which is what the
>> query-cpu-model-* do: suppress individual features when their group is
>> complete.
>
> Yes. Expand the group to single features on user input, expand the
> single features to the group on user output (if all features are enabled).
Okay, let's play.
Here's query-cpu-definitions:
$ echo -e '{"execute": "qmp_capabilities"}\n{"execute":
"query-cpu-definitions"}\n{"execute": "quit"}' |
../qemu/bld/s390x-softmmu/qemu-system-s390x -qmp-pretty stdio -S -display none
| egrep 'msa4|pcc-cmac-e?aes-256'
Before this patch:
28 "pcc-cmac-eaes-256",
After:
14 "pcc-cmac-aes-256",
14 "pcc-cmac-eaes-256",
No msa4.
Here's query-cpu-model-expansion for model "qemu" (arbitrarily chosen),
type "static":
$ echo -e '{"execute": "qmp_capabilities"}\n{"execute":
"query-cpu-model-expansion", "arguments": {"type": "full", "model": {"name":
"qemu"}}}\n{"execute": "quit"}' | $i -qmp-pretty stdio -S -display none | egrep
'msa4|pcc-cmac-e?aes-256' | sort | uniq -c
Before and after:
1 "msa4-base": true,
Same with type "full":
$ echo -e '{"execute": "qmp_capabilities"}\n{"execute":
"query-cpu-model-expansion", "arguments": {"type": "full", "model": {"name":
"qemu"}}}\n{"execute": "quit"}' | $i -qmp-pretty stdio -S -display none | egrep
'msa4|pcc-cmac-e?aes-256' | sort | uniq -c
Before:
1 "msa4-base": true,
1 "pcc-cmac-eaes-256": false,
After
1 "msa4-base": true,
1 "pcc-cmac-aes-256": false,
1 "pcc-cmac-eaes-256": false,
The grouping and masking you described appears to apply to
query-cpu-model-expansion with type "static". With type "full", I can
see the grouping, but not the masking. With query-cpu-definitions, I
can't see either.
I haven't played with query-cpu-model-comparison and
query-cpu-model-baseline.
>> But "'-cpu' setup" doesn't seem to be about
Re: [PATCH 08/17] s390x/cpumodel: Fix UI to CPU features pcc-cmac-{aes,eaes}-256
On 02.05.20 07:15, Markus Armbruster wrote: > Christian Borntraeger writes: > >> On 29.04.20 10:54, Christian Borntraeger wrote: >>> >>> >>> On 28.04.20 19:13, David Hildenbrand wrote: On 28.04.20 18:34, Markus Armbruster wrote: > Both s390_features[S390_FEAT_PCC_CMAC_AES_256].name and > s390_features[S390_FEAT_PCC_CMAC_EAES_256].name is > "pcc-cmac-eaes-256". The former is obviously a pasto. > > Impact: > > * s390_feat_bitmap_to_ascii() misidentifies S390_FEAT_PCC_CMAC_AES_256 > as "pcc-cmac-eaes-256". Affects QMP commands query-cpu-definitions, > query-cpu-model-expansion, query-cpu-model-baseline, > query-cpu-model-comparison, and the error message when > s390_realize_cpu_model() fails in check_compatibility(). > > * s390_realize_cpu_model() misidentifies it in check_consistency() > warnings. > > * s390_cpu_list() likewise. Affects -cpu help. > > * s390_cpu_model_register_props() creates CPU property > "pcc-cmac-eaes-256" twice. The second one fails, but the error is > ignored (a later commit will change that). Results in a single > property "pcc-cmac-eaes-256" with the description for > S390_FEAT_PCC_CMAC_AES_256, and no property for > S390_FEAT_PCC_CMAC_EAES_256. CPU properties are visible in CLI -cpu > and -device, QMP & HMP device_add, QMP device-list-properties, and > QOM introspection. > > Fix by deleting the wayward 'e'. Very nice catch - thanks! While this sounds very bad, it's luckily not that bad in practice (currently). The feature (or rather, both features) is part of the feature group "msa4". As long as we have all sub-features part of that group (which is usually the case), we will always indicate "msa4" to the user, instead of all the separate sub-features. So, expansion, baseline, comparison will usually only work with "msa4". (in addition, current KVM is not capable of actually masking off these sub-features, so it will still, always see the feature, even if not explicitly specified via "-cpu X,pcc-cmac-aes-256=on) I think we should do stable backports. >>> >>> makes sense, but I would like to do some testing upfront (old QEMU <-> new >>> QEMU >> >> So migration does work between a qemu with and without the patch for >> host-model and >> custom model=z14. > > Is this a Tested-by? Yes. As David pointed out when a user really starts to pick manual things in MSA4 then we can have a non-migrateable guests. But this is still the right thing I guess.
Re: [PATCH 08/17] s390x/cpumodel: Fix UI to CPU features pcc-cmac-{aes,eaes}-256
On 02.05.20 08:26, Markus Armbruster wrote: > David Hildenbrand writes: > >> On 30.04.20 20:22, Markus Armbruster wrote: >>> David Hildenbrand writes: >>> On 28.04.20 18:34, Markus Armbruster wrote: > Both s390_features[S390_FEAT_PCC_CMAC_AES_256].name and > s390_features[S390_FEAT_PCC_CMAC_EAES_256].name is > "pcc-cmac-eaes-256". The former is obviously a pasto. > > Impact: > > * s390_feat_bitmap_to_ascii() misidentifies S390_FEAT_PCC_CMAC_AES_256 > as "pcc-cmac-eaes-256". Affects QMP commands query-cpu-definitions, > query-cpu-model-expansion, query-cpu-model-baseline, > query-cpu-model-comparison, and the error message when > s390_realize_cpu_model() fails in check_compatibility(). > > * s390_realize_cpu_model() misidentifies it in check_consistency() > warnings. > > * s390_cpu_list() likewise. Affects -cpu help. > > * s390_cpu_model_register_props() creates CPU property > "pcc-cmac-eaes-256" twice. The second one fails, but the error is > ignored (a later commit will change that). Results in a single > property "pcc-cmac-eaes-256" with the description for > S390_FEAT_PCC_CMAC_AES_256, and no property for > S390_FEAT_PCC_CMAC_EAES_256. CPU properties are visible in CLI -cpu > and -device, QMP & HMP device_add, QMP device-list-properties, and > QOM introspection. > > Fix by deleting the wayward 'e'. Very nice catch - thanks! >>> >>> :) >>> While this sounds very bad, it's luckily not that bad in practice (currently). The feature (or rather, both features) is part of the feature group "msa4". As long as we have all sub-features part of that group (which is usually the case), we will always indicate "msa4" to the user, instead of all the separate sub-features. So, expansion, baseline, comparison will usually only work with "msa4". (in addition, current KVM is not capable of actually masking off these sub-features, so it will still, always see the feature, even if not explicitly specified via "-cpu X,pcc-cmac-aes-256=on) >>> >>> Would you like to propose an commit message improvements? >> >> Maybe something like >> >> "Both affected features are part of the feature group msa4. In current >> setups, we will always see the msa4 feature instead of the separate >> contained sub-features (because all sub-features are around). Therefore, >> both features are currently never passed from/to the user explicitly >> (e.g., via cpu model expansion, comparison, baseline and '-cpu' setup)." >> >> Thanks! > > I think I can guess how this could work for reporting features (I > haven't checked my guess against the code), which is what the > query-cpu-model-* do: suppress individual features when their group is > complete. Yes. Expand the group to single features on user input, expand the single features to the group on user output (if all features are enabled). > > But "'-cpu' setup" doesn't seem to be about reporting features. Am I > confused? > Let me clarify. Any user input would be broken if the two sub-features would be specified explicitly, instead of the whole "msa4" group. This applies to any user input, also the user input for query-cpu-model-. In the usual cases, libvirt will expand a cpu model (e.g., "host", "z15") and start QEMU with that (-cpu ...). We will only have the complete msa4 group here in practice. Yes, if some user would pick and chose such features manually, it would be broken - it's just not the common on s390x with the huge amount of features. But that's why I thing stable backports still make sense. > While testing, I noticed that > > $ s390x-softmmu/qemu-system-s390x > > flashes a window at me, then terminates successfully, without printing > anything. With -S, it behaves like other targets. Bug? > Think this is expected. t480s: ~ $ qemu-system-s390x --nographic LOADPARM=[] Could not find a suitable boot device (none specified) The s390-ccw bios will come up, detect that there is nothing to boot and quit. The bios can only print to the sclp console, not to a graphical output. What the others do (e.g., ppc64, x86_64) is boot the bios/firmware and then halt there. -- Thanks, David / dhildenb
Re: [PATCH 08/17] s390x/cpumodel: Fix UI to CPU features pcc-cmac-{aes, eaes}-256
David Hildenbrand writes: > On 30.04.20 20:22, Markus Armbruster wrote: >> David Hildenbrand writes: >> >>> On 28.04.20 18:34, Markus Armbruster wrote: Both s390_features[S390_FEAT_PCC_CMAC_AES_256].name and s390_features[S390_FEAT_PCC_CMAC_EAES_256].name is "pcc-cmac-eaes-256". The former is obviously a pasto. Impact: * s390_feat_bitmap_to_ascii() misidentifies S390_FEAT_PCC_CMAC_AES_256 as "pcc-cmac-eaes-256". Affects QMP commands query-cpu-definitions, query-cpu-model-expansion, query-cpu-model-baseline, query-cpu-model-comparison, and the error message when s390_realize_cpu_model() fails in check_compatibility(). * s390_realize_cpu_model() misidentifies it in check_consistency() warnings. * s390_cpu_list() likewise. Affects -cpu help. * s390_cpu_model_register_props() creates CPU property "pcc-cmac-eaes-256" twice. The second one fails, but the error is ignored (a later commit will change that). Results in a single property "pcc-cmac-eaes-256" with the description for S390_FEAT_PCC_CMAC_AES_256, and no property for S390_FEAT_PCC_CMAC_EAES_256. CPU properties are visible in CLI -cpu and -device, QMP & HMP device_add, QMP device-list-properties, and QOM introspection. Fix by deleting the wayward 'e'. >>> >>> Very nice catch - thanks! >> >> :) >> >>> While this sounds very bad, it's luckily not that bad in practice >>> (currently). >>> >>> The feature (or rather, both features) is part of the feature group >>> "msa4". As long as we have all sub-features part of that group (which is >>> usually the case), we will always indicate "msa4" to the user, instead >>> of all the separate sub-features. So, expansion, baseline, comparison >>> will usually only work with "msa4". >>> >>> (in addition, current KVM is not capable of actually masking off these >>> sub-features, so it will still, always see the feature, even if not >>> explicitly specified via "-cpu X,pcc-cmac-aes-256=on) >> >> Would you like to propose an commit message improvements? > > Maybe something like > > "Both affected features are part of the feature group msa4. In current > setups, we will always see the msa4 feature instead of the separate > contained sub-features (because all sub-features are around). Therefore, > both features are currently never passed from/to the user explicitly > (e.g., via cpu model expansion, comparison, baseline and '-cpu' setup)." > > Thanks! I think I can guess how this could work for reporting features (I haven't checked my guess against the code), which is what the query-cpu-model-* do: suppress individual features when their group is complete. But "'-cpu' setup" doesn't seem to be about reporting features. Am I confused? While testing, I noticed that $ s390x-softmmu/qemu-system-s390x flashes a window at me, then terminates successfully, without printing anything. With -S, it behaves like other targets. Bug?
Re: [PATCH 08/17] s390x/cpumodel: Fix UI to CPU features pcc-cmac-{aes, eaes}-256
Christian Borntraeger writes: > On 29.04.20 10:54, Christian Borntraeger wrote: >> >> >> On 28.04.20 19:13, David Hildenbrand wrote: >>> On 28.04.20 18:34, Markus Armbruster wrote: Both s390_features[S390_FEAT_PCC_CMAC_AES_256].name and s390_features[S390_FEAT_PCC_CMAC_EAES_256].name is "pcc-cmac-eaes-256". The former is obviously a pasto. Impact: * s390_feat_bitmap_to_ascii() misidentifies S390_FEAT_PCC_CMAC_AES_256 as "pcc-cmac-eaes-256". Affects QMP commands query-cpu-definitions, query-cpu-model-expansion, query-cpu-model-baseline, query-cpu-model-comparison, and the error message when s390_realize_cpu_model() fails in check_compatibility(). * s390_realize_cpu_model() misidentifies it in check_consistency() warnings. * s390_cpu_list() likewise. Affects -cpu help. * s390_cpu_model_register_props() creates CPU property "pcc-cmac-eaes-256" twice. The second one fails, but the error is ignored (a later commit will change that). Results in a single property "pcc-cmac-eaes-256" with the description for S390_FEAT_PCC_CMAC_AES_256, and no property for S390_FEAT_PCC_CMAC_EAES_256. CPU properties are visible in CLI -cpu and -device, QMP & HMP device_add, QMP device-list-properties, and QOM introspection. Fix by deleting the wayward 'e'. >>> >>> Very nice catch - thanks! >>> >>> While this sounds very bad, it's luckily not that bad in practice >>> (currently). >>> >>> The feature (or rather, both features) is part of the feature group >>> "msa4". As long as we have all sub-features part of that group (which is >>> usually the case), we will always indicate "msa4" to the user, instead >>> of all the separate sub-features. So, expansion, baseline, comparison >>> will usually only work with "msa4". >>> >>> (in addition, current KVM is not capable of actually masking off these >>> sub-features, so it will still, always see the feature, even if not >>> explicitly specified via "-cpu X,pcc-cmac-aes-256=on) >>> >>> I think we should do stable backports. >> >> makes sense, but I would like to do some testing upfront (old QEMU <-> new >> QEMU > > So migration does work between a qemu with and without the patch for > host-model and > custom model=z14. Is this a Tested-by?
Re: [PATCH 08/17] s390x/cpumodel: Fix UI to CPU features pcc-cmac-{aes,eaes}-256
On 30.04.20 20:22, Markus Armbruster wrote: > David Hildenbrand writes: > >> On 28.04.20 18:34, Markus Armbruster wrote: >>> Both s390_features[S390_FEAT_PCC_CMAC_AES_256].name and >>> s390_features[S390_FEAT_PCC_CMAC_EAES_256].name is >>> "pcc-cmac-eaes-256". The former is obviously a pasto. >>> >>> Impact: >>> >>> * s390_feat_bitmap_to_ascii() misidentifies S390_FEAT_PCC_CMAC_AES_256 >>> as "pcc-cmac-eaes-256". Affects QMP commands query-cpu-definitions, >>> query-cpu-model-expansion, query-cpu-model-baseline, >>> query-cpu-model-comparison, and the error message when >>> s390_realize_cpu_model() fails in check_compatibility(). >>> >>> * s390_realize_cpu_model() misidentifies it in check_consistency() >>> warnings. >>> >>> * s390_cpu_list() likewise. Affects -cpu help. >>> >>> * s390_cpu_model_register_props() creates CPU property >>> "pcc-cmac-eaes-256" twice. The second one fails, but the error is >>> ignored (a later commit will change that). Results in a single >>> property "pcc-cmac-eaes-256" with the description for >>> S390_FEAT_PCC_CMAC_AES_256, and no property for >>> S390_FEAT_PCC_CMAC_EAES_256. CPU properties are visible in CLI -cpu >>> and -device, QMP & HMP device_add, QMP device-list-properties, and >>> QOM introspection. >>> >>> Fix by deleting the wayward 'e'. >> >> Very nice catch - thanks! > > :) > >> While this sounds very bad, it's luckily not that bad in practice >> (currently). >> >> The feature (or rather, both features) is part of the feature group >> "msa4". As long as we have all sub-features part of that group (which is >> usually the case), we will always indicate "msa4" to the user, instead >> of all the separate sub-features. So, expansion, baseline, comparison >> will usually only work with "msa4". >> >> (in addition, current KVM is not capable of actually masking off these >> sub-features, so it will still, always see the feature, even if not >> explicitly specified via "-cpu X,pcc-cmac-aes-256=on) > > Would you like to propose an commit message improvements? Maybe something like "Both affected features are part of the feature group msa4. In current setups, we will always see the msa4 feature instead of the separate contained sub-features (because all sub-features are around). Therefore, both features are currently never passed from/to the user explicitly (e.g., via cpu model expansion, comparison, baseline and '-cpu' setup)." Thanks! -- Thanks, David / dhildenb
Re: [PATCH 08/17] s390x/cpumodel: Fix UI to CPU features pcc-cmac-{aes,eaes}-256
On 29.04.20 10:54, Christian Borntraeger wrote: > > > On 28.04.20 19:13, David Hildenbrand wrote: >> On 28.04.20 18:34, Markus Armbruster wrote: >>> Both s390_features[S390_FEAT_PCC_CMAC_AES_256].name and >>> s390_features[S390_FEAT_PCC_CMAC_EAES_256].name is >>> "pcc-cmac-eaes-256". The former is obviously a pasto. >>> >>> Impact: >>> >>> * s390_feat_bitmap_to_ascii() misidentifies S390_FEAT_PCC_CMAC_AES_256 >>> as "pcc-cmac-eaes-256". Affects QMP commands query-cpu-definitions, >>> query-cpu-model-expansion, query-cpu-model-baseline, >>> query-cpu-model-comparison, and the error message when >>> s390_realize_cpu_model() fails in check_compatibility(). >>> >>> * s390_realize_cpu_model() misidentifies it in check_consistency() >>> warnings. >>> >>> * s390_cpu_list() likewise. Affects -cpu help. >>> >>> * s390_cpu_model_register_props() creates CPU property >>> "pcc-cmac-eaes-256" twice. The second one fails, but the error is >>> ignored (a later commit will change that). Results in a single >>> property "pcc-cmac-eaes-256" with the description for >>> S390_FEAT_PCC_CMAC_AES_256, and no property for >>> S390_FEAT_PCC_CMAC_EAES_256. CPU properties are visible in CLI -cpu >>> and -device, QMP & HMP device_add, QMP device-list-properties, and >>> QOM introspection. >>> >>> Fix by deleting the wayward 'e'. >> >> Very nice catch - thanks! >> >> While this sounds very bad, it's luckily not that bad in practice >> (currently). >> >> The feature (or rather, both features) is part of the feature group >> "msa4". As long as we have all sub-features part of that group (which is >> usually the case), we will always indicate "msa4" to the user, instead >> of all the separate sub-features. So, expansion, baseline, comparison >> will usually only work with "msa4". >> >> (in addition, current KVM is not capable of actually masking off these >> sub-features, so it will still, always see the feature, even if not >> explicitly specified via "-cpu X,pcc-cmac-aes-256=on) >> >> I think we should do stable backports. > > makes sense, but I would like to do some testing upfront (old QEMU <-> new > QEMU So migration does work between a qemu with and without the patch for host-model and custom model=z14.
Re: [PATCH 08/17] s390x/cpumodel: Fix UI to CPU features pcc-cmac-{aes, eaes}-256
David Hildenbrand writes: > On 28.04.20 18:34, Markus Armbruster wrote: >> Both s390_features[S390_FEAT_PCC_CMAC_AES_256].name and >> s390_features[S390_FEAT_PCC_CMAC_EAES_256].name is >> "pcc-cmac-eaes-256". The former is obviously a pasto. >> >> Impact: >> >> * s390_feat_bitmap_to_ascii() misidentifies S390_FEAT_PCC_CMAC_AES_256 >> as "pcc-cmac-eaes-256". Affects QMP commands query-cpu-definitions, >> query-cpu-model-expansion, query-cpu-model-baseline, >> query-cpu-model-comparison, and the error message when >> s390_realize_cpu_model() fails in check_compatibility(). >> >> * s390_realize_cpu_model() misidentifies it in check_consistency() >> warnings. >> >> * s390_cpu_list() likewise. Affects -cpu help. >> >> * s390_cpu_model_register_props() creates CPU property >> "pcc-cmac-eaes-256" twice. The second one fails, but the error is >> ignored (a later commit will change that). Results in a single >> property "pcc-cmac-eaes-256" with the description for >> S390_FEAT_PCC_CMAC_AES_256, and no property for >> S390_FEAT_PCC_CMAC_EAES_256. CPU properties are visible in CLI -cpu >> and -device, QMP & HMP device_add, QMP device-list-properties, and >> QOM introspection. >> >> Fix by deleting the wayward 'e'. > > Very nice catch - thanks! :) > While this sounds very bad, it's luckily not that bad in practice > (currently). > > The feature (or rather, both features) is part of the feature group > "msa4". As long as we have all sub-features part of that group (which is > usually the case), we will always indicate "msa4" to the user, instead > of all the separate sub-features. So, expansion, baseline, comparison > will usually only work with "msa4". > > (in addition, current KVM is not capable of actually masking off these > sub-features, so it will still, always see the feature, even if not > explicitly specified via "-cpu X,pcc-cmac-aes-256=on) Would you like to propose an commit message improvements? > I think we should do stable backports. > > Reviewed-by: David Hildenbrand Thanks!
Re: [PATCH 08/17] s390x/cpumodel: Fix UI to CPU features pcc-cmac-{aes,eaes}-256
On 28.04.20 19:13, David Hildenbrand wrote: > On 28.04.20 18:34, Markus Armbruster wrote: >> Both s390_features[S390_FEAT_PCC_CMAC_AES_256].name and >> s390_features[S390_FEAT_PCC_CMAC_EAES_256].name is >> "pcc-cmac-eaes-256". The former is obviously a pasto. >> >> Impact: >> >> * s390_feat_bitmap_to_ascii() misidentifies S390_FEAT_PCC_CMAC_AES_256 >> as "pcc-cmac-eaes-256". Affects QMP commands query-cpu-definitions, >> query-cpu-model-expansion, query-cpu-model-baseline, >> query-cpu-model-comparison, and the error message when >> s390_realize_cpu_model() fails in check_compatibility(). >> >> * s390_realize_cpu_model() misidentifies it in check_consistency() >> warnings. >> >> * s390_cpu_list() likewise. Affects -cpu help. >> >> * s390_cpu_model_register_props() creates CPU property >> "pcc-cmac-eaes-256" twice. The second one fails, but the error is >> ignored (a later commit will change that). Results in a single >> property "pcc-cmac-eaes-256" with the description for >> S390_FEAT_PCC_CMAC_AES_256, and no property for >> S390_FEAT_PCC_CMAC_EAES_256. CPU properties are visible in CLI -cpu >> and -device, QMP & HMP device_add, QMP device-list-properties, and >> QOM introspection. >> >> Fix by deleting the wayward 'e'. > > Very nice catch - thanks! > > While this sounds very bad, it's luckily not that bad in practice > (currently). > > The feature (or rather, both features) is part of the feature group > "msa4". As long as we have all sub-features part of that group (which is > usually the case), we will always indicate "msa4" to the user, instead > of all the separate sub-features. So, expansion, baseline, comparison > will usually only work with "msa4". > > (in addition, current KVM is not capable of actually masking off these > sub-features, so it will still, always see the feature, even if not > explicitly specified via "-cpu X,pcc-cmac-aes-256=on) > > I think we should do stable backports. makes sense, but I would like to do some testing upfront (old QEMU <-> new QEMU
Re: [PATCH 08/17] s390x/cpumodel: Fix UI to CPU features pcc-cmac-{aes,eaes}-256
On 28.04.20 18:34, Markus Armbruster wrote: > Both s390_features[S390_FEAT_PCC_CMAC_AES_256].name and > s390_features[S390_FEAT_PCC_CMAC_EAES_256].name is > "pcc-cmac-eaes-256". The former is obviously a pasto. > > Impact: > > * s390_feat_bitmap_to_ascii() misidentifies S390_FEAT_PCC_CMAC_AES_256 > as "pcc-cmac-eaes-256". Affects QMP commands query-cpu-definitions, > query-cpu-model-expansion, query-cpu-model-baseline, > query-cpu-model-comparison, and the error message when > s390_realize_cpu_model() fails in check_compatibility(). > > * s390_realize_cpu_model() misidentifies it in check_consistency() > warnings. > > * s390_cpu_list() likewise. Affects -cpu help. > > * s390_cpu_model_register_props() creates CPU property > "pcc-cmac-eaes-256" twice. The second one fails, but the error is > ignored (a later commit will change that). Results in a single > property "pcc-cmac-eaes-256" with the description for > S390_FEAT_PCC_CMAC_AES_256, and no property for > S390_FEAT_PCC_CMAC_EAES_256. CPU properties are visible in CLI -cpu > and -device, QMP & HMP device_add, QMP device-list-properties, and > QOM introspection. > > Fix by deleting the wayward 'e'. Very nice catch - thanks! While this sounds very bad, it's luckily not that bad in practice (currently). The feature (or rather, both features) is part of the feature group "msa4". As long as we have all sub-features part of that group (which is usually the case), we will always indicate "msa4" to the user, instead of all the separate sub-features. So, expansion, baseline, comparison will usually only work with "msa4". (in addition, current KVM is not capable of actually masking off these sub-features, so it will still, always see the feature, even if not explicitly specified via "-cpu X,pcc-cmac-aes-256=on) I think we should do stable backports. Reviewed-by: David Hildenbrand > > Fixes: 782417446279717aa85320191a519b51f6d5dd31 > Cc: Halil Pasic > Cc: Cornelia Huck > Cc: Christian Borntraeger > Cc: Richard Henderson > Cc: David Hildenbrand > Cc: qemu-s3...@nongnu.org > Signed-off-by: Markus Armbruster > --- > target/s390x/cpu_features_def.inc.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/s390x/cpu_features_def.inc.h > b/target/s390x/cpu_features_def.inc.h > index 31dff0d84e..a8d562d688 100644 > --- a/target/s390x/cpu_features_def.inc.h > +++ b/target/s390x/cpu_features_def.inc.h > @@ -310,7 +310,7 @@ DEF_FEAT(PCC_CMAC_ETDEA_192, "pcc-cmac-etdea-128", PCC, > 10, "PCC Compute-Last-Bl > DEF_FEAT(PCC_CMAC_TDEA, "pcc-cmac-etdea-192", PCC, 11, "PCC > Compute-Last-Block-CMAC-Using-EncryptedTDEA-192") > DEF_FEAT(PCC_CMAC_AES_128, "pcc-cmac-aes-128", PCC, 18, "PCC > Compute-Last-Block-CMAC-Using-AES-128") > DEF_FEAT(PCC_CMAC_AES_192, "pcc-cmac-aes-192", PCC, 19, "PCC > Compute-Last-Block-CMAC-Using-AES-192") > -DEF_FEAT(PCC_CMAC_AES_256, "pcc-cmac-eaes-256", PCC, 20, "PCC > Compute-Last-Block-CMAC-Using-AES-256") > +DEF_FEAT(PCC_CMAC_AES_256, "pcc-cmac-aes-256", PCC, 20, "PCC > Compute-Last-Block-CMAC-Using-AES-256") > DEF_FEAT(PCC_CMAC_EAES_128, "pcc-cmac-eaes-128", PCC, 26, "PCC > Compute-Last-Block-CMAC-Using-Encrypted-AES-128") > DEF_FEAT(PCC_CMAC_EAES_192, "pcc-cmac-eaes-192", PCC, 27, "PCC > Compute-Last-Block-CMAC-Using-Encrypted-AES-192") > DEF_FEAT(PCC_CMAC_EAES_256, "pcc-cmac-eaes-256", PCC, 28, "PCC > Compute-Last-Block-CMAC-Using-Encrypted-AES-256") > -- Thanks, David / dhildenb
[PATCH 08/17] s390x/cpumodel: Fix UI to CPU features pcc-cmac-{aes, eaes}-256
Both s390_features[S390_FEAT_PCC_CMAC_AES_256].name and s390_features[S390_FEAT_PCC_CMAC_EAES_256].name is "pcc-cmac-eaes-256". The former is obviously a pasto. Impact: * s390_feat_bitmap_to_ascii() misidentifies S390_FEAT_PCC_CMAC_AES_256 as "pcc-cmac-eaes-256". Affects QMP commands query-cpu-definitions, query-cpu-model-expansion, query-cpu-model-baseline, query-cpu-model-comparison, and the error message when s390_realize_cpu_model() fails in check_compatibility(). * s390_realize_cpu_model() misidentifies it in check_consistency() warnings. * s390_cpu_list() likewise. Affects -cpu help. * s390_cpu_model_register_props() creates CPU property "pcc-cmac-eaes-256" twice. The second one fails, but the error is ignored (a later commit will change that). Results in a single property "pcc-cmac-eaes-256" with the description for S390_FEAT_PCC_CMAC_AES_256, and no property for S390_FEAT_PCC_CMAC_EAES_256. CPU properties are visible in CLI -cpu and -device, QMP & HMP device_add, QMP device-list-properties, and QOM introspection. Fix by deleting the wayward 'e'. Fixes: 782417446279717aa85320191a519b51f6d5dd31 Cc: Halil Pasic Cc: Cornelia Huck Cc: Christian Borntraeger Cc: Richard Henderson Cc: David Hildenbrand Cc: qemu-s3...@nongnu.org Signed-off-by: Markus Armbruster --- target/s390x/cpu_features_def.inc.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/s390x/cpu_features_def.inc.h b/target/s390x/cpu_features_def.inc.h index 31dff0d84e..a8d562d688 100644 --- a/target/s390x/cpu_features_def.inc.h +++ b/target/s390x/cpu_features_def.inc.h @@ -310,7 +310,7 @@ DEF_FEAT(PCC_CMAC_ETDEA_192, "pcc-cmac-etdea-128", PCC, 10, "PCC Compute-Last-Bl DEF_FEAT(PCC_CMAC_TDEA, "pcc-cmac-etdea-192", PCC, 11, "PCC Compute-Last-Block-CMAC-Using-EncryptedTDEA-192") DEF_FEAT(PCC_CMAC_AES_128, "pcc-cmac-aes-128", PCC, 18, "PCC Compute-Last-Block-CMAC-Using-AES-128") DEF_FEAT(PCC_CMAC_AES_192, "pcc-cmac-aes-192", PCC, 19, "PCC Compute-Last-Block-CMAC-Using-AES-192") -DEF_FEAT(PCC_CMAC_AES_256, "pcc-cmac-eaes-256", PCC, 20, "PCC Compute-Last-Block-CMAC-Using-AES-256") +DEF_FEAT(PCC_CMAC_AES_256, "pcc-cmac-aes-256", PCC, 20, "PCC Compute-Last-Block-CMAC-Using-AES-256") DEF_FEAT(PCC_CMAC_EAES_128, "pcc-cmac-eaes-128", PCC, 26, "PCC Compute-Last-Block-CMAC-Using-Encrypted-AES-128") DEF_FEAT(PCC_CMAC_EAES_192, "pcc-cmac-eaes-192", PCC, 27, "PCC Compute-Last-Block-CMAC-Using-Encrypted-AES-192") DEF_FEAT(PCC_CMAC_EAES_256, "pcc-cmac-eaes-256", PCC, 28, "PCC Compute-Last-Block-CMAC-Using-Encrypted-AES-256") -- 2.21.1
