Am 24/01/2024 um 10:49 schrieb Markus Frank:
> vIOMMU is the emulation of a hardware IOMMU within a virtual machine,
> providing improved memory access control and security for virtualized I/O 
> devices.
> vIOMMU also enables the option to passthrough pci devices to L2 VMs
> in L1 VMs via Nested Virtualisation.
> 
> Currently there are two vIOMMU implementation in QEMU to choose:
> intel & virtio
> 
> Virtio-iommu is more recent but less used in production than intel-iommu.
> 
> The check_machine_config function prevents using intel-iommu with
> i440fx.
> 
> Signed-off-by: Markus Frank <m.fr...@proxmox.com>
> ---
>  PVE/API2/Qemu.pm                        |  2 ++
>  PVE/QemuServer.pm                       | 12 ++++++++++++
>  PVE/QemuServer/Machine.pm               | 17 ++++++++++++++++-
>  test/cfg2cmd/q35-viommu-intel.conf      |  1 +
>  test/cfg2cmd/q35-viommu-intel.conf.cmd  | 23 +++++++++++++++++++++++
>  test/cfg2cmd/q35-viommu-virtio.conf     |  1 +
>  test/cfg2cmd/q35-viommu-virtio.conf.cmd | 23 +++++++++++++++++++++++
>  7 files changed, 78 insertions(+), 1 deletion(-)
>  create mode 100644 test/cfg2cmd/q35-viommu-intel.conf
>  create mode 100644 test/cfg2cmd/q35-viommu-intel.conf.cmd
>  create mode 100644 test/cfg2cmd/q35-viommu-virtio.conf
>  create mode 100644 test/cfg2cmd/q35-viommu-virtio.conf.cmd
> 

this one needs to be rebased.

> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index c23b16a..4a5a833 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -1056,6 +1056,7 @@ __PACKAGE__->register_method({
>                           $conf->{machine} = 
> PVE::QemuServer::Machine::print_machine($machine_conf);
>                       }
>                   }
> +                 PVE::QemuServer::Machine::check_machine_config($conf, 
> $machine_conf);
>  
>                   PVE::QemuConfig->write_config($vmid, $conf);
>  
> @@ -1894,6 +1895,7 @@ my $update_vm_api  = sub {
>                   $conf->{pending}->{$opt} = $param->{$opt};
>               } elsif ($opt eq 'machine') {
>                   my $machine_conf = 
> PVE::QemuServer::Machine::parse_machine($param->{$opt});
> +                 PVE::QemuServer::Machine::check_machine_config($conf, 
> $machine_conf);
>                   $conf->{pending}->{$opt} = $param->{$opt};
>               } else {
>                   $conf->{pending}->{$opt} = $param->{$opt};
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 6bb2ec3..92832f8 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -4070,6 +4070,18 @@ sub config_to_command {
>      }
>      push @$machineFlags, "type=${machine_type_min}";
>  
> +    PVE::QemuServer::Machine::check_machine_config($conf, $machine_conf);


> +
> +    if ($machine_conf->{viommu}) {
> +     if ($machine_conf->{viommu} eq 'intel') {
> +         unshift @$devices, '-device', 
> 'intel-iommu,intremap=on,caching-mode=on';
> +         push @$machineFlags, 'kernel-irqchip=split';
> +     }
> +     if ($machine_conf->{viommu} eq 'virtio') {


could be merged with the line before as `} elsif (...) {`

> +         push @$devices, '-device', 'virtio-iommu-pci';
> +     }
> +    }
> +
>      push @$cmd, @$devices;
>      push @$cmd, '-rtc', join(',', @$rtcFlags) if scalar(@$rtcFlags);
>      push @$cmd, '-machine', join(',', @$machineFlags) if 
> scalar(@$machineFlags);
> diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
> index 5e3a75c..71790c4 100644
> --- a/PVE/QemuServer/Machine.pm
> +++ b/PVE/QemuServer/Machine.pm
> @@ -23,12 +23,19 @@ my $machine_fmt = {
>       format_description => 'machine type',
>       optional => 1,
>      },
> +    viommu => {
> +     type => 'string',
> +     description => "Enable/disable guest vIOMMU"
> +         ." (needs kvm to be enabled and q35 to be set as machine type).",

early newline

> +     enum => ['intel', 'virtio'],
> +     optional => 1,
> +    },
>  };
>  
>  PVE::JSONSchema::register_format('pve-qemu-machine-fmt', $machine_fmt);
>  
>  PVE::JSONSchema::register_standard_option('pve-qemu-machine', {
> -    description => "Specify the QEMU machine type.",
> +    description => "Specify the QEMU machine type & enable/disable vIOMMU.",
>      type => 'string',
>      optional => 1,
>      format => PVE::JSONSchema::get_format('pve-qemu-machine-fmt'),
> @@ -48,6 +55,14 @@ sub print_machine {
>      return print_property_string($machine_conf, $machine_fmt);
>  }
>  
> +sub check_machine_config {

maybe name that `assert_valid_machine_property` to better convey that it can die

> +    my ($conf, $machine_conf) = @_;
> +    my $q35 = $machine_conf->{type} && ($machine_conf->{type} =~ m/q35/) ? 1 
> : 0;
> +    if ($machine_conf->{viommu} && $machine_conf->{viommu} eq "intel" && 
> !$q35) {
> +     die "to use Intel vIOMMU please set the machine type to q35\n";
> +    }
> +}
> +
>  sub machine_type_is_q35 {
>      my ($conf) = @_;
>  
> diff --git a/test/cfg2cmd/q35-viommu-intel.conf 
> b/test/cfg2cmd/q35-viommu-intel.conf
> new file mode 100644
> index 0000000..e500ab0
> --- /dev/null
> +++ b/test/cfg2cmd/q35-viommu-intel.conf

one test is great, but they do not cost that much (even if big line-wise) so 
maybe
also test the `virtio` one and some error behavior (e.g., enabled on a VM that 
has
i440fx as machine-type (one can specify an expected error in a source config, 
grep
"EXPECT_ERROR" for examples).


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to