Andrew Cooper <andrew.cooper3@xxxxxxxxxx> writes: > On 04/06/15 07:38, H. Peter Anvin wrote: >> On 06/03/2015 02:31 AM, Andrew Cooper wrote: >>> There appears to be no formal statement of what pv_irq_ops.save_fl() is >>> supposed to return precisely. Native returns the full flags, while lguest and >>> Xen only return the Interrupt Flag, and both have comments by the >>> implementations stating that only the Interrupt Flag is looked at. This may >>> have been true when initially implemented, but no longer is. >>> >>> To make matters worse, the Xen PVOP leaves the upper bits undefined, making >>> the BUG_ON() undefined behaviour. Experimentally, this now trips for 32bit PV >>> guests on Broadwell hardware. The BUG_ON() is consistent for an individual >>> build, but not consistent for all builds. It has also been a sitting timebomb >>> since SMAP support was introduced. >>> >>> Use native_save_fl() instead, which will obtain an accurate view of the AC >>> flag. >> Could we fix the Xen pvops wrapper instead to not do things like this? >> >> -hpa >> >> > > We could, and I have a patch for that, but the check would still then be > broken in lguest, and it makes a hotpath rather longer. > > Either pv_irq_ops.save_fl() gets defined to handle all flags, and Xen & > lguest need correcting in this regard, or save_fl() gets defined to > handle the interrupt flag only, and this becomes the single problematic > caller in the codebase. > > The problem with expanding save_fl() to handle all flags is that > restore_fl() should follow suit, and there are a number of system flags > are inapplicable in such a situation. Yeah, hard cases make bad law. I'm not too unhappy with this fix; ideally we'd rename save_fl and restore_fl to save_eflags_if and restore_eflags_if too. Cheers, Rusty. -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html