[libvirt PATCH v2 03/24] cpu: make x86 feature alias names machine readable

Tim Wiederhake posted 24 patches 3 years, 3 months ago
There is a newer version of this series
[libvirt PATCH v2 03/24] cpu: make x86 feature alias names machine readable
Posted by Tim Wiederhake 3 years, 3 months ago
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
---
 src/cpu_map/x86_features.xml | 55 +++++++++++++++++++++++++-----------
 1 file changed, 38 insertions(+), 17 deletions(-)

diff --git a/src/cpu_map/x86_features.xml b/src/cpu_map/x86_features.xml
index 4cf3ff0804..90d0f43fc6 100644
--- a/src/cpu_map/x86_features.xml
+++ b/src/cpu_map/x86_features.xml
@@ -98,10 +98,12 @@
   </feature>
 
   <!-- standard features, ECX -->
-  <feature name='pni'> <!-- sse3 -->
+  <feature name='pni'>
+    <alias name='sse3'/>
     <cpuid eax_in='0x01' ecx='0x00000001'/>
   </feature>
-  <feature name='pclmuldq'> <!-- pclmulqdq -->
+  <feature name='pclmuldq'>
+    <alias name='pclmulqdq'/>
     <cpuid eax_in='0x01' ecx='0x00000002'/>
   </feature>
   <feature name='dtes64'>
@@ -110,7 +112,8 @@
   <feature name='monitor'>
     <cpuid eax_in='0x01' ecx='0x00000008'/>
   </feature>
-  <feature name='ds_cpl'> <!-- ds-cpl -->
+  <feature name='ds_cpl'>
+    <alias name='ds-cpl'/>
     <cpuid eax_in='0x01' ecx='0x00000010'/>
   </feature>
   <feature name='vmx'>
@@ -149,10 +152,14 @@
   <feature name='dca'>
     <cpuid eax_in='0x01' ecx='0x00040000'/>
   </feature>
-  <feature name='sse4.1'> <!-- sse4-1, sse4_1 -->
+  <feature name='sse4.1'>
+    <alias name='sse4-1'/>
+    <alias name='sse4_1'/>
     <cpuid eax_in='0x01' ecx='0x00080000'/>
   </feature>
-  <feature name='sse4.2'> <!-- sse4-2, sse4_2 -->
+  <feature name='sse4.2'>
+    <alias name='sse4-2'/>
+    <alias name='sse4_2'/>
     <cpuid eax_in='0x01' ecx='0x00100000'/>
   </feature>
   <feature name='x2apic'>
@@ -198,7 +205,8 @@
   <feature name='fsgsbase'>
     <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00000001'/>
   </feature>
-  <feature name='tsc_adjust'> <!-- tsc-adjust -->
+  <feature name='tsc_adjust'>
+    <alias name='tsc-adjust'/>
     <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00000002'/>
   </feature>
   <feature name='bmi1'>
@@ -225,7 +233,8 @@
   <feature name='rtm'>
     <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00000800'/>
   </feature>
-  <feature name='cmt'> <!-- cqm -->
+  <feature name='cmt'>
+    <alias name='cqm'/>
     <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00001000'/>
   </feature>
   <feature name='mpx'>
@@ -341,7 +350,8 @@
   <feature name='fsrm'>
     <cpuid eax_in='0x07' ecx_in='0x00' edx='0x00000010'/>
   </feature>
-  <feature name='md-clear'> <!-- md_clear -->
+  <feature name='md-clear'>
+    <alias name='md_clear'/>
     <cpuid eax_in='0x07' ecx_in='0x00' edx='0x00000400'/>
   </feature>
   <feature name='pconfig'>
@@ -353,7 +363,9 @@
   <feature name='stibp'>
     <cpuid eax_in='0x07' ecx_in='0x00' edx='0x08000000'/>
   </feature>
