On Oct 1, 2014, at 7:04 PM, Radim Krčmář <rkrcmar@xxxxxxxxxx> wrote: > 2014-09-30 20:49+0300, Nadav Amit: >> Determining flat mode according to cid_mask is wrong, since currently KVM >> supports zero clusters in x2apic mode. Use ldr_bits instead. > > No, it is in else branch of 'if (apic_x2apic_mode(apic))', so it works > as intended now, and ldr_bits == 8 is always true. The condition “!new->cid_mask” is certainly always true (unless we already set the cid/lid according to cluster-mode settings). I did not feel comfortable removing it without replacing it with something “equivalent”. > >> Since we recalculate the apic map whenever the DFR is changed, the bug appears >> to have no effect, and perhaps the entire check can be removed. > > It could, for the check is just an optimization. > (This whole code deserves a rewrite though ...) I did not hit such bug, but IMO the code is buggy. The APIC mode is determined by processors that enabled their apic enabled (in the spurious vector), and all the enabled one should configure the same mode. Nonetheless, as stated in the SDM 10.4.7.2 "Local APIC State After It Has Been Software Disabled APICs” - the disabled APICs should still respond to NMIs, INIT, etc. So it appears the loop should be broken into two loops - first determine the apic mode, according to the first enabled APIC. Then, iterate again over vCPUs and build the logical map. Regards, Nadav
Attachment:
signature.asc
Description: Message signed with OpenPGP using GPGMail
