| Message ID | 83c2a504-bce4-d3e7-1d9a-76ac0ca17bab@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/spec-ctrl: IBPB improvements | expand |
On Tue, Feb 14, 2023 at 05:11:05PM +0100, Jan Beulich wrote: > In order to avoid clobbering Xen's own predictions, defer the barrier as > much as possible. Merely mark the CPU as needing a barrier issued the > next time we're exiting to guest context. While I understand that doing the flush in the middle of the guest context might not be ideal, as it's my understanding we also needlessly flush Xen predictions, I'm unsure whether this makes any difference in practice, and IMO just makes the exit to guest paths more complex. Thanks, Roger.
On 18.12.2023 13:39, Roger Pau Monné wrote: > On Tue, Feb 14, 2023 at 05:11:05PM +0100, Jan Beulich wrote: >> In order to avoid clobbering Xen's own predictions, defer the barrier as >> much as possible. Merely mark the CPU as needing a barrier issued the >> next time we're exiting to guest context. > > While I understand that doing the flush in the middle of the guest > context might not be ideal, as it's my understanding we also s/guest context/context switch/? > needlessly flush Xen predictions, I'm unsure whether this makes any > difference in practice, and IMO just makes the exit to guest paths > more complex. I need to redirect this question to Andrew, who suggested that doing so can be expected to make a difference. When we were discussing this, I could easily see it might make a difference, but I cannot provide hard proof. Jan
On Mon, Dec 18, 2023 at 02:58:01PM +0100, Jan Beulich wrote: > On 18.12.2023 13:39, Roger Pau Monné wrote: > > On Tue, Feb 14, 2023 at 05:11:05PM +0100, Jan Beulich wrote: > >> In order to avoid clobbering Xen's own predictions, defer the barrier as > >> much as possible. Merely mark the CPU as needing a barrier issued the > >> next time we're exiting to guest context. > > > > While I understand that doing the flush in the middle of the guest > > context might not be ideal, as it's my understanding we also > > s/guest context/context switch/? Indeed, sorry. > > needlessly flush Xen predictions, I'm unsure whether this makes any > > difference in practice, and IMO just makes the exit to guest paths > > more complex. > > I need to redirect this question to Andrew, who suggested that doing so > can be expected to make a difference. When we were discussing this, I > could easily see it might make a difference, but I cannot provide hard > proof. That's fine, but with the added complexity in the return to guests paths I think we need some kind of justification for such a change. If it was the other way around I could easily see it as a benefits if just for code clarity reasons. Thanks, Roger.
--- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -2038,7 +2038,7 @@ void context_switch(struct vcpu *prev, s */ if ( *last_id != next_id ) { - spec_ctrl_new_guest_context(); + info->spec_ctrl_flags |= SCF_new_pred_ctxt; *last_id = next_id; } } --- a/xen/arch/x86/include/asm/spec_ctrl.h +++ b/xen/arch/x86/include/asm/spec_ctrl.h @@ -67,28 +67,6 @@ void init_speculation_mitigations(void); void spec_ctrl_init_domain(struct domain *d); -/* - * Switch to a new guest prediction context. - * - * This flushes all indirect branch predictors (BTB, RSB/RAS), so guest code - * which has previously run on this CPU can't attack subsequent guest code. - * - * As this flushes the RSB/RAS, it destroys the predictions of the calling - * context. For best performace, arrange for this to be used when we're going - * to jump out of the current context, e.g. with reset_stack_and_jump(). - * - * For hardware which mis-implements IBPB, fix up by flushing the RSB/RAS - * manually. - */ -static always_inline void spec_ctrl_new_guest_context(void) -{ - wrmsrl(MSR_PRED_CMD, PRED_CMD_IBPB); - - /* (ab)use alternative_input() to specify clobbers. */ - alternative_input("", "DO_OVERWRITE_RSB", X86_BUG_IBPB_NO_RET, - : "rax", "rcx"); -} - extern int8_t opt_ibpb_ctxt_switch; extern bool opt_ssbd; extern int8_t opt_eager_fpu; --- a/xen/arch/x86/spec_ctrl.c +++ b/xen/arch/x86/spec_ctrl.c @@ -854,6 +854,11 @@ static void __init ibpb_calculations(voi */ if ( opt_ibpb_ctxt_switch == -1 ) opt_ibpb_ctxt_switch = !(opt_ibpb_entry_hvm && opt_ibpb_entry_pv); + if ( opt_ibpb_ctxt_switch ) + { + setup_force_cpu_cap(X86_FEATURE_NEW_PRED_CTXT_PV); + setup_force_cpu_cap(X86_FEATURE_NEW_PRED_CTXT_HVM); + } } /* Calculate whether this CPU is vulnerable to L1TF. */
In order to avoid clobbering Xen's own predictions, defer the barrier as much as possible. Merely mark the CPU as needing a barrier issued the next time we're exiting to guest context. Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> --- I couldn't find any sensible (central/unique) place where to move the comment which is being deleted alongside spec_ctrl_new_guest_context(). (If this patch is to survive in the first place, it was suggested to move to spect_ctrl_asm.h, next to the #define of the controlling bit.) --- v4: Re-base in particular over changes earlier in the series. v3: New.