-  <feature name='arch-capabilities'> <!-- arch_capabilities, arch-facilities -->
+  <feature name='arch-capabilities'>
+    <alias name='arch_capabilities'/>
+    <alias name='arch-facilities'/>
     <cpuid eax_in='0x07' ecx_in='0x00' edx='0x20000000'/>
   </feature>
   <feature name='core-capability'>
@@ -396,13 +408,16 @@
   <feature name='syscall'>
     <cpuid eax_in='0x80000001' edx='0x00000800'/>
   </feature>
-  <feature name='nx'> <!-- xd -->
+  <feature name='nx'>
+    <alias name='xd'/>
     <cpuid eax_in='0x80000001' edx='0x00100000'/>
   </feature>
   <feature name='mmxext'>
     <cpuid eax_in='0x80000001' edx='0x00400000'/>
   </feature>
-  <feature name='fxsr_opt'> <!-- ffxsr, fxsr-opt -->
+  <feature name='fxsr_opt'>
+    <alias name='ffxsr'/>
+    <alias name='fxsr-opt'/>
     <cpuid eax_in='0x80000001' edx='0x02000000'/>
   </feature>
   <feature name='pdpe1gb'>
@@ -411,7 +426,8 @@
   <feature name='rdtscp'>
     <cpuid eax_in='0x80000001' edx='0x08000000'/>
   </feature>
-  <feature name='lm'> <!-- i64 -->
+  <feature name='lm'>
+    <alias name='i64'/>
     <cpuid eax_in='0x80000001' edx='0x20000000'/>
   </feature>
   <feature name='3dnowext'>
@@ -422,10 +438,12 @@
   </feature>
 
   <!-- extended features, ECX -->
-  <feature name='lahf_lm'> <!-- lahf-lm -->
+  <feature name='lahf_lm'>
+    <alias name='lahf-lm'/>
     <cpuid eax_in='0x80000001' ecx='0x00000001'/>
   </feature>
-  <feature name='cmp_legacy'> <!-- cmp-legacy -->
+  <feature name='cmp_legacy'>
+    <alias name='cmp-legacy'/>
     <cpuid eax_in='0x80000001' ecx='0x00000002'/>
   </feature>
   <feature name='svm'>
@@ -476,7 +494,8 @@
   <feature name='cvt16'>
     <cpuid eax_in='0x80000001' ecx='0x00040000'/>
   </feature>
-  <feature name='nodeid_msr'> <!-- nodeid-msr -->
+  <feature name='nodeid_msr'>
+    <alias name='nodeid-msr'/>
     <cpuid eax_in='0x80000001' ecx='0x00080000'/>
   </feature>
   <feature name='tbm'>
@@ -485,10 +504,12 @@
   <feature name='topoext'>
     <cpuid eax_in='0x80000001' ecx='0x00400000'/>
   </feature>
-  <feature name='perfctr_core'> <!-- perfctr-core -->
+  <feature name='perfctr_core'>
+    <alias name='perfctr-core'/>
     <cpuid eax_in='0x80000001' ecx='0x00800000'/>
   </feature>
-  <feature name='perfctr_nb'> <!-- perfctr-nb -->
+  <feature name='perfctr_nb'>
+    <alias name='perfctr-nb'/>
     <cpuid eax_in='0x80000001' ecx='0x01000000'/>
   </feature>
 
-- 
2.36.1
Re: [libvirt PATCH v2 03/24] cpu: make x86 feature alias names machine readable
Posted by Daniel P. Berrangé 3 years, 3 months ago
On Fri, Oct 28, 2022 at 05:06:34PM +0200, Tim Wiederhake wrote:
> Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
> ---
>  src/cpu_map/x86_features.xml | 55 +++++++++++++++++++++++++-----------
>  1 file changed, 38 insertions(+), 17 deletions(-)
> 
> diff --git a/src/cpu_map/x86_features.xml b/src/cpu_map/x86_features.xml
> index 4cf3ff0804..90d0f43fc6 100644
> --- a/src/cpu_map/x86_features.xml
> +++ b/src/cpu_map/x86_features.xml
> @@ -98,10 +98,12 @@
>    </feature>
>  
>    <!-- standard features, ECX -->
> -  <feature name='pni'> <!-- sse3 -->
> +  <feature name='pni'>
> +    <alias name='sse3'/>

