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
