1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH 0/2] target/i386: kvm: Fix treatment of AMD SVM in nested migration

Liran Alon posted 2 patches 4 years, 10 months ago
Test s390x passed
Test checkpatch failed
Test asan failed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test FreeBSD passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190621213712.16222-1-liran.alon@oracle.com
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <rth@twiddle.net>, Eduardo Habkost <ehabkost@redhat.com>, Marcelo Tosatti <mtosatti@redhat.com>
[Qemu-devel] [PATCH 0/2] target/i386: kvm: Fix treatment of AMD SVM in nested migration
Posted by Liran Alon 4 years, 10 months ago 
Hi,

This patch series aims to fix the recent patch-series that was just merged to
upstream QEMU master branch which adds support for nested migration.

The already merged patch-series was modified during merge to allow migration of vCPU
exposed with SVM even though kernel does not support save/restore of required nested state.
This was done because of considering backwards-compatability.

However, during discussion made after merge, it was realised that since QEMU commit
75d373ef9729 ("target-i386: Disable SVM by default in KVM mode"), an AMD vCPU that
is virtualized by KVM doesn't expose SVM by default, even if you use "-cpu host".
Therefore, it is unlikely that vCPU expose SVM CPUID flag when user is not running
an SVM workload inside guest.

Therefore, this patch series change code back to original intent to block migration
in case of vCPU exposed with SVM if kernel does not support required capabilities
tos ave/restore nested-state.

Regards,
-Liran
Re: [Qemu-devel] [PATCH 0/2] target/i386: kvm: Fix treatment of AMD SVM in nested migration
Posted by Paolo Bonzini 4 years, 10 months ago 
On 21/06/19 23:37, Liran Alon wrote:
> However, during discussion made after merge, it was realised that since QEMU commit
> 75d373ef9729 ("target-i386: Disable SVM by default in KVM mode"), an AMD vCPU that
> is virtualized by KVM doesn't expose SVM by default, even if you use "-cpu host".
> Therefore, it is unlikely that vCPU expose SVM CPUID flag when user is not running
> an SVM workload inside guest.

libvirt has "host-model" mode, which constructs a "-cpu
model,+feature,+feature" command line option that matches the host as
good as possible.  This lets libvirt check migratability while retaining
a lot of the benefits of "-cpu host", and is the default for OpenStack
for example.  I need to check if libvirt adds SVM to this configuration,
if it does the QEMU commit you mention is unfortunately not enough.

Paolo
Re: [Qemu-devel] [PATCH 0/2] target/i386: kvm: Fix treatment of AMD SVM in nested migration
Posted by Liran Alon 4 years, 10 months ago 

> On 22 Jun 2019, at 2:59, Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> On 21/06/19 23:37, Liran Alon wrote:
>> However, during discussion made after merge, it was realised that since QEMU commit
>> 75d373ef9729 ("target-i386: Disable SVM by default in KVM mode"), an AMD vCPU that
>> is virtualized by KVM doesn't expose SVM by default, even if you use "-cpu host".
>> Therefore, it is unlikely that vCPU expose SVM CPUID flag when user is not running
>> an SVM workload inside guest.
> 
> libvirt has "host-model" mode, which constructs a "-cpu
> model,+feature,+feature" command line option that matches the host as
> good as possible.  This lets libvirt check migratability while retaining
> a lot of the benefits of "-cpu host", and is the default for OpenStack
> for example.  I need to check if libvirt adds SVM to this configuration,
> if it does the QEMU commit you mention is unfortunately not enough.
> 
> Paolo