IMHO, we should distinguish this as internal data via a NS to
re-inforce to our future selves that its only used when syncing
data from QEMU.

   <qemu:alias nmame="sse3"/>

And add xmlns:qemu="https://libvirt.org/cpufeature/qemu/1.0" on
the top level.

>      <cpuid eax_in='0x01' ecx='0x00000001'/>
>    </feature>
> -  <feature name='pclmuldq'> <!-- pclmulqdq -->
> +  <feature name='pclmuldq'>
> +    <alias name='pclmulqdq'/>
>      <cpuid eax_in='0x01' ecx='0x00000002'/>
>    </feature>
>    <feature name='dtes64'>
> @@ -110,7 +112,8 @@
>    <feature name='monitor'>
>      <cpuid eax_in='0x01' ecx='0x00000008'/>
>    </feature>
> -  <feature name='ds_cpl'> <!-- ds-cpl -->
> +  <feature name='ds_cpl'>
> +    <alias name='ds-cpl'/>
>      <cpuid eax_in='0x01' ecx='0x00000010'/>
>    </feature>
>    <feature name='vmx'>
> @@ -149,10 +152,14 @@
>    <feature name='dca'>
>      <cpuid eax_in='0x01' ecx='0x00040000'/>
>    </feature>
> -  <feature name='sse4.1'> <!-- sse4-1, sse4_1 -->
> +  <feature name='sse4.1'>
> +    <alias name='sse4-1'/>
> +    <alias name='sse4_1'/>
>      <cpuid eax_in='0x01' ecx='0x00080000'/>
>    </feature>
> -  <feature name='sse4.2'> <!-- sse4-2, sse4_2 -->
> +  <feature name='sse4.2'>
> +    <alias name='sse4-2'/>
> +    <alias name='sse4_2'/>
>      <cpuid eax_in='0x01' ecx='0x00100000'/>
>    </feature>
>    <feature name='x2apic'>
> @@ -198,7 +205,8 @@
>    <feature name='fsgsbase'>
>      <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00000001'/>
>    </feature>
> -  <feature name='tsc_adjust'> <!-- tsc-adjust -->
> +  <feature name='tsc_adjust'>
> +    <alias name='tsc-adjust'/>
>      <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00000002'/>
>    </feature>
>    <feature name='bmi1'>
> @@ -225,7 +233,8 @@
>    <feature name='rtm'>
>      <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00000800'/>
>    </feature>
> -  <feature name='cmt'> <!-- cqm -->
> +  <feature name='cmt'>
> +    <alias name='cqm'/>
>      <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00001000'/>
>    </feature>
>    <feature name='mpx'>
> @@ -341,7 +350,8 @@
>    <feature name='fsrm'>
>      <cpuid eax_in='0x07' ecx_in='0x00' edx='0x00000010'/>
>    </feature>
> -  <feature name='md-clear'> <!-- md_clear -->
> +  <feature name='md-clear'>
> +    <alias name='md_clear'/>
>      <cpuid eax_in='0x07' ecx_in='0x00' edx='0x00000400'/>
>    </feature>
>    <feature name='pconfig'>
> @@ -353,7 +363,9 @@
>    <feature name='stibp'>
>      <cpuid eax_in='0x07' ecx_in='0x00' edx='0x08000000'/>
>    </feature>
> -  <feature name='arch-capabilities'> <!-- arch_capabilities, arch-facilities -->
> +  <feature name='arch-capabilities'>
> +    <alias name='arch_capabilities'/>
> +    <alias name='arch-facilities'/>
>      <cpuid eax_in='0x07' ecx_in='0x00' edx='0x20000000'/>
>    </feature>
>    <feature name='core-capability'>
> @@ -396,13 +408,16 @@
>    <feature name='syscall'>
>      <cpuid eax_in='0x80000001' edx='0x00000800'/>
>    </feature>
> -  <feature name='nx'> <!-- xd -->
> +  <feature name='nx'>
> +    <alias name='xd'/>
>      <cpuid eax_in='0x80000001' edx='0x00100000'/>
>    </feature>
>    <feature name='mmxext'>
>      <cpuid eax_in='0x80000001' edx='0x00400000'/>
>    </feature>
> -  <feature name='fxsr_opt'> <!-- ffxsr, fxsr-opt -->
> +  <feature name='fxsr_opt'>
> +    <alias name='ffxsr'/>
> +    <alias name='fxsr-opt'/>
>      <cpuid eax_in='0x80000001' edx='0x02000000'/>
>    </feature>
>    <feature name='pdpe1gb'>
> @@ -411,7 +426,8 @@
>    <feature name='rdtscp'>
>      <cpuid eax_in='0x80000001' edx='0x08000000'/>
>    </feature>
> -  <feature name='lm'> <!-- i64 -->
> +  <feature name='lm'>
> +    <alias name='i64'/>
>      <cpuid eax_in='0x80000001' edx='0x20000000'/>
>    </feature>
>    <feature name='3dnowext'>
> @@ -422,10 +438,12 @@
>    </feature>
>  
>    <!-- extended features, ECX -->
> -  <feature name='lahf_lm'> <!-- lahf-lm -->
> +  <feature name='lahf_lm'>
> +    <alias name='lahf-lm'/>
>      <cpuid eax_in='0x80000001' ecx='0x00000001'/>
>    </feature>
> -  <feature name='cmp_legacy'> <!-- cmp-legacy -->
> +  <feature name='cmp_legacy'>
> +    <alias name='cmp-legacy'/>
>      <cpuid eax_in='0x80000001' ecx='0x00000002'/>
>    </feature>
>    <feature name='svm'>
> @@ -476,7 +494,8 @@
>    <feature name='cvt16'>
>      <cpuid eax_in='0x80000001' ecx='0x00040000'/>
>    </feature>
> -  <feature name='nodeid_msr'> <!-- nodeid-msr -->
> +  <feature name='nodeid_msr'>
> +    <alias name='nodeid-msr'/>
>      <cpuid eax_in='0x80000001' ecx='0x00080000'/>
>    </feature>
>    <feature name='tbm'>
> @@ -485,10 +504,12 @@
>    <feature name='topoext'>
>      <cpuid eax_in='0x80000001' ecx='0x00400000'/>
>    </feature>
> -  <feature name='perfctr_core'> <!-- perfctr-core -->
> +  <feature name='perfctr_core'>
> +    <alias name='perfctr-core'/>
>      <cpuid eax_in='0x80000001' ecx='0x00800000'/>
>    </feature>
> -  <feature name='perfctr_nb'> <!-- perfctr-nb -->
> +  <feature name='perfctr_nb'>
> +    <alias name='perfctr-nb'/>
>      <cpuid eax_in='0x80000001' ecx='0x01000000'/>
>    </feature>
>  
> -- 
> 2.36.1
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [libvirt PATCH v2 03/24] cpu: make x86 feature alias names machine readable
Posted by Tim Wiederhake 3 years, 3 months ago
On Mon, 2022-10-31 at 09:38 +0000, Daniel P. Berrangé wrote:
> On Fri, Oct 28, 2022 at 05:06:34PM +0200, Tim Wiederhake wrote:
> > Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
> > Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
> > ---
> >  src/cpu_map/x86_features.xml | 55 +++++++++++++++++++++++++-------
> > ----
> >  1 file changed, 38 insertions(+), 17 deletions(-)
> > 
> > diff --git a/src/cpu_map/x86_features.xml
> > b/src/cpu_map/x86_features.xml
> > index 4cf3ff0804..90d0f43fc6 100644
> > --- a/src/cpu_map/x86_features.xml
> > +++ b/src/cpu_map/x86_features.xml
> > @@ -98,10 +98,12 @@
> >    </feature>
> >  
> >    <!-- standard features, ECX -->
> > -  <feature name='pni'> <!-- sse3 -->
> > +  <feature name='pni'>
> > +    <alias name='sse3'/>
> 
> IMHO, we should distinguish this as internal data via a NS to
> re-inforce to our future selves that its only used when syncing
> data from QEMU.
> 
>    <qemu:alias nmame="sse3"/>
> 
> And add xmlns:qemu="https://libvirt.org/cpufeature/qemu/1.0" on
> the top level.
> 
I believe that using namespaces makes using this information in the
sync script, and later in virQEMUCapsCPUFeatureTranslate, harder than
strictly necessary. Would you be fine instead with a "source" or "type"
attribute instead?

    <alias name='foo' source='qemu'/>
    <alias name='bar' source='linux'/>
    <alias name='baz' source='where-the-feature-is-called-this-way'/>

