Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 2 Sep 2021 17:07:06 -0700
From:   Josh Poimboeuf <jpoimboe@redhat.com>
To:     Babu Moger <babu.moger@amd.com>
Cc:     bp@alien8.de, bsd@redhat.com, corbet@lwn.net, hpa@zytor.com,
        linux-kernel@vger.kernel.org, mingo@redhat.com, tglx@linutronix.de,
        x86@kernel.org
Subject: Re: [v6 1/1] x86/bugs: Implement mitigation for Predictive Store
Message-ID: <20210903000706.fb43tzhjanyg64cx@treble>
References: <20210812234440.tcssf2iqs435bgdo@treble>
 <20210902181637.244879-1-babu.moger@amd.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20210902181637.244879-1-babu.moger@amd.com>
Precedence: bulk

On Thu, Sep 02, 2021 at 01:16:37PM -0500, Babu Moger wrote:
> I dont have this thread in my mailbox. Replying via git send-email.
> 
> Josh,
>    I took care of all your comments except this one below.
> 
> >I'd also recommend an "auto" option:
>    >
>    >	{ "auto",	PREDICTIVE_STORE_FORWARD_CMD_AUTO },    /* Platform decides */
> 
> >   which would be the default.  For now it would function the same as
> >"off", but would give room for tweaking defaults later.
> 
> There is no plan for tweaking this option in the near future.  I feel
> adding 'auto' option now is probably overkill and confusing.

But if the PSF disable interface is modeled after SSB disable (which I
believe it needs to be) then it's only logical to mirror SSB's default
"auto" option.

And, I actually think that calling it 'psf_disable=off' is *more*
confusing than 'psf_disable=auto'.  Because in this case, 'off' doesn't
actually mean "off".  It means

  "off, unless !ssb_disable=off, in which case implicitly mirror the SSB policy".

So maybe there shouldn't even be an 'psf_disable=off' option, because
it's not possible to ensure that PSF doesn't get disabled by SSB
disable.

BTW, is the list of PSF-affected CPUs the same as the list of
SSB-affected CPUs?  If there might be PSF CPUs which don't have SSB,
then more logic will need to be added to ensure a sensible default.

On a related note, is there a realistic, non-hypothetical need to have
separate policies and cmdline options for both SSB and PSF?  i.e. is
there a real-world scenario where a user needs to disable PSF while
leaving SSB enabled?

Because trying to give them separate interfaces, when PSF disable is
intertwined with SSB disable in hardware, is awkward and confusing.  And
the idea of adding another double-negative interface (disable=off!),
just because a vulnerability is considered to be a CPU "feature", isn't
very appetizing.

So instead of adding a new double-negative interface, which only *half*
works due to the ssb_disable dependency, and which is guaranteed to
further confuse users, and which not even be used in the real world
except possibly by confused users...

I'm wondering if we can just start out with the simplest possible
approach: don't change any code and instead just document the fact that
"spec_store_bypass_disable=" also affects PSF.

Then, later on, if a real-world need is demonstrated, actual code could
be added to support disabling PSF independently (but of course it would
never be fully independent since PSF disable is forced by SSB disable).

-- 
Josh