[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] x86/apic: fix disabling LVT0 in disconnect_bsp_APIC
On Fri, Jan 17, 2020 at 05:25:12PM +0100, Jan Beulich wrote: > On 17.01.2020 17:08, Roger Pau Monné wrote: > > On Fri, Jan 17, 2020 at 04:56:00PM +0100, Jan Beulich wrote: > >> On 17.01.2020 16:09, Roger Pau Monne wrote: > >>> The Intel SDM states: > >>> > >>> "When an illegal vector value (0 to 15) is written to a LVT entry and > >>> the delivery mode is Fixed (bits 8-11 equal 0), the APIC may signal an > >>> illegal vector error, without regard to whether the mask bit is set or > >>> whether an interrupt is actually seen on the input." > >>> > >>> And that's exactly what's currently done in disconnect_bsp_APIC when > >>> virt_wire_setup is true and LVT LINT0 is being masked. By writing only > >>> APIC_LVT_MASKED Xen is actually setting the vector to 0 and the > >>> delivery mode to Fixed (0), and hence it triggers an APIC error even > >>> when the LVT entry is masked. > >> > >> But there are many more instances where we (have a risk to) do so, > >> most notably in clear_local_APIC(). The two step logic there is > >> anyway somewhat in conflict with the citation above. > > > > clear_local_APIC masks the error vector before doing any write, and > > clears ESR afterwards, there's a comment at the top: > > > > "Masking an LVT entry on a P6 can trigger a local APIC error > > if the vector is zero. Mask LVTERR first to prevent this." > > > > We could do the same (ie: mask LVTERR first and clear ESR afterwards) > > if that seems preferable. There's a maxlvt check in clear_local_APIC, > > but the sdm doesn't specify anyway to check if the lapic will accept a > > masked vector 0 write or not, so not sure whether we should replicate > > that check or just do it unconditionally on both disconnect_bsp_APIC > > and clear_local_APIC. > > I think doing it the most careful way is going to be best. I find it > surprising anyway that disconnect_bsp_APIC() doesn't write LVTERR > (or other LVTs except for LVT1) at all. The function looks to have a > goal of putting the APIC back into the state that we found it when > booting. Maybe it would be better to just call clear_local_APIC before trying to configure LVT{0/1}, which will leave LVT0 in a reset state and thus no write would be required in the !virt_wire_setup case? Roger. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |