[pve-devel] [PATCH qemu-server] pci: allow override of PCI vendor/device ids
Dominik Csapak
d.csapak at proxmox.com
Tue Jan 18 12:58:43 CET 2022
Hi, Thanks for the patch!
the approach looks good (thanks for adding tests!),
but i have some comments (most inline)
I'd prefer to have the text from the cover letter (i.e.
the rationale for this) in the commit message here. That
way we have it in the git history and stumbling upon the
commit directly gives the reason (instead of having to search
the mailing list)
gui part looks fine, remaining comments inline
On 1/18/22 09:43, nick at nicksherlock.com wrote:
> From: Nicholas Sherlock <n.sherlock at gmail.com>
>
> e.g. hostpci0: 03:00,x-pci-vendor-id=0x8086,x-pci-device-id=0x10f6
>
> Signed-off-by: Nicholas Sherlock <n.sherlock at 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..45790b6 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
> + },
> + 'x-pci-vendor-id' => {
since we are here in pci context anyway, i'd drop the
'x-pci-' prefix altogether and simply
have it named 'vendor-id', 'device-id' and so on.
makes the config a bit shorter and more readable
(would need gui patch adaption too)
> + type => 'string',
> + pattern => qr/^0x[0-9a-fA-F]{4}$/,
> + format_description => 'hex id',
> + optional => 1,
> + description => "Override PCI vendor ID visible to guest"
> + },
> + 'x-pci-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"
> + },
> + 'x-pci-sub-vendor-id' => {
> + type => 'string',
> + pattern => qr/^0x[0-9a-fA-F]{4}$/,
> + format_description => 'hex id',
> + optional => 1,
> + description => "Override PCI sub-vendor ID visible to guest"
> + },
> + 'x-pci-sub-device-id' => {
> + type => 'string',
> + pattern => qr/^0x[0-9a-fA-F]{4}$/,
> + format_description => 'hex id',
> + optional => 1,
> + description => "Override PCI sub-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};
> + foreach ("x-pci-vendor-id","x-pci-device-id","x-pci-sub-vendor-id","x-pci-sub-device-id") {
> + $devicestr .= ",$_=$d->{$_}" if $d->{$_};
> + }
while its valid perl, it does not really use our style (see [0])
i'd use something like this (not tested though):
---8<---
for my $option (qw(vendor-id device-id sub-vendor-id sub-device-id)) {
$devicestr .= ",x-pci-$option=$d->{$option}" if $d->{$option};
}
--->8---
0: https://pve.proxmox.com/wiki/Perl_Style_Guide
> }
>
> 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..c5ab9bd
> --- /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,x-pci-vendor-id=0x1234,x-pci-device-id=0x5678,x-pci-sub-vendor-id=0x2233,x-pci-sub-device-id=0x0000
> +hostpci1: d0:13.0,pcie=1,x-pci-vendor-id=0x1234,x-pci-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'
More information about the pve-devel
mailing list