[libvirt] [PATCH v4 5/8] libxl: add support for CPUID features policy

Marek Marczykowski-Górecki posted 8 patches 7 years, 3 months ago
There is a newer version of this series
[libvirt] [PATCH v4 5/8] libxl: add support for CPUID features policy
Posted by Marek Marczykowski-Górecki 7 years, 3 months ago
Convert CPU features policy into libxl cpuid policy settings. Use new
("libxl") syntax, which allow to enable/disable specific bits, using
host CPU as a base. For this reason, only "host-passthrough" mode is
accepted.
Libxl do not have distinction between "force" and "required" policy
(there is only "force") and also between "forbid" and "disable" (there
is only "disable"). So, merge them appropriately. If anything, "require"
and "forbid" should be enforced outside of specific driver.
Nested HVM (vmx and svm features) is handled separately, so exclude it
from translation.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
Changes since v2:
 - drop spurious changes
 - move libxlTranslateCPUFeature function to xen_xl.c, to be reused by
 xenconfig driver
Changes since v1:
 - use new ("libxl") syntax to set only bits explicitly mentioned in
 domain XML
---
 src/libxl/libxl_conf.c | 35 ++++++++++++++++++++++++++++++++++-
 src/xenconfig/xen_xl.c | 34 ++++++++++++++++++++++++++++++++++
 src/xenconfig/xen_xl.h |  2 ++
 3 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 417ce7c..d0afc1f 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -50,6 +50,7 @@
 #include "secret_util.h"
 #include "cpu/cpu.h"
 #include "xen_common.h"
+#include "xen_xl.h"
 
 
 #define VIR_FROM_THIS VIR_FROM_LIBXL
@@ -358,6 +359,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
         if (caps && def->cpu) {
             bool hasHwVirt = false;
             bool svm = false, vmx = false;
+            char xlCPU[32];
 
             if (def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) {
                 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -382,12 +384,43 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
                         case VIR_CPU_FEATURE_DISABLE:
                         case VIR_CPU_FEATURE_FORBID:
                             if ((vmx && STREQ(def->cpu->features[i].name, "vmx")) ||
-                                (svm && STREQ(def->cpu->features[i].name, "svm")))
+                                (svm && STREQ(def->cpu->features[i].name, "svm"))) {
                                 hasHwVirt = false;
+                                continue;
+                            }
+
+                            snprintf(xlCPU,
+                                    sizeof(xlCPU),
+                                    "%s=0",
+                                    xenTranslateCPUFeature(
+                                        def->cpu->features[i].name,
+                                        false));
+                            if (libxl_cpuid_parse_config(&b_info->cpuid, xlCPU)) {
+                                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                                        _("unsupported cpu feature '%s'"),
+                                        def->cpu->features[i].name);
+                                return -1;
+                            }
                             break;
 
                         case VIR_CPU_FEATURE_FORCE:
                         case VIR_CPU_FEATURE_REQUIRE:
+                            if ((vmx && STREQ(def->cpu->features[i].name, "vmx")) ||
+                                (svm && STREQ(def->cpu->features[i].name, "svm")))
+                                continue;
+
+                            snprintf(xlCPU,
+                                    sizeof(xlCPU),
+                                    "%s=1",
+                                    xenTranslateCPUFeature(
+                                        def->cpu->features[i].name, false));
+                            if (libxl_cpuid_parse_config(&b_info->cpuid, xlCPU)) {
+                                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                                        _("unsupported cpu feature '%s'"),
+                                        def->cpu->features[i].name);
+                                return -1;
+                            }
+                            break;
                         case VIR_CPU_FEATURE_OPTIONAL:
                         case VIR_CPU_FEATURE_LAST:
                             break;
diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index 394cc0d..d5dff59 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -218,6 +218,40 @@ xenParseXLOS(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps)
     return 0;
 }
 
+/*
+ * Translate CPU feature name from libvirt to libxl (from_libxl=false) or from
+ * libxl to libvirt (from_libxl=true).
+ */
+const char *
+xenTranslateCPUFeature(const char *feature_name, bool from_libxl)
+{
+    static const char *translation_table[][2] = {
+        /* libvirt name, libxl name */
+        { "cx16", "cmpxchg16" },
+        { "cid", "cntxid" },
+        { "ds_cpl", "dscpl" },
+        { "pclmuldq", "pclmulqdq" },
+        { "pni", "sse3" },
+        { "ht", "htt" },
+        { "pn", "psn" },
+        { "clflush", "clfsh" },
+        { "sep", "sysenter" },
+        { "cx8", "cmpxchg8" },
+        { "nodeid_msr", "nodeid" },
+        { "cr8legacy", "altmovcr8" },
+        { "lahf_lm", "lahfsahf" },
+        { "cmp_legacy", "cmplegacy" },
+        { "fxsr_opt", "ffxsr" },
+        { "pdpe1gb", "page1gb" },
+    };
+    size_t i;
+
+    for (i = 0; i < ARRAY_CARDINALITY(translation_table); i++)
+        if (STREQ(translation_table[i][from_libxl], feature_name))
+            return translation_table[i][!from_libxl];
+    return feature_name;
+}
+
 
 static int
 xenParseXLSpice(virConfPtr conf, virDomainDefPtr def)
