[PATCH docs/guest-common/qemu-server v3 0/4] add new pci passthrough specific hookscript phase

[PATCH docs/guest-common/qemu-server v3 0/4] add new pci passthrough specific hookscript phase

[Dominik Csapak]

this series adds a new phase to the guest hookscript that is called for each
passed throug pci device after it's prepared, but before the qemu process is
started.

See the qemu-server commit for why that is interesting.

changes from v2:
* rebase on current master
* prefix hook script env variables with 'PVE_HOOK_'

pve-guest-common:

Dominik Csapak (1):
  helpers: exec hookscript: add optional parameters

 src/PVE/GuestHelpers.pm | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)


qemu-server:

Dominik Csapak (2):
  pci: factor 'prepare_pci_devices' out to PVE::QemuServer::PCI module
  pci: call hookscript for each prepared pci device

 src/PVE/QemuServer.pm     | 45 +++++----------------------
 src/PVE/QemuServer/PCI.pm | 65 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 37 deletions(-)


pve-docs:

Dominik Csapak (1):
  examples: add new hookscript phase to example hookscript

 examples/guest-example-hookscript.pl | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)


Summary over all repositories:
  4 files changed, 108 insertions(+), 38 deletions(-)

-- 
Generated by git-murpp 0.8.1

[PATCH guest-common v3 1/1] helpers: exec hookscript: add optional parameters

[Dominik Csapak]

sometimes we may want to call the hookscript with additional parameters
in some phases, e.g. we want to call it for each pci device that was
prepared before starting with the correct uuid or pci id.

Add these new parameters to the environment instead of the positional
parameters of the hookscript, since that is more future proof and we get
a key/value pair instead of just the position.

