Patchwork Re: [PATCH 3/3] Add svm cpuid features

login
register
mail settings
Submitter Joerg Roedel
Date Sept. 28, 2010, 10:05 a.m.
Message ID <20100928100520.GL9817@amd.com>
Download mbox | patch
Permalink /patch/65952/
State New
Headers show

Comments

Joerg Roedel - Sept. 28, 2010, 10:05 a.m.
On Tue, Sep 28, 2010 at 05:37:58AM -0400, Avi Kivity wrote:
>   On 09/28/2010 11:28 AM, Roedel, Joerg wrote:

> > Weird, it worked here as I tested it. I had it on qemu/master and with
> > all three patches. But patch 1 should not make the difference. I take a
> > look, have you pushed the failing uq/master?
> 
> Yes, 8fe6a21c76.
> 
> > What was your command line?
> 
> qemu-system-x86_64 -m 2G -cpu kvm64,+svm,+npt -enable-kvm ...
> 
> Note this is qemu.git, so -enable-kvm is needed.

Ok, I apparently forgot to force the CPUID xlevel to be 0x8000000A when
SVM is enabled, probably because I only tested CPUID models where xlevel
already defaults to 0x8000000A. Attached is a fix, thanks for catching
this.

	Joerg


From d18d973d037f1af1869e114aea1f076ea3a1ee7a Mon Sep 17 00:00:00 2001
From: Joerg Roedel <joerg.roedel@amd.com>
Date: Tue, 28 Sep 2010 11:58:49 +0200
Subject: [PATCH 1/1] qemu: Force xlevel to at least 0x8000000A if SVM is enabled

This patch forces the extended CPUID level to be at least
0x8000000A if SVM is enabled in the CPU model.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 target-i386/cpuid.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)
Avi Kivity - Sept. 28, 2010, 10:16 a.m.
On 09/28/2010 12:05 PM, Roedel, Joerg wrote:
> On Tue, Sep 28, 2010 at 05:37:58AM -0400, Avi Kivity wrote:
> >    On 09/28/2010 11:28 AM, Roedel, Joerg wrote:
>
> >  >  Weird, it worked here as I tested it. I had it on qemu/master and with
> >  >  all three patches. But patch 1 should not make the difference. I take a
> >  >  look, have you pushed the failing uq/master?
> >
> >  Yes, 8fe6a21c76.
> >
> >  >  What was your command line?
> >
> >  qemu-system-x86_64 -m 2G -cpu kvm64,+svm,+npt -enable-kvm ...
> >
> >  Note this is qemu.git, so -enable-kvm is needed.
>
> Ok, I apparently forgot to force the CPUID xlevel to be 0x8000000A when
> SVM is enabled, probably because I only tested CPUID models where xlevel
> already defaults to 0x8000000A. Attached is a fix, thanks for catching
> this.
>
> 	Joerg
>
>
>  From d18d973d037f1af1869e114aea1f076ea3a1ee7a Mon Sep 17 00:00:00 2001
> From: Joerg Roedel<joerg.roedel@amd.com>
> Date: Tue, 28 Sep 2010 11:58:49 +0200
> Subject: [PATCH 1/1] qemu: Force xlevel to at least 0x8000000A if SVM is enabled
>
> This patch forces the extended CPUID level to be at least
> 0x8000000A if SVM is enabled in the CPU model.
>
> Signed-off-by: Joerg Roedel<joerg.roedel@amd.com>
> ---
>   target-i386/cpuid.c |    5 +++++
>   1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
> index 0e0bf60..0630fe1 100644
> --- a/target-i386/cpuid.c
> +++ b/target-i386/cpuid.c
> @@ -714,6 +714,11 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
>       x86_cpu_def->ext3_features&= ~minus_ext3_features;
>       x86_cpu_def->kvm_features&= ~minus_kvm_features;
>       x86_cpu_def->svm_features&= ~minus_svm_features;
> +    if ((x86_cpu_def->ext3_features&  CPUID_EXT3_SVM)&&
> +        (x86_cpu_def->xlevel<  0x8000000A)) {
> +        /* Force xlevel to at least 0x8000000A if SVM enabled */
> +        x86_cpu_def->xlevel = 0x8000000A;
> +    }
>       if (check_cpuid) {
>           if (check_features_against_host(x86_cpu_def)&&  enforce_cpuid)
>               goto error;

I can't say I like the interdependency, but saying something like -cpu 
kvm64,+svm,xlevel=0x8000000a is much worse.

If no one has a better idea, I'll apply the patch.
Marcelo Tosatti - Oct. 6, 2010, 6:57 p.m.
On Tue, Sep 28, 2010 at 12:05:20PM +0200, Roedel, Joerg wrote:
> On Tue, Sep 28, 2010 at 05:37:58AM -0400, Avi Kivity wrote:
> >   On 09/28/2010 11:28 AM, Roedel, Joerg wrote:
> 
> > > Weird, it worked here as I tested it. I had it on qemu/master and with
> > > all three patches. But patch 1 should not make the difference. I take a
> > > look, have you pushed the failing uq/master?
> > 
> > Yes, 8fe6a21c76.
> > 
> > > What was your command line?
> > 
> > qemu-system-x86_64 -m 2G -cpu kvm64,+svm,+npt -enable-kvm ...
> > 
> > Note this is qemu.git, so -enable-kvm is needed.
> 
> Ok, I apparently forgot to force the CPUID xlevel to be 0x8000000A when
> SVM is enabled, probably because I only tested CPUID models where xlevel
> already defaults to 0x8000000A. Attached is a fix, thanks for catching
> this.
> 
> 	Joerg

Applied, thanks.

Patch

diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index 0e0bf60..0630fe1 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -714,6 +714,11 @@  static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
     x86_cpu_def->ext3_features &= ~minus_ext3_features;
     x86_cpu_def->kvm_features &= ~minus_kvm_features;
     x86_cpu_def->svm_features &= ~minus_svm_features;
+    if ((x86_cpu_def->ext3_features & CPUID_EXT3_SVM) &&
+        (x86_cpu_def->xlevel < 0x8000000A)) {
+        /* Force xlevel to at least 0x8000000A if SVM enabled */
+        x86_cpu_def->xlevel = 0x8000000A;
+    }
     if (check_cpuid) {
         if (check_features_against_host(x86_cpu_def) && enforce_cpuid)
             goto error;