[pve-devel] [PATCH v2 qemu-server 0/1] pci: allow override of PCI vendor/device ids

[pve-devel] [PATCH v2 qemu-server 0/1] pci: allow override of PCI vendor/device ids

[Nicholas Sherlock]

In this version, changed proposed x-pci-device-id names to remove x-pci- prefix to shorten configs, and improved Perl code style, thanks to Dominik Csapak

[PATCH v2 qemu-server] pci: allow override of PCI vendor/device ids
[PATCH v2 pve-manager] ui: pci passthrough: editor for pci-id

Re: [pve-devel] [PATCH v2 qemu-server 0/1] pci: allow override of PCI vendor/device ids

[Dominik Csapak]

On 1/18/22 22:38, Nicholas Sherlock wrote:
> In this version, changed proposed x-pci-device-id names to remove x-pci- prefix to shorten configs, and improved Perl code style, thanks to Dominik Csapak
> 
> [PATCH v2 qemu-server] pci: allow override of PCI vendor/device ids
> [PATCH v2 pve-manager] ui: pci passthrough: editor for pci-id
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 

Thanks,

2 nits left (you could send a v3, or we can fix it up, please
write what you prefer)

* the commit message should not have lines longer than 70 characters[0]
* and the gettext is not optimal, i'd omit the  'Set ' prefix (sorry i did not
   catch it the first time), so 'Vendor ID' instead of 'Set vendor ID'

otherwise:

Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
Tested-by: Dominik Csapak <d.csapak@proxmox.com>

0: https://pve.proxmox.com/wiki/Developer_Documentation#Commits_and_Commit_Messages

Re: [pve-devel] [PATCH v2 qemu-server 0/1] pci: allow override of PCI vendor/device ids

[Nicholas Sherlock]

On Fri, 21 Jan 2022, 4:48 am Dominik Csapak, <d.csapak@proxmox.com> wrote:

> 2 nits left (you could send a v3, or we can fix it up, please
> write what you prefer)
>
> * the commit message should not have lines longer than 70 characters[0]
> * and the gettext is not optimal, i'd omit the  'Set ' prefix (sorry i did
> not
>    catch it the first time), so 'Vendor ID' instead of 'Set vendor ID'
>

I would recommend keeping the "set" text in, because otherwise it makes it
look like you can pick which host device you want to pass through by using
those fields. I think it could confuse people.

Happy for you to fix the patch as needed

Cheers,
Nick



> otherwise:
>
> Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
> Tested-by: Dominik Csapak <d.csapak@proxmox.com>
>
> 0:
> https://pve.proxmox.com/wiki/Developer_Documentation#Commits_and_Commit_Messages
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>

[pve-devel] [PATCH v2 qemu-server] pci: allow override of PCI vendor/device ids

[Nicholas Sherlock]

From: Nicholas Sherlock <n.sherlock@gmail.com>

This allows mobile- and vGPUs to be presented to the guest as if they were the original desktop variants of the card. It also allows device-ID variants that guests don't know about to be renamed to match compatible sibling devices the guest does have drivers for (e.g. to remove manufacturer-specific vendor ID variants that prevent the use of a device which would otherwise have a supported chipset)

e.g. hostpci0: 03:00,vendor-id=0x8086,device-id=0x10f6

Signed-off-by: Nicholas Sherlock <n.sherlock@gmail.com>
---
 PVE/QemuServer/PCI.pm                         | 31 ++++++++++++++++
 .../q35-linux-hostpci-x-pci-overrides.conf    | 16 +++++++++
 ...q35-linux-hostpci-x-pci-overrides.conf.cmd | 35 +++++++++++++++++++
 3 files changed, 82 insertions(+)
 create mode 100644 test/cfg2cmd/q35-linux-hostpci-x-pci-overrides.conf
 create mode 100644 test/cfg2cmd/q35-linux-hostpci-x-pci-overrides.conf.cmd

diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
index 4033784..70987d8 100644
--- a/PVE/QemuServer/PCI.pm
+++ b/PVE/QemuServer/PCI.pm
@@ -77,6 +77,34 @@ The type of mediated device to use.
 An instance of this type will be created on startup of the VM and
 will be cleaned up when the VM stops.
 EODESCR
