Re: [PATCH V2 1/4] conf: Add support for specifying CPU max physical address size

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 7/29/22 21:34, Jim Fehlig wrote:
> From: Dario Faggioli <dfaggioli@xxxxxxxx>
> 
> This patch introduces the
> 
>     <maxphysaddr mode='passthrough'/>
>     <maxphysaddr mode='emulate' bits='42'/>
> 
> sub element of /domain/cpu, which allows specifying the guest virtual CPU
> address size. This can be useful if the guest needs to have a large amount
> of memory.
> 
> If mode='passthrough', the virtual CPU will have the same number of address
> bits as the host. If mode='emulate', the mandatory bits attribute specifies
> the number of address bits.
> 
> Signed-off-by: Dario Faggioli <dfaggioli@xxxxxxxx>
> Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
> ---
>  docs/formatdomain.rst                         | 23 ++++++++
>  src/conf/cpu_conf.c                           | 54 +++++++++++++++++++
>  src/conf/cpu_conf.h                           | 17 ++++++
>  src/conf/schemas/cputypes.rng                 | 19 +++++++
>  src/libvirt_private.syms                      |  2 +
>  .../cpu-phys-bits-emulate.xml                 | 20 +++++++
>  .../cpu-phys-bits-passthrough.xml             | 20 +++++++
>  tests/genericxml2xmltest.c                    |  3 ++
>  8 files changed, 158 insertions(+)
> 
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index 1ed969ac3e..adfdd7b7a5 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -1336,6 +1336,7 @@ following collection of elements. :since:`Since 0.7.5`
>       <vendor>Intel</vendor>
>       <topology sockets='1' dies='1' cores='2' threads='1'/>
>       <cache level='3' mode='emulate'/>
> +     <maxphysaddr mode='emulate' bits='42'>
>       <feature policy='disable' name='lahf_lm'/>
>     </cpu>
>     ...
> @@ -1352,6 +1353,7 @@ following collection of elements. :since:`Since 0.7.5`
>  
>     <cpu mode='host-passthrough' migratable='off'>
>       <cache mode='passthrough'/>
> +     <maxphysaddr mode='passthrough'>
>       <feature policy='disable' name='lahf_lm'/>
>     ...
>  
> @@ -1600,6 +1602,27 @@ In case no restrictions need to be put on CPU model and its features, a simpler
>           The virtual CPU will report no CPU cache of the specified level (or no
>           cache at all if the ``level`` attribute is missing).
>  
> +``maxphysaddr``
> +   :since:`Since 8.7.0` the ``maxphysaddr`` element describes the virtual CPU
> +   address size in bits. The hypervisor default is used if the element is missing.
> +
> +   ``mode``
> +      This mandatory attribute specifies how the address size is presented. The
> +      follow modes are supported:
> +
> +      ``passthrough``
> +         The number of physical address bits reported by the host CPU will be
> +         passed through to the virtual CPUs
> +      ``emulate``
> +         The hypervisor will define a specific value for the number of bits
> +         of physical addresses via the ``bits`` attribute, which is mandatory.
> +	 The number of bits cannot exceed the number of physical address bits
> +	 supported by the hypervisor.
> +
> +   ``bits``
> +      The ``bits`` attribute is mandatory if the ``mode`` attribute is set to
> +      ``emulate`` and specifies the virtual CPU address size in bits.
> +
>  Guest NUMA topology can be specified using the ``numa`` element. :since:`Since
>  0.9.8`
>  
> diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
> index 8d80bbd842..e31c4ab467 100644
> --- a/src/conf/cpu_conf.c
> +++ b/src/conf/cpu_conf.c
> @@ -82,6 +82,12 @@ VIR_ENUM_IMPL(virCPUCacheMode,
>                "disable",
>  );
>  
> +VIR_ENUM_IMPL(virCPUMaxPhysAddrMode,
> +              VIR_CPU_MAX_PHYS_ADDR_MODE_LAST,
> +              "emulate",
> +              "passthrough",
> +);
> +
>  
>  virCPUDef *virCPUDefNew(void)
>  {
> @@ -127,6 +133,7 @@ virCPUDefFree(virCPUDef *def)
>      if (g_atomic_int_dec_and_test(&def->refs)) {
>          virCPUDefFreeModel(def);
>          g_free(def->cache);
> +        g_free(def->addr);
>          g_free(def->tsc);
>          g_free(def);
>      }
> @@ -252,6 +259,11 @@ virCPUDefCopyWithoutModel(const virCPUDef *cpu)
>          *copy->cache = *cpu->cache;
>      }
>  
> +    if (cpu->addr) {
> +        copy->addr = g_new0(virCPUMaxPhysAddrDef, 1);
> +        *copy->addr = *cpu->addr;
> +    }
> +
>      if (cpu->tsc) {
>          copy->tsc = g_new0(virHostCPUTscInfo, 1);
>          *copy->tsc = *cpu->tsc;
> @@ -644,6 +656,39 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt,
>          def->cache->mode = mode;
>      }
>  
> +    if (virXPathInt("count(./maxphysaddr)", ctxt, &n) < 0) {
> +        return -1;
> +    } else if (n > 1) {
> +        virReportError(VIR_ERR_XML_ERROR, "%s",
> +                       _("at most one CPU maximum physical address bits "
> +                         "element may be specified"));
> +        return -1;
> +    } else if (n == 1) {
> +        g_autofree char *strmode = NULL;
> +        int mode;
> +        int bits = -1;
> +
> +        if (!(strmode = virXPathString("string(./maxphysaddr[1]/@mode)", ctxt)) ||
> +            (mode = virCPUMaxPhysAddrModeTypeFromString(strmode)) < 0) {
> +            virReportError(VIR_ERR_XML_ERROR, "%s",
> +                           _("missing or invalid CPU maximum physical "
> +                             "address bits mode"));
> +            return -1;
> +        }
> +
> +        if (virXPathBoolean("boolean(./maxphysaddr[1]/@bits)", ctxt) == 1 &&
> +            (virXPathInt("string(./maxphysaddr[1]/@bits)", ctxt, &bits) < 0 ||
> +             bits < 0)) {
> +            virReportError(VIR_ERR_XML_ERROR, "%s",
> +                           _("CPU maximum physical address bits < 0"));
> +            return -1;
> +        }

We have a bit better option here: virXMLProp*().
Which reminds me, the validation (which you put into PostParse callback
in patch 3/4) should be moved into this patch.

Therefore, I suggest squashing this in:

diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index e31c4ab467..d385d76e23 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -334,6 +334,7 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt,
     g_autoptr(virCPUDef) def = NULL;
     g_autofree xmlNodePtr *nodes = NULL;
     xmlNodePtr topology = NULL;
+    xmlNodePtr maxphysaddrNode = NULL;
     VIR_XPATH_NODE_AUTORESTORE(ctxt)
     int n;
     int rv;
@@ -656,37 +657,19 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt,
         def->cache->mode = mode;
     }
 
