On 30/09/2019 21:17, Julien Grall wrote: > Hi, > > On 9/30/19 7:24 PM, Andrew Cooper wrote: >> The code generation for barrier_nospec_true() is not correct. We are >> taking a >> perf it from the added fences, but not gaining any speculative safety. > > s/it/hit/?
Yes. > >> >> This is caused by inline assembly trying to fight the compiler >> optimiser, and >> the optimiser winning. There is no clear way to achieve safety, so >> turn the >> perf hit off for now. >> >> This also largely reverts 3860d5534df4. The name 'l1tf-barrier', and >> making >> barrier_nospec_true() depend on CONFIG_HVM was constrained by what >> could be >> discussed publicly at the time. Now that MDS is public, neither >> aspects are >> correct. >> >> As l1tf-barrier hasn't been in a release of Xen, and >> CONFIG_SPECULATIVE_BRANCH_HARDEN is disabled until we can find a safe >> way of >> implementing the functionality, remove the l1tf-barrier command line >> option. >> >> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com> >> --- >> CC: Jan Beulich <jbeul...@suse.com> >> CC: Wei Liu <w...@xen.org> >> CC: Roger Pau Monné <roger....@citrix.com> >> CC: Juergen Gross <jgr...@suse.com> >> CC: Norbert Manthey <nmant...@amazon.de> >> --- >> docs/misc/xen-command-line.pandoc | 8 +------- >> xen/arch/x86/spec_ctrl.c | 17 ++--------------- >> xen/common/Kconfig | 17 +++++++++++++++++ > > I think this wanted to have "THE REST" CCed. > >> xen/include/asm-x86/cpufeatures.h | 2 +- >> xen/include/asm-x86/nospec.h | 4 ++-- >> xen/include/asm-x86/spec_ctrl.h | 1 - >> 6 files changed, 23 insertions(+), 26 deletions(-) > > [...] > >> diff --git a/xen/common/Kconfig b/xen/common/Kconfig >> index 9644cc9911..d851e63083 100644 >> --- a/xen/common/Kconfig >> +++ b/xen/common/Kconfig >> @@ -96,6 +96,23 @@ config SPECULATIVE_ARRAY_HARDEN >> If unsure, say Y. >> +config SPECULATIVE_BRANCH_HARDEN >> + bool "Speculative Branch Hardening" >> + depends on BROKEN >> + ---help--- >> + Contemporary processors may use speculative execution as a >> + performance optimisation, but this can potentially be abused >> by an >> + attacker to leak data via speculative sidechannels. >> + >> + One source of misbehaviour is by executing the wrong basic block >> + following a conditional jump. >> + >> + When enabled, specific conditions which have been deemed >> liable to >> + be speculatively abused will be hardened to avoid entering the >> wrong >> + basic block. >> + >> + !!! WARNING - This is broken and doesn't generate safe code !!! > > Any reason to add that in common code when this is x86 only? In principle, its not x86 specific. > My worry is this gate config gate nothing on Arm so the user may have > a false sense that it can be used (even though it is clearly BROKEN). > > Also the name is quite close to the CONFIG_HARDEN_PREDICTOR on Arm and > may confuse user. Although, I don't have a better name so far :/ The "depends on BROKEN" means it will never show up to a user in menuconfig, which is why it was only CC to x86, and not to rest. As for naming, I'm open to suggestions, but this was the best I could come up with. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
