1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH 0/4] TSEG v2
  4. View patch

[libvirt] [PATCH 4/4] qemu: Add support for setting the TSEG size

Martin Kletzander posted 4 patches 6 years, 11 months ago
[libvirt] [PATCH 4/4] qemu: Add support for setting the TSEG size
Posted by Martin Kletzander 6 years, 11 months ago 
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] qemu: Add support for setting the TSEG size
Posted by Ján Tomko 6 years, 11 months ago 
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] qemu: Add support for setting the TSEG size
Posted by Martin Kletzander 6 years, 11 months ago 
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'?
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] qemu: Add support for setting the TSEG size
Posted by Laszlo Ersek 6 years, 11 months ago 
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] qemu: Add support for setting the TSEG size
Posted by Laszlo Ersek 6 years, 11 months ago 
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] qemu: Add support for setting the TSEG size
Posted by Martin Kletzander 6 years, 11 months ago 
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.--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.