On 11.09.2010, at 16:20, Joerg Roedel wrote:
> On Sat, Sep 11, 2010 at 03:43:02PM +0200, Alexander Graf wrote:
>>> @@ -305,6 +322,8 @@ 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_NRIPSAVE |
>>> + CPUID_SVM_VMCBCLEAN,
>>
>> Does that phenom already do all those? It does NPT, but I'm not sure
>> about NRIPSAVE for example.
>
> 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.
>
>>> + /*
>>> + * 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
>>> + */
>>> + x86_cpu_def->svm_features = CPUID_SVM_NPT | CPUID_SVM_LBRV |
>>> + CPUID_SVM_NRIPSAVE | CPUID_SVM_VMCBCLEAN;
>>
>> Hrm. Wouldn't it make more sense to declare this to -1? This will
>> still go through the kernel space matcher which tells us which
>> features are available anyways, right?
>
> Yeah, that would make sense. I thought about it while porting the
> patches but could not actually made me do it because I am not entirely
> sure that this is a good idea. But I may revisit that, especially after
> your question :-)
>
>>> -
>>> - if (kvm_enabled()) {
>>> - /* Nested SVM not yet supported in upstream QEMU */
>>> - *ecx &= ~CPUID_EXT3_SVM;
>>> - }
>>
>> Have you made sure that the default cpu type doesn't enable the SVM
>> bit? I couldn't find any trace of an override to "kvm64" as default
>> type when KVM is used.
>
> 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().
Alex
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
