On Tue, 2024-03-05 at 14:17 +0000, Daniel P. Berrangé wrote: > > On Tue, Feb 06, 2024 at 02:47:34PM +0100, Tim Wiederhake wrote: > > > > Synchronizing the list of cpu features and models with qemu is > > > > a > > > > recurring > > > > task in libvirt. For x86, this is done by reading > > > > qom-list-properties for > > > > max-x86_64-cpu and manually filtering out everthing that does > > > > not > > > > look like > > > > a feature name, as well as parsing target/i386/cpu.c for cpu > > > > models. > > > > > > > > This is a flawed, tedious and error-prone procedure. Ideally, > > > > qemu > > > > and libvirt would query a common source for cpu feature and > > > > model > > > > related information. Meanwhile, converting this information > > > > into an > > > > easier > > > > to parse format would help libvirt a lot. > > > > > > > > This patch series converts the cpu feature information present > > > > in > > > > target/i386/cpu.c (`feature_word_info`) into a yaml file and > > > > adds a > > > > script to generate the c code from this data. > > > > Looking at this fresh, I'm left wondering why I didn't suggested > > using 'QMP' to expose this information when reviewing the earlier > > versions. I see Igor did indeed suggest this: > > > > > > https://lists.nongnu.org/archive/html/qemu-devel/2023-09/msg03905.html > > > > Your commentry that "qom-list-properties" doesn't distinguish > > between CPU features and other random QOM properties is bang > > on the money. > > > > I think what this highlights, is that 'qom-list-properties' > > is a very poor design/fit for the problem that management apps > > need to solve in this regard. > > > > Libvirt should not need to manually exclude non-feature properties > > like 'check' 'enforce' 'migratable' etc. > > > > QEMU already has this knowledge, as IIUC, 'query-cpu-model- > > expansion' > > can distinguish this: > > > > query-cpu-model-expansion type=static model={'name':'Nehalem'} > > { > > "return": { > > "model": { > > "name": "base", > > "props": { > > "3dnow": false, > > ...snip... > > "xtpr": false > > } > > } > > } > > } > > > > We still have the problem that we're not exposing the CPUID/MSR > > leafs/register bits. So query-cpu-model-expansion isn't a fit > > for the problem. > > > > Rather than try to design something super general purpose, I'd > > suggest we take a short cut and design something entirley x86 > > specific, and simply mark the QMP command as "unstable" > > eg a 'x-query-x86-cpu-model-features', and then basically > > report all the information libvirt needs there. > > > > This is functionally equivalent to what you expose in the YAML > > file, while still using QEMU's formal 'QMP' API mechanism, so > > we avoid inventing a new API concept via YAML. > > > > I think this would avoid need to have a code generator refactor > > the CPU definitions too. We just need to expose the values of > > the existing CPUID_xxx constants against each register. > > > > > > > > With regards, > > Daniel
Thank you for your feedback. I do not see the patches and your proposed x-query-x86-cpu-model- features QMP command being mutually exclusive. In fact, I'd advocate for merging this patches still, as they provide a solution (albeit not through QMP) already whereas the QMP command would still need to be written. Additionally, there are more benefits to the generate-code approach, as the code generator can be extended to also generate the feature bits "#define CPUID_* (1U << ...)" in cpu.h, removing one more source of errors. And with the generated `feature_word_info` structure being virtually identical to the current version, I see no downsides: If the generator does become obsolete in the future, simply remove the python script and the yaml file, and all that is left is the original feature_word_info code, but better formatted. Regards, Tim
