Re: [PATCH v2 04/14] arm64/sysreg: Add definitions for immediate versions of MSR ALLINT

Marc Zyngier
2022-12-07T19:20:18Z

On Mon, 05 Dec 2022 17:11:38 +0000,
Mark Brown <broonie@kernel.org> wrote:
>
> [1 <text/plain; us-ascii (7bit)>]
> On Mon, Dec 05, 2022 at 04:38:53PM +0000, Marc Zyngier wrote:
> > Mark Brown <broonie@kernel.org> wrote:
>
> > > }
> > > +
> > > #endif
>
> > Spurious change?
>
> Yes.
>
> > > +++ b/arch/arm64/include/asm/nmi.h
> > > @@ -0,0 +1,18 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-only */
>
> > > +static __always_inline void _allint_set(void)
> > > +{
> > > + asm volatile(__msr_s(SYS_ALLINT_SET, "xzr"));
> > > +}
>
> > If this *really* must be a separate include file, it should at least
> > directly include its dependencies. My gut feeling is that it would be
> > better placed in daiflags.h.
>
> Yeah, I was swithering on that. Some versions of the code have had more
> in here at which point having the separate header made more sense. I
> think part of the problem here is that we should do some combination of
> renaming daifflags.h or layering an a more abstracted API on top of it,
> putting things that are not DAIF into daifflags.h doesn't feel great.
>
> > > @@ -126,6 +126,8 @@
> > > * System registers, organised loosely by encoding but grouped together
> > > * where the architected name contains an index. e.g. ID_MMFR<n>_EL1.
> > > */
> > > +#define SYS_ALLINT_CLR sys_reg(0, 1, 4, 0, 0)
> > > +#define SYS_ALLINT_SET sys_reg(0, 1, 4, 1, 0)
>
> > This only covers the immediate versions of ALLINT, and misses the
> > definition for the register version, aka sys_reg(3, 0, 4, 3, 0).
>
> That is already present upstream, we only need to add the immediate
> versions which the generated header stuff doesn't have any model for
> yet.

Ah, missed that one, thanks.

Out of curiosity, what is missing in the generator to deal with this
stuff?

M.

--
Without deviation from the norm, progress is not possible.