* [PATCH 0/3] SVM feature support for qemu @ 2010-09-14 15:52 ` Joerg Roedel 0 siblings, 0 replies; 39+ messages in thread From: Joerg Roedel @ 2010-09-14 15:52 UTC (permalink / raw) To: Avi Kivity, Marcelo Tosatti; +Cc: Alexander Graf, kvm, qemu-devel Hi, here is the next round of the svm feature support patches for qemu. Key change in this version is that it now makes kvm{64|32} the default cpu definition for qemu when kvm is enabled (as requested by Alex). Otherwise I removed the NRIP_SAVE feature from the phenom definition and set svm_features per default to -1 for -cpu host to support all svm features that kvm can emulate. I appreciate your comments. Joerg ^ permalink raw reply [flat|nested] 39+ messages in thread
* [Qemu-devel] [PATCH 0/3] SVM feature support for qemu @ 2010-09-14 15:52 ` Joerg Roedel 0 siblings, 0 replies; 39+ messages in thread From: Joerg Roedel @ 2010-09-14 15:52 UTC (permalink / raw) To: Avi Kivity, Marcelo Tosatti; +Cc: Alexander Graf, kvm, qemu-devel Hi, here is the next round of the svm feature support patches for qemu. Key change in this version is that it now makes kvm{64|32} the default cpu definition for qemu when kvm is enabled (as requested by Alex). Otherwise I removed the NRIP_SAVE feature from the phenom definition and set svm_features per default to -1 for -cpu host to support all svm features that kvm can emulate. I appreciate your comments. Joerg ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 1/3] Make kvm64 the default cpu model when kvm_enabled() 2010-09-14 15:52 ` [Qemu-devel] " Joerg Roedel @ 2010-09-14 15:52 ` Joerg Roedel -1 siblings, 0 replies; 39+ messages in thread From: Joerg Roedel @ 2010-09-14 15:52 UTC (permalink / raw) To: Avi Kivity, Marcelo Tosatti; +Cc: Alexander Graf, kvm, qemu-devel, Joerg Roedel As requested by Alex this patch makes kvm64 the default CPU model when qemu is started with -enable-kvm. Signed-off-by: Joerg Roedel <joerg.roedel@amd.com> --- hw/pc.c | 19 ++++++++++++++----- 1 files changed, 14 insertions(+), 5 deletions(-) diff --git a/hw/pc.c b/hw/pc.c index 69b13bf..f531d0d 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -40,6 +40,16 @@ #include "sysbus.h" #include "sysemu.h" #include "blockdev.h" +#include "kvm.h" + + +#ifdef TARGET_X86_64 +#define DEFAULT_KVM_CPU_MODEL "kvm64" +#define DEFAULT_QEMU_CPU_MODEL "qemu64" +#else +#define DEFAULT_KVM_CPU_MODEL "kvm32" +#define DEFAULT_QEMU_CPU_MODEL "qemu32" +#endif /* output Bochs bios info messages */ //#define DEBUG_BIOS @@ -867,11 +877,10 @@ void pc_cpus_init(const char *cpu_model) /* init CPUs */ if (cpu_model == NULL) { -#ifdef TARGET_X86_64 - cpu_model = "qemu64"; -#else - cpu_model = "qemu32"; -#endif + if (kvm_enabled()) + cpu_model = DEFAULT_KVM_CPU_MODEL; + else + cpu_model = DEFAULT_QEMU_CPU_MODEL; } for(i = 0; i < smp_cpus; i++) { -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [Qemu-devel] [PATCH 1/3] Make kvm64 the default cpu model when kvm_enabled() @ 2010-09-14 15:52 ` Joerg Roedel 0 siblings, 0 replies; 39+ messages in thread From: Joerg Roedel @ 2010-09-14 15:52 UTC (permalink / raw) To: Avi Kivity, Marcelo Tosatti; +Cc: Joerg Roedel, Alexander Graf, kvm, qemu-devel As requested by Alex this patch makes kvm64 the default CPU model when qemu is started with -enable-kvm. Signed-off-by: Joerg Roedel <joerg.roedel@amd.com> --- hw/pc.c | 19 ++++++++++++++----- 1 files changed, 14 insertions(+), 5 deletions(-) diff --git a/hw/pc.c b/hw/pc.c index 69b13bf..f531d0d 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -40,6 +40,16 @@ #include "sysbus.h" #include "sysemu.h" #include "blockdev.h" +#include "kvm.h" + + +#ifdef TARGET_X86_64 +#define DEFAULT_KVM_CPU_MODEL "kvm64" +#define DEFAULT_QEMU_CPU_MODEL "qemu64" +#else +#define DEFAULT_KVM_CPU_MODEL "kvm32" +#define DEFAULT_QEMU_CPU_MODEL "qemu32" +#endif /* output Bochs bios info messages */ //#define DEBUG_BIOS @@ -867,11 +877,10 @@ void pc_cpus_init(const char *cpu_model) /* init CPUs */ if (cpu_model == NULL) { -#ifdef TARGET_X86_64 - cpu_model = "qemu64"; -#else - cpu_model = "qemu32"; -#endif + if (kvm_enabled()) + cpu_model = DEFAULT_KVM_CPU_MODEL; + else + cpu_model = DEFAULT_QEMU_CPU_MODEL; } for(i = 0; i < smp_cpus; i++) { -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] Make kvm64 the default cpu model when kvm_enabled() 2010-09-14 15:52 ` [Qemu-devel] " Joerg Roedel @ 2010-09-14 15:58 ` Alexander Graf -1 siblings, 0 replies; 39+ messages in thread From: Alexander Graf @ 2010-09-14 15:58 UTC (permalink / raw) To: Joerg Roedel; +Cc: Avi Kivity, Marcelo Tosatti, kvm, qemu-devel Joerg Roedel wrote: > As requested by Alex this patch makes kvm64 the default CPU > model when qemu is started with -enable-kvm. > > Signed-off-by: Joerg Roedel <joerg.roedel@amd.com> > --- > hw/pc.c | 19 ++++++++++++++----- > 1 files changed, 14 insertions(+), 5 deletions(-) > > diff --git a/hw/pc.c b/hw/pc.c > index 69b13bf..f531d0d 100644 > --- a/hw/pc.c > +++ b/hw/pc.c > @@ -40,6 +40,16 @@ > #include "sysbus.h" > #include "sysemu.h" > #include "blockdev.h" > +#include "kvm.h" > + > + > +#ifdef TARGET_X86_64 > +#define DEFAULT_KVM_CPU_MODEL "kvm64" > +#define DEFAULT_QEMU_CPU_MODEL "qemu64" > +#else > +#define DEFAULT_KVM_CPU_MODEL "kvm32" > +#define DEFAULT_QEMU_CPU_MODEL "qemu32" > +#endif > > /* output Bochs bios info messages */ > //#define DEBUG_BIOS > @@ -867,11 +877,10 @@ void pc_cpus_init(const char *cpu_model) > > /* init CPUs */ > if (cpu_model == NULL) { > -#ifdef TARGET_X86_64 > - cpu_model = "qemu64"; > -#else > - cpu_model = "qemu32"; > -#endif > + if (kvm_enabled()) > + cpu_model = DEFAULT_KVM_CPU_MODEL; > + else > + cpu_model = DEFAULT_QEMU_CPU_MODEL; > Braces :(. Alex ^ permalink raw reply [flat|nested] 39+ messages in thread
* [Qemu-devel] Re: [PATCH 1/3] Make kvm64 the default cpu model when kvm_enabled() @ 2010-09-14 15:58 ` Alexander Graf 0 siblings, 0 replies; 39+ messages in thread From: Alexander Graf @ 2010-09-14 15:58 UTC (permalink / raw) To: Joerg Roedel; +Cc: Marcelo Tosatti, Avi Kivity, kvm, qemu-devel Joerg Roedel wrote: > As requested by Alex this patch makes kvm64 the default CPU > model when qemu is started with -enable-kvm. > > Signed-off-by: Joerg Roedel <joerg.roedel@amd.com> > --- > hw/pc.c | 19 ++++++++++++++----- > 1 files changed, 14 insertions(+), 5 deletions(-) > > diff --git a/hw/pc.c b/hw/pc.c > index 69b13bf..f531d0d 100644 > --- a/hw/pc.c > +++ b/hw/pc.c > @@ -40,6 +40,16 @@ > #include "sysbus.h" > #include "sysemu.h" > #include "blockdev.h" > +#include "kvm.h" > + > + > +#ifdef TARGET_X86_64 > +#define DEFAULT_KVM_CPU_MODEL "kvm64" > +#define DEFAULT_QEMU_CPU_MODEL "qemu64" > +#else > +#define DEFAULT_KVM_CPU_MODEL "kvm32" > +#define DEFAULT_QEMU_CPU_MODEL "qemu32" > +#endif > > /* output Bochs bios info messages */ > //#define DEBUG_BIOS > @@ -867,11 +877,10 @@ void pc_cpus_init(const char *cpu_model) > > /* init CPUs */ > if (cpu_model == NULL) { > -#ifdef TARGET_X86_64 > - cpu_model = "qemu64"; > -#else > - cpu_model = "qemu32"; > -#endif > + if (kvm_enabled()) > + cpu_model = DEFAULT_KVM_CPU_MODEL; > + else > + cpu_model = DEFAULT_QEMU_CPU_MODEL; > Braces :(. Alex ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] Make kvm64 the default cpu model when kvm_enabled() 2010-09-14 15:58 ` [Qemu-devel] " Alexander Graf @ 2010-09-14 16:21 ` Roedel, Joerg -1 siblings, 0 replies; 39+ messages in thread From: Roedel, Joerg @ 2010-09-14 16:21 UTC (permalink / raw) To: Alexander Graf; +Cc: Avi Kivity, Marcelo Tosatti, kvm, qemu-devel On Tue, Sep 14, 2010 at 11:58:03AM -0400, Alexander Graf wrote: > > + if (kvm_enabled()) > > + cpu_model = DEFAULT_KVM_CPU_MODEL; > > + else > > + cpu_model = DEFAULT_QEMU_CPU_MODEL; > > > > Braces :(. Okay, here is the new patch: >From f49e78edbd4143d05128228d9cc24bd5abc3abf1 Mon Sep 17 00:00:00 2001 From: Joerg Roedel <joerg.roedel@amd.com> Date: Tue, 14 Sep 2010 16:52:11 +0200 Subject: [PATCH 1/3] Make kvm64 the default cpu model when kvm_enabled() As requested by Alex this patch makes kvm64 the default CPU model when qemu is started with -enable-kvm. Signed-off-by: Joerg Roedel <joerg.roedel@amd.com> --- hw/pc.c | 20 +++++++++++++++----- 1 files changed, 15 insertions(+), 5 deletions(-) diff --git a/hw/pc.c b/hw/pc.c index 69b13bf..a6355f3 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -40,6 +40,16 @@ #include "sysbus.h" #include "sysemu.h" #include "blockdev.h" +#include "kvm.h" + + +#ifdef TARGET_X86_64 +#define DEFAULT_KVM_CPU_MODEL "kvm64" +#define DEFAULT_QEMU_CPU_MODEL "qemu64" +#else +#define DEFAULT_KVM_CPU_MODEL "kvm32" +#define DEFAULT_QEMU_CPU_MODEL "qemu32" +#endif /* output Bochs bios info messages */ //#define DEBUG_BIOS @@ -867,11 +877,11 @@ void pc_cpus_init(const char *cpu_model) /* init CPUs */ if (cpu_model == NULL) { -#ifdef TARGET_X86_64 - cpu_model = "qemu64"; -#else - cpu_model = "qemu32"; -#endif + if (kvm_enabled()) { + cpu_model = DEFAULT_KVM_CPU_MODEL; + } else { + cpu_model = DEFAULT_QEMU_CPU_MODEL; + } } for(i = 0; i < smp_cpus; i++) { -- 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 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [Qemu-devel] Re: [PATCH 1/3] Make kvm64 the default cpu model when kvm_enabled() @ 2010-09-14 16:21 ` Roedel, Joerg 0 siblings, 0 replies; 39+ messages in thread From: Roedel, Joerg @ 2010-09-14 16:21 UTC (permalink / raw) To: Alexander Graf; +Cc: Marcelo Tosatti, Avi Kivity, kvm, qemu-devel On Tue, Sep 14, 2010 at 11:58:03AM -0400, Alexander Graf wrote: > > + if (kvm_enabled()) > > + cpu_model = DEFAULT_KVM_CPU_MODEL; > > + else > > + cpu_model = DEFAULT_QEMU_CPU_MODEL; > > > > Braces :(. Okay, here is the new patch: >From f49e78edbd4143d05128228d9cc24bd5abc3abf1 Mon Sep 17 00:00:00 2001 From: Joerg Roedel <joerg.roedel@amd.com> Date: Tue, 14 Sep 2010 16:52:11 +0200 Subject: [PATCH 1/3] Make kvm64 the default cpu model when kvm_enabled() As requested by Alex this patch makes kvm64 the default CPU model when qemu is started with -enable-kvm. Signed-off-by: Joerg Roedel <joerg.roedel@amd.com> --- hw/pc.c | 20 +++++++++++++++----- 1 files changed, 15 insertions(+), 5 deletions(-) diff --git a/hw/pc.c b/hw/pc.c index 69b13bf..a6355f3 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -40,6 +40,16 @@ #include "sysbus.h" #include "sysemu.h" #include "blockdev.h" +#include "kvm.h" + + +#ifdef TARGET_X86_64 +#define DEFAULT_KVM_CPU_MODEL "kvm64" +#define DEFAULT_QEMU_CPU_MODEL "qemu64" +#else +#define DEFAULT_KVM_CPU_MODEL "kvm32" +#define DEFAULT_QEMU_CPU_MODEL "qemu32" +#endif /* output Bochs bios info messages */ //#define DEBUG_BIOS @@ -867,11 +877,11 @@ void pc_cpus_init(const char *cpu_model) /* init CPUs */ if (cpu_model == NULL) { -#ifdef TARGET_X86_64 - cpu_model = "qemu64"; -#else - cpu_model = "qemu32"; -#endif + if (kvm_enabled()) { + cpu_model = DEFAULT_KVM_CPU_MODEL; + } else { + cpu_model = DEFAULT_QEMU_CPU_MODEL; + } } for(i = 0; i < smp_cpus; i++) { -- 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 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] Make kvm64 the default cpu model when kvm_enabled() 2010-09-14 15:52 ` [Qemu-devel] " Joerg Roedel @ 2010-09-16 14:03 ` Avi Kivity -1 siblings, 0 replies; 39+ messages in thread From: Avi Kivity @ 2010-09-16 14:03 UTC (permalink / raw) To: Joerg Roedel; +Cc: Marcelo Tosatti, Alexander Graf, kvm, qemu-devel On 09/14/2010 05:52 PM, Joerg Roedel wrote: > As requested by Alex this patch makes kvm64 the default CPU > model when qemu is started with -enable-kvm. > > This breaks compatiblity; if started with -M 0.13 or prior we should default to qemu64. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 39+ messages in thread
* [Qemu-devel] Re: [PATCH 1/3] Make kvm64 the default cpu model when kvm_enabled() @ 2010-09-16 14:03 ` Avi Kivity 0 siblings, 0 replies; 39+ messages in thread From: Avi Kivity @ 2010-09-16 14:03 UTC (permalink / raw) To: Joerg Roedel; +Cc: Marcelo Tosatti, Alexander Graf, kvm, qemu-devel On 09/14/2010 05:52 PM, Joerg Roedel wrote: > As requested by Alex this patch makes kvm64 the default CPU > model when qemu is started with -enable-kvm. > > This breaks compatiblity; if started with -M 0.13 or prior we should default to qemu64. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] Make kvm64 the default cpu model when kvm_enabled() 2010-09-16 14:03 ` [Qemu-devel] " Avi Kivity @ 2010-09-16 14:21 ` Roedel, Joerg -1 siblings, 0 replies; 39+ messages in thread From: Roedel, Joerg @ 2010-09-16 14:21 UTC (permalink / raw) To: Avi Kivity; +Cc: Marcelo Tosatti, Alexander Graf, kvm, qemu-devel On Thu, Sep 16, 2010 at 10:03:19AM -0400, Avi Kivity wrote: > On 09/14/2010 05:52 PM, Joerg Roedel wrote: > > As requested by Alex this patch makes kvm64 the default CPU > > model when qemu is started with -enable-kvm. > > > > > > This breaks compatiblity; if started with -M 0.13 or prior we should > default to qemu64. Ok, I can change that. But its ok to make kvm64 the default for anything > 0.13? 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [Qemu-devel] Re: [PATCH 1/3] Make kvm64 the default cpu model when kvm_enabled() @ 2010-09-16 14:21 ` Roedel, Joerg 0 siblings, 0 replies; 39+ messages in thread From: Roedel, Joerg @ 2010-09-16 14:21 UTC (permalink / raw) To: Avi Kivity; +Cc: Marcelo Tosatti, Alexander Graf, kvm, qemu-devel On Thu, Sep 16, 2010 at 10:03:19AM -0400, Avi Kivity wrote: > On 09/14/2010 05:52 PM, Joerg Roedel wrote: > > As requested by Alex this patch makes kvm64 the default CPU > > model when qemu is started with -enable-kvm. > > > > > > This breaks compatiblity; if started with -M 0.13 or prior we should > default to qemu64. Ok, I can change that. But its ok to make kvm64 the default for anything > 0.13? 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] Make kvm64 the default cpu model when kvm_enabled() 2010-09-16 14:21 ` [Qemu-devel] " Roedel, Joerg @ 2010-09-16 14:22 ` Avi Kivity -1 siblings, 0 replies; 39+ messages in thread From: Avi Kivity @ 2010-09-16 14:22 UTC (permalink / raw) To: Roedel, Joerg; +Cc: Marcelo Tosatti, Alexander Graf, kvm, qemu-devel On 09/16/2010 04:21 PM, Roedel, Joerg wrote: > On Thu, Sep 16, 2010 at 10:03:19AM -0400, Avi Kivity wrote: > > On 09/14/2010 05:52 PM, Joerg Roedel wrote: > > > As requested by Alex this patch makes kvm64 the default CPU > > > model when qemu is started with -enable-kvm. > > > > > > > > > > This breaks compatiblity; if started with -M 0.13 or prior we should > > default to qemu64. > > Ok, I can change that. But its ok to make kvm64 the default for > anything> 0.13? > Sure. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 39+ messages in thread
* [Qemu-devel] Re: [PATCH 1/3] Make kvm64 the default cpu model when kvm_enabled() @ 2010-09-16 14:22 ` Avi Kivity 0 siblings, 0 replies; 39+ messages in thread From: Avi Kivity @ 2010-09-16 14:22 UTC (permalink / raw) To: Roedel, Joerg; +Cc: Marcelo Tosatti, Alexander Graf, kvm, qemu-devel On 09/16/2010 04:21 PM, Roedel, Joerg wrote: > On Thu, Sep 16, 2010 at 10:03:19AM -0400, Avi Kivity wrote: > > On 09/14/2010 05:52 PM, Joerg Roedel wrote: > > > As requested by Alex this patch makes kvm64 the default CPU > > > model when qemu is started with -enable-kvm. > > > > > > > > > > This breaks compatiblity; if started with -M 0.13 or prior we should > > default to qemu64. > > Ok, I can change that. But its ok to make kvm64 the default for > anything> 0.13? > Sure. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 2/3] Set cpuid definition to 0 before initializing it 2010-09-14 15:52 ` [Qemu-devel] " Joerg Roedel @ 2010-09-14 15:52 ` Joerg Roedel -1 siblings, 0 replies; 39+ messages in thread From: Joerg Roedel @ 2010-09-14 15:52 UTC (permalink / raw) To: Avi Kivity, Marcelo Tosatti; +Cc: Alexander Graf, kvm, qemu-devel, Joerg Roedel This patch cleans the (stack-allocated) cpuid definition to 0 before actually initializing it. Signed-off-by: Joerg Roedel <joerg.roedel@amd.com> --- target-i386/cpuid.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c index 04ba8d5..3fcf78f 100644 --- a/target-i386/cpuid.c +++ b/target-i386/cpuid.c @@ -788,6 +788,8 @@ int cpu_x86_register (CPUX86State *env, const char *cpu_model) { x86_def_t def1, *def = &def1; + memset(def, 0, sizeof(*def)); + if (cpu_x86_find_by_name(def, cpu_model) < 0) return -1; if (def->vendor1) { -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [Qemu-devel] [PATCH 2/3] Set cpuid definition to 0 before initializing it @ 2010-09-14 15:52 ` Joerg Roedel 0 siblings, 0 replies; 39+ messages in thread From: Joerg Roedel @ 2010-09-14 15:52 UTC (permalink / raw) To: Avi Kivity, Marcelo Tosatti; +Cc: Joerg Roedel, Alexander Graf, kvm, qemu-devel This patch cleans the (stack-allocated) cpuid definition to 0 before actually initializing it. Signed-off-by: Joerg Roedel <joerg.roedel@amd.com> --- target-i386/cpuid.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c index 04ba8d5..3fcf78f 100644 --- a/target-i386/cpuid.c +++ b/target-i386/cpuid.c @@ -788,6 +788,8 @@ int cpu_x86_register (CPUX86State *env, const char *cpu_model) { x86_def_t def1, *def = &def1; + memset(def, 0, sizeof(*def)); + if (cpu_x86_find_by_name(def, cpu_model) < 0) return -1; if (def->vendor1) { -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 3/3] Add svm cpuid features 2010-09-14 15:52 ` [Qemu-devel] " Joerg Roedel @ 2010-09-14 15:52 ` Joerg Roedel -1 siblings, 0 replies; 39+ messages in thread From: Joerg Roedel @ 2010-09-14 15:52 UTC (permalink / raw) To: Avi Kivity, Marcelo Tosatti; +Cc: Alexander Graf, kvm, qemu-devel, Joerg Roedel 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.roedel@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 = 0x8000001A, .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, ","); - uint32_t plus_features = 0, plus_ext_features = 0, plus_ext2_features = 0, plus_ext3_features = 0, plus_kvm_features = 0; - uint32_t minus_features = 0, minus_ext_features = 0, minus_ext2_features = 0, minus_ext3_features = 0, minus_kvm_features = 0; + /* Features to be added*/ + uint32_t plus_features = 0, plus_ext_features = 0; + uint32_t plus_ext2_features = 0, plus_ext3_features = 0; + uint32_t plus_kvm_features = 0, plus_svm_features = 0; + /* Features to be removed */ + uint32_t minus_features = 0, minus_ext_features = 0; + uint32_t minus_ext2_features = 0, minus_ext3_features = 0; + uint32_t minus_kvm_features = 0, minus_svm_features = 0; uint32_t numvalue; for (def = x86_defs; def; def = def->next) @@ -579,16 +610,22 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model) add_flagname_to_bitmaps("hypervisor", &plus_features, &plus_ext_features, &plus_ext2_features, &plus_ext3_features, - &plus_kvm_features); + &plus_kvm_features, &plus_svm_features); featurestr = strtok(NULL, ","); while (featurestr) { char *val; if (featurestr[0] == '+') { - add_flagname_to_bitmaps(featurestr + 1, &plus_features, &plus_ext_features, &plus_ext2_features, &plus_ext3_features, &plus_kvm_features); + add_flagname_to_bitmaps(featurestr + 1, &plus_features, + &plus_ext_features, &plus_ext2_features, + &plus_ext3_features, &plus_kvm_features, + &plus_svm_features); } else if (featurestr[0] == '-') { - add_flagname_to_bitmaps(featurestr + 1, &minus_features, &minus_ext_features, &minus_ext2_features, &minus_ext3_features, &minus_kvm_features); + add_flagname_to_bitmaps(featurestr + 1, &minus_features, + &minus_ext_features, &minus_ext2_features, + &minus_ext3_features, &minus_kvm_features, + &minus_svm_features); } else if ((val = strchr(featurestr, '='))) { *val = 0; val++; if (!strcmp(featurestr, "family")) { @@ -670,11 +707,13 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model) x86_cpu_def->ext2_features |= plus_ext2_features; x86_cpu_def->ext3_features |= plus_ext3_features; x86_cpu_def->kvm_features |= plus_kvm_features; + x86_cpu_def->svm_features |= plus_svm_features; x86_cpu_def->features &= ~minus_features; x86_cpu_def->ext_features &= ~minus_ext_features; x86_cpu_def->ext2_features &= ~minus_ext2_features; 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 (check_cpuid) { if (check_features_against_host(x86_cpu_def) && enforce_cpuid) goto error; @@ -816,6 +855,7 @@ int cpu_x86_register (CPUX86State *env, const char *cpu_model) env->cpuid_ext3_features = def->ext3_features; env->cpuid_xlevel = def->xlevel; env->cpuid_kvm_features = def->kvm_features; + env->cpuid_svm_features = def->svm_features; if (!kvm_enabled()) { env->cpuid_features &= TCG_FEATURES; env->cpuid_ext_features &= TCG_EXT_FEATURES; @@ -825,6 +865,7 @@ int cpu_x86_register (CPUX86State *env, const char *cpu_model) #endif ); env->cpuid_ext3_features &= TCG_EXT3_FEATURES; + env->cpuid_svm_features &= TCG_SVM_FEATURES; } { const char *model_id = def->model_id; @@ -1135,11 +1176,6 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, *ecx |= 1 << 1; /* CmpLegacy bit */ } } - - if (kvm_enabled()) { - /* Nested SVM not yet supported in upstream QEMU */ - *ecx &= ~CPUID_EXT3_SVM; - } break; case 0x80000002: case 0x80000003: @@ -1184,10 +1220,17 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, } break; case 0x8000000A: - *eax = 0x00000001; /* SVM Revision */ - *ebx = 0x00000010; /* nr of ASIDs */ - *ecx = 0; - *edx = 0; /* optional features */ + if (env->cpuid_ext3_features & CPUID_EXT3_SVM) { + *eax = 0x00000001; /* SVM Revision */ + *ebx = 0x00000010; /* nr of ASIDs */ + *ecx = 0; + *edx = env->cpuid_svm_features; /* optional features */ + } else { + *eax = 0; + *ebx = 0; + *ecx = 0; + *edx = 0; + } break; default: /* reserved values: zero */ diff --git a/target-i386/kvm.c b/target-i386/kvm.c index a33d2fa..74e7b4f 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -192,6 +192,9 @@ int kvm_arch_init_vcpu(CPUState *env) 0, R_EDX); env->cpuid_ext3_features &= kvm_arch_get_supported_cpuid(env, 0x80000001, 0, R_ECX); + env->cpuid_svm_features &= kvm_arch_get_supported_cpuid(env, 0x8000000A, + 0, R_EDX); + cpuid_i = 0; -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [Qemu-devel] [PATCH 3/3] Add svm cpuid features @ 2010-09-14 15:52 ` Joerg Roedel 0 siblings, 0 replies; 39+ messages in thread From: Joerg Roedel @ 2010-09-14 15:52 UTC (permalink / raw) To: Avi Kivity, Marcelo Tosatti; +Cc: Joerg Roedel, Alexander Graf, kvm, qemu-devel 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.roedel@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 = 0x8000001A, .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, ","); - uint32_t plus_features = 0, plus_ext_features = 0, plus_ext2_features = 0, plus_ext3_features = 0, plus_kvm_features = 0; - uint32_t minus_features = 0, minus_ext_features = 0, minus_ext2_features = 0, minus_ext3_features = 0, minus_kvm_features = 0; + /* Features to be added*/ + uint32_t plus_features = 0, plus_ext_features = 0; + uint32_t plus_ext2_features = 0, plus_ext3_features = 0; + uint32_t plus_kvm_features = 0, plus_svm_features = 0; + /* Features to be removed */ + uint32_t minus_features = 0, minus_ext_features = 0; + uint32_t minus_ext2_features = 0, minus_ext3_features = 0; + uint32_t minus_kvm_features = 0, minus_svm_features = 0; uint32_t numvalue; for (def = x86_defs; def; def = def->next) @@ -579,16 +610,22 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model) add_flagname_to_bitmaps("hypervisor", &plus_features, &plus_ext_features, &plus_ext2_features, &plus_ext3_features, - &plus_kvm_features); + &plus_kvm_features, &plus_svm_features); featurestr = strtok(NULL, ","); while (featurestr) { char *val; if (featurestr[0] == '+') { - add_flagname_to_bitmaps(featurestr + 1, &plus_features, &plus_ext_features, &plus_ext2_features, &plus_ext3_features, &plus_kvm_features); + add_flagname_to_bitmaps(featurestr + 1, &plus_features, + &plus_ext_features, &plus_ext2_features, + &plus_ext3_features, &plus_kvm_features, + &plus_svm_features); } else if (featurestr[0] == '-') { - add_flagname_to_bitmaps(featurestr + 1, &minus_features, &minus_ext_features, &minus_ext2_features, &minus_ext3_features, &minus_kvm_features); + add_flagname_to_bitmaps(featurestr + 1, &minus_features, + &minus_ext_features, &minus_ext2_features, + &minus_ext3_features, &minus_kvm_features, + &minus_svm_features); } else if ((val = strchr(featurestr, '='))) { *val = 0; val++; if (!strcmp(featurestr, "family")) { @@ -670,11 +707,13 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model) x86_cpu_def->ext2_features |= plus_ext2_features; x86_cpu_def->ext3_features |= plus_ext3_features; x86_cpu_def->kvm_features |= plus_kvm_features; + x86_cpu_def->svm_features |= plus_svm_features; x86_cpu_def->features &= ~minus_features; x86_cpu_def->ext_features &= ~minus_ext_features; x86_cpu_def->ext2_features &= ~minus_ext2_features; 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 (check_cpuid) { if (check_features_against_host(x86_cpu_def) && enforce_cpuid) goto error; @@ -816,6 +855,7 @@ int cpu_x86_register (CPUX86State *env, const char *cpu_model) env->cpuid_ext3_features = def->ext3_features; env->cpuid_xlevel = def->xlevel; env->cpuid_kvm_features = def->kvm_features; + env->cpuid_svm_features = def->svm_features; if (!kvm_enabled()) { env->cpuid_features &= TCG_FEATURES; env->cpuid_ext_features &= TCG_EXT_FEATURES; @@ -825,6 +865,7 @@ int cpu_x86_register (CPUX86State *env, const char *cpu_model) #endif ); env->cpuid_ext3_features &= TCG_EXT3_FEATURES; + env->cpuid_svm_features &= TCG_SVM_FEATURES; } { const char *model_id = def->model_id; @@ -1135,11 +1176,6 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, *ecx |= 1 << 1; /* CmpLegacy bit */ } } - - if (kvm_enabled()) { - /* Nested SVM not yet supported in upstream QEMU */ - *ecx &= ~CPUID_EXT3_SVM; - } break; case 0x80000002: case 0x80000003: @@ -1184,10 +1220,17 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, } break; case 0x8000000A: - *eax = 0x00000001; /* SVM Revision */ - *ebx = 0x00000010; /* nr of ASIDs */ - *ecx = 0; - *edx = 0; /* optional features */ + if (env->cpuid_ext3_features & CPUID_EXT3_SVM) { + *eax = 0x00000001; /* SVM Revision */ + *ebx = 0x00000010; /* nr of ASIDs */ + *ecx = 0; + *edx = env->cpuid_svm_features; /* optional features */ + } else { + *eax = 0; + *ebx = 0; + *ecx = 0; + *edx = 0; + } break; default: /* reserved values: zero */ diff --git a/target-i386/kvm.c b/target-i386/kvm.c index a33d2fa..74e7b4f 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -192,6 +192,9 @@ int kvm_arch_init_vcpu(CPUState *env) 0, R_EDX); env->cpuid_ext3_features &= kvm_arch_get_supported_cpuid(env, 0x80000001, 0, R_ECX); + env->cpuid_svm_features &= kvm_arch_get_supported_cpuid(env, 0x8000000A, + 0, R_EDX); + cpuid_i = 0; -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH 3/3] Add svm cpuid features 2010-09-14 15:52 ` [Qemu-devel] " Joerg Roedel @ 2010-09-16 14:06 ` Avi Kivity -1 siblings, 0 replies; 39+ messages in thread From: Avi Kivity @ 2010-09-16 14:06 UTC (permalink / raw) To: Joerg Roedel; +Cc: Marcelo Tosatti, Alexander Graf, kvm, qemu-devel 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. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 39+ messages in thread
* [Qemu-devel] Re: [PATCH 3/3] Add svm cpuid features @ 2010-09-16 14:06 ` Avi Kivity 0 siblings, 0 replies; 39+ messages in thread From: Avi Kivity @ 2010-09-16 14:06 UTC (permalink / raw) To: Joerg Roedel; +Cc: Marcelo Tosatti, Alexander Graf, kvm, qemu-devel 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. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 3/3] Add svm cpuid features 2010-09-16 14:06 ` [Qemu-devel] " Avi Kivity @ 2010-09-16 14:12 ` Roedel, Joerg -1 siblings, 0 replies; 39+ messages in thread From: Roedel, Joerg @ 2010-09-16 14:12 UTC (permalink / raw) To: Avi Kivity; +Cc: Marcelo Tosatti, Alexander Graf, kvm, qemu-devel 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [Qemu-devel] Re: [PATCH 3/3] Add svm cpuid features @ 2010-09-16 14:12 ` Roedel, Joerg 0 siblings, 0 replies; 39+ messages in thread From: Roedel, Joerg @ 2010-09-16 14:12 UTC (permalink / raw) To: Avi Kivity; +Cc: Marcelo Tosatti, Alexander Graf, kvm, qemu-devel 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 3/3] Add svm cpuid features 2010-09-16 14:12 ` [Qemu-devel] " Roedel, Joerg @ 2010-09-16 14:17 ` Alexander Graf -1 siblings, 0 replies; 39+ messages in thread From: Alexander Graf @ 2010-09-16 14:17 UTC (permalink / raw) To: Roedel, Joerg; +Cc: Avi Kivity, Marcelo Tosatti, kvm, qemu-devel 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [Qemu-devel] Re: [PATCH 3/3] Add svm cpuid features @ 2010-09-16 14:17 ` Alexander Graf 0 siblings, 0 replies; 39+ messages in thread From: Alexander Graf @ 2010-09-16 14:17 UTC (permalink / raw) To: Roedel, Joerg; +Cc: Marcelo Tosatti, Avi Kivity, kvm, qemu-devel 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 3/3] Add svm cpuid features 2010-09-16 14:17 ` [Qemu-devel] " Alexander Graf @ 2010-09-16 14:26 ` Roedel, Joerg -1 siblings, 0 replies; 39+ messages in thread From: Roedel, Joerg @ 2010-09-16 14:26 UTC (permalink / raw) To: Alexander Graf; +Cc: Avi Kivity, Marcelo Tosatti, kvm, qemu-devel 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [Qemu-devel] Re: [PATCH 3/3] Add svm cpuid features @ 2010-09-16 14:26 ` Roedel, Joerg 0 siblings, 0 replies; 39+ messages in thread From: Roedel, Joerg @ 2010-09-16 14:26 UTC (permalink / raw) To: Alexander Graf; +Cc: Marcelo Tosatti, Avi Kivity, kvm, qemu-devel 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 3/3] Add svm cpuid features 2010-09-16 14:26 ` [Qemu-devel] " Roedel, Joerg @ 2010-09-16 14:28 ` Alexander Graf -1 siblings, 0 replies; 39+ messages in thread From: Alexander Graf @ 2010-09-16 14:28 UTC (permalink / raw) To: Roedel, Joerg; +Cc: Avi Kivity, Marcelo Tosatti, kvm, qemu-devel 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [Qemu-devel] Re: [PATCH 3/3] Add svm cpuid features @ 2010-09-16 14:28 ` Alexander Graf 0 siblings, 0 replies; 39+ messages in thread From: Alexander Graf @ 2010-09-16 14:28 UTC (permalink / raw) To: Roedel, Joerg; +Cc: Marcelo Tosatti, Avi Kivity, kvm, qemu-devel 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 3/3] Add svm cpuid features 2010-09-16 14:28 ` [Qemu-devel] " Alexander Graf @ 2010-09-16 14:35 ` Roedel, Joerg -1 siblings, 0 replies; 39+ messages in thread From: Roedel, Joerg @ 2010-09-16 14:35 UTC (permalink / raw) To: Alexander Graf; +Cc: Avi Kivity, Marcelo Tosatti, kvm, qemu-devel 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [Qemu-devel] Re: [PATCH 3/3] Add svm cpuid features @ 2010-09-16 14:35 ` Roedel, Joerg 0 siblings, 0 replies; 39+ messages in thread From: Roedel, Joerg @ 2010-09-16 14:35 UTC (permalink / raw) To: Alexander Graf; +Cc: Marcelo Tosatti, Avi Kivity, kvm, qemu-devel 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Qemu-devel] [PATCH 0/3] SVM feature support for qemu 2010-09-14 15:52 ` [Qemu-devel] " Joerg Roedel @ 2010-09-14 20:27 ` Alexander Graf -1 siblings, 0 replies; 39+ messages in thread From: Alexander Graf @ 2010-09-14 20:27 UTC (permalink / raw) To: Joerg Roedel; +Cc: Avi Kivity, Marcelo Tosatti, kvm, qemu-devel On 14.09.2010, at 17:52, Joerg Roedel wrote: > Hi, > > here is the next round of the svm feature support patches for qemu. Key > change in this version is that it now makes kvm{64|32} the default cpu > definition for qemu when kvm is enabled (as requested by Alex). > Otherwise I removed the NRIP_SAVE feature from the phenom definition and > set svm_features per default to -1 for -cpu host to support all svm > features that kvm can emulate. I appreciate your comments. With the updated 1/3 patch, it looks really good to me. Acked-by: Alexander Graf <agraf@suse.de> ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Qemu-devel] [PATCH 0/3] SVM feature support for qemu @ 2010-09-14 20:27 ` Alexander Graf 0 siblings, 0 replies; 39+ messages in thread From: Alexander Graf @ 2010-09-14 20:27 UTC (permalink / raw) To: Joerg Roedel; +Cc: Marcelo Tosatti, Avi Kivity, kvm, qemu-devel On 14.09.2010, at 17:52, Joerg Roedel wrote: > Hi, > > here is the next round of the svm feature support patches for qemu. Key > change in this version is that it now makes kvm{64|32} the default cpu > definition for qemu when kvm is enabled (as requested by Alex). > Otherwise I removed the NRIP_SAVE feature from the phenom definition and > set svm_features per default to -1 for -cpu host to support all svm > features that kvm can emulate. I appreciate your comments. With the updated 1/3 patch, it looks really good to me. Acked-by: Alexander Graf <agraf@suse.de> ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 0/3] SVM feature support for qemu v3 @ 2010-09-27 13:16 Joerg Roedel 2010-09-27 13:16 ` [PATCH 1/3] Make kvm64 the default cpu model when kvm_enabled() Joerg Roedel 0 siblings, 1 reply; 39+ messages in thread From: Joerg Roedel @ 2010-09-27 13:16 UTC (permalink / raw) To: Avi Kivity, Marcelo Tosatti; +Cc: Alexander Graf, kvm, qemu-devel Hi, here is the third round of the patches to add svm-features support to qemu. The patches are rebases against latest qemu/master. Other changes since v2: * Made kvm64 the default model only for qemu-versions >= 0.14.0 * Removed clean-bits from the phenom cpu-model Feedback and/or merge appreciated :-) Joerg ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 1/3] Make kvm64 the default cpu model when kvm_enabled() 2010-09-27 13:16 [PATCH 0/3] SVM feature support for qemu v3 Joerg Roedel @ 2010-09-27 13:16 ` Joerg Roedel 2010-10-06 18:53 ` Marcelo Tosatti 0 siblings, 1 reply; 39+ messages in thread From: Joerg Roedel @ 2010-09-27 13:16 UTC (permalink / raw) To: Avi Kivity, Marcelo Tosatti; +Cc: Joerg Roedel, Alexander Graf, kvm, qemu-devel As requested by Alex this patch makes kvm64 the default CPU model when qemu is started with -enable-kvm. This takes only effect for qemu-versions newer or equal to 0.14.0. Signed-off-by: Joerg Roedel <joerg.roedel@amd.com> --- hw/boards.h | 1 + hw/pc.c | 21 ++++++++++++++++----- hw/pc_piix.c | 6 ++++++ qemu-version.h | 35 +++++++++++++++++++++++++++++++++++ vl.c | 4 ++++ 5 files changed, 62 insertions(+), 5 deletions(-) create mode 100644 qemu-version.h diff --git a/hw/boards.h b/hw/boards.h index 6f0f0d7..2d41b2d 100644 --- a/hw/boards.h +++ b/hw/boards.h @@ -19,6 +19,7 @@ typedef struct QEMUMachine { QEMUMachineInitFunc *init; int use_scsi; int max_cpus; + unsigned int compat_version; unsigned int no_serial:1, no_parallel:1, use_virtcon:1, diff --git a/hw/pc.c b/hw/pc.c index 69b13bf..372ec4c 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -40,6 +40,16 @@ #include "sysbus.h" #include "sysemu.h" #include "blockdev.h" +#include "kvm.h" +#include "qemu-version.h" + +#ifdef TARGET_X86_64 +#define DEFAULT_KVM_CPU_MODEL "kvm64" +#define DEFAULT_QEMU_CPU_MODEL "qemu64" +#else +#define DEFAULT_KVM_CPU_MODEL "kvm32" +#define DEFAULT_QEMU_CPU_MODEL "qemu32" +#endif /* output Bochs bios info messages */ //#define DEBUG_BIOS @@ -867,11 +877,12 @@ void pc_cpus_init(const char *cpu_model) /* init CPUs */ if (cpu_model == NULL) { -#ifdef TARGET_X86_64 - cpu_model = "qemu64"; -#else - cpu_model = "qemu32"; -#endif + if (kvm_enabled() && + qemu_compat_version >= QEMU_COMPAT_VERSION(0, 14, 0)) { + cpu_model = DEFAULT_KVM_CPU_MODEL; + } else { + cpu_model = DEFAULT_QEMU_CPU_MODEL; + } } for(i = 0; i < smp_cpus; i++) { diff --git a/hw/pc_piix.c b/hw/pc_piix.c index 12359a7..9e46b71 100644 --- a/hw/pc_piix.c +++ b/hw/pc_piix.c @@ -35,6 +35,7 @@ #include "sysemu.h" #include "sysbus.h" #include "blockdev.h" +#include "qemu-version.h" #define MAX_IDE_BUS 2 @@ -217,6 +218,7 @@ static QEMUMachine pc_machine = { .desc = "Standard PC", .init = pc_init_pci, .max_cpus = 255, + .compat_version = QEMU_COMPAT_VERSION(0, 13, 0), .is_default = 1, }; @@ -225,6 +227,7 @@ static QEMUMachine pc_machine_v0_12 = { .desc = "Standard PC", .init = pc_init_pci, .max_cpus = 255, + .compat_version = QEMU_COMPAT_VERSION(0, 12, 0), .compat_props = (GlobalProperty[]) { { .driver = "virtio-serial-pci", @@ -244,6 +247,7 @@ static QEMUMachine pc_machine_v0_11 = { .desc = "Standard PC, qemu 0.11", .init = pc_init_pci, .max_cpus = 255, + .compat_version = QEMU_COMPAT_VERSION(0, 11, 0), .compat_props = (GlobalProperty[]) { { .driver = "virtio-blk-pci", @@ -279,6 +283,7 @@ static QEMUMachine pc_machine_v0_10 = { .desc = "Standard PC, qemu 0.10", .init = pc_init_pci, .max_cpus = 255, + .compat_version = QEMU_COMPAT_VERSION(0, 10, 0), .compat_props = (GlobalProperty[]) { { .driver = "virtio-blk-pci", @@ -325,6 +330,7 @@ static QEMUMachine isapc_machine = { .name = "isapc", .desc = "ISA-only PC", .init = pc_init_isa, + .compat_version = QEMU_COMPAT_VERSION(0, 10, 0), .max_cpus = 1, }; diff --git a/qemu-version.h b/qemu-version.h new file mode 100644 index 0000000..b4bfe48 --- /dev/null +++ b/qemu-version.h @@ -0,0 +1,35 @@ +/* + * qemu-version.h + * + * Defines needed for handling QEMU version compatibility + * + * Copyright (c) 2010 Joerg Roedel <joerg.roedel@amd.com> + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#ifndef _QEMU_VERSION_H_ +#define _QEMU_VERSION_H_ + +extern unsigned int qemu_compat_version; + +#define QEMU_COMPAT_VERSION(major, minor, patchlevel) \ + ((unsigned int)(major << 16) | (minor << 8) | (patchlevel)) + +#endif diff --git a/vl.c b/vl.c index d352d18..37727f3 100644 --- a/vl.c +++ b/vl.c @@ -161,6 +161,7 @@ int main(int argc, char **argv) #include "qemu-queue.h" #include "cpus.h" #include "arch_init.h" +#include "qemu-version.h" //#define DEBUG_NET //#define DEBUG_SLIRP @@ -169,6 +170,7 @@ int main(int argc, char **argv) #define MAX_VIRTIO_CONSOLES 1 +unsigned int qemu_compat_version; static const char *data_dir; const char *bios_name = NULL; enum vga_retrace_method vga_retrace_method = VGA_RETRACE_DUMB; @@ -2696,6 +2698,8 @@ int main(int argc, char **argv, char **envp) default_sdcard = 0; } + qemu_compat_version = machine->compat_version; + if (display_type == DT_NOGRAPHIC) { if (default_parallel) add_device_config(DEV_PARALLEL, "null"); -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] Make kvm64 the default cpu model when kvm_enabled() 2010-09-27 13:16 ` [PATCH 1/3] Make kvm64 the default cpu model when kvm_enabled() Joerg Roedel @ 2010-10-06 18:53 ` Marcelo Tosatti 2010-10-06 19:24 ` Anthony Liguori 0 siblings, 1 reply; 39+ messages in thread From: Marcelo Tosatti @ 2010-10-06 18:53 UTC (permalink / raw) To: Joerg Roedel, Anthony Liguori; +Cc: Avi Kivity, Alexander Graf, kvm, qemu-devel On Mon, Sep 27, 2010 at 03:16:15PM +0200, Joerg Roedel wrote: > As requested by Alex this patch makes kvm64 the default CPU > model when qemu is started with -enable-kvm. This takes only > effect for qemu-versions newer or equal to 0.14.0. > > Signed-off-by: Joerg Roedel <joerg.roedel@amd.com> > --- > hw/boards.h | 1 + > hw/pc.c | 21 ++++++++++++++++----- > hw/pc_piix.c | 6 ++++++ > qemu-version.h | 35 +++++++++++++++++++++++++++++++++++ > vl.c | 4 ++++ > 5 files changed, 62 insertions(+), 5 deletions(-) > create mode 100644 qemu-version.h > > diff --git a/hw/boards.h b/hw/boards.h > index 6f0f0d7..2d41b2d 100644 > --- a/hw/boards.h > +++ b/hw/boards.h > @@ -19,6 +19,7 @@ typedef struct QEMUMachine { > QEMUMachineInitFunc *init; > int use_scsi; > int max_cpus; > + unsigned int compat_version; > unsigned int no_serial:1, > no_parallel:1, > use_virtcon:1, > diff --git a/hw/pc.c b/hw/pc.c > index 69b13bf..372ec4c 100644 > --- a/hw/pc.c > +++ b/hw/pc.c > @@ -40,6 +40,16 @@ > #include "sysbus.h" > #include "sysemu.h" > #include "blockdev.h" > +#include "kvm.h" > +#include "qemu-version.h" > + > +#ifdef TARGET_X86_64 > +#define DEFAULT_KVM_CPU_MODEL "kvm64" > +#define DEFAULT_QEMU_CPU_MODEL "qemu64" > +#else > +#define DEFAULT_KVM_CPU_MODEL "kvm32" > +#define DEFAULT_QEMU_CPU_MODEL "qemu32" > +#endif > > /* output Bochs bios info messages */ > //#define DEBUG_BIOS > @@ -867,11 +877,12 @@ void pc_cpus_init(const char *cpu_model) > > /* init CPUs */ > if (cpu_model == NULL) { > -#ifdef TARGET_X86_64 > - cpu_model = "qemu64"; > -#else > - cpu_model = "qemu32"; > -#endif > + if (kvm_enabled() && > + qemu_compat_version >= QEMU_COMPAT_VERSION(0, 14, 0)) { > + cpu_model = DEFAULT_KVM_CPU_MODEL; > + } else { > + cpu_model = DEFAULT_QEMU_CPU_MODEL; > + } > } > > for(i = 0; i < smp_cpus; i++) { > diff --git a/hw/pc_piix.c b/hw/pc_piix.c > index 12359a7..9e46b71 100644 > --- a/hw/pc_piix.c > +++ b/hw/pc_piix.c > @@ -35,6 +35,7 @@ > #include "sysemu.h" > #include "sysbus.h" > #include "blockdev.h" > +#include "qemu-version.h" > > #define MAX_IDE_BUS 2 > > @@ -217,6 +218,7 @@ static QEMUMachine pc_machine = { > .desc = "Standard PC", > .init = pc_init_pci, > .max_cpus = 255, > + .compat_version = QEMU_COMPAT_VERSION(0, 13, 0), > .is_default = 1, > }; > > @@ -225,6 +227,7 @@ static QEMUMachine pc_machine_v0_12 = { > .desc = "Standard PC", > .init = pc_init_pci, > .max_cpus = 255, > + .compat_version = QEMU_COMPAT_VERSION(0, 12, 0), > .compat_props = (GlobalProperty[]) { > { > .driver = "virtio-serial-pci", > @@ -244,6 +247,7 @@ static QEMUMachine pc_machine_v0_11 = { > .desc = "Standard PC, qemu 0.11", > .init = pc_init_pci, > .max_cpus = 255, > + .compat_version = QEMU_COMPAT_VERSION(0, 11, 0), > .compat_props = (GlobalProperty[]) { > { > .driver = "virtio-blk-pci", > @@ -279,6 +283,7 @@ static QEMUMachine pc_machine_v0_10 = { > .desc = "Standard PC, qemu 0.10", > .init = pc_init_pci, > .max_cpus = 255, > + .compat_version = QEMU_COMPAT_VERSION(0, 10, 0), > .compat_props = (GlobalProperty[]) { > { > .driver = "virtio-blk-pci", > @@ -325,6 +330,7 @@ static QEMUMachine isapc_machine = { > .name = "isapc", > .desc = "ISA-only PC", > .init = pc_init_isa, > + .compat_version = QEMU_COMPAT_VERSION(0, 10, 0), > .max_cpus = 1, > }; > > diff --git a/qemu-version.h b/qemu-version.h > new file mode 100644 > index 0000000..b4bfe48 > --- /dev/null > +++ b/qemu-version.h > @@ -0,0 +1,35 @@ > +/* > + * qemu-version.h > + * > + * Defines needed for handling QEMU version compatibility > + * > + * Copyright (c) 2010 Joerg Roedel <joerg.roedel@amd.com> > + * > + * Permission is hereby granted, free of charge, to any person obtaining a copy > + * of this software and associated documentation files (the "Software"), to deal > + * in the Software without restriction, including without limitation the rights > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell > + * copies of the Software, and to permit persons to whom the Software is > + * furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice shall be included in > + * all copies or substantial portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN > + * THE SOFTWARE. > + */ > + > +#ifndef _QEMU_VERSION_H_ > +#define _QEMU_VERSION_H_ > + > +extern unsigned int qemu_compat_version; > + > +#define QEMU_COMPAT_VERSION(major, minor, patchlevel) \ > + ((unsigned int)(major << 16) | (minor << 8) | (patchlevel)) > + > +#endif > diff --git a/vl.c b/vl.c > index d352d18..37727f3 100644 > --- a/vl.c > +++ b/vl.c > @@ -161,6 +161,7 @@ int main(int argc, char **argv) > #include "qemu-queue.h" > #include "cpus.h" > #include "arch_init.h" > +#include "qemu-version.h" > > //#define DEBUG_NET > //#define DEBUG_SLIRP > @@ -169,6 +170,7 @@ int main(int argc, char **argv) > > #define MAX_VIRTIO_CONSOLES 1 > > +unsigned int qemu_compat_version; > static const char *data_dir; > const char *bios_name = NULL; > enum vga_retrace_method vga_retrace_method = VGA_RETRACE_DUMB; > @@ -2696,6 +2698,8 @@ int main(int argc, char **argv, char **envp) > default_sdcard = 0; > } > > + qemu_compat_version = machine->compat_version; > + > if (display_type == DT_NOGRAPHIC) { > if (default_parallel) > add_device_config(DEV_PARALLEL, "null"); > -- > 1.7.0.4 Looks fine to me, given CPUs are not in qdev. Anthony? ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] Make kvm64 the default cpu model when kvm_enabled() 2010-10-06 18:53 ` Marcelo Tosatti @ 2010-10-06 19:24 ` Anthony Liguori 2010-10-07 8:42 ` Roedel, Joerg 0 siblings, 1 reply; 39+ messages in thread From: Anthony Liguori @ 2010-10-06 19:24 UTC (permalink / raw) To: Marcelo Tosatti Cc: Joerg Roedel, Anthony Liguori, Avi Kivity, Alexander Graf, kvm, qemu-devel On 10/06/2010 01:53 PM, Marcelo Tosatti wrote: > On Mon, Sep 27, 2010 at 03:16:15PM +0200, Joerg Roedel wrote: > >> As requested by Alex this patch makes kvm64 the default CPU >> model when qemu is started with -enable-kvm. This takes only >> effect for qemu-versions newer or equal to 0.14.0. >> >> Signed-off-by: Joerg Roedel<joerg.roedel@amd.com> >> --- >> hw/boards.h | 1 + >> hw/pc.c | 21 ++++++++++++++++----- >> hw/pc_piix.c | 6 ++++++ >> qemu-version.h | 35 +++++++++++++++++++++++++++++++++++ >> vl.c | 4 ++++ >> 5 files changed, 62 insertions(+), 5 deletions(-) >> create mode 100644 qemu-version.h >> >> diff --git a/hw/boards.h b/hw/boards.h >> index 6f0f0d7..2d41b2d 100644 >> --- a/hw/boards.h >> +++ b/hw/boards.h >> @@ -19,6 +19,7 @@ typedef struct QEMUMachine { >> QEMUMachineInitFunc *init; >> int use_scsi; >> int max_cpus; >> + unsigned int compat_version; >> unsigned int no_serial:1, >> no_parallel:1, >> use_virtcon:1, >> diff --git a/hw/pc.c b/hw/pc.c >> index 69b13bf..372ec4c 100644 >> --- a/hw/pc.c >> +++ b/hw/pc.c >> @@ -40,6 +40,16 @@ >> #include "sysbus.h" >> #include "sysemu.h" >> #include "blockdev.h" >> +#include "kvm.h" >> +#include "qemu-version.h" >> + >> +#ifdef TARGET_X86_64 >> +#define DEFAULT_KVM_CPU_MODEL "kvm64" >> +#define DEFAULT_QEMU_CPU_MODEL "qemu64" >> +#else >> +#define DEFAULT_KVM_CPU_MODEL "kvm32" >> +#define DEFAULT_QEMU_CPU_MODEL "qemu32" >> +#endif >> >> /* output Bochs bios info messages */ >> //#define DEBUG_BIOS >> @@ -867,11 +877,12 @@ void pc_cpus_init(const char *cpu_model) >> >> /* init CPUs */ >> if (cpu_model == NULL) { >> -#ifdef TARGET_X86_64 >> - cpu_model = "qemu64"; >> -#else >> - cpu_model = "qemu32"; >> -#endif >> + if (kvm_enabled()&& >> + qemu_compat_version>= QEMU_COMPAT_VERSION(0, 14, 0)) { >> + cpu_model = DEFAULT_KVM_CPU_MODEL; >> + } else { >> + cpu_model = DEFAULT_QEMU_CPU_MODEL; >> + } >> } >> >> for(i = 0; i< smp_cpus; i++) { >> diff --git a/hw/pc_piix.c b/hw/pc_piix.c >> index 12359a7..9e46b71 100644 >> --- a/hw/pc_piix.c >> +++ b/hw/pc_piix.c >> @@ -35,6 +35,7 @@ >> #include "sysemu.h" >> #include "sysbus.h" >> #include "blockdev.h" >> +#include "qemu-version.h" >> >> #define MAX_IDE_BUS 2 >> >> @@ -217,6 +218,7 @@ static QEMUMachine pc_machine = { >> .desc = "Standard PC", >> .init = pc_init_pci, >> .max_cpus = 255, >> + .compat_version = QEMU_COMPAT_VERSION(0, 13, 0), >> .is_default = 1, >> }; >> >> @@ -225,6 +227,7 @@ static QEMUMachine pc_machine_v0_12 = { >> .desc = "Standard PC", >> .init = pc_init_pci, >> .max_cpus = 255, >> + .compat_version = QEMU_COMPAT_VERSION(0, 12, 0), >> .compat_props = (GlobalProperty[]) { >> { >> .driver = "virtio-serial-pci", >> @@ -244,6 +247,7 @@ static QEMUMachine pc_machine_v0_11 = { >> .desc = "Standard PC, qemu 0.11", >> .init = pc_init_pci, >> .max_cpus = 255, >> + .compat_version = QEMU_COMPAT_VERSION(0, 11, 0), >> .compat_props = (GlobalProperty[]) { >> { >> .driver = "virtio-blk-pci", >> @@ -279,6 +283,7 @@ static QEMUMachine pc_machine_v0_10 = { >> .desc = "Standard PC, qemu 0.10", >> .init = pc_init_pci, >> .max_cpus = 255, >> + .compat_version = QEMU_COMPAT_VERSION(0, 10, 0), >> .compat_props = (GlobalProperty[]) { >> { >> .driver = "virtio-blk-pci", >> @@ -325,6 +330,7 @@ static QEMUMachine isapc_machine = { >> .name = "isapc", >> .desc = "ISA-only PC", >> .init = pc_init_isa, >> + .compat_version = QEMU_COMPAT_VERSION(0, 10, 0), >> .max_cpus = 1, >> }; >> >> diff --git a/qemu-version.h b/qemu-version.h >> new file mode 100644 >> index 0000000..b4bfe48 >> --- /dev/null >> +++ b/qemu-version.h >> @@ -0,0 +1,35 @@ >> +/* >> + * qemu-version.h >> + * >> + * Defines needed for handling QEMU version compatibility >> + * >> + * Copyright (c) 2010 Joerg Roedel<joerg.roedel@amd.com> >> + * >> + * Permission is hereby granted, free of charge, to any person obtaining a copy >> + * of this software and associated documentation files (the "Software"), to deal >> + * in the Software without restriction, including without limitation the rights >> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell >> + * copies of the Software, and to permit persons to whom the Software is >> + * furnished to do so, subject to the following conditions: >> + * >> + * The above copyright notice and this permission notice shall be included in >> + * all copies or substantial portions of the Software. >> + * >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL >> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER >> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, >> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN >> + * THE SOFTWARE. >> + */ >> + >> +#ifndef _QEMU_VERSION_H_ >> +#define _QEMU_VERSION_H_ >> + >> +extern unsigned int qemu_compat_version; >> + >> +#define QEMU_COMPAT_VERSION(major, minor, patchlevel) \ >> + ((unsigned int)(major<< 16) | (minor<< 8) | (patchlevel)) >> + >> +#endif >> diff --git a/vl.c b/vl.c >> index d352d18..37727f3 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -161,6 +161,7 @@ int main(int argc, char **argv) >> #include "qemu-queue.h" >> #include "cpus.h" >> #include "arch_init.h" >> +#include "qemu-version.h" >> >> //#define DEBUG_NET >> //#define DEBUG_SLIRP >> @@ -169,6 +170,7 @@ int main(int argc, char **argv) >> >> #define MAX_VIRTIO_CONSOLES 1 >> >> +unsigned int qemu_compat_version; >> static const char *data_dir; >> const char *bios_name = NULL; >> enum vga_retrace_method vga_retrace_method = VGA_RETRACE_DUMB; >> @@ -2696,6 +2698,8 @@ int main(int argc, char **argv, char **envp) >> default_sdcard = 0; >> } >> >> + qemu_compat_version = machine->compat_version; >> + >> if (display_type == DT_NOGRAPHIC) { >> if (default_parallel) >> add_device_config(DEV_PARALLEL, "null"); >> -- >> 1.7.0.4 >> > Looks fine to me, given CPUs are not in qdev. Anthony? > The idea is fine, but why not just add the default CPU to the machine description? Regards, Anthony Liguori ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] Make kvm64 the default cpu model when kvm_enabled() 2010-10-06 19:24 ` Anthony Liguori @ 2010-10-07 8:42 ` Roedel, Joerg 2010-10-07 12:48 ` Anthony Liguori 0 siblings, 1 reply; 39+ messages in thread From: Roedel, Joerg @ 2010-10-07 8:42 UTC (permalink / raw) To: Anthony Liguori Cc: Marcelo Tosatti, Anthony Liguori, Avi Kivity, Alexander Graf, kvm, qemu-devel On Wed, Oct 06, 2010 at 03:24:59PM -0400, Anthony Liguori wrote: > >> + qemu_compat_version = machine->compat_version; > >> + > >> if (display_type == DT_NOGRAPHIC) { > >> if (default_parallel) > >> add_device_config(DEV_PARALLEL, "null"); > >> -- > >> 1.7.0.4 > >> > > Looks fine to me, given CPUs are not in qdev. Anthony? > > > > The idea is fine, but why not just add the default CPU to the machine > description? If I remember correctly the reason was that the machine description was not accessible in the cpuid initialization path because it is a function local variable. I could have made it a global variable but considered the compat_version approach simpler. The qemu_compat_version might also be useful at other places. 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] Make kvm64 the default cpu model when kvm_enabled() 2010-10-07 8:42 ` Roedel, Joerg @ 2010-10-07 12:48 ` Anthony Liguori 2010-10-18 8:22 ` Roedel, Joerg 0 siblings, 1 reply; 39+ messages in thread From: Anthony Liguori @ 2010-10-07 12:48 UTC (permalink / raw) To: Roedel, Joerg Cc: Marcelo Tosatti, Anthony Liguori, Avi Kivity, Alexander Graf, kvm, qemu-devel [-- Attachment #1: Type: text/plain, Size: 1497 bytes --] On 10/07/2010 03:42 AM, Roedel, Joerg wrote: > On Wed, Oct 06, 2010 at 03:24:59PM -0400, Anthony Liguori wrote: > >>>> + qemu_compat_version = machine->compat_version; >>>> + >>>> if (display_type == DT_NOGRAPHIC) { >>>> if (default_parallel) >>>> add_device_config(DEV_PARALLEL, "null"); >>>> -- >>>> 1.7.0.4 >>>> >>>> >>> Looks fine to me, given CPUs are not in qdev. Anthony? >>> >>> >> The idea is fine, but why not just add the default CPU to the machine >> description? >> > If I remember correctly the reason was that the machine description was > not accessible in the cpuid initialization path because it is a function > local variable. Not tested at all but I think the attached patch addresses it in a pretty nice way. There's a couple ways you could support your patch on top of this. You could add a kvm_cpu_model to the machine structure that gets defaulted too if kvm_enabled(). You could also introduce a new KVM machine type that gets defaulted to if no explicit machine is specified. > I could have made it a global variable but considered > the compat_version approach simpler. The qemu_compat_version might also > be useful at other places. > The reason we've avoided having a builtin notion of versions is that we have many downstreams where versioning would get very complicated. If we stick to features it makes it much easier for downstreams. Regards, Anthony Liguori > Joerg > > [-- Attachment #2: 0001-machine-make-default-cpu-model-part-of-machine-struc.patch --] [-- Type: text/x-patch, Size: 3253 bytes --] >From d2370c88cef4b07d48ba3c4804e35ae2db8db7c0 Mon Sep 17 00:00:00 2001 From: Anthony Liguori <aliguori@us.ibm.com> Date: Thu, 7 Oct 2010 07:43:42 -0500 Subject: [PATCH] machine: make default cpu model part of machine structure Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> diff --git a/hw/boards.h b/hw/boards.h index 6f0f0d7..8c6ef27 100644 --- a/hw/boards.h +++ b/hw/boards.h @@ -16,6 +16,7 @@ typedef struct QEMUMachine { const char *name; const char *alias; const char *desc; + const char *cpu_model; QEMUMachineInitFunc *init; int use_scsi; int max_cpus; diff --git a/hw/pc.c b/hw/pc.c index 69b13bf..0826107 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -866,14 +866,6 @@ void pc_cpus_init(const char *cpu_model) int i; /* init CPUs */ - if (cpu_model == NULL) { -#ifdef TARGET_X86_64 - cpu_model = "qemu64"; -#else - cpu_model = "qemu32"; -#endif - } - for(i = 0; i < smp_cpus; i++) { pc_new_cpu(cpu_model); } diff --git a/hw/pc_piix.c b/hw/pc_piix.c index 12359a7..919b4d6 100644 --- a/hw/pc_piix.c +++ b/hw/pc_piix.c @@ -204,17 +204,22 @@ static void pc_init_isa(ram_addr_t ram_size, const char *initrd_filename, const char *cpu_model) { - if (cpu_model == NULL) - cpu_model = "486"; pc_init1(ram_size, boot_device, kernel_filename, kernel_cmdline, initrd_filename, cpu_model, 0); } +#ifdef TARGET_X86_64 +#define DEF_CPU_MODEL "qemu64" +#else +#define DEF_CPU_MODEL "qemu32" +#endif + static QEMUMachine pc_machine = { .name = "pc-0.13", .alias = "pc", .desc = "Standard PC", + .cpu_model = DEF_CPU_MODEL, .init = pc_init_pci, .max_cpus = 255, .is_default = 1, @@ -223,6 +228,7 @@ static QEMUMachine pc_machine = { static QEMUMachine pc_machine_v0_12 = { .name = "pc-0.12", .desc = "Standard PC", + .cpu_model = DEF_CPU_MODEL, .init = pc_init_pci, .max_cpus = 255, .compat_props = (GlobalProperty[]) { @@ -242,6 +248,7 @@ static QEMUMachine pc_machine_v0_12 = { static QEMUMachine pc_machine_v0_11 = { .name = "pc-0.11", .desc = "Standard PC, qemu 0.11", + .cpu_model = DEF_CPU_MODEL, .init = pc_init_pci, .max_cpus = 255, .compat_props = (GlobalProperty[]) { @@ -277,6 +284,7 @@ static QEMUMachine pc_machine_v0_11 = { static QEMUMachine pc_machine_v0_10 = { .name = "pc-0.10", .desc = "Standard PC, qemu 0.10", + .cpu_model = DEF_CPU_MODEL, .init = pc_init_pci, .max_cpus = 255, .compat_props = (GlobalProperty[]) { @@ -324,6 +332,7 @@ static QEMUMachine pc_machine_v0_10 = { static QEMUMachine isapc_machine = { .name = "isapc", .desc = "ISA-only PC", + .cpu_model = "486", .init = pc_init_isa, .max_cpus = 1, }; diff --git a/vl.c b/vl.c index df414ef..3a55cc8 100644 --- a/vl.c +++ b/vl.c @@ -2904,6 +2904,10 @@ int main(int argc, char **argv, char **envp) } qemu_add_globals(); + if (cpu_model == NULL) { + cpu_model = machine->cpu_model; + } + machine->init(ram_size, boot_devices, kernel_filename, kernel_cmdline, initrd_filename, cpu_model); -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] Make kvm64 the default cpu model when kvm_enabled() 2010-10-07 12:48 ` Anthony Liguori @ 2010-10-18 8:22 ` Roedel, Joerg 2010-10-18 14:16 ` Anthony Liguori 0 siblings, 1 reply; 39+ messages in thread From: Roedel, Joerg @ 2010-10-18 8:22 UTC (permalink / raw) To: Anthony Liguori Cc: Marcelo Tosatti, Anthony Liguori, Avi Kivity, Alexander Graf, kvm, qemu-devel (Sorry for the late reply) On Thu, Oct 07, 2010 at 08:48:06AM -0400, Anthony Liguori wrote: > On 10/07/2010 03:42 AM, Roedel, Joerg wrote: > > On Wed, Oct 06, 2010 at 03:24:59PM -0400, Anthony Liguori wrote: > > > >>>> + qemu_compat_version = machine->compat_version; > >>>> + > >>>> if (display_type == DT_NOGRAPHIC) { > >>>> if (default_parallel) > >>>> add_device_config(DEV_PARALLEL, "null"); > >>>> -- > >>>> 1.7.0.4 > >>>> > >>>> > >>> Looks fine to me, given CPUs are not in qdev. Anthony? > >>> > >>> > >> The idea is fine, but why not just add the default CPU to the machine > >> description? > >> > > If I remember correctly the reason was that the machine description was > > not accessible in the cpuid initialization path because it is a function > > local variable. > > Not tested at all but I think the attached patch addresses it in a > pretty nice way. > > There's a couple ways you could support your patch on top of this. You > could add a kvm_cpu_model to the machine structure that gets defaulted > too if kvm_enabled(). You could also introduce a new KVM machine type > that gets defaulted to if no explicit machine is specified. I had something similar in mind but then I realized that we need at least a cpu_model and a cpu_model_kvm to distinguish between the TCG and the KVM case. Further the QEMUMachine data structure is used for all architectures in QEMU and the model-names only make sense for x86. So I decided for the comapt-version way (which doesn't mean I object against this one ;-) ) Joerg > From d2370c88cef4b07d48ba3c4804e35ae2db8db7c0 Mon Sep 17 00:00:00 2001 > From: Anthony Liguori <aliguori@us.ibm.com> > Date: Thu, 7 Oct 2010 07:43:42 -0500 > Subject: [PATCH] machine: make default cpu model part of machine structure > > Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> > > diff --git a/hw/boards.h b/hw/boards.h > index 6f0f0d7..8c6ef27 100644 > --- a/hw/boards.h > +++ b/hw/boards.h > @@ -16,6 +16,7 @@ typedef struct QEMUMachine { > const char *name; > const char *alias; > const char *desc; > + const char *cpu_model; > QEMUMachineInitFunc *init; > int use_scsi; > int max_cpus; > diff --git a/hw/pc.c b/hw/pc.c > index 69b13bf..0826107 100644 > --- a/hw/pc.c > +++ b/hw/pc.c > @@ -866,14 +866,6 @@ void pc_cpus_init(const char *cpu_model) > int i; > > /* init CPUs */ > - if (cpu_model == NULL) { > -#ifdef TARGET_X86_64 > - cpu_model = "qemu64"; > -#else > - cpu_model = "qemu32"; > -#endif > - } > - > for(i = 0; i < smp_cpus; i++) { > pc_new_cpu(cpu_model); > } > diff --git a/hw/pc_piix.c b/hw/pc_piix.c > index 12359a7..919b4d6 100644 > --- a/hw/pc_piix.c > +++ b/hw/pc_piix.c > @@ -204,17 +204,22 @@ static void pc_init_isa(ram_addr_t ram_size, > const char *initrd_filename, > const char *cpu_model) > { > - if (cpu_model == NULL) > - cpu_model = "486"; > pc_init1(ram_size, boot_device, > kernel_filename, kernel_cmdline, > initrd_filename, cpu_model, 0); > } > > +#ifdef TARGET_X86_64 > +#define DEF_CPU_MODEL "qemu64" > +#else > +#define DEF_CPU_MODEL "qemu32" > +#endif > + > static QEMUMachine pc_machine = { > .name = "pc-0.13", > .alias = "pc", > .desc = "Standard PC", > + .cpu_model = DEF_CPU_MODEL, > .init = pc_init_pci, > .max_cpus = 255, > .is_default = 1, > @@ -223,6 +228,7 @@ static QEMUMachine pc_machine = { > static QEMUMachine pc_machine_v0_12 = { > .name = "pc-0.12", > .desc = "Standard PC", > + .cpu_model = DEF_CPU_MODEL, > .init = pc_init_pci, > .max_cpus = 255, > .compat_props = (GlobalProperty[]) { > @@ -242,6 +248,7 @@ static QEMUMachine pc_machine_v0_12 = { > static QEMUMachine pc_machine_v0_11 = { > .name = "pc-0.11", > .desc = "Standard PC, qemu 0.11", > + .cpu_model = DEF_CPU_MODEL, > .init = pc_init_pci, > .max_cpus = 255, > .compat_props = (GlobalProperty[]) { > @@ -277,6 +284,7 @@ static QEMUMachine pc_machine_v0_11 = { > static QEMUMachine pc_machine_v0_10 = { > .name = "pc-0.10", > .desc = "Standard PC, qemu 0.10", > + .cpu_model = DEF_CPU_MODEL, > .init = pc_init_pci, > .max_cpus = 255, > .compat_props = (GlobalProperty[]) { > @@ -324,6 +332,7 @@ static QEMUMachine pc_machine_v0_10 = { > static QEMUMachine isapc_machine = { > .name = "isapc", > .desc = "ISA-only PC", > + .cpu_model = "486", > .init = pc_init_isa, > .max_cpus = 1, > }; > diff --git a/vl.c b/vl.c > index df414ef..3a55cc8 100644 > --- a/vl.c > +++ b/vl.c > @@ -2904,6 +2904,10 @@ int main(int argc, char **argv, char **envp) > } > qemu_add_globals(); > > + if (cpu_model == NULL) { > + cpu_model = machine->cpu_model; > + } > + > machine->init(ram_size, boot_devices, > kernel_filename, kernel_cmdline, initrd_filename, cpu_model); > > -- > 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] Make kvm64 the default cpu model when kvm_enabled() 2010-10-18 8:22 ` Roedel, Joerg @ 2010-10-18 14:16 ` Anthony Liguori 0 siblings, 0 replies; 39+ messages in thread From: Anthony Liguori @ 2010-10-18 14:16 UTC (permalink / raw) To: Roedel, Joerg Cc: Marcelo Tosatti, Anthony Liguori, Avi Kivity, Alexander Graf, kvm, qemu-devel On 10/18/2010 03:22 AM, Roedel, Joerg wrote: > (Sorry for the late reply) > > On Thu, Oct 07, 2010 at 08:48:06AM -0400, Anthony Liguori wrote: > >> On 10/07/2010 03:42 AM, Roedel, Joerg wrote: >> >>> On Wed, Oct 06, 2010 at 03:24:59PM -0400, Anthony Liguori wrote: >>> >>> >>>>>> + qemu_compat_version = machine->compat_version; >>>>>> + >>>>>> if (display_type == DT_NOGRAPHIC) { >>>>>> if (default_parallel) >>>>>> add_device_config(DEV_PARALLEL, "null"); >>>>>> -- >>>>>> 1.7.0.4 >>>>>> >>>>>> >>>>>> >>>>> Looks fine to me, given CPUs are not in qdev. Anthony? >>>>> >>>>> >>>>> >>>> The idea is fine, but why not just add the default CPU to the machine >>>> description? >>>> >>>> >>> If I remember correctly the reason was that the machine description was >>> not accessible in the cpuid initialization path because it is a function >>> local variable. >>> >> Not tested at all but I think the attached patch addresses it in a >> pretty nice way. >> >> There's a couple ways you could support your patch on top of this. You >> could add a kvm_cpu_model to the machine structure that gets defaulted >> too if kvm_enabled(). You could also introduce a new KVM machine type >> that gets defaulted to if no explicit machine is specified. >> > I had something similar in mind but then I realized that we need at > least a cpu_model and a cpu_model_kvm to distinguish between the TCG and > the KVM case. > I would think that having different default machines for KVM and TCG would be a better solution. > Further the QEMUMachine data structure is used for all architectures in > QEMU and the model-names only make sense for x86. SPARC uses cpu_model too FWIW. I believe Blue Swirl has even discussed using a feature-format similar to how x86 does it for SPARC CPUs. Regards, Anthony Liguori > So I decided for the > comapt-version way (which doesn't mean I object against this one ;-) ) > > Joerg > > >> From d2370c88cef4b07d48ba3c4804e35ae2db8db7c0 Mon Sep 17 00:00:00 2001 >> From: Anthony Liguori<aliguori@us.ibm.com> >> Date: Thu, 7 Oct 2010 07:43:42 -0500 >> Subject: [PATCH] machine: make default cpu model part of machine structure >> >> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com> >> >> diff --git a/hw/boards.h b/hw/boards.h >> index 6f0f0d7..8c6ef27 100644 >> --- a/hw/boards.h >> +++ b/hw/boards.h >> @@ -16,6 +16,7 @@ typedef struct QEMUMachine { >> const char *name; >> const char *alias; >> const char *desc; >> + const char *cpu_model; >> QEMUMachineInitFunc *init; >> int use_scsi; >> int max_cpus; >> diff --git a/hw/pc.c b/hw/pc.c >> index 69b13bf..0826107 100644 >> --- a/hw/pc.c >> +++ b/hw/pc.c >> @@ -866,14 +866,6 @@ void pc_cpus_init(const char *cpu_model) >> int i; >> >> /* init CPUs */ >> - if (cpu_model == NULL) { >> -#ifdef TARGET_X86_64 >> - cpu_model = "qemu64"; >> -#else >> - cpu_model = "qemu32"; >> -#endif >> - } >> - >> for(i = 0; i< smp_cpus; i++) { >> pc_new_cpu(cpu_model); >> } >> diff --git a/hw/pc_piix.c b/hw/pc_piix.c >> index 12359a7..919b4d6 100644 >> --- a/hw/pc_piix.c >> +++ b/hw/pc_piix.c >> @@ -204,17 +204,22 @@ static void pc_init_isa(ram_addr_t ram_size, >> const char *initrd_filename, >> const char *cpu_model) >> { >> - if (cpu_model == NULL) >> - cpu_model = "486"; >> pc_init1(ram_size, boot_device, >> kernel_filename, kernel_cmdline, >> initrd_filename, cpu_model, 0); >> } >> >> +#ifdef TARGET_X86_64 >> +#define DEF_CPU_MODEL "qemu64" >> +#else >> +#define DEF_CPU_MODEL "qemu32" >> +#endif >> + >> static QEMUMachine pc_machine = { >> .name = "pc-0.13", >> .alias = "pc", >> .desc = "Standard PC", >> + .cpu_model = DEF_CPU_MODEL, >> .init = pc_init_pci, >> .max_cpus = 255, >> .is_default = 1, >> @@ -223,6 +228,7 @@ static QEMUMachine pc_machine = { >> static QEMUMachine pc_machine_v0_12 = { >> .name = "pc-0.12", >> .desc = "Standard PC", >> + .cpu_model = DEF_CPU_MODEL, >> .init = pc_init_pci, >> .max_cpus = 255, >> .compat_props = (GlobalProperty[]) { >> @@ -242,6 +248,7 @@ static QEMUMachine pc_machine_v0_12 = { >> static QEMUMachine pc_machine_v0_11 = { >> .name = "pc-0.11", >> .desc = "Standard PC, qemu 0.11", >> + .cpu_model = DEF_CPU_MODEL, >> .init = pc_init_pci, >> .max_cpus = 255, >> .compat_props = (GlobalProperty[]) { >> @@ -277,6 +284,7 @@ static QEMUMachine pc_machine_v0_11 = { >> static QEMUMachine pc_machine_v0_10 = { >> .name = "pc-0.10", >> .desc = "Standard PC, qemu 0.10", >> + .cpu_model = DEF_CPU_MODEL, >> .init = pc_init_pci, >> .max_cpus = 255, >> .compat_props = (GlobalProperty[]) { >> @@ -324,6 +332,7 @@ static QEMUMachine pc_machine_v0_10 = { >> static QEMUMachine isapc_machine = { >> .name = "isapc", >> .desc = "ISA-only PC", >> + .cpu_model = "486", >> .init = pc_init_isa, >> .max_cpus = 1, >> }; >> diff --git a/vl.c b/vl.c >> index df414ef..3a55cc8 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -2904,6 +2904,10 @@ int main(int argc, char **argv, char **envp) >> } >> qemu_add_globals(); >> >> + if (cpu_model == NULL) { >> + cpu_model = machine->cpu_model; >> + } >> + >> machine->init(ram_size, boot_devices, >> kernel_filename, kernel_cmdline, initrd_filename, cpu_model); >> >> -- >> 1.7.0.4 >> >> > > ^ permalink raw reply [flat|nested] 39+ messages in thread
end of thread, other threads:[~2010-10-18 18:13 UTC | newest] Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-09-14 15:52 [PATCH 0/3] SVM feature support for qemu Joerg Roedel 2010-09-14 15:52 ` [Qemu-devel] " Joerg Roedel 2010-09-14 15:52 ` [PATCH 1/3] Make kvm64 the default cpu model when kvm_enabled() Joerg Roedel 2010-09-14 15:52 ` [Qemu-devel] " Joerg Roedel 2010-09-14 15:58 ` Alexander Graf 2010-09-14 15:58 ` [Qemu-devel] " Alexander Graf 2010-09-14 16:21 ` Roedel, Joerg 2010-09-14 16:21 ` [Qemu-devel] " Roedel, Joerg 2010-09-16 14:03 ` Avi Kivity 2010-09-16 14:03 ` [Qemu-devel] " Avi Kivity 2010-09-16 14:21 ` Roedel, Joerg 2010-09-16 14:21 ` [Qemu-devel] " Roedel, Joerg 2010-09-16 14:22 ` Avi Kivity 2010-09-16 14:22 ` [Qemu-devel] " Avi Kivity 2010-09-14 15:52 ` [PATCH 2/3] Set cpuid definition to 0 before initializing it Joerg Roedel 2010-09-14 15:52 ` [Qemu-devel] " Joerg Roedel 2010-09-14 15:52 ` [PATCH 3/3] Add svm cpuid features Joerg Roedel 2010-09-14 15:52 ` [Qemu-devel] " Joerg Roedel 2010-09-16 14:06 ` Avi Kivity 2010-09-16 14:06 ` [Qemu-devel] " Avi Kivity 2010-09-16 14:12 ` Roedel, Joerg 2010-09-16 14:12 ` [Qemu-devel] " Roedel, Joerg 2010-09-16 14:17 ` Alexander Graf 2010-09-16 14:17 ` [Qemu-devel] " Alexander Graf 2010-09-16 14:26 ` Roedel, Joerg 2010-09-16 14:26 ` [Qemu-devel] " Roedel, Joerg 2010-09-16 14:28 ` Alexander Graf 2010-09-16 14:28 ` [Qemu-devel] " Alexander Graf 2010-09-16 14:35 ` Roedel, Joerg 2010-09-16 14:35 ` [Qemu-devel] " Roedel, Joerg 2010-09-14 20:27 ` [Qemu-devel] [PATCH 0/3] SVM feature support for qemu Alexander Graf 2010-09-14 20:27 ` Alexander Graf 2010-09-27 13:16 [PATCH 0/3] SVM feature support for qemu v3 Joerg Roedel 2010-09-27 13:16 ` [PATCH 1/3] Make kvm64 the default cpu model when kvm_enabled() Joerg Roedel 2010-10-06 18:53 ` Marcelo Tosatti 2010-10-06 19:24 ` Anthony Liguori 2010-10-07 8:42 ` Roedel, Joerg 2010-10-07 12:48 ` Anthony Liguori 2010-10-18 8:22 ` Roedel, Joerg 2010-10-18 14:16 ` Anthony Liguori
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.