Re: [PATCH v2] kvm: nVMX: CPUID.01H:EDX.APIC[bit 9] should mirror IA32_APIC_BASE[11]

[Paolo Bonzini <pbonzini@redhat.com>]

On 10/11/2016 18:16, Jim Mattson wrote:
> What if I were to adopt your previous suggestion:
> 
>> The three new lines should be inside "if (apic)", because you are not calling kvm_update_cpuid at all in kvm_lapic_set_base's "if (!apic)" case.
> 
> That would preserve the existing behavior for the QEMU userspace irqchip.
> 
> I do have a kvm-unit-test for the in-kernel APIC.

Send it, we can test it with QEMU userspace irqchip.  Always send unit
tests if you have them. :)

Paolo

> On Wed, Nov 9, 2016 at 9:43 AM, Paolo Bonzini <pbonzini@redhat.com
> <mailto:pbonzini@redhat.com>> wrote:
> 
> 
> 
>     On 09/11/2016 18:41, Jim Mattson wrote:
>     > Agreed, but as you can probably tell, I don't use QEMU, so I will
>     > leave that to someone who does.
> 
>     If you can find a colleague or minion to do it, fine, otherwise I'll
>     drop the patch.  The benefit is small and the corner cases large.
> 
>     Paolo
> 
>     > On Wed, Nov 9, 2016 at 9:37 AM, Paolo Bonzini <pbonzini@redhat.com
>     <mailto:pbonzini@redhat.com>> wrote:
>     >>
>     >>
>     >> On 09/11/2016 18:28, Jim Mattson wrote:
>     >>> Better would be to call kvm_update_cpuid regardless, I think.
>     >>
>     >> Sure.  A kvm-unit-tests patch would be even better so that we can
>     ensure
>     >> it works with QEMU userspace irqchip. :)
>     >>
>     >> Paolo
>     >>
>     >>> On Wed, Nov 9, 2016 at 9:14 AM, Paolo Bonzini
>     <pbonzini@redhat.com <mailto:pbonzini@redhat.com>> wrote:
>     >>>>
>     >>>>
>     >>>> On 09/11/2016 18:04, Jim Mattson wrote:
>     >>>>>
>     >>>>> +     best->edx &= ~F(APIC);
>     >>>>> +     if (vcpu->arch.apic_base & MSR_IA32_APICBASE_ENABLE)
>     >>>>> +             best->edx |= F(APIC);
>     >>>>> +
>     >>>>>       if (apic) {
>     >>>>>               if (best->ecx & F(TSC_DEADLINE_TIMER))
>     >>>>>                       apic->lapic_timer.timer_mode_mask = 3 << 17;
>     >>>>
>     >>>> The three new lines should be inside "if (apic)", because you
>     are not
>     >>>> calling kvm_update_cpuid at all in kvm_lapic_set_base's "if
>     (!apic)" case.
>     >>>>
>     >>>> Thanks,
>     >>>>
>     >>>> Paolo
> 
>