[libvirt] [PATCH 0/4] TSEG v2

I forgot to note the list of changes against v1, so here goes an arbitrary list of some changes that I remember (can suck out of e-mail history): - SMM=on/off is allowed to be specified for i440fx - More use of virXMLFormatElement - Fixed some commit messages - Fixed wording and some details in docs - Don't record the size of extended TSEG in config XML if not explicitly requested Martin Kletzander (4): qemu: Move checks for SMM from command-line creation into validation phase conf, schema, docs: Add support for TSEG size setting qemu: Add capability flag for setting the extended tseg size qemu: Add support for setting the TSEG size docs/formatdomain.html.in | 48 ++++++++++++- docs/schemas/domaincommon.rng | 5 ++ src/conf/domain_conf.c | 65 +++++++++++++++++- src/conf/domain_conf.h | 3 + src/qemu/qemu_capabilities.c | 21 +++--- src/qemu/qemu_capabilities.h | 5 +- src/qemu/qemu_command.c | 30 ++++++--- src/qemu/qemu_domain.c | 50 +++++++++++++- tests/genericxml2xmlindata/tseg.xml | 23 +++++++ tests/genericxml2xmltest.c | 2 + .../caps_1.5.3.x86_64.replies | 38 +++++++++-- .../caps_1.5.3.x86_64.xml | 3 +- .../caps_1.6.0.x86_64.replies | 38 +++++++++-- .../caps_1.6.0.x86_64.xml | 3 +- .../caps_1.7.0.x86_64.replies | 38 +++++++++-- .../caps_1.7.0.x86_64.xml | 3 +- .../caps_2.1.1.x86_64.replies | 38 +++++++++-- .../caps_2.1.1.x86_64.xml | 3 +- .../caps_2.10.0.x86_64.replies | 48 ++++++++++--- .../caps_2.10.0.x86_64.xml | 3 +- .../caps_2.12.0.x86_64.replies | 67 +++++++++++++++---- .../caps_2.12.0.x86_64.xml | 4 +- .../caps_2.4.0.x86_64.replies | 38 +++++++++-- .../caps_2.4.0.x86_64.xml | 3 +- .../caps_2.5.0.x86_64.replies | 40 +++++++++-- .../caps_2.5.0.x86_64.xml | 3 +- .../caps_2.6.0.x86_64.replies | 40 +++++++++-- .../caps_2.6.0.x86_64.xml | 3 +- .../caps_2.7.0.x86_64.replies | 40 +++++++++-- .../caps_2.7.0.x86_64.xml | 3 +- .../caps_2.8.0.x86_64.replies | 40 +++++++++-- .../caps_2.8.0.x86_64.xml | 3 +- .../caps_2.9.0.x86_64.replies | 48 ++++++++++--- .../caps_2.9.0.x86_64.xml | 3 +- .../tseg-explicit-size.x86_64-latest.args | 35 ++++++++++ tests/qemuxml2argvdata/tseg-explicit-size.xml | 23 +++++++ tests/qemuxml2argvdata/tseg-i440fx.xml | 23 +++++++ tests/qemuxml2argvdata/tseg-invalid-size.xml | 23 +++++++ tests/qemuxml2argvtest.c | 25 +++++++ .../qemuxml2xmloutdata/tseg-explicit-size.xml | 46 +++++++++++++ .../tseg-old-machine-type.xml | 44 ++++++++++++ tests/qemuxml2xmloutdata/tseg.xml | 44 ++++++++++++ tests/qemuxml2xmltest.c | 9 +++ 43 files changed, 941 insertions(+), 133 deletions(-) create mode 100644 tests/genericxml2xmlindata/tseg.xml create mode 100644 tests/qemuxml2argvdata/tseg-explicit-size.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/tseg-explicit-size.xml create mode 100644 tests/qemuxml2argvdata/tseg-i440fx.xml create mode 100644 tests/qemuxml2argvdata/tseg-invalid-size.xml create mode 100644 tests/qemuxml2xmloutdata/tseg-explicit-size.xml create mode 100644 tests/qemuxml2xmloutdata/tseg-old-machine-type.xml create mode 100644 tests/qemuxml2xmloutdata/tseg.xml -- 2.17.1

We are still hoping all of such checks will be moved there and this is one small step in that direction. One of the things that this is improving is the fact that instead of error message *that was wrong) you get when starting a domain with SMM and i440fx we allow the setting to go through. SMM option exists and makes sense on i440fx as well (basically whenever that _SMM_OPT capability is set). Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_capabilities.c | 11 ----------- src/qemu/qemu_capabilities.h | 3 --- src/qemu/qemu_command.c | 12 ++---------- src/qemu/qemu_domain.c | 15 ++++++++++++--- 4 files changed, 14 insertions(+), 27 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 52178f0b0d86..5f48042ac752 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4773,17 +4773,6 @@ virQEMUCapsSupportsVmport(virQEMUCapsPtr qemuCaps, } -bool -virQEMUCapsSupportsSMM(virQEMUCapsPtr qemuCaps, - const virDomainDef *def) -{ - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_SMM_OPT)) - return false; - - return qemuDomainIsQ35(def); -} - - bool virQEMUCapsIsMachineSupported(virQEMUCapsPtr qemuCaps, const char *canonical_machine) diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index aad8f398caed..7e602049ca71 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -505,9 +505,6 @@ bool virQEMUCapsHasPCIMultiBus(virQEMUCapsPtr qemuCaps, bool virQEMUCapsSupportsVmport(virQEMUCapsPtr qemuCaps, const virDomainDef *def); -bool virQEMUCapsSupportsSMM(virQEMUCapsPtr qemuCaps, - const virDomainDef *def); - char *virQEMUCapsFlagsString(virQEMUCapsPtr qemuCaps); const char *virQEMUCapsGetBinary(virQEMUCapsPtr qemuCaps); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5190a88ad3ff..6bc9bf5ffab8 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7136,16 +7136,8 @@ qemuBuildMachineCommandLine(virCommandPtr cmd, virTristateSwitchTypeToString(vmport)); } - if (smm) { - if (!virQEMUCapsSupportsSMM(qemuCaps, def)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("smm is not available with this QEMU binary")); - goto cleanup; - } - - virBufferAsprintf(&buf, ",smm=%s", - virTristateSwitchTypeToString(smm)); - } + if (smm) + virBufferAsprintf(&buf, ",smm=%s", virTristateSwitchTypeToString(smm)); if (def->mem.dump_core) { virBufferAsprintf(&buf, ",dump-guest-core=%s", diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c5237e4d418d..a2c4d3a36090 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3743,7 +3743,8 @@ qemuDomainDefGetVcpuHotplugGranularity(const virDomainDef *def) static int -qemuDomainDefValidateFeatures(const virDomainDef *def) +qemuDomainDefValidateFeatures(const virDomainDef *def, + virQEMUCapsPtr qemuCaps) { size_t i; @@ -3790,6 +3791,15 @@ qemuDomainDefValidateFeatures(const virDomainDef *def) } break; + case VIR_DOMAIN_FEATURE_SMM: + if (def->features[i] != VIR_TRISTATE_SWITCH_ABSENT && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_SMM_OPT)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("smm is not available with this QEMU binary")); + return -1; + } + break; + case VIR_DOMAIN_FEATURE_ACPI: case VIR_DOMAIN_FEATURE_APIC: case VIR_DOMAIN_FEATURE_PAE: @@ -3802,7 +3812,6 @@ qemuDomainDefValidateFeatures(const virDomainDef *def) case VIR_DOMAIN_FEATURE_CAPABILITIES: case VIR_DOMAIN_FEATURE_PMU: case VIR_DOMAIN_FEATURE_VMPORT: - case VIR_DOMAIN_FEATURE_SMM: case VIR_DOMAIN_FEATURE_VMCOREINFO: case VIR_DOMAIN_FEATURE_LAST: break; @@ -3925,7 +3934,7 @@ qemuDomainDefValidate(const virDomainDef *def, } } - if (qemuDomainDefValidateFeatures(def) < 0) + if (qemuDomainDefValidateFeatures(def, qemuCaps) < 0) goto cleanup; ret = 0; -- 2.17.1

The commit summary speaks of moving checks, but the commit actually relaxes them. I would not expect functional changes from that message. How about: qemu: relax and move SMM checks to validation phase On Thu, Jun 07, 2018 at 10:37:40AM +0200, Martin Kletzander wrote:
We are still hoping all of such checks will be moved there and this is one small step in that direction.
One of the things that this is improving is the fact that instead of error message *that was wrong) you get when starting a domain with SMM and i440fx we
(that was wrong)
allow the setting to go through. SMM option exists and makes sense on i440fx as well (basically whenever that _SMM_OPT capability is set).
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_capabilities.c | 11 ----------- src/qemu/qemu_capabilities.h | 3 --- src/qemu/qemu_command.c | 12 ++---------- src/qemu/qemu_domain.c | 15 ++++++++++++--- 4 files changed, 14 insertions(+), 27 deletions(-)
With the commit summary adjusted or changes split in two: Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

On Thu, Jun 07, 2018 at 09:31:50PM +0200, Ján Tomko wrote:
The commit summary speaks of moving checks, but the commit actually relaxes them. I would not expect functional changes from that message.
How about: qemu: relax and move SMM checks to validation phase
Here you suggest changing the commit message to describe both changes.
On Thu, Jun 07, 2018 at 10:37:40AM +0200, Martin Kletzander wrote:
We are still hoping all of such checks will be moved there and this is one small step in that direction.
One of the things that this is improving is the fact that instead of error message *that was wrong) you get when starting a domain with SMM and i440fx we
(that was wrong)
allow the setting to go through. SMM option exists and makes sense on i440fx as well (basically whenever that _SMM_OPT capability is set).
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_capabilities.c | 11 ----------- src/qemu/qemu_capabilities.h | 3 --- src/qemu/qemu_command.c | 12 ++---------- src/qemu/qemu_domain.c | 15 ++++++++++++--- 4 files changed, 14 insertions(+), 27 deletions(-)
With the commit summary adjusted or changes split in two:
Here you are suggesting to split it into two. I'm fine with both, just tell me what you meant so I can fix it up, thanks.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jano

