Re: [PATCH 02/14] arm64: Call ARCH_WORKAROUND_2 on transitions between EL0 and EL1

[Mark Rutland]

On Tue, May 22, 2018 at 04:06:36PM +0100, Marc Zyngier wrote:
> In order for the kernel to protect itself, let's call the SSBD mitigation
> implemented by the higher exception level (either hypervisor or firmware)
> on each transition between userspace and kernel.
> 
> We must take the PSCI conduit into account in order to target the
> right exception level, hence the introduction of a runtime patching
> callback.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
> ---
>  arch/arm64/kernel/cpu_errata.c | 18 ++++++++++++++++++
>  arch/arm64/kernel/entry.S      | 22 ++++++++++++++++++++++
>  include/linux/arm-smccc.h      |  5 +++++
>  3 files changed, 45 insertions(+)
> 
> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> index a900befadfe8..46b3aafb631a 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c
> @@ -232,6 +232,24 @@ enable_smccc_arch_workaround_1(const struct arm64_cpu_capabilities *entry)
>  }
>  #endif	/* CONFIG_HARDEN_BRANCH_PREDICTOR */
>  
> +#ifdef CONFIG_ARM64_SSBD
> +void __init arm64_update_smccc_conduit(struct alt_instr *alt,
> +				       __le32 *origptr, __le32 *updptr,
> +				       int nr_inst)
> +{
> +	u32 insn;
> +
> +	BUG_ON(nr_inst != 1);
> +
> +	if (psci_ops.conduit == PSCI_CONDUIT_HVC)
> +		insn = aarch64_insn_get_hvc_value();
> +	else
> +		insn = aarch64_insn_get_smc_value();

Shouldn't this also handle the case where there is no conduit?

See below comment in apply_ssbd for rationale.

> +
> +	*updptr = cpu_to_le32(insn);
> +}
> +#endif	/* CONFIG_ARM64_SSBD */
> +
>  #define CAP_MIDR_RANGE(model, v_min, r_min, v_max, r_max)	\
>  	.matches = is_affected_midr_range,			\
>  	.midr_range = MIDR_RANGE(model, v_min, r_min, v_max, r_max)
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index ec2ee720e33e..f33e6aed3037 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -18,6 +18,7 @@
>   * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> +#include <linux/arm-smccc.h>
>  #include <linux/init.h>
>  #include <linux/linkage.h>
>  
> @@ -137,6 +138,18 @@ alternative_else_nop_endif
>  	add	\dst, \dst, #(\sym - .entry.tramp.text)
>  	.endm
>  
> +	// This macro corrupts x0-x3. It is the caller's duty
> +	// to save/restore them if required.
> +	.macro	apply_ssbd, state
> +#ifdef CONFIG_ARM64_SSBD
> +	mov	w0, #ARM_SMCCC_ARCH_WORKAROUND_2
> +	mov	w1, #\state
> +alternative_cb	arm64_update_smccc_conduit
> +	nop					// Patched to SMC/HVC #0
> +alternative_cb_end
> +#endif
> +	.endm

If my system doesn't have SMCCC1.1, or the FW doesn't have an
implementation of ARCH_WORKAROUND_2, does this stay as a NOP?

It looks like this would be patched to an SMC, which would be fatal on
systems without EL3 FW.

> +
>  	.macro	kernel_entry, el, regsize = 64
>  	.if	\regsize == 32
>  	mov	w0, w0				// zero upper 32 bits of x0
> @@ -163,6 +176,13 @@ alternative_else_nop_endif
>  	ldr	x19, [tsk, #TSK_TI_FLAGS]	// since we can unmask debug
>  	disable_step_tsk x19, x20		// exceptions when scheduling.
>  
> +	apply_ssbd 1


... and thus kernel_entry would be fatal.

Thanks,
Mark.