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

[PATCH docs/guest-common/qemu-server v4 0/7] 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 last qemu-server commit for why that is interesting.

changes from v4:
* rebase on current master
* fix `local %ENV` clearing the whole environment instead of
  just shadowing it, so the hookscript keeps the inherited environment
* split the uuid handling out of the refactoring commit
* call the hookscript once for both the mdev and the plain passthrough
  case instead of duplicating the call in both branches
* add missing `use PVE::GuestHelpers;`
* move the new phase before 'post-start' to match the actual call
  order, document that PVE_HOOK_MDEV_UUID is only set for mdevs, and
  mention what the phase is useful for
* fix typos

changes from v3:
* split unrelated logic/refactoring changes out to their own patches
* fix typos

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 | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)


qemu-server:

Dominik Csapak (5):
  pci: mdev preparation: always generate local uuid first
  pci: nvidia vgpu: correct wrong comment about uuid
  pci: factor 'prepare_pci_devices' out to PVE::QemuServer::PCI module
  pci: preparation: mdev: only generate uuid once
  pci: call hookscript for each prepared pci device

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


pve-docs:

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

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


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

-- 
Generated by murpp 0.11.0

[PATCH guest-common v4 1/7] 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 | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/src/PVE/GuestHelpers.pm b/src/PVE/GuestHelpers.pm
index f8d112b..6899b05 100644
--- a/src/PVE/GuestHelpers.pm
+++ b/src/PVE/GuestHelpers.pm
@@ -115,14 +115,24 @@ 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 $@;
 
+        # copy EVN before adding to it
+        local %ENV = (%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 v4 2/7] pci: mdev preparation: always generate local uuid first

[Dominik Csapak]

We still prefer the uuid from the smbios config, but this makes the
pci preparation code independent of `parse_smbios1` and we can later
move it out.

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

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 63d8c135..ff916667 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -5728,14 +5728,17 @@ sub vm_start_nolock {
 
             # 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::Mdev::generate_mdev_uuid($vmid, $index)
-                    if !defined($uuid);
+                $uuid = PVE::QemuServer::PCI::Mdev::generate_mdev_uuid($vmid, $index);
             }
         }
+
+        # 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 = $@) {
-- 
2.47.3

[PATCH qemu-server v4 3/7] pci: nvidia vgpu: correct wrong comment about uuid

[Dominik Csapak]

At the time the mdev code was written, it was believed that the uuid of
the qemu vm has to be the same as the mdev. This is obviously not true,
since we already prefer the smbios uuid, and if more than one vGPU is
given to a VM, this can't hold.

So the comment must be wrong, and the nvidia driver simply expects *a*
uuid, not the one of the mdev.

Also, it's not named NVIDIA GRID anymore, so use the current vGPU
terminology.

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

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index ff916667..9c4c13b9 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -5726,7 +5726,7 @@ sub vm_start_nolock {
             next if !$d->{mdev} && !$d->{nvidia};
             die "could not create mediated device\n" if !defined($chosen_mdev);
 
-            # nvidia grid needs the uuid of the mdev as qemu parameter
+            # nvidia vgpu needs a uuid as qemu parameter
             if (!defined($uuid) && $chosen_mdev->{vendor} =~ m/^(0x)?10de$/) {
                 $uuid = PVE::QemuServer::PCI::Mdev::generate_mdev_uuid($vmid, $index);
             }
-- 
2.47.3

[PATCH qemu-server v4 4/7] pci: factor 'prepare_pci_devices' out to PVE::QemuServer::PCI module

[Dominik Csapak]

no functional change intended.

Note: $conf is unused in `prepare_pci_devices` for now, but will be used
later.

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

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 9c4c13b9..3eeea871 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -5697,40 +5697,8 @@ 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);
-
-            # nvidia vgpu needs a uuid as qemu parameter
-            if (!defined($uuid) && $chosen_mdev->{vendor} =~ m/^(0x)?10de$/) {
-                $uuid = PVE::QemuServer::PCI::Mdev::generate_mdev_uuid($vmid, $index);
-            }
-        }
+        ($pci_reserve_list, my $uuid) =
+            PVE::QemuServer::PCI::prepare_pci_devices($conf, $vmid, $pci_devices, $start_timeout);
 
         # uuid for nvidia vgpu
         # prefer the smbios1 uuid if we have and need it
