On 23.10.2019 15:58, Andrew Cooper wrote: > evaluate_nospec() is incredibly fragile, and this is one giant bodge. > > To correctly protect jumps, the generated code needs to be of the form: > > cmp/test <cond> > jcc 1f > lfence > ... > 1: lfence > ... > > Critically, the lfence must be at the head of both basic blocks, later in the > instruction stream than the conditional jump in need of protection. > > When a static inline is involved, the optimiser decides to be clever and > rearranges the code as: > > pred: > lfence > <calculate cond> > ret > > call pred > cmp $0, %eax > jcc 1f > ... > 1: ... > > which breaks the speculative safety.
Aiui "pred" is a non-inlined static inline here. There's no "optimiser decides to be clever" in this case imo - it all is a result of not inlining, when the construct in evaluate_nospec() is specifically assuming this wouldn't happen. Therefore I continue to think that the description is misleading. > Any use of evaluate_nospec() needs all static inline predicates which use it > to be declared always_inline to prevent the optimiser having the flexibility > to generate unsafe code. I agree with this part. > Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com> > --- > CC: George Dunlap <george.dun...@eu.citrix.com> > CC: Ian Jackson <ian.jack...@citrix.com> > CC: Jan Beulich <jbeul...@suse.com> > CC: Konrad Rzeszutek Wilk <konrad.w...@oracle.com> > CC: Stefano Stabellini <sstabell...@kernel.org> > CC: Wei Liu <w...@xen.org> > CC: Julien Grall <jul...@xen.org> > CC: Juergen Gross <jgr...@suse.com> > > This is the transitive set of predicates which I can spot which need > protecting. There are probably ones I've missed. Personally, I'm -1 for this > approach, but the only other option for 4.13 is to revert it all to unbreak > livepatching. To unbreak livepatching, aiui what you need is symbol disambiguation, a patch for which has been sent. With this I think we should focus on code generation aspects here. I'm fine ack-ing the code changes with a modified description. But since you're -1 for this, I'm not sure in the first place that we want to go this route. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