Use the prefix 'PVE_HOOK_' so they're always in a separate namespace.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/PVE/GuestHelpers.pm | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/PVE/GuestHelpers.pm b/src/PVE/GuestHelpers.pm
index f8d112b..1536259 100644
--- a/src/PVE/GuestHelpers.pm
+++ b/src/PVE/GuestHelpers.pm
@@ -115,14 +115,23 @@ sub check_hookscript {
 }
 
 sub exec_hookscript {
-    my ($conf, $vmid, $phase, $stop_on_error) = @_;
+    my ($conf, $vmid, $phase, $stop_on_error, $params) = @_;
 
     return if !$conf->{hookscript};
 
+    $params //= {};
+
     eval {
         my $hookscript = check_hookscript($conf->{hookscript});
         die $@ if $@;
 
+        local %ENV;
+
+        for my $key (keys $params->%*) {
+            my $new_key = "PVE_HOOK_" . uc($key);
+            $ENV{$new_key} = $params->{$key};
+        }
+
         PVE::Tools::run_command([$hookscript, $vmid, $phase]);
     };
     if (my $err = $@) {
-- 
2.47.3

[PATCH qemu-server v3 1/2] pci: factor 'prepare_pci_devices' out to PVE::QemuServer::PCI module

[Dominik Csapak]

only slight change to the logic here was to return a uuid from an mdev
in any case if we need it, and overwrite it in vm_start_nolock with
the one from smbios1 if possible, instead of doing it the other way
round.

This was necessary since we don't have access to parse_smbios1 in the
PCI module.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/PVE/QemuServer.pm     | 45 +++++++--------------------------------
 src/PVE/QemuServer/PCI.pm | 45 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 37 deletions(-)

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 545758dc..73328460 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -5629,45 +5629,16 @@ sub vm_start_nolock {
 
         push $cmd->@*, $state_cmdline->@*;
 
-        for my $device (values $pci_devices->%*) {
-            next if $device->{mdev}; # we don't reserve for mdev devices
-            push $pci_reserve_list->@*, map { $_->{id} } $device->{ids}->@*;
-        }
-
-        # reserve all PCI IDs before actually doing anything with them
-        PVE::QemuServer::PCI::reserve_pci_usage($pci_reserve_list, $vmid, $start_timeout);
-
-        my $uuid;
-        for my $id (sort keys %$pci_devices) {
-            my $d = $pci_devices->{$id};
-            my ($index) = ($id =~ m/^hostpci(\d+)$/);
-
-            my $chosen_mdev;
-            for my $dev ($d->{ids}->@*) {
-                my $info =
-                    eval { PVE::QemuServer::PCI::prepare_pci_device($vmid, $dev->{id}, $index, $d) };
-                if ($d->{mdev} || $d->{nvidia}) {
-                    warn $@ if $@;
-                    $chosen_mdev = $info;
-                    last if $chosen_mdev; # if successful, we're done
-                } else {
-                    die $@ if $@;
-                }
-            }
-
-            next if !$d->{mdev} && !$d->{nvidia};
-            die "could not create mediated device\n" if !defined($chosen_mdev);
+        ($pci_reserve_list, my $uuid) =
+            PVE::QemuServer::PCI::prepare_pci_devices($conf, $vmid, $pci_devices, $start_timeout);
 
-            # nvidia grid needs the uuid of the mdev as qemu parameter
-            if (!defined($uuid) && $chosen_mdev->{vendor} =~ m/^(0x)?10de$/) {
-                if (defined($conf->{smbios1})) {
-                    my $smbios_conf = parse_smbios1($conf->{smbios1});
-                    $uuid = $smbios_conf->{uuid} if defined($smbios_conf->{uuid});
-                }
-                $uuid = PVE::QemuServer::PCI::generate_mdev_uuid($vmid, $index)
-                    if !defined($uuid);
-            }
+        # uuid for nvidia vgpu
+        # prefer the smbios1 uuid if we have and need it
+        if (defined($uuid) && defined($conf->{smbios1})) {
+            my $smbios_conf = parse_smbios1($conf->{smbios1});
+            $uuid = $smbios_conf->{uuid} if defined($smbios_conf->{uuid});
         }
+
         push @$cmd, '-uuid', $uuid if defined($uuid);
     };
     if (my $err = $@) {
diff --git a/src/PVE/QemuServer/PCI.pm b/src/PVE/QemuServer/PCI.pm
index c9cf8de0..f778c60f 100644
--- a/src/PVE/QemuServer/PCI.pm
+++ b/src/PVE/QemuServer/PCI.pm
@@ -736,6 +736,51 @@ sub print_hostpci_devices {
     return ($kvm_off, $gpu_passthrough, $legacy_igd, $pci_devices);
 }
 
+sub prepare_pci_devices {
+    my ($conf, $vmid, $pci_devices, $start_timeout) = @_;
+
+    my $pci_reserve_list = [];
+    my $uuid;
+
+    for my $device (values $pci_devices->%*) {
+        next if $device->{mdev}; # we don't reserve for mdev devices
+        push $pci_reserve_list->@*, map { $_->{id} } $device->{ids}->@*;
+    }
+
+    # reserve all PCI IDs before actually doing anything with them
+    reserve_pci_usage($pci_reserve_list, $vmid, $start_timeout);
+
+    for my $id (sort keys %$pci_devices) {
+        my $d = $pci_devices->{$id};
+        my ($index) = ($id =~ m/^hostpci(\d+)$/);
+
+        my $chosen_mdev;
+        for my $dev ($d->{ids}->@*) {
+            my $info =
+                eval { prepare_pci_device($vmid, $dev->{id}, $index, $d) };
+            if ($d->{mdev} || $d->{nvidia}) {
+                warn $@ if $@;
+                $chosen_mdev = $info;
+                last if $chosen_mdev; # if successful, we're done
+            } else {
+                die $@ if $@;
+            }
+        }
+
+        next if !$d->{mdev} && !$d->{nvidia};
+        die "could not create mediated device\n" if !defined($chosen_mdev);
+
+        # nvidia vgpu need a uuid parameter, we want the smbios one but we can't parse
+        # that here, so returnt any mdev uuid to signal we want one and as a fallback,
+        # in case there is not smbios uuid
+        if (!defined($uuid) && $chosen_mdev->{vendor} =~ m/^(0x)?10de$/) {
+            $uuid = generate_mdev_uuid($vmid, $index) if !defined($uuid);
+        }
+    }
+
+    return ($pci_reserve_list, $uuid);
+}
+
 sub prepare_pci_device {
     my ($vmid, $pciid, $index, $device) = @_;
 
-- 
2.47.3

[PATCH qemu-server v3 2/2] pci: call hookscript for each prepared pci device

[Dominik Csapak]

There are situations where a user might want to do extra things
for a passed through PCI device after it has been prepared/created (e.g.
in case of vGPU/mdev) but before the actual QEMU process is started.

Two examples are (both are used with NVIDIA vGPUs):
* setting 'vgpu_params' such as removing the frame-rate-limiter
* setting the gpu_instance_id for MIG devices

So instead of creating (nvidia-specific) interfaces for these, give a
user the ability to do it themselves via the hookscript as a first step.

Call it for each prepared device, so that we can give the hookscript the
'hostpciX' id, and the used uuid (in case of mdevs) or the pci id (in
case of regular or modern vGPU passthrough).

Include the generated mdev uuid in the return value of
`prepare_pci_device`, to avoid having to generate that multiple times.
With that we can get rid of one extra generation here too.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/PVE/QemuServer/PCI.pm | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/src/PVE/QemuServer/PCI.pm b/src/PVE/QemuServer/PCI.pm
index f778c60f..5d9c7ab2 100644
--- a/src/PVE/QemuServer/PCI.pm
+++ b/src/PVE/QemuServer/PCI.pm
@@ -761,9 +761,28 @@ sub prepare_pci_devices {
             if ($d->{mdev} || $d->{nvidia}) {
                 warn $@ if $@;
                 $chosen_mdev = $info;
-                last if $chosen_mdev; # if successful, we're done
+                if (defined($chosen_mdev)) {
+                    my $params = { id => $id, pciid => $chosen_mdev->{name} };
+                    $params->{mdev_uuid} = $chosen_mdev->{uuid};
+                    PVE::GuestHelpers::exec_hookscript(
+                        $conf, $vmid, 'post-pci-prepare', 1, $params,
+                    );
+                    last;
+                }
             } else {
                 die $@ if $@;
+                if (defined($info)) {
+                    PVE::GuestHelpers::exec_hookscript(
+                        $conf,
+                        $vmid,
+                        'post-pci-prepare',
+                        1,
+                        {
+                            id => $id,
+                            pciid => $info->{name},
+                        },
+                    );
+                }
             }
         }
 
@@ -774,7 +793,7 @@ sub prepare_pci_devices {
         # that here, so returnt any mdev uuid to signal we want one and as a fallback,
         # in case there is not smbios uuid
         if (!defined($uuid) && $chosen_mdev->{vendor} =~ m/^(0x)?10de$/) {
-            $uuid = generate_mdev_uuid($vmid, $index) if !defined($uuid);
+            $uuid = $chosen_mdev->{uuid} if !defined($uuid);
         }
     }
 
@@ -795,6 +814,7 @@ sub prepare_pci_device {
     } elsif (my $mdev = $device->{mdev}) {
         my $uuid = generate_mdev_uuid($vmid, $index);
         PVE::SysFSTools::pci_create_mdev_device($pciid, $uuid, $mdev);
+        $info->{uuid} = $uuid;
     } else {
         die "can't unbind/bind PCI group to VFIO '$pciid'\n"
             if !PVE::SysFSTools::pci_dev_group_bind_to_vfio($pciid);
-- 
2.47.3

[PATCH docs v3 1/1] examples: add new hookscript phase to example hookscript

[Dominik Csapak]

qemu-server has a new phase 'post-pci-prepare' that is called for vms
with pci passthrough for each device prepared.

add that to the example hookscript and explain when it's called and it's
parameters with a comment.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 examples/guest-example-hookscript.pl | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/examples/guest-example-hookscript.pl b/examples/guest-example-hookscript.pl
index 1cce2e3..da14fd1 100755
--- a/examples/guest-example-hookscript.pl
+++ b/examples/guest-example-hookscript.pl
@@ -38,6 +38,31 @@ if ($phase eq 'pre-start') {
 
     print "$vmid started successfully.\n";
 
+} elsif ($phase eq 'post-pci-prepare') {
+
+    # Only called for virtual machines, not containers.
+    #
+    # This phase will be called for each pci device that is passed through,
+    # after it was prepared by the PVE stack. In other words when either
+    # * the mdev/vGPU was created
+    # * the driver was changed to vfio-pci and the device was reset
+    #
+    # This phase has 3 additional parameters given via the environment
+
+    # the id from the config, e.g. 'hostpci0'
+    my $hostpci_x = $ENV{PVE_HOOK_ID};
+
+    # the pciid of the passed through device or the underlying device in case of an mdev/vGPU
+    # e.g. '0000:01:00.0'
+    my $pciid = $ENV{PVE_HOOK_PCIID};
+
+    # the uuid of the mediated device if it was one,
+    # e.g. '00000001-0000-0000-0000-000000008006'
+    my $mdev_uuid = $ENV{PVE_HOOK_MDEV_UUID};
+
+    print "Prepared PCI device for $hostpci_x with pciid: $pciid.\n";
+    print "It is a mediated device with UUID: $mdev_uuid\n" if $mdev_uuid;
+
 } elsif ($phase eq 'pre-stop') {
 
     # Third phase 'pre-stop' will be executed before stopping the guest
-- 
2.47.3