Re: [PATCH 05/37] arm64: Add cpus_have_final_boot_cap()

[Mark Rutland <mark.rutland@arm.com>]

On Thu, Sep 21, 2023 at 10:13:31AM +0100, Suzuki K Poulose wrote:
> Hi Mark
> 
> On 19/09/2023 10:28, Mark Rutland wrote:
> > The cpus_have_final_boot_cap() function can be used to test a cpucap
> 
> nit: cpus_have_final_cap()

Thanks; fixed now.

> > while also verifying that we do not consume the cpucap until system
> > capabilities have been finalized. It would be helpful if we could do
> > likewise for boot cpucaps.
> > 
> > This patch adds a new cpus_have_final_boot_cap() helper which can be
> > used to test a cpucap while also verifying that boot capabilities have
> > been finalized. Users will be added in subsequent patches.
> > 
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Mark Brown <broonie@kernel.org>
> > Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> > Cc: Will Deacon <will@kernel.org>
> > ---
> >   arch/arm64/include/asm/cpufeature.h | 27 +++++++++++++++++++++++++--
> >   1 file changed, 25 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> > index 7d5317bc2429f..e832b86c6b57f 100644
> > --- a/arch/arm64/include/asm/cpufeature.h
> > +++ b/arch/arm64/include/asm/cpufeature.h
> > @@ -438,6 +438,11 @@ unsigned long cpu_get_elf_hwcap2(void);
> >   #define cpu_set_named_feature(name) cpu_set_feature(cpu_feature(name))
> >   #define cpu_have_named_feature(name) cpu_have_feature(cpu_feature(name))
> > +static __always_inline bool boot_capabilities_finalized(void)
> > +{
> > +	return alternative_has_cap_likely(ARM64_ALWAYS_BOOT);
> > +}
> > +
> >   static __always_inline bool system_capabilities_finalized(void)
> >   {
> >   	return alternative_has_cap_likely(ARM64_ALWAYS_SYSTEM);
> > @@ -473,8 +478,26 @@ static __always_inline bool __cpus_have_const_cap(int num)
> >   /*
> >    * Test for a capability without a runtime check.
> >    *
> > - * Before capabilities are finalized, this will BUG().
> > - * After capabilities are finalized, this is patched to avoid a runtime check.
> > + * Before boot capabilities are finalized, this will BUG().
> > + * After boot capabilities are finalized, this is patched to avoid a runtime
> > + * check.
> > + *
> > + * @num must be a compile-time constant.
> > + */
> > +static __always_inline bool cpus_have_final_boot_cap(int num)
> > +{
> > +	if (boot_capabilities_finalized())
> 
> Does this need to make sure the cap is really a "BOOT" cap ? It is a bit of
> an overkill, but prevents users from incorrectly assuming the cap is
> finalised ?

Do you have an idea in mind for how to do that?

I had also wanted that, but we don't have the information available when
compiling the callsites today since that's determined by the
arm64_cpu_capabilities::type flags.

We could us an alternative callback for boot_capabilities_finalized() that
goes and checks the arm64_cpu_capabilities::type flags, but that doesn't seem
very nice.

Otherwise, given this only has a few users, I could have those directly use:

	BUG_ON(!boot_capabilities_finalized());

... and remove cpus_have_final_boot_cap() for now?

Thanks,
Mark.

> 
> 
> Suzuki
> 
> > +		return __cpus_have_const_cap(num);
> > +	else
> > +		BUG();
> > +}
> > +
> > +/*
> > + * Test for a capability without a runtime check.
> > + *
> > + * Before system capabilities are finalized, this will BUG().
> > + * After system capabilities are finalized, this is patched to avoid a runtime
> > + * check.
> >    *
> >    * @num must be a compile-time constant.
> >    */
> 

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