[PATCH V2 3/4] qemu: Add support for max physical address size
Michal Prívozník
mprivozn at redhat.com
Wed Aug 3 15:29:08 UTC 2022
On 7/29/22 21:34, Jim Fehlig wrote:
> From: Dario Faggioli <dfaggioli at suse.com>
>
> This patch maps /domain/cpu/maxphysaddr into -cpu parameters:
>
> - <maxphysaddr mode='passthrough'/> becomes host-phys-bits=on
> - <maxphysaddr mode='emualte' bits='42'/> becomes phys-bits=42
>
> Passthrough mode can only be used if the chosen CPU model is
> 'host-passthrough'. Also validate that an explicitly specified
> bits value does not exceed the physical address bits on the host.
>
> The feature is available since QEMU 2.7.0.
>
> Signed-off-by: Dario Faggioli <dfaggioli at suse.com>
> Signed-off-by: Jim Fehlig <jfehlig at suse.com>
> ---
> src/qemu/qemu_command.c | 21 +++++++++
> src/qemu/qemu_domain.c | 46 +++++++++++++++++++
> src/qemu/qemu_validate.c | 12 +++++
> .../cpu-phys-bits-emulate.args | 32 +++++++++++++
> .../cpu-phys-bits-emulate.xml | 20 ++++++++
> .../cpu-phys-bits-emulate2.args | 32 +++++++++++++
> .../cpu-phys-bits-emulate2.xml | 20 ++++++++
> .../cpu-phys-bits-emulate3.err | 1 +
> .../cpu-phys-bits-emulate3.xml | 20 ++++++++
> .../cpu-phys-bits-passthrough.args | 32 +++++++++++++
> .../cpu-phys-bits-passthrough.xml | 20 ++++++++
> .../cpu-phys-bits-passthrough2.err | 1 +
> .../cpu-phys-bits-passthrough2.xml | 20 ++++++++
> .../cpu-phys-bits-passthrough3.err | 1 +
> .../cpu-phys-bits-passthrough3.xml | 20 ++++++++
> tests/qemuxml2argvtest.c | 7 +++
> 16 files changed, 305 insertions(+)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 30c9bbbf2e..acf214be00 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -6847,6 +6847,27 @@ qemuBuildCpuCommandLine(virCommand *cmd,
> virBufferAddLit(&buf, ",l3-cache=off");
> }
>
> + if (def->cpu && def->cpu->addr) {
> + virCPUMaxPhysAddrDef *addr = def->cpu->addr;
> +
> + switch (addr->mode) {
> + case VIR_CPU_MAX_PHYS_ADDR_MODE_PASSTHROUGH:
> + virBufferAddLit(&buf, ",host-phys-bits=on");
> + break;
> +
> + case VIR_CPU_MAX_PHYS_ADDR_MODE_EMULATE:
> + if (addr->bits != -1)
> + virBufferAsprintf(&buf, ",phys-bits=%d", addr->bits);
> + else
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("Physical address bits unspecified"));
Since we've checked for this case in validation step this situation
can't happen really. Our XML parsing happens in three steps:
1) actual parsing, i.e. using libxml (well, our wrappers over it) to
fill in internal structs according to passed XML document,
2) so called PostParse phase, where missing information is filled in,
3) validation phase, where we check whether data makes sense as whole,
whether underlying hypervisor has requested features, etc.
Now, steps 2) and 3) have consist of two parts each: hypervisor agnostic
part and hypervisor specific part. Because some info can be filled
in/validated regardless of underlying hypervisor and some require
knowledge of hypervisor features.
As usual, this is ideal state and if you run into older code it's likely
that it does some checking during parsing. In some cases this is fine
(e.g. rejecting an integer value out of range, failing parsing after
invalid enum2string conversion, etc.), but in others we like to move
validation into their respective functions. And for a brief moment we
used to misuse PostParse for validation too. Yes, it's a mess.
> + break;
> +
> + case VIR_CPU_MAX_PHYS_ADDR_MODE_LAST:
> + break;
> + }
> + }
> +
> cpu = virBufferContentAndReset(&cpu_buf);
> cpu_flags = virBufferContentAndReset(&buf);
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index b02ffc9a2e..0c97744b72 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -4429,6 +4429,52 @@ qemuDomainDefCPUPostParse(virDomainDef *def,
> }
> }
>
> + if (def->cpu->addr) {
> + virCPUMaxPhysAddrDef *addr = def->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;
> + }
> + }
> +
So this is the code I've mentioned earlier. Due to reasons above, this
should be in qemuValidateDomainDefCpu() (or ideally in a hypervisor
agnostic version, because neither of these checks are specific to QEMU,
but we don't have one yet).
> for (i = 0; i < def->cpu->nfeatures; i++) {
> virCPUFeatureDef *feature = &def->cpu->features[i];
>
> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index 764d5b029e..b6c7d3b06a 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -380,6 +380,18 @@ qemuValidateDomainDefCpu(virQEMUDriver *driver,
> break;
> }
>
> + if (cpu->addr &&
> + cpu->addr->mode == VIR_CPU_MAX_PHYS_ADDR_MODE_EMULATE &&
> + driver->hostcpu &&
> + driver->hostcpu->addr) {
> + if (cpu->addr->bits > driver->hostcpu->addr->bits) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("The number of virtual CPU address bits cannot "
> + "exceed the number supported by the host CPU"));
> + return -1;
> + }
And after the hunk above is moved here, this can be simplified to:
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 5664241dff..5d46dd1247 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -366,6 +366,14 @@ qemuValidateDomainDefCpu(virQEMUDriver *driver,
virCPUMaxPhysAddrModeTypeToString(VIR_CPU_MAX_PHYS_ADDR_MODE_EMULATE));
return -1;
}
+
+ if (driver->hostcpu &&
+ driver->hostcpu->addr &&
+ cpu->addr->bits > driver->hostcpu->addr->bits) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("The number of virtual CPU address bits cannot exceed the number supported by the host CPU"));
+ return -1;
+ }
break;
case VIR_CPU_MAX_PHYS_ADDR_MODE_LAST:
@@ -470,18 +478,6 @@ qemuValidateDomainDefCpu(virQEMUDriver *driver,
}
}
- if (cpu->addr &&
- cpu->addr->mode == VIR_CPU_MAX_PHYS_ADDR_MODE_EMULATE &&
- driver->hostcpu &&
- driver->hostcpu->addr) {
- if (cpu->addr->bits > driver->hostcpu->addr->bits) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("The number of virtual CPU address bits cannot "
- "exceed the number supported by the host CPU"));
- return -1;
- }
- }
-
return 0;
}
Michal
More information about the libvir-list
mailing list