On Thu, Jun 07, 2018 at 11:04:48PM +0200, Martin Kletzander wrote:
On Thu, Jun 07, 2018 at 09:31:50PM +0200, Ján Tomko wrote:
The commit summary speaks of moving checks, but the commit actually relaxes them. I would not expect functional changes from that message.
How about: qemu: relax and move SMM checks to validation phase
Here you suggest changing the commit message to describe both changes.
On Thu, Jun 07, 2018 at 10:37:40AM +0200, Martin Kletzander wrote:
We are still hoping all of such checks will be moved there and this is one small step in that direction.
One of the things that this is improving is the fact that instead of error message *that was wrong) you get when starting a domain with SMM and i440fx we
(that was wrong)
allow the setting to go through. SMM option exists and makes sense on i440fx as well (basically whenever that _SMM_OPT capability is set).
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_capabilities.c | 11 ----------- src/qemu/qemu_capabilities.h | 3 --- src/qemu/qemu_command.c | 12 ++---------- src/qemu/qemu_domain.c | 15 ++++++++++++--- 4 files changed, 14 insertions(+), 27 deletions(-)
With the commit summary adjusted or changes split in two:
Here you are suggesting to split it into two. I'm fine with both, just tell me what you meant so I can fix it up, thanks.
Summary adjusted OR changes split, IOW: don't leave functional changes in the commit named 'Move' Of course, the split is nicer. Jano

On Thu, Jun 07, 2018 at 11:09:41PM +0200, Ján Tomko wrote:
On Thu, Jun 07, 2018 at 11:04:48PM +0200, Martin Kletzander wrote:
On Thu, Jun 07, 2018 at 09:31:50PM +0200, Ján Tomko wrote:
The commit summary speaks of moving checks, but the commit actually relaxes them. I would not expect functional changes from that message.
How about: qemu: relax and move SMM checks to validation phase
Here you suggest changing the commit message to describe both changes.
On Thu, Jun 07, 2018 at 10:37:40AM +0200, Martin Kletzander wrote:
We are still hoping all of such checks will be moved there and this is one small step in that direction.
One of the things that this is improving is the fact that instead of error message *that was wrong) you get when starting a domain with SMM and i440fx we
(that was wrong)
allow the setting to go through. SMM option exists and makes sense on i440fx as well (basically whenever that _SMM_OPT capability is set).
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_capabilities.c | 11 ----------- src/qemu/qemu_capabilities.h | 3 --- src/qemu/qemu_command.c | 12 ++---------- src/qemu/qemu_domain.c | 15 ++++++++++++--- 4 files changed, 14 insertions(+), 27 deletions(-)
With the commit summary adjusted or changes split in two: ^^ DUH!!!
Here you are suggesting to split it into two. I'm fine with both, just tell me what you meant so I can fix it up, thanks.
Summary adjusted OR changes split, IOW: don't leave functional changes in the commit named 'Move'
Of course, the split is nicer.
Of course, will do that.

