[pve-devel] [PATCH v2 qemu-server 1/1] Add support for up to 16 PCI(e) devices

Dominik Csapak d.csapak at proxmox.com
Fri Sep 6 14:45:19 CEST 2019


looks mostly ok, one (important) comment inline

On 9/5/19 6:13 PM, Aaron Lauterer wrote:
> For non pci express passthrough additional addresses are reserved.
> For pcie passthrough pcie root ports are needed (unless guest is like
> windows 7).
> 
> The first 4 pcie root ports are defined by default in the pve-q35*.cfg
> files. If more than 4 pcie devices are passed through the needed root
> ports are created on demand. This helps to keep live migration possible
> without adding a new pve-q35*.cfg file.
> 
> For the windows 7 like guests additional addresses are reserved as well.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer at proxmox.com>
> ---
> 
> I decided to move the creation of the device string for the additional
> root ports to the `print_pcie_root_port` function in order to avoid
> overly long code lines and not to bloat the config_to_command function
> more than necessary.
> 
> For the addresses reserved: I looked for possible areas where to place
> them that would not create address conflicts in my tests:
> * win 10
> * win 7
> * ubuntu 19.04
> all with machines types Q35 (pcie and non pcie) as well as i440fx (non
> pcie)
> 
> I wasn't sure if I should put the `hostpciX` in quotes or not as the
> PCI.pm file is a bit of a mess in that regard.
> 
>   PVE/QemuServer.pm     |  8 +++--
>   PVE/QemuServer/PCI.pm | 70 +++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 76 insertions(+), 2 deletions(-)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 6e3b19e..901cb2c 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -33,7 +33,7 @@ use PVE::QemuConfig;
>   use PVE::QMPClient;
>   use PVE::RPCEnvironment;
>   use PVE::GuestHelpers;
> -use PVE::QemuServer::PCI qw(print_pci_addr print_pcie_addr);
> +use PVE::QemuServer::PCI qw(print_pci_addr print_pcie_addr print_pcie_root_port);
>   use PVE::QemuServer::Memory;
>   use PVE::QemuServer::USB qw(parse_usb_device);
>   use PVE::QemuServer::Cloudinit;
> @@ -769,7 +769,7 @@ my $MAX_SATA_DISKS = 6;
>   my $MAX_USB_DEVICES = 5;
>   my $MAX_NETS = 32;
>   my $MAX_UNUSED_DISKS = 256;
> -my $MAX_HOSTPCI_DEVICES = 4;
> +my $MAX_HOSTPCI_DEVICES = 16;
>   my $MAX_SERIAL_PORTS = 4;
>   my $MAX_PARALLEL_PORTS = 3;
>   my $MAX_NUMA = 8;
> @@ -3750,6 +3750,10 @@ sub config_to_command {
>   	    if ($winversion == 7) {
>   		$pciaddr = print_pcie_addr("hostpci${i}bus0");
>   	    } else {
> +		# add more root ports if needed, 4 are present by default
> +		if ($i > 3) {
> +		    push @$devices, '-device', print_pcie_root_port($i);
> +		}
>   		$pciaddr = print_pcie_addr("hostpci$i");
>   	    }
>   	} else {
> diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
> index 9c72f3a..728cde3 100644
> --- a/PVE/QemuServer/PCI.pm
> +++ b/PVE/QemuServer/PCI.pm
> @@ -5,6 +5,7 @@ use base 'Exporter';
>   our @EXPORT_OK = qw(
>   print_pci_addr
>   print_pcie_addr
> +print_pcie_root_port
>   );
>   
>   my $devices = {
> @@ -80,6 +81,18 @@ my $devices = {
>       'virtio15' => { bus => 2, addr => 10 },
>       'ivshmem' => { bus => 2, addr => 11 },
>       'audio0' => { bus => 2, addr => 12 },
> +    hostpci4 => { bus => 2, addr => 13 },
> +    hostpci5 => { bus => 2, addr => 14 },
> +    hostpci6 => { bus => 2, addr => 15 },
> +    hostpci7 => { bus => 2, addr => 16 },
> +    hostpci8 => { bus => 2, addr => 17 },
> +    hostpci9 => { bus => 2, addr => 18 },
> +    hostpci10 => { bus => 2, addr => 19 },
> +    hostpci11 => { bus => 2, addr => 20 },
> +    hostpci12 => { bus => 2, addr => 21 },
> +    hostpci13 => { bus => 2, addr => 22 },
> +    hostpci14 => { bus => 2, addr => 23 },
> +    hostpci15 => { bus => 2, addr => 24 },
>       'virtioscsi0' => { bus => 3, addr => 1 },
>       'virtioscsi1' => { bus => 3, addr => 2 },
>       'virtioscsi2' => { bus => 3, addr => 3 },
> @@ -147,12 +160,36 @@ sub print_pcie_addr {
>   	hostpci1 => { bus => "ich9-pcie-port-2", addr => 0 },
>   	hostpci2 => { bus => "ich9-pcie-port-3", addr => 0 },
>   	hostpci3 => { bus => "ich9-pcie-port-4", addr => 0 },
> +	hostpci4 => { bus => "ich9-pcie-port-5", addr => 0 },
> +	hostpci5 => { bus => "ich9-pcie-port-6", addr => 0 },
> +	hostpci6 => { bus => "ich9-pcie-port-7", addr => 0 },
> +	hostpci7 => { bus => "ich9-pcie-port-8", addr => 0 },
> +	hostpci8 => { bus => "ich9-pcie-port-9", addr => 0 },
> +	hostpci9 => { bus => "ich9-pcie-port-10", addr => 0 },
> +	hostpci10 => { bus => "ich9-pcie-port-11", addr => 0 },
> +	hostpci11 => { bus => "ich9-pcie-port-12", addr => 0 },
> +	hostpci12 => { bus => "ich9-pcie-port-13", addr => 0 },
> +	hostpci13 => { bus => "ich9-pcie-port-14", addr => 0 },
> +	hostpci14 => { bus => "ich9-pcie-port-15", addr => 0 },
> +	hostpci15 => { bus => "ich9-pcie-port-16", addr => 0 },
>   	# win7 is picky about pcie assignments
>   	hostpci0bus0 => { bus => "pcie.0", addr => 16 },
>   	hostpci1bus0 => { bus => "pcie.0", addr => 17 },
>   	hostpci2bus0 => { bus => "pcie.0", addr => 18 },
>   	hostpci3bus0 => { bus => "pcie.0", addr => 19 },
>   	ivshmem => { bus => 'pcie.0', addr => 20 },
> +	hostpci4bus0 => { bus => "pcie.0", addr => 9 },
> +	hostpci5bus0 => { bus => "pcie.0", addr => 10 },
> +	hostpci6bus0 => { bus => "pcie.0", addr => 11 },
> +	hostpci7bus0 => { bus => "pcie.0", addr => 12 },
> +	hostpci8bus0 => { bus => "pcie.0", addr => 13 },
> +	hostpci9bus0 => { bus => "pcie.0", addr => 14 },
> +	hostpci10bus0 => { bus => "pcie.0", addr => 15 },
> +	hostpci11bus0 => { bus => "pcie.0", addr => 20 },

addr 20 is already used by ivshmem

i would prefer to have the list in order of the addresses, so that this
will be more obvious and does not happen. also thomas mentioned offlist 
that it would be nice to have a test that automatically checks this, and 
i agree, but no one had time to do this (for now)

> +	hostpci12bus0 => { bus => "pcie.0", addr => 21 },
> +	hostpci13bus0 => { bus => "pcie.0", addr => 22 },
> +	hostpci14bus0 => { bus => "pcie.0", addr => 23 },
> +	hostpci15bus0 => { bus => "pcie.0", addr => 24 },
>       };
>   
>       if (defined($devices->{$id}->{bus}) && defined($devices->{$id}->{addr})) {
> @@ -164,4 +201,37 @@ sub print_pcie_addr {
>   
>   }
>   
> +# Generates the device strings for additional pcie root ports. The first 4 pcie
> +# root ports are defined in the pve-q35*.cfg files.
> +sub print_pcie_root_port {
> +    my ($i) = @_;
> +    my $res = '';
> +
> +    my $id = $i + 1;
> +
> +    my $root_port_addresses = {
> +	4 => "10.0",
> +	5 => "10.1",
> +	6 => "10.2",
> +	7 => "10.3",
> +	8 => "10.4",
> +	9 => "10.5",
> +	10 => "10.6",
> +	11 => "10.7",
> +	12 => "11.0",
> +	13 => "11.1",
> +	14 => "11.2",
> +	15 => "11.3",
> +    };
> +
> +    if (defined($root_port_addresses->{$i})) {
> +	$res = "pcie-root-port,id=ich9-pcie-port-${id}";
> +	$res .= ",addr=$root_port_addresses->{$i}";
> +	$res .= ",x-speed=16,x-width=32,multifunction=on,bus=pcie.0";
> +	$res .= ",port=${id},chassis=${id}";
> +    }
> +
> +    return $res;
> +}
> +
>   1;
> 





More information about the pve-devel mailing list