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@xxxxxxxxxx > <mailto:pbonzini@xxxxxxxxxx>> 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@xxxxxxxxxx > <mailto:pbonzini@xxxxxxxxxx>> 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@xxxxxxxxxx <mailto:pbonzini@xxxxxxxxxx>> 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 > > -- 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