Re: [PATCH v2] kernel: arm64: add Spectre-V2 mitigation cb for NV CPUs

[Mark Rutland <mark.rutland@arm.com>]

[adding James Morse, Marc Zyngier]

On Wed, Oct 05, 2022 at 10:53:19AM -0700, Rich Wiley wrote:
> NVIDIA cores can flush indirect branch predictors with a sysreg write
> that can be done from the kernel without the need for a FW call.
> 
> Given that NVIDIA cores are not susceptible to CVE-2022-23960 and FW
> mitigation is not required for CVE-2017-5715, ATF can report via
> SMCCC_ARCH_WORKAROUND_3 that FW mitigation is not required for either.
> 
> Fixes: commit ba2689234be9 ("arm64: entry: Add vectors that have the bhb
> mitigation sequences")

The proper format for this is:

| Fixes: ba2689234be9 ("arm64: entry: Add vectors that have the bhb mitigation sequences")

... though given you say NVIDIA cores are not susceptible to CVE-2022-23960
(aka spectre-BHB), I don't think that tag is correct, and I don't think you
need a fixes tag at all, unless you're seeing an actual problem. If you are,
can you please describe that in the commit message?

The patch says 'v2'. Do you have a link to v1, and a description of any changes
since then?

> 
> Signed-off-by: Rich Wiley <rwiley@nvidia.com>
> ---
>  arch/arm64/kernel/proton-pack.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/kernel/proton-pack.c b/arch/arm64/kernel/proton-pack.c
> index 40be3a7c2c53..0de77b0ff8d4 100644
> --- a/arch/arm64/kernel/proton-pack.c
> +++ b/arch/arm64/kernel/proton-pack.c
> @@ -258,14 +258,26 @@ static noinstr void qcom_link_stack_sanitisation(void)
>  		     : "=&r" (tmp));
>  }
>  
> +/* Called during entry so must be noinstr */
> +static noinstr void nvidia_indirect_branch_pred_flush(void)
> +{
> +	asm volatile("msr s3_0_c15_c0_6, %0" :: "r" (0x1UL));

Please use write_sysreg() or write_sysreg_s() for this.

> +	isb();
> +}

This is an IMPLEMENTATION DEFINED register, so the usual problems apply here:

* This is IMPLEMENTATION DEFINED, and usually IMP-DEF features are subject to
  EL3 access controls.

  Are there any EL3 access controls, and is it possible that this will trap to
  EL3 on some firmware?

* This is IMPLEMENTATION DEFINED, and hypervisors normally trap-end-emulate
  IMP-DEF features as UNDEFINED. So this is not going to be safe in a VM, and
  we cannot use this when running booted at EL1.

  Have you tested this with a VM?

  Will KVM expose a usabel interface to the guest (e.g. the SMCCC mitigation,
  backing that with the MSR at EL2)? I don't see any KVM plumbing for that, so
  I suspect not.

Thanks,
Mark.

> +
>  static bp_hardening_cb_t spectre_v2_get_sw_mitigation_cb(void)
>  {
>  	u32 midr = read_cpuid_id();
> -	if (((midr & MIDR_CPU_MODEL_MASK) != MIDR_QCOM_FALKOR) &&
> -	    ((midr & MIDR_CPU_MODEL_MASK) != MIDR_QCOM_FALKOR_V1))
> -		return NULL;
> +	if (((midr & MIDR_CPU_MODEL_MASK) == MIDR_QCOM_FALKOR) ||
> +	    ((midr & MIDR_CPU_MODEL_MASK) == MIDR_QCOM_FALKOR_V1))
> +		return qcom_link_stack_sanitisation;
> +
> +	if (((midr & MIDR_CPU_MODEL_MASK) == MIDR_NVIDIA_DENVER) ||
> +	    ((midr & MIDR_CPU_MODEL_MASK) == MIDR_NVIDIA_CARMEL))
> +		return nvidia_indirect_branch_pred_flush;
> +
> +	return NULL;
>  
> -	return qcom_link_stack_sanitisation;
>  }
>  
>  static enum mitigation_state spectre_v2_enable_fw_mitigation(void)
> -- 
> 2.17.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel