[pve-devel] [PATCH qemu-server/docs/manager v6 0/4] vIOMMU-Feature #3784

[pve-devel] [PATCH qemu-server/docs/manager v6 0/4] vIOMMU-Feature #3784

[Markus Frank]

qemu-server:

v6:
* added helper function for machine config validation
* replaced old standard option for "pve-qemu-machine" to new property string

v5:
* set $kvm to 1 if is_native, so that api kvm check works.

v4:
* added kvm/q35 checks in API
* reused pve-qemu-machine

v3:
* replaced old machine type with property-string with viommu-parameter

v2:
* moved viommu-parameter inside of machine_fmt and added it the new
parameter machine_properties
new Config -> machine_properties: viommu=1,etc
* check if kvm and q35 are set


Markus Frank (2):
  feature #3784: Parameter for guest vIOMMU & machine as property-string
  tests: test-cases for new machine-syntax & viommu

 PVE/API2/Qemu.pm                     | 11 ++++-
 PVE/QemuConfig.pm                    |  3 +-
 PVE/QemuServer.pm                    | 62 ++++++++++++++++++++++++++--
 PVE/QemuServer/Machine.pm            |  6 ++-
 test/cfg2cmd/q35-viommu-alt.conf     |  1 +
 test/cfg2cmd/q35-viommu-alt.conf.cmd | 23 +++++++++++
 test/cfg2cmd/q35-viommu.conf         |  1 +
 test/cfg2cmd/q35-viommu.conf.cmd     | 23 +++++++++++
 8 files changed, 122 insertions(+), 8 deletions(-)
 create mode 100644 test/cfg2cmd/q35-viommu-alt.conf
 create mode 100644 test/cfg2cmd/q35-viommu-alt.conf.cmd
 create mode 100644 test/cfg2cmd/q35-viommu.conf
 create mode 100644 test/cfg2cmd/q35-viommu.conf.cmd


docs:

v6:
* changed capitalization and rephrased the text a bit.

v5:
* changed Host and VM Requirements

Markus Frank (1):
  add vIOMMU documentation

 qm-pci-passthrough.adoc | 31 +++++++++++++++++++++++++++++++
 qm.adoc                 |  1 +
 2 files changed, 32 insertions(+)


manager:

v6:
* replaced '=== "1"' check with PVE.Parser.parseBoolean
* replaced hidden kvm ui with view property
* a few style changes

v5:
* added check if kvm is undefined or null

v4:
* check if kvm is enabled
* added kvm+q35 hint

Markus Frank (1):
  ui: MachineEdit with viommu checkbox

 www/manager6/qemu/MachineEdit.js | 49 ++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

-- 
2.39.2

[pve-devel] [PATCH qemu-server v6 1/4] feature #3784: Parameter for guest vIOMMU & machine as property-string

[Markus Frank]

vIOMMU enables the option to passthrough pci devices to L2 VMs
in L1 VMs via Nested Virtualisation.

QEMU-Parameters:
https://www.qemu.org/docs/master/system/qemu-manpage.html
https://wiki.qemu.org/Features/VT-d

-machine ...,kernel-irqchip=split:

"split" because of intremap see below.

-device intel-iommu:

* caching-mode=on:

"It is required for -device vfio-pci to work with the VT-d device, because host
assigned devices requires to setup the DMA mapping on the host before guest DMA
starts."

* intremap=on:

"This enables interrupt remapping feature. It's required to enable complete
x2apic. Currently it only supports kvm kernel-irqchip modes off or split, while
full kernel-irqchip is not yet supported."

Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
 PVE/API2/Qemu.pm          | 11 +++++--
 PVE/QemuConfig.pm         |  3 +-
 PVE/QemuServer.pm         | 62 +++++++++++++++++++++++++++++++++++++--
 PVE/QemuServer/Machine.pm |  6 ++--
 4 files changed, 74 insertions(+), 8 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 9606e72..a968f40 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -1043,13 +1043,16 @@ __PACKAGE__->register_method({
 			$conf->{vmgenid} = PVE::QemuServer::generate_uuid();
 		    }
 
-		    my $machine = $conf->{machine};
+		    my $machine_conf = PVE::QemuServer::parse_machine($conf->{machine});
+		    my $machine = $machine_conf->{type};
 		    if (!$machine || $machine =~ m/^(?:pc|q35|virt)$/) {
 			# always pin Windows' machine version on create, they get to easily confused
 			if (PVE::QemuServer::Helpers::windows_version($conf->{ostype})) {
-			    $conf->{machine} = PVE::QemuServer::windows_get_pinned_machine_version($machine);
+			    $machine_conf->{type} = PVE::QemuServer::windows_get_pinned_machine_version($machine);
+			    $conf->{machine} = PVE::QemuServer::print_machine($machine_conf);
 			}
 		    }
+		    PVE::QemuServer::check_machine_config($conf, $machine_conf);
 
 		    PVE::QemuConfig->write_config($vmid, $conf);
 
@@ -1880,6 +1883,10 @@ my $update_vm_api  = sub {
 			);
 		    }
 		    $conf->{pending}->{$opt} = $param->{$opt};