Tim

> >      <cpuid eax_in='0x01' ecx='0x00000001'/>
> >    </feature>
> > -  <feature name='pclmuldq'> <!-- pclmulqdq -->
> > +  <feature name='pclmuldq'>
> > +    <alias name='pclmulqdq'/>
> >      <cpuid eax_in='0x01' ecx='0x00000002'/>
> >    </feature>
> >    <feature name='dtes64'>
> > @@ -110,7 +112,8 @@
> >    <feature name='monitor'>
> >      <cpuid eax_in='0x01' ecx='0x00000008'/>
> >    </feature>
> > -  <feature name='ds_cpl'> <!-- ds-cpl -->
> > +  <feature name='ds_cpl'>
> > +    <alias name='ds-cpl'/>
> >      <cpuid eax_in='0x01' ecx='0x00000010'/>
> >    </feature>
> >    <feature name='vmx'>
> > @@ -149,10 +152,14 @@
> >    <feature name='dca'>
> >      <cpuid eax_in='0x01' ecx='0x00040000'/>
> >    </feature>
> > -  <feature name='sse4.1'> <!-- sse4-1, sse4_1 -->
> > +  <feature name='sse4.1'>
> > +    <alias name='sse4-1'/>
> > +    <alias name='sse4_1'/>
> >      <cpuid eax_in='0x01' ecx='0x00080000'/>
> >    </feature>
> > -  <feature name='sse4.2'> <!-- sse4-2, sse4_2 -->
> > +  <feature name='sse4.2'>
> > +    <alias name='sse4-2'/>
> > +    <alias name='sse4_2'/>
> >      <cpuid eax_in='0x01' ecx='0x00100000'/>
> >    </feature>
> >    <feature name='x2apic'>
> > @@ -198,7 +205,8 @@
> >    <feature name='fsgsbase'>
> >      <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00000001'/>
> >    </feature>
> > -  <feature name='tsc_adjust'> <!-- tsc-adjust -->
> > +  <feature name='tsc_adjust'>
> > +    <alias name='tsc-adjust'/>
> >      <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00000002'/>
> >    </feature>
> >    <feature name='bmi1'>
> > @@ -225,7 +233,8 @@
> >    <feature name='rtm'>
> >      <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00000800'/>
> >    </feature>
> > -  <feature name='cmt'> <!-- cqm -->
> > +  <feature name='cmt'>
> > +    <alias name='cqm'/>
> >      <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00001000'/>
> >    </feature>
> >    <feature name='mpx'>
> > @@ -341,7 +350,8 @@
> >    <feature name='fsrm'>
> >      <cpuid eax_in='0x07' ecx_in='0x00' edx='0x00000010'/>
> >    </feature>
> > -  <feature name='md-clear'> <!-- md_clear -->
> > +  <feature name='md-clear'>
> > +    <alias name='md_clear'/>
> >      <cpuid eax_in='0x07' ecx_in='0x00' edx='0x00000400'/>
> >    </feature>
> >    <feature name='pconfig'>
> > @@ -353,7 +363,9 @@
> >    <feature name='stibp'>
> >      <cpuid eax_in='0x07' ecx_in='0x00' edx='0x08000000'/>
> >    </feature>
> > -  <feature name='arch-capabilities'> <!-- arch_capabilities, arch-
> > facilities -->
> > +  <feature name='arch-capabilities'>
> > +    <alias name='arch_capabilities'/>
> > +    <alias name='arch-facilities'/>
> >      <cpuid eax_in='0x07' ecx_in='0x00' edx='0x20000000'/>
> >    </feature>
> >    <feature name='core-capability'>
> > @@ -396,13 +408,16 @@
> >    <feature name='syscall'>
> >      <cpuid eax_in='0x80000001' edx='0x00000800'/>
> >    </feature>
> > -  <feature name='nx'> <!-- xd -->
> > +  <feature name='nx'>
> > +    <alias name='xd'/>
> >      <cpuid eax_in='0x80000001' edx='0x00100000'/>
> >    </feature>
> >    <feature name='mmxext'>
> >      <cpuid eax_in='0x80000001' edx='0x00400000'/>
> >    </feature>
> > -  <feature name='fxsr_opt'> <!-- ffxsr, fxsr-opt -->
> > +  <feature name='fxsr_opt'>
> > +    <alias name='ffxsr'/>
> > +    <alias name='fxsr-opt'/>
> >      <cpuid eax_in='0x80000001' edx='0x02000000'/>
> >    </feature>
> >    <feature name='pdpe1gb'>
> > @@ -411,7 +426,8 @@
> >    <feature name='rdtscp'>
> >      <cpuid eax_in='0x80000001' edx='0x08000000'/>
> >    </feature>
> > -  <feature name='lm'> <!-- i64 -->
> > +  <feature name='lm'>
> > +    <alias name='i64'/>
> >      <cpuid eax_in='0x80000001' edx='0x20000000'/>
> >    </feature>
> >    <feature name='3dnowext'>
> > @@ -422,10 +438,12 @@
> >    </feature>
> >  
> >    <!-- extended features, ECX -->
> > -  <feature name='lahf_lm'> <!-- lahf-lm -->
> > +  <feature name='lahf_lm'>
> > +    <alias name='lahf-lm'/>
> >      <cpuid eax_in='0x80000001' ecx='0x00000001'/>
> >    </feature>
> > -  <feature name='cmp_legacy'> <!-- cmp-legacy -->
> > +  <feature name='cmp_legacy'>
> > +    <alias name='cmp-legacy'/>
> >      <cpuid eax_in='0x80000001' ecx='0x00000002'/>
> >    </feature>
> >    <feature name='svm'>
> > @@ -476,7 +494,8 @@
> >    <feature name='cvt16'>
> >      <cpuid eax_in='0x80000001' ecx='0x00040000'/>
> >    </feature>
> > -  <feature name='nodeid_msr'> <!-- nodeid-msr -->
> > +  <feature name='nodeid_msr'>
> > +    <alias name='nodeid-msr'/>
> >      <cpuid eax_in='0x80000001' ecx='0x00080000'/>
> >    </feature>
> >    <feature name='tbm'>
> > @@ -485,10 +504,12 @@
> >    <feature name='topoext'>
> >      <cpuid eax_in='0x80000001' ecx='0x00400000'/>
> >    </feature>
> > -  <feature name='perfctr_core'> <!-- perfctr-core -->
> > +  <feature name='perfctr_core'>
> > +    <alias name='perfctr-core'/>
> >      <cpuid eax_in='0x80000001' ecx='0x00800000'/>
> >    </feature>
> > -  <feature name='perfctr_nb'> <!-- perfctr-nb -->
> > +  <feature name='perfctr_nb'>
> > +    <alias name='perfctr-nb'/>
> >      <cpuid eax_in='0x80000001' ecx='0x01000000'/>
> >    </feature>
> >  
> > -- 
> > 2.36.1
> > 
> 
> With regards,
> Daniel
Re: [libvirt PATCH v2 03/24] cpu: make x86 feature alias names machine readable
Posted by Daniel P. Berrangé 3 years, 3 months ago
On Thu, Nov 03, 2022 at 04:21:34PM +0100, Tim Wiederhake wrote:
> On Mon, 2022-10-31 at 09:38 +0000, Daniel P. Berrangé wrote:
> > On Fri, Oct 28, 2022 at 05:06:34PM +0200, Tim Wiederhake wrote:
> > > Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
> > > Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
> > > ---
> > >  src/cpu_map/x86_features.xml | 55 +++++++++++++++++++++++++-------
> > > ----
> > >  1 file changed, 38 insertions(+), 17 deletions(-)
> > > 
> > > diff --git a/src/cpu_map/x86_features.xml
> > > b/src/cpu_map/x86_features.xml
> > > index 4cf3ff0804..90d0f43fc6 100644
> > > --- a/src/cpu_map/x86_features.xml
> > > +++ b/src/cpu_map/x86_features.xml
> > > @@ -98,10 +98,12 @@
> > >    </feature>
> > >  
> > >    <!-- standard features, ECX -->
> > > -  <feature name='pni'> <!-- sse3 -->
> > > +  <feature name='pni'>
> > > +    <alias name='sse3'/>
> > 
> > IMHO, we should distinguish this as internal data via a NS to
> > re-inforce to our future selves that its only used when syncing
> > data from QEMU.
> > 
> >    <qemu:alias nmame="sse3"/>
> > 
> > And add xmlns:qemu="https://libvirt.org/cpufeature/qemu/1.0" on
> > the top level.
> > 
> I believe that using namespaces makes using this information in the
> sync script, and later in virQEMUCapsCPUFeatureTranslate, harder than
> strictly necessary. Would you be fine instead with a "source" or "type"
> attribute instead?
> 
>     <alias name='foo' source='qemu'/>
>     <alias name='bar' source='linux'/>
>     <alias name='baz' source='where-the-feature-is-called-this-way'/>

