Re: [bootwrapper PATCH v3 01/15] aarch64: correct ZCR_EL3.LEN initialization

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

On Thu, Jan 27, 2022 at 06:55:00PM +0000, Mark Brown wrote:
> On Thu, Jan 27, 2022 at 04:08:50PM +0000, Mark Rutland wrote:
> > On Tue, Jan 25, 2022 at 05:44:47PM +0000, Mark Brown wrote:
> > > On Tue, Jan 25, 2022 at 04:33:39PM +0000, Mark Rutland wrote:
> > > > On Tue, Jan 25, 2022 at 03:59:38PM +0000, Mark Brown wrote:
> 
> > > > a) Use <register>_<field>_<valuename> definition, as here.
> 
> > > > b) Use <register>_<field>_MASK, and add comments at each usage as to why we use
> > > >    a mask as a value, to explain why that isn't a bug.
> 
> > > > c) Have both <register>_<field>_MASK and some value definition, and use some
> > > >    insertion helper to insert the value.
> 
> > > > ... and I went with (a) because it was the simplest.
> 
> > > > Is there a problem with this?
> 
> > > My concern is continuity of the enumeration algorithm between the
> > > various implementations we have, it's something we're not currently
> > > great with and this creates a separation between the kernel and the boot
> > > wrapper implementations.  There's no change in what's actually being
> > > done but it creates some additional effort to figure out why we're
> > > setting a maximum here and not trying to set all the bits as we do in
> > > the kernel.
> 
> > TBH, I'd argue if we're setting those bits in the kernel it's probably a bug,
> > because we don't know *exactly* what effect they'll have when allocated in the
> > future.
> 
> My comments there are related purely to the renaming of the constant,
> not to the narrowing of the bitmask which is less of an issue.

Ok.

> Basically avoiding the question "why is that a maximum and not a mask"
> and if there's limits on the valid values that are more restrictive than
> the representable values.
>
> The ideal thing for the boot wrapper would be option (c) with either a
> configuration option to set the maximum to something lower than all
> bits, unless someone was unduly enthusiastic and wanted to have an all
> CPUs enumeration algorithm, but both are probably more trouble than it's
> worth.

FWIW, if it's useful for testing to be able to configure the ZCR_EL3.LEN to
some smaller value, I'd be happy to move to (c) and get that plumbed into the
build system.

For now, I plan to take the series with this as-is, since having
ZCR_EL3_LEN_MAX is consistent with other field values we only insert, e.g.
MDCR_EL3_NSPB_NS_NOTRAP.

I'm happy to rejig that in future.

Thanks,
Mark

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