[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YmwZYEGtJn3qs0j4@zn.tnic>
Date: Fri, 29 Apr 2022 18:59:12 +0200
From: Borislav Petkov <bp@...en8.de>
To: Jon Kohler <jon@...anix.com>
Cc: Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>,
Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
"H. Peter Anvin" <hpa@...or.com>,
Paolo Bonzini <pbonzini@...hat.com>,
Sean Christopherson <seanjc@...gle.com>,
Vitaly Kuznetsov <vkuznets@...hat.com>,
Wanpeng Li <wanpengli@...cent.com>,
Jim Mattson <jmattson@...gle.com>,
Joerg Roedel <joro@...tes.org>,
Josh Poimboeuf <jpoimboe@...hat.com>,
Peter Zijlstra <peterz@...radead.org>,
Balbir Singh <sblbir@...zon.com>,
Kim Phillips <kim.phillips@....com>,
linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
Andrea Arcangeli <aarcange@...hat.com>,
Kees Cook <keescook@...omium.org>,
Waiman Long <longman@...hat.com>
Subject: Re: [PATCH v3] x86/speculation, KVM: only IBPB for
switch_mm_always_ibpb on vCPU load
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
Powered by blists - more mailing lists