+    },
+    'vendor-id' => {
+	type => 'string',
+	pattern => qr/^0x[0-9a-fA-F]{4}$/,
+	format_description => 'hex id',
+	optional => 1,
+	description => "Override PCI vendor ID visible to guest"
+    },
+    'device-id' => {
+	type => 'string',
+	pattern => qr/^0x[0-9a-fA-F]{4}$/,
+	format_description => 'hex id',
+	optional => 1,
+	description => "Override PCI device ID visible to guest"
+    },
+    'sub-vendor-id' => {
+	type => 'string',
+	pattern => qr/^0x[0-9a-fA-F]{4}$/,
+	format_description => 'hex id',
+	optional => 1,
+	description => "Override PCI subsystem vendor ID visible to guest"
+    },
+    'sub-device-id' => {
+	type => 'string',
+	pattern => qr/^0x[0-9a-fA-F]{4}$/,
+	format_description => 'hex id',
+	optional => 1,
+	description => "Override PCI subsystem device ID visible to guest"
     }
 };
 PVE::JSONSchema::register_format('pve-qm-hostpci', $hostpci_fmt);
@@ -457,6 +485,9 @@ sub print_hostpci_devices {
 		$devicestr .= ",multifunction=on" if $multifunction;
 		$devicestr .= ",romfile=/usr/share/kvm/$d->{romfile}" if $d->{romfile};
 		$devicestr .= ",bootindex=$bootorder->{$id}" if $bootorder->{$id};
+		for my $option (qw(vendor-id device-id sub-vendor-id sub-device-id)) {
+		    $devicestr .= ",x-pci-$option=$d->{$option}" if $d->{$option};
+		}
 	    }
 
 	    push @$devices, '-device', $devicestr;
diff --git a/test/cfg2cmd/q35-linux-hostpci-x-pci-overrides.conf b/test/cfg2cmd/q35-linux-hostpci-x-pci-overrides.conf
new file mode 100644
index 0000000..b726a3a
--- /dev/null
+++ b/test/cfg2cmd/q35-linux-hostpci-x-pci-overrides.conf
@@ -0,0 +1,16 @@
+# TEST: Overriding PCI vendor/device IDs reported to guest
+bios: ovmf
+bootdisk: scsi0
+cores: 1
+efidisk0: local:100/vm-100-disk-1.qcow2,size=128K
+hostpci0: 00:ff.1,vendor-id=0x1234,device-id=0x5678,sub-vendor-id=0x2233,sub-device-id=0x0000
+hostpci1: d0:13.0,pcie=1,vendor-id=0x1234,device-id=0x5678
+machine: q35
+memory: 512
+net0: virtio=2E:01:68:F9:9C:87,bridge=vmbr0
+numa: 1
+ostype: l26
+scsihw: virtio-scsi-pci
+smbios1: uuid=3dd750ce-d910-44d0-9493-525c0be4e687
+sockets: 2
+vmgenid: 54d1c06c-8f5b-440f-b5b2-6eab1380e13d
diff --git a/test/cfg2cmd/q35-linux-hostpci-x-pci-overrides.conf.cmd b/test/cfg2cmd/q35-linux-hostpci-x-pci-overrides.conf.cmd
new file mode 100644
index 0000000..7a215c9
--- /dev/null
+++ b/test/cfg2cmd/q35-linux-hostpci-x-pci-overrides.conf.cmd
@@ -0,0 +1,35 @@
+/usr/bin/kvm \
+  -id 8006 \
+  -name vm8006 \
+  -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 \
+  -smbios 'type=1,uuid=3dd750ce-d910-44d0-9493-525c0be4e687' \
+  -drive 'if=pflash,unit=0,format=raw,readonly=on,file=/usr/share/pve-edk2-firmware//OVMF_CODE.fd' \
+  -drive 'if=pflash,unit=1,format=qcow2,id=drive-efidisk0,file=/var/lib/vz/images/100/vm-100-disk-1.qcow2' \
+  -global 'ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off' \
+  -smp '2,sockets=2,cores=1,maxcpus=2' \
+  -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 \
+  -object 'memory-backend-ram,id=ram-node0,size=256M' \
+  -numa 'node,nodeid=0,cpus=0,memdev=ram-node0' \
+  -object 'memory-backend-ram,id=ram-node1,size=256M' \
+  -numa 'node,nodeid=1,cpus=1,memdev=ram-node1' \
+  -readconfig /usr/share/qemu-server/pve-q35-4.0.cfg \
+  -device 'vmgenid,guid=54d1c06c-8f5b-440f-b5b2-6eab1380e13d' \
+  -device 'usb-tablet,id=tablet,bus=ehci.0,port=1' \
+  -device 'vfio-pci,host=0000:00:ff.1,id=hostpci0,bus=pci.0,addr=0x10,x-pci-vendor-id=0x1234,x-pci-device-id=0x5678,x-pci-sub-vendor-id=0x2233,x-pci-sub-device-id=0x0000' \
+  -device 'vfio-pci,host=0000:d0:13.0,id=hostpci1,bus=ich9-pcie-port-2,addr=0x0,x-pci-vendor-id=0x1234,x-pci-device-id=0x5678' \
+  -device 'VGA,id=vga,bus=pcie.0,addr=0x1' \
+  -device 'virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3' \
+  -iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \
+  -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \
+  -device 'virtio-net-pci,mac=2E:01:68:F9:9C:87,netdev=net0,bus=pci.0,addr=0x12,id=net0,bootindex=300' \
+  -machine 'type=q35+pve0'
-- 
2.34.1

