[libvirt] X86CPU "feature-words" property on QEMU (was Re: [PATCH v3 1/3] x86: Data structure changes to support MSR based) features

On Fri, Aug 17, 2018 at 07:34:13PM +0200, Paolo Bonzini wrote:
On 17/08/2018 17:55, Eduardo Habkost wrote:
The names will be X86_CPU_FEATURE_WORD_TYPE_CPUID and X86_CPU_FEATURE_WORD_TYPE_MSR. I wouldn't like to make this an external API unless really necessary. I would rather deprecate the "feature-words" property because nobody ended up using it.
I think we should either remove it directly, or extend it to support MSR features. Deprecating and only supporting CPUID features is the worst of both worlds...
So let's do what's necessary to remove it. But I don't think the removal of "feature-words" should block the inclusion of this series. Now, should QOM properties follow our feature deprecation policy, or they were never a supported external API and we can remove it immediately? CCing Jiri and libvir-list, because I just found that there's code on libvirt that uses it, but I don't know exactly it does with that info. -- Eduardo

On 17/08/2018 19:48, Eduardo Habkost wrote:
So let's do what's necessary to remove it. But I don't think the removal of "feature-words" should block the inclusion of this series.
Now, should QOM properties follow our feature deprecation policy, or they were never a supported external API and we can remove it immediately?
CCing Jiri and libvir-list, because I just found that there's code on libvirt that uses it, but I don't know exactly it does with that info.
It is used to check whether the host supports invtsc and pv-unhalt and avoid changing the guest ABI when migrating: see https://marc.info/?l=libvir-list&m=152445761414746&w=2 for a patch that introduces one user. I think we should extend it to MSRs, not remove it. Paolo

On Fri, Aug 17, 2018 at 07:59:40PM +0200, Paolo Bonzini wrote:
On 17/08/2018 19:48, Eduardo Habkost wrote:
So let's do what's necessary to remove it. But I don't think the removal of "feature-words" should block the inclusion of this series.
Now, should QOM properties follow our feature deprecation policy, or they were never a supported external API and we can remove it immediately?
CCing Jiri and libvir-list, because I just found that there's code on libvirt that uses it, but I don't know exactly it does with that info.
It is used to check whether the host supports invtsc and pv-unhalt and avoid changing the guest ABI when migrating: see https://marc.info/?l=libvir-list&m=152445761414746&w=2 for a patch that introduces one user.
I think we should extend it to MSRs, not remove it.
Well, I would prefer if libvirt simply did (e.g.) "qom-get property=invtsc" instead of fetching raw feature-words data. But if we do have an existing user, I now agree with you: let's keep it working and extend it to include MSR info too. BTW, libvirt must stop using this hardcoded QOM path: #define QOM_CPU_PATH "/machine/unattached/device[0]" It should use the "qom_path" field of "query-cpus" instead. -- Eduardo
participants (2)
-
Eduardo Habkost
-
Paolo Bonzini