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

[Marc Zyngier <marc.zyngier@arm.com>]

On 24/05/18 12:23, Mark Rutland wrote:
> On Thu, May 24, 2018 at 12:00:58PM +0100, Mark Rutland wrote:
>> 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@arm.com>
>>> ---
>>>  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?
> 
> Due to the config symbol not being defined yet, and various other fixups
> in later patches, this is actually benign.
> 
> However, if you make this:
> 
> 	switch (psci_ops.conduit) {
> 	case PSCI_CONDUIT_NONE:
> 		return;
> 	case PSCI_CONDUIT_HVC:
> 		insn = aarch64_insn_get_hvc_value();
> 		break;
> 	case PSCI_CONDUIT_SMC:
> 		insn = aarch64_insn_get_smc_value();
> 		break;
> 	}
> 
> ... then we won't even bother patching the nop in the default case
> regardless, which is nicer, IMO.

Yup, looks better to me too. I'll fold that in.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...