On 02.05.20 08:26, Markus Armbruster wrote: > David Hildenbrand <da...@redhat.com> writes: > >> On 30.04.20 20:22, Markus Armbruster wrote: >>> David Hildenbrand <da...@redhat.com> 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