TSEG (Top of Memory Segment) is one of many regions that SMM (System Management Mode) can occupy. This one, however is special, because a) most of the SMM code lives in TSEG nowadays and b) QEMU just (well, some time ago) added support for so called 'extended' TSEG. The difference to the TSEG implemented in real q35's MCH (Memory Controller Hub) is that it can offer one extra size to the guest OS apart from the standard TSEG's 1, 2, and 8 MiB and that size can be selected in 1 MiB increments. Maximum may vary based on QEMU and is way too big, so we don't need to check for the maximum here. Similarly to the memory size we'll leave it to the hypervisor to try satisfying that and giving us an error message in case it is not possible. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- docs/formatdomain.html.in | 48 +++++++++++++++++++++- docs/schemas/domaincommon.rng | 5 +++ src/conf/domain_conf.c | 64 ++++++++++++++++++++++++++++- src/conf/domain_conf.h | 3 ++ tests/genericxml2xmlindata/tseg.xml | 23 +++++++++++ tests/genericxml2xmltest.c | 2 + 6 files changed, 143 insertions(+), 2 deletions(-) create mode 100644 tests/genericxml2xmlindata/tseg.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index ba5bd1e5027e..2d1e1a7051d9 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1924,6 +1924,9 @@ <ioapic driver='qemu'/> <hpt resizing='required'/> <vmcoreinfo state='on'/> + <smm state='on'> + <tseg unit='MiB'>48</tseg> + </smm> </features> ...</pre> @@ -2079,10 +2082,53 @@ <span class="since">Since 1.2.16</span> </dd> <dt><code>smm</code></dt> - <dd>Depending on the <code>state</code> attribute (values <code>on</code>, + <dd> + <p> + Depending on the <code>state</code> attribute (values <code>on</code>, <code>off</code>, default <code>on</code>) enable or disable System Management Mode. <span class="since">Since 2.1.0</span> + </p><p> Optional sub-element <code>tseg</code> can be used to specify + the amount of memory dedicated to SMM's extended TSEG. That offers a + fourth option size apart from the existing ones (1 MiB, 2 MiB and 8 + MiB) that the guest OS (or rather loader) can choose from. The size + can be specified as a value of that element, optional attribute + <code>unit</code> can be used to specify the unit of the + aforementioned value (defaults to 'MiB'). + </p><p> + <b>If the VM is booting you should leave this option alone, unless you + are very certain you know what you are doing.</b> + </p><p> + This value is configurable due to the fact that the calculation cannot + be done right with the guarantee that it will work correctly. In + QEMU, the user-configurable extended TSEG feature was unavailable up + to and including <code>pc-q35-2.9</code>. Starting with + <code>pc-q35-2.10</code> the feature is available, with default size + 16 MiB. That should suffice for up to roughly 272 VCPUs, 5 GiB guest + RAM in total, no hotplug memory range, and 32 GiB of 64-bit PCI MMIO + aperture. Or for 48 VCPUs, with 1TB of guest RAM, no hotplug DIMM + range, and 32GB of 64-bit PCI MMIO aperture. The values may also vary + based on the loader the VM is using. + </p><p> + Additional size might be needed for significantly higher VCPU counts + or increased address space (that can be memory, maxMemory, 64-bit PCI + MMIO aperture size; roughly 8 MiB of TSEG per 1 TiB of address space) + which can also be rounded up. + </p><p> + Due to the nature of this setting being similar to "how much RAM + should the guest have" users are advised to either consult the + documentation of the guest OS or loader (if there is any), or test + this by trial-and-error changing the value until the VM boots + successfully. Yet another guiding value for users might be the fact + that 48 MiB should be enough for pretty large guests (240 VCPUs and + 4TB guest RAM), but it is on purpose not set as default as 48 MiB of + unavailable RAM might be too much for small guests (e.g. with 512 MiB + of RAM). + </p><p> + See <a href="#elementsMemoryAllocation">Memory Allocation</a> + for more details about the <code>unit</code> attribute. + <span class="since">Since 4.5.0</span> (QEMU only) + </p> </dd> <dt><code>ioapic</code></dt> <dd>Tune the I/O APIC. Possible values for the diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index c5c8c575b8cd..550fb10159e5 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4846,6 +4846,11 @@ <attribute name="state"> <ref name="virOnOff"/> </attribute> + <optional> + <element name="tseg"> + <ref name="scaledInteger"/> + </element> + </optional> </optional> </element> </optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5be773cda48a..62bf6bb803bb 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -19898,6 +19898,19 @@ virDomainDefParseXML(xmlDocPtr xml, VIR_FREE(nodes); } + if (def->features[VIR_DOMAIN_FEATURE_SMM] == VIR_TRISTATE_SWITCH_ON) { + int rv = virDomainParseScaledValue("string(./features/smm/tseg)", + "string(./features/smm/tseg/@unit)", + ctxt, + &def->tseg_size, + 1024 * 1024, /* Defaults to mebibytes */ + ULLONG_MAX, + false); + if (rv < 0) + goto error; + def->tseg_specified = rv; + } + if ((n = virXPathNodeSet("./features/capabilities/*", ctxt, &nodes)) < 0) goto error; @@ -22020,6 +22033,32 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src, } } + /* smm */ + if (src->features[VIR_DOMAIN_FEATURE_SMM] == VIR_TRISTATE_SWITCH_ON) { + if (src->tseg_specified != dst->tseg_specified) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("SMM TSEG differs: source: %s, destination: '%s'"), + src->tseg_specified ? _("specified") : _("not specified"), + dst->tseg_specified ? _("specified") : _("not specified")); + return false; + } + + if (src->tseg_size != dst->tseg_size) { + const char *unit_src, *unit_dst; + unsigned long long short_size_src = virFormatIntPretty(src->tseg_size, + &unit_src); + unsigned long long short_size_dst = virFormatIntPretty(dst->tseg_size, + &unit_dst); + + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Size of SMM TSEG size differs: " + "source: '%llu %s', destination: '%llu %s'"), + short_size_src, unit_src, + short_size_dst, unit_dst); + return false; + } + } + return true; } @@ -27446,7 +27485,6 @@ virDomainDefFormatInternal(virDomainDefPtr def, case VIR_DOMAIN_FEATURE_PMU: case VIR_DOMAIN_FEATURE_PVSPINLOCK: case VIR_DOMAIN_FEATURE_VMPORT: - case VIR_DOMAIN_FEATURE_SMM: switch ((virTristateSwitch) def->features[i]) { case VIR_TRISTATE_SWITCH_LAST: case VIR_TRISTATE_SWITCH_ABSENT: @@ -27463,6 +27501,30 @@ virDomainDefFormatInternal(virDomainDefPtr def, break; + case VIR_DOMAIN_FEATURE_SMM: + if (def->features[i] != VIR_TRISTATE_SWITCH_ABSENT) { + virTristateSwitch state = def->features[i]; + virBuffer attrBuf = VIR_BUFFER_INITIALIZER; + virBuffer childBuf = VIR_BUFFER_INITIALIZER; + + virBufferAsprintf(&attrBuf, " state='%s'", + virTristateSwitchTypeToString(state)); + + if (state == VIR_TRISTATE_SWITCH_ON) { + const char *unit; + unsigned long long short_size = virFormatIntPretty(def->tseg_size, + &unit); + + virBufferSetChildIndent(&childBuf, buf); + virBufferAsprintf(&childBuf, "<tseg unit='%s'>%llu</tseg>\n", + unit, short_size); + } + + virXMLFormatElement(buf, "smm", &attrBuf, &childBuf); + } + + break; + case VIR_DOMAIN_FEATURE_APIC: if (def->features[i] == VIR_TRISTATE_SWITCH_ON) { virBufferAddLit(buf, "<apic"); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 8a8121bf83b2..a41cdce6ff63 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2421,6 +2421,9 @@ struct _virDomainDef { char *hyperv_vendor_id; int apic_eoi; + bool tseg_specified; + unsigned long long tseg_size; + virDomainClockDef clock; size_t ngraphics; diff --git a/tests/genericxml2xmlindata/tseg.xml b/tests/genericxml2xmlindata/tseg.xml new file mode 100644 index 000000000000..49483f874cd4 --- /dev/null +++ b/tests/genericxml2xmlindata/tseg.xml @@ -0,0 +1,23 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='q35'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <smm state='on'> + <tseg unit='MiB'>48</tseg> + </smm> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </devices> +</domain> diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c index d8270a6cae82..daad6e0f78d8 100644 --- a/tests/genericxml2xmltest.c +++ b/tests/genericxml2xmltest.c @@ -141,6 +141,8 @@ mymain(void) DO_TEST_FULL("cachetune-colliding-types", false, true, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); + DO_TEST("tseg"); + virObjectUnref(caps); virObjectUnref(xmlopt); -- 2.17.1

On 06/07/18 10:37, Martin Kletzander wrote:
TSEG (Top of Memory Segment) is one of many regions that SMM (System Management Mode) can occupy. This one, however is special, because a) most of the SMM code lives in TSEG nowadays and b) QEMU just (well, some time ago) added support for so called 'extended' TSEG. The difference to the TSEG implemented in real q35's MCH (Memory Controller Hub) is that it can offer one extra size to the guest OS apart from the standard TSEG's 1, 2, and 8 MiB and that size can be selected in 1 MiB increments. Maximum may vary based on QEMU and is way too big, so we don't need to check for the maximum here. Similarly to the memory size we'll leave it to the hypervisor to try satisfying that and giving us an error message in case it is not possible.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- docs/formatdomain.html.in | 48 +++++++++++++++++++++- docs/schemas/domaincommon.rng | 5 +++ src/conf/domain_conf.c | 64 ++++++++++++++++++++++++++++- src/conf/domain_conf.h | 3 ++ tests/genericxml2xmlindata/tseg.xml | 23 +++++++++++ tests/genericxml2xmltest.c | 2 + 6 files changed, 143 insertions(+), 2 deletions(-) create mode 100644 tests/genericxml2xmlindata/tseg.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index ba5bd1e5027e..2d1e1a7051d9 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1924,6 +1924,9 @@ <ioapic driver='qemu'/> <hpt resizing='required'/> <vmcoreinfo state='on'/> + <smm state='on'> + <tseg unit='MiB'>48</tseg> + </smm> </features> ...</pre>
@@ -2079,10 +2082,53 @@ <span class="since">Since 1.2.16</span> </dd> <dt><code>smm</code></dt> - <dd>Depending on the <code>state</code> attribute (values <code>on</code>, + <dd> + <p> + Depending on the <code>state</code> attribute (values <code>on</code>, <code>off</code>, default <code>on</code>) enable or disable System Management Mode. <span class="since">Since 2.1.0</span> + </p><p> Optional sub-element <code>tseg</code> can be used to specify + the amount of memory dedicated to SMM's extended TSEG. That offers a + fourth option size apart from the existing ones (1 MiB, 2 MiB and 8 + MiB) that the guest OS (or rather loader) can choose from. The size + can be specified as a value of that element, optional attribute + <code>unit</code> can be used to specify the unit of the + aforementioned value (defaults to 'MiB'). + </p><p> + <b>If the VM is booting you should leave this option alone, unless you + are very certain you know what you are doing.</b> + </p><p> + This value is configurable due to the fact that the calculation cannot + be done right with the guarantee that it will work correctly. In + QEMU, the user-configurable extended TSEG feature was unavailable up + to and including <code>pc-q35-2.9</code>. Starting with + <code>pc-q35-2.10</code> the feature is available, with default size + 16 MiB. That should suffice for up to roughly 272 VCPUs, 5 GiB guest + RAM in total, no hotplug memory range, and 32 GiB of 64-bit PCI MMIO + aperture. Or for 48 VCPUs, with 1TB of guest RAM, no hotplug DIMM + range, and 32GB of 64-bit PCI MMIO aperture. The values may also vary + based on the loader the VM is using. + </p><p> + Additional size might be needed for significantly higher VCPU counts + or increased address space (that can be memory, maxMemory, 64-bit PCI + MMIO aperture size; roughly 8 MiB of TSEG per 1 TiB of address space) + which can also be rounded up. + </p><p> + Due to the nature of this setting being similar to "how much RAM + should the guest have" users are advised to either consult the + documentation of the guest OS or loader (if there is any), or test + this by trial-and-error changing the value until the VM boots + successfully. Yet another guiding value for users might be the fact + that 48 MiB should be enough for pretty large guests (240 VCPUs and + 4TB guest RAM), but it is on purpose not set as default as 48 MiB of + unavailable RAM might be too much for small guests (e.g. with 512 MiB + of RAM). + </p><p> + See <a href="#elementsMemoryAllocation">Memory Allocation</a> + for more details about the <code>unit</code> attribute. + <span class="since">Since 4.5.0</span> (QEMU only) + </p> </dd> <dt><code>ioapic</code></dt> <dd>Tune the I/O APIC. Possible values for the diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index c5c8c575b8cd..550fb10159e5 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4846,6 +4846,11 @@ <attribute name="state"> <ref name="virOnOff"/> </attribute> + <optional> + <element name="tseg"> + <ref name="scaledInteger"/> + </element> + </optional> </optional> </element> </optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5be773cda48a..62bf6bb803bb 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -19898,6 +19898,19 @@ virDomainDefParseXML(xmlDocPtr xml, VIR_FREE(nodes); }
+ if (def->features[VIR_DOMAIN_FEATURE_SMM] == VIR_TRISTATE_SWITCH_ON) { + int rv = virDomainParseScaledValue("string(./features/smm/tseg)", + "string(./features/smm/tseg/@unit)", + ctxt, + &def->tseg_size, + 1024 * 1024, /* Defaults to mebibytes */ + ULLONG_MAX, + false); + if (rv < 0) + goto error; + def->tseg_specified = rv; + } + if ((n = virXPathNodeSet("./features/capabilities/*", ctxt, &nodes)) < 0) goto error;
@@ -22020,6 +22033,32 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src, } }
+ /* smm */ + if (src->features[VIR_DOMAIN_FEATURE_SMM] == VIR_TRISTATE_SWITCH_ON) { + if (src->tseg_specified != dst->tseg_specified) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("SMM TSEG differs: source: %s, destination: '%s'"), + src->tseg_specified ? _("specified") : _("not specified"), + dst->tseg_specified ? _("specified") : _("not specified")); + return false; + } + + if (src->tseg_size != dst->tseg_size) { + const char *unit_src, *unit_dst; + unsigned long long short_size_src = virFormatIntPretty(src->tseg_size, + &unit_src); + unsigned long long short_size_dst = virFormatIntPretty(dst->tseg_size, + &unit_dst); + + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Size of SMM TSEG size differs: " + "source: '%llu %s', destination: '%llu %s'"), + short_size_src, unit_src, + short_size_dst, unit_dst); + return false; + } + } + return true; }
@@ -27446,7 +27485,6 @@ virDomainDefFormatInternal(virDomainDefPtr def, case VIR_DOMAIN_FEATURE_PMU: case VIR_DOMAIN_FEATURE_PVSPINLOCK: case VIR_DOMAIN_FEATURE_VMPORT: - case VIR_DOMAIN_FEATURE_SMM: switch ((virTristateSwitch) def->features[i]) { case VIR_TRISTATE_SWITCH_LAST: case VIR_TRISTATE_SWITCH_ABSENT: @@ -27463,6 +27501,30 @@ virDomainDefFormatInternal(virDomainDefPtr def,
break;
+ case VIR_DOMAIN_FEATURE_SMM: + if (def->features[i] != VIR_TRISTATE_SWITCH_ABSENT) { + virTristateSwitch state = def->features[i]; + virBuffer attrBuf = VIR_BUFFER_INITIALIZER; + virBuffer childBuf = VIR_BUFFER_INITIALIZER; + + virBufferAsprintf(&attrBuf, " state='%s'", + virTristateSwitchTypeToString(state)); + + if (state == VIR_TRISTATE_SWITCH_ON) { + const char *unit; + unsigned long long short_size = virFormatIntPretty(def->tseg_size, + &unit); + + virBufferSetChildIndent(&childBuf, buf); + virBufferAsprintf(&childBuf, "<tseg unit='%s'>%llu</tseg>\n", + unit, short_size); + } + + virXMLFormatElement(buf, "smm", &attrBuf, &childBuf); + } + + break; + case VIR_DOMAIN_FEATURE_APIC: if (def->features[i] == VIR_TRISTATE_SWITCH_ON) { virBufferAddLit(buf, "<apic"); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 8a8121bf83b2..a41cdce6ff63 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2421,6 +2421,9 @@ struct _virDomainDef { char *hyperv_vendor_id; int apic_eoi;
+ bool tseg_specified; + unsigned long long tseg_size; + virDomainClockDef clock;
size_t ngraphics; diff --git a/tests/genericxml2xmlindata/tseg.xml b/tests/genericxml2xmlindata/tseg.xml new file mode 100644 index 000000000000..49483f874cd4 --- /dev/null +++ b/tests/genericxml2xmlindata/tseg.xml @@ -0,0 +1,23 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='q35'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <smm state='on'> + <tseg unit='MiB'>48</tseg> + </smm> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </devices> +</domain> diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c index d8270a6cae82..daad6e0f78d8 100644 --- a/tests/genericxml2xmltest.c +++ b/tests/genericxml2xmltest.c @@ -141,6 +141,8 @@ mymain(void) DO_TEST_FULL("cachetune-colliding-types", false, true, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE);
+ DO_TEST("tseg"); + virObjectUnref(caps); virObjectUnref(xmlopt);
I checked the commit message and the docs; they look OK to me. Acked-by: Laszlo Ersek <lersek@redhat.com> Thanks, Laszlo

On Thu, Jun 07, 2018 at 10:37:41AM +0200, Martin Kletzander wrote:
TSEG (Top of Memory Segment) is one of many regions that SMM (System Management Mode) can occupy. This one, however is special, because a) most of the SMM code lives in TSEG nowadays and b) QEMU just (well, some time ago) added support for so called 'extended' TSEG. The difference to the TSEG implemented in real q35's MCH (Memory Controller Hub) is that it can offer one extra size to the guest OS apart from the standard TSEG's 1, 2, and 8 MiB and that size can be selected in 1 MiB increments. Maximum may vary based on QEMU and is way too big, so we don't need to check for the maximum here. Similarly to the memory size we'll leave it to the hypervisor to try satisfying that and giving us an error message in case it is not possible.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- docs/formatdomain.html.in | 48 +++++++++++++++++++++- docs/schemas/domaincommon.rng | 5 +++ src/conf/domain_conf.c | 64 ++++++++++++++++++++++++++++- src/conf/domain_conf.h | 3 ++ tests/genericxml2xmlindata/tseg.xml | 23 +++++++++++ tests/genericxml2xmltest.c | 2 + 6 files changed, 143 insertions(+), 2 deletions(-) create mode 100644 tests/genericxml2xmlindata/tseg.xml
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

For getting the reply I queried the newest and oldest QEMU using test/qemucapsprobe. From the differences I only extracted the reply to the new QMP command and discarded the rest. For all the versions below the one which added support for the new option I used the output from the oldest QEMU release and for those that support it I used the output from the newest one. In order to make doubly sure the reply is where it is supposed to be (the replies files are very forgiving) I added the property to all the replies files, reran the tests again and fixed the order in replies files so that all the versions are reporting the new capability. Then removed that one property. After that I used test/qemucapsfixreplies to fix the reply IDs. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_capabilities.c | 10 +++ src/qemu/qemu_capabilities.h | 2 + .../caps_1.5.3.x86_64.replies | 38 +++++++++-- .../caps_1.5.3.x86_64.xml | 3 +- .../caps_1.6.0.x86_64.replies | 38 +++++++++-- .../caps_1.6.0.x86_64.xml | 3 +- .../caps_1.7.0.x86_64.replies | 38 +++++++++-- .../caps_1.7.0.x86_64.xml | 3 +- .../caps_2.1.1.x86_64.replies | 38 +++++++++-- .../caps_2.1.1.x86_64.xml | 3 +- .../caps_2.10.0.x86_64.replies | 48 ++++++++++--- .../caps_2.10.0.x86_64.xml | 3 +- .../caps_2.12.0.x86_64.replies | 67 +++++++++++++++---- .../caps_2.12.0.x86_64.xml | 4 +- .../caps_2.4.0.x86_64.replies | 38 +++++++++-- .../caps_2.4.0.x86_64.xml | 3 +- .../caps_2.5.0.x86_64.replies | 40 +++++++++-- .../caps_2.5.0.x86_64.xml | 3 +- .../caps_2.6.0.x86_64.replies | 40 +++++++++-- .../caps_2.6.0.x86_64.xml | 3 +- .../caps_2.7.0.x86_64.replies | 40 +++++++++-- .../caps_2.7.0.x86_64.xml | 3 +- .../caps_2.8.0.x86_64.replies | 40 +++++++++-- .../caps_2.8.0.x86_64.xml | 3 +- .../caps_2.9.0.x86_64.replies | 48 ++++++++++--- .../caps_2.9.0.x86_64.xml | 3 +- 26 files changed, 458 insertions(+), 104 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 5f48042ac752..8311985d5227 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -495,6 +495,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "vhost-vsock", "chardev-fd-pass", "tpm-emulator", + "mch", + "mch.extended-tseg-mbytes", ); @@ -1132,6 +1134,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "hda-output", QEMU_CAPS_HDA_OUTPUT }, { "vmgenid", QEMU_CAPS_DEVICE_VMGENID }, { "vhost-vsock-device", QEMU_CAPS_DEVICE_VHOST_VSOCK }, + { "mch", QEMU_CAPS_DEVICE_MCH }, }; static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsVirtioBalloon[] = { @@ -1277,6 +1280,10 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtualCSSBridge[] = { "cssid-unrestricted", QEMU_CAPS_CCW_CSSID_UNRESTRICTED }, }; +static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsMCH[] = { + { "extended-tseg-mbytes", QEMU_CAPS_MCH_EXTENDED_TSEG_MBYTES }, +}; + /* see documentation for virQEMUQAPISchemaPathGet for the query format */ static struct virQEMUCapsStringFlags virQEMUCapsQMPSchemaQueries[] = { { "blockdev-add/arg-type/options/+gluster/debug-level", QEMU_CAPS_GLUSTER_DEBUG_LEVEL}, @@ -1406,6 +1413,9 @@ static virQEMUCapsObjectTypeProps virQEMUCapsDeviceProps[] = { { "virtual-css-bridge", virQEMUCapsObjectPropsVirtualCSSBridge, ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtualCSSBridge), QEMU_CAPS_CCW }, + { "mch", virQEMUCapsDevicePropsMCH, + ARRAY_CARDINALITY(virQEMUCapsDevicePropsMCH), + QEMU_CAPS_DEVICE_MCH }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsMemoryBackendFile[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 7e602049ca71..884b40650c5e 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -479,6 +479,8 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_DEVICE_VHOST_VSOCK, /* -device vhost-vsock-* */ QEMU_CAPS_CHARDEV_FD_PASS, /* Passing pre-opened FDs for chardevs */ QEMU_CAPS_DEVICE_TPM_EMULATOR, /* -tpmdev emulator */ + QEMU_CAPS_DEVICE_MCH, /* Northbridge in q35 machine types */ + QEMU_CAPS_MCH_EXTENDED_TSEG_MBYTES, /* -global mch.extended-tseg-mbytes */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.replies b/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.replies index 8da1b149d09f..dd501221ade9 100644 --- a/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.replies @@ -2044,6 +2044,32 @@ "id": "libvirt-36" } +{ + "return": [ + { + "name": "command_serr_enable", + "type": "on/off" + }, + { + "name": "multifunction", + "type": "on/off" + }, + { + "name": "rombar", + "type": "uint32" + }, + { + "name": "romfile", + "type": "string" + }, + { + "name": "addr", + "type": "pci-devfn" + } + ], + "id": "libvirt-37" +} + { "return": [ { @@ -2114,7 +2140,7 @@ "cpu-max": 1 } ], - "id": "libvirt-37" + "id": "libvirt-38" } { @@ -2192,19 +2218,19 @@ "name": "qemu64" } ], - "id": "libvirt-38" + "id": "libvirt-39" } { "return": [ ], - "id": "libvirt-39" + "id": "libvirt-40" } { "return": [ ], - "id": "libvirt-40" + "id": "libvirt-41" } { @@ -2905,7 +2931,7 @@ "option": "drive" } ], - "id": "libvirt-41" + "id": "libvirt-42" } { @@ -2915,7 +2941,7 @@ "capability": "xbzrle" } ], - "id": "libvirt-42" + "id": "libvirt-43" } { diff --git a/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml b/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml index 3e700cb42782..d5c60dece06b 100644 --- a/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml @@ -107,9 +107,10 @@ <flag name='kernel-irqchip'/> <flag name='isa-serial'/> <flag name='hda-output'/> + <flag name='mch'/> <version>1005003</version> <kvmVersion>0</kvmVersion> - <microcodeVersion>46523</microcodeVersion> + <microcodeVersion>46889</microcodeVersion> <package></package> <arch>x86_64</arch> <cpu type='kvm' name='Opteron_G5'/> diff --git a/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.replies index d53fb576d266..09b2b9d4a4fa 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.replies @@ -2089,6 +2089,32 @@ "id": "libvirt-36" } +{ + "return": [ + { + "name": "command_serr_enable", + "type": "on/off" + }, + { + "name": "multifunction", + "type": "on/off" + }, + { + "name": "rombar", + "type": "uint32" + }, + { + "name": "romfile", + "type": "string" + }, + { + "name": "addr", + "type": "pci-devfn" + } + ], + "id": "libvirt-37" +} + { "return": [ { @@ -2167,7 +2193,7 @@ "cpu-max": 1 } ], - "id": "libvirt-37" + "id": "libvirt-38" } { @@ -2245,19 +2271,19 @@ "name": "qemu64" } ], - "id": "libvirt-38" + "id": "libvirt-39" } { "return": [ ], - "id": "libvirt-39" + "id": "libvirt-40" } { "return": [ ], - "id": "libvirt-40" + "id": "libvirt-41" } { @@ -2860,7 +2886,7 @@ "option": "drive" } ], - "id": "libvirt-41" + "id": "libvirt-42" } { @@ -2882,7 +2908,7 @@ "capability": "zero-blocks" } ], - "id": "libvirt-42" + "id": "libvirt-43" } { diff --git a/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml index 14eca7e7cb8f..e5cb3c3c29c4 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml @@ -112,9 +112,10 @@ <flag name='kernel-irqchip'/> <flag name='isa-serial'/> <flag name='hda-output'/> + <flag name='mch'/> <version>1006000</version> <kvmVersion>0</kvmVersion> - <microcodeVersion>44752</microcodeVersion> + <microcodeVersion>45118</microcodeVersion> <package></package> <arch>x86_64</arch> <cpu type='kvm' name='Opteron_G5'/> diff --git a/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.replies index 4fcc7aa25a2e..72ba4c27e833 100644 --- a/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.replies @@ -2106,6 +2106,32 @@ "id": "libvirt-36" } +{ + "return": [ + { + "name": "command_serr_enable", + "type": "on/off" + }, + { + "name": "multifunction", + "type": "on/off" + }, + { + "name": "rombar", + "type": "uint32" + }, + { + "name": "romfile", + "type": "string" + }, + { + "name": "addr", + "type": "pci-devfn" + } + ], + "id": "libvirt-37" +} + { "return": [ { @@ -2192,7 +2218,7 @@ "cpu-max": 1 } ], - "id": "libvirt-37" + "id": "libvirt-38" } { @@ -2270,19 +2296,19 @@ "name": "qemu64" } ], - "id": "libvirt-38" + "id": "libvirt-39" } { "return": [ ], - "id": "libvirt-39" + "id": "libvirt-40" } { "return": [ ], - "id": "libvirt-40" + "id": "libvirt-41" } { @@ -3075,7 +3101,7 @@ "option": "drive" } ], - "id": "libvirt-41" + "id": "libvirt-42" } { @@ -3097,7 +3123,7 @@ "capability": "zero-blocks" } ], - "id": "libvirt-42" + "id": "libvirt-43" } { diff --git a/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml index 43b5374fd230..35d14f21b1c6 100644 --- a/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml @@ -114,9 +114,10 @@ <flag name='kernel-irqchip'/> <flag name='isa-serial'/> <flag name='hda-output'/> + <flag name='mch'/> <version>1007000</version> <kvmVersion>0</kvmVersion> - <microcodeVersion>50196</microcodeVersion> + <microcodeVersion>50562</microcodeVersion> <package></package> <arch>x86_64</arch> <cpu type='kvm' name='Opteron_G5'/> diff --git a/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.replies b/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.replies index 543bce9defc1..1a21b7e8d9a7 100644 --- a/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.replies @@ -2523,6 +2523,32 @@ "id": "libvirt-36" } +{ + "return": [ + { + "name": "command_serr_enable", + "type": "on/off" + }, + { + "name": "multifunction", + "type": "on/off" + }, + { + "name": "rombar", + "type": "uint32" + }, + { + "name": "romfile", + "type": "string" + }, + { + "name": "addr", + "type": "pci-devfn" + } + ], + "id": "libvirt-37" +} + { "return": [ { @@ -2625,7 +2651,7 @@ "cpu-max": 255 } ], - "id": "libvirt-37" + "id": "libvirt-38" } { @@ -2706,21 +2732,21 @@ "name": "qemu64" } ], - "id": "libvirt-38" + "id": "libvirt-39" } { "return": [ "tpm-tis" ], - "id": "libvirt-39" + "id": "libvirt-40" } { "return": [ "passthrough" ], - "id": "libvirt-40" + "id": "libvirt-41" } { @@ -3580,7 +3606,7 @@ "option": "drive" } ], - "id": "libvirt-41" + "id": "libvirt-42" } { @@ -3602,7 +3628,7 @@ "capability": "zero-blocks" } ], - "id": "libvirt-42" + "id": "libvirt-43" } { diff --git a/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml index ac52c68d9662..d9a69c961596 100644 --- a/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml @@ -130,9 +130,10 @@ <flag name='kernel-irqchip'/> <flag name='isa-serial'/> <flag name='hda-output'/> + <flag name='mch'/> <version>2001001</version> <kvmVersion>0</kvmVersion> - <microcodeVersion>58992</microcodeVersion> + <microcodeVersion>59358</microcodeVersion> <package></package> <arch>x86_64</arch> <cpu type='kvm' name='Opteron_G5'/> diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.replies index 6c6ecc26874c..a32bcc7ba0fa 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.replies @@ -4471,6 +4471,32 @@ "id": "libvirt-39" } +{ + "return": [ + { + "name": "command_serr_enable", + "type": "on/off" + }, + { + "name": "multifunction", + "type": "on/off" + }, + { + "name": "rombar", + "type": "uint32" + }, + { + "name": "romfile", + "type": "string" + }, + { + "name": "addr", + "type": "pci-devfn" + } + ], + "id": "libvirt-40" +} + { "return": [ { @@ -4657,7 +4683,7 @@ "alias": "q35" } ], - "id": "libvirt-40" + "id": "libvirt-41" } { @@ -4992,21 +5018,21 @@ "migration-safe": true } ], - "id": "libvirt-41" + "id": "libvirt-42" } { "return": [ "tpm-tis" ], - "id": "libvirt-42" + "id": "libvirt-43" } { "return": [ "passthrough" ], - "id": "libvirt-43" + "id": "libvirt-44" } { @@ -6285,7 +6311,7 @@ "option": "drive" } ], - "id": "libvirt-44" + "id": "libvirt-45" } { @@ -6335,7 +6361,7 @@ "capability": "return-path" } ], - "id": "libvirt-45" + "id": "libvirt-46" } { @@ -16058,7 +16084,7 @@ "meta-type": "object" } ], - "id": "libvirt-46" + "id": "libvirt-47" } { @@ -16237,7 +16263,7 @@ } } }, - "id": "libvirt-47" + "id": "libvirt-48" } { @@ -16480,7 +16506,7 @@ } } }, - "id": "libvirt-48" + "id": "libvirt-49" } { @@ -16659,7 +16685,7 @@ } } }, - "id": "libvirt-49" + "id": "libvirt-50" } { @@ -16902,7 +16928,7 @@ } } }, - "id": "libvirt-50" + "id": "libvirt-51" } { diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml index 7c5aa50d5942..733e7b4e3888 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml @@ -202,9 +202,10 @@ <flag name='blockdev-del'/> <flag name='vmgenid'/> <flag name='vhost-vsock'/> + <flag name='mch'/> <version>2010000</version> <kvmVersion>0</kvmVersion> - <microcodeVersion>344938</microcodeVersion> + <microcodeVersion>345304</microcodeVersion> <package> (v2.10.0)</package> <arch>x86_64</arch> <hostCPU type='kvm' model='base' migratability='yes'> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.replies index c40046beef6b..78e1b450cda3 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.replies @@ -4605,6 +4605,49 @@ "id": "libvirt-39" } +{ + "return": [ + { + "name": "rombar", + "type": "uint32" + }, + { + "name": "x-pcie-lnksta-dllla", + "description": "on/off", + "type": "bool" + }, + { + "name": "multifunction", + "description": "on/off", + "type": "bool" + }, + { + "name": "extended-tseg-mbytes", + "type": "uint16" + }, + { + "name": "romfile", + "type": "str" + }, + { + "name": "x-pcie-extcap-init", + "description": "on/off", + "type": "bool" + }, + { + "name": "command_serr_enable", + "description": "on/off", + "type": "bool" + }, + { + "name": "addr", + "description": "Slot and optional function number, example: 06.0 or 06", + "type": "int32" + } + ], + "id": "libvirt-40" +} + { "return": [ { @@ -4656,7 +4699,7 @@ "type": "string" } ], - "id": "libvirt-40" + "id": "libvirt-41" } { @@ -4855,7 +4898,7 @@ "cpu-max": 255 } ], - "id": "libvirt-41" + "id": "libvirt-42" } { @@ -5369,7 +5412,7 @@ "migration-safe": true } ], - "id": "libvirt-42" + "id": "libvirt-43" } { @@ -5377,7 +5420,7 @@ "tpm-crb", "tpm-tis" ], - "id": "libvirt-43" + "id": "libvirt-44" } { @@ -5385,7 +5428,7 @@ "passthrough", "emulator" ], - "id": "libvirt-44" + "id": "libvirt-45" } { @@ -6672,7 +6715,7 @@ "option": "drive" } ], - "id": "libvirt-45" + "id": "libvirt-46" } { @@ -6734,7 +6777,7 @@ "capability": "dirty-bitmaps" } ], - "id": "libvirt-46" + "id": "libvirt-47" } { @@ -18102,7 +18145,7 @@ "meta-type": "object" } ], - "id": "libvirt-47" + "id": "libvirt-48" } { @@ -18292,7 +18335,7 @@ } } }, - "id": "libvirt-48" + "id": "libvirt-49" } { @@ -18547,7 +18590,7 @@ } } }, - "id": "libvirt-49" + "id": "libvirt-50" } { @@ -18737,7 +18780,7 @@ } } }, - "id": "libvirt-50" + "id": "libvirt-51" } { @@ -18992,7 +19035,7 @@ } } }, - "id": "libvirt-51" + "id": "libvirt-52" } { diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml index 6b7c4926eaba..2afd7adc60e9 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml @@ -208,9 +208,11 @@ <flag name='vhost-vsock'/> <flag name='chardev-fd-pass'/> <flag name='tpm-emulator'/> + <flag name='mch'/> + <flag name='mch.extended-tseg-mbytes'/> <version>2011090</version> <kvmVersion>0</kvmVersion> - <microcodeVersion>390813</microcodeVersion> + <microcodeVersion>391586</microcodeVersion> <package>v2.12.0-rc0</package> <arch>x86_64</arch> <hostCPU type='kvm' model='base' migratability='yes'> diff --git a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies index 68ecb0c17dc7..bf8e7b4379ff 100644 --- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies @@ -3114,6 +3114,32 @@ "id": "libvirt-39" } +{ + "return": [ + { + "name": "command_serr_enable", + "type": "on/off" + }, + { + "name": "multifunction", + "type": "on/off" + }, + { + "name": "rombar", + "type": "uint32" + }, + { + "name": "romfile", + "type": "string" + }, + { + "name": "addr", + "type": "pci-devfn" + } + ], + "id": "libvirt-40" +} + { "return": [ { @@ -3240,7 +3266,7 @@ "cpu-max": 255 } ], - "id": "libvirt-40" + "id": "libvirt-41" } { @@ -3330,21 +3356,21 @@ "name": "qemu64" } ], - "id": "libvirt-41" + "id": "libvirt-42" } { "return": [ "tpm-tis" ], - "id": "libvirt-42" + "id": "libvirt-43" } { "return": [ "passthrough" ], - "id": "libvirt-43" + "id": "libvirt-44" } { @@ -4352,7 +4378,7 @@ "option": "drive" } ], - "id": "libvirt-44" + "id": "libvirt-45" } { @@ -4382,7 +4408,7 @@ "capability": "events" } ], - "id": "libvirt-45" + "id": "libvirt-46" } { diff --git a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml index ffbb3da12449..1aafec2b7099 100644 --- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml @@ -156,9 +156,10 @@ <flag name='isa-serial'/> <flag name='sdl-gl'/> <flag name='hda-output'/> + <flag name='mch'/> <version>2004000</version> <kvmVersion>0</kvmVersion> - <microcodeVersion>75406</microcodeVersion> + <microcodeVersion>75772</microcodeVersion> <package></package> <arch>x86_64</arch> <cpu type='kvm' name='Opteron_G5'/> diff --git a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.replies index 5bc505abb32c..57bf70f0e29b 100644 --- a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.replies @@ -3277,6 +3277,32 @@ "id": "libvirt-39" } +{ + "return": [ + { + "name": "command_serr_enable", + "type": "on/off" + }, + { + "name": "multifunction", + "type": "on/off" + }, + { + "name": "rombar", + "type": "uint32" + }, + { + "name": "romfile", + "type": "string" + }, + { + "name": "addr", + "type": "pci-devfn" + } + ], + "id": "libvirt-40" +} + { "return": [ { @@ -3411,7 +3437,7 @@ "cpu-max": 255 } ], - "id": "libvirt-40" + "id": "libvirt-41" } { @@ -3501,21 +3527,21 @@ "name": "qemu64" } ], - "id": "libvirt-41" + "id": "libvirt-42" } { "return": [ "tpm-tis" ], - "id": "libvirt-42" + "id": "libvirt-43" } { "return": [ "passthrough" ], - "id": "libvirt-43" + "id": "libvirt-44" } { @@ -4560,7 +4586,7 @@ "option": "drive" } ], - "id": "libvirt-44" + "id": "libvirt-45" } { @@ -4594,7 +4620,7 @@ "capability": "x-postcopy-ram" } ], - "id": "libvirt-45" + "id": "libvirt-46" } { @@ -12139,7 +12165,7 @@ "meta-type": "array" } ], - "id": "libvirt-46" + "id": "libvirt-47" } { diff --git a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml index 98aad7cb1789..da921e88c219 100644 --- a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml @@ -162,9 +162,10 @@ <flag name='isa-serial'/> <flag name='sdl-gl'/> <flag name='hda-output'/> + <flag name='mch'/> <version>2005000</version> <kvmVersion>0</kvmVersion> - <microcodeVersion>216528</microcodeVersion> + <microcodeVersion>216894</microcodeVersion> <package></package> <arch>x86_64</arch> <cpu type='kvm' name='Opteron_G5'/> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.replies index 73a22ed0bb95..436c824c14e8 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.replies @@ -3359,6 +3359,32 @@ "id": "libvirt-39" } +{ + "return": [ + { + "name": "command_serr_enable", + "type": "on/off" + }, + { + "name": "multifunction", + "type": "on/off" + }, + { + "name": "rombar", + "type": "uint32" + }, + { + "name": "romfile", + "type": "string" + }, + { + "name": "addr", + "type": "pci-devfn" + } + ], + "id": "libvirt-40" +} + { "return": [ { @@ -3469,7 +3495,7 @@ "cpu-max": 255 } ], - "id": "libvirt-40" + "id": "libvirt-41" } { @@ -3559,21 +3585,21 @@ "name": "qemu64" } ], - "id": "libvirt-41" + "id": "libvirt-42" } { "return": [ "tpm-tis" ], - "id": "libvirt-42" + "id": "libvirt-43" } { "return": [ "passthrough" ], - "id": "libvirt-43" + "id": "libvirt-44" } { @@ -4667,7 +4693,7 @@ "option": "drive" } ], - "id": "libvirt-44" + "id": "libvirt-45" } { @@ -4701,7 +4727,7 @@ "capability": "postcopy-ram" } ], - "id": "libvirt-45" + "id": "libvirt-46" } { @@ -12706,7 +12732,7 @@ "meta-type": "array" } ], - "id": "libvirt-46" + "id": "libvirt-47" } { diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml index de0e8a8a9818..468017fc8056 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml @@ -174,9 +174,10 @@ <flag name='nbd-tls'/> <flag name='sdl-gl'/> <flag name='hda-output'/> + <flag name='mch'/> <version>2006000</version> <kvmVersion>0</kvmVersion> - <microcodeVersion>227332</microcodeVersion> + <microcodeVersion>227698</microcodeVersion> <package></package> <arch>x86_64</arch> <cpu type='kvm' name='Opteron_G5'/> diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.replies index eaa84d3381a6..9d71070b0831 100644 --- a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.replies @@ -3554,6 +3554,32 @@ "id": "libvirt-39" } +{ + "return": [ + { + "name": "command_serr_enable", + "type": "on/off" + }, + { + "name": "multifunction", + "type": "on/off" + }, + { + "name": "rombar", + "type": "uint32" + }, + { + "name": "romfile", + "type": "string" + }, + { + "name": "addr", + "type": "pci-devfn" + } + ], + "id": "libvirt-40" +} + { "return": [ { @@ -3700,7 +3726,7 @@ "cpu-max": 255 } ], - "id": "libvirt-40" + "id": "libvirt-41" } { @@ -3793,21 +3819,21 @@ "name": "qemu64" } ], - "id": "libvirt-41" + "id": "libvirt-42" } { "return": [ "tpm-tis" ], - "id": "libvirt-42" + "id": "libvirt-43" } { "return": [ "passthrough" ], - "id": "libvirt-43" + "id": "libvirt-44" } { @@ -4905,7 +4931,7 @@ "option": "drive" } ], - "id": "libvirt-44" + "id": "libvirt-45" } { @@ -4939,7 +4965,7 @@ "capability": "postcopy-ram" } ], - "id": "libvirt-45" + "id": "libvirt-46" } { @@ -13295,7 +13321,7 @@ "meta-type": "object" } ], - "id": "libvirt-46" + "id": "libvirt-47" } { diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml index 939b091fe76d..c892526d9ee0 100644 --- a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml @@ -179,9 +179,10 @@ <flag name='nbd-tls'/> <flag name='sdl-gl'/> <flag name='hda-output'/> + <flag name='mch'/> <version>2007000</version> <kvmVersion>0</kvmVersion> - <microcodeVersion>239029</microcodeVersion> + <microcodeVersion>239395</microcodeVersion> <package> (v2.7.0)</package> <arch>x86_64</arch> <cpu type='kvm' name='Opteron_G5'/> diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.replies index 30d28c7b5f4a..336f9fbca2f6 100644 --- a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.replies @@ -3696,6 +3696,32 @@ "id": "libvirt-39" } +{ + "return": [ + { + "name": "command_serr_enable", + "type": "on/off" + }, + { + "name": "multifunction", + "type": "on/off" + }, + { + "name": "rombar", + "type": "uint32" + }, + { + "name": "romfile", + "type": "string" + }, + { + "name": "addr", + "type": "pci-devfn" + } + ], + "id": "libvirt-40" +} + { "return": [ { @@ -3862,7 +3888,7 @@ "cpu-max": 255 } ], - "id": "libvirt-40" + "id": "libvirt-41" } { @@ -4068,21 +4094,21 @@ "static": false } ], - "id": "libvirt-41" + "id": "libvirt-42" } { "return": [ "tpm-tis" ], - "id": "libvirt-42" + "id": "libvirt-43" } { "return": [ "passthrough" ], - "id": "libvirt-43" + "id": "libvirt-44" } { @@ -5205,7 +5231,7 @@ "option": "drive" } ], - "id": "libvirt-44" + "id": "libvirt-45" } { @@ -5243,7 +5269,7 @@ "capability": "x-colo" } ], - "id": "libvirt-45" + "id": "libvirt-46" } { @@ -14013,7 +14039,7 @@ "meta-type": "object" } ], - "id": "libvirt-46" + "id": "libvirt-47" } { diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml index 7a658e2b6afc..3e8d9ee3a89b 100644 --- a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml @@ -182,9 +182,10 @@ <flag name='sdl-gl'/> <flag name='hda-output'/> <flag name='vhost-vsock'/> + <flag name='mch'/> <version>2008000</version> <kvmVersion>0</kvmVersion> - <microcodeVersion>255684</microcodeVersion> + <microcodeVersion>256050</microcodeVersion> <package> (v2.8.0)</package> <arch>x86_64</arch> <cpu type='kvm' name='host' usable='yes'/> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.replies index 5da1b41c2ee1..b03e3d495045 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.replies @@ -4015,6 +4015,32 @@ "id": "libvirt-39" } +{ + "return": [ + { + "name": "command_serr_enable", + "type": "on/off" + }, + { + "name": "multifunction", + "type": "on/off" + }, + { + "name": "rombar", + "type": "uint32" + }, + { + "name": "romfile", + "type": "string" + }, + { + "name": "addr", + "type": "pci-devfn" + } + ], + "id": "libvirt-40" +} + { "return": [ { @@ -4191,7 +4217,7 @@ "cpu-max": 255 } ], - "id": "libvirt-40" + "id": "libvirt-41" } { @@ -4473,21 +4499,21 @@ "migration-safe": true } ], - "id": "libvirt-41" + "id": "libvirt-42" } { "return": [ "tpm-tis" ], - "id": "libvirt-42" + "id": "libvirt-43" } { "return": [ "passthrough" ], - "id": "libvirt-43" + "id": "libvirt-44" } { @@ -5736,7 +5762,7 @@ "option": "drive" } ], - "id": "libvirt-44" + "id": "libvirt-45" } { @@ -5778,7 +5804,7 @@ "capability": "release-ram" } ], - "id": "libvirt-45" + "id": "libvirt-46" } { @@ -15064,7 +15090,7 @@ "meta-type": "object" } ], - "id": "libvirt-46" + "id": "libvirt-47" } { @@ -15243,7 +15269,7 @@ } } }, - "id": "libvirt-47" + "id": "libvirt-48" } { @@ -15484,7 +15510,7 @@ } } }, - "id": "libvirt-48" + "id": "libvirt-49" } { @@ -15663,7 +15689,7 @@ } } }, - "id": "libvirt-49" + "id": "libvirt-50" } { @@ -15904,7 +15930,7 @@ } } }, - "id": "libvirt-50" + "id": "libvirt-51" } { diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml index 33412b5a9a6b..66d5c60e160c 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml @@ -197,9 +197,10 @@ <flag name='blockdev-del'/> <flag name='vmgenid'/> <flag name='vhost-vsock'/> + <flag name='mch'/> <version>2009000</version> <kvmVersion>0</kvmVersion> - <microcodeVersion>320947</microcodeVersion> + <microcodeVersion>321313</microcodeVersion> <package> (v2.9.0)</package> <arch>x86_64</arch> <hostCPU type='kvm' model='base' migratability='yes'> -- 2.17.1

On Thu, Jun 07, 2018 at 10:37:42AM +0200, Martin Kletzander wrote:
For getting the reply I queried the newest and oldest QEMU using test/qemucapsprobe. From the differences I only extracted the reply to the new QMP command and discarded the rest. For all the versions below the one which added support for the new option I used the output from the oldest QEMU release and for those that support it I used the output from the newest one.
In order to make doubly sure the reply is where it is supposed to be (the replies files are very forgiving) I added the property to all the replies files, reran the tests again and fixed the order in replies files so that all the versions are reporting the new capability. Then removed that one property.
After that I used test/qemucapsfixreplies to fix the reply IDs.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> ---
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

The default is stable per machine type so there should be no need to keep that. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1469338 Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/conf/domain_conf.c | 3 +- src/qemu/qemu_command.c | 18 ++++++++ src/qemu/qemu_domain.c | 35 ++++++++++++++ .../tseg-explicit-size.x86_64-latest.args | 35 ++++++++++++++ tests/qemuxml2argvdata/tseg-explicit-size.xml | 23 ++++++++++ tests/qemuxml2argvdata/tseg-i440fx.xml | 23 ++++++++++ tests/qemuxml2argvdata/tseg-invalid-size.xml | 23 ++++++++++ tests/qemuxml2argvtest.c | 25 ++++++++++ .../qemuxml2xmloutdata/tseg-explicit-size.xml | 46 +++++++++++++++++++ .../tseg-old-machine-type.xml | 44 ++++++++++++++++++ tests/qemuxml2xmloutdata/tseg.xml | 44 ++++++++++++++++++ tests/qemuxml2xmltest.c | 9 ++++ 12 files changed, 327 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/tseg-explicit-size.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/tseg-explicit-size.xml create mode 100644 tests/qemuxml2argvdata/tseg-i440fx.xml create mode 100644 tests/qemuxml2argvdata/tseg-invalid-size.xml create mode 100644 tests/qemuxml2xmloutdata/tseg-explicit-size.xml create mode 100644 tests/qemuxml2xmloutdata/tseg-old-machine-type.xml create mode 100644 tests/qemuxml2xmloutdata/tseg.xml diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 62bf6bb803bb..e83487d6b0de 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -22043,7 +22043,8 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src, return false; } - if (src->tseg_size != dst->tseg_size) { + if (src->tseg_specified && + src->tseg_size != dst->tseg_size) { const char *unit_src, *unit_dst; unsigned long long short_size_src = virFormatIntPretty(src->tseg_size, &unit_src); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6bc9bf5ffab8..4a87b892b7c5 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7295,6 +7295,22 @@ qemuBuildMachineCommandLine(virCommandPtr cmd, return ret; } + +static void +qemuBuildTSEGCommandLine(virCommandPtr cmd, + const virDomainDef *def) +{ + if (!def->tseg_size) + return; + + virCommandAddArg(cmd, "-global"); + + /* PostParse callback guarantees that the size is divisible by 1 MiB */ + virCommandAddArgFormat(cmd, "mch.extended-tseg-mbytes=%llu", + def->tseg_size >> 20); +} + + static int qemuBuildSmpCommandLine(virCommandPtr cmd, virDomainDefPtr def) @@ -10108,6 +10124,8 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (qemuBuildMachineCommandLine(cmd, cfg, def, qemuCaps) < 0) goto error; + qemuBuildTSEGCommandLine(cmd, def); + if (qemuBuildCpuCommandLine(cmd, driver, def, qemuCaps) < 0) goto error; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a2c4d3a36090..643fca52c17b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3632,6 +3632,38 @@ qemuDomainDefCPUPostParse(virDomainDefPtr def) } +static int +qemuDomainDefTsegPostParse(virDomainDefPtr def, + virQEMUCapsPtr qemuCaps) +{ + if (def->features[VIR_DOMAIN_FEATURE_SMM] != VIR_TRISTATE_SWITCH_ON) + return 0; + + if (!def->tseg_size) + return 0; + + if (!qemuDomainIsQ35(def)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("SMM TSEG is only supported with q35 machine type")); + return -1; + } + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MCH_EXTENDED_TSEG_MBYTES)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Setting TSEG size is not supported with this QEMU binary")); + return -1; + } + + if (def->tseg_size & ((1 << 20) - 1)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("SMM TSEG size must be divisible by 1 MiB")); + return -1; + } + + return 0; +} + + static int qemuDomainDefPostParseBasic(virDomainDefPtr def, virCapsPtr caps, @@ -3702,6 +3734,9 @@ qemuDomainDefPostParse(virDomainDefPtr def, if (qemuDomainDefCPUPostParse(def) < 0) goto cleanup; + if (qemuDomainDefTsegPostParse(def, qemuCaps) < 0) + goto cleanup; + ret = 0; cleanup: virObjectUnref(cfg); diff --git a/tests/qemuxml2argvdata/tseg-explicit-size.x86_64-latest.args b/tests/qemuxml2argvdata/tseg-explicit-size.x86_64-latest.args new file mode 100644 index 000000000000..110761b96e0e --- /dev/null +++ b/tests/qemuxml2argvdata/tseg-explicit-size.x86_64-latest.args @@ -0,0 +1,35 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ +-machine pc-q35-2.10,accel=tcg,usb=off,smm=on,dump-guest-core=off \ +-global mch.extended-tseg-mbytes=48 \ +-m 214 \ +-realtime mlock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device pcie-root-port,port=0x8,chassis=1,id=pci.1,bus=pcie.0,multifunction=on,\ +addr=0x1 \ +-device pcie-root-port,port=0x9,chassis=2,id=pci.2,bus=pcie.0,addr=0x1.0x1 \ +-device pcie-root-port,port=0xa,chassis=3,id=pci.3,bus=pcie.0,addr=0x1.0x2 \ +-device qemu-xhci,id=usb,bus=pci.1,addr=0x0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.2,addr=0x0 \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ +resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/tseg-explicit-size.xml b/tests/qemuxml2argvdata/tseg-explicit-size.xml new file mode 100644 index 000000000000..ae3121048495 --- /dev/null +++ b/tests/qemuxml2argvdata/tseg-explicit-size.xml @@ -0,0 +1,23 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc-q35-2.10'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <smm state='on'> + <tseg>48</tseg> + </smm> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/tseg-i440fx.xml b/tests/qemuxml2argvdata/tseg-i440fx.xml new file mode 100644 index 000000000000..5bd832d50829 --- /dev/null +++ b/tests/qemuxml2argvdata/tseg-i440fx.xml @@ -0,0 +1,23 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <smm state='on'> + <tseg unit='MiB'>48</tseg> + </smm> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/tseg-invalid-size.xml b/tests/qemuxml2argvdata/tseg-invalid-size.xml new file mode 100644 index 000000000000..3ac8069a81ce --- /dev/null +++ b/tests/qemuxml2argvdata/tseg-invalid-size.xml @@ -0,0 +1,23 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='q35'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <smm state='on'> + <tseg unit='KiB'>12345</tseg> + </smm> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index cd103b650627..5cdf8bd9fd60 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2852,6 +2852,31 @@ mymain(void) DO_TEST_CAPS_LATEST("disk-virtio-scsi-reservations"); + DO_TEST_CAPS_LATEST("tseg-explicit-size"); + DO_TEST_PARSE_ERROR("tseg-i440fx", + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_ICH9_AHCI, + QEMU_CAPS_MACHINE_SMM_OPT, + QEMU_CAPS_VIRTIO_SCSI, + QEMU_CAPS_MCH_EXTENDED_TSEG_MBYTES); + DO_TEST_PARSE_ERROR("tseg-explicit-size", + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_ICH9_AHCI, + QEMU_CAPS_MACHINE_SMM_OPT, + QEMU_CAPS_VIRTIO_SCSI); + DO_TEST_PARSE_ERROR("tseg-invalid-size", + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_ICH9_AHCI, + QEMU_CAPS_MACHINE_SMM_OPT, + QEMU_CAPS_VIRTIO_SCSI, + QEMU_CAPS_MCH_EXTENDED_TSEG_MBYTES); + /* Test disks with format probing enabled for legacy reasons. * New tests should not go in this section. */ driver.config->allowDiskFormatProbing = true; diff --git a/tests/qemuxml2xmloutdata/tseg-explicit-size.xml b/tests/qemuxml2xmloutdata/tseg-explicit-size.xml new file mode 100644 index 000000000000..e1a6e15b610e --- /dev/null +++ b/tests/qemuxml2xmloutdata/tseg-explicit-size.xml @@ -0,0 +1,46 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc-q35-2.10'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <smm state='on'> + <tseg unit='MiB'>48</tseg> + </smm> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='dmi-to-pci-bridge'> + <model name='i82801b11-bridge'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1e' function='0x0'/> + </controller> + <controller type='pci' index='2' model='pci-bridge'> + <model name='pci-bridge'/> + <target chassisNr='2'/> + <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> + </controller> + <controller type='pci' index='3' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='3' port='0x10'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </controller> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/tseg-old-machine-type.xml b/tests/qemuxml2xmloutdata/tseg-old-machine-type.xml new file mode 100644 index 000000000000..594c5c025d2e --- /dev/null +++ b/tests/qemuxml2xmloutdata/tseg-old-machine-type.xml @@ -0,0 +1,44 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc-q35-2.9'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <smm state='on'/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='dmi-to-pci-bridge'> + <model name='i82801b11-bridge'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1e' function='0x0'/> + </controller> + <controller type='pci' index='2' model='pci-bridge'> + <model name='pci-bridge'/> + <target chassisNr='2'/> + <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> + </controller> + <controller type='pci' index='3' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='3' port='0x10'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </controller> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/tseg.xml b/tests/qemuxml2xmloutdata/tseg.xml new file mode 100644 index 000000000000..ad80648c73e4 --- /dev/null +++ b/tests/qemuxml2xmloutdata/tseg.xml @@ -0,0 +1,44 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc-q35-2.10'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <smm state='on'/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='dmi-to-pci-bridge'> + <model name='i82801b11-bridge'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1e' function='0x0'/> + </controller> + <controller type='pci' index='2' model='pci-bridge'> + <model name='pci-bridge'/> + <target chassisNr='2'/> + <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> + </controller> + <controller type='pci' index='3' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='3' port='0x10'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </controller> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 115db6e64bf6..6d3d8c781999 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1184,6 +1184,15 @@ mymain(void) QEMU_CAPS_DEVICE_VIRTIO_MOUSE_CCW, QEMU_CAPS_DEVICE_VIRTIO_TABLET_CCW); + DO_TEST("tseg-explicit-size", + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_ICH9_AHCI, + QEMU_CAPS_MACHINE_SMM_OPT, + QEMU_CAPS_VIRTIO_SCSI, + QEMU_CAPS_MCH_EXTENDED_TSEG_MBYTES); + /* Test disks with format probing enabled for legacy reasons. * New tests should not go in this section. */ driver.config->allowDiskFormatProbing = true; -- 2.17.1