-    if (virXPathInt("count(./maxphysaddr)", ctxt, &n) < 0) {
-        return -1;
-    } else if (n > 1) {
-        virReportError(VIR_ERR_XML_ERROR, "%s",
-                       _("at most one CPU maximum physical address bits "
-                         "element may be specified"));
-        return -1;
-    } else if (n == 1) {
-        g_autofree char *strmode = NULL;
-        int mode;
-        int bits = -1;
-
-        if (!(strmode = virXPathString("string(./maxphysaddr[1]/@mode)", ctxt)) ||
-            (mode = virCPUMaxPhysAddrModeTypeFromString(strmode)) < 0) {
-            virReportError(VIR_ERR_XML_ERROR, "%s",
-                           _("missing or invalid CPU maximum physical "
-                             "address bits mode"));
-            return -1;
-        }
-
-        if (virXPathBoolean("boolean(./maxphysaddr[1]/@bits)", ctxt) == 1 &&
-            (virXPathInt("string(./maxphysaddr[1]/@bits)", ctxt, &bits) < 0 ||
-             bits < 0)) {
-            virReportError(VIR_ERR_XML_ERROR, "%s",
-                           _("CPU maximum physical address bits < 0"));
-            return -1;
-        }
-
+    if ((maxphysaddrNode = virXPathNode("./maxphysaddr[1]", ctxt))) {
         def->addr = g_new0(virCPUMaxPhysAddrDef, 1);
-        def->addr->bits = bits;
-        def->addr->mode = mode;
+
+        if (virXMLPropEnum(maxphysaddrNode, "mode",
+                           virCPUMaxPhysAddrModeTypeFromString,
+                           VIR_XML_PROP_REQUIRED,
+                           &def->addr->mode) < 0)
+            return -1;
+
+        if (virXMLPropInt(maxphysaddrNode, "bits", 10,
+                          VIR_XML_PROP_NONNEGATIVE,
+                          &def->addr->bits, -1) < 0)
+            return -1;
     }
 
     *cpu = g_steal_pointer(&def);
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 7fa899e411..36e236f08e 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -332,6 +332,47 @@ qemuValidateDomainDefCpu(virQEMUDriver *driver,
     if (!cpu)
         return 0;
 
+    if (cpu->addr) {
+        const virCPUMaxPhysAddrDef *addr = cpu->addr;
+
+        if (!ARCH_IS_X86(def->os.arch)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("CPU maximum physical address bits specification is not supported for '%s' architecture"),
+                           virArchToString(def->os.arch));
+            return -1;
+        }
+
+        switch (addr->mode) {
+        case VIR_CPU_MAX_PHYS_ADDR_MODE_PASSTHROUGH:
+            if (def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                               _("CPU maximum physical address bits mode '%s' can only be used with '%s' CPUs"),
+                               virCPUMaxPhysAddrModeTypeToString(addr->mode),
+                               virCPUModeTypeToString(VIR_CPU_MODE_HOST_PASSTHROUGH));
+                return -1;
+            }
+            if (addr->bits != -1) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                               _("CPU maximum physical address bits number specification cannot be used with mode='%s'"),
+                               virCPUMaxPhysAddrModeTypeToString(VIR_CPU_MAX_PHYS_ADDR_MODE_PASSTHROUGH));
+                return -1;
+            }
+            break;
+
+        case VIR_CPU_MAX_PHYS_ADDR_MODE_EMULATE:
+            if (addr->bits == -1) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                               _("if using CPU maximum physical address mode='%s', bits= must be specified too"),
+                               virCPUMaxPhysAddrModeTypeToString(VIR_CPU_MAX_PHYS_ADDR_MODE_EMULATE));
+                return -1;
+            }
+            break;
+
+        case VIR_CPU_MAX_PHYS_ADDR_MODE_LAST:
+            break;
+        }
+    }
+
     if (def->cpu->cache) {
         virCPUCacheDef *cache = def->cpu->cache;
 
diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c
index bbe0d02226..3501eadf55 100644
--- a/tests/genericxml2xmltest.c
+++ b/tests/genericxml2xmltest.c
@@ -249,7 +249,6 @@ mymain(void)
     DO_TEST("cpu-phys-bits-emulate");
     DO_TEST("cpu-phys-bits-passthrough");
 
-
     virObjectUnref(caps);
     virObjectUnref(xmlopt);
 

Michal




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux