[libvirt] [PATCH] qemu: cpu: fix fullCPU to include all emulatable qemu features

Nikolay Shirokovskiy posted 1 patch 6 years, 4 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1511182541-570000-1-git-send-email-nshirokovskiy@virtuozzo.com
src/qemu/qemu_capabilities.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
[libvirt] [PATCH] qemu: cpu: fix fullCPU to include all emulatable qemu features
Posted by Nikolay Shirokovskiy 6 years, 4 months ago
On Core i5 650 guest fail to start with error [1] if guest cpu config is taken
from domcapabilities and check is set to partial.

The problem is in qemu caps fullCPU calculation in virQEMUCapsInitHostCPUModel.
It is supposed to include features emulated by qemu and missed on host. Some of
such features may be not included however.

For mentioned cpu host cpu is detected as Westmere and guest cpu as
SandyBridge. x2apic is missed on host and provided by installed qemu. The
feature is not mentioned in guest cpu features explicitly because SandyBridge
model include it. As a result fullCPU does not include x2apic too.

Solution is to expand guest cpu features before updating fullCPU features.

[1] error: the CPU is incompatible with host CPU: Host CPU does not
    provide required features: x2apic, tsc-deadline
---
 src/qemu/qemu_capabilities.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 9c1eeac..edba716 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3475,6 +3475,7 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
                             virDomainVirtType type)
 {
     virCPUDefPtr cpu = NULL;
+    virCPUDefPtr cpuExpanded = NULL;
     virCPUDefPtr migCPU = NULL;
     virCPUDefPtr hostCPU = NULL;
     virCPUDefPtr fullCPU = NULL;
@@ -3504,9 +3505,13 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
                                       NULL, NULL)))
             goto error;
 
