1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH 0/6] qemu: Fix "Property '.pconfig' not found" error
  4. View patch

[libvirt] [PATCH 6/6] cpu_map: Drop pconfig from Icelake-Server CPU model

Jiri Denemark posted 6 patches 5 years ago
[libvirt] [PATCH 6/6] cpu_map: Drop pconfig from Icelake-Server CPU model
Posted by Jiri Denemark 5 years ago 
The pconfig feature was enabled in QEMU by accident in 3.1.0. All other
newer versions do not support it and it was removed from the
Icelake-Server CPU model in QEMU.

We don't normally change our CPU models even when QEMU does so to avoid
breaking migrations between different versions of libvirt. But we can
safely do so in this specific case. QEMU never supported enabling
pconfig so any domain which was able to start has pconfig disabled.

With a small compatibility hack which explicitly disables pconfig when
CPU model equals Icelake-Server in migratable domain definition, only
one migration scenario stays broken (and there's nothing we can do about
it): from any host to a host with libvirt < 5.10.0 and QEMU > 3.1.0.

https://bugzilla.redhat.com/show_bug.cgi?id=1749672

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
 src/cpu_map/x86_Icelake-Server.xml            |  1 -
 src/qemu/qemu_domain.c                        | 23 +++++++++++++++++++
 src/qemu/qemu_domain.h                        |  3 +++
 src/qemu/qemu_migration_cookie.c              |  3 +++
 .../x86_64-cpuid-Ice-Lake-Server-guest.xml    |  1 -
 .../x86_64-cpuid-Ice-Lake-Server-host.xml     | 11 +--------
 .../x86_64-cpuid-Ice-Lake-Server-json.xml     |  1 -
 7 files changed, 30 insertions(+), 13 deletions(-)

diff --git a/src/cpu_map/x86_Icelake-Server.xml b/src/cpu_map/x86_Icelake-Server.xml
index ecd21cf5c7..a565371977 100644
--- a/src/cpu_map/x86_Icelake-Server.xml
+++ b/src/cpu_map/x86_Icelake-Server.xml
@@ -54,7 +54,6 @@
     <feature name='pat'/>
     <feature name='pcid'/>
     <feature name='pclmuldq'/>
-    <feature name='pconfig'/>
     <feature name='pdpe1gb'/>
     <feature name='pge'/>
     <feature name='pku'/>
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 58a82fbd60..4a641019be 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8921,6 +8921,26 @@ qemuDomainDefCopy(virQEMUDriverPtr driver,
 }
 
 
