[pve-devel] [PATCH-SERIES qemu/qemu-server/manager 0/1] add and set x86-64-v2 as default model for new vms

[pve-devel] [PATCH-SERIES qemu/qemu-server/manager 0/1] add and set x86-64-v2 as default model for new vms

[Alexandre Derumier]

Hi,

we used kvm64 as default cpumodel since the begin of proxmox. (basically, it's like a pentium4 cpu flags).

New distros like rhel9 are compiled to use more modern cpu flags.
(and windows already use new flags since year, and we already add some extra cpu flags)

"
In 2020, AMD, Intel, Red Hat, and SUSE worked together to define
three microarchitecture levels on top of the historical x86-64
baseline:

  * x86-64:    original x86_64 baseline instruction set
  * x86-64-v2: vector instructions up to Streaming SIMD
               Extensions 4.2 (SSE4.2)  and Supplemental
               Streaming SIMD Extensions 3 (SSSE3), the
               POPCNT instruction, and CMPXCHG16B
  * x86-64-v3: vector instructions up to AVX2, MOVBE,
               and additional bit-manipulation instructions.
  * x86-64-v4: vector instructions from some of the
               AVX-512 variants.

"


(x86-64 is kvm64/qemu64 cpu model).

This patch series add new models (patch was found on qemu mailing, but never appplied).

"x86-64-abiX" : (as vX are reserved in qemu for revision versioning)

https://lore.kernel.org/all/20210526144038.278899-1-berrange@redhat.com/T/


In addition to theses model, I have enabled aes too.
I think it's really important, because a lot of users use default values and have
bad performance with ssl and other crypto stuffs.


This was discussed on the qemu mailing

"
Crypto accelerator caveats
==========================

Similarly I'm not a huge fan of leaving out the "aes"
instruction for accelerated crypto, as missing "aes" is
also one of the key factors in making qemu64 a bad choice.

If we include 'aes' in x86-64-abi2, then we loose support
for Nehalem hosts.

If we include 'aes' in x86-64-abi3 then we further loose
support for Dhyana hosts (an EPYC derived CPU).
"

Nahelemn is a 2008 cpu, so I think it's ok, we are in 2013 ;)
(and user can disable aes flag in gui too)

That mean than the minimum compatible cpu for v2 is Intel Westmere (2010)
and Amd Bulldozer (2011).


This patch series add new models, and set x86-64-abi2 model as default in pve-manager
wizard only.
(to not break current vm, where kvm64 is the default when cputype is not defined
in configuration)

The patch serie is for proxmox8/qemu8.
(qemu patch need to be reordered, I'm waiting than Fiona qemu 8.0 patches are applied first)


pve-qemu:

Alexandre Derumier (1):
  patch: add 0001-add-cpu-models-x86-64-abi.patch

 .../pve/0001-add-cpu-models-x86-64-abi.patch  | 272 ++++++++++++++++++
 debian/patches/series                         |   1 +
 2 files changed, 273 insertions(+)
 create mode 100644 debian/patches/pve/0001-add-cpu-models-x86-64-abi.patch

qemu-server:

Alexandre Derumier (1):
  cpuconfig: add new x86-64-abi models

 PVE/QemuServer/CPUConfig.pm | 4 ++++
 1 file changed, 4 insertions(+)

pve-manager:

Alexandre Derumier (1):
  qemu: processor : set x86-64-abi2 as default cputype for create wizard

 www/manager6/qemu/OSDefaults.js    |  1 +
 www/manager6/qemu/ProcessorEdit.js | 13 +++++++++++++
 2 files changed, 14 insertions(+)

-- 
2.30.2

[pve-devel] [PATCH qemu-server 1/1] cpuconfig: add new x86-64-abi models

[Alexandre Derumier]

Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
---
 PVE/QemuServer/CPUConfig.pm | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/PVE/QemuServer/CPUConfig.pm b/PVE/QemuServer/CPUConfig.pm
index fb0861b..68364d1 100644
--- a/PVE/QemuServer/CPUConfig.pm
+++ b/PVE/QemuServer/CPUConfig.pm
@@ -96,6 +96,10 @@ my $cpu_vendor_list = {
     kvm64 => 'default',
     qemu32 => 'default',
     qemu64 => 'default',
+    'x86-64-abi1' => 'default',
+    'x86-64-abi2' => 'default',
+    'x86-64-abi3' => 'default',
+    'x86-64-abi4' => 'default',
     max => 'default',
 };
 
-- 
2.30.2

[pve-devel] [PATCH pve-qemu 1/1] patch: add 0001-add-cpu-models-x86-64-abi.patch

[Alexandre Derumier]

Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
---
 .../pve/0001-add-cpu-models-x86-64-abi.patch  | 272 ++++++++++++++++++
 debian/patches/series                         |   1 +
 2 files changed, 273 insertions(+)
 create mode 100644 debian/patches/pve/0001-add-cpu-models-x86-64-abi.patch

diff --git a/debian/patches/pve/0001-add-cpu-models-x86-64-abi.patch b/debian/patches/pve/0001-add-cpu-models-x86-64-abi.patch
new file mode 100644
index 0000000..e98f862
--- /dev/null
+++ b/debian/patches/pve/0001-add-cpu-models-x86-64-abi.patch
@@ -0,0 +1,272 @@
+From 7d9ff48a96a7613a15e5427bc8987358e5529e45 Mon Sep 17 00:00:00 2001
+From: Alexandre Derumier <aderumier@odiso.com>
+Date: Tue, 16 May 2023 11:36:19 +0200
+Subject: [PATCH] target/i386: define CPU models to model x86-64 ABI levels
+
+https://lists.gnu.org/archive/html/qemu-devel/2021-05/msg07807.html
+
+To paraphrase:
+  
+https://developers.redhat.com/blog/2021/01/05/building-red-hat-enterprise-linux-9-for-the-x86-64-v2-microarchitecture-level/
+
+In 2020, AMD, Intel, Red Hat, and SUSE worked together to define
+three microarchitecture levels on top of the historical x86-64
+baseline:
+
+  * x86-64:    original x86_64 baseline instruction set
+  * x86-64-v2: vector instructions up to Streaming SIMD
+               Extensions 4.2 (SSE4.2)  and Supplemental
+               Streaming SIMD Extensions 3 (SSSE3), the
+               POPCNT instruction, and CMPXCHG16B
+  * x86-64-v3: vector instructions up to AVX2, MOVBE,
+               and additional bit-manipulation instructions.
+  * x86-64-v4: vector instructions from some of the
+               AVX-512 variants.
+
+This list of features is defined in the doc at:
+
+  https://gitlab.com/x86-psABIs/x86-64-ABI/
+
+QEMU has historically defaulted to the "qemu64" CPU model on
+x86_64 targets, which is approximately the x86-64 baseline
+ABI, with 'SVM' added.
+
+It is thought it might be desirable if QEMU could provide CPU models
+that closely correspond to the ABI levels, while offering portability
+across the maximum number of physical CPUs.
+
+Historically we've found that defining CPU models with an arbitrary
+combination of CPU features can be problematic, as some guest OS
+will not check all features they use, and instead assume that if
+they see feature "XX", then "YY" will always exist. This is reasonable
+in bare metal, but subject to breakage in virtualization.
+
+Thus in defining the CPU models for the ABI levels, this patch attempted
+to base them off an existing CPU model.
+
+While each x86-64-abiNNN has a designated vendor, they are designed
+to be vendor agnostic models. ie they are capable of running on any
+physical x86_64 CPU model that satisfies the ABI level. eg although
+the x86-64-abi2 model is based on Nehalem, it should be able to run
+guests on an Opteron_G4/G5/EPYC host, since those CPUs support the
+features required by the x86-64 v2 ABI.
+
+More precisely the models were defined as:
+
+ * x86-64-abi1: close match for Opteron_G1, minus
+                vme
+ * x86-64-abi2: perfect match for Nehalem
+ * x86-64-abi3: close match of Haswell-noTSX, minus
+                aes pcid erms invpcid tsc-deadline
+                x2apic pclmulqdq
+ * x86-64-abi4: close match of Skylake-Server-noTSX-IBRS, minus
+                spec-ctrl
+
+None of the CPU models declare any VMX/SVM capability features.
+IOW, even if a "vmx"/"svm" flag is added, it will still be unsafe
+to attempt to live migrate the L1 guest if there are any L2
+guests running with hardware virtualization.
+
+Given their vendor agnostic design, these CPU models are primarily
+thought tobe  useful as the default CPU model for machine types.
+QEMU upstream is quite conservative in mandating new hardware features,
+but a downstream vendor may choose to mandate a newer x86-64 ABI level
+for downstream only machine types.
+
+Note that TCG is not capable of supporting the 2 newest ABI levels
+currently due to missing features:
+
+* x86-64-abi3:
+
+  CPUID.01H:ECX.fma [bit 12]
+  CPUID.01H:ECX.avx [bit 28]
+  CPUID.01H:ECX.f16c [bit 29]
+  CPUID.07H:EBX.avx2 [bit 5]
+
+* x86-64-abi4:
+
+  CPUID.01H:ECX.pcid [bit 17]
+  CPUID.01H:ECX.x2apic [bit 21]
+  CPUID.01H:ECX.tsc-deadline [bit 24]
+  CPUID.07H:EBX.invpcid [bit 10]
+  CPUID.07H:EBX.avx512f [bit 16]
+  CPUID.07H:EBX.avx512dq [bit 17]
+  CPUID.07H:EBX.rdseed [bit 18]
+  CPUID.07H:EBX.avx512cd [bit 28]
+  CPUID.07H:EBX.avx512bw [bit 30]
+  CPUID.07H:EBX.avx512vl [bit 31]
+  CPUID.80000001H:ECX.3dnowprefetch [bit 8]
+  CPUID.0DH:EAX.xsavec [bit 1]
+
+---
+ target/i386/cpu.c | 162 ++++++++++++++++++++++++++++++++++++++++++++++
+ 1 file changed, 162 insertions(+)
+
+diff --git a/target/i386/cpu.c b/target/i386/cpu.c
+index 22b681ca3..294c20c53 100644
+--- a/target/i386/cpu.c
++++ b/target/i386/cpu.c
+@@ -1853,6 +1861,160 @@ static const X86CPUDefinition builtin_x86_defs[] = {
+         .xlevel = 0x8000000A,
+         .model_id = "QEMU Virtual CPU version " QEMU_HW_VERSION,
+     },
++    /*
++     * These first few CPU models are designed to satisfy the
++     * x86_64 ABI levels defined in:
++     *
++     *   https://gitlab.com/x86-psABIs/x86-64-ABI/
++     *
++     * They were constructed as follows:
++     *
++     *   - Find all the CPU models which can satisfy the ABI
++     *   - Calculate the lowest common denominator (LCD) of these
++     *     models' features
++     *   - Find the named model most closely matching the LCD
++     *   - Strip its features back to the LCD
++     *
++     * The above spec uses the "x86-64-vNN" naming convention.
++     * This clashes with the "vNN" suffix QEMU uses for versioning.
++     * Thus we use "abiNNN" as a suffix.
++     */
++    {
++        /*
++         * Derived from Opteron_G1, minus
++         *   vme
++         */
++        .name = "x86-64-abi1",
++        .level = 5,
++        .vendor = CPUID_VENDOR_AMD,
++        .family = 15,
++        .model = 6,
++        .stepping = 1,
++        .features[FEAT_1_EDX] =
++            CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
++            CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
++            CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
++            CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
++            CPUID_DE | CPUID_FP87,
++        .features[FEAT_1_ECX] =
++            CPUID_EXT_SSE3,
++        .features[FEAT_8000_0001_EDX] =
++            CPUID_EXT2_LM | CPUID_EXT2_NX | CPUID_EXT2_SYSCALL,
++        .xlevel = 0x80000008,
++        .model_id = "QEMU x86-64 baseline ABI",
++    },
++    {
++        /* Derived from Nehalem */
++        .name = "x86-64-abi2",
++        .level = 11,
++        .vendor = CPUID_VENDOR_INTEL,
++        .family = 6,
++        .model = 26,
++        .stepping = 3,
++        .features[FEAT_1_EDX] =
++            CPUID_VME | CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
++            CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
++            CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
++            CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
++            CPUID_DE | CPUID_FP87,
++        .features[FEAT_1_ECX] =
++            CPUID_EXT_POPCNT | CPUID_EXT_SSE42 | CPUID_EXT_SSE41 | CPUID_EXT_AES |
++            CPUID_EXT_CX16 | CPUID_EXT_SSSE3 | CPUID_EXT_SSE3,
++        .features[FEAT_8000_0001_EDX] =
++            CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
++        .features[FEAT_8000_0001_ECX] =
++            CPUID_EXT3_LAHF_LM,
++        .xlevel = 0x80000008,
++        .model_id = "QEMU x86-64-v2 ABI",
++    },
++    {
++        /*
++         * Derived from Haswell-noTSX, minus
++         *   pcid erms invpcid tsc-deadline x2apic pclmulqdq
++         */
++        .name = "x86-64-abi3",
++        .level = 0xd,
++        .vendor = CPUID_VENDOR_INTEL,
++        .family = 6,
++        .model = 60,
++        .stepping = 1,
++        .features[FEAT_1_EDX] =
++            CPUID_VME | CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
++            CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
++            CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
++            CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
++            CPUID_DE | CPUID_FP87,
++        .features[FEAT_1_ECX] =
++            CPUID_EXT_AVX | CPUID_EXT_XSAVE | CPUID_EXT_AES |
++            CPUID_EXT_POPCNT | CPUID_EXT_SSE42 |
++            CPUID_EXT_SSE41 | CPUID_EXT_CX16 | CPUID_EXT_SSSE3 |
++            CPUID_EXT_SSE3 |
++            CPUID_EXT_FMA | CPUID_EXT_MOVBE |
++            CPUID_EXT_F16C | CPUID_EXT_RDRAND,
++        .features[FEAT_8000_0001_EDX] =
++            CPUID_EXT2_LM | CPUID_EXT2_RDTSCP | CPUID_EXT2_NX |
++            CPUID_EXT2_SYSCALL,
++        .features[FEAT_8000_0001_ECX] =
++            CPUID_EXT3_ABM | CPUID_EXT3_LAHF_LM,
++        .features[FEAT_7_0_EBX] =
++            CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 |
++            CPUID_7_0_EBX_AVX2 | CPUID_7_0_EBX_SMEP |
++            CPUID_7_0_EBX_BMI2,
++        .features[FEAT_XSAVE] =
++            CPUID_XSAVE_XSAVEOPT,
++        .features[FEAT_6_EAX] =
++            CPUID_6_EAX_ARAT,
++        .xlevel = 0x80000008,
++        .model_id = "QEMU x86-64-v3 ABI",
++    },
++    {
++        /*
++         * Derived from Skylake-Server-noTSX-IBRS, minus:
++         *  spec-ctrl
++         */
++        .name = "x86-64-abi4",
++        .level = 0xd,
++        .vendor = CPUID_VENDOR_INTEL,
++        .family = 6,
++        .model = 85,
++        .stepping = 4,
++        .features[FEAT_1_EDX] =
++            CPUID_VME | CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
++            CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
++            CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
++            CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
++            CPUID_DE | CPUID_FP87,
++        .features[FEAT_1_ECX] =
++            CPUID_EXT_AVX | CPUID_EXT_XSAVE | CPUID_EXT_AES |
++            CPUID_EXT_POPCNT | CPUID_EXT_X2APIC | CPUID_EXT_SSE42 |
++            CPUID_EXT_SSE41 | CPUID_EXT_CX16 | CPUID_EXT_SSSE3 |
++            CPUID_EXT_PCLMULQDQ | CPUID_EXT_SSE3 |
++            CPUID_EXT_TSC_DEADLINE_TIMER | CPUID_EXT_FMA | CPUID_EXT_MOVBE |
++            CPUID_EXT_PCID | CPUID_EXT_F16C | CPUID_EXT_RDRAND,
++        .features[FEAT_8000_0001_EDX] =
++            CPUID_EXT2_LM | CPUID_EXT2_PDPE1GB | CPUID_EXT2_RDTSCP |
++            CPUID_EXT2_NX | CPUID_EXT2_SYSCALL,
++        .features[FEAT_8000_0001_ECX] =
++            CPUID_EXT3_ABM | CPUID_EXT3_LAHF_LM | CPUID_EXT3_3DNOWPREFETCH,
++        .features[FEAT_7_0_EBX] =
++            CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 |
++            CPUID_7_0_EBX_AVX2 | CPUID_7_0_EBX_SMEP |
++            CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ERMS | CPUID_7_0_EBX_INVPCID |
++            CPUID_7_0_EBX_RDSEED | CPUID_7_0_EBX_ADX |
++            CPUID_7_0_EBX_SMAP | CPUID_7_0_EBX_CLWB |
++            CPUID_7_0_EBX_AVX512F | CPUID_7_0_EBX_AVX512DQ |
++            CPUID_7_0_EBX_AVX512BW | CPUID_7_0_EBX_AVX512CD |
++            CPUID_7_0_EBX_AVX512VL,
++        .features[FEAT_7_0_ECX] =
++            CPUID_7_0_ECX_PKU,
++        .features[FEAT_XSAVE] =
++            CPUID_XSAVE_XSAVEOPT | CPUID_XSAVE_XSAVEC |
++            CPUID_XSAVE_XGETBV1,
++        .features[FEAT_6_EAX] =
++            CPUID_6_EAX_ARAT,
++        .xlevel = 0x80000008,
++        .model_id = "QEMU x86-64-v4 ABI",
++    },
+     {
+         .name = "phenom",
+         .level = 5,
+-- 
+2.30.2
+
diff --git a/debian/patches/series b/debian/patches/series
index 4e8ddd6..5273fab 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -94,3 +94,4 @@ pve/0059-vma-create-support-64KiB-unaligned-input-images.patch
 pve/0060-vma-create-avoid-triggering-assertion-in-error-case.patch
 pve/0061-block-alloc-track-avoid-premature-break.patch
 pve/0062-PVE-Backup-allow-passing-max-workers-performance-set.patch
+pve/0001-add-cpu-models-x86-64-abi.patch
\ No newline at end of file
-- 
2.30.2

[pve-devel] [PATCH pve-manager 1/1] qemu: processor : set x86-64-abi2 as default cputype for create wizard

[Alexandre Derumier]

Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
---
 www/manager6/qemu/OSDefaults.js    |  1 +
 www/manager6/qemu/ProcessorEdit.js | 13 +++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/www/manager6/qemu/OSDefaults.js b/www/manager6/qemu/OSDefaults.js
index 5e588a58..889e63ca 100644
--- a/www/manager6/qemu/OSDefaults.js
+++ b/www/manager6/qemu/OSDefaults.js
@@ -43,6 +43,7 @@ Ext.define('PVE.qemu.OSDefaults', {
 		    virtio: 1,
 	    },
 	    scsihw: 'virtio-scsi-single',
+	    cputype: 'x86-64-abi2',
 	};
 
        // virtio-net is in kernel since 2.6.25
diff --git a/www/manager6/qemu/ProcessorEdit.js b/www/manager6/qemu/ProcessorEdit.js
index b845ff66..727d1679 100644
--- a/www/manager6/qemu/ProcessorEdit.js
+++ b/www/manager6/qemu/ProcessorEdit.js
@@ -29,6 +29,19 @@ Ext.define('PVE.qemu.ProcessorInputPanel', {
 
 	    viewModel.set('userIsRoot', Proxmox.UserName === 'root@pam');
 	},
+	control: {
+	    '#': {
+		afterrender: 'setCputype',
+	    },
+	},
+	setCputype: function() {
+	    let me = this;
+	    let view = me.getView();
+	    let cputype = view.down('CPUModelSelector[name=cputype]');
+	    if (view.insideWizard) {
+		cputype.setValue(PVE.qemu.OSDefaults.generic.cputype);
+	    }
+        },
     },
 
     onGetValues: function(values) {
-- 
2.30.2

Re: [pve-devel] [PATCH-SERIES qemu/qemu-server/manager 0/1] add and set x86-64-v2 as default model for new vms

[Fabian Grünbichler]

On May 17, 2023 9:02 am, Alexandre Derumier wrote:
> Hi,
> 
> we used kvm64 as default cpumodel since the begin of proxmox. (basically, it's like a pentium4 cpu flags).
> 
> New distros like rhel9 are compiled to use more modern cpu flags.
> (and windows already use new flags since year, and we already add some extra cpu flags)
> 
> "
> In 2020, AMD, Intel, Red Hat, and SUSE worked together to define
> three microarchitecture levels on top of the historical x86-64
> baseline:
> 
>   * x86-64:    original x86_64 baseline instruction set
>   * x86-64-v2: vector instructions up to Streaming SIMD
>                Extensions 4.2 (SSE4.2)  and Supplemental
>                Streaming SIMD Extensions 3 (SSSE3), the
>                POPCNT instruction, and CMPXCHG16B
>   * x86-64-v3: vector instructions up to AVX2, MOVBE,
>                and additional bit-manipulation instructions.
>   * x86-64-v4: vector instructions from some of the
>                AVX-512 variants.
> 
> "
> 
> 
> (x86-64 is kvm64/qemu64 cpu model).
> 
> This patch series add new models (patch was found on qemu mailing, but never appplied).
> 
> "x86-64-abiX" : (as vX are reserved in qemu for revision versioning)
> 
> https://lore.kernel.org/all/20210526144038.278899-1-berrange@redhat.com/T/

FWIW, there was a v3: https://lists.gnu.org/archive/html/qemu-devel/2021-06/msg01591.html
(20210607135843.196595-1-berrange@redhat.com) that has some more
discussion/information (only the doc table + script to generate it have
been applied it seems).

Re: [pve-devel] [PATCH pve-qemu 1/1] patch: add 0001-add-cpu-models-x86-64-abi.patch

[Fiona Ebner]

Am 17.05.23 um 09:02 schrieb Alexandre Derumier:
> Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
> ---
>  .../pve/0001-add-cpu-models-x86-64-abi.patch  | 272 ++++++++++++++++++
>  debian/patches/series                         |   1 +
>  2 files changed, 273 insertions(+)
>  create mode 100644 debian/patches/pve/0001-add-cpu-models-x86-64-abi.patch
> 

Do we really need this? Can't we just define these as custom CPU models
in qemu-server? I'd really prefer to not pick up such patches if not
necessary. Ideally, we reduce divergence from upstream QEMU rather than
increase it. Since the patch was never applied, what other solution did
upstream or libvirt go for?

If we really do need this, we should rather ask upstream again if they
can apply it or why they didn't first. Because it's two years old, so if
there were good reasons, so without asking, it's unlikely that they will
apply it soon and then we'd have to carry it around forever ourselves...

> diff --git a/debian/patches/pve/0001-add-cpu-models-x86-64-abi.patch b/debian/patches/pve/0001-add-cpu-models-x86-64-abi.patch
> new file mode 100644
> index 0000000..e98f862
> --- /dev/null
> +++ b/debian/patches/pve/0001-add-cpu-models-x86-64-abi.patch
> @@ -0,0 +1,272 @@
> +From 7d9ff48a96a7613a15e5427bc8987358e5529e45 Mon Sep 17 00:00:00 2001
> +From: Alexandre Derumier <aderumier@odiso.com>

How did you apply the patch from the mail? Here should be the name of
the one how sent the mail, not yours.

> +
> +  CPUID.01H:ECX.pcid [bit 17]
> +  CPUID.01H:ECX.x2apic [bit 21]
> +  CPUID.01H:ECX.tsc-deadline [bit 24]
> +  CPUID.07H:EBX.invpcid [bit 10]
> +  CPUID.07H:EBX.avx512f [bit 16]
> +  CPUID.07H:EBX.avx512dq [bit 17]
> +  CPUID.07H:EBX.rdseed [bit 18]
> +  CPUID.07H:EBX.avx512cd [bit 28]
> +  CPUID.07H:EBX.avx512bw [bit 30]
> +  CPUID.07H:EBX.avx512vl [bit 31]
> +  CPUID.80000001H:ECX.3dnowprefetch [bit 8]
> +  CPUID.0DH:EAX.xsavec [bit 1]
> +

And here you lost the original

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

That is not okay.

If you modified the patch you should do:

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
[AD: describe your changes here]
Signed-off-by: Alexandre Derumier <aderumier@odiso.com>

With AD being your initials, but you can also use A or leave them out
completely. The important part is that the changes are mentioned ;)

> +---
> + target/i386/cpu.c | 162 ++++++++++++++++++++++++++++++++++++++++++++++
> + 1 file changed, 162 insertions(+)
> +

Re: [pve-devel] [PATCH pve-qemu 1/1] patch: add 0001-add-cpu-models-x86-64-abi.patch

[DERUMIER, Alexandre]

Le mercredi 17 mai 2023 à 09:46 +0200, Fiona Ebner a écrit :
> Am 17.05.23 um 09:02 schrieb Alexandre Derumier:
> > Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
> > ---
> >  .../pve/0001-add-cpu-models-x86-64-abi.patch  | 272
> > ++++++++++++++++++
> >  debian/patches/series                         |   1 +
> >  2 files changed, 273 insertions(+)
> >  create mode 100644 debian/patches/pve/0001-add-cpu-models-x86-64-
> > abi.patch
> > 
> 
> Do we really need this? Can't we just define these as custom CPU
> models
> in qemu-server? I'd really prefer to not pick up such patches if not
> necessary. Ideally, we reduce divergence from upstream QEMU rather
> than
> increase it. 
yes sure we can do it in qemu-server, with simply setting flags in
command line.

It was more by security to not forgot a cpu flag.


> Since the patch was never applied, what other solution did
> upstream or libvirt go for?
The end of the discussion was that it could be done userland
like qemu-server ;)


I'll rework the patch series.

(BTW, what do you think about bumping the default model ? is it ok for
you ?)




> 
> If we really do need this, we should rather ask upstream again if
> they
> can apply it or why they didn't first. Because it's two years old, so
> if
> there were good reasons, so without asking, it's unlikely that they
> will
> apply it soon and then we'd have to carry it around forever
> ourselves...
> 
> > diff --git a/debian/patches/pve/0001-add-cpu-models-x86-64-
> > abi.patch b/debian/patches/pve/0001-add-cpu-models-x86-64-abi.patch
> > new file mode 100644
> > index 0000000..e98f862
> > --- /dev/null
> > +++ b/debian/patches/pve/0001-add-cpu-models-x86-64-abi.patch
> > @@ -0,0 +1,272 @@
> > +From 7d9ff48a96a7613a15e5427bc8987358e5529e45 Mon Sep 17 00:00:00
> > 2001
> > +From: Alexandre Derumier <aderumier@odiso.com>
> 
> How did you apply the patch from the mail? Here should be the name of
> the one how sent the mail, not yours.
> 
> > +
> > +  CPUID.01H:ECX.pcid [bit 17]
> > +  CPUID.01H:ECX.x2apic [bit 21]
> > +  CPUID.01H:ECX.tsc-deadline [bit 24]
> > +  CPUID.07H:EBX.invpcid [bit 10]
> > +  CPUID.07H:EBX.avx512f [bit 16]
> > +  CPUID.07H:EBX.avx512dq [bit 17]
> > +  CPUID.07H:EBX.rdseed [bit 18]
> > +  CPUID.07H:EBX.avx512cd [bit 28]
> > +  CPUID.07H:EBX.avx512bw [bit 30]
> > +  CPUID.07H:EBX.avx512vl [bit 31]
> > +  CPUID.80000001H:ECX.3dnowprefetch [bit 8]
> > +  CPUID.0DH:EAX.xsavec [bit 1]
> > +
> 
> And here you lost the original
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> 
> That is not okay.
> 
> If you modified the patch you should do:
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> [AD: describe your changes here]
> Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
> 
> With AD being your initials, but you can also use A or leave them out
> completely. The important part is that the changes are mentioned ;)
> 
> > +---
> > + target/i386/cpu.c | 162
> > ++++++++++++++++++++++++++++++++++++++++++++++
> > + 1 file changed, 162 insertions(+)
> > +
> 

Re: [pve-devel] [PATCH pve-qemu 1/1] patch: add 0001-add-cpu-models-x86-64-abi.patch

[Fiona Ebner]

Am 17.05.23 um 10:25 schrieb DERUMIER, Alexandre:
> Le mercredi 17 mai 2023 à 09:46 +0200, Fiona Ebner a écrit :
>> Am 17.05.23 um 09:02 schrieb Alexandre Derumier:
>>> Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
>>> ---
>>>  .../pve/0001-add-cpu-models-x86-64-abi.patch  | 272
>>> ++++++++++++++++++
>>>  debian/patches/series                         |   1 +
>>>  2 files changed, 273 insertions(+)
>>>  create mode 100644 debian/patches/pve/0001-add-cpu-models-x86-64-
>>> abi.patch
>>>
>>
>> Do we really need this? Can't we just define these as custom CPU
>> models
>> in qemu-server? I'd really prefer to not pick up such patches if not
>> necessary. Ideally, we reduce divergence from upstream QEMU rather
>> than
>> increase it. 
> yes sure we can do it in qemu-server, with simply setting flags in
> command line.
> 
> It was more by security to not forgot a cpu flag.
> 
> 
>> Since the patch was never applied, what other solution did
>> upstream or libvirt go for?
> The end of the discussion was that it could be done userland
> like qemu-server ;)

Yes, let's just introduce a custom CPU model there, which will be the
new UI default.

> I'll rework the patch series.
> 
> (BTW, what do you think about bumping the default model ? is it ok for
> you ?)
> 

Yes, I do think it should be done, because most currently in-use CPUs
should be fine with x86-64-abi2 and it's not nice to users if the
defaults don't work out-of-the-box[0]. And other people can just select
kvm64. Changing the default in the UI for new VMs should not lead to any
breakage and your series does just that. Still, something we'll want to
mention in the PVE 8 release notes to not catch people off guard.

[0]:
https://forum.proxmox.com/threads/kernel-panic-when-creating-vms-centos-9-stream-iso.104656/

Re: [pve-devel] [PATCH-SERIES qemu/qemu-server/manager 0/1] add and set x86-64-v2 as default model for new vms

[DERUMIER, Alexandre]

> FWIW, there was a v3: 

Yes, this was last version.

The end of discussion was here:
https://lists.gnu.org/archive/html/qemu-devel/2021-06/msg02128.html


I wasn"t merged, because moslty, you can't have spectre/meltdown/...
mitigation enabled cross amd/intel, and also, generally users don't mix
intel/amd, so it's already possible to choose the lowest inte/amd cpu
model of the cluster.


Personnally, I think it could really be usefull for mixed intel/amd
cluster. (as kvm64 will not work with newer distros). It's better than
kvm64 anymay. (but no mitigation).


a better way could be to auto choose the best compatible cpu model
across cluster for user at vm creation.

with some kind of new virtual "best" cpumodel at vm creation only (then
the correct best compatible cpu model is compute and write in vm
configuration).

We could stream host cpumodel/revision/flags with pvestatd,
then compare flags to choose the correct model.
(mitigations autotuning could be great too)

all flags are documented in libvirt here:
https://gitlab.com/libvirt/libvirt/-/blob/master/src/cpu_map/


the simple algo is something like:
 - if nocluster/single node, set cpumodel to max/host
 - elsif cluster have only intel hosts, find best intel cpumodel
 - elsif cluster have only amd hosts, find best amd cpumodel
 - elsif cluster have mixed intel/amd hosts, find best x64-64-vX cpu
model

Re: [pve-devel] [PATCH pve-qemu 1/1] patch: add 0001-add-cpu-models-x86-64-abi.patch

[Thomas Lamprecht]

Am 17/05/2023 um 11:08 schrieb Fiona Ebner:
>> (BTW, what do you think about bumping the default model ? is it ok for
>> you ?)
>>
> Yes, I do think it should be done, because most currently in-use CPUs
> should be fine with x86-64-abi2 and it's not nice to users if the
> defaults don't work out-of-the-box[0]. And other people can just select
> kvm64. Changing the default in the UI for new VMs should not lead to any
> breakage and your series does just that. Still, something we'll want to
> mention in the PVE 8 release notes to not catch people off guard.

+1 from my side.