-        for (i = 0; i < cpu->nfeatures; i++) {
-            if (cpu->features[i].policy == VIR_CPU_FEATURE_REQUIRE &&
-                virCPUDefUpdateFeature(fullCPU, cpu->features[i].name,
+        if (!(cpuExpanded = virCPUDefCopy(cpu)) ||
+            virCPUExpandFeatures(qemuCaps->arch, cpuExpanded) < 0)
+            goto error;
+
+        for (i = 0; i < cpuExpanded->nfeatures; i++) {
+            if (cpuExpanded->features[i].policy == VIR_CPU_FEATURE_REQUIRE &&
+                virCPUDefUpdateFeature(fullCPU, cpuExpanded->features[i].name,
                                        VIR_CPU_FEATURE_REQUIRE) < 0)
                 goto error;
         }
@@ -3528,6 +3533,7 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
     virQEMUCapsSetHostModel(qemuCaps, type, cpu, migCPU, fullCPU);
 
  cleanup:
+    virCPUDefFree(cpuExpanded);
     virCPUDefFree(hostCPU);
     return;
 
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: cpu: fix fullCPU to include all emulatable qemu features
Posted by Nikolay Shirokovskiy 6 years, 1 month ago
ping

On 20.11.2017 15:55, Nikolay Shirokovskiy wrote:
> On Core i5 650 guest fail to start with error [1] if guest cpu config is taken
> from domcapabilities and check is set to partial.
> 
> The problem is in qemu caps fullCPU calculation in virQEMUCapsInitHostCPUModel.
> It is supposed to include features emulated by qemu and missed on host. Some of
> such features may be not included however.
> 
> For mentioned cpu host cpu is detected as Westmere and guest cpu as
> SandyBridge. x2apic is missed on host and provided by installed qemu. The
> feature is not mentioned in guest cpu features explicitly because SandyBridge
> model include it. As a result fullCPU does not include x2apic too.
> 
> Solution is to expand guest cpu features before updating fullCPU features.
> 
> [1] error: the CPU is incompatible with host CPU: Host CPU does not
>     provide required features: x2apic, tsc-deadline
> ---
>  src/qemu/qemu_capabilities.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 9c1eeac..edba716 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -3475,6 +3475,7 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
>                              virDomainVirtType type)
>  {
>      virCPUDefPtr cpu = NULL;
> +    virCPUDefPtr cpuExpanded = NULL;
>      virCPUDefPtr migCPU = NULL;
>      virCPUDefPtr hostCPU = NULL;
>      virCPUDefPtr fullCPU = NULL;
> @@ -3504,9 +3505,13 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
>                                        NULL, NULL)))
>              goto error;
>  
> -        for (i = 0; i < cpu->nfeatures; i++) {
> -            if (cpu->features[i].policy == VIR_CPU_FEATURE_REQUIRE &&
> -                virCPUDefUpdateFeature(fullCPU, cpu->features[i].name,
> +        if (!(cpuExpanded = virCPUDefCopy(cpu)) ||
> +            virCPUExpandFeatures(qemuCaps->arch, cpuExpanded) < 0)
> +            goto error;
> +
> +        for (i = 0; i < cpuExpanded->nfeatures; i++) {
> +            if (cpuExpanded->features[i].policy == VIR_CPU_FEATURE_REQUIRE &&
> +                virCPUDefUpdateFeature(fullCPU, cpuExpanded->features[i].name,
>                                         VIR_CPU_FEATURE_REQUIRE) < 0)
>                  goto error;
>          }
> @@ -3528,6 +3533,7 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
>      virQEMUCapsSetHostModel(qemuCaps, type, cpu, migCPU, fullCPU);
>  
>   cleanup:
> +    virCPUDefFree(cpuExpanded);
>      virCPUDefFree(hostCPU);
>      return;
>  
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: cpu: fix fullCPU to include all emulatable qemu features
Posted by Nikolay Shirokovskiy 5 years, 11 months ago
I've checked against latest upstream version of libvirt. Issue is still not fixed.


I guess I should provide cpu data for tests like in next patch:

Author: Jiri Denemark <jdenemar@redhat.com>
Date:   Fri Jan 5 14:52:45 2018 +0100

    cputest: Add data for Intel(R) Xeon(R) CPU E5-2609 v3
    
    The CPU contains the updated microcode for CVE-2017-5715.
    
    Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
    Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

 tests/cputest.c                                             |   1 +
 tests/cputestdata/x86_64-cpuid-Xeon-E5-2609-v3-disabled.xml |   6 +
 tests/cputestdata/x86_64-cpuid-Xeon-E5-2609-v3-enabled.xml  |   8 +
 tests/cputestdata/x86_64-cpuid-Xeon-E5-2609-v3-guest.xml    |  32 +++
 tests/cputestdata/x86_64-cpuid-Xeon-E5-2609-v3-host.xml     |  33 +++
 tests/cputestdata/x86_64-cpuid-Xeon-E5-2609-v3-json.xml     |  15 ++
 tests/cputestdata/x86_64-cpuid-Xeon-E5-2609-v3.json         | 726 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/cputestdata/x86_64-cpuid-Xeon-E5-2609-v3.xml          |  37 +++
 8 files changed, 858 insertions(+)

I just can not find a script in libvirt repo that generates them.

On 20.11.2017 15:55, Nikolay Shirokovskiy wrote:
> On Core i5 650 guest fail to start with error [1] if guest cpu config is taken
> from domcapabilities and check is set to partial.
> 
> The problem is in qemu caps fullCPU calculation in virQEMUCapsInitHostCPUModel.
> It is supposed to include features emulated by qemu and missed on host. Some of
> such features may be not included however.
> 
> For mentioned cpu host cpu is detected as Westmere and guest cpu as
> SandyBridge. x2apic is missed on host and provided by installed qemu. The
> feature is not mentioned in guest cpu features explicitly because SandyBridge
> model include it. As a result fullCPU does not include x2apic too.
> 
> Solution is to expand guest cpu features before updating fullCPU features.
> 
> [1] error: the CPU is incompatible with host CPU: Host CPU does not
>     provide required features: x2apic, tsc-deadline
> ---
>  src/qemu/qemu_capabilities.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 9c1eeac..edba716 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -3475,6 +3475,7 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
>                              virDomainVirtType type)
>  {
>      virCPUDefPtr cpu = NULL;
> +    virCPUDefPtr cpuExpanded = NULL;
>      virCPUDefPtr migCPU = NULL;
>      virCPUDefPtr hostCPU = NULL;
>      virCPUDefPtr fullCPU = NULL;
> @@ -3504,9 +3505,13 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
>                                        NULL, NULL)))
>              goto error;
>  
> -        for (i = 0; i < cpu->nfeatures; i++) {
> -            if (cpu->features[i].policy == VIR_CPU_FEATURE_REQUIRE &&
> -                virCPUDefUpdateFeature(fullCPU, cpu->features[i].name,
> +        if (!(cpuExpanded = virCPUDefCopy(cpu)) ||
> +            virCPUExpandFeatures(qemuCaps->arch, cpuExpanded) < 0)
> +            goto error;
> +
> +        for (i = 0; i < cpuExpanded->nfeatures; i++) {
> +            if (cpuExpanded->features[i].policy == VIR_CPU_FEATURE_REQUIRE &&
> +                virCPUDefUpdateFeature(fullCPU, cpuExpanded->features[i].name,
>                                         VIR_CPU_FEATURE_REQUIRE) < 0)
>                  goto error;
>          }
> @@ -3528,6 +3533,7 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
>      virQEMUCapsSetHostModel(qemuCaps, type, cpu, migCPU, fullCPU);
>  
>   cleanup:
> +    virCPUDefFree(cpuExpanded);
>      virCPUDefFree(hostCPU);
>      return;
>  
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: cpu: fix fullCPU to include all emulatable qemu features
Posted by Jiri Denemark 5 years, 11 months ago
On Thu, Apr 05, 2018 at 10:26:48 +0300, Nikolay Shirokovskiy wrote:
> I've checked against latest upstream version of libvirt. Issue is still not fixed.
> 
> I guess I should provide cpu data for tests like in next patch:

Yeah, CPU data would be helpful to see more details about the CPU. You
can generate them in tests/cputestdata using

    ./cpu-gather.sh | ./cpu-parse.sh

You may need to install the cpuid tool before using them and ideally
have a modern QEMU binary installed too.

Jirka

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