Re: [Qemu-devel] [PATCH 2/2] qemu-kvm: Add svm cpuid features

[Alexander Graf]

On 11.09.2010, at 16:36, Joerg Roedel wrote:

> On Sat, Sep 11, 2010 at 04:29:18PM +0200, Alexander Graf wrote:
>>> Depends on which Phenom you have. A Phenom II has NRIPSAVE but the old
>>> Phenoms don't have it. For the SVM features it is not that important
>>> what the host hardware supports but what KVM can emulate. VMCBCLEAN can
>>> be emulated without supporting it in the host for example.
>> 
>> That particular one was my workstation - a Phenom 9550 which is one of
>> the early 4-core ones.
> 
> Yes, the 9550 don't have the nripsave feature.
> 
>>> No, the default CPU type has SVM still enabled by default. I thought
>>> about removing the SVM flag from the qemu64 cpu definition but that
>>> breaks on TCG where SVM is emulated too.
>>> What I implemented in this patch is to enable SVM by default and mask it
>>> out if KVM does not support it on the given machine. Problem here is
>>> that KVM is currently buggy because it always reports support for SVM,
>>> even on Intel machines. I fixed that with patch 29 of my npt-virt
>>> patch-set. The patch will hopefully make it into the various stable
>>> trees and then we have a clean solution.
>> 
>> It still won't be clean as it breaks cross vendor migration :(. The
>> real fix would be to set the default machine to "kvm64" instead of
>> "qemu64" in pc.c when kvm_enabled().
> 
> I am not sure that I am the right person to do such an invasive change.
> At least not in this patch-set. I could think of removing SVM from the
> qemu64 definition and add it again in the TCG specific path.

It's not an invasive change and IMHO the only correct one. I'm not even sure 
why it's not done yet - after all the reason for the kvm* cpu types is exactly 
that. Please just add it as an early patch in your series.

Alex