Hmm nice catch. I haven’t thought about it (Not familiar much with libvirt).
I agree that if libvirt adds SVM to this configuration than we must not block
migration for an AMD vCPU that is exposed with SVM… :(

Please update once you figure out libvirt behaviour regarding this,
-Liran
Re: [Qemu-devel] [PATCH 0/2] target/i386: kvm: Fix treatment of AMD SVM in nested migration
Posted by no-reply@patchew.org 4 years, 10 months ago 
Patchew URL: https://patchew.org/QEMU/20190621213712.16222-1-liran.alon@oracle.com/



Hi,

This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

Configure options:
--enable-werror --target-list=x86_64-softmmu --prefix=/tmp/qemu-test/install --python=/usr/bin/python3 --enable-debug --enable-sanitizers --cxx=clang++ --cc=clang --host-cc=clang

ERROR: "clang" either does not exist or does not work

# QEMU configure log Sun Jun 23 08:35:11 UTC 2019
# Configured with: '/tmp/qemu-test/src/configure' '--enable-werror' '--target-list=x86_64-softmmu' '--prefix=/tmp/qemu-test/install' '--python=/usr/bin/python3' '--enable-debug' '--enable-sanitizers' '--cxx=clang++' '--cc=clang' '--host-cc=clang'
---
funcs: do_compiler do_cc compile_object check_define main
lines: 92 122 621 638 0
clang -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c
ccache: error: Failed to create temporary file for /var/tmp/ccache/tmp/qemu-conf.stdout: Permission denied

funcs: do_compiler do_cc compile_object check_define main
lines: 92 122 621 640 0
clang -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c
ccache: error: Failed to create temporary file for /var/tmp/ccache/tmp/qemu-conf.stdout: Permission denied

funcs: do_compiler do_cc compile_object check_define main
lines: 92 122 621 642 0
clang -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c
ccache: error: Failed to create temporary file for /var/tmp/ccache/tmp/qemu-conf.stdout: Permission denied

funcs: do_compiler do_cc compile_object check_define main
lines: 92 122 621 644 0
clang -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c
ccache: error: Failed to create temporary file for /var/tmp/ccache/tmp/qemu-conf.stdout: Permission denied

funcs: do_compiler do_cc compile_object check_define main
lines: 92 122 621 646 0
clang -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c
ccache: error: Failed to create temporary file for /var/tmp/ccache/tmp/qemu-conf.stdout: Permission denied

funcs: do_compiler do_cc compile_object check_define main
lines: 92 122 621 648 0
clang -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c
ccache: error: Failed to create temporary file for /var/tmp/ccache/tmp/qemu-conf.stdout: Permission denied

funcs: do_compiler do_cc compile_object check_define main
lines: 92 122 621 650 0
clang -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c
ccache: error: Failed to create temporary file for /var/tmp/ccache/tmp/qemu-conf.stdout: Permission denied

funcs: do_compiler do_cc compile_object check_define main
lines: 92 122 621 652 0
clang -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c
ccache: error: Failed to create temporary file for /var/tmp/ccache/tmp/qemu-conf.stdout: Permission denied

funcs: do_compiler do_cc compile_object check_define main
lines: 92 122 621 654 0
clang -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c
ccache: error: Failed to create temporary file for /var/tmp/ccache/tmp/qemu-conf.stdout: Permission denied

funcs: do_compiler do_cc compile_object check_define main
lines: 92 122 621 656 0
clang -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c
ccache: error: Failed to create temporary file for /var/tmp/ccache/tmp/qemu-conf.stdout: Permission denied

funcs: do_compiler do_cc compile_object check_define main
lines: 92 122 621 690 0
clang -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c
ccache: error: Failed to create temporary file for /var/tmp/ccache/tmp/qemu-conf.stdout: Permission denied

funcs: do_compiler do_cc compile_object check_define main
lines: 92 122 621 692 0
clang -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c
ccache: error: Failed to create temporary file for /var/tmp/ccache/tmp/qemu-conf.stdout: Permission denied

funcs: do_compiler do_cc compile_object check_define main
lines: 92 122 621 698 0
clang -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c
ccache: error: Failed to create temporary file for /var/tmp/ccache/tmp/qemu-conf.stdout: Permission denied

funcs: do_compiler do_cc compile_object check_define main
lines: 92 122 621 704 0
clang -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c
ccache: error: Failed to create temporary file for /var/tmp/ccache/tmp/qemu-conf.stdout: Permission denied

funcs: do_compiler do_cc compile_object check_define main
lines: 92 122 621 714 0
clang -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c
ccache: error: Failed to create temporary file for /var/tmp/ccache/tmp/qemu-conf.stdout: Permission denied

funcs: do_compiler do_cc compile_object check_define main
lines: 92 122 621 716 0
clang -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c
ccache: error: Failed to create temporary file for /var/tmp/ccache/tmp/qemu-conf.stdout: Permission denied

funcs: do_compiler do_cc compile_object check_define main
lines: 92 122 621 722 0
clang -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c
ccache: error: Failed to create temporary file for /var/tmp/ccache/tmp/qemu-conf.stdout: Permission denied

funcs: do_compiler do_cc compile_object check_define main
lines: 92 122 621 728 0
clang -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c
ccache: error: Failed to create temporary file for /var/tmp/ccache/tmp/qemu-conf.stdout: Permission denied

funcs: do_compiler do_cc compile_object check_define main
lines: 92 122 621 730 0
clang -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c
ccache: error: Failed to create temporary file for /var/tmp/ccache/tmp/qemu-conf.stdout: Permission denied

funcs: do_compiler do_cc compile_object main
lines: 92 122 1871 0
clang -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c
ccache: error: Failed to create temporary file for /var/tmp/ccache/tmp/qemu-conf.stdout: Permission denied
Failed to run 'configure'
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 615, in <module>


The full log is available at
http://patchew.org/logs/20190621213712.16222-1-liran.alon@oracle.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [Qemu-devel] [PATCH 0/2] target/i386: kvm: Fix treatment of AMD SVM in nested migration
Posted by no-reply@patchew.org 4 years, 10 months ago 
Patchew URL: https://patchew.org/QEMU/20190621213712.16222-1-liran.alon@oracle.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH 0/2] target/i386: kvm: Fix treatment of AMD SVM in nested migration
Type: series
Message-id: 20190621213712.16222-1-liran.alon@oracle.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

From https://github.com/patchew-project/qemu
 * [new tag]               patchew/20190621213712.16222-1-liran.alon@oracle.com -> patchew/20190621213712.16222-1-liran.alon@oracle.com
Switched to a new branch 'test'
a5de9408a8 target/i386: kvm: Init nested-state in case of vCPU exposed with SVM
06ca99d907 target/i386: kvm: Block migration on vCPU exposed with SVM when kernel lacks caps to save/restore nested state

=== OUTPUT BEGIN ===
1/2 Checking commit 06ca99d907bc (target/i386: kvm: Block migration on vCPU exposed with SVM when kernel lacks caps to save/restore nested state)
ERROR: return is not a function, parentheses are not required
#46: FILE: target/i386/cpu.h:1877:
+    return (cpu_has_vmx(env) || cpu_has_svm(env));

total: 1 errors, 0 warnings, 32 lines checked

Patch 1/2 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

2/2 Checking commit a5de9408a89a (target/i386: kvm: Init nested-state in case of vCPU exposed with SVM)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190621213712.16222-1-liran.alon@oracle.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [Qemu-devel] [PATCH 0/2] target/i386: kvm: Fix treatment of AMD SVM in nested migration
Posted by Liran Alon 4 years, 10 months ago 

> On 22 Jun 2019, at 5:39, no-reply@patchew.org wrote:
> 
> Patchew URL: https://urldefense.proofpoint.com/v2/url?u=https-3A__patchew.org_QEMU_20190621213712.16222-2D1-2Dliran.alon-40oracle.com_&d=DwIGaQ&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=Jk6Q8nNzkQ6LJ6g42qARkg6ryIDGQr-yKXPNGZbpTx0&m=XheZ4_IReq-ruli16BfJeGb3_F7yec8LhFweZ5i6zf8&s=ZYZOCSnRRy8FBDWmZ7sm21_IHQoZJHKXDo6_GHyY6xo&e=
> 
> 
> 
> Hi,
> 
> This series seems to have some coding style problems. See output below for
> more information:
> 
> Subject: [Qemu-devel] [PATCH 0/2] target/i386: kvm: Fix treatment of AMD SVM in nested migration
> Type: series
> Message-id: 20190621213712.16222-1-liran.alon@oracle.com
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> git rev-parse base > /dev/null || exit 0
> git config --local diff.renamelimit 0
> git config --local diff.renames True
> git config --local diff.algorithm histogram
> ./scripts/checkpatch.pl --mailback base..
> === TEST SCRIPT END ===
> 
> From https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_patchew-2Dproject_qemu&d=DwIGaQ&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=Jk6Q8nNzkQ6LJ6g42qARkg6ryIDGQr-yKXPNGZbpTx0&m=XheZ4_IReq-ruli16BfJeGb3_F7yec8LhFweZ5i6zf8&s=QFR1iC1wuS3a7nr5wT0nl1N49SaJCGcwFH_g0Uv7FrU&e=
> * [new tag]               patchew/20190621213712.16222-1-liran.alon@oracle.com -> patchew/20190621213712.16222-1-liran.alon@oracle.com
> Switched to a new branch 'test'
> a5de9408a8 target/i386: kvm: Init nested-state in case of vCPU exposed with SVM
> 06ca99d907 target/i386: kvm: Block migration on vCPU exposed with SVM when kernel lacks caps to save/restore nested state
> 
> === OUTPUT BEGIN ===
> 1/2 Checking commit 06ca99d907bc (target/i386: kvm: Block migration on vCPU exposed with SVM when kernel lacks caps to save/restore nested state)
> ERROR: return is not a function, parentheses are not required
> #46: FILE: target/i386/cpu.h:1877:
> +    return (cpu_has_vmx(env) || cpu_has_svm(env));
> 
> total: 1 errors, 0 warnings, 32 lines checked
> 
> Patch 1/2 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 
> 2/2 Checking commit a5de9408a89a (target/i386: kvm: Init nested-state in case of vCPU exposed with SVM)
> === OUTPUT END ===

I kinda disagree that adding parentheses at return statements is a bad thing…
Why do we enforce such a coding convention?

Anyway, I think this can be fixed when applying if we decide to apply this.

-Liran

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.