[libvirt PATCHv2 3/3] qemu: allow forcing emulated maxphysaddr

Ján Tomko posted 3 patches 2 years, 9 months ago
[libvirt PATCHv2 3/3] qemu: allow forcing emulated maxphysaddr
Posted by Ján Tomko 2 years, 9 months ago
Treat:
  <maxphysaddr mode="emulate"/>
as a request not to take the maximum address size from the host.
This is useful if QEMU changes the default.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
---
 docs/formatdomain.rst                         |  3 +-
 src/qemu/qemu_command.c                       |  5 ++-
 src/qemu/qemu_validate.c                      |  7 -----
 ...-phys-bits-emulate-bare.x86_64-latest.args | 13 ++++----
 .../cpu-phys-bits-emulate-bare.xml            | 20 ++++++++++++
 .../cpu-phys-bits-emulate3.err                |  1 -
 .../cpu-phys-bits-emulate3.xml                | 20 ------------
 tests/qemuxml2argvtest.c                      |  2 +-
 ...u-phys-bits-emulate-bare.x86_64-latest.xml | 31 +++++++++++++++++++
 tests/qemuxml2xmltest.c                       |  1 +
 10 files changed, 65 insertions(+), 38 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/cpu-phys-bits-emulate-bare.xml
 delete mode 100644 tests/qemuxml2argvdata/cpu-phys-bits-emulate3.err
 delete mode 100644 tests/qemuxml2argvdata/cpu-phys-bits-emulate3.xml
 create mode 100644 tests/qemuxml2xmloutdata/cpu-phys-bits-emulate-bare.x86_64-latest.xml

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index f5bbfdc20a..2e8d31dd74 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -1636,7 +1636,8 @@ In case no restrictions need to be put on CPU model and its features, a simpler
          passed through to the virtual CPUs
       ``emulate``
          The hypervisor will define a specific value for the number of bits