+int
+qemuDomainMakeCPUMigratable(virCPUDefPtr cpu)
+{
+    if (cpu->mode == VIR_CPU_MODE_CUSTOM &&
+        STREQ_NULLABLE(cpu->model, "Icelake-Server")) {
+        /* Originally Icelake-Server CPU model contained pconfig CPU feature.
+         * It was never actually enabled and thus it was removed. To enable
+         * migration to QEMU 3.1.0 (with both new and old libvirt), we
+         * explicitly disable pconfig in migration XML (otherwise old libvirt
+         * would think it was implicitly enabled on the source). New libvirt
+         * will drop it from the XML before starting the domain on new QEMU.
+         */
+        if (virCPUDefUpdateFeature(cpu, "pconfig", VIR_CPU_FEATURE_DISABLE) < 0)
+            return -1;
+    }
+
+    return 0;
+}
+
+
 static int
 qemuDomainDefFormatBufInternal(virQEMUDriverPtr driver,
                                virQEMUCapsPtr qemuCaps,
@@ -9103,6 +9123,9 @@ qemuDomainDefFormatBufInternal(virQEMUDriverPtr driver,
             if (!(def->cpu = virCPUDefCopy(origCPU)))
                 goto cleanup;
         }
+
+        if (qemuDomainMakeCPUMigratable(def->cpu) < 0)
+            goto cleanup;
     }
 
  format:
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index b23912ee98..a458af79f7 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -1225,3 +1225,6 @@ qemuDomainValidateActualNetDef(const virDomainNetDef *net,
 int
 qemuDomainSupportsCheckpointsBlockjobs(virDomainObjPtr vm)
     G_GNUC_WARN_UNUSED_RESULT;
+
+int
+qemuDomainMakeCPUMigratable(virCPUDefPtr cpu);
diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c
index 27e6bde601..5f64800aec 100644
--- a/src/qemu/qemu_migration_cookie.c
+++ b/src/qemu/qemu_migration_cookie.c
@@ -536,6 +536,9 @@ qemuMigrationCookieAddCPU(qemuMigrationCookiePtr mig,
     if (!(mig->cpu = virCPUDefCopy(vm->def->cpu)))
         return -1;
 
+    if (qemuDomainMakeCPUMigratable(mig->cpu) < 0)
+        return -1;
+
     mig->flags |= QEMU_MIGRATION_COOKIE_CPU;
 
     return 0;
diff --git a/tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-guest.xml b/tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-guest.xml
index 6ca2099b33..4676f3aa7d 100644
--- a/tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-guest.xml
+++ b/tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-guest.xml
@@ -32,5 +32,4 @@
   <feature policy='require' name='rdctl-no'/>
   <feature policy='require' name='ibrs-all'/>
   <feature policy='require' name='skip-l1dfl-vmentry'/>
-  <feature policy='disable' name='pconfig'/>
 </cpu>
diff --git a/tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-host.xml b/tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-host.xml
index 31af20bc85..35b9e39629 100644
--- a/tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-host.xml
+++ b/tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-host.xml
@@ -1,6 +1,6 @@
 <cpu>
   <arch>x86_64</arch>
-  <model>Icelake-Client</model>
+  <model>Icelake-Server</model>
   <vendor>Intel</vendor>
   <feature name='ds'/>
   <feature name='acpi'/>
@@ -21,23 +21,14 @@
   <feature name='osxsave'/>
   <feature name='tsc_adjust'/>
   <feature name='cmt'/>
-  <feature name='avx512f'/>
-  <feature name='avx512dq'/>
   <feature name='avx512ifma'/>
-  <feature name='clflushopt'/>
-  <feature name='clwb'/>
-  <feature name='avx512cd'/>
   <feature name='sha-ni'/>
-  <feature name='avx512bw'/>
-  <feature name='avx512vl'/>
   <feature name='ospke'/>
-  <feature name='la57'/>
   <feature name='stibp'/>
   <feature name='arch-capabilities'/>
   <feature name='xsaves'/>
   <feature name='mbm_total'/>
   <feature name='mbm_local'/>
-  <feature name='pdpe1gb'/>
   <feature name='invtsc'/>
   <feature name='rdctl-no'/>
   <feature name='ibrs-all'/>
diff --git a/tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-json.xml b/tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-json.xml
index b043db58d7..ada11d2608 100644
--- a/tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-json.xml
+++ b/tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-json.xml
@@ -13,5 +13,4 @@
   <feature policy='require' name='ibrs-all'/>
   <feature policy='require' name='skip-l1dfl-vmentry'/>
   <feature policy='disable' name='intel-pt'/>
-  <feature policy='disable' name='pconfig'/>
 </cpu>
-- 
2.24.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/6] cpu_map: Drop pconfig from Icelake-Server CPU model
Posted by Daniel P. Berrangé 5 years ago 
On Mon, Nov 11, 2019 at 09:35:37PM +0100, Jiri Denemark wrote:
> The pconfig feature was enabled in QEMU by accident in 3.1.0. All other
> newer versions do not support it and it was removed from the
> Icelake-Server CPU model in QEMU.
> 
> We don't normally change our CPU models even when QEMU does so to avoid
> breaking migrations between different versions of libvirt. But we can
> safely do so in this specific case. QEMU never supported enabling
> pconfig so any domain which was able to start has pconfig disabled.
> 
> With a small compatibility hack which explicitly disables pconfig when
> CPU model equals Icelake-Server in migratable domain definition, only
> one migration scenario stays broken (and there's nothing we can do about
> it): from any host to a host with libvirt < 5.10.0 and QEMU > 3.1.0.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1749672
> 
> Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
> ---
>  src/cpu_map/x86_Icelake-Server.xml            |  1 -
>  src/qemu/qemu_domain.c                        | 23 +++++++++++++++++++
>  src/qemu/qemu_domain.h                        |  3 +++
>  src/qemu/qemu_migration_cookie.c              |  3 +++
>  .../x86_64-cpuid-Ice-Lake-Server-guest.xml    |  1 -
>  .../x86_64-cpuid-Ice-Lake-Server-host.xml     | 11 +--------
>  .../x86_64-cpuid-Ice-Lake-Server-json.xml     |  1 -
>  7 files changed, 30 insertions(+), 13 deletions(-)
> 
> diff --git a/src/cpu_map/x86_Icelake-Server.xml b/src/cpu_map/x86_Icelake-Server.xml
> index ecd21cf5c7..a565371977 100644
> --- a/src/cpu_map/x86_Icelake-Server.xml
> +++ b/src/cpu_map/x86_Icelake-Server.xml
> @@ -54,7 +54,6 @@
>      <feature name='pat'/>
>      <feature name='pcid'/>
>      <feature name='pclmuldq'/>
> -    <feature name='pconfig'/>
>      <feature name='pdpe1gb'/>
>      <feature name='pge'/>
>      <feature name='pku'/>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 58a82fbd60..4a641019be 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -8921,6 +8921,26 @@ qemuDomainDefCopy(virQEMUDriverPtr driver,
>  }
>  
>  
> +int
> +qemuDomainMakeCPUMigratable(virCPUDefPtr cpu)
> +{
> +    if (cpu->mode == VIR_CPU_MODE_CUSTOM &&
> +        STREQ_NULLABLE(cpu->model, "Icelake-Server")) {

This might need tweaking when we merge support for versioned CPU
I presume, but not a blocker for this series as is.

> +        /* Originally Icelake-Server CPU model contained pconfig CPU feature.
> +         * It was never actually enabled and thus it was removed. To enable
> +         * migration to QEMU 3.1.0 (with both new and old libvirt), we
> +         * explicitly disable pconfig in migration XML (otherwise old libvirt
> +         * would think it was implicitly enabled on the source). New libvirt
> +         * will drop it from the XML before starting the domain on new QEMU.
> +         */
> +        if (virCPUDefUpdateFeature(cpu, "pconfig", VIR_CPU_FEATURE_DISABLE) < 0)
> +            return -1;
> +    }
> +
> +    return 0;
> +}
> +

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.