2016-09-19 21:44 GMT+08:00 Radim Krčmář <rkrcmar@xxxxxxxxxx>: > 2016-09-18 14:53+0800, Wanpeng Li: >> 2016-09-15 23:58 GMT+08:00 Radim Krčmář <rkrcmar@xxxxxxxxxx>: >>> 2016-09-15 15:05+0800, Wanpeng Li: >>>> 2016-09-14 20:03 GMT+08:00 Radim Krčmář <rkrcmar@xxxxxxxxxx>: >>>>> 2016-09-14 11:40+0200, Paolo Bonzini: >>>>>> On 14/09/2016 09:58, Wanpeng Li wrote: >>>>>>> From: Wanpeng Li <wanpeng.li@xxxxxxxxxxx> >>>>>>> >>>>>>> I observed that kvmvapic(to optimize flexpriority=N or AMD) is used >>>>>>> to boost TPR access when testing kvm-unit-test/eventinj.flat tpr case >>>>>>> on my haswell desktop (w/ flexpriority, w/o APICv). Commit (8d14695f9542 >>>>>>> x86, apicv: add virtual x2apic support) disable virtual x2apic mode >>>>>>> completely if w/o APICv, and the author also told me that windows guest >>>>>>> can't enter into x2apic mode when he developed the APICv feature several >>>>>>> years ago. However, it is not truth currently, Interrupt Remapping and >>>>>>> vIOMMU is added to qemu and the developers from Intel test windows 8 can >>>>>>> work in x2apic mode w/ Interrupt Remapping enabled recently. >>>>>>> >>>>>>> This patch enables TPR shadow for virtual x2apic mode to boost >>>>>>> windows guest in x2apic mode even if w/o APICv. >>>>>>> >>>>>>> Can pass the kvm-unit-test. >>>>>> >>>>>> Ok, now I see what you meant; this actually makes sense. I don't expect >>>>>> much speedup though, because Linux doesn't touch the TPR and Windows is >>>>>> likely going to use the Hyper-V APIC MSRs when APICv is disabled. For >>>>>> this reason I'm not sure if the patch is useful in practice. >>>>> >>>>> I agree with Paolo on the use case -- what configurations benefit from >>>>> this change? >>>>> >>>>>> To test this patch, you have to run kvm-unit-tests with Hyper-V >>>>>> synthetic interrupt enabled. Did you do this? >>>>> >>>>> The patch is buggy. MSR bitmaps are global and we'd have a CVE if one >>>>> guests used synic (=> disabled apicv) and one didn't. >>>>> You'd want a new set of bitmaps and assign them in vmx_set_msr_bitmap() >>>>> (or completely rewrite our management). >>>> >>>> Do you think introduce per-VM x2apic msr bitmap make sense? >>> >>> Not much. It would still need different msr bitmaps for VCPUs in >>> various modes, so it would take more memory and be slower without giving >>> nicer code as we'd have to do pretty much the same as we do now. >>> We could improve clarity of the caching solution instead ... >>> >>> Per-VCPU could allow a slightly clearer design, but that is very >>> wasteful -- the caching isn't that bad. >> >> Could you elaborate the caching design in your mind? :) > > The one we already do -- precompute all possible bitmaps at KVM > initialization and assign the appropriate ones at runtime. I see. :) > >> In addition, >> I'm not sure whether we still can get benefit from x2apic tpr shadow >> w/o APICv since the overhead of the other bitmaps/caching. > > Overhead from assigning a cached MSR bitmap should be less than one VM > exit caused by a TPR write when there are no pending interrupts. > Are there other sources of overhead? Then I understand what the cached MSR bitmap you mean instead of introducing another caching. > >> Btw, I heard from Tianyu from Intel, you said there was a x2apic bug >> in kvm forum and the bug maybe in kvm, I guess I meet the same bug >> when run a windows guest(server version of windows 7, 2008 or 2012) w/ >> x2apic enabled in guest and -machine q35,kernel_irqchip=spit -device >> intel-iommu, intremap=on in the QEMU command line. > > Does it happen when you are running less than 8 VCPUs (max APIC ID < 8)? > QEMU always enabled x2APIC support in IOMMU (EIME) even though it > doesn't work under some configurations. Yes, less than 8 vCPUs in my testing and "bcdedit /set x2apicpolicy enable" to enable x2apic in windows server guests, the windows guests BSOD after reboot. Regards, Wanpeng Li -- 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