On Thu, Jun 07, 2018 at 10:37:43AM +0200, Martin Kletzander wrote:
The default is stable per machine type so there should be no need to keep that.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1469338
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/conf/domain_conf.c | 3 +- src/qemu/qemu_command.c | 18 ++++++++ src/qemu/qemu_domain.c | 35 ++++++++++++++ .../tseg-explicit-size.x86_64-latest.args | 35 ++++++++++++++ tests/qemuxml2argvdata/tseg-explicit-size.xml | 23 ++++++++++ tests/qemuxml2argvdata/tseg-i440fx.xml | 23 ++++++++++ tests/qemuxml2argvdata/tseg-invalid-size.xml | 23 ++++++++++ tests/qemuxml2argvtest.c | 25 ++++++++++ .../qemuxml2xmloutdata/tseg-explicit-size.xml | 46 +++++++++++++++++++ .../tseg-old-machine-type.xml | 44 ++++++++++++++++++ tests/qemuxml2xmloutdata/tseg.xml | 44 ++++++++++++++++++ tests/qemuxml2xmltest.c | 9 ++++ 12 files changed, 327 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/tseg-explicit-size.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/tseg-explicit-size.xml create mode 100644 tests/qemuxml2argvdata/tseg-i440fx.xml create mode 100644 tests/qemuxml2argvdata/tseg-invalid-size.xml create mode 100644 tests/qemuxml2xmloutdata/tseg-explicit-size.xml create mode 100644 tests/qemuxml2xmloutdata/tseg-old-machine-type.xml create mode 100644 tests/qemuxml2xmloutdata/tseg.xml
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 62bf6bb803bb..e83487d6b0de 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -22043,7 +22043,8 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src, return false; }
- if (src->tseg_size != dst->tseg_size) { + if (src->tseg_specified &&
Why this change? IIUC if they weren't specified on both sides, they should both be 0 here. If you're sure it's needed, put it in the commit adding this check.
+ src->tseg_size != dst->tseg_size) { const char *unit_src, *unit_dst; unsigned long long short_size_src = virFormatIntPretty(src->tseg_size, &unit_src); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6bc9bf5ffab8..4a87b892b7c5 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7295,6 +7295,22 @@ qemuBuildMachineCommandLine(virCommandPtr cmd, return ret; }
+ +static void +qemuBuildTSEGCommandLine(virCommandPtr cmd, + const virDomainDef *def) +{ + if (!def->tseg_size) + return;
If you don't need the tseg_specified bool at all, I suggest just dropping it. Does a size of 0 MiB make sense? It's divisible by 1 MiB.
+ + virCommandAddArg(cmd, "-global"); + + /* PostParse callback guarantees that the size is divisible by 1 MiB */ + virCommandAddArgFormat(cmd, "mch.extended-tseg-mbytes=%llu", + def->tseg_size >> 20); +} + + static int qemuBuildSmpCommandLine(virCommandPtr cmd, virDomainDefPtr def) @@ -10108,6 +10124,8 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (qemuBuildMachineCommandLine(cmd, cfg, def, qemuCaps) < 0) goto error;
+ qemuBuildTSEGCommandLine(cmd, def); + if (qemuBuildCpuCommandLine(cmd, driver, def, qemuCaps) < 0) goto error;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a2c4d3a36090..643fca52c17b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3632,6 +3632,38 @@ qemuDomainDefCPUPostParse(virDomainDefPtr def) }
+static int +qemuDomainDefTsegPostParse(virDomainDefPtr def,
s/qemuDomainDefTsegPostParse/qemuDomainDefTSEGPostParse/
+ virQEMUCapsPtr qemuCaps) +{ + if (def->features[VIR_DOMAIN_FEATURE_SMM] != VIR_TRISTATE_SWITCH_ON) + return 0; +
With the bool variable dropped or defended and consistently used: Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

On 06/07/18 21:58, Ján Tomko wrote:
Does a size of 0 MiB make sense? It's divisible by 1 MiB.
"-global mch.extended-tseg-mbytes=0" makes QEMU behave as if it lacked the extended TSEG feature; the guest will believe that only the standard 1 / 2 / 8 MB TSEG sizes are available, and pick one of those. Technically, in QEMU the extended TSEG feature is disabled for older machine types by setting "mch.extended-tseg-mbytes" to zero: [include/hw/i386/pc.h] #define PC_COMPAT_2_9 \ HW_COMPAT_2_9 \ {\ .driver = "mch",\ .property = "extended-tseg-mbytes",\ .value = stringify(0),\ },\ I'm unsure whether this -- i.e., disabling the feature -- is useful to expose through the domain config. Probably not. Thanks Laszlo

On Thu, Jun 07, 2018 at 10:14:38PM +0200, Laszlo Ersek wrote:
On 06/07/18 21:58, Ján Tomko wrote:
Does a size of 0 MiB make sense? It's divisible by 1 MiB.
"-global mch.extended-tseg-mbytes=0" makes QEMU behave as if it lacked the extended TSEG feature; the guest will believe that only the standard 1 / 2 / 8 MB TSEG sizes are available, and pick one of those.
Technically, in QEMU the extended TSEG feature is disabled for older machine types by setting "mch.extended-tseg-mbytes" to zero:
[include/hw/i386/pc.h]
#define PC_COMPAT_2_9 \ HW_COMPAT_2_9 \ {\ .driver = "mch",\ .property = "extended-tseg-mbytes",\ .value = stringify(0),\ },\
I'm unsure whether this -- i.e., disabling the feature -- is useful to expose through the domain config. Probably not.
I wouldn't want to disable that option now in a way that we won't be able to fix later. So from my point of view it makes sense.

On Thu, Jun 07, 2018 at 09:58:04PM +0200, Ján Tomko wrote:
On Thu, Jun 07, 2018 at 10:37:43AM +0200, Martin Kletzander wrote:
The default is stable per machine type so there should be no need to keep that.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1469338
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/conf/domain_conf.c | 3 +- src/qemu/qemu_command.c | 18 ++++++++ src/qemu/qemu_domain.c | 35 ++++++++++++++ .../tseg-explicit-size.x86_64-latest.args | 35 ++++++++++++++ tests/qemuxml2argvdata/tseg-explicit-size.xml | 23 ++++++++++ tests/qemuxml2argvdata/tseg-i440fx.xml | 23 ++++++++++ tests/qemuxml2argvdata/tseg-invalid-size.xml | 23 ++++++++++ tests/qemuxml2argvtest.c | 25 ++++++++++ .../qemuxml2xmloutdata/tseg-explicit-size.xml | 46 +++++++++++++++++++ .../tseg-old-machine-type.xml | 44 ++++++++++++++++++ tests/qemuxml2xmloutdata/tseg.xml | 44 ++++++++++++++++++ tests/qemuxml2xmltest.c | 9 ++++ 12 files changed, 327 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/tseg-explicit-size.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/tseg-explicit-size.xml create mode 100644 tests/qemuxml2argvdata/tseg-i440fx.xml create mode 100644 tests/qemuxml2argvdata/tseg-invalid-size.xml create mode 100644 tests/qemuxml2xmloutdata/tseg-explicit-size.xml create mode 100644 tests/qemuxml2xmloutdata/tseg-old-machine-type.xml create mode 100644 tests/qemuxml2xmloutdata/tseg.xml
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 62bf6bb803bb..e83487d6b0de 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -22043,7 +22043,8 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src, return false; }
- if (src->tseg_size != dst->tseg_size) { + if (src->tseg_specified &&
Why this change?
IIUC if they weren't specified on both sides, they should both be 0 here.
If you're sure it's needed, put it in the commit adding this check.
Oh, it is very easy to explain. The first line (the one being removed) is from the previous version of this series. The second one (which is being added) is from version two in which case I used fixup during interactive rebase on a wrong commit. Otherwise this would be part of the commit in which this was added. And as said in the other reply to Laszlo's e-mail (which I had to separate due to this par tof review being omitted there) it makes sense for the tseg size to be 0 as that means the extended size is disabled. I agree that it should be part of the documentation, though. @Laszlo: When thinking about it, even though it is a very stupid idea, technically there isn't really a difference between 'extended-tseg-mbytes=0' and 'extended-tseg-mbytes=8' (of course the second option will still provide a fw_cfg info about the size etc., but the guest will behave the same way), right? That brings me to another question (maybe even more stupid), what size does OVMF choose if I specify 'extended-tseg-mbytes=6'?

On 06/07/18 23:14, Martin Kletzander wrote:
@Laszlo: When thinking about it, even though it is a very stupid idea, technically there isn't really a difference between 'extended-tseg-mbytes=0' and 'extended-tseg-mbytes=8'
that's correct
(of course the second option will still provide a fw_cfg info about the size etc., but the guest will behave the same way), right?
- it's not fw_cfg but a made-up register in PCI config space - the guest won't behave *exactly* the same way, technically speaking, but the end result will be 8MB in both cases, yes.
That brings me to another question (maybe even more stupid), what size does OVMF choose if I specify 'extended-tseg-mbytes=6'?
6MB. Thanks, Laszlo

Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- docs/news.xml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 6e079e8321ff..5df8f7c579e9 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -44,6 +44,14 @@ its own virtual TPM. </description> </change> + <change> + <summary> + qemu: Add support for extended TSEG size + </summary> + <description> + Support specifying extended TSEG size for SMM in QEMU. + </description> + </change> </section> <section title="Improvements"> <change> -- 2.17.1

On Thu, Jun 07, 2018 at 03:59:58PM +0200, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- docs/news.xml | 8 ++++++++ 1 file changed, 8 insertions(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (3)
-
Ján Tomko
-
Laszlo Ersek
-
Martin Kletzander