diff --git a/src/xenconfig/xen_xl.h b/src/xenconfig/xen_xl.h
index dd96326..68f332a 100644
--- a/src/xenconfig/xen_xl.h
+++ b/src/xenconfig/xen_xl.h
@@ -33,4 +33,6 @@ virDomainDefPtr xenParseXL(virConfPtr conn,
 
 virConfPtr xenFormatXL(virDomainDefPtr def, virConnectPtr);
 
+const char *xenTranslateCPUFeature(const char *feature_name, bool from_libxl);
+
 #endif /* __VIR_XEN_XL_H__ */
-- 
git-series 0.9.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 5/8] libxl: add support for CPUID features policy
Posted by Jim Fehlig 7 years, 2 months ago
On 02/08/2018 03:58 PM, Marek Marczykowski-Górecki wrote:
> Convert CPU features policy into libxl cpuid policy settings. Use new
> ("libxl") syntax, which allow to enable/disable specific bits, using
> host CPU as a base. For this reason, only "host-passthrough" mode is
> accepted.
> Libxl do not have distinction between "force" and "required" policy
> (there is only "force") and also between "forbid" and "disable" (there
> is only "disable"). So, merge them appropriately. If anything, "require"
> and "forbid" should be enforced outside of specific driver.
> Nested HVM (vmx and svm features) is handled separately, so exclude it
> from translation.
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> ---
> Changes since v2:
>   - drop spurious changes
>   - move libxlTranslateCPUFeature function to xen_xl.c, to be reused by
>   xenconfig driver
> Changes since v1:
>   - use new ("libxl") syntax to set only bits explicitly mentioned in
>   domain XML
> ---
>   src/libxl/libxl_conf.c | 35 ++++++++++++++++++++++++++++++++++-
>   src/xenconfig/xen_xl.c | 34 ++++++++++++++++++++++++++++++++++
>   src/xenconfig/xen_xl.h |  2 ++
>   3 files changed, 70 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index 417ce7c..d0afc1f 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -50,6 +50,7 @@
>   #include "secret_util.h"
>   #include "cpu/cpu.h"
>   #include "xen_common.h"
> +#include "xen_xl.h"
>   
>   
>   #define VIR_FROM_THIS VIR_FROM_LIBXL
> @@ -358,6 +359,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
>           if (caps && def->cpu) {
>               bool hasHwVirt = false;
>               bool svm = false, vmx = false;
> +            char xlCPU[32];
>   
>               if (def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) {
>                   virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> @@ -382,12 +384,43 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
>                           case VIR_CPU_FEATURE_DISABLE:
>                           case VIR_CPU_FEATURE_FORBID:
>                               if ((vmx && STREQ(def->cpu->features[i].name, "vmx")) ||
> -                                (svm && STREQ(def->cpu->features[i].name, "svm")))
> +                                (svm && STREQ(def->cpu->features[i].name, "svm"))) {
>                                   hasHwVirt = false;
> +                                continue;
> +                            }
> +
> +                            snprintf(xlCPU,
> +                                    sizeof(xlCPU),
> +                                    "%s=0",
> +                                    xenTranslateCPUFeature(
> +                                        def->cpu->features[i].name,
> +                                        false));
> +                            if (libxl_cpuid_parse_config(&b_info->cpuid, xlCPU)) {
> +                                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                                        _("unsupported cpu feature '%s'"),
> +                                        def->cpu->features[i].name);
> +                                return -1;
> +                            }
>                               break;
>   
>                           case VIR_CPU_FEATURE_FORCE:
>                           case VIR_CPU_FEATURE_REQUIRE:
> +                            if ((vmx && STREQ(def->cpu->features[i].name, "vmx")) ||
> +                                (svm && STREQ(def->cpu->features[i].name, "svm")))
> +                                continue;
> +
> +                            snprintf(xlCPU,
> +                                    sizeof(xlCPU),
> +                                    "%s=1",
> +                                    xenTranslateCPUFeature(
> +                                        def->cpu->features[i].name, false));
> +                            if (libxl_cpuid_parse_config(&b_info->cpuid, xlCPU)) {
> +                                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                                        _("unsupported cpu feature '%s'"),
> +                                        def->cpu->features[i].name);
> +                                return -1;
> +                            }
> +                            break;
>                           case VIR_CPU_FEATURE_OPTIONAL:
>                           case VIR_CPU_FEATURE_LAST:
>                               break;
> diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
> index 394cc0d..d5dff59 100644
> --- a/src/xenconfig/xen_xl.c
> +++ b/src/xenconfig/xen_xl.c
> @@ -218,6 +218,40 @@ xenParseXLOS(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps)
>       return 0;
>   }
>   
> +/*
> + * Translate CPU feature name from libvirt to libxl (from_libxl=false) or from
> + * libxl to libvirt (from_libxl=true).
> + */
> +const char *
> +xenTranslateCPUFeature(const char *feature_name, bool from_libxl)
> +{
> +    static const char *translation_table[][2] = {
> +        /* libvirt name, libxl name */
> +        { "cx16", "cmpxchg16" },
> +        { "cid", "cntxid" },
> +        { "ds_cpl", "dscpl" },
> +        { "pclmuldq", "pclmulqdq" },
> +        { "pni", "sse3" },
> +        { "ht", "htt" },
> +        { "pn", "psn" },
> +        { "clflush", "clfsh" },
> +        { "sep", "sysenter" },
> +        { "cx8", "cmpxchg8" },
> +        { "nodeid_msr", "nodeid" },
> +        { "cr8legacy", "altmovcr8" },
> +        { "lahf_lm", "lahfsahf" },
> +        { "cmp_legacy", "cmplegacy" },
> +        { "fxsr_opt", "ffxsr" },
> +        { "pdpe1gb", "page1gb" },
> +    };

In the case of ibrsb and stibp added in xen commit 0d703a701cc and spec-ctrl 
added in libvirt commit 8b605530e80, have we already encountered the drawback of 
this approach discussed in V3?

Regards,
Jim

> +    size_t i;
> +
> +    for (i = 0; i < ARRAY_CARDINALITY(translation_table); i++)
> +        if (STREQ(translation_table[i][from_libxl], feature_name))
> +            return translation_table[i][!from_libxl];
> +    return feature_name;
> +}
> +
>   
>   static int
>   xenParseXLSpice(virConfPtr conf, virDomainDefPtr def)
> diff --git a/src/xenconfig/xen_xl.h b/src/xenconfig/xen_xl.h
> index dd96326..68f332a 100644
> --- a/src/xenconfig/xen_xl.h
> +++ b/src/xenconfig/xen_xl.h
> @@ -33,4 +33,6 @@ virDomainDefPtr xenParseXL(virConfPtr conn,
>   
>   virConfPtr xenFormatXL(virDomainDefPtr def, virConnectPtr);
>   
> +const char *xenTranslateCPUFeature(const char *feature_name, bool from_libxl);
> +
>   #endif /* __VIR_XEN_XL_H__ */
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 5/8] libxl: add support for CPUID features policy
Posted by Marek Marczykowski-Górecki 7 years, 2 months ago
On Mon, Feb 26, 2018 at 04:08:44PM -0700, Jim Fehlig wrote:
> On 02/08/2018 03:58 PM, Marek Marczykowski-Górecki wrote:
> > +const char *
> > +xenTranslateCPUFeature(const char *feature_name, bool from_libxl)
> > +{
> > +    static const char *translation_table[][2] = {
> > +        /* libvirt name, libxl name */
> > +        { "cx16", "cmpxchg16" },
> > +        { "cid", "cntxid" },
> > +        { "ds_cpl", "dscpl" },
> > +        { "pclmuldq", "pclmulqdq" },
> > +        { "pni", "sse3" },
> > +        { "ht", "htt" },
> > +        { "pn", "psn" },
> > +        { "clflush", "clfsh" },
> > +        { "sep", "sysenter" },
> > +        { "cx8", "cmpxchg8" },
> > +        { "nodeid_msr", "nodeid" },
> > +        { "cr8legacy", "altmovcr8" },
> > +        { "lahf_lm", "lahfsahf" },
> > +        { "cmp_legacy", "cmplegacy" },
> > +        { "fxsr_opt", "ffxsr" },
> > +        { "pdpe1gb", "page1gb" },
> > +    };
> 
> In the case of ibrsb and stibp added in xen commit 0d703a701cc and spec-ctrl
> added in libvirt commit 8b605530e80, have we already encountered the
> drawback of this approach discussed in V3?

Great... I'll add those names to the list...
Anyway I think this is still better approach than the alternative.
Unless someone want to extend the CPU API with single bit lookup...

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list