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

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

[Rich Wiley]

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")


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));
+	isb();
+}
+
 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

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

[Mark Rutland]

[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

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

[James Morse]

Hi Rich,

On 05/10/2022 19:16, Mark Rutland wrote:
> [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.

While you might not need firmware to do the mitigation, supporting this has the advantage
that any OS that doesn't know about your imp-def register (or can't access it), it can
still use the SMC for mitigation.

Do you have a firmware version that does this predictor flush? Doing this will cover the
existing distro kernels and KVM guests without rebuilding them, so it should be the first
step.
After that, there may be a performance improvement to be gained by writing to this
register directly, but for the reasons Mark describes - it won't be possible in all
circumstances.

(I assume these parts don't support nested-virt)


>> 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?

I think you just want a cc-stable for this.


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

>> 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.

KVM calls the SMC on CPUs where it is needed on every guest exit, and then returns to the
guest quickly if that is all the guest wanted.


Thanks,

James

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