On Fri, Apr 22, 2022 at 12:21:01PM -0400, Jon Kohler wrote:
> Both switch_mm_always_ibpb and switch_mm_cond_ibpb are handled by
> switch_mm() -> cond_mitigation(), which works well in cases where
> switching vCPUs (i.e. switching tasks) also switches mm_struct;
> however, this misses a paranoid case where user space may be running
> multiple guests in a single process (i.e. single mm_struct).
You lost me here. I admit I'm no virt guy so you'll have to explain in
more detail what use case that is that you want to support.
What guests share mm_struct?
What is the paranoid aspect here? You want to protect a single guest
from all the others sharing a mm_struct?
> +/*
> + * Issue IBPB when switching guest vCPUs IFF switch_mm_always_ibpb.
> + * For the more common case of running VMs in their own dedicated process,
> + * switching vCPUs that belong to different VMs, i.e. switching tasks,
> + * will also switch mm_structs and thus do IPBP via cond_mitigation();
> + * however, in the always_ibpb case, take a paranoid approach and issue
> + * IBPB on both switch_mm() and vCPU switch.
> + */
> +static inline void x86_virt_guest_switch_ibpb(void)
> +{
> + if (static_branch_unlikely(&switch_mm_always_ibpb))
> + indirect_branch_prediction_barrier();
If this switch is going to be conditional, then make it so:
static void x86_do_cond_ibpb(void)
{
if (static_branch_likely(&switch_mm_cond_ibpb))
indirect_branch_prediction_barrier();
}
and there's nothing "virt" about it - might as well call the function
what it does. And I'd put that function in bugs.c...
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index 6296e1ebed1d..6aafb0279cbc 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -68,8 +68,12 @@ u64 __ro_after_init x86_amd_ls_cfg_ssbd_mask;
> DEFINE_STATIC_KEY_FALSE(switch_to_cond_stibp);
> /* Control conditional IBPB in switch_mm() */
> DEFINE_STATIC_KEY_FALSE(switch_mm_cond_ibpb);
> -/* Control unconditional IBPB in switch_mm() */
> +/* Control unconditional IBPB in both switch_mm() and
> + * x86_virt_guest_switch_ibpb().
> + * See notes on x86_virt_guest_switch_ibpb() for KVM use case details.
> + */
> DEFINE_STATIC_KEY_FALSE(switch_mm_always_ibpb);
> +EXPORT_SYMBOL_GPL(switch_mm_always_ibpb);
... so that I don't export that key to modules.
I'd like to have the big picture clarified first, why we need this, etc.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