diff --git a/src/PVE/QemuServer/PCI.pm b/src/PVE/QemuServer/PCI.pm
index 0b67943c..f7167dcc 100644
--- a/src/PVE/QemuServer/PCI.pm
+++ b/src/PVE/QemuServer/PCI.pm
@@ -704,6 +704,48 @@ 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 needs a uuid as qemu parameter
+        if (!defined($uuid) && $chosen_mdev->{vendor} =~ m/^(0x)?10de$/) {
+            $uuid = PVE::QemuServer::PCI::Mdev::generate_mdev_uuid($vmid, $index);
+        }
+    }
+
+    return ($pci_reserve_list, $uuid);
+}
+
 sub prepare_pci_device {
     my ($vmid, $pciid, $index, $device) = @_;
 
-- 
2.47.3

[PATCH qemu-server v4 5/7] pci: preparation: mdev: only generate uuid once

[Dominik Csapak]

In `prepare_pci_device` we already generate a uuid for the mdev. Later,
for NVIDIA vGPU, we need the uuid again to add it to the qemu cmdline.
Instead of generating it twice, return it from `prepare_pci_device` and
reuse that if we can.

If the device uses the modern nvidia vgpu stack, we don't need to create
a mdev, so generate the fallback uuid in `prepare_pci_devices`.

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

diff --git a/src/PVE/QemuServer/PCI.pm b/src/PVE/QemuServer/PCI.pm
index f7167dcc..5d2fdabb 100644
--- a/src/PVE/QemuServer/PCI.pm
+++ b/src/PVE/QemuServer/PCI.pm
@@ -739,7 +739,8 @@ sub prepare_pci_devices {
 
         # nvidia vgpu needs a uuid as qemu parameter
         if (!defined($uuid) && $chosen_mdev->{vendor} =~ m/^(0x)?10de$/) {
-            $uuid = PVE::QemuServer::PCI::Mdev::generate_mdev_uuid($vmid, $index);
+            $uuid = $chosen_mdev->{uuid}
+                // PVE::QemuServer::PCI::Mdev::generate_mdev_uuid($vmid, $index);
         }
     }
 
@@ -760,6 +761,7 @@ sub prepare_pci_device {
     } elsif (my $mdev = $device->{mdev}) {
         my $uuid = PVE::QemuServer::PCI::Mdev::generate_mdev_uuid($vmid, $index);
         PVE::QemuServer::PCI::Mdev::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 qemu-server v4 6/7] 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).

A failing hookscript aborts, and blocks the VM start.

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

diff --git a/src/PVE/QemuServer/PCI.pm b/src/PVE/QemuServer/PCI.pm
index 5d2fdabb..755945a9 100644
--- a/src/PVE/QemuServer/PCI.pm
+++ b/src/PVE/QemuServer/PCI.pm
@@ -5,6 +5,7 @@ use strict;
 
 use IO::File;
 
+use PVE::GuestHelpers;
 use PVE::JSONSchema;
 use PVE::Mapping::PCI;
 use PVE::SysFSTools;
@@ -728,10 +729,17 @@ sub prepare_pci_devices {
             if ($d->{mdev} || $d->{nvidia}) {
                 warn $@ if $@;
                 $chosen_mdev = $info;
-                last if $chosen_mdev; # if successful, we're done
             } else {
                 die $@ if $@;
             }
+
+            next if !defined($info);
+
+            my $params = { id => $id, pciid => $info->{name} };
+            $params->{mdev_uuid} = $info->{uuid} if defined($info->{uuid});
+            PVE::GuestHelpers::exec_hookscript($conf, $vmid, 'post-pci-prepare', 1, $params);
+
+            last if defined($chosen_mdev);
         }
 
         next if !$d->{mdev} && !$d->{nvidia};
-- 
2.47.3

[PATCH docs v4 7/7] 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 its
parameters with a comment.

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

diff --git a/examples/guest-example-hookscript.pl b/examples/guest-example-hookscript.pl
index 1cce2e3..02e7e7d 100755
--- a/examples/guest-example-hookscript.pl
+++ b/examples/guest-example-hookscript.pl
@@ -31,6 +31,35 @@ if ($phase eq 'pre-start') {
     # print "preparations failed, aborting."
     # exit(1);
 
+} 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 Proxmox VE 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 can be useful to do additional preparation that the Proxmox VE
+    # stack does not do, like setting vgpu_params for NVIDIA vGPU.
+    #
+    # This phase has 3 additional parameters (one optional) given via the environment:
+
+    # the id from the config, e.g. 'hostpci0'
+    my $hostpci_id = $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'
+    # This is not set if the pci device is not an mdev.
+    my $mdev_uuid = $ENV{PVE_HOOK_MDEV_UUID};
+
+    print "Prepared PCI device for $hostpci_id with pciid: $pciid.\n";
+    print "It is a mediated device with UUID: $mdev_uuid\n" if $mdev_uuid;
+
 } elsif ($phase eq 'post-start') {
 
     # Second phase 'post-start' will be executed after the guest
-- 
2.47.3