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

From: Marc Zyngier <maz@kernel.org>
To: Mark Brown <broonie@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Lorenzo Pieralisi <lpieralisi@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Sami Mujawar <Sami.Mujawar@arm.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev
Subject: Re: [PATCH v2 04/14] arm64/sysreg: Add definitions for immediate versions of MSR ALLINT
Date: Wed, 07 Dec 2022 19:18:35 +0000	[thread overview]
Message-ID: <87tu27rpis.wl-maz@kernel.org> (raw)
In-Reply-To: <Y44mSokVwLHbGdN7@sirena.org.uk>

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.

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