[pve-devel] [PATCH qemu-server 2/3] PCI.pm: improve and extend lspci

Thomas Lamprecht t.lamprecht at proxmox.com
Fri Nov 16 08:54:58 CET 2018


On 11/15/18 3:30 PM, Dominik Csapak wrote:
> this implements following improvements and optimizations for lspci
> 
> * removes the unecessary split between id and function
>   since everywhere we need that information, we stitch them together
>   anyway. to preserve ordering, simply order by id with string
>   comparison 'cmp' (this is important for the shorthand syntax '00:01' in
>   the config)
> * returns now a list directly, instead of an hash with lists
> * filter now does not have to be an exact match, only the beginning has
>   to match, this is not a problem for parse_hostpci, as we check
>   the syntax there explicitely
> * adds a filterunusable flag, which filters not pass-throughable
>   devices, such as memory controllers, processors, etc.
>   the pci classes are documented [1]
> * adds a verbose flag to include more information about the device,
>   such as device/vendor, iommu-group, mdev support, etc.
>   this will be used for the pci scan api call for the gui
> 
> 1: https://pci-ids.ucw.cz/read/PD/

higher level questions, what does this do in qemu-server? It's a general
_host_ PCI scan, IIUC. So pve-common would make more sense?
Also there's RFE #754 which requests device pass through for CTs, which could
benefit from a move too. 

I guess your rationale was more like, this is already here and I do not want
to touch another repo? We can do this later too, just to clarify the reason.

