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

From: Mark Rutland <mark.rutland@arm.com>
To: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: linux-arm-kernel@lists.infradead.org, ardb@kernel.org,
	bertrand.marquis@arm.com, boris.ostrovsky@oracle.com,
	broonie@kernel.org, catalin.marinas@arm.com,
	daniel.lezcano@linaro.org, james.morse@arm.com, jgross@suse.com,
	maz@kernel.org, oliver.upton@linux.dev, pcc@google.com,
	sstabellini@kernel.org, tglx@linutronix.de,
	vladimir.murzin@arm.com, will@kernel.org
Subject: Re: [PATCH 05/37] arm64: Add cpus_have_final_boot_cap()
Date: Thu, 5 Oct 2023 10:23:54 +0100	[thread overview]
Message-ID: <ZR6AqrDiV1H2Qzpx@FVFF77S0Q05N> (raw)
In-Reply-To: <bb6a85af-cba8-94f9-b22c-a7da10c04213@arm.com>

On Fri, Sep 22, 2023 at 11:26:21AM +0100, Suzuki K Poulose wrote:
> On 21/09/2023 17:36, Mark Rutland wrote:
> > On Thu, Sep 21, 2023 at 10:13:31AM +0100, Suzuki K Poulose wrote:
> > > On 19/09/2023 10:28, Mark Rutland wrote:

> > > >    /*
> > > >     * 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.
> > 
> Thats what I had initially in mind, and is why I called it an
> overkill.
> 
> But may be another option is to have a different alternative construct
> for all capabilities, which defaults to BUG() and then patched to
> "false" or "true" based on the real status ? This may be more
> complicated.
> 
> > 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?
> 
> I don't think that is necessary. We could keep your patch
> as is, if we can't verify the boot capability easily.

I had a go at reworking this to automatically do the right thing, and it needs
a more substantial rework of the way we handle the cpucap bitmaps and walk over
the capability structures.

Given that, I'd like to leave this as-is for now, and follow up with the rework
for that.

Thanks,
Mark.

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