[pve-devel] [PATCH-SERIES qemu-server/manager 0/7] VM CPU flags: introduce vendor-agnostic 'nested-virt' CPU flag

[pve-devel] [PATCH-SERIES qemu-server/manager 0/7] VM CPU flags: introduce vendor-agnostic 'nested-virt' CPU flag

[Fiona Ebner]

The flag will automatically resolve to the flag required for the
current CPU on the host. The 'nested-virt' flag takes precendence over
the CPU-specific flag for nesting which might already be present with
custom CPU models. In that case, a warning is printed.

Moves the cpu flag descriptions to qemu-server and introduces an API
endpoint for querying. Could later be extended with further
information, for example, which flags are actually supported on the
current host (some are specific to vendor or CPU).

Dependency and build-dependency bump pve-manger -> qemu-server needed!

qemu-server:

Fiona Ebner (4):
  cpu config: style fix: avoid multiline post-if expressions
  cpu config: style fix: avoid overly long ternary conditional
    expression
  api: add endpoint for querying available cpu flags
  cpu config: introduce vendor-agnostic 'nested-virt' CPU flag

 src/PVE/API2/Qemu/CPUFlags.pm   |  45 +++++++
 src/PVE/API2/Qemu/Makefile      |   2 +-
 src/PVE/QemuServer/CPUConfig.pm | 222 ++++++++++++++++++++++----------
 3 files changed, 199 insertions(+), 70 deletions(-)
 create mode 100644 src/PVE/API2/Qemu/CPUFlags.pm


pve-manager:

Fiona Ebner (3):
  api: capabilities: register module for VM CPU flags
  ui: cpu flag selector: code style: use 'let' for declarations
  ui: cpu flag selector: query CPU flag list via API

 PVE/API2/Capabilities.pm               |   6 ++
 www/manager6/form/VMCPUFlagSelector.js | 126 +++++++++++--------------
 2 files changed, 61 insertions(+), 71 deletions(-)


Summary over all repositories:
  5 files changed, 260 insertions(+), 141 deletions(-)

-- 
Generated by git-murpp 0.5.0


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

[pve-devel] [PATCH qemu-server 1/7] cpu config: style fix: avoid multiline post-if expressions

[Fiona Ebner]

Should improve readability. No functional change intended.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/QemuServer/CPUConfig.pm | 98 +++++++++++++++++----------------
 1 file changed, 52 insertions(+), 46 deletions(-)

diff --git a/src/PVE/QemuServer/CPUConfig.pm b/src/PVE/QemuServer/CPUConfig.pm
index 786a99d8..1fdae415 100644
--- a/src/PVE/QemuServer/CPUConfig.pm
+++ b/src/PVE/QemuServer/CPUConfig.pm
@@ -326,19 +326,19 @@ sub validate_vm_cpu_conf {
     if (is_custom_model($cputype)) {
         # dies on unknown model
         get_custom_model($cputype);
-    } else {
-        die "Built-in cputype '$cputype' is not defined (missing 'custom-' prefix?)\n"
-            if !defined($cpu_vendor_list->{$cputype}) && !defined($builtin_models->{$cputype});
+    } elsif (!defined($cpu_vendor_list->{$cputype}) && !defined($builtin_models->{$cputype})) {
+        die "Built-in cputype '$cputype' is not defined (missing 'custom-' prefix?)\n";
     }
 
     # in a VM-specific config, certain properties are limited/forbidden
 
-    die "VM-specific CPU flags must be a subset of: @{[join(', ', @supported_cpu_flags)]}\n"
-        if ($cpu->{flags}
-            && $cpu->{flags} !~ m/^$cpu_flag_supported_re(;$cpu_flag_supported_re)*$/);
+    if ($cpu->{flags} && $cpu->{flags} !~ m/^$cpu_flag_supported_re(;$cpu_flag_supported_re)*$/) {
+        die "VM-specific CPU flags must be a subset of: @{[join(', ', @supported_cpu_flags)]}\n";
+    }
 
-    die "Property 'reported-model' not allowed in VM-specific CPU config.\n"
-        if defined($cpu->{'reported-model'});
+    if (defined($cpu->{'reported-model'})) {
+        die "Property 'reported-model' not allowed in VM-specific CPU config.\n";
+    }
 
     return $cpu;
 }
