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

["Daniel P. Berrange" <berrange@redhat.com>]

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 <joerg.roedel@amd.com>
> > ---
> > 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 :|