> 
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
>  PVE/QemuServer.pm     | 19 +++++------
>  PVE/QemuServer/PCI.pm | 89 ++++++++++++++++++++++++++++++++++++++++++++-------
>  2 files changed, 87 insertions(+), 21 deletions(-)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 3a756df..e5f4dea 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -2114,11 +2114,13 @@ sub parse_hostpci {
>      delete $res->{host};
>      foreach my $id (@idlist) {
>  	if ($id =~ /^$PCIRE$/) {
> -	    if (defined($2)) {
> -		push @{$res->{pciid}}, { id => $1, function => $2 };
> +	    my ($shortid, $function) = ($1, $2);
> +	    if (defined($function)) {
> +		push @{$res->{pciid}}, {
> +		    id => "$shortid.$function",
> +		};
>  	    } else {
> -		my $pcidevices = PVE::QemuServer::PCI::lspci($1);
> -		$res->{pciid} = $pcidevices->{$1};
> +		$res->{pciid} = PVE::QemuServer::PCI::lspci($shortid);
>  	    }
>  	} else {
>  	    # should have been caught by parse_property_string already
> @@ -3554,9 +3556,8 @@ sub config_to_command {
>  	my $sysfspath;
>  	if ($d->{mdev} && scalar(@$pcidevices) == 1) {
>  	    my $id = $pcidevices->[0]->{id};
> -	    my $function = $pcidevices->[0]->{function};
>  	    my $uuid = PVE::QemuServer::PCI::generate_mdev_uuid($vmid, $i);
> -	    $sysfspath = "$pcisysfs/devices/0000:$id.$function/$uuid";
> +	    $sysfspath = "$pcisysfs/devices/0000:$id/$uuid";
>  	} elsif ($d->{mdev}) {
>  	    warn "ignoring mediated device with multifunction device\n";
>  	}
> @@ -3572,7 +3573,7 @@ sub config_to_command {
>  	    if ($sysfspath) {
>  		$devicestr .= ",sysfsdev=$sysfspath";
>  	    } else {
> -		$devicestr .= ",host=$pcidevice->{id}.$pcidevice->{function}";
> +		$devicestr .= ",host=$pcidevice->{id}";
>  	    }
>  	    $devicestr .= ",id=$id$addr";
>  
> @@ -5164,7 +5165,7 @@ sub vm_start {
>            next if !$d;
>  	  my $pcidevices = $d->{pciid};
>  	  foreach my $pcidevice (@$pcidevices) {
> -		my $pciid = $pcidevice->{id}.".".$pcidevice->{function};
> +		my $pciid = $pcidevice->{id};
>  
>  		my $info = PVE::QemuServer::PCI::pci_device_info("0000:$pciid");
>  		die "IOMMU not present\n" if !PVE::QemuServer::PCI::check_iommu_support();
> @@ -5425,7 +5426,7 @@ sub vm_stop_cleanup {
>  	    my $uuid = PVE::QemuServer::PCI::generate_mdev_uuid($vmid, $hostpciindex);
>  
>  	    foreach my $pci (@{$d->{pciid}}) {
> -		my $pciid = $pci->{id} . "." . $pci->{function};
> +		my $pciid = $pci->{id};
>  		PVE::QemuServer::PCI::pci_cleanup_mdev_device($pciid, $uuid);
>  	    }
>  	}
> diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
> index 19aebd7..52bffe5 100644
> --- a/PVE/QemuServer/PCI.pm
> +++ b/PVE/QemuServer/PCI.pm
> @@ -161,24 +161,89 @@ sub print_pcie_addr {
>  my $pcisysfs = "/sys/bus/pci";
>  my $pciregex = "([a-f0-9]{4}):([a-f0-9]{2}):([a-f0-9]{2})\.([a-f0-9])";
>  
> +my $parse_pci_ids = sub {
> +    my $ids = {};
> +
> +    open(my $fh, '<', "/usr/share/misc/pci.ids")
> +	or return $ids;
> +
> +    my $curvendor;
> +    my $curdevice;
> +    while (my $line = <$fh>) {
> +	if ($line =~ m/^([0-9a-fA-F]{4})\s+(.*)$/) {
> +	    $curvendor = "0x$1";
> +	    $ids->{$curvendor}->{name} = $2;
> +	} elsif ($line =~ m/^\t([0-9a-fA-F]{4})\s+(.*)$/) {
> +	    $curdevice = "0x$1";
> +	    $ids->{$curvendor}->{devices}->{$curdevice}->{name} = $2;
> +	} elsif ($line =~ m/^\t\t([0-9a-fA-F]{4}) ([0-9a-fA-F]{4})\s+(.*)$/) {
> +	    $ids->{$curvendor}->{devices}->{$curdevice}->{subs}->{"0x$1"}->{"0x$2"} = $3;
> +	}
> +    }
> +
> +    return $ids;
> +};
> +
>  sub lspci {
> -    my ($filter) = @_;
> +    my ($filter, $filterunusable, $verbose) = @_;
>  
> -    my $devices = {};
> +    my $devices = [];
> +    my $ids = {};
> +    if ($verbose) {
> +	$ids = $parse_pci_ids->();
> +    }
>  
>      dir_glob_foreach("$pcisysfs/devices", $pciregex, sub {
> -            my (undef, undef, $bus, $slot, $function) = @_;
> -	    my $id = "$bus:$slot";
> -	    return if defined($filter) && $id ne $filter;
> -	    my $res = { id => $id, function => $function};
> -	    push @{$devices->{$id}}, $res;
> +	    my ($fullid, $domain, $bus, $slot, $function) = @_;
> +	    my $id = "$bus:$slot.$function";
> +	    return if defined($filter) && $id !~ m/^\Q$filter\E/;
> +	    my $res = { id => $id, };
> +
> +	    my $devdir = "$pcisysfs/devices/$fullid";
> +
> +	    if ($filterunusable) {
> +		my $class = substr(PVE::Tools::file_read_firstline("$devdir/class"), 2, 2);
> +		return if $class =~ m/^05|06|08|0b$/;
> +	    }
> +
> +	    if ($verbose) {
> +		my $vendorid = PVE::Tools::file_read_firstline("$devdir/vendor");
> +		my $deviceid = PVE::Tools::file_read_firstline("$devdir/device");
> +		my $svendorid = PVE::Tools::file_read_firstline("$devdir/subsystem_vendor");
> +		my $sdeviceid = PVE::Tools::file_read_firstline("$devdir/subsystem_device");
> +
> +		my $iommugroup = -1;
> +		if (-e "$devdir/iommu_group") {
> +		    ($iommugroup) = (readlink("$devdir/iommu_group") =~ m/\/(\d+)$/);
> +		    $iommugroup = int($iommugroup);
> +		}
> +
> +		my $vendor = $ids->{$vendorid}->{name};
> +		my $device = $ids->{$vendorid}->{devices}->{$deviceid}->{name};
> +		my $svendor = $ids->{$svendorid}->{name};
> +		my $sdevice = $ids->{$vendorid}->{devices}->{$deviceid}->{subs}->{$svendorid}->{$sdeviceid};
> +
> +		$res->{vendorid} = $vendorid;
> +		$res->{deviceid} = $deviceid;
> +
> +		if (-d "$devdir/mdev_supported_types") {
> +		    $res->{mdev} = 1;
> +		}
> +
> +		$res->{vendor} = $vendor if defined($vendor);
> +		$res->{device} = $device if defined($device);
> +		$res->{subsystem_vendorid} = $svendorid if defined($svendorid);
> +		$res->{subsystem_vendor} = $svendor if defined($svendor);
> +		$res->{subsystem_deviceid} = $sdeviceid if defined($sdeviceid);
> +		$res->{subsystem_device} = $sdevice if defined($sdevice);
> +		$res->{iommugroup} = $iommugroup if defined($iommugroup);
> +	    }
> +
> +	    push @$devices, $res;
>      });
>  
> -    # Entries should be sorted by functions.
> -    foreach my $id (keys %$devices) {
> -	my $dev = $devices->{$id};
> -	$devices->{$id} = [ sort { $a->{function} <=> $b->{function} } @$dev ];
> -    }
> +    # Entries should be sorted by ids
> +    $devices = [ sort { $a->{id} cmp $b->{id} } @$devices ];
>  
>      return $devices;
>  }
> 





More information about the pve-devel mailing list