Re: [PATCH v2 06/26] arm64: Add debug registers affected by HDFGxTR_EL2

From: Marc Zyngier <maz@kernel.org>
To: Miguel Luis <miguel.luis@oracle.com>
Cc: "kvmarm@lists.linux.dev" <kvmarm@lists.linux.dev>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Eric Auger <eric.auger@redhat.com>,
	Mark Brown <broonie@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Will Deacon <will@kernel.org>,
	Alexandru Elisei <alexandru.elisei@arm.com>,
	Andre Przywara <andre.przywara@arm.com>,
	Chase Conklin <chase.conklin@arm.com>,
	Ganapatrao Kulkarni <gankulkarni@os.amperecomputing.com>,
	Darren Hart <darren@os.amperecomputing.com>,
	James Morse <james.morse@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Oliver Upton <oliver.upton@linux.dev>,
	Zenghui Yu <yuzenghui@huawei.com>
Subject: Re: [PATCH v2 06/26] arm64: Add debug registers affected by HDFGxTR_EL2
Date: Wed, 02 Aug 2023 18:52:55 +0100	[thread overview]
Message-ID: <86leet5o20.wl-maz@kernel.org> (raw)
In-Reply-To: <61B845D3-A42B-451F-B74D-51B4A1FD28C6@oracle.com>

On Mon, 31 Jul 2023 17:41:41 +0100,
Miguel Luis <miguel.luis@oracle.com> wrote:
> 
> Hi Marc,
> 
> A few comments on this one, please see below.
> 
> > On 28 Jul 2023, at 08:29, Marc Zyngier <maz@kernel.org> wrote:
> > 
> > The HDFGxTR_EL2 registers trap a (huge) set of debug and trace
> > related registers. Add their encodings (and only that, because
> > we really don't care about what these registers actually do at
> > this stage).
> > 
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> > arch/arm64/include/asm/sysreg.h | 78 +++++++++++++++++++++++++++++++++
> > 1 file changed, 78 insertions(+)
> > 
> > diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> > index 76289339b43b..9dfd127be55a 100644
> > --- a/arch/arm64/include/asm/sysreg.h
> > +++ b/arch/arm64/include/asm/sysreg.h
> > @@ -194,6 +194,84 @@
> > #define SYS_DBGDTRTX_EL0 sys_reg(2, 3, 0, 5, 0)
> > #define SYS_DBGVCR32_EL2 sys_reg(2, 4, 0, 7, 0)
> > 
> > +#define SYS_BRBINF_EL1(n) sys_reg(2, 1, 8, (n & 15), (((n & 16) >> 2) | 0))
> > +#define SYS_BRBINFINJ_EL1 sys_reg(2, 1, 9, 1, 0)
> > +#define SYS_BRBSRC_EL1(n) sys_reg(2, 1, 8, (n & 15), (((n & 16) >> 2) | 1))
> > +#define SYS_BRBSRCINJ_EL1 sys_reg(2, 1, 9, 1, 1)
> > +#define SYS_BRBTGT_EL1(n) sys_reg(2, 1, 8, (n & 15), (((n & 16) >> 2) | 2))
> > +#define SYS_BRBTGTINJ_EL1 sys_reg(2, 1, 9, 1, 2)
> > +#define SYS_BRBTS_EL1 sys_reg(2, 1, 9, 0, 2)
> > +
> > +#define SYS_BRBCR_EL1 sys_reg(2, 1, 9, 0, 0)
> > +#define SYS_BRBFCR_EL1 sys_reg(2, 1, 9, 0, 1)
> > +#define SYS_BRBIDR0_EL1 sys_reg(2, 1, 9, 2, 0)
> > +
> > +#define SYS_TRCITECR_EL1 sys_reg(3, 0, 1, 2, 3)
> > +#define SYS_TRCITECR_EL1 sys_reg(3, 0, 1, 2, 3)
> 
> SYS_TRCITECR_EL1 shows up twice.

Ah, nice one. Too many registers.

> 
> > +#define SYS_TRCACATR(m) sys_reg(2, 1, 2, ((m & 7) << 1), (2 | (m >> 3)))
> 
> Besides m’s restrictions it could be sanitised in op2 to consider only bit m[3].
> Suggestion for op2: (2 | ((m & 8) >> 3)))

It is fully expected that 'm' will be in the 0-15 range, as per the
architecture (D19.4.8), and the tables only use that exact range.

Do you see an actual bug, or is this defensive programming?

Thanks,

	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