[pve-devel] applied: [PATCH v2 qemu-server] pci: allow override of PCI vendor/device ids

[Thomas Lamprecht]

On 18.01.22 22:38, Nicholas Sherlock wrote:
> From: Nicholas Sherlock <n.sherlock@gmail.com>
> 
> This allows mobile- and vGPUs to be presented to the guest as if they were the original desktop variants of the card. It also allows device-ID variants that guests don't know about to be renamed to match compatible sibling devices the guest does have drivers for (e.g. to remove manufacturer-specific vendor ID variants that prevent the use of a device which would otherwise have a supported chipset)
> 
> e.g. hostpci0: 03:00,vendor-id=0x8086,device-id=0x10f6
> 
> Signed-off-by: Nicholas Sherlock <n.sherlock@gmail.com>
> ---
>  PVE/QemuServer/PCI.pm                         | 31 ++++++++++++++++
>  .../q35-linux-hostpci-x-pci-overrides.conf    | 16 +++++++++
>  ...q35-linux-hostpci-x-pci-overrides.conf.cmd | 35 +++++++++++++++++++
>  3 files changed, 82 insertions(+)
>  create mode 100644 test/cfg2cmd/q35-linux-hostpci-x-pci-overrides.conf
>  create mode 100644 test/cfg2cmd/q35-linux-hostpci-x-pci-overrides.conf.cmd
> 
>

applied, with the commit message reformat to fit into 70cc and Dominik's
T-b & R-B tags, thanks!

[pve-devel] [PATCH v2 pve-manager] ui: pci passthrough: editor for pci-id overrides

[Nicholas Sherlock]

From: Nicholas Sherlock <n.sherlock@gmail.com>

Signed-off-by: Nicholas Sherlock <n.sherlock@gmail.com>
---
 www/manager6/Toolkit.js      |  2 ++
 www/manager6/qemu/PCIEdit.js | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/www/manager6/Toolkit.js b/www/manager6/Toolkit.js
