Re: [PATCH v2 2/6] arm64: alternative: Apply alternatives early in boot process

[Suzuki K Poulose]

On 11/05/18 09:12, Julien Thierry wrote:
> On 09/05/18 22:52, Suzuki K Poulose wrote:
> > On 05/04/2018 11:06 AM, Julien Thierry wrote:
> > > Hi,
> > >
> > > In order to prepare the v3 of this patchset, I'd like people's opinion on what this patch does. More below.
> > >
> > > On 17/01/18 11:54, Julien Thierry wrote:
> > > > From: Daniel Thompson <daniel.thompson@xxxxxxxxxx>
> > > >
> > > > Currently alternatives are applied very late in the boot process (and
> > > > a long time after we enable scheduling). Some alternative sequences,
> > > > such as those that alter the way CPU context is stored, must be applied
> > > > much earlier in the boot sequence.
> >
> > > > +/*
> > > > + * early-apply features are detected using only the boot CPU and checked on
> > > > + * secondary CPUs startup, even then,
> > > > + * These early-apply features should only include features where we must
> > > > + * patch the kernel very early in the boot process.
> > > > + *
> > > > + * Note that the cpufeature logic *must* be made aware of early-apply
> > > > + * features to ensure they are reported as enabled without waiting
> > > > + * for other CPUs to boot.
> > > > + */
> > > > +#define EARLY_APPLY_FEATURE_MASK BIT(ARM64_HAS_SYSREG_GIC_CPUIF)
> > > > +
> > >
> > > Following the change in the cpufeature infrastructure, ARM64_HAS_SYSREG_GIC_CPUIF will have the scope ARM64_CPUCAP_SCOPE_BOOT_CPU in order to be checked early in the boot process.
> >
> > Thats correct.
> >
> > > Now, regarding the early application of alternative, I am wondering whether we can apply all the alternatives associated with SCOPE_BOOT features that *do not* have a cpu_enable callback.
> >
> > I don't understand why would you skip the ones that have a "cpu_enable" callback. Could you explain this a bit ? Ideally you should be able to
> > apply the alternatives for features with the SCOPE_BOOT, provided the
> > cpu_enable() callback is written properly.
>
> In my mind the "cpu_enable" callback is the setup a cpu should perform before using the feature (i.e. the code getting patched in by the alternative). So I was worried about the code getting patched by the boot cpu and then have the secondary cpus ending up executing patched code before the cpu_enable for the corresponding feature gets called.
> Or is there a requirement for secondary cpu startup code to be free of alternative code?

There are no imposed restrictions. It is upto the capability to decide
what can be done in cpu_enable() and what can be patched. So, if you
make sure the patched code can be safely executed by secondary it is
fine. May be you could even patch in some code in the early boot up, to
do what you do in "cpu_enable()" for the secondary to safely execute
the patched code.

Anyway, if the secondary CPUs don't have the feature you are going to
panic the system. So I don't think there is a big difference in the
outcome if there is a mismatch, except for a clean message about the
conflict.


Cheers
Suzuki