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

Mark Rutland
2022-10-05T18:17:46Z

[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