@@ -387,13 +387,15 @@ sub write_config {
     for my $model (keys %{ $cfg->{ids} }) {
         my $model_conf = $cfg->{ids}->{$model};
 
-        die
-            "internal error: tried saving built-in CPU model (or missing prefix): $model_conf->{cputype}\n"
-            if !is_custom_model($model_conf->{cputype});
+        if (!is_custom_model($model_conf->{cputype})) {
+            die "internal error: tried saving built-in CPU model (or missing prefix):"
+                . " $model_conf->{cputype}\n";
+        }
 
-        die
-            "internal error: tried saving custom cpumodel with cputype (ignoring prefix: $model_conf->{cputype}) not equal to \$cfg->ids entry ($model)\n"
-            if "custom-$model" ne $model_conf->{cputype};
+        if ("custom-$model" ne $model_conf->{cputype}) {
+            die "internal error: tried saving custom cpumodel with cputype (ignoring prefix:"
+                . " $model_conf->{cputype}) not equal to \$cfg->ids entry ($model)\n";
+        }
 
         # saved in section header
         delete $model_conf->{cputype};
@@ -654,23 +656,23 @@ sub get_cpu_options {
     my $vm_flags = parse_cpuflag_list($cpu_flag_supported_re, "manually set for VM", $cpu->{flags});
 
     my $pve_forced_flags = {};
-    $pve_forced_flags->{'enforce'} = {
-        reason => "error if requested CPU settings not available",
-        }
-        if $cputype ne 'host' && $kvm && $arch eq 'x86_64';
-    $pve_forced_flags->{'kvm'} = {
-        value => "off",
-        reason => "hide KVM virtualization from guest",
-        }
-        if $kvm_off;
+    if ($cputype ne 'host' && $kvm && $arch eq 'x86_64') {
+        $pve_forced_flags->{'enforce'} = {
+            reason => "error if requested CPU settings not available",
+        };
+    }
+    if ($kvm_off) {
+        $pve_forced_flags->{'kvm'} = {
+            value => "off",
+            reason => "hide KVM virtualization from guest",
+        };
+    }
 
     # $cputype is the "reported-model" for custom types, so we can just look up
     # the vendor in the default list
     my $cpu_vendor = $cpu_vendor_list->{$cputype};
     if ($cpu_vendor) {
-        $pve_forced_flags->{'vendor'} = {
-            value => $cpu_vendor,
-        } if $cpu_vendor ne 'default';
+        $pve_forced_flags->{'vendor'} = { value => $cpu_vendor } if $cpu_vendor ne 'default';
     } elsif ($arch ne 'aarch64') {
         die "internal error"; # should not happen
     }
@@ -714,29 +716,33 @@ sub get_pve_cpu_flags {
     my $pve_flags = {};
     my $pve_msg = "set by PVE;";
 
-    $pve_flags->{'lahf_lm'} = {
-        op => '+',
-        reason => "$pve_msg to support Windows 8.1+",
-        }
-        if $cputype eq 'kvm64' && $arch eq 'x86_64';
+    if ($cputype eq 'kvm64' && $arch eq 'x86_64') {
+        $pve_flags->{'lahf_lm'} = {
+            op => '+',
+            reason => "$pve_msg to support Windows 8.1+",
+        };
+    }
 
-    $pve_flags->{'x2apic'} = {
-        op => '-',
-        reason => "$pve_msg incompatible with Solaris",
-        }
-        if $conf->{ostype} && $conf->{ostype} eq 'solaris';
+    if ($conf->{ostype} && $conf->{ostype} eq 'solaris') {
+        $pve_flags->{'x2apic'} = {
+            op => '-',
+            reason => "$pve_msg incompatible with Solaris",
+        };
+    }
 
-    $pve_flags->{'sep'} = {
-        op => '+',
-        reason => "$pve_msg to support Windows 8+ and improve Windows XP+",
-        }
-        if $cputype eq 'kvm64' || $cputype eq 'kvm32';
+    if ($cputype eq 'kvm64' || $cputype eq 'kvm32') {
+        $pve_flags->{'sep'} = {
+            op => '+',
+            reason => "$pve_msg to support Windows 8+ and improve Windows XP+",
+        };
+    }
 
-    $pve_flags->{'rdtscp'} = {
-        op => '-',
-        reason => "$pve_msg broken on AMD Opteron",
-        }
-        if $cputype =~ m/^Opteron/;
+    if ($cputype =~ m/^Opteron/) {
+        $pve_flags->{'rdtscp'} = {
+            op => '-',
+            reason => "$pve_msg broken on AMD Opteron",
+        };
+    }
 
     if (min_version($machine_version, 2, 3) && $kvm && $arch eq 'x86_64') {
         $pve_flags->{'kvm_pv_unhalt'} = {
-- 
2.47.3



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

[pve-devel] [PATCH qemu-server 2/7] cpu config: style fix: avoid overly long ternary conditional expression

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/QemuServer/CPUConfig.pm | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/PVE/QemuServer/CPUConfig.pm b/src/PVE/QemuServer/CPUConfig.pm
index 1fdae415..e72bdf2f 100644
--- a/src/PVE/QemuServer/CPUConfig.pm
+++ b/src/PVE/QemuServer/CPUConfig.pm
@@ -636,16 +636,16 @@ sub get_cpu_options {
 
     my $pve_flags = get_pve_cpu_flags($conf, $kvm, $cputype, $arch, $machine_version);
 
-    my $hv_flags =
-        $kvm
-        ? get_hyperv_enlightenments(
+    my $hv_flags;
+    if ($kvm) {
+        $hv_flags = get_hyperv_enlightenments(
             $winversion,
             $machine_version,
             $conf->{bios},
             $gpu_passthrough,
             $hv_vendor_id,
-        )
-        : undef;
+        );
+    }
 
     my $builtin_cputype_flags =
         parse_cpuflag_list($cpu_flag_any_re, "set by builtin CPU model", $builtin_cpu->{flags});
-- 
2.47.3



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

[pve-devel] [PATCH qemu-server 3/7] api: add endpoint for querying available cpu flags

[Fiona Ebner]

Descriptions and ordering are taken from pve-manager's
VMCPUFlagSelector.js. The double quotes in the descriptions were
replaced with single quotes to have nicer JSON output.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/API2/Qemu/CPUFlags.pm   | 45 +++++++++++++++++++
 src/PVE/API2/Qemu/Makefile      |  2 +-
 src/PVE/QemuServer/CPUConfig.pm | 76 +++++++++++++++++++++++++--------
 3 files changed, 105 insertions(+), 18 deletions(-)
 create mode 100644 src/PVE/API2/Qemu/CPUFlags.pm

diff --git a/src/PVE/API2/Qemu/CPUFlags.pm b/src/PVE/API2/Qemu/CPUFlags.pm
new file mode 100644
index 00000000..cc06a1d6
--- /dev/null
+++ b/src/PVE/API2/Qemu/CPUFlags.pm
@@ -0,0 +1,45 @@
+package PVE::API2::Qemu::CPUFlags;
+
+use v5.36;
+
+use PVE::RESTHandler;
+use PVE::JSONSchema qw(get_standard_option);
+use PVE::QemuServer::CPUConfig;
+
+use base qw(PVE::RESTHandler);
+
+__PACKAGE__->register_method({
+    name => 'index',
+    path => '',
+    method => 'GET',
+    description => 'List of available VM-specific CPU flags.',
+    permissions => { user => 'all' },
+    parameters => {
+        additionalProperties => 0,
+        properties => {
+            node => get_standard_option('pve-node'),
+        },
+    },
+    returns => {
+        type => 'array',
+        items => {
+            type => 'object',
+            properties => {
+                name => {
+                    type => 'string',
+                    description => "Name of the CPU flag.",
+                },
+                description => {
+                    type => 'string',
+                    description => "Description of the CPU flag.",
+                },
+            },
+        },
+        links => [{ rel => 'child', href => '{flag}' }],
+    },
+    code => sub {
+        return $PVE::QemuServer::CPUConfig::supported_cpu_flags;
+    },
+});
+
+1;
diff --git a/src/PVE/API2/Qemu/Makefile b/src/PVE/API2/Qemu/Makefile
index 7c539702..c348af75 100644
--- a/src/PVE/API2/Qemu/Makefile
+++ b/src/PVE/API2/Qemu/Makefile
@@ -2,7 +2,7 @@ DESTDIR=
 PREFIX=/usr
 PERLDIR=$(PREFIX)/share/perl5
 
-SOURCES=Agent.pm CPU.pm HMPPerms.pm Machine.pm
+SOURCES=Agent.pm CPU.pm CPUFlags.pm HMPPerms.pm Machine.pm
 
 .PHONY: install
 install:
diff --git a/src/PVE/QemuServer/CPUConfig.pm b/src/PVE/QemuServer/CPUConfig.pm
index e72bdf2f..20e26ee2 100644
--- a/src/PVE/QemuServer/CPUConfig.pm
+++ b/src/PVE/QemuServer/CPUConfig.pm
@@ -161,21 +161,62 @@ my $cpu_vendor_list = {
     max => 'default',
 };
 
-my @supported_cpu_flags = (
-    'pcid',
-    'spec-ctrl',
-    'ibpb',
-    'ssbd',
-    'virt-ssbd',
-    'amd-ssbd',
-    'amd-no-ssb',
-    'pdpe1gb',
-    'md-clear',
-    'hv-tlbflush',
-    'hv-evmcs',
-    'aes',
-);
-my $cpu_flag_supported_re = qr/([+-])(@{[join('|', @supported_cpu_flags)]})/;
+our $supported_cpu_flags = [
+    {
+        name => 'md-clear',
+        description => "Required to let the guest OS know if MDS is mitigated correctly.",
+    },
+    {
+        name => 'pcid',
+        description =>
+            "Meltdown fix cost reduction on Westmere, Sandy-, and IvyBridge Intel CPUs.",
+    },
+    {
+        name => 'spec-ctrl',
+        description => "Allows improved Spectre mitigation with Intel CPUs.",
+    },
+    {
+        name => 'ssbd',
+        description => "Protection for 'Speculative Store Bypass' for Intel models.",
+    },
+    {
+        name => 'ibpb',
+        description => "Allows improved Spectre mitigation with AMD CPUs.",
+    },
+    {
+        name => 'virt-ssbd',
+        description => "Basis for 'Speculative Store Bypass' protection for AMD models.",
+    },
+    {
+        name => 'amd-ssbd',
+        description => "Improves Spectre mitigation performance with AMD CPUs, best used with"
+            . " 'virt-ssbd'.",
+    },
+    {
+        name => 'amd-no-ssb',
+        description => "Notifies guest OS that host is not vulnerable for Spectre on AMD CPUs.",
+    },
+    {
+        name => 'pdpe1gb',
+        description => "Allow guest OS to use 1GB size pages, if host HW supports it.",
+    },
+    {
+        name => 'hv-tlbflush',
+        description => "Improve performance in overcommitted Windows guests. May lead to guest"
+            . " bluescreens on old CPUs.",
+    },
+    {
+        name => 'hv-evmcs',
+        description => "Improve performance for nested virtualization. Only supported on Intel"
+            . " CPUs.",
+    },
+    {
+        name => 'aes',
+        description => "Activate AES instruction set for HW acceleration.",
+    },
+];
+my @supported_cpu_flags_names = map { $_->{name} } $supported_cpu_flags->@*;
+my $cpu_flag_supported_re = qr/([+-])(@{[join('|', @supported_cpu_flags_names)]})/;
 my $cpu_flag_any_re = qr/([+-])([a-zA-Z0-9\-_\.]+)/;
 
 our $qemu_cmdline_cpu_re = qr/^((?>[+-]?[\w\-\._=]+,?)+)$/;
@@ -217,7 +258,7 @@ my $cpu_fmt = {
         description => "List of additional CPU flags separated by ';'. Use '+FLAG' to enable,"
             . " '-FLAG' to disable a flag. Custom CPU models can specify any flag supported by"
             . " QEMU/KVM, VM-specific flags must be from the following set for security reasons: "
-            . join(', ', @supported_cpu_flags),
+            . join(', ', @supported_cpu_flags_names),
         format_description => '+FLAG[;-FLAG...]',
         type => 'string',
         pattern => qr/$cpu_flag_any_re(;$cpu_flag_any_re)*/,
@@ -333,7 +374,8 @@ sub validate_vm_cpu_conf {
     # in a VM-specific config, certain properties are limited/forbidden
 
     if ($cpu->{flags} && $cpu->{flags} !~ m/^$cpu_flag_supported_re(;$cpu_flag_supported_re)*$/) {
-        die "VM-specific CPU flags must be a subset of: @{[join(', ', @supported_cpu_flags)]}\n";
+        die "VM-specific CPU flags must be a subset of: "
+            . join(', ', @supported_cpu_flags_names) . "\n";
     }
 
     if (defined($cpu->{'reported-model'})) {
-- 
2.47.3



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

[pve-devel] [PATCH qemu-server 4/7] cpu config: introduce vendor-agnostic 'nested-virt' CPU flag

[Fiona Ebner]

The flag will automatically resolve to the flag required for the
current CPU on the host. The 'nested-virt' flag takes precendence over
the CPU-specific flag for nesting which might already be present with
custom CPU models. In that case, a warning is printed.

Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

I was thinking about using special characters in the flag name or some
custom-/pve-/special- prefix to distinguish from regular flags, but
decided against it in the end, because I got the gut feeling it might
cause more confusion than it helps. Users who are interested in
details will hopefully read the description and for others, having the
flag name be direct and descriptive is better.

 src/PVE/QemuServer/CPUConfig.pm | 40 +++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/src/PVE/QemuServer/CPUConfig.pm b/src/PVE/QemuServer/CPUConfig.pm
index 20e26ee2..dc8929c9 100644
--- a/src/PVE/QemuServer/CPUConfig.pm
+++ b/src/PVE/QemuServer/CPUConfig.pm
@@ -7,7 +7,10 @@ use JSON;
 
 use PVE::JSONSchema;
 use PVE::Cluster qw(cfs_register_file cfs_read_file);
+use PVE::ProcFSTools;
+use PVE::RESTEnvironment qw(log_warn);
 use PVE::Tools qw(run_command get_host_arch);
+
 use PVE::QemuServer::Helpers qw(min_version);
 
 use base qw(PVE::SectionConfig Exporter);
@@ -162,6 +165,11 @@ my $cpu_vendor_list = {
 };
 
 our $supported_cpu_flags = [
+    {
+        name => 'nested-virt',
+        description => "Controls nested virtualization, namely 'svm' for AMD CPUs and 'vmx' for"
+            . " Intel CPUs.",
+    },
     {
         name => 'md-clear',
         description => "Required to let the guest OS know if MDS is mitigated correctly.",
@@ -256,8 +264,10 @@ my $cpu_fmt = {
     },
     flags => {
         description => "List of additional CPU flags separated by ';'. Use '+FLAG' to enable,"
-            . " '-FLAG' to disable a flag. Custom CPU models can specify any flag supported by"
-            . " QEMU/KVM, VM-specific flags must be from the following set for security reasons: "
+            . " '-FLAG' to disable a flag. There is a special 'nested-virt' shorthand which"
+            . " controls nested virtualization for the current CPU ('svm' for AMD and 'vmx' for"
+            . " Intel). Custom CPU models can specify any flag supported by QEMU/KVM, VM-specific"
+            . " flags must be from the following set for security reasons: "
             . join(', ', @supported_cpu_flags_names),
         format_description => '+FLAG[;-FLAG...]',
         type => 'string',
@@ -578,8 +588,34 @@ sub print_cpu_device {
 sub resolve_cpu_flags {
     my $flags = {};
 
+    my $nested_flag;
+    my $nested_flag_resolved;
+    my $resolve_nested_flag = sub {
+        if (!$nested_flag_resolved) {
+            my $host_cpu_flags = PVE::ProcFSTools::read_cpuinfo()->{flags};
+            if ($host_cpu_flags =~ m/\s(svm|vmx)\s/) {
+                $nested_flag = $1;
+            } else {
+                log_warn("ignoring 'nested-virt' CPU flag - unable to resolve from host CPU flags");
+            }
+            $nested_flag_resolved = 1;
+        }
+        return $nested_flag;
+    };
+
     for my $hash (@_) {
         for my $flag_name (keys %$hash) {
+            if ($flag_name eq 'nested-virt') {
+                my $nested_flag_name = $resolve_nested_flag->() or next;
+                if ($hash->{$nested_flag_name}) {
+                    warn "warning: CPU flag '$flag_name' overrides '$nested_flag_name'\n";
+                } else {
+                    print "CPU flag '$flag_name' resolved to '$nested_flag_name'\n";
+                }
+                $hash->{$nested_flag_name} = delete($hash->{$flag_name});
+                $flag_name = $nested_flag_name;
+            }
+
             my $flag = $hash->{$flag_name};
             my $old_flag = $flags->{$flag_name};
 
-- 
2.47.3



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

[pve-devel] [PATCH manager 5/7] api: capabilities: register module for VM CPU flags

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Depends and build-depends on new qemu-server!

 PVE/API2/Capabilities.pm | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/PVE/API2/Capabilities.pm b/PVE/API2/Capabilities.pm
index d45c548e..a027142b 100644
--- a/PVE/API2/Capabilities.pm
+++ b/PVE/API2/Capabilities.pm
@@ -7,6 +7,7 @@ use PVE::JSONSchema qw(get_standard_option);
 use PVE::RESTHandler;
 
 use PVE::API2::Qemu::CPU;
+use PVE::API2::Qemu::CPUFlags;
 use PVE::API2::Qemu::Machine;
 use PVE::API2::NodeCapabilities::Qemu::Migration;
 
@@ -17,6 +18,11 @@ __PACKAGE__->register_method({
     path => 'qemu/cpu',
 });
 
+__PACKAGE__->register_method({
+    subclass => "PVE::API2::Qemu::CPUFlags",
+    path => 'qemu/cpu-flags',
+});
+
 __PACKAGE__->register_method({
     subclass => "PVE::API2::Qemu::Machine",
     path => 'qemu/machines',
-- 
2.47.3



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

[pve-devel] [PATCH manager 6/7] ui: cpu flag selector: code style: use 'let' for declarations

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 www/manager6/form/VMCPUFlagSelector.js | 28 +++++++++++++-------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/www/manager6/form/VMCPUFlagSelector.js b/www/manager6/form/VMCPUFlagSelector.js
index 874068cf..2dbc21dc 100644
--- a/www/manager6/form/VMCPUFlagSelector.js
+++ b/www/manager6/form/VMCPUFlagSelector.js
@@ -66,13 +66,13 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
     },
 
     getValue: function () {
-        var me = this;
-        var store = me.getStore();
-        var flags = '';
+        let me = this;
+        let store = me.getStore();
+        let flags = '';
 
         // ExtJS does not has a nice getAllRecords interface for stores :/
         store.queryBy(Ext.returnTrue).each(function (rec) {
-            var s = rec.get('state');
+            let s = rec.get('state');
             if (s && s !== '=') {
                 let f = rec.get('flag');
                 if (flags === '') {
@@ -89,8 +89,8 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
     },
 
     setValue: function (value) {
-        var me = this;
-        var store = me.getStore();
+        let me = this;
+        let store = me.getStore();
 
         me.value = value || '';
 
@@ -102,12 +102,12 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
                 rec.set('state', '=');
             });
 
-        var flags = value ? value.split(';') : [];
+        let flags = value ? value.split(';') : [];
         flags.forEach(function (flag) {
-            var sign = flag.substr(0, 1);
+            let sign = flag.substr(0, 1);
             flag = flag.substr(1);
 
-            var rec = store.findRecord('flag', flag, 0, false, true, true);
+            let rec = store.findRecord('flag', flag, 0, false, true, true);
             if (rec !== null) {
                 rec.set('state', sign);
             } else {
@@ -116,7 +116,7 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
         });
         store.reload();
 
-        var res = me.mixins.field.setValue.call(me, value);
+        let res = me.mixins.field.setValue.call(me, value);
 
         return res;
     },
@@ -142,7 +142,7 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
             dataIndex: 'state',
             width: 95,
             onWidgetAttach: function (column, widget, record) {
-                var val = record.get('state') || '=';
+                let val = record.get('state') || '=';
                 widget.down('[inputValue=' + val + ']').setValue(true);
                 // TODO: disable if selected CPU model and flag are incompatible
             },
@@ -154,10 +154,10 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
                 value: '=',
                 listeners: {
                     change: function (f, value) {
-                        var v = Object.values(value)[0];
+                        let v = Object.values(value)[0];
                         f.getWidgetRecord().set('state', v);
 
-                        var view = this.up('grid');
+                        let view = this.up('grid');
                         view.dirty = view.getValue() !== view.originalValue;
                         view.checkDirty();
                         //view.checkChange();
@@ -195,7 +195,7 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
     ],
 
     initComponent: function () {
-        var me = this;
+        let me = this;
 
         // static class store, thus gets not recreated, so ensure defaults are set!
         me.getStore().data.forEach(function (v) {
-- 
2.47.3



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

[pve-devel] [PATCH manager 7/7] ui: cpu flag selector: query CPU flag list via API

[Fiona Ebner]

There is coupling between the store and value for the top-level grid,
which has a field mixin. Becuase of this, the setValue() method needs
to update the 'state' value for the records in the store accordingly.
This can however only be done if the store is already loaded, so the
store gets a refresh() callback. It also remembers which value it was
already adjusted for for potential subsequent setValue() calls.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Happy to hear suggestions if this is too involved already! The grid
itself is a field (via mixin) and the 'state' values in the store are
coupled with the field value, so I couldn't figure out a simpler way
yet.

 www/manager6/form/VMCPUFlagSelector.js | 100 +++++++++++--------------
 1 file changed, 42 insertions(+), 58 deletions(-)

diff --git a/www/manager6/form/VMCPUFlagSelector.js b/www/manager6/form/VMCPUFlagSelector.js
index 2dbc21dc..1bd5a60f 100644
--- a/www/manager6/form/VMCPUFlagSelector.js
+++ b/www/manager6/form/VMCPUFlagSelector.js
@@ -18,63 +18,43 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
 
     store: {
         type: 'store',
-        fields: ['flag', { name: 'state', defaultValue: '=' }, 'desc'],
-        data: [
-            // FIXME: let qemu-server host this and autogenerate or get from API call??
-            {
-                flag: 'md-clear',
-                desc: 'Required to let the guest OS know if MDS is mitigated correctly',
-            },
-            {
-                flag: 'pcid',
-                desc: 'Meltdown fix cost reduction on Westmere, Sandy-, and IvyBridge Intel CPUs',
-            },
-            { flag: 'spec-ctrl', desc: 'Allows improved Spectre mitigation with Intel CPUs' },
-            { flag: 'ssbd', desc: 'Protection for "Speculative Store Bypass" for Intel models' },
-            { flag: 'ibpb', desc: 'Allows improved Spectre mitigation with AMD CPUs' },
-            {
-                flag: 'virt-ssbd',
-                desc: 'Basis for "Speculative Store Bypass" protection for AMD models',
-            },
-            {
-                flag: 'amd-ssbd',
-                desc: 'Improves Spectre mitigation performance with AMD CPUs, best used with "virt-ssbd"',
-            },
-            {
-                flag: 'amd-no-ssb',
-                desc: 'Notifies guest OS that host is not vulnerable for Spectre on AMD CPUs',
-            },
-            {
-                flag: 'pdpe1gb',
-                desc: 'Allow guest OS to use 1GB size pages, if host HW supports it',
-            },
-            {
-                flag: 'hv-tlbflush',
-                desc: 'Improve performance in overcommitted Windows guests. May lead to guest bluescreens on old CPUs.',
-            },
-            {
-                flag: 'hv-evmcs',
-                desc: 'Improve performance for nested virtualization. Only supported on Intel CPUs.',
-            },
-            { flag: 'aes', desc: 'Activate AES instruction set for HW acceleration.' },
-        ],
+        fields: ['name', { name: 'state', defaultValue: '=' }, 'description'],
+        autoLoad: true,
+        proxy: {
+            type: 'proxmox',
+            url: '/api2/json/nodes/localhost/capabilities/qemu/cpu-flags',
+        },
         listeners: {
             update: function () {
                 this.commitChanges();
             },
+            refresh: function (store, eOpts) {
+                let me = this;
+                let view = me.view;
+
+                if (store.adjustedForValue !== view.value) {
+                    view.adjustStoreForValue();
+                }
+            },
         },
+        adjustedForValue: undefined,
     },
 
     getValue: function () {
         let me = this;
         let store = me.getStore();
+
+        if (!store.isLoaded()) {
+            return me.value;
+        }
+
         let flags = '';
 
         // ExtJS does not has a nice getAllRecords interface for stores :/
         store.queryBy(Ext.returnTrue).each(function (rec) {
             let s = rec.get('state');
             if (s && s !== '=') {
-                let f = rec.get('flag');
+                let f = rec.get('name');
                 if (flags === '') {
                     flags = s + f;
                 } else {
@@ -88,33 +68,41 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
         return flags;
     },
 
-    setValue: function (value) {
+    // Adjusts the store for the current value and determines the unkown flags based on what the
+    // store does not know.
+    adjustStoreForValue: function () {
         let me = this;
         let store = me.getStore();
-
-        me.value = value || '';
+        let value = me.value;
 
         me.unkownFlags = [];
 
-        me.getStore()
-            .queryBy(Ext.returnTrue)
-            .each(function (rec) {
-                rec.set('state', '=');
-            });
+        store.queryBy(Ext.returnTrue).each((rec) => rec.set('state', '='));
 
         let flags = value ? value.split(';') : [];
         flags.forEach(function (flag) {
             let sign = flag.substr(0, 1);
             flag = flag.substr(1);
 
-            let rec = store.findRecord('flag', flag, 0, false, true, true);
+            let rec = store.findRecord('name', flag, 0, false, true, true);
             if (rec !== null) {
                 rec.set('state', sign);
             } else {
                 me.unkownFlags.push(flag);
             }
         });
-        store.reload();
+
+        store.adjustedForValue = value;
+    },
+
+    setValue: function (value) {
+        let me = this;
+
+        me.value = value || '';
+
+        if (me.getStore().isLoaded()) {
+            me.adjustForValue();
+        } // if not yet loaded, the store will trigger the function
 
         let res = me.mixins.field.setValue.call(me, value);
 
@@ -184,11 +172,11 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
             },
         },
         {
-            dataIndex: 'flag',
+            dataIndex: 'name',
             width: 100,
         },
         {
-            dataIndex: 'desc',
+            dataIndex: 'description',
             cellWrap: true,
             flex: 1,
         },
@@ -197,12 +185,8 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
     initComponent: function () {
         let me = this;
 
-        // static class store, thus gets not recreated, so ensure defaults are set!
-        me.getStore().data.forEach(function (v) {
-            v.state = '=';
-        });
-
         me.value = me.originalValue = '';
+        me.store.view = me;
 
         me.callParent(arguments);
     },
-- 
2.47.3



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

Re: [pve-devel] [PATCH manager 7/7] ui: cpu flag selector: query CPU flag list via API

[Daniel Kral]

On Fri Oct 31, 2025 at 1:27 PM CET, Fiona Ebner wrote:
> There is coupling between the store and value for the top-level grid,
> which has a field mixin. Becuase of this, the setValue() method needs
> to update the 'state' value for the records in the store accordingly.
> This can however only be done if the store is already loaded, so the
> store gets a refresh() callback. It also remembers which value it was
> already adjusted for for potential subsequent setValue() calls.
>
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>
> Happy to hear suggestions if this is too involved already! The grid
> itself is a field (via mixin) and the 'state' values in the store are
> coupled with the field value, so I couldn't figure out a simpler way
> yet.

-->8 snip 8<---

>      getValue: function () {
>          let me = this;
>          let store = me.getStore();
> +
> +        if (!store.isLoaded()) {
> +            return me.value;
> +        }
> +
>          let flags = '';
>  
>          // ExtJS does not has a nice getAllRecords interface for stores :/
>          store.queryBy(Ext.returnTrue).each(function (rec) {

unrelated to the patch, but shouldn't `store.getData()` return all the
records? `store.getData().getSource()` would be the equivalent, but as
the store for cpu capabilities isn't filtered right now, the former
should work.

>              let s = rec.get('state');
>              if (s && s !== '=') {
> -                let f = rec.get('flag');
> +                let f = rec.get('name');
>                  if (flags === '') {
>                      flags = s + f;
>                  } else {
> @@ -88,33 +68,41 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
>          return flags;
>      },
>  
> -    setValue: function (value) {
> +    // Adjusts the store for the current value and determines the unkown flags based on what the
> +    // store does not know.
> +    adjustStoreForValue: function () {
>          let me = this;
>          let store = me.getStore();
> -
> -        me.value = value || '';
> +        let value = me.value;
>  
>          me.unkownFlags = [];
>  
> -        me.getStore()
> -            .queryBy(Ext.returnTrue)
> -            .each(function (rec) {
> -                rec.set('state', '=');
> -            });
> +        store.queryBy(Ext.returnTrue).each((rec) => rec.set('state', '='));

same here

>  
>          let flags = value ? value.split(';') : [];
>          flags.forEach(function (flag) {
>              let sign = flag.substr(0, 1);
>              flag = flag.substr(1);
>  
> -            let rec = store.findRecord('flag', flag, 0, false, true, true);
> +            let rec = store.findRecord('name', flag, 0, false, true, true);
>              if (rec !== null) {
>                  rec.set('state', sign);
>              } else {
>                  me.unkownFlags.push(flag);
>              }
>          });
> -        store.reload();
> +
> +        store.adjustedForValue = value;
> +    },
> +
> +    setValue: function (value) {
> +        let me = this;
> +
> +        me.value = value || '';
> +
> +        if (me.getStore().isLoaded()) {
> +            me.adjustForValue();

Shouldn't this be `me.adjustStoreForValue();`? :)

I haven't yet found a way to trigger this though - neither in the Create
wizard nor the CPU modal with and without values - but maybe my
debugging strategy here is a bit wrong. How did you trigger it?

> +        } // if not yet loaded, the store will trigger the function
>  
>          let res = me.mixins.field.setValue.call(me, value);
>  
> @@ -184,11 +172,11 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
>              },
>          },
>          {
> -            dataIndex: 'flag',
> +            dataIndex: 'name',
>              width: 100,
>          },
>          {
> -            dataIndex: 'desc',
> +            dataIndex: 'description',
>              cellWrap: true,
>              flex: 1,
>          },
> @@ -197,12 +185,8 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
>      initComponent: function () {
>          let me = this;
>  
> -        // static class store, thus gets not recreated, so ensure defaults are set!
> -        me.getStore().data.forEach(function (v) {
> -            v.state = '=';
> -        });
> -
>          me.value = me.originalValue = '';
> +        me.store.view = me;
>  
>          me.callParent(arguments);
>      },



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

Re: [pve-devel] [PATCH qemu-server 4/7] cpu config: introduce vendor-agnostic 'nested-virt' CPU flag

[Daniel Kral]

On Fri Oct 31, 2025 at 1:27 PM CET, Fiona Ebner wrote:
> @@ -578,8 +588,34 @@ sub print_cpu_device {
>  sub resolve_cpu_flags {
>      my $flags = {};
>  
> +    my $nested_flag;
> +    my $nested_flag_resolved;
> +    my $resolve_nested_flag = sub {
> +        if (!$nested_flag_resolved) {
> +            my $host_cpu_flags = PVE::ProcFSTools::read_cpuinfo()->{flags};
> +            if ($host_cpu_flags =~ m/\s(svm|vmx)\s/) {
> +                $nested_flag = $1;
> +            } else {
> +                log_warn("ignoring 'nested-virt' CPU flag - unable to resolve from host CPU flags");
> +            }
> +            $nested_flag_resolved = 1;
> +        }
> +        return $nested_flag;
> +    };
> +
>      for my $hash (@_) {

nit: maybe now's a good time to give @_ a name with the $nested_flag bit
     in between? But I can see the naming collision with $flags..

>          for my $flag_name (keys %$hash) {
> +            if ($flag_name eq 'nested-virt') {
> +                my $nested_flag_name = $resolve_nested_flag->() or next;
> +                if ($hash->{$nested_flag_name}) {
> +                    warn "warning: CPU flag '$flag_name' overrides '$nested_flag_name'\n";
> +                } else {
> +                    print "CPU flag '$flag_name' resolved to '$nested_flag_name'\n";
> +                }
> +                $hash->{$nested_flag_name} = delete($hash->{$flag_name});
> +                $flag_name = $nested_flag_name;
> +            }
> +
>              my $flag = $hash->{$flag_name};
>              my $old_flag = $flags->{$flag_name};
>  



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

Re: [pve-devel] [PATCH-SERIES qemu-server/manager 0/7] VM CPU flags: introduce vendor-agnostic 'nested-virt' CPU flag

[Daniel Kral]

On Fri Oct 31, 2025 at 1:27 PM CET, Fiona Ebner wrote:
> The flag will automatically resolve to the flag required for the
> current CPU on the host. The 'nested-virt' flag takes precendence over
> the CPU-specific flag for nesting which might already be present with
> custom CPU models. In that case, a warning is printed.
>
> Moves the cpu flag descriptions to qemu-server and introduces an API
> endpoint for querying. Could later be extended with further
> information, for example, which flags are actually supported on the
> current host (some are specific to vendor or CPU).
>
> Dependency and build-dependency bump pve-manger -> qemu-server needed!

Works great for what I have tested on an Intel CPU:

- setting both `-nested-virt` and `+nested-virt` results in:

    CPU flag 'nested-virt' resolved to 'vmx'

but only the latter will make `lscpu | grep vmx` true. I think it's
valueable to also show it in the former case `-nested-virt`.


- setting e.g. `-vmx` in a custom model and `+nested-virt` in the VM's
  cpu flags will result in:

    CPU flag 'nested-virt' resolved to 'vmx'
    warning: CPU flag/setting '+vmx' (manually set for VM) overwrites
    '-vmx' (set by custom CPU model)

- but setting `-vmx` in the custom model and `-nested-virt` in the VM's
  cpu flags will result in only:

    CPU flag 'nested-virt' resolved to 'vmx'

The same happens vice versa with `+vmx` in the custom model and
`{+,-}nested-virt` in the VM's cpu flags.


From what I can figure this should also work with live migration,
because resolve_cpu_flags will be called at vm_start, right?


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

Re: [pve-devel] [PATCH qemu-server 3/7] api: add endpoint for querying available cpu flags

[Thomas Lamprecht]

Am 31.10.25 um 13:28 schrieb Fiona Ebner:
> Descriptions and ordering are taken from pve-manager's
> VMCPUFlagSelector.js. The double quotes in the descriptions were
> replaced with single quotes to have nicer JSON output.
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>  src/PVE/API2/Qemu/CPUFlags.pm   | 45 +++++++++++++++++++
>  src/PVE/API2/Qemu/Makefile      |  2 +-
>  src/PVE/QemuServer/CPUConfig.pm | 76 +++++++++++++++++++++++++--------
>  3 files changed, 105 insertions(+), 18 deletions(-)
>  create mode 100644 src/PVE/API2/Qemu/CPUFlags.pm
> 
> diff --git a/src/PVE/API2/Qemu/CPUFlags.pm b/src/PVE/API2/Qemu/CPUFlags.pm
> new file mode 100644
> index 00000000..cc06a1d6
> --- /dev/null
> +++ b/src/PVE/API2/Qemu/CPUFlags.pm
> @@ -0,0 +1,45 @@

> +__PACKAGE__->register_method({

> +        links => [{ rel => 'child', href => '{flag}' }],

We do not have any routes below this, is there any benefit of
declaring the links upfront? OTOH, there's also no real downside
either IIRC so probably can stay.

> diff --git a/src/PVE/QemuServer/CPUConfig.pm b/src/PVE/QemuServer/CPUConfig.pm
> index e72bdf2f..20e26ee2 100644
> --- a/src/PVE/QemuServer/CPUConfig.pm
> +++ b/src/PVE/QemuServer/CPUConfig.pm
> @@ -161,21 +161,62 @@ my $cpu_vendor_list = {
>      max => 'default',
>  };
>  
> -my @supported_cpu_flags = (
> -    'pcid',
> -    'spec-ctrl',
> -    'ibpb',
> -    'ssbd',
> -    'virt-ssbd',
> -    'amd-ssbd',
> -    'amd-no-ssb',
> -    'pdpe1gb',
> -    'md-clear',
> -    'hv-tlbflush',
> -    'hv-evmcs',
> -    'aes',
> -);
> -my $cpu_flag_supported_re = qr/([+-])(@{[join('|', @supported_cpu_flags)]})/;
> +our $supported_cpu_flags = [

I'd slightly prefer keeping this private to the module here and instead add a
getter method. Doing so reduces the impact for side-effect when some usage side
would modify this array-ref by accident.




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

[pve-devel] applied: [PATCH qemu-server 1/7] cpu config: style fix: avoid multiline post-if expressions

[Thomas Lamprecht]

On Fri, 31 Oct 2025 13:27:37 +0100, Fiona Ebner wrote:
> Should improve readability. No functional change intended.
> 
> 

Applied this one already, thanks!

[1/7] cpu config: style fix: avoid multiline post-if expressions
      commit: bc2f44bb6cdfa5d21cbf92d36b9399547717f819


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

[pve-devel] applied: [PATCH qemu-server 2/7] cpu config: style fix: avoid overly long ternary conditional expression

[Thomas Lamprecht]

On Fri, 31 Oct 2025 13:27:38 +0100, Fiona Ebner wrote:
> 


Applied this one already, thanks!

[2/7] cpu config: style fix: avoid overly long ternary conditional expression
      commit: 4879276a55990cf07a935f1da61507e24840b49a


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

Re: [pve-devel] [PATCH qemu-server 3/7] api: add endpoint for querying available cpu flags

[Thomas Lamprecht]

Am 06.11.25 um 19:02 schrieb Thomas Lamprecht:
> Am 31.10.25 um 13:28 schrieb Fiona Ebner:

>> -my $cpu_flag_supported_re = qr/([+-])(@{[join('|', @supported_cpu_flags)]})/;
>> +our $supported_cpu_flags = [
> 
> I'd slightly prefer keeping this private to the module here and instead add a
> getter method. Doing so reduces the impact for side-effect when some usage side
> would modify this array-ref by accident.

Correction: only really true if we actually create a new array clone on
returning, but even if we - whyever - wouldn't want, a getter method is IMO
still slightly nicer.


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