Re: [Qemu-devel] [PATCH 1/3] qemu-kvm: Invert svm-flag setting logic

[Daniel P. Berrange]

On Tue, Sep 07, 2010 at 02:23:55PM +0200, Alexander Graf wrote:
> 
> On 07.09.2010, at 13:27, Joerg Roedel wrote:
> 
> > This patch changes the setting logic for the svm bit in
> > qemu-kvm. The bit is now explicitly set on -enable-nesting
> > instead of masked out if the parameter is not supplied.
> > 
> > Signed-off-by: Joerg Roedel <address@hidden>
> > ---
> > target-i386/cpuid.c |   12 ++++++------
> > 1 files changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
> > index d63fdcb..5fa0dd0 100644
> > --- a/target-i386/cpuid.c
> > +++ b/target-i386/cpuid.c
> > @@ -276,8 +276,8 @@ static x86_def_t builtin_x86_defs[] = {
> >         .ext_features = CPUID_EXT_SSE3 | CPUID_EXT_CX16 | CPUID_EXT_POPCNT,
> >         .ext2_features = (PPRO_FEATURES & EXT2_FEATURE_MASK) |
> >             CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
> > -        .ext3_features = CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM |
> > -            CPUID_EXT3_ABM | CPUID_EXT3_SSE4A,
> > +        .ext3_features = CPUID_EXT3_LAHF_LM | CPUID_EXT3_ABM |
> > +                         CPUID_EXT3_SSE4A,
> >         .xlevel = 0x8000000A,
> >         .model_id = "QEMU Virtual CPU version " QEMU_VERSION,
> >     },
> > @@ -303,8 +303,8 @@ static x86_def_t builtin_x86_defs[] = {
> >                     CPUID_EXT3_CR8LEG,
> >                     CPUID_EXT3_MISALIGNSSE, CPUID_EXT3_3DNOWPREFETCH,
> >                     CPUID_EXT3_OSVW, CPUID_EXT3_IBS */
> > -        .ext3_features = CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM |
> > -            CPUID_EXT3_ABM | CPUID_EXT3_SSE4A,
> > +        .ext3_features = CPUID_EXT3_LAHF_LM | CPUID_EXT3_ABM |
> > +                         CPUID_EXT3_SSE4A,
> >         .xlevel = 0x8000001A,
> >         .model_id = "AMD Phenom(tm) 9550 Quad-Core Processor"
> >     },
> > @@ -1154,8 +1154,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
> > uint32_t count,
> >             /* disable CPU features that KVM cannot support */
> > 
> >             /* svm */
> > -            if (!kvm_nested)
> > -                *ecx &= ~CPUID_EXT3_SVM;
> > +            if (kvm_nested)
> 
> I think we should get rid of kvm_nested and -enable-nesting. Instead, we 
> should enable the SVM bit in the "host" and "qemu64" cpu types, but not in 
> "kvm64". This way users are safe to not use nested svm, but can choose to do 
> so if they like.
> 
> Also, it should be possible to do something like -cpu kvm64,flags=+svm. 
> Without having to mess with -enable-nesting.

I agree that its much nicer for mgmt tools if svm/vmx CPU flags can just be
toggled in the normal manner. Might we also want to have a enable/disable
nesting flag, so we can mirror the way real hardware lets you disable virt
in the BIOS even when the CPU(s) have the vmx/svm flags set ?

Regards,
Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|