+		} elsif ($opt eq 'machine') {
+		    my $machine_conf = PVE::QemuServer::parse_machine($param->{$opt});
+		    PVE::QemuServer::check_machine_config($conf, $machine_conf);
+		    $conf->{pending}->{$opt} = $param->{$opt};
 		} else {
 		    $conf->{pending}->{$opt} = $param->{$opt};
 
diff --git a/PVE/QemuConfig.pm b/PVE/QemuConfig.pm
index 10e6929..c4834a7 100644
--- a/PVE/QemuConfig.pm
+++ b/PVE/QemuConfig.pm
@@ -433,7 +433,8 @@ sub __snapshot_rollback_hook {
 	} else {
 	    # Note: old code did not store 'machine', so we try to be smart
 	    # and guess the snapshot was generated with kvm 1.4 (pc-i440fx-1.4).
-	    $data->{forcemachine} = $conf->{machine} || 'pc-i440fx-1.4';
+	    my $machine_conf = PVE::QemuServer::parse_machine($conf->{machine});
+	    $data->{forcemachine} = $machine_conf->{type} || 'pc-i440fx-1.4';
 
 	    # we remove the 'machine' configuration if not explicitly specified
 	    # in the original config.
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index bf1de17..013792d 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -118,12 +118,32 @@ PVE::JSONSchema::register_standard_option('pve-qm-stateuri', {
     optional => 1,
 });
 
-PVE::JSONSchema::register_standard_option('pve-qemu-machine', {
+my $machine_fmt = {
+    type => {
+	default_key => 1,
 	description => "Specifies the QEMU machine type.",
 	type => 'string',
 	pattern => '(pc|pc(-i440fx)?-\d+(\.\d+)+(\+pve\d+)?(\.pxe)?|q35|pc-q35-\d+(\.\d+)+(\+pve\d+)?(\.pxe)?|virt(?:-\d+(\.\d+)+)?(\+pve\d+)?)',
 	maxLength => 40,
+	format_description => 'machine type',
 	optional => 1,
+    },
+    viommu => {
+	type => 'boolean',
+	description => "Enable/disable guest vIOMMU"
+	    ." (needs kvm to be enabled and q35 to be set as machine type).",
+	default => 0,
+	optional => 1,
+    },
+};
+
+PVE::JSONSchema::register_format('pve-qemu-machine-fmt', $machine_fmt);
+
+PVE::JSONSchema::register_standard_option('pve-qemu-machine', {
+    description => "Specify the QEMU machine type & enable/disable vIOMMU.",
+    type => 'string',
+    optional => 1,
+    format => PVE::JSONSchema::get_format('pve-qemu-machine-fmt'),
 });
 
 # FIXME: remove in favor of just using the INotify one, it's cached there exactly the same way
@@ -2133,6 +2153,31 @@ sub parse_watchdog {
     return $res;
 }
 
+sub parse_machine {
+    my ($value) = @_;
+
+    return if !$value;
+
+    my $res = parse_property_string($machine_fmt, $value);
+    return $res;
+}
+
+sub check_machine_config {
+    my ($conf, $machine_conf) = @_;
+    my $q35 = $machine_conf->{type} && ($machine_conf->{type} =~ m/q35/) ? 1 : 0;
+    my $kvm = $conf->{kvm};
+    my $arch = get_vm_arch($conf);
+    $kvm //= 1 if is_native($arch);
+    if ($machine_conf->{viommu} && (!$kvm || !$q35)) {
+	die "to use vIOMMU please enable kvm and set the machine type to q35\n";
+    }
+}
+
+sub print_machine {
+    my ($machine_conf) = @_;
+    return PVE::JSONSchema::print_property_string($machine_conf, $machine_fmt);
+}
+
 sub parse_guest_agent {
     my ($conf) = @_;
 
@@ -2204,8 +2249,9 @@ sub qemu_created_version_fixups {
     # check if we need to apply some handling for VMs that always use the latest machine version but
     # had a machine version transition happen that affected HW such that, e.g., an OS config change
     # would be required (we do not want to pin machine version for non-windows OS type)
+    my $machine_conf = parse_machine($conf->{machine});
     if (
-	(!defined($conf->{machine}) || $conf->{machine} =~ m/^(?:pc|q35|virt)$/) # non-versioned machine
+	(!defined($machine_conf->{type}) || $machine_conf->{type} =~ m/^(?:pc|q35|virt)$/) # non-versioned machine
 	&& (!defined($meta->{'creation-qemu'}) || !min_version($meta->{'creation-qemu'}, 6, 1)) # created before 6.1
 	&& (!$forced_vers || min_version($forced_vers, 6, 1)) # handle snapshot-rollback/migrations
 	&& min_version($kvmver, 6, 1) # only need to apply the change since 6.1
@@ -3364,7 +3410,8 @@ sub windows_get_pinned_machine_version {
 sub get_vm_machine {
     my ($conf, $forcemachine, $arch, $add_pve_version, $kvmversion) = @_;
 
-    my $machine = $forcemachine || $conf->{machine};
+    my $machine_conf = parse_machine($conf->{machine});
+    my $machine = $forcemachine || $machine_conf->{type};
 
     if (!$machine || $machine =~ m/^(?:pc|q35|virt)$/) {
 	$kvmversion //= kvm_user_version();
@@ -3609,6 +3656,8 @@ sub config_to_command {
     my $kvm = $conf->{kvm};
     my $nodename = nodename();
 
+    my $machine_conf = parse_machine($conf->{machine});
+
     my $arch = get_vm_arch($conf);
     my $kvm_binary = get_command_for_arch($arch);
     my $kvmver = kvm_user_version($kvm_binary);
@@ -4174,6 +4223,13 @@ sub config_to_command {
     }
     push @$machineFlags, "type=${machine_type_min}";
 
+    check_machine_config($conf, $machine_conf);
+
+    if ($machine_conf->{viommu}) {
+	unshift @$devices, '-device', "intel-iommu,intremap=on,caching-mode=on";
+	push @$machineFlags, 'kernel-irqchip=split';
+    }
+
     push @$cmd, @$devices;
     push @$cmd, '-rtc', join(',', @$rtcFlags) if scalar(@$rtcFlags);
     push @$cmd, '-machine', join(',', @$machineFlags) if scalar(@$machineFlags);
diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
index d9429ed..bfbde59 100644
--- a/PVE/QemuServer/Machine.pm
+++ b/PVE/QemuServer/Machine.pm
@@ -15,7 +15,8 @@ our $PVE_MACHINE_VERSION = {
 sub machine_type_is_q35 {
     my ($conf) = @_;
 
-    return $conf->{machine} && ($conf->{machine} =~ m/q35/) ? 1 : 0;
+    my $machine_conf = PVE::QemuServer::parse_machine($conf->{machine});
+    return $machine_conf->{type} && ($machine_conf->{type} =~ m/q35/) ? 1 : 0;
 }
 
 sub current_from_query_machines {
@@ -120,7 +121,8 @@ sub qemu_machine_pxe {
 
     my $machine =  get_current_qemu_machine($vmid);
 
-    if ($conf->{machine} && $conf->{machine} =~ m/\.pxe$/) {
+    my $machine_conf = PVE::QemuServer::parse_machine($conf->{machine});
+    if ($machine_conf->{type} && $machine_conf->{type} =~ m/\.pxe$/) {
 	$machine .= '.pxe';
     }
 
-- 
2.39.2

[pve-devel] [PATCH qemu-server v6 2/4] tests: test-cases for new machine-syntax & viommu

[Markus Frank]

two test-cases for the new machine parameter with viommu

Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
 test/cfg2cmd/q35-viommu-alt.conf     |  1 +
 test/cfg2cmd/q35-viommu-alt.conf.cmd | 23 +++++++++++++++++++++++
 test/cfg2cmd/q35-viommu.conf         |  1 +
 test/cfg2cmd/q35-viommu.conf.cmd     | 23 +++++++++++++++++++++++
 4 files changed, 48 insertions(+)
 create mode 100644 test/cfg2cmd/q35-viommu-alt.conf
 create mode 100644 test/cfg2cmd/q35-viommu-alt.conf.cmd
 create mode 100644 test/cfg2cmd/q35-viommu.conf
 create mode 100644 test/cfg2cmd/q35-viommu.conf.cmd

diff --git a/test/cfg2cmd/q35-viommu-alt.conf b/test/cfg2cmd/q35-viommu-alt.conf
new file mode 100644
index 0000000..44d38e2
--- /dev/null
+++ b/test/cfg2cmd/q35-viommu-alt.conf
@@ -0,0 +1 @@
+machine: q35,viommu=1
diff --git a/test/cfg2cmd/q35-viommu-alt.conf.cmd b/test/cfg2cmd/q35-viommu-alt.conf.cmd
new file mode 100644
index 0000000..24e873d
--- /dev/null
+++ b/test/cfg2cmd/q35-viommu-alt.conf.cmd
@@ -0,0 +1,23 @@
+/usr/bin/kvm \
+  -id 8006 \
+  -name 'vm8006,debug-threads=on' \
+  -no-shutdown \
+  -chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server=on,wait=off' \
+  -mon 'chardev=qmp,mode=control' \
+  -chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
+  -mon 'chardev=qmp-event,mode=control' \
+  -pidfile /var/run/qemu-server/8006.pid \
+  -daemonize \
+  -smp '1,sockets=1,cores=1,maxcpus=1' \
+  -nodefaults \
+  -boot 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg' \
+  -vnc 'unix:/var/run/qemu-server/8006.vnc,password=on' \
+  -cpu kvm64,enforce,+kvm_pv_eoi,+kvm_pv_unhalt,+lahf_lm,+sep \
+  -m 512 \
+  -device 'intel-iommu,intremap=on,caching-mode=on' \
+  -readconfig /usr/share/qemu-server/pve-q35-4.0.cfg \
+  -device 'usb-tablet,id=tablet,bus=ehci.0,port=1' \
+  -device 'VGA,id=vga,bus=pcie.0,addr=0x1' \
+  -device 'virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3,free-page-reporting=on' \
+  -iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \
+  -machine 'type=q35+pve0,kernel-irqchip=split'
diff --git a/test/cfg2cmd/q35-viommu.conf b/test/cfg2cmd/q35-viommu.conf
new file mode 100644
index 0000000..6925a74
--- /dev/null
+++ b/test/cfg2cmd/q35-viommu.conf
@@ -0,0 +1 @@
+machine: type=q35,viommu=1
diff --git a/test/cfg2cmd/q35-viommu.conf.cmd b/test/cfg2cmd/q35-viommu.conf.cmd
new file mode 100644
index 0000000..24e873d
--- /dev/null
+++ b/test/cfg2cmd/q35-viommu.conf.cmd
@@ -0,0 +1,23 @@
+/usr/bin/kvm \
+  -id 8006 \
+  -name 'vm8006,debug-threads=on' \
+  -no-shutdown \
+  -chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server=on,wait=off' \
+  -mon 'chardev=qmp,mode=control' \
+  -chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
+  -mon 'chardev=qmp-event,mode=control' \
+  -pidfile /var/run/qemu-server/8006.pid \
+  -daemonize \
+  -smp '1,sockets=1,cores=1,maxcpus=1' \
+  -nodefaults \
+  -boot 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg' \
+  -vnc 'unix:/var/run/qemu-server/8006.vnc,password=on' \
+  -cpu kvm64,enforce,+kvm_pv_eoi,+kvm_pv_unhalt,+lahf_lm,+sep \
+  -m 512 \
+  -device 'intel-iommu,intremap=on,caching-mode=on' \
+  -readconfig /usr/share/qemu-server/pve-q35-4.0.cfg \
+  -device 'usb-tablet,id=tablet,bus=ehci.0,port=1' \
+  -device 'VGA,id=vga,bus=pcie.0,addr=0x1' \
+  -device 'virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3,free-page-reporting=on' \
+  -iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \
+  -machine 'type=q35+pve0,kernel-irqchip=split'
-- 
2.39.2

[pve-devel] [PATCH docs v6 3/4] add vIOMMU documentation

[Markus Frank]

Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
 qm-pci-passthrough.adoc | 31 +++++++++++++++++++++++++++++++
 qm.adoc                 |  1 +
 2 files changed, 32 insertions(+)

diff --git a/qm-pci-passthrough.adoc b/qm-pci-passthrough.adoc
index b90a0b9..9737165 100644
--- a/qm-pci-passthrough.adoc
+++ b/qm-pci-passthrough.adoc
@@ -408,6 +408,37 @@ properly used with HA and hardware changes are detected and non root users
 can configure them. See xref:resource_mapping[Resource Mapping]
 for details on that.
 
+[[qm_pci_viommu]]
+vIOMMU
+~~~~~~
+
+Using the vIOMMU option allows you to to passthrough PCI devices to level-2
+VMs in level-1 VMs via nested virtualisation.
+
+Host requirement:
+
+* Add `intel_iommu=on` or `amd_iommu=on` depending on your CPU to your kernel
+command line.
+
+VM requirements:
+
+* Whether you are using an Intel or AMD CPU on your host, it is important to set
+`intel_iommu=on` in the VMs kernel parameters.
+This is necessary because the Intel variant in QEMU is better documentated
+and has more features.
+
+* To use vIOMMU you need to set *q35* as the machine type and *kvm* needs
+to be enabled.
+
+If all requirements are met, you can add `viommu=1` to the machine parameter
+in the configuration of the VM that should be able to passthrough PCI devices.
+
+----
+# qm set VMID -machine q35,viommu=1
+----
+
+https://wiki.qemu.org/Features/VT-d
+
 ifdef::wiki[]
 
 See Also
diff --git a/qm.adoc b/qm.adoc
index b3c3034..33f02f2 100644
--- a/qm.adoc
+++ b/qm.adoc
@@ -145,6 +145,7 @@ default https://en.wikipedia.org/wiki/Intel_440FX[Intel 440FX] or the
 https://ark.intel.com/content/www/us/en/ark/products/31918/intel-82q35-graphics-and-memory-controller.html[Q35]
 chipset, which also provides a virtual PCIe bus, and thus may be desired if
 one wants to pass through PCIe hardware.
+Additionally you can use xref:qm_pci_viommu[vIOMMU] with Q35.
 
 [[qm_hard_disk]]
 Hard Disk
-- 
2.39.2

[pve-devel] [PATCH manager v6 4/4] ui: MachineEdit with viommu checkbox

[Markus Frank]

Add a checkbox to enable viommu, if q35 is selected.
Otherwise (i440fx & !kvm) the checkbox is disabled, if not ticked on
before. If ticked on before, the user is able to uncheck the checkbox.

If kvm is deactivated or i440fx is selected, a hint tells that q35 and
kvm are required for vIOMMU.

The UI also needs to parse the new machine parameter as PropertyString.

Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
 www/manager6/qemu/MachineEdit.js | 49 ++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/www/manager6/qemu/MachineEdit.js b/www/manager6/qemu/MachineEdit.js
index f928c80c..25e8dc83 100644
--- a/www/manager6/qemu/MachineEdit.js
+++ b/www/manager6/qemu/MachineEdit.js
@@ -1,6 +1,7 @@
 Ext.define('PVE.qemu.MachineInputPanel', {
     extend: 'Proxmox.panel.InputPanel',
     xtype: 'pveMachineInputPanel',
+    onlineHelp: 'qm_system_settings',
 
     controller: {
 	xclass: 'Ext.app.ViewController',
@@ -12,11 +13,26 @@ Ext.define('PVE.qemu.MachineInputPanel', {
 	onMachineChange: function(field, value) {
 	    let me = this;
 	    let version = me.lookup('version');
+	    let kvm = me.getView().kvm;
+	    let viommu = me.lookup('viommu');
+	    let kvmHint = me.lookup('kvmQ35Hint');
 	    let store = version.getStore();
 	    let oldRec = store.findRecord('id', version.getValue(), 0, false, false, true);
 	    let type = value === 'q35' ? 'q35' : 'i440fx';
 	    store.clearFilter();
 	    store.addFilter(val => val.data.id === 'latest' || val.data.type === type);
+	    if ((type === 'q35' && kvm) || viommu.getValue()) {
+		viommu.setDisabled(false);
+		kvmHint.setVisible(false);
+	    } else {
+		// disable checkbox if vIOMMU is not possible and checkbox was not
+		// ticked on before
+		viommu.setDisabled(true);
+	    }
+	    if (type === 'i440fx' || !kvm) {
+		// show hint when vIOMMU cannot be used
+		kvmHint.setVisible(true);
+	    }
 	    if (!me.getView().isWindows) {
 		version.setValue('latest');
 	    } else {
@@ -38,14 +54,26 @@ Ext.define('PVE.qemu.MachineInputPanel', {
 	if (values.version && values.version !== 'latest') {
 	    values.machine = values.version;
 	    delete values.delete;
+	} else if (values.machine === undefined && values.viommu) {
+	    // set machine to pc to raise the viommu + i440fx error
+	    // from qemu-server instead of a regex error
+	    values.machine = "pc";
+	    delete values.delete;
 	}
 	delete values.version;
+	if (values.viommu) {
+	    values.machine += ",viommu=1";
+	}
+	delete values.viommu;
 	return values;
     },
 
     setValues: function(values) {
 	let me = this;
 
+	let machineConf = PVE.Parser.parsePropertyString(values.machine, "type");
+	values.machine = machineConf.type;
+
 	me.isWindows = values.isWindows;
 	if (values.machine === 'pc') {
 	    values.machine = '__default__';
@@ -58,6 +86,11 @@ Ext.define('PVE.qemu.MachineInputPanel', {
 		values.version = 'pc-q35-5.1';
 	    }
 	}
+
+	me.kvm = values.kvm;
+	values.viommu = PVE.Parser.parseBoolean(machineConf.viommu);
+	me.lookup('viommu').setValue(values.viommu);
+
 	if (values.machine !== '__default__' && values.machine !== 'q35') {
 	    values.version = values.machine;
 	    values.machine = values.version.match(/q35/) ? 'q35' : '__default__';
@@ -113,6 +146,20 @@ Ext.define('PVE.qemu.MachineInputPanel', {
 	    fieldLabel: gettext('Note'),
 	    value: gettext('Machine version change may affect hardware layout and settings in the guest OS.'),
 	},
+	{
+	    xtype: 'proxmoxcheckbox',
+	    fieldLabel: gettext('vIOMMU'),
+	    name: 'viommu',
+	    reference: 'viommu',
+	},
+	{
+	    xtype: 'displayfield',
+	    name: 'kvmQ35Hint',
+	    reference: 'kvmQ35Hint',
+	    userCls: 'pmx-hint',
+	    value: gettext('vIOMMU needs kvm enabled and q35 machine type'),
+	    hidden: true,
+	},
     ],
 });
 
@@ -135,8 +182,10 @@ Ext.define('PVE.qemu.MachineEdit', {
 	me.load({
 	    success: function(response) {
 		let conf = response.result.data;
+		conf.kvm ??= 1;
 		let values = {
 		    machine: conf.machine || '__default__',
+		    kvm: conf.kvm,
 		};
 		values.isWindows = PVE.Utils.is_windows(conf.ostype);
 		me.setValues(values);
-- 
2.39.2

Re: [pve-devel] [PATCH qemu-server v6 1/4] feature #3784: Parameter for guest vIOMMU & machine as property-string

[Thomas Lamprecht]

Am 22/08/2023 um 14:40 schrieb Markus Frank:
> vIOMMU enables the option to passthrough pci devices to L2 VMs
> in L1 VMs via Nested Virtualisation.
> 
> QEMU-Parameters:
> https://www.qemu.org/docs/master/system/qemu-manpage.html
> https://wiki.qemu.org/Features/VT-d
> 
> -machine ...,kernel-irqchip=split:
> 
> "split" because of intremap see below.
> 
> -device intel-iommu:
> 
> * caching-mode=on:
> 
> "It is required for -device vfio-pci to work with the VT-d device, because host
> assigned devices requires to setup the DMA mapping on the host before guest DMA
> starts."
> 
> * intremap=on:
> 
> "This enables interrupt remapping feature. It's required to enable complete
> x2apic. Currently it only supports kvm kernel-irqchip modes off or split, while
> full kernel-irqchip is not yet supported."


looks ok, but I'd like to have the format change and additions of helpers
infrastructure split from adding the "viommu" property.

I.e., first add only the format and default_key and change the call sites,
this should have no semantic effect at all.
Then add the "viommu" parameter to the new format and whatever it needs to
respect that, the latter could then be merged with the test patches.

Re: [pve-devel] [PATCH qemu-server v6 1/4] feature #3784: Parameter for guest vIOMMU & machine as property-string

[Fiona Ebner]

Am 22.08.23 um 14:40 schrieb Markus Frank:
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index bf1de17..013792d 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -118,12 +118,32 @@ PVE::JSONSchema::register_standard_option('pve-qm-stateuri', {
>      optional => 1,
>  });
>  
> -PVE::JSONSchema::register_standard_option('pve-qemu-machine', {
> +my $machine_fmt = {
> +    type => {
> +	default_key => 1,
>  	description => "Specifies the QEMU machine type.",
>  	type => 'string',
>  	pattern => '(pc|pc(-i440fx)?-\d+(\.\d+)+(\+pve\d+)?(\.pxe)?|q35|pc-q35-\d+(\.\d+)+(\+pve\d+)?(\.pxe)?|virt(?:-\d+(\.\d+)+)?(\+pve\d+)?)',
>  	maxLength => 40,
> +	format_description => 'machine type',
>  	optional => 1,
> +    },
> +    viommu => {
> +	type => 'boolean',
> +	description => "Enable/disable guest vIOMMU"
> +	    ." (needs kvm to be enabled and q35 to be set as machine type).",
> +	default => 0,
> +	optional => 1,
> +    },
> +};
> +
> +PVE::JSONSchema::register_format('pve-qemu-machine-fmt', $machine_fmt);
> +
> +PVE::JSONSchema::register_standard_option('pve-qemu-machine', {
> +    description => "Specify the QEMU machine type & enable/disable vIOMMU.",
> +    type => 'string',
> +    optional => 1,
> +    format => PVE::JSONSchema::get_format('pve-qemu-machine-fmt'),
>  });
>  
>  # FIXME: remove in favor of just using the INotify one, it's cached there exactly the same way
> @@ -2133,6 +2153,31 @@ sub parse_watchdog {
>      return $res;
>  }
>  
> +sub parse_machine {
> +    my ($value) = @_;
> +
> +    return if !$value;
> +
> +    my $res = parse_property_string($machine_fmt, $value);
> +    return $res;
> +}
> +
> +sub check_machine_config {
> +    my ($conf, $machine_conf) = @_;
> +    my $q35 = $machine_conf->{type} && ($machine_conf->{type} =~ m/q35/) ? 1 : 0;
> +    my $kvm = $conf->{kvm};
> +    my $arch = get_vm_arch($conf);
> +    $kvm //= 1 if is_native($arch);
> +    if ($machine_conf->{viommu} && (!$kvm || !$q35)) {
> +	die "to use vIOMMU please enable kvm and set the machine type to q35\n";
> +    }
> +}
> +
> +sub print_machine {
> +    my ($machine_conf) = @_;
> +    return PVE::JSONSchema::print_property_string($machine_conf, $machine_fmt);
> +}
> +

Was there any reason why you didn't put the above, i.e. format
definition/parsing/printing in the QemuServer::Machine module instead
like I suggested in my review of v5?

> diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
> index d9429ed..bfbde59 100644
> --- a/PVE/QemuServer/Machine.pm
> +++ b/PVE/QemuServer/Machine.pm
> @@ -15,7 +15,8 @@ our $PVE_MACHINE_VERSION = {
>  sub machine_type_is_q35 {
>      my ($conf) = @_;
>  
> -    return $conf->{machine} && ($conf->{machine} =~ m/q35/) ? 1 : 0;
> +    my $machine_conf = PVE::QemuServer::parse_machine($conf->{machine});
> +    return $machine_conf->{type} && ($machine_conf->{type} =~ m/q35/) ? 1 : 0;
>  }
>  
>  sub current_from_query_machines {
> @@ -120,7 +121,8 @@ sub qemu_machine_pxe {
>  
>      my $machine =  get_current_qemu_machine($vmid);
>  
> -    if ($conf->{machine} && $conf->{machine} =~ m/\.pxe$/) {
> +    my $machine_conf = PVE::QemuServer::parse_machine($conf->{machine});
> +    if ($machine_conf->{type} && $machine_conf->{type} =~ m/\.pxe$/) {
>  	$machine .= '.pxe';
>      }
>  

Because there still are these cyclic calls back into the QemuServer
module (and you would need the cylcic use statement which is what we
want to avoid).