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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
Thanks,
Laszlo
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
