Re: [PATCH v4 07/30] xen/asm-generic: introdure nospec.h

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Wed, 2024-02-21 at 12:00 +0100, Jan Beulich wrote:
> On 20.02.2024 21:30, Oleksii wrote:
> > On Mon, 2024-02-19 at 13:18 +0100, Jan Beulich wrote:
> > > On 19.02.2024 12:59, Oleksii wrote:
> > > > Hi Julien,
> > > > 
> > > > On Sun, 2024-02-18 at 18:30 +0000, Julien Grall wrote:
> > > > > Hi Oleksii,
> > > > > 
> > > > > Title: Typo s/introdure/introduce/
> > > > > 
> > > > > On 05/02/2024 15:32, Oleksii Kurochko wrote:
> > > > > > The <asm/nospec.h> header is similar between Arm, PPC, and
> > > > > > RISC-V,
> > > > > > so it has been moved to asm-generic.
> > > > > 
> > > > > I am not 100% convinced that moving this header to asm-
> > > > > generic is
> > > > > a
> > > > > good 
> > > > > idea. At least for Arm, those helpers ought to be non-empty,
> > > > > what
> > > > > about 
> > > > > RISC-V?
> > > > For Arm, they are not taking any action, are they? There are no
> > > > specific fences or other mechanisms inside
> > > > evaluate_nospec()/block_speculation() to address speculation.
> > > 
> > > The question isn't the status quo, but how things should be
> > > looking
> > > like
> > > if everything was in place that's (in principle) needed.
> > > 
> > > > For RISC-V, it can be implemented in a similar manner, at least
> > > > for
> > > > now. Since these functions are only used in the grant tables
> > > > code (
> > > > for
> > > > Arm and so for RISC-V ), which is not supported by RISC-V.
> > > 
> > > Same here - the question is whether long term, when gnttab is
> > > also
> > > supported, RISC-V would get away without doing anything. Still
> > > ...
> > > 
> > > > > If the answer is they should be non-empty. Then I would
> > > > > consider
> > > > > to
> > > > > keep 
> > > > > the duplication to make clear that each architecture should
> > > > > take
> > > > > their 
> > > > > own decision in term of security.
> > > > > 
> > > > > The alternative, is to have a generic implementation that is
> > > > > safe
> > > > > by 
> > > > > default (if that's even possible).
> > > > I am not certain that we can have a generic implementation, as
> > > > each
> > > > architecture may have specific speculation issues.
> > > 
> > > ... it's theoretically possible that there'd be an arch with no
> > > speculation issues, maybe simply because of not speculating.
> > 
> > I am not sure that understand your and Julien point.
> > 
> > For example, modern CPU uses speculative execution to reduce the
> > cost
> > of conditional branch instructions using schemes that predict the
> > execution path of a program based on the history of branch
> > executions.
> > 
> > Arm CPUs are vulnerable for speculative execution, but if to look
> > at
> > the code of evaluate_nospec()/block_speculation() functions they
> > are
> > doing nothing for Arm.
> 
> Which, as I understood Julien say, likely isn't correct. In which
> case
> this header shouldn't be dropped, using the generic one instead. The
> generic headers, as pointed out several times before, should imo be
> used
> only if their use results in correct behavior. What is acceptable is
> if
> their use results in sub-optimal behavior (e.g. reduced performance
> or
> lack of a certain feature that another architecture maybe
> implements).

As I understand it, evaluate_nospec()/block_speculation() were
introduced for x86 to address the L1TF vulnerability specific to x86
CPUs. This vulnerability is exclusive to x86 architectures [1], which
explains why evaluate_nospec()/block_speculation() are left empty for
Arm, RISC-V, and PPC.

It is unclear whether these functions should be utilized to mitigate
other speculation vulnerabilities. If they should, then, based on the
current implementation, the Arm platform seems to accept having
speculative vulnerabilities.

The question arises: why can't other architectures make their own
decisions regarding security? By default, if an architecture leaves the
mentioned functions empty, it implies an agreement to potentially have
speculative vulnerabilities. Subsequently, if an architecture needs to
address such vulnerabilities, they can develop arch-specific nospec.h
to implement the required code.

If reaching an agreement to potentially have speculative
vulnerabilities is deemed unfavorable, I am open to reconsidering these
changes and reverting to the use of arch-specific nospec.h.
Your input on this matter is appreciated.

[1] https://docs.kernel.org/admin-guide/hw-vuln/l1tf.html