index 1b6085a4..f69c376a 100644
--- a/www/manager6/Toolkit.js
+++ b/www/manager6/Toolkit.js
@@ -13,6 +13,8 @@ Ext.apply(Ext.form.field.VTypes, {
     IP64AddressWithSuffixList: v => PVE.Utils.verify_ip64_address_list(v, true),
     IP64AddressListText: gettext('Example') + ': 192.168.1.1,192.168.1.2',
     IP64AddressListMask: /[A-Fa-f0-9,:.; ]/,
+    PciIdText: gettext('Example') + ': 0x8086',
+    PciId: v => /^0x[0-9a-fA-F]{4}$/.test(v),
 });
 
 Ext.define('PVE.form.field.Display', {
diff --git a/www/manager6/qemu/PCIEdit.js b/www/manager6/qemu/PCIEdit.js
index f505e34f..ec22a89c 100644
--- a/www/manager6/qemu/PCIEdit.js
+++ b/www/manager6/qemu/PCIEdit.js
@@ -190,6 +190,24 @@ Ext.define('PVE.qemu.PCIInputPanel', {
 		fieldLabel: 'ROM-File',
 		name: 'romfile',
 	    },
+	    {
+		xtype: 'textfield',
+		name: 'vendor-id',
+		fieldLabel: gettext('Set vendor ID'),
+		vtype: 'PciId',
+		allowBlank: true,
+		emptyText: Proxmox.Utils.defaultText,
+		submitEmpty: false,
+	    },
+	    {
+		xtype: 'textfield',
+		name: 'device-id',
+		fieldLabel: gettext('Set device ID'),
+		vtype: 'PciId',
+		allowBlank: true,
+		emptyText: Proxmox.Utils.defaultText,
+		submitEmpty: false,
+	    },
 	];
 
 	me.advancedColumn2 = [
@@ -198,6 +216,24 @@ Ext.define('PVE.qemu.PCIInputPanel', {
 		fieldLabel: 'PCI-Express',
 		name: 'pcie',
 	    },
+	    {
+		xtype: 'textfield',
+		name: 'sub-vendor-id',
+		fieldLabel: gettext('Set sub-vendor ID'),
+		vtype: 'PciId',
+		allowBlank: true,
+		emptyText: Proxmox.Utils.defaultText,
+		submitEmpty: false,
+	    },
+	    {
+		xtype: 'textfield',
+		name: 'sub-device-id',
+		fieldLabel: gettext('Set sub-device ID'),
+		vtype: 'PciId',
+		allowBlank: true,
+		emptyText: Proxmox.Utils.defaultText,
+		submitEmpty: false,
+	    },
 	];
 
 	me.callParent();
-- 
2.34.1

[pve-devel] applied: [PATCH v2 pve-manager] ui: pci passthrough: editor for pci-id overrides

[Thomas Lamprecht]

On 18.01.22 22:38, Nicholas Sherlock wrote:
> From: Nicholas Sherlock <n.sherlock@gmail.com>
> 
> Signed-off-by: Nicholas Sherlock <n.sherlock@gmail.com>
> ---
>  www/manager6/Toolkit.js      |  2 ++
>  www/manager6/qemu/PCIEdit.js | 36 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 38 insertions(+)
> 
>

applied, thanks!

I adapted the labels in a follow up, but also tried to clarify where the
value comes from if empty with a "From Device" empty-text.

Further clarifications would be probably good to have in the documentation,
would you be willing to write some short paragraph for the respective section
in pve-doc's qm-pci-passthrough.adoc? No pressure though, Dominik offered to
check that out too if you don't get around or the like.

cheers,
Thomas

Re: [pve-devel] applied: [PATCH v2 pve-manager] ui: pci passthrough: editor for pci-id overrides

[Nicholas Sherlock]

Sure, I'll send one along shortly.

Cheers,
Nick

On Fri, 28 Jan 2022 at 03:21, Thomas Lamprecht <nick@nicksherlock.com>
wrote:

> On 18.01.22 22:38, Nicholas Sherlock wrote:
> > From: Nicholas Sherlock <n.sherlock@gmail.com>
> >
> > Signed-off-by: Nicholas Sherlock <n.sherlock@gmail.com>
> > ---
> >  www/manager6/Toolkit.js      |  2 ++
> >  www/manager6/qemu/PCIEdit.js | 36 ++++++++++++++++++++++++++++++++++++
> >  2 files changed, 38 insertions(+)
> >
> >
>
> applied, thanks!
>
> I adapted the labels in a follow up, but also tried to clarify where the
> value comes from if empty with a "From Device" empty-text.
>
> Further clarifications would be probably good to have in the documentation,
> would you be willing to write some short paragraph for the respective
> section
> in pve-doc's qm-pci-passthrough.adoc? No pressure though, Dominik offered
> to
> check that out too if you don't get around or the like.
>
> cheers,
> Thomas
>
>