On Oct 1, 2014, at 9:27 PM, Radim Krčmář <rkrcmar@xxxxxxxxxx> wrote: > 2014-10-01 20:30+0300, Nadav Amit: >> On Oct 1, 2014, at 7:04 PM, Radim Krčmář <rkrcmar@xxxxxxxxxx> wrote: >>> 2014-09-30 20:49+0300, Nadav Amit: >> The condition “!new->cid_mask” is certainly always true (unless we already set the cid/lid according to cluster-mode settings). > > Exactly, it "optimizes" the switch to a non-default clustered mode. > >> 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. > > Our assumption that all have the same mode is horrible. > (Do they all have be the same?) Yes: "All processors that have their APIC software enabled (using the spurious vector enable/disable bit) must have their DFRs (Destination Format Registers) programmed identically." > > The only thing we allow out of your scenario that I can see is software > disabled x2apic after enabled clustered xapic processors and that > doesn't need two loops, just a sw check at x2apic. > Practically, it is a harmless bug :) So does xsa-108... ;-) Now seriously: First, the bug may affect certain cases of cpu hot-plug, etc. Second, there are additional implications. Consider a situation in which the first VCPUs have lapic disabled, and they do not have the same DFR/x2apic mode as the rest of the VCPUs. This is ok according to the SDM, but in such case, the logical map they would have would not match the spic-mode. Therefore, they may not receive NMIs, INIT, etc. - which they should regardless to the fact their LAPIC is disabled. Regards, Nadav
Attachment:
signature.asc
Description: Message signed with OpenPGP using GPGMail
