[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