[Qemu-devel] Re: [PATCH 3/3] Add svm cpuid features
On Tue, Sep 28, 2010 at 12:05:20PM +0200, Roedel, Joerg wrote: On Tue, Sep 28, 2010 at 05:37:58AM -0400, Avi Kivity wrote: On 09/28/2010 11:28 AM, Roedel, Joerg wrote: Weird, it worked here as I tested it. I had it on qemu/master and with all three patches. But patch 1 should not make the difference. I take a look, have you pushed the failing uq/master? Yes, 8fe6a21c76. What was your command line? qemu-system-x86_64 -m 2G -cpu kvm64,+svm,+npt -enable-kvm ... Note this is qemu.git, so -enable-kvm is needed. Ok, I apparently forgot to force the CPUID xlevel to be 0x800A when SVM is enabled, probably because I only tested CPUID models where xlevel already defaults to 0x800A. Attached is a fix, thanks for catching this. Joerg Applied, thanks.
[Qemu-devel] Re: [PATCH 3/3] Add svm cpuid features
On Mon, Sep 27, 2010 at 12:22:13PM -0400, Avi Kivity wrote: On 09/27/2010 05:40 PM, Roedel, Joerg wrote: On Mon, Sep 27, 2010 at 11:02:23AM -0400, Avi Kivity wrote: On 09/27/2010 04:58 PM, Avi Kivity wrote: On 09/27/2010 03:16 PM, Joerg Roedel wrote: This patch adds the svm cpuid feature flags to the qemu intialization path. It also adds the svm features available on phenom to its cpu-definition and extends the host cpu type to support all svm features KVM can provide. This should really be all svm features since we'll later mask them with kvm capabilities. This way, if we later add more capabilities, we automatically gain support in userspace. Yes, that is what -cpu host does with these patches applied. The svm_features variable is set to -1 in this case and masked out later with the KVM capabilities. Well, running current uq/master I get: has_svm: can't execute cpuid 0x800a kvm: no hardware support ? Weird, it worked here as I tested it. I had it on qemu/master and with all three patches. But patch 1 should not make the difference. I take a look, have you pushed the failing uq/master? What was your command line? Joerg -- AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
[Qemu-devel] Re: [PATCH 3/3] Add svm cpuid features
On Tue, Sep 28, 2010 at 05:37:58AM -0400, Avi Kivity wrote:
On 09/28/2010 11:28 AM, Roedel, Joerg wrote:
Weird, it worked here as I tested it. I had it on qemu/master and with
all three patches. But patch 1 should not make the difference. I take a
look, have you pushed the failing uq/master?
Yes, 8fe6a21c76.
What was your command line?
qemu-system-x86_64 -m 2G -cpu kvm64,+svm,+npt -enable-kvm ...
Note this is qemu.git, so -enable-kvm is needed.
Ok, I apparently forgot to force the CPUID xlevel to be 0x800A when
SVM is enabled, probably because I only tested CPUID models where xlevel
already defaults to 0x800A. Attached is a fix, thanks for catching
this.
Joerg
From d18d973d037f1af1869e114aea1f076ea3a1ee7a Mon Sep 17 00:00:00 2001
From: Joerg Roedel joerg.roe...@amd.com
Date: Tue, 28 Sep 2010 11:58:49 +0200
Subject: [PATCH 1/1] qemu: Force xlevel to at least 0x800A if SVM is enabled
This patch forces the extended CPUID level to be at least
0x800A if SVM is enabled in the CPU model.
Signed-off-by: Joerg Roedel joerg.roe...@amd.com
---
target-i386/cpuid.c |5 +
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index 0e0bf60..0630fe1 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -714,6 +714,11 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def,
const char *cpu_model)
x86_cpu_def-ext3_features = ~minus_ext3_features;
x86_cpu_def-kvm_features = ~minus_kvm_features;
x86_cpu_def-svm_features = ~minus_svm_features;
+if ((x86_cpu_def-ext3_features CPUID_EXT3_SVM)
+(x86_cpu_def-xlevel 0x800A)) {
+/* Force xlevel to at least 0x800A if SVM enabled */
+x86_cpu_def-xlevel = 0x800A;
+}
if (check_cpuid) {
if (check_features_against_host(x86_cpu_def) enforce_cpuid)
goto error;
--
1.7.0.4
--
AMD Operating System Research Center
Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
[Qemu-devel] Re: [PATCH 3/3] Add svm cpuid features
On 09/28/2010 12:05 PM, Roedel, Joerg wrote:
On Tue, Sep 28, 2010 at 05:37:58AM -0400, Avi Kivity wrote:
On 09/28/2010 11:28 AM, Roedel, Joerg wrote:
Weird, it worked here as I tested it. I had it on qemu/master and with
all three patches. But patch 1 should not make the difference. I take a
look, have you pushed the failing uq/master?
Yes, 8fe6a21c76.
What was your command line?
qemu-system-x86_64 -m 2G -cpu kvm64,+svm,+npt -enable-kvm ...
Note this is qemu.git, so -enable-kvm is needed.
Ok, I apparently forgot to force the CPUID xlevel to be 0x800A when
SVM is enabled, probably because I only tested CPUID models where xlevel
already defaults to 0x800A. Attached is a fix, thanks for catching
this.
Joerg
From d18d973d037f1af1869e114aea1f076ea3a1ee7a Mon Sep 17 00:00:00 2001
From: Joerg Roedeljoerg.roe...@amd.com
Date: Tue, 28 Sep 2010 11:58:49 +0200
Subject: [PATCH 1/1] qemu: Force xlevel to at least 0x800A if SVM is enabled
This patch forces the extended CPUID level to be at least
0x800A if SVM is enabled in the CPU model.
Signed-off-by: Joerg Roedeljoerg.roe...@amd.com
---
target-i386/cpuid.c |5 +
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index 0e0bf60..0630fe1 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -714,6 +714,11 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def,
const char *cpu_model)
x86_cpu_def-ext3_features= ~minus_ext3_features;
x86_cpu_def-kvm_features= ~minus_kvm_features;
x86_cpu_def-svm_features= ~minus_svm_features;
+if ((x86_cpu_def-ext3_features CPUID_EXT3_SVM)
+(x86_cpu_def-xlevel 0x800A)) {
+/* Force xlevel to at least 0x800A if SVM enabled */
+x86_cpu_def-xlevel = 0x800A;
+}
if (check_cpuid) {
if (check_features_against_host(x86_cpu_def) enforce_cpuid)
goto error;
I can't say I like the interdependency, but saying something like -cpu
kvm64,+svm,xlevel=0x800a is much worse.
If no one has a better idea, I'll apply the patch.
--
error compiling committee.c: too many arguments to function
[Qemu-devel] Re: [PATCH 3/3] Add svm cpuid features
On 09/27/2010 03:16 PM, Joerg Roedel wrote: This patch adds the svm cpuid feature flags to the qemu intialization path. It also adds the svm features available on phenom to its cpu-definition and extends the host cpu type to support all svm features KVM can provide. This should really be all svm features since we'll later mask them with kvm capabilities. This way, if we later add more capabilities, we automatically gain support in userspace. (or rather, all svm features that don't need save/restore support in userspace; I don't think any do, beyond svm itself) -- error compiling committee.c: too many arguments to function
[Qemu-devel] Re: [PATCH 3/3] Add svm cpuid features
On 09/27/2010 04:58 PM, Avi Kivity wrote: On 09/27/2010 03:16 PM, Joerg Roedel wrote: This patch adds the svm cpuid feature flags to the qemu intialization path. It also adds the svm features available on phenom to its cpu-definition and extends the host cpu type to support all svm features KVM can provide. This should really be all svm features since we'll later mask them with kvm capabilities. This way, if we later add more capabilities, we automatically gain support in userspace. (or rather, all svm features that don't need save/restore support in userspace; I don't think any do, beyond svm itself) I applied 2 and 3 (to branch uq/master). If you want to add more svm features, please send a patch. For patch 1, I'd like a review by someone who understands the compat code. -- error compiling committee.c: too many arguments to function
[Qemu-devel] Re: [PATCH 3/3] Add svm cpuid features
On Mon, Sep 27, 2010 at 11:02:23AM -0400, Avi Kivity wrote: On 09/27/2010 04:58 PM, Avi Kivity wrote: On 09/27/2010 03:16 PM, Joerg Roedel wrote: This patch adds the svm cpuid feature flags to the qemu intialization path. It also adds the svm features available on phenom to its cpu-definition and extends the host cpu type to support all svm features KVM can provide. This should really be all svm features since we'll later mask them with kvm capabilities. This way, if we later add more capabilities, we automatically gain support in userspace. Yes, that is what -cpu host does with these patches applied. The svm_features variable is set to -1 in this case and masked out later with the KVM capabilities. (or rather, all svm features that don't need save/restore support in userspace; I don't think any do, beyond svm itself) I applied 2 and 3 (to branch uq/master). If you want to add more svm features, please send a patch. For patch 1, I'd like a review by someone who understands the compat code. Great, thanks. Joerg -- AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
[Qemu-devel] Re: [PATCH 3/3] Add svm cpuid features
On 09/27/2010 05:40 PM, Roedel, Joerg wrote: On Mon, Sep 27, 2010 at 11:02:23AM -0400, Avi Kivity wrote: On 09/27/2010 04:58 PM, Avi Kivity wrote: On 09/27/2010 03:16 PM, Joerg Roedel wrote: This patch adds the svm cpuid feature flags to the qemu intialization path. It also adds the svm features available on phenom to its cpu-definition and extends the host cpu type to support all svm features KVM can provide. This should really be all svm features since we'll later mask them with kvm capabilities. This way, if we later add more capabilities, we automatically gain support in userspace. Yes, that is what -cpu host does with these patches applied. The svm_features variable is set to -1 in this case and masked out later with the KVM capabilities. Well, running current uq/master I get: has_svm: can't execute cpuid 0x800a kvm: no hardware support ? -- error compiling committee.c: too many arguments to function
[Qemu-devel] Re: [PATCH 3/3] Add svm cpuid features
On Thu, Sep 16, 2010 at 10:06:48AM -0400, Avi Kivity wrote: On 09/14/2010 05:52 PM, Joerg Roedel wrote: This patch adds the svm cpuid feature flags to the qemu intialization path. It also adds the svm features available on phenom to its cpu-definition and extends the host cpu type to support all svm features KVM can provide. Does phenom really have vmcbclean? I thought it was a really new feature. No, but we could emulate it on almost all hardware that has SVM. Its basically the same as with x2apic. Joerg -- AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
[Qemu-devel] Re: [PATCH 3/3] Add svm cpuid features
Roedel, Joerg wrote: On Thu, Sep 16, 2010 at 10:06:48AM -0400, Avi Kivity wrote: On 09/14/2010 05:52 PM, Joerg Roedel wrote: This patch adds the svm cpuid feature flags to the qemu intialization path. It also adds the svm features available on phenom to its cpu-definition and extends the host cpu type to support all svm features KVM can provide. Does phenom really have vmcbclean? I thought it was a really new feature. No, but we could emulate it on almost all hardware that has SVM. Its basically the same as with x2apic. -cpu non-host is not about what we could emulate, but what the hardware really is. Features not supported by the particular CPU do not belong there. Alex
[Qemu-devel] Re: [PATCH 3/3] Add svm cpuid features
On Thu, Sep 16, 2010 at 10:17:09AM -0400, Alexander Graf wrote: Roedel, Joerg wrote: On Thu, Sep 16, 2010 at 10:06:48AM -0400, Avi Kivity wrote: On 09/14/2010 05:52 PM, Joerg Roedel wrote: This patch adds the svm cpuid feature flags to the qemu intialization path. It also adds the svm features available on phenom to its cpu-definition and extends the host cpu type to support all svm features KVM can provide. Does phenom really have vmcbclean? I thought it was a really new feature. No, but we could emulate it on almost all hardware that has SVM. Its basically the same as with x2apic. -cpu non-host is not about what we could emulate, but what the hardware really is. Features not supported by the particular CPU do not belong there. I'll remove it then. It would have been nice to have it there because it will improve performance of svm emulation. But if it is about emulating the real cpu then I'll just drop it. Joerg -- AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
[Qemu-devel] Re: [PATCH 3/3] Add svm cpuid features
Roedel, Joerg wrote: On Thu, Sep 16, 2010 at 10:17:09AM -0400, Alexander Graf wrote: Roedel, Joerg wrote: On Thu, Sep 16, 2010 at 10:06:48AM -0400, Avi Kivity wrote: On 09/14/2010 05:52 PM, Joerg Roedel wrote: This patch adds the svm cpuid feature flags to the qemu intialization path. It also adds the svm features available on phenom to its cpu-definition and extends the host cpu type to support all svm features KVM can provide. Does phenom really have vmcbclean? I thought it was a really new feature. No, but we could emulate it on almost all hardware that has SVM. Its basically the same as with x2apic. -cpu non-host is not about what we could emulate, but what the hardware really is. Features not supported by the particular CPU do not belong there. I'll remove it then. It would have been nice to have it there because it will improve performance of svm emulation. But if it is about emulating the real cpu then I'll just drop it. speed == -cpu host :). If the performance increase is significant, we can always add a new cpu type for a cpu that does support those flags so people can use it there and still be migration safe. In fact, that would even benefit you guys as you'd sell more new machines for speed ;). Alex
[Qemu-devel] Re: [PATCH 3/3] Add svm cpuid features
On Thu, Sep 16, 2010 at 10:28:01AM -0400, Alexander Graf wrote: Roedel, Joerg wrote: I'll remove it then. It would have been nice to have it there because it will improve performance of svm emulation. But if it is about emulating the real cpu then I'll just drop it. speed == -cpu host :). If the performance increase is significant, we can always add a new cpu type for a cpu that does support those flags so people can use it there and still be migration safe. In fact, that would even benefit you guys as you'd sell more new machines for speed ;). Well, since its emulated completly in software it would be available on all hosts that start with -cpu phenom (at least if they use the same kvm version). So this would not break migration. Joerg -- AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