-         of physical addresses via the ``bits`` attribute, which is mandatory.
+         of physical addresses via the ``bits`` attribute, which is optional
+         :since:`since 9.2.0`
 	 The number of bits cannot exceed the number of physical address bits
 	 supported by the hypervisor.
 
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1d99f8a46a..1356b6e975 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6530,7 +6530,10 @@ qemuBuildCpuCommandLine(virCommand *cmd,
             break;
 
         case VIR_CPU_MAX_PHYS_ADDR_MODE_EMULATE:
-            virBufferAsprintf(&buf, ",phys-bits=%d", addr->bits);
+            if (addr->bits > 0)
+                virBufferAsprintf(&buf, ",phys-bits=%d", addr->bits);
+            else
+                virBufferAddLit(&buf, ",host-phys-bits=off");
             break;
 
         case VIR_CPU_MAX_PHYS_ADDR_MODE_LAST:
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index fdfb4c6407..da4b9a3b35 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -269,13 +269,6 @@ qemuValidateDomainDefCpu(virQEMUDriver *driver,
             break;
 
         case VIR_CPU_MAX_PHYS_ADDR_MODE_EMULATE:
-            if (addr->bits == -1) {
-                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                               _("if using CPU maximum physical address mode='%1$s', bits= must be specified too"),
-                               virCPUMaxPhysAddrModeTypeToString(VIR_CPU_MAX_PHYS_ADDR_MODE_EMULATE));
-                return -1;
-            }
-
             if (driver->hostcpu &&
                 driver->hostcpu->addr &&
                 cpu->addr->bits > driver->hostcpu->addr->bits) {
diff --git a/tests/qemuxml2argvdata/cpu-phys-bits-emulate-bare.x86_64-latest.args b/tests/qemuxml2argvdata/cpu-phys-bits-emulate-bare.x86_64-latest.args
index 3fa8384ace..28578fac20 100644
--- a/tests/qemuxml2argvdata/cpu-phys-bits-emulate-bare.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/cpu-phys-bits-emulate-bare.x86_64-latest.args
@@ -1,16 +1,16 @@
 LC_ALL=C \
 PATH=/bin \
-HOME=/tmp/lib/domain--1-foo \
+HOME=/var/lib/libvirt/qemu/domain--1-foo \
 USER=test \
 LOGNAME=test \
-XDG_DATA_HOME=/tmp/lib/domain--1-foo/.local/share \
-XDG_CACHE_HOME=/tmp/lib/domain--1-foo/.cache \
-XDG_CONFIG_HOME=/tmp/lib/domain--1-foo/.config \
+XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-foo/.local/share \
+XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-foo/.cache \
+XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-foo/.config \
 /usr/bin/qemu-system-x86_64 \
 -name guest=foo,debug-threads=on \
 -S \
--object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-foo/master-key.aes"}' \
--machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram \
+-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-foo/master-key.aes"}' \
+-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram,acpi=off \
 -accel kvm \
 -cpu host,migratable=on,host-phys-bits=off \
 -m 214 \
@@ -25,7 +25,6 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-foo/.config \
 -mon chardev=charmonitor,id=monitor,mode=control \
 -rtc base=utc \
 -no-shutdown \
--no-acpi \
 -boot strict=on \
 -device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \
 -audiodev '{"id":"audio1","driver":"none"}' \
diff --git a/tests/qemuxml2argvdata/cpu-phys-bits-emulate-bare.xml b/tests/qemuxml2argvdata/cpu-phys-bits-emulate-bare.xml
new file mode 100644
index 0000000000..30a14894dd
--- /dev/null
+++ b/tests/qemuxml2argvdata/cpu-phys-bits-emulate-bare.xml
@@ -0,0 +1,20 @@
+<domain type='kvm'>
+  <name>foo</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='x86_64' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <cpu mode='host-passthrough'>
+    <maxphysaddr mode='emulate'/>
+  </cpu>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/cpu-phys-bits-emulate3.err b/tests/qemuxml2argvdata/cpu-phys-bits-emulate3.err
deleted file mode 100644
index 5e21998259..0000000000
--- a/tests/qemuxml2argvdata/cpu-phys-bits-emulate3.err
+++ /dev/null
@@ -1 +0,0 @@
-unsupported configuration: if using CPU maximum physical address mode='emulate', bits= must be specified too
diff --git a/tests/qemuxml2argvdata/cpu-phys-bits-emulate3.xml b/tests/qemuxml2argvdata/cpu-phys-bits-emulate3.xml
deleted file mode 100644
index 30a14894dd..0000000000
--- a/tests/qemuxml2argvdata/cpu-phys-bits-emulate3.xml
+++ /dev/null
@@ -1,20 +0,0 @@
-<domain type='kvm'>
-  <name>foo</name>
-  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
-  <memory unit='KiB'>219136</memory>
-  <currentMemory unit='KiB'>219136</currentMemory>
-  <vcpu placement='static'>1</vcpu>
-  <os>
-    <type arch='x86_64' machine='pc'>hvm</type>
-    <boot dev='hd'/>
-  </os>
-  <cpu mode='host-passthrough'>
-    <maxphysaddr mode='emulate'/>
-  </cpu>
-  <clock offset='utc'/>
-  <on_poweroff>destroy</on_poweroff>
-  <on_reboot>restart</on_reboot>
-  <on_crash>destroy</on_crash>
-  <devices>
-  </devices>
-</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 48ddc102f7..270d6fa59e 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2720,9 +2720,9 @@ mymain(void)
     DO_TEST("cpu-phys-bits-passthrough", QEMU_CAPS_KVM);
     DO_TEST("cpu-phys-bits-emulate", QEMU_CAPS_KVM);
     DO_TEST("cpu-phys-bits-emulate2", QEMU_CAPS_KVM);
-    DO_TEST_PARSE_ERROR("cpu-phys-bits-emulate3", QEMU_CAPS_KVM);
     DO_TEST_PARSE_ERROR("cpu-phys-bits-passthrough2", QEMU_CAPS_KVM);
     DO_TEST_CAPS_LATEST("cpu-phys-bits-limit");
+    DO_TEST_CAPS_LATEST("cpu-phys-bits-emulate-bare");
 
     DO_TEST_CAPS_VER("sgx-epc", "7.0.0");
 
diff --git a/tests/qemuxml2xmloutdata/cpu-phys-bits-emulate-bare.x86_64-latest.xml b/tests/qemuxml2xmloutdata/cpu-phys-bits-emulate-bare.x86_64-latest.xml
new file mode 100644
index 0000000000..94b3627a46
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/cpu-phys-bits-emulate-bare.x86_64-latest.xml
@@ -0,0 +1,31 @@
+<domain type='kvm'>
+  <name>foo</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='x86_64' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <cpu mode='host-passthrough' check='none' migratable='on'>
+    <maxphysaddr mode='emulate'/>
+  </cpu>
+  <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='usb' index='0' model='piix3-uhci'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+    </controller>
+    <controller type='pci' index='0' model='pci-root'/>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <audio id='1' type='none'/>
+    <memballoon model='virtio'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
+    </memballoon>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 598a8d9065..066c5a49c4 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1248,6 +1248,7 @@ mymain(void)
     DO_TEST_CAPS_LATEST("crypto-builtin");
 
     DO_TEST_CAPS_LATEST("cpu-phys-bits-limit");
+    DO_TEST_CAPS_LATEST("cpu-phys-bits-emulate-bare");
 
  cleanup:
     qemuTestDriverFree(&driver);
-- 
2.39.2

Re: [libvirt PATCHv2 3/3] qemu: allow forcing emulated maxphysaddr
Posted by Martin Kletzander 2 years, 9 months ago
On Mon, Apr 24, 2023 at 03:40:04PM +0200, Ján Tomko wrote:
>Treat:
>  <maxphysaddr mode="emulate"/>
>as a request not to take the maximum address size from the host.
>This is useful if QEMU changes the default.
>
>Signed-off-by: Ján Tomko <jtomko@redhat.com>
>---
> docs/formatdomain.rst                         |  3 +-
> src/qemu/qemu_command.c                       |  5 ++-
> src/qemu/qemu_validate.c                      |  7 -----
> ...-phys-bits-emulate-bare.x86_64-latest.args | 13 ++++----
> .../cpu-phys-bits-emulate-bare.xml            | 20 ++++++++++++
> .../cpu-phys-bits-emulate3.err                |  1 -
> .../cpu-phys-bits-emulate3.xml                | 20 ------------
> tests/qemuxml2argvtest.c                      |  2 +-
> ...u-phys-bits-emulate-bare.x86_64-latest.xml | 31 +++++++++++++++++++
> tests/qemuxml2xmltest.c                       |  1 +
> 10 files changed, 65 insertions(+), 38 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/cpu-phys-bits-emulate-bare.xml
> delete mode 100644 tests/qemuxml2argvdata/cpu-phys-bits-emulate3.err
> delete mode 100644 tests/qemuxml2argvdata/cpu-phys-bits-emulate3.xml
> create mode 100644 tests/qemuxml2xmloutdata/cpu-phys-bits-emulate-bare.x86_64-latest.xml
>
>diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
>index f5bbfdc20a..2e8d31dd74 100644
>--- a/docs/formatdomain.rst
>+++ b/docs/formatdomain.rst
>@@ -1636,7 +1636,8 @@ In case no restrictions need to be put on CPU model and its features, a simpler
>          passed through to the virtual CPUs
>       ``emulate``
>          The hypervisor will define a specific value for the number of bits
>-         of physical addresses via the ``bits`` attribute, which is mandatory.
>+         of physical addresses via the ``bits`` attribute, which is optional
>+         :since:`since 9.2.0`

What does this `since` mean?  Attribute mode='emulate' was introduced
together with <maxphysaddr/> in 8.7.0, but the optional part (which I
presume you meant to be related to the `since`) will not be in 9.2.0,
AFAIK we are entering a freeze for 9.3.0 tomorrow.

It might look better like this maybe:

          of physical addresses via the ``bits`` attribute (optional
          :since:`since 9.2.0`)

but I guess both are fine, it's easy to check.

>diff --git a/tests/qemuxml2argvdata/cpu-phys-bits-emulate-bare.x86_64-latest.args b/tests/qemuxml2argvdata/cpu-phys-bits-emulate-bare.x86_64-latest.args
>index 3fa8384ace..28578fac20 100644
>--- a/tests/qemuxml2argvdata/cpu-phys-bits-emulate-bare.x86_64-latest.args
>+++ b/tests/qemuxml2argvdata/cpu-phys-bits-emulate-bare.x86_64-latest.args
>@@ -1,16 +1,16 @@
> LC_ALL=C \
> PATH=/bin \
>-HOME=/tmp/lib/domain--1-foo \
>+HOME=/var/lib/libvirt/qemu/domain--1-foo \
> USER=test \
> LOGNAME=test \
>-XDG_DATA_HOME=/tmp/lib/domain--1-foo/.local/share \
>-XDG_CACHE_HOME=/tmp/lib/domain--1-foo/.cache \
>-XDG_CONFIG_HOME=/tmp/lib/domain--1-foo/.config \
>+XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-foo/.local/share \
>+XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-foo/.cache \
>+XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-foo/.config \
> /usr/bin/qemu-system-x86_64 \
> -name guest=foo,debug-threads=on \
> -S \
>--object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-foo/master-key.aes"}' \
>--machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram \
>+-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-foo/master-key.aes"}' \
>+-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram,acpi=off \
> -accel kvm \
> -cpu host,migratable=on,host-phys-bits=off \
> -m 214 \
>@@ -25,7 +25,6 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-foo/.config \
> -mon chardev=charmonitor,id=monitor,mode=control \
> -rtc base=utc \
> -no-shutdown \
>--no-acpi \
> -boot strict=on \
> -device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \
> -audiodev '{"id":"audio1","driver":"none"}' \

This should just be an addition once the file from 2/3 is merged with
this patch.

With this ^^ fixed

Reviewed-by: Martin Kletzander <mkletzan@redhat.com>