[PATCH 1/1] set vm physical bits(phys_bits)
Set the vm phys_bits through the phys and hostphysbits in XML <phys bits='43' /> corresponds to "-cpu-phys-bits=42" <hostphysbits /> corresponds to "host-phys-bits=on" <cpu mode='host-passthrough' check='none'> <phys bits='43' /> <hostphysbits /> </cpu> --- src/conf/cpu_conf.c | 34 ++++++++++++++++++++++++++++++++++ src/conf/cpu_conf.h | 2 ++ src/qemu/qemu_command.c | 6 ++++++ 3 files changed, 42 insertions(+) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 380a74691d..41f7c26f63 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -158,6 +158,8 @@ virCPUDefCopyModelFilter(virCPUDefPtr dst, dst->model = g_strdup(src->model); dst->vendor = g_strdup(src->vendor); dst->vendor_id = g_strdup(src->vendor_id); + dst->phys_bits = src->phys_bits; + dst->host_phys_bits = src->host_phys_bits; dst->microcodeVersion = src->microcodeVersion; dst->nfeatures_max = src->nfeatures; dst->nfeatures = 0; @@ -540,6 +542,18 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt, return -1; } + if (virXPathNode("./phys[1]", ctxt)) { + unsigned long phys_bits; + if (virXPathULong("string(./phys[1]/@bits)", + ctxt, &phys_bits) >=0 ) { + def->phys_bits = (unsigned int) phys_bits; + } + } + + if (virXPathNode("./hostphysbits[1]", ctxt)) { + def->host_phys_bits = true; + } + if (virXPathNode("./topology[1]", ctxt)) { unsigned long ul; @@ -811,6 +825,12 @@ virCPUDefFormatBuf(virBufferPtr buf, virBufferAddLit(buf, "/>\n"); } + if (def->phys_bits > 0) + virBufferAsprintf(buf, "<phys bits='%u' />\n", def->phys_bits); + + if (def->host_phys_bits) + virBufferAddLit(buf, "<hostphysbits />\n"); + if (def->sockets && def->dies && def->cores && def->threads) { virBufferAddLit(buf, "<topology"); virBufferAsprintf(buf, " sockets='%u'", def->sockets); @@ -1067,6 +1087,20 @@ virCPUDefIsEqual(virCPUDefPtr src, return false; } + if (src->phys_bits != dst->phys_bits) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target CPU phys_bits %d does not match source %d"), + dst->phys_bits, src->phys_bits); + goto cleanup; + } + + if (src->host_phys_bits != dst->host_phys_bits) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target CPU host_phys_bits %d does not match source %d"), + dst->host_phys_bits, src->host_phys_bits); + goto cleanup; + } + if (src->sockets != dst->sockets) { MISMATCH(_("Target CPU sockets %d does not match source %d"), dst->sockets, src->sockets); diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h index 7ab198d370..f2a23ad41e 100644 --- a/src/conf/cpu_conf.h +++ b/src/conf/cpu_conf.h @@ -132,6 +132,8 @@ struct _virCPUDef { char *vendor_id; /* vendor id returned by CPUID in the guest */ int fallback; /* enum virCPUFallback */ char *vendor; + unsigned int phys_bits; + bool host_phys_bits; unsigned int microcodeVersion; unsigned int sockets; unsigned int dies; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1b4fa77867..d9bf3d5ce8 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6729,6 +6729,12 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, virBufferAddLit(&buf, ",l3-cache=off"); } + if (def->cpu && def->cpu->phys_bits > 0) + virBufferAsprintf(&buf, ",phys-bits=%u", def->cpu->phys_bits); + + if (def->cpu && def->cpu->host_phys_bits) + virBufferAddLit(&buf, ",host-phys-bits=on"); + cpu = virBufferContentAndReset(&cpu_buf); cpu_flags = virBufferContentAndReset(&buf); -- 2.24.3
On Wed, 2021-03-24 at 11:01 +0000, Wang,Liang(ACG CCN) wrote:
Set the vm phys_bits through the phys and hostphysbits in XML <phys bits='43' /> corresponds to "-cpu-phys-bits=42" <hostphysbits /> corresponds to "host-phys-bits=on"
<cpu mode='host-passthrough' check='none'> <phys bits='43' /> <hostphysbits /> </cpu>
Please don't forget to change the RNG schema accordingly, see docs/schema/cputypes.rng.
--- src/conf/cpu_conf.c | 34 ++++++++++++++++++++++++++++++++++ src/conf/cpu_conf.h | 2 ++ src/qemu/qemu_command.c | 6 ++++++ 3 files changed, 42 insertions(+)
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 380a74691d..41f7c26f63 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -158,6 +158,8 @@ virCPUDefCopyModelFilter(virCPUDefPtr dst, dst->model = g_strdup(src->model); dst->vendor = g_strdup(src->vendor); dst->vendor_id = g_strdup(src->vendor_id); + dst->phys_bits = src->phys_bits; + dst->host_phys_bits = src->host_phys_bits; dst->microcodeVersion = src->microcodeVersion; dst->nfeatures_max = src->nfeatures; dst->nfeatures = 0; @@ -540,6 +542,18 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt, return -1; }
+ if (virXPathNode("./phys[1]", ctxt)) { + unsigned long phys_bits; + if (virXPathULong("string(./phys[1]/@bits)", + ctxt, &phys_bits) >=0 ) { + def->phys_bits = (unsigned int) phys_bits;
I think you can use "virXPathUInt" here.
+ } + } + + if (virXPathNode("./hostphysbits[1]", ctxt)) { + def->host_phys_bits = true; + } + if (virXPathNode("./topology[1]", ctxt)) { unsigned long ul;
@@ -811,6 +825,12 @@ virCPUDefFormatBuf(virBufferPtr buf, virBufferAddLit(buf, "/>\n"); }
phys_bits);
+ if (def->phys_bits > 0) + virBufferAsprintf(buf, "<phys bits='%u' />\n", def- + + if (def->host_phys_bits) + virBufferAddLit(buf, "<hostphysbits />\n"); + if (def->sockets && def->dies && def->cores && def->threads) { virBufferAddLit(buf, "<topology"); virBufferAsprintf(buf, " sockets='%u'", def->sockets); @@ -1067,6 +1087,20 @@ virCPUDefIsEqual(virCPUDefPtr src, return false; }
+ if (src->phys_bits != dst->phys_bits) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target CPU phys_bits %d does not match source %d"), + dst->phys_bits, src->phys_bits); + goto cleanup; + } + + if (src->host_phys_bits != dst->host_phys_bits) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target CPU host_phys_bits %d does not match source %d"), + dst->host_phys_bits, src->host_phys_bits); + goto cleanup; + } + if (src->sockets != dst->sockets) { MISMATCH(_("Target CPU sockets %d does not match source %d"), dst->sockets, src->sockets); diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h index 7ab198d370..f2a23ad41e 100644 --- a/src/conf/cpu_conf.h +++ b/src/conf/cpu_conf.h @@ -132,6 +132,8 @@ struct _virCPUDef { char *vendor_id; /* vendor id returned by CPUID in the guest */ int fallback; /* enum virCPUFallback */ char *vendor; + unsigned int phys_bits; + bool host_phys_bits; unsigned int microcodeVersion; unsigned int sockets; unsigned int dies; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1b4fa77867..d9bf3d5ce8 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6729,6 +6729,12 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, virBufferAddLit(&buf, ",l3-cache=off"); }
phys_bits);
+ if (def->cpu && def->cpu->phys_bits > 0) + virBufferAsprintf(&buf, ",phys-bits=%u", def->cpu- + + if (def->cpu && def->cpu->host_phys_bits) + virBufferAddLit(&buf, ",host-phys-bits=on"); + cpu = virBufferContentAndReset(&cpu_buf); cpu_flags = virBufferContentAndReset(&buf);
-- 2.24.3
Set the vm phys_bits through the phys and hostphysbits in XML <phys bits='43' /> corresponds to "-cpu-phys-bits=42" <hostphysbits /> corresponds to "host-phys-bits=on" <cpu mode='host-passthrough' check='none'> <phys bits='43' /> <hostphysbits /> </cpu> --- docs/schemas/cputypes.rng | 20 ++++++++++++++++++++ src/conf/cpu_conf.c | 34 ++++++++++++++++++++++++++++++++++ src/conf/cpu_conf.h | 2 ++ src/qemu/qemu_command.c | 6 ++++++ 4 files changed, 62 insertions(+) diff --git a/docs/schemas/cputypes.rng b/docs/schemas/cputypes.rng index 77c8fa783b..fb8a14ddea 100644 --- a/docs/schemas/cputypes.rng +++ b/docs/schemas/cputypes.rng @@ -300,6 +300,20 @@ </element> </define> + <define name="cpuPhysBits"> + <element name="phys"> + <attribute name="bits"> + <ref name="positiveInteger"/> + </attribute> + </element> + </define> + + <define name="cpuHostPhysBits"> + <element name="hostphysbits"> + <empty/> + </element> + </define> + <define name="hostcpu"> <element name="cpu"> <element name="arch"> @@ -414,6 +428,12 @@ <optional> <ref name="cpuCache"/> </optional> + <optional> + <ref name="cpuPhysBits"/> + </optional> + <optional> + <ref name="cpuHostPhysBits"/> + </optional> </interleave> </element> </define> diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 380a74691d..24b0fa67ef 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -158,6 +158,8 @@ virCPUDefCopyModelFilter(virCPUDefPtr dst, dst->model = g_strdup(src->model); dst->vendor = g_strdup(src->vendor); dst->vendor_id = g_strdup(src->vendor_id); + dst->phys_bits = src->phys_bits; + dst->host_phys_bits = src->host_phys_bits; dst->microcodeVersion = src->microcodeVersion; dst->nfeatures_max = src->nfeatures; dst->nfeatures = 0; @@ -540,6 +542,18 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt, return -1; } + if (virXPathNode("./phys[1]", ctxt)) { + unsigned long phys_bits; + if (virXPathUInt("string(./phys[1]/@bits)", + ctxt, &phys_bits) >=0 ) { + def->phys_bits = (unsigned int) phys_bits; + } + } + + if (virXPathNode("./hostphysbits[1]", ctxt)) { + def->host_phys_bits = true; + } + if (virXPathNode("./topology[1]", ctxt)) { unsigned long ul; @@ -811,6 +825,12 @@ virCPUDefFormatBuf(virBufferPtr buf, virBufferAddLit(buf, "/>\n"); } + if (def->phys_bits > 0) + virBufferAsprintf(buf, "<phys bits='%u' />\n", def->phys_bits); + + if (def->host_phys_bits) + virBufferAddLit(buf, "<hostphysbits />\n"); + if (def->sockets && def->dies && def->cores && def->threads) { virBufferAddLit(buf, "<topology"); virBufferAsprintf(buf, " sockets='%u'", def->sockets); @@ -1067,6 +1087,20 @@ virCPUDefIsEqual(virCPUDefPtr src, return false; } + if (src->phys_bits != dst->phys_bits) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target CPU phys_bits %d does not match source %d"), + dst->phys_bits, src->phys_bits); + goto cleanup; + } + + if (src->host_phys_bits != dst->host_phys_bits) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target CPU host_phys_bits %d does not match source %d"), + dst->host_phys_bits, src->host_phys_bits); + goto cleanup; + } + if (src->sockets != dst->sockets) { MISMATCH(_("Target CPU sockets %d does not match source %d"), dst->sockets, src->sockets); diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h index 7ab198d370..f2a23ad41e 100644 --- a/src/conf/cpu_conf.h +++ b/src/conf/cpu_conf.h @@ -132,6 +132,8 @@ struct _virCPUDef { char *vendor_id; /* vendor id returned by CPUID in the guest */ int fallback; /* enum virCPUFallback */ char *vendor; + unsigned int phys_bits; + bool host_phys_bits; unsigned int microcodeVersion; unsigned int sockets; unsigned int dies; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1b4fa77867..d9bf3d5ce8 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6729,6 +6729,12 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, virBufferAddLit(&buf, ",l3-cache=off"); } + if (def->cpu && def->cpu->phys_bits > 0) + virBufferAsprintf(&buf, ",phys-bits=%u", def->cpu->phys_bits); + + if (def->cpu && def->cpu->host_phys_bits) + virBufferAddLit(&buf, ",host-phys-bits=on"); + cpu = virBufferContentAndReset(&cpu_buf); cpu_flags = virBufferContentAndReset(&buf); -- 2.24.3 (Apple Git-128) On Wed, Mar 24, 2021 at 8:53 PM Tim Wiederhake <twiederh@redhat.com> wrote:
On Wed, 2021-03-24 at 11:01 +0000, Wang,Liang(ACG CCN) wrote:
Set the vm phys_bits through the phys and hostphysbits in XML <phys bits='43' /> corresponds to "-cpu-phys-bits=42" <hostphysbits /> corresponds to "host-phys-bits=on"
<cpu mode='host-passthrough' check='none'> <phys bits='43' /> <hostphysbits /> </cpu>
Please don't forget to change the RNG schema accordingly, see docs/schema/cputypes.rng.
--- src/conf/cpu_conf.c | 34 ++++++++++++++++++++++++++++++++++ src/conf/cpu_conf.h | 2 ++ src/qemu/qemu_command.c | 6 ++++++ 3 files changed, 42 insertions(+)
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 380a74691d..41f7c26f63 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -158,6 +158,8 @@ virCPUDefCopyModelFilter(virCPUDefPtr dst, dst->model = g_strdup(src->model); dst->vendor = g_strdup(src->vendor); dst->vendor_id = g_strdup(src->vendor_id); + dst->phys_bits = src->phys_bits; + dst->host_phys_bits = src->host_phys_bits; dst->microcodeVersion = src->microcodeVersion; dst->nfeatures_max = src->nfeatures; dst->nfeatures = 0; @@ -540,6 +542,18 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt, return -1; }
+ if (virXPathNode("./phys[1]", ctxt)) { + unsigned long phys_bits; + if (virXPathULong("string(./phys[1]/@bits)", + ctxt, &phys_bits) >=0 ) { + def->phys_bits = (unsigned int) phys_bits;
I think you can use "virXPathUInt" here.
+ } + } + + if (virXPathNode("./hostphysbits[1]", ctxt)) { + def->host_phys_bits = true; + } + if (virXPathNode("./topology[1]", ctxt)) { unsigned long ul;
@@ -811,6 +825,12 @@ virCPUDefFormatBuf(virBufferPtr buf, virBufferAddLit(buf, "/>\n"); }
phys_bits);
+ if (def->phys_bits > 0) + virBufferAsprintf(buf, "<phys bits='%u' />\n", def- + + if (def->host_phys_bits) + virBufferAddLit(buf, "<hostphysbits />\n"); + if (def->sockets && def->dies && def->cores && def->threads) { virBufferAddLit(buf, "<topology"); virBufferAsprintf(buf, " sockets='%u'", def->sockets); @@ -1067,6 +1087,20 @@ virCPUDefIsEqual(virCPUDefPtr src, return false; }
+ if (src->phys_bits != dst->phys_bits) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target CPU phys_bits %d does not match source %d"), + dst->phys_bits, src->phys_bits); + goto cleanup; + } + + if (src->host_phys_bits != dst->host_phys_bits) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target CPU host_phys_bits %d does not match source %d"), + dst->host_phys_bits, src->host_phys_bits); + goto cleanup; + } + if (src->sockets != dst->sockets) { MISMATCH(_("Target CPU sockets %d does not match source %d"), dst->sockets, src->sockets); diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h index 7ab198d370..f2a23ad41e 100644 --- a/src/conf/cpu_conf.h +++ b/src/conf/cpu_conf.h @@ -132,6 +132,8 @@ struct _virCPUDef { char *vendor_id; /* vendor id returned by CPUID in the guest */ int fallback; /* enum virCPUFallback */ char *vendor; + unsigned int phys_bits; + bool host_phys_bits; unsigned int microcodeVersion; unsigned int sockets; unsigned int dies; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1b4fa77867..d9bf3d5ce8 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6729,6 +6729,12 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, virBufferAddLit(&buf, ",l3-cache=off"); }
phys_bits);
+ if (def->cpu && def->cpu->phys_bits > 0) + virBufferAsprintf(&buf, ",phys-bits=%u", def->cpu- + + if (def->cpu && def->cpu->host_phys_bits) + virBufferAddLit(&buf, ",host-phys-bits=on"); + cpu = virBufferContentAndReset(&cpu_buf); cpu_flags = virBufferContentAndReset(&buf);
-- 2.24.3
On 3/25/21 2:24 AM, Paul Schlacter wrote:
Set the vm phys_bits through the phys and hostphysbits in XML
<phys bits='43' /> corresponds to "-cpu-phys-bits=42"
<hostphysbits /> corresponds to "host-phys-bits=on"
<cpu mode='host-passthrough' check='none'>
<phys bits='43' />
<hostphysbits />
</cpu>
---
docs/schemas/cputypes.rng | 20 ++++++++++++++++++++
src/conf/cpu_conf.c | 34 ++++++++++++++++++++++++++++++++++
src/conf/cpu_conf.h |2 ++
src/qemu/qemu_command.c |6 ++++++
4 files changed, 62 insertions(+)
I'm sorry, but it appears that your MTA is broken, because this patch doesn't apply on the top of master. Either git-send-email or git-publish are known to work well. https://github.com/stefanha/git-publish Also, if this is a v2 it's perfectly okay to start a new thread and state that it is a v2 of $url. For instance like this: https://listman.redhat.com/archives/libvir-list/2021-March/msg00607.html Anyway, what's still missing is documentation - how should users know that: a) it is possible to change this, b) what version this was introduced in? For instance like this: https://listman.redhat.com/archives/libvir-list/2021-March/msg00930.html Also, a test case would be nice. For both qemuxml2argvtest and qemuxml2xmltest - so that we know that XML parser/formatter works and cmd line generator works too. As a bonus, once added to git virschematest will check if XML matches schema.
diff --git a/docs/schemas/cputypes.rng b/docs/schemas/cputypes.rng
index 77c8fa783b..fb8a14ddea 100644
--- a/docs/schemas/cputypes.rng
+++ b/docs/schemas/cputypes.rng
@@ -300,6 +300,20 @@
</element>
</define>
+<define name="cpuPhysBits">
+<element name="phys">
+<attribute name="bits">
+<ref name="positiveInteger"/>
+</attribute>
+</element>
+</define>
+
+<define name="cpuHostPhysBits">
+<element name="hostphysbits">
+<empty/>
+</element>
+</define>
+
<define name="hostcpu">
<element name="cpu">
<element name="arch">
@@ -414,6 +428,12 @@
<optional>
<ref name="cpuCache"/>
</optional>
+<optional>
+<ref name="cpuPhysBits"/>
+</optional>
+<optional>
+<ref name="cpuHostPhysBits"/>
+</optional>
</interleave>
</element>
</define>
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 380a74691d..24b0fa67ef 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -158,6 +158,8 @@ virCPUDefCopyModelFilter(virCPUDefPtr dst,
dst->model = g_strdup(src->model);
dst->vendor = g_strdup(src->vendor);
dst->vendor_id = g_strdup(src->vendor_id);
+dst->phys_bits = src->phys_bits;
+dst->host_phys_bits = src->host_phys_bits;
dst->microcodeVersion = src->microcodeVersion;
dst->nfeatures_max = src->nfeatures;
dst->nfeatures = 0;
Doesn't virCPUDefStealModel() need the same treatment?
@@ -540,6 +542,18 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt,
return -1;
}
+if (virXPathNode("./phys[1]", ctxt)) {
+unsigned long phys_bits;
+if (virXPathUInt("string(./phys[1]/@bits)",
+ctxt, &phys_bits) >=0 ) {
+def->phys_bits = (unsigned int) phys_bits;
+}
+}
+
+if (virXPathNode("./hostphysbits[1]", ctxt)) {
+def->host_phys_bits = true;
+}
+
if (virXPathNode("./topology[1]", ctxt)) {
unsigned long ul;
@@ -811,6 +825,12 @@ virCPUDefFormatBuf(virBufferPtr buf,
virBufferAddLit(buf, "/>\n");
}
+if (def->phys_bits > 0)
+virBufferAsprintf(buf, "<phys bits='%u' />\n", def->phys_bits);
+
+if (def->host_phys_bits)
+virBufferAddLit(buf, "<hostphysbits />\n");
+
if (def->sockets && def->dies && def->cores && def->threads) {
virBufferAddLit(buf, "<topology");
virBufferAsprintf(buf, " sockets='%u'", def->sockets);
@@ -1067,6 +1087,20 @@ virCPUDefIsEqual(virCPUDefPtr src,
return false;
}
+if (src->phys_bits != dst->phys_bits) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Target CPU phys_bits %d does not match source %d"),
+ dst->phys_bits, src->phys_bits);
+goto cleanup;
+}
+
+if (src->host_phys_bits != dst->host_phys_bits) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Target CPU host_phys_bits %d does not match source %d"),
+ dst->host_phys_bits, src->host_phys_bits);
+goto cleanup;
+}
+
if (src->sockets != dst->sockets) {
MISMATCH(_("Target CPU sockets %d does not match source %d"),
dst->sockets, src->sockets);
diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h
index 7ab198d370..f2a23ad41e 100644
--- a/src/conf/cpu_conf.h
+++ b/src/conf/cpu_conf.h
@@ -132,6 +132,8 @@ struct _virCPUDef {
char *vendor_id;/* vendor id returned by CPUID in the guest */
int fallback; /* enum virCPUFallback */
char *vendor;
+unsigned int phys_bits;
+bool host_phys_bits;
unsigned int microcodeVersion;
unsigned int sockets;
unsigned int dies;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1b4fa77867..d9bf3d5ce8 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6729,6 +6729,12 @@ qemuBuildCpuCommandLine(virCommandPtr cmd,
virBufferAddLit(&buf, ",l3-cache=off");
}
+if (def->cpu && def->cpu->phys_bits > 0)
+virBufferAsprintf(&buf, ",phys-bits=%u", def->cpu->phys_bits);
+
+if (def->cpu && def->cpu->host_phys_bits)
+virBufferAddLit(&buf, ",host-phys-bits=on");
It's not that simple. phys-bits property was introduced to QEMU in v2.7.0-rc0~7^2~26 and host-phys-bits a few commits later in v2.7.0-rc0~7^2~21. The minimal version of QEMU we currently support is 1.5.0 (see QEMU_MIN_{MAJOR,MINOR,MICRO} in qemu_capabilities.c). Therefore, it is possible that we're constructing a command line for QEMU that doesn't have the properties. The way we detect this is so called QEMU capabilities. It is a bitmap where each bit has a name and is flipped on or off depending whether QEMU supports corresponding feature. And I think we will need one and produce an error message if QEMU doesn't have the property. I think we can safely assume that either QEMU has both properties or none. I don't think there is a downstream maintainer that would backport only one of the properties.
+
cpu = virBufferContentAndReset(&cpu_buf);
cpu_flags = virBufferContentAndReset(&buf);
--
Looking forward to v3. Michal
Set the vm phys_bits through the phys and hostphysbits in XML
<phys bits='43' /> corresponds to "-cpu-phys-bits=42"
<hostphysbits /> corresponds to "host-phys-bits=on"
<cpu mode='host-passthrough' check='none'>
<phys bits='43' />
<hostphysbits />
</cpu>
---
docs/schemas/cputypes.rng | 20 ++++++++++++++++++++
src/conf/cpu_conf.c | 34 ++++++++++++++++++++++++++++++++++
src/conf/cpu_conf.h | 2 ++
src/qemu/qemu_command.c | 6 ++++++
4 files changed, 62 insertions(+)
diff --git a/docs/schemas/cputypes.rng b/docs/schemas/cputypes.rng
index 77c8fa783b..fb8a14ddea 100644
--- a/docs/schemas/cputypes.rng
+++ b/docs/schemas/cputypes.rng
@@ -300,6 +300,20 @@
</element>
</define>
+ <define name="cpuPhysBits">
+ <element name="phys">
+ <attribute name="bits">
+ <ref name="positiveInteger"/>
+ </attribute>
+ </element>
+ </define>
+
+ <define name="cpuHostPhysBits">
+ <element name="hostphysbits">
+ <empty/>
+ </element>
+ </define>
+
<define name="hostcpu">
<element name="cpu">
<element name="arch">
@@ -414,6 +428,12 @@
<optional>
<ref name="cpuCache"/>
</optional>
+ <optional>
+ <ref name="cpuPhysBits"/>
+ </optional>
+ <optional>
+ <ref name="cpuHostPhysBits"/>
+ </optional>
</interleave>
</element>
</define>
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 380a74691d..18d788c528 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -158,6 +158,8 @@ virCPUDefCopyModelFilter(virCPUDefPtr dst,
dst->model = g_strdup(src->model);
dst->vendor = g_strdup(src->vendor);
dst->vendor_id = g_strdup(src->vendor_id);
+ dst->phys_bits = src->phys_bits;
+ dst->host_phys_bits = src->host_phys_bits;
dst->microcodeVersion = src->microcodeVersion;
dst->nfeatures_max = src->nfeatures;
dst->nfeatures = 0;
@@ -540,6 +542,18 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt,
return -1;
}
+ if (virXPathNode("./phys[1]", ctxt)) {
+ unsigned int phys_bits;
+ if (virXPathUInt("string(./phys[1]/@bits)",
+ ctxt, &phys_bits) >=0 ) {
+ def->phys_bits = (unsigned int) phys_bits;
+ }
+ }
+
+ if (virXPathNode("./hostphysbits[1]", ctxt)) {
+ def->host_phys_bits = true;
+ }
+
if (virXPathNode("./topology[1]", ctxt)) {
unsigned long ul;
@@ -811,6 +825,12 @@ virCPUDefFormatBuf(virBufferPtr buf,
virBufferAddLit(buf, "/>\n");
}
+ if (def->phys_bits > 0)
+ virBufferAsprintf(buf, "<phys bits='%u' />\n", def->phys_bits);
+
+ if (def->host_phys_bits)
+ virBufferAddLit(buf, "<hostphysbits />\n");
+
if (def->sockets && def->dies && def->cores && def->threads) {
virBufferAddLit(buf, "<topology");
virBufferAsprintf(buf, " sockets='%u'", def->sockets);
@@ -1067,6 +1087,20 @@ virCPUDefIsEqual(virCPUDefPtr src,
return false;
}
+ if (src->phys_bits != dst->phys_bits) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Target CPU phys_bits %d does not match source %d"),
+ dst->phys_bits, src->phys_bits);
+ goto cleanup;
+ }
+
+ if (src->host_phys_bits != dst->host_phys_bits) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Target CPU host_phys_bits %d does not match source %d"),
+ dst->host_phys_bits, src->host_phys_bits);
+ goto cleanup;
+ }
+
if (src->sockets != dst->sockets) {
MISMATCH(_("Target CPU sockets %d does not match source %d"),
dst->sockets, src->sockets);
diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h
index 7ab198d370..f2a23ad41e 100644
--- a/src/conf/cpu_conf.h
+++ b/src/conf/cpu_conf.h
@@ -132,6 +132,8 @@ struct _virCPUDef {
char *vendor_id; /* vendor id returned by CPUID in the guest */
int fallback; /* enum virCPUFallback */
char *vendor;
+ unsigned int phys_bits;
+ bool host_phys_bits;
unsigned int microcodeVersion;
unsigned int sockets;
unsigned int dies;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1b4fa77867..d9bf3d5ce8 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6729,6 +6729,12 @@ qemuBuildCpuCommandLine(virCommandPtr cmd,
virBufferAddLit(&buf, ",l3-cache=off");
}
+ if (def->cpu && def->cpu->phys_bits > 0)
+ virBufferAsprintf(&buf, ",phys-bits=%u", def->cpu->phys_bits);
+
+ if (def->cpu && def->cpu->host_phys_bits)
+ virBufferAddLit(&buf, ",host-phys-bits=on");
+
cpu = virBufferContentAndReset(&cpu_buf);
cpu_flags = virBufferContentAndReset(&buf);
--
2.24.3 (Apple Git-128)
On Thu, 2021-03-25 at 14:45 +0800, Paul Schlacter wrote:
Set the vm phys_bits through the phys and hostphysbits in XML <phys bits='43' /> corresponds to "-cpu-phys-bits=42" Is the 43 -> 42 change a typo? I do not see the "-1" in the code below.
<hostphysbits /> corresponds to "host-phys-bits=on"
<cpu mode='host-passthrough' check='none'> <phys bits='43' /> <hostphysbits />
This looks odd to me, I would have expected something like "<phys bits='43' hostphysbits='on'/>" or "<hostphysbits state='on'/>". Additionally, if this is qemu-specific and not applicable to other hypervisors, it might be better if this goes into the features/kvm section. Note that I am not in a position to decide on that and you need feedback from other libvirt maintainers.
</cpu> --- docs/schemas/cputypes.rng | 20 ++++++++++++++++++++ src/conf/cpu_conf.c | 34 ++++++++++++++++++++++++++++++++++ src/conf/cpu_conf.h | 2 ++ src/qemu/qemu_command.c | 6 ++++++ 4 files changed, 62 insertions(+)
diff --git a/docs/schemas/cputypes.rng b/docs/schemas/cputypes.rng index 77c8fa783b..fb8a14ddea 100644 --- a/docs/schemas/cputypes.rng +++ b/docs/schemas/cputypes.rng @@ -300,6 +300,20 @@ </element> </define>
+ <define name="cpuPhysBits"> + <element name="phys"> + <attribute name="bits"> + <ref name="positiveInteger"/> + </attribute> + </element> + </define> + + <define name="cpuHostPhysBits"> + <element name="hostphysbits"> + <empty/> + </element> + </define> + <define name="hostcpu"> <element name="cpu"> <element name="arch"> @@ -414,6 +428,12 @@ <optional> <ref name="cpuCache"/> </optional> + <optional> + <ref name="cpuPhysBits"/> + </optional> + <optional> + <ref name="cpuHostPhysBits"/> + </optional> </interleave> </element> </define> diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 380a74691d..18d788c528 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -158,6 +158,8 @@ virCPUDefCopyModelFilter(virCPUDefPtr dst, dst->model = g_strdup(src->model); dst->vendor = g_strdup(src->vendor); dst->vendor_id = g_strdup(src->vendor_id); + dst->phys_bits = src->phys_bits; + dst->host_phys_bits = src->host_phys_bits; dst->microcodeVersion = src->microcodeVersion; dst->nfeatures_max = src->nfeatures; dst->nfeatures = 0; @@ -540,6 +542,18 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt, return -1; }
+ if (virXPathNode("./phys[1]", ctxt)) { + unsigned int phys_bits; + if (virXPathUInt("string(./phys[1]/@bits)", + ctxt, &phys_bits) >=0 ) { Indentation looks off
+ def->phys_bits = (unsigned int) phys_bits; + } By using virXPathUint I believe you can directly pass def->phys_bits in and do not need the local variable any longer.
+ } + + if (virXPathNode("./hostphysbits[1]", ctxt)) { + def->host_phys_bits = true; + } + if (virXPathNode("./topology[1]", ctxt)) { unsigned long ul;
@@ -811,6 +825,12 @@ virCPUDefFormatBuf(virBufferPtr buf, virBufferAddLit(buf, "/>\n"); }
phys_bits);
+ if (def->phys_bits > 0) + virBufferAsprintf(buf, "<phys bits='%u' />\n", def- + + if (def->host_phys_bits) + virBufferAddLit(buf, "<hostphysbits />\n"); + if (def->sockets && def->dies && def->cores && def->threads) { virBufferAddLit(buf, "<topology"); virBufferAsprintf(buf, " sockets='%u'", def->sockets); @@ -1067,6 +1087,20 @@ virCPUDefIsEqual(virCPUDefPtr src, return false; }
+ if (src->phys_bits != dst->phys_bits) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target CPU phys_bits %d does not match source %d"), + dst->phys_bits, src->phys_bits); + goto cleanup; + } + + if (src->host_phys_bits != dst->host_phys_bits) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target CPU host_phys_bits %d does not match source %d"), + dst->host_phys_bits, src->host_phys_bits); + goto cleanup; + } + if (src->sockets != dst->sockets) { MISMATCH(_("Target CPU sockets %d does not match source %d"), dst->sockets, src->sockets); diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h index 7ab198d370..f2a23ad41e 100644 --- a/src/conf/cpu_conf.h +++ b/src/conf/cpu_conf.h @@ -132,6 +132,8 @@ struct _virCPUDef { char *vendor_id; /* vendor id returned by CPUID in the guest */ int fallback; /* enum virCPUFallback */ char *vendor; + unsigned int phys_bits; + bool host_phys_bits; unsigned int microcodeVersion; unsigned int sockets; unsigned int dies; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1b4fa77867..d9bf3d5ce8 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6729,6 +6729,12 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, virBufferAddLit(&buf, ",l3-cache=off"); }
phys_bits);
+ if (def->cpu && def->cpu->phys_bits > 0) + virBufferAsprintf(&buf, ",phys-bits=%u", def->cpu- + + if (def->cpu && def->cpu->host_phys_bits) + virBufferAddLit(&buf, ",host-phys-bits=on"); + cpu = virBufferContentAndReset(&cpu_buf); cpu_flags = virBufferContentAndReset(&buf);
-- 2.24.3 (Apple Git-128)
participants (4)
-
Michal Privoznik -
Paul Schlacter -
Tim Wiederhake -
Wang,Liang(ACG CCN)