Re: [PATCH 04/14] arm64: Add ARCH_WORKAROUND_2 probing

[Suzuki K Poulose]

On 24/05/18 12:39, Will Deacon wrote:
> On Thu, May 24, 2018 at 10:58:43AM +0100, Suzuki K Poulose wrote:
> > On 22/05/18 16:06, Marc Zyngier wrote:
> > > As for Spectre variant-2, we rely on SMCCC 1.1 to provide the
> > > discovery mechanism for detecting the SSBD mitigation.
> > >
> > > A new capability is also allocated for that purpose, and a
> > > config option.
> > >
> > > Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
> >
> >
> > > +static bool has_ssbd_mitigation(const struct arm64_cpu_capabilities *entry,
> > > +				    int scope)
> > > +{
> > > +	struct arm_smccc_res res;
> > > +	bool supported = true;
> > > +
> > > +	WARN_ON(scope != SCOPE_LOCAL_CPU || preemptible());
> > > +
> > > +	if (psci_ops.smccc_version == SMCCC_VERSION_1_0)
> > > +		return false;
> > > +
> > > +	/*
> > > +	 * The probe function return value is either negative
> > > +	 * (unsupported or mitigated), positive (unaffected), or zero
> > > +	 * (requires mitigation). We only need to do anything in the
> > > +	 * last case.
> > > +	 */
> > > +	switch (psci_ops.conduit) {
> > > +	case PSCI_CONDUIT_HVC:
> > > +		arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
> > > +				  ARM_SMCCC_ARCH_WORKAROUND_2, &res);
> > > +		if ((int)res.a0 != 0)
> > > +			supported = false;
> > > +		break;
> > > +
> > > +	case PSCI_CONDUIT_SMC:
> > > +		arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
> > > +				  ARM_SMCCC_ARCH_WORKAROUND_2, &res);
> > > +		if ((int)res.a0 != 0)
> > > +			supported = false;
> > > +		break;
> > > +
> > > +	default:
> > > +		supported = false;
> > > +	}
> > > +
> > > +	if (supported) {
> > > +		__this_cpu_write(arm64_ssbd_callback_required, 1);
> > > +		do_ssbd(true);
> > > +	}
> >
> >
> > Marc,
> >
> > As discussed, we have minor issue with the "corner case". If a CPU
> > is hotplugged in which requires the mitigation, after the system has
> > finalised the cap to "not available", the CPU could go ahead and
> > do the "work around" as above, while not effectively doing anything
> > about it at runtime for KVM guests (as thats the only place where
> > we rely on the CAP being set).
> >
> > But, yes this is real corner case. There is no easy way to solve it
> > other than
> >
> > 1) Allow late modifications to CPU hwcaps
> >
> > OR
> >
> > 2) Penalise the fastpath to always check per-cpu setting.
>
> Shouldn't we just avoid bring up CPUs that require the mitigation after
> we've finalised the capability to say that it's not required? Assuming this
> is just another issue with maxcpus=, then I don't much care for it.
Ah! Sorry, yes we do kill the CPU. But it is just that it will set the
ssbd_callback_required flag and issue the do_ssbd(), which is not an issue.

Yes this can only be triggered by maxcpus=.

Suzuki