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. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] Add svm cpuid features
On 09/28/2010 11:28 AM, Roedel, Joerg wrote: 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? 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. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
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
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] Add svm cpuid features
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.
Signed-off-by: Joerg Roedel joerg.roe...@amd.com
---
target-i386/cpu.h | 12
target-i386/cpuid.c | 77 +++---
target-i386/kvm.c |3 ++
3 files changed, 75 insertions(+), 17 deletions(-)
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 1144d4e..77eeab1 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -405,6 +405,17 @@
#define CPUID_EXT3_IBS (1 10)
#define CPUID_EXT3_SKINIT (1 12)
+#define CPUID_SVM_NPT (1 0)
+#define CPUID_SVM_LBRV (1 1)
+#define CPUID_SVM_SVMLOCK (1 2)
+#define CPUID_SVM_NRIPSAVE (1 3)
+#define CPUID_SVM_TSCSCALE (1 4)
+#define CPUID_SVM_VMCBCLEAN(1 5)
+#define CPUID_SVM_FLUSHASID(1 6)
+#define CPUID_SVM_DECODEASSIST (1 7)
+#define CPUID_SVM_PAUSEFILTER (1 10)
+#define CPUID_SVM_PFTHRESHOLD (1 12)
+
#define CPUID_VENDOR_INTEL_1 0x756e6547 /* Genu */
#define CPUID_VENDOR_INTEL_2 0x49656e69 /* ineI */
#define CPUID_VENDOR_INTEL_3 0x6c65746e /* ntel */
@@ -702,6 +713,7 @@ typedef struct CPUX86State {
uint8_t has_error_code;
uint32_t sipi_vector;
uint32_t cpuid_kvm_features;
+uint32_t cpuid_svm_features;
/* in order to simplify APIC support, we leave this pointer to the
user */
diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index 3fcf78f..8e67af0 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -79,6 +79,17 @@ static const char *kvm_feature_name[] = {
NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
};
+static const char *svm_feature_name[] = {
+npt, lbrv, svm_lock, nrip_save,
+tsc_scale, vmcb_clean, flushbyasid, decodeassists,
+NULL, NULL, pause_filter, NULL,
+pfthreshold, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+};
+
/* collects per-function cpuid data
*/
typedef struct model_features_t {
@@ -192,13 +203,15 @@ static void add_flagname_to_bitmaps(const char *flagname,
uint32_t *features,
uint32_t *ext_features,
uint32_t *ext2_features,
uint32_t *ext3_features,
-uint32_t *kvm_features)
+uint32_t *kvm_features,
+uint32_t *svm_features)
{
if (!lookup_feature(features, flagname, NULL, feature_name)
!lookup_feature(ext_features, flagname, NULL, ext_feature_name)
!lookup_feature(ext2_features, flagname, NULL, ext2_feature_name)
!lookup_feature(ext3_features, flagname, NULL, ext3_feature_name)
-!lookup_feature(kvm_features, flagname, NULL, kvm_feature_name))
+!lookup_feature(kvm_features, flagname, NULL, kvm_feature_name)
+!lookup_feature(svm_features, flagname, NULL, svm_feature_name))
fprintf(stderr, CPU feature %s not found\n, flagname);
}
@@ -210,7 +223,8 @@ typedef struct x86_def_t {
int family;
int model;
int stepping;
-uint32_t features, ext_features, ext2_features, ext3_features,
kvm_features;
+uint32_t features, ext_features, ext2_features, ext3_features;
+uint32_t kvm_features, svm_features;
uint32_t xlevel;
char model_id[48];
int vendor_override;
@@ -253,6 +267,7 @@ typedef struct x86_def_t {
CPUID_EXT2_PDPE1GB */
#define TCG_EXT3_FEATURES (CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM | \
CPUID_EXT3_CR8LEG | CPUID_EXT3_ABM | CPUID_EXT3_SSE4A)
+#define TCG_SVM_FEATURES 0
/* maintains list of cpu model definitions
*/
@@ -305,6 +320,7 @@ static x86_def_t builtin_x86_defs[] = {
CPUID_EXT3_OSVW, CPUID_EXT3_IBS */
.ext3_features = CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM |
CPUID_EXT3_ABM | CPUID_EXT3_SSE4A,
+.svm_features = CPUID_SVM_NPT | CPUID_SVM_LBRV | CPUID_SVM_VMCBCLEAN,
.xlevel = 0x801A,
.model_id = AMD Phenom(tm) 9550 Quad-Core Processor
},
@@ -505,6 +521,15 @@ static int cpu_x86_fill_host(x86_def_t *x86_cpu_def)
cpu_x86_fill_model_id(x86_cpu_def-model_id);
x86_cpu_def-vendor_override = 0;
+
+/*
+ * Every SVM feature requires emulation support in KVM - so we can't just
+ * read the host features here. KVM might even support SVM features not
+ * available on the host hardware. Just set all bits and mask out the
+ * unsupported ones later.
+ */
+x86_cpu_def-svm_features = -1;
+
return 0;
}
@@ -560,8 +585,14 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def,
const char *cpu_model)
char *s = strdup(cpu_model);
char *featurestr, *name = strtok(s, ,);
-
