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

Mark Brown
2022-12-05T17:11:43Z

[-- Attachment #1: Type: text/plain, Size: 1487 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]