Re: [Qemu-devel] 3.1.0-rc{0,1} doesn't start

[Paolo Bonzini]

On 20/11/18 19:48, Eduardo Habkost wrote:
> On Tue, Nov 20, 2018 at 07:40:01PM +0100, Paolo Bonzini wrote:
>> On 20/11/18 00:00, Eduardo Habkost wrote:
>>> On Mon, Nov 19, 2018 at 04:55:13PM -0500, Bandan Das wrote:
>>>> address@hidden writes:
>>>>
>>>>> hello
>>>>>
>>>>> I'm building qemu from source and happily using it since a bit
>>>>> (2.3.0)
>>>>>
>>>>> Since 3.1.0-rc0 (including latest 3.1.0-rc1) I'm no more able to start
>>>>> qemu, getting:
>>>>>
>>>>>     ----8<----
>>>>>     install:115> qemu
>>>>>     qemu: error: failed to set MSR 0x10a to 0x0
>>>>>     qemu: 
>>>>> /home/balducci/tmp/install-us-d/qemu-3.1.0-rc1.d/qemu-3.1.0-rc0/target/i386/kvm.c:2185:
>>>>>  kvm_put_msrs: Assertion `ret == cpu->kvm_msr_buf->nmsrs' failed.
>>>>>     Aborted
>>>>>     ---->8----
>>>>>
>>>> I believe the check on whether MSR_IA32_ARCH_CAPABILITIES is present is
>>>> incomplete because it can return 0 for data. Can you try this:
>>>>
>>>> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
>>>> index f524e7d929..4878ffb90b 100644
>>>> --- a/target/i386/kvm.c
>>>> +++ b/target/i386/kvm.c
>>>> @@ -2002,14 +2002,9 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
>>>>  #endif
>>>>  
>>>>      /* If host supports feature MSR, write down. */
>>>> -    if (kvm_feature_msrs) {
>>>> -        int i;
>>>> -        for (i = 0; i < kvm_feature_msrs->nmsrs; i++)
>>>> -            if (kvm_feature_msrs->indices[i] == 
>>>> MSR_IA32_ARCH_CAPABILITIES) {
>>>> -                kvm_msr_entry_add(cpu, MSR_IA32_ARCH_CAPABILITIES,
>>>> +    if (kvm_arch_get_supported_msr_feature(kvm_state, 
>>>> MSR_IA32_ARCH_CAPABILITIES)) {
>>>> +        kvm_msr_entry_add(cpu, MSR_IA32_ARCH_CAPABILITIES,
>>>>                                env->features[FEAT_ARCH_CAPABILITIES]);
>>>
>>> kvm_arch_get_supported_msr_feature() will return the value of the
>>> MSR on the host side (kvm/x86.c:kvm_get_msr_feature()).  Having
>>> it return non-zero doesn't mean KVM's
>>> svm_set_msr(MSR_IA32_ARCH_CAPABILITIES) will work.
>>>
>>> If the MSR doesn't work on KVM_SET_MSRS, it is not supposed to
>>> appear on KVM_GET_MSR_INDEX_LIST (even if it appears on
>>> KVM_GET_MSR_FEATURE_INDEX_LIST).  QEMU must check
>>> KVM_GET_MSR_INDEX_LIST too before including the MSR on the
>>> KVM_SET_MSRS call.
>>
>> Yes, this is a KVM bug.  For 3.1, making it "writable if nonzero" is a
>> valid workaround, because AMD processors always return 0. It's not the
>> prettiest thing, but it works.
> 
> So it looks like we have 2 bugs?  Is KVM incorrectly returning
> the MSR on KVM_GET_MSR_INDEX_LIST?
> 
> I thought the bug was only in QEMU, which is supposed to be
> checking KVM_GET_MSR_INDEX_LIST before setting the MSR (it is
> only checking KVM_GET_MSR_FEATURE_INDEX_LIST).

Oh, wait.  I didn't understand that.  Then yeah, fixing QEMU is enough.
 KVM is not reporting the MSR in the list on AMD, but it should (and it
should implement the MSR, of course!).

Paolo