Yes, that could be ok, and makes it clear where the alias
came from.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [libvirt PATCH v2 03/24] cpu: make x86 feature alias names machine readable
Posted by Jiri Denemark 3 years, 3 months ago
On Mon, Oct 31, 2022 at 09:38:13 +0000, Daniel P. Berrangé wrote:
> On Fri, Oct 28, 2022 at 05:06:34PM +0200, Tim Wiederhake wrote:
> > Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
> > Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
> > ---
> >  src/cpu_map/x86_features.xml | 55 +++++++++++++++++++++++++-----------
> >  1 file changed, 38 insertions(+), 17 deletions(-)
> > 
> > diff --git a/src/cpu_map/x86_features.xml b/src/cpu_map/x86_features.xml
> > index 4cf3ff0804..90d0f43fc6 100644
> > --- a/src/cpu_map/x86_features.xml
> > +++ b/src/cpu_map/x86_features.xml
> > @@ -98,10 +98,12 @@
> >    </feature>
> >  
> >    <!-- standard features, ECX -->
> > -  <feature name='pni'> <!-- sse3 -->
> > +  <feature name='pni'>
> > +    <alias name='sse3'/>
> 
> IMHO, we should distinguish this as internal data via a NS to
> re-inforce to our future selves that its only used when syncing
> data from QEMU.
> 
>    <qemu:alias nmame="sse3"/>
> 
> And add xmlns:qemu="https://libvirt.org/cpufeature/qemu/1.0" on
> the top level.

Well, the existing alternative names in comments were not just different
QEMU spellings. Some of them were names used by the kernel. But I don't
really mind either way.

Jirka