From: Anshuman Khandual <anshuman.khandual@arm.com>
To use the Branch Record Buffer Extension (BRBE), some configuration is
necessary at EL3 and EL2. This patch documents the requirements and adds
the initial EL2 setup code, which largely consists of configuring the
fine-grained traps and initializing a couple of BRBE control registers.
Before this patch, __init_el2_fgt() would initialize HDFGRTR_EL2 and
HDFGWTR_EL2 with the same value, relying on the read/write trap controls
for a register occupying the same bit position in either register. The
'nBRBIDR' trap control only exists in bit 59 of HDFGRTR_EL2, while bit
59 of HDFGWTR_EL2 is RES0, and so this assumption no longer holds.
To handle HDFGRTR_EL2 and HDFGWTR_EL2 having (slightly) different bit
layouts, __init_el2_fgt() is changed to accumulate the HDFGRTR_EL2 and
HDFGWTR_EL2 control bits separately. While making this change the
open-coded value (1 << 62) is replaced with
HDFG{R,W}TR_EL2_nPMSNEVFR_EL1_MASK.
The BRBCR_EL1 and BRBCR_EL2 registers are unusual and require special
initialisation: even though they are subject to E2H renaming, both have
an effect regardless of HCR_EL2.TGE, even when running at EL2, and
consequently both need to be initialised. This is handled in
__init_el2_brbe() with a comment to explain the situation.
Cc: Marc Zyngier <maz@kernel.org>
Cc: Oliver Upton <oliver.upton@linux.dev>
Reviewed-by: Leo Yan <leo.yan@arm.com>
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
[Mark: rewrite commit message, fix typo in comment]
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: "Rob Herring (Arm)" <robh@kernel.org>
Tested-by: James Clark <james.clark@linaro.org>
---
v20:
- Document that MDCR_EL3.SBRBE can be 0b01 also
- Fix "HDFGWTR_EL2 is RES0" in commit msg
---
Documentation/arch/arm64/booting.rst | 21 +++++++++
arch/arm64/include/asm/el2_setup.h | 86 ++++++++++++++++++++++++++++++++++--
2 files changed, 104 insertions(+), 3 deletions(-)
diff --git a/Documentation/arch/arm64/booting.rst b/Documentation/arch/arm64/booting.rst
index dee7b6de864f..a627c1e0e4a0 100644
--- a/Documentation/arch/arm64/booting.rst
+++ b/Documentation/arch/arm64/booting.rst
@@ -358,6 +358,27 @@ Before jumping into the kernel, the following conditions must be met:
- HWFGWTR_EL2.nSMPRI_EL1 (bit 54) must be initialised to 0b01.
+ For CPUs with feature Branch Record Buffer Extension (FEAT_BRBE):
+
+ - If EL3 is present:
+
+ - MDCR_EL3.SBRBE (bits 33:32) must be initialised to 0b01 or 0b11.
+
+ - If the kernel is entered at EL1 and EL2 is present:
+
+ - BRBCR_EL2.CC (bit 3) must be initialised to 0b1.
+ - BRBCR_EL2.MPRED (bit 4) must be initialised to 0b1.
+
+ - HDFGRTR_EL2.nBRBDATA (bit 61) must be initialised to 0b1.
+ - HDFGRTR_EL2.nBRBCTL (bit 60) must be initialised to 0b1.
+ - HDFGRTR_EL2.nBRBIDR (bit 59) must be initialised to 0b1.
+
+ - HDFGWTR_EL2.nBRBDATA (bit 61) must be initialised to 0b1.
+ - HDFGWTR_EL2.nBRBCTL (bit 60) must be initialised to 0b1.
+
+ - HFGITR_EL2.nBRBIALL (bit 56) must be initialised to 0b1.
+ - HFGITR_EL2.nBRBINJ (bit 55) must be initialised to 0b1.
+
For CPUs with the Scalable Matrix Extension FA64 feature (FEAT_SME_FA64):
- If EL3 is present:
diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h
index ebceaae3c749..e7fcba1e7d8e 100644
--- a/arch/arm64/include/asm/el2_setup.h
+++ b/arch/arm64/include/asm/el2_setup.h
@@ -189,6 +189,39 @@
.Lskip_set_cptr_\@:
.endm
+/*
+ * Configure BRBE to permit recording cycle counts and branch mispredicts.
+ *
+ * At any EL, to record cycle counts BRBE requires that both BRBCR_EL2.CC=1 and
+ * BRBCR_EL1.CC=1.
+ *
+ * At any EL, to record branch mispredicts BRBE requires that both
+ * BRBCR_EL2.MPRED=1 and BRBCR_EL1.MPRED=1.
+ *
+ * When HCR_EL2.E2H=1, the BRBCR_EL1 encoding is redirected to BRBCR_EL2, but
+ * the {CC,MPRED} bits in the real BRBCR_EL1 register still apply.
+ *
+ * Set {CC,MPRED} in both BRBCR_EL2 and BRBCR_EL1 so that at runtime we only
+ * need to enable/disable these in BRBCR_EL1 regardless of whether the kernel
+ * ends up executing in EL1 or EL2.
+ */
+.macro __init_el2_brbe
+ mrs x1, id_aa64dfr0_el1
+ ubfx x1, x1, #ID_AA64DFR0_EL1_BRBE_SHIFT, #4
+ cbz x1, .Lskip_brbe_\@
+
+ mov_q x0, BRBCR_ELx_CC | BRBCR_ELx_MPRED
+ msr_s SYS_BRBCR_EL2, x0
+
+ __check_hvhe .Lset_brbe_nvhe_\@, x1
+ msr_s SYS_BRBCR_EL12, x0 // VHE
+ b .Lskip_brbe_\@
+
+.Lset_brbe_nvhe_\@:
+ msr_s SYS_BRBCR_EL1, x0 // NVHE
+.Lskip_brbe_\@:
+.endm
+
/* Disable any fine grained traps */
.macro __init_el2_fgt
mrs x1, id_aa64mmfr0_el1
@@ -196,16 +229,48 @@
cbz x1, .Lskip_fgt_\@
mov x0, xzr
+ mov x2, xzr
mrs x1, id_aa64dfr0_el1
ubfx x1, x1, #ID_AA64DFR0_EL1_PMSVer_SHIFT, #4
cmp x1, #3
b.lt .Lskip_spe_fgt_\@
+
/* Disable PMSNEVFR_EL1 read and write traps */
- orr x0, x0, #(1 << 62)
+ orr x0, x0, #HDFGRTR_EL2_nPMSNEVFR_EL1_MASK
+ orr x2, x2, #HDFGWTR_EL2_nPMSNEVFR_EL1_MASK
.Lskip_spe_fgt_\@:
+#ifdef CONFIG_ARM64_BRBE
+ mrs x1, id_aa64dfr0_el1
+ ubfx x1, x1, #ID_AA64DFR0_EL1_BRBE_SHIFT, #4
+ cbz x1, .Lskip_brbe_reg_fgt_\@
+
+ /*
+ * Disable read traps for the following registers
+ *
+ * [BRBSRC|BRBTGT|RBINF]_EL1
+ * [BRBSRCINJ|BRBTGTINJ|BRBINFINJ|BRBTS]_EL1
+ */
+ orr x0, x0, #HDFGRTR_EL2_nBRBDATA_MASK
+
+ /*
+ * Disable write traps for the following registers
+ *
+ * [BRBSRCINJ|BRBTGTINJ|BRBINFINJ|BRBTS]_EL1
+ */
+ orr x2, x2, #HDFGWTR_EL2_nBRBDATA_MASK
+
+ /* Disable read and write traps for [BRBCR|BRBFCR]_EL1 */
+ orr x0, x0, #HDFGRTR_EL2_nBRBCTL_MASK
+ orr x2, x2, #HDFGWTR_EL2_nBRBCTL_MASK
+
+ /* Disable read traps for BRBIDR_EL1 */
+ orr x0, x0, #HDFGRTR_EL2_nBRBIDR_MASK
+
+.Lskip_brbe_reg_fgt_\@:
+#endif /* CONFIG_ARM64_BRBE */
msr_s SYS_HDFGRTR_EL2, x0
- msr_s SYS_HDFGWTR_EL2, x0
+ msr_s SYS_HDFGWTR_EL2, x2
mov x0, xzr
mrs x1, id_aa64pfr1_el1
@@ -246,7 +311,21 @@
.Lset_fgt_\@:
msr_s SYS_HFGRTR_EL2, x0
msr_s SYS_HFGWTR_EL2, x0
- msr_s SYS_HFGITR_EL2, xzr
+ mov x0, xzr
+#ifdef CONFIG_ARM64_BRBE
+ mrs x1, id_aa64dfr0_el1
+ ubfx x1, x1, #ID_AA64DFR0_EL1_BRBE_SHIFT, #4
+ cbz x1, .Lskip_brbe_insn_fgt_\@
+
+ /* Disable traps for BRBIALL instruction */
+ orr x0, x0, #HFGITR_EL2_nBRBIALL_MASK
+
+ /* Disable traps for BRBINJ instruction */
+ orr x0, x0, #HFGITR_EL2_nBRBINJ_MASK
+
+.Lskip_brbe_insn_fgt_\@:
+#endif /* CONFIG_ARM64_BRBE */
+ msr_s SYS_HFGITR_EL2, x0
mrs x1, id_aa64pfr0_el1 // AMU traps UNDEF without AMU
ubfx x1, x1, #ID_AA64PFR0_EL1_AMU_SHIFT, #4
@@ -320,6 +399,7 @@
__init_el2_hcrx
__init_el2_timers
__init_el2_debug
+ __init_el2_brbe
__init_el2_lor
__init_el2_stage2
__init_el2_gicv3
--
2.47.2On Mon, Apr 07, 2025 at 12:41:31PM -0500, Rob Herring (Arm) wrote:
> From: Anshuman Khandual <anshuman.khandual@arm.com>
>
> To use the Branch Record Buffer Extension (BRBE), some configuration is
> necessary at EL3 and EL2. This patch documents the requirements and adds
> the initial EL2 setup code, which largely consists of configuring the
> fine-grained traps and initializing a couple of BRBE control registers.
>
> Before this patch, __init_el2_fgt() would initialize HDFGRTR_EL2 and
> HDFGWTR_EL2 with the same value, relying on the read/write trap controls
> for a register occupying the same bit position in either register. The
> 'nBRBIDR' trap control only exists in bit 59 of HDFGRTR_EL2, while bit
> 59 of HDFGWTR_EL2 is RES0, and so this assumption no longer holds.
>
> To handle HDFGRTR_EL2 and HDFGWTR_EL2 having (slightly) different bit
> layouts, __init_el2_fgt() is changed to accumulate the HDFGRTR_EL2 and
> HDFGWTR_EL2 control bits separately. While making this change the
> open-coded value (1 << 62) is replaced with
> HDFG{R,W}TR_EL2_nPMSNEVFR_EL1_MASK.
>
> The BRBCR_EL1 and BRBCR_EL2 registers are unusual and require special
> initialisation: even though they are subject to E2H renaming, both have
> an effect regardless of HCR_EL2.TGE, even when running at EL2, and
> consequently both need to be initialised. This is handled in
> __init_el2_brbe() with a comment to explain the situation.
>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Oliver Upton <oliver.upton@linux.dev>
> Reviewed-by: Leo Yan <leo.yan@arm.com>
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> [Mark: rewrite commit message, fix typo in comment]
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: "Rob Herring (Arm)" <robh@kernel.org>
> Tested-by: James Clark <james.clark@linaro.org>
> ---
> v20:
> - Document that MDCR_EL3.SBRBE can be 0b01 also
> - Fix "HDFGWTR_EL2 is RES0" in commit msg
> ---
> Documentation/arch/arm64/booting.rst | 21 +++++++++
> arch/arm64/include/asm/el2_setup.h | 86 ++++++++++++++++++++++++++++++++++--
> 2 files changed, 104 insertions(+), 3 deletions(-)
It would be good to have an Ack from the kvm/arm64 side on this, but in
the meantime I've left some comments inline.
> diff --git a/Documentation/arch/arm64/booting.rst b/Documentation/arch/arm64/booting.rst
> index dee7b6de864f..a627c1e0e4a0 100644
> --- a/Documentation/arch/arm64/booting.rst
> +++ b/Documentation/arch/arm64/booting.rst
> @@ -358,6 +358,27 @@ Before jumping into the kernel, the following conditions must be met:
>
> - HWFGWTR_EL2.nSMPRI_EL1 (bit 54) must be initialised to 0b01.
>
> + For CPUs with feature Branch Record Buffer Extension (FEAT_BRBE):
This doesn't make sense ^^^
> diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h
> index ebceaae3c749..e7fcba1e7d8e 100644
> --- a/arch/arm64/include/asm/el2_setup.h
> +++ b/arch/arm64/include/asm/el2_setup.h
> @@ -189,6 +189,39 @@
> .Lskip_set_cptr_\@:
> .endm
>
> +/*
> + * Configure BRBE to permit recording cycle counts and branch mispredicts.
> + *
> + * At any EL, to record cycle counts BRBE requires that both BRBCR_EL2.CC=1 and
> + * BRBCR_EL1.CC=1.
> + *
> + * At any EL, to record branch mispredicts BRBE requires that both
> + * BRBCR_EL2.MPRED=1 and BRBCR_EL1.MPRED=1.
> + *
> + * When HCR_EL2.E2H=1, the BRBCR_EL1 encoding is redirected to BRBCR_EL2, but
> + * the {CC,MPRED} bits in the real BRBCR_EL1 register still apply.
> + *
> + * Set {CC,MPRED} in both BRBCR_EL2 and BRBCR_EL1 so that at runtime we only
> + * need to enable/disable these in BRBCR_EL1 regardless of whether the kernel
> + * ends up executing in EL1 or EL2.
> + */
> +.macro __init_el2_brbe
> + mrs x1, id_aa64dfr0_el1
> + ubfx x1, x1, #ID_AA64DFR0_EL1_BRBE_SHIFT, #4
> + cbz x1, .Lskip_brbe_\@
> +
> + mov_q x0, BRBCR_ELx_CC | BRBCR_ELx_MPRED
> + msr_s SYS_BRBCR_EL2, x0
> +
> + __check_hvhe .Lset_brbe_nvhe_\@, x1
> + msr_s SYS_BRBCR_EL12, x0 // VHE
> + b .Lskip_brbe_\@
> +
> +.Lset_brbe_nvhe_\@:
> + msr_s SYS_BRBCR_EL1, x0 // NVHE
> +.Lskip_brbe_\@:
Why do we have to poke BRBCR_EL12/BRBCR_EL1 here rather than in the BRBE
driver code?
> +.endm
> +
> /* Disable any fine grained traps */
> .macro __init_el2_fgt
> mrs x1, id_aa64mmfr0_el1
> @@ -196,16 +229,48 @@
> cbz x1, .Lskip_fgt_\@
>
> mov x0, xzr
> + mov x2, xzr
> mrs x1, id_aa64dfr0_el1
> ubfx x1, x1, #ID_AA64DFR0_EL1_PMSVer_SHIFT, #4
> cmp x1, #3
> b.lt .Lskip_spe_fgt_\@
> +
> /* Disable PMSNEVFR_EL1 read and write traps */
> - orr x0, x0, #(1 << 62)
> + orr x0, x0, #HDFGRTR_EL2_nPMSNEVFR_EL1_MASK
> + orr x2, x2, #HDFGWTR_EL2_nPMSNEVFR_EL1_MASK
>
> .Lskip_spe_fgt_\@:
> +#ifdef CONFIG_ARM64_BRBE
Why is this gated on CONFIG_ARM64_BRBE?
> + mrs x1, id_aa64dfr0_el1
> + ubfx x1, x1, #ID_AA64DFR0_EL1_BRBE_SHIFT, #4
> + cbz x1, .Lskip_brbe_reg_fgt_\@
> +
> + /*
> + * Disable read traps for the following registers
> + *
> + * [BRBSRC|BRBTGT|RBINF]_EL1
> + * [BRBSRCINJ|BRBTGTINJ|BRBINFINJ|BRBTS]_EL1
> + */
> + orr x0, x0, #HDFGRTR_EL2_nBRBDATA_MASK
> +
> + /*
> + * Disable write traps for the following registers
> + *
> + * [BRBSRCINJ|BRBTGTINJ|BRBINFINJ|BRBTS]_EL1
> + */
> + orr x2, x2, #HDFGWTR_EL2_nBRBDATA_MASK
> +
> + /* Disable read and write traps for [BRBCR|BRBFCR]_EL1 */
> + orr x0, x0, #HDFGRTR_EL2_nBRBCTL_MASK
> + orr x2, x2, #HDFGWTR_EL2_nBRBCTL_MASK
> +
> + /* Disable read traps for BRBIDR_EL1 */
> + orr x0, x0, #HDFGRTR_EL2_nBRBIDR_MASK
> +
> +.Lskip_brbe_reg_fgt_\@:
I think this label should become .Lset_debug_fgt_\@:. That way, we have
a clear point at which we're done with HDFG*TR_EL2. We can zero x0 and
x2 and from then on we can focus on HFG*TR + HFGITR.
nit: the existing .Lskip_debug_fgt_\@ label looks to be misnamed -- it
should probably be .Lskip_sme_fgt_\@ ?
> +#endif /* CONFIG_ARM64_BRBE */
> msr_s SYS_HDFGRTR_EL2, x0
> - msr_s SYS_HDFGWTR_EL2, x0
> + msr_s SYS_HDFGWTR_EL2, x2
nit: It would be cleaner to use x0/x1 for the pair of trap registers
but I can see that would be a more invasive change.
> mov x0, xzr
> mrs x1, id_aa64pfr1_el1
> @@ -246,7 +311,21 @@
> .Lset_fgt_\@:
> msr_s SYS_HFGRTR_EL2, x0
> msr_s SYS_HFGWTR_EL2, x0
> - msr_s SYS_HFGITR_EL2, xzr
> + mov x0, xzr
> +#ifdef CONFIG_ARM64_BRBE
> + mrs x1, id_aa64dfr0_el1
> + ubfx x1, x1, #ID_AA64DFR0_EL1_BRBE_SHIFT, #4
> + cbz x1, .Lskip_brbe_insn_fgt_\@
It would probably scale better if we unconditionally stick x2 in
HFGITR_EL2 and zero that register in '.Lset_debug_fgt_\@'. We still have
two checks for BRBE, but at least '.Lset_fgt_\@' could just stick to
writing the registers.
Will
On Mon, May 19, 2025 at 03:07:15PM +0100, Will Deacon wrote:
> On Mon, Apr 07, 2025 at 12:41:31PM -0500, Rob Herring (Arm) wrote:
> > From: Anshuman Khandual <anshuman.khandual@arm.com>
> >
> > To use the Branch Record Buffer Extension (BRBE), some configuration is
> > necessary at EL3 and EL2. This patch documents the requirements and adds
> > the initial EL2 setup code, which largely consists of configuring the
> > fine-grained traps and initializing a couple of BRBE control registers.
> >
> > Before this patch, __init_el2_fgt() would initialize HDFGRTR_EL2 and
> > HDFGWTR_EL2 with the same value, relying on the read/write trap controls
> > for a register occupying the same bit position in either register. The
> > 'nBRBIDR' trap control only exists in bit 59 of HDFGRTR_EL2, while bit
> > 59 of HDFGWTR_EL2 is RES0, and so this assumption no longer holds.
> >
> > To handle HDFGRTR_EL2 and HDFGWTR_EL2 having (slightly) different bit
> > layouts, __init_el2_fgt() is changed to accumulate the HDFGRTR_EL2 and
> > HDFGWTR_EL2 control bits separately. While making this change the
> > open-coded value (1 << 62) is replaced with
> > HDFG{R,W}TR_EL2_nPMSNEVFR_EL1_MASK.
> >
> > The BRBCR_EL1 and BRBCR_EL2 registers are unusual and require special
> > initialisation: even though they are subject to E2H renaming, both have
> > an effect regardless of HCR_EL2.TGE, even when running at EL2, and
> > consequently both need to be initialised. This is handled in
> > __init_el2_brbe() with a comment to explain the situation.
> >
> > Cc: Marc Zyngier <maz@kernel.org>
> > Cc: Oliver Upton <oliver.upton@linux.dev>
> > Reviewed-by: Leo Yan <leo.yan@arm.com>
> > Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> > [Mark: rewrite commit message, fix typo in comment]
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > Signed-off-by: "Rob Herring (Arm)" <robh@kernel.org>
> > Tested-by: James Clark <james.clark@linaro.org>
> > ---
> > v20:
> > - Document that MDCR_EL3.SBRBE can be 0b01 also
> > - Fix "HDFGWTR_EL2 is RES0" in commit msg
> > ---
> > Documentation/arch/arm64/booting.rst | 21 +++++++++
> > arch/arm64/include/asm/el2_setup.h | 86 ++++++++++++++++++++++++++++++++++--
> > 2 files changed, 104 insertions(+), 3 deletions(-)
>
> It would be good to have an Ack from the kvm/arm64 side on this, but in
> the meantime I've left some comments inline.
>
> > diff --git a/Documentation/arch/arm64/booting.rst b/Documentation/arch/arm64/booting.rst
> > index dee7b6de864f..a627c1e0e4a0 100644
> > --- a/Documentation/arch/arm64/booting.rst
> > +++ b/Documentation/arch/arm64/booting.rst
> > @@ -358,6 +358,27 @@ Before jumping into the kernel, the following conditions must be met:
> >
> > - HWFGWTR_EL2.nSMPRI_EL1 (bit 54) must be initialised to 0b01.
> >
> > + For CPUs with feature Branch Record Buffer Extension (FEAT_BRBE):
>
> This doesn't make sense ^^^
You mean it should be "with the Branch Record Buffer Extension feature"
instead?
>
> > diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h
> > index ebceaae3c749..e7fcba1e7d8e 100644
> > --- a/arch/arm64/include/asm/el2_setup.h
> > +++ b/arch/arm64/include/asm/el2_setup.h
> > @@ -189,6 +189,39 @@
> > .Lskip_set_cptr_\@:
> > .endm
> >
> > +/*
> > + * Configure BRBE to permit recording cycle counts and branch mispredicts.
> > + *
> > + * At any EL, to record cycle counts BRBE requires that both BRBCR_EL2.CC=1 and
> > + * BRBCR_EL1.CC=1.
> > + *
> > + * At any EL, to record branch mispredicts BRBE requires that both
> > + * BRBCR_EL2.MPRED=1 and BRBCR_EL1.MPRED=1.
> > + *
> > + * When HCR_EL2.E2H=1, the BRBCR_EL1 encoding is redirected to BRBCR_EL2, but
> > + * the {CC,MPRED} bits in the real BRBCR_EL1 register still apply.
> > + *
> > + * Set {CC,MPRED} in both BRBCR_EL2 and BRBCR_EL1 so that at runtime we only
> > + * need to enable/disable these in BRBCR_EL1 regardless of whether the kernel
> > + * ends up executing in EL1 or EL2.
> > + */
> > +.macro __init_el2_brbe
> > + mrs x1, id_aa64dfr0_el1
> > + ubfx x1, x1, #ID_AA64DFR0_EL1_BRBE_SHIFT, #4
> > + cbz x1, .Lskip_brbe_\@
> > +
> > + mov_q x0, BRBCR_ELx_CC | BRBCR_ELx_MPRED
> > + msr_s SYS_BRBCR_EL2, x0
> > +
> > + __check_hvhe .Lset_brbe_nvhe_\@, x1
> > + msr_s SYS_BRBCR_EL12, x0 // VHE
> > + b .Lskip_brbe_\@
> > +
> > +.Lset_brbe_nvhe_\@:
> > + msr_s SYS_BRBCR_EL1, x0 // NVHE
> > +.Lskip_brbe_\@:
>
> Why do we have to poke BRBCR_EL12/BRBCR_EL1 here rather than in the BRBE
> driver code?
Yeah, I think we can drop this. Originally, the driver did not touch
SYS_BRBCR_EL12, but it turns out we need to for freeze on overflow to
work correctly with VHE (see the comment in the driver for
SYS_BRBCR_EL12 access).
The only other reason I can come up with is some fields reset to UNKNOWN
and there may be no driver. However, the important ones, ExBRE, reset to
0, so we should be fine.
> > +.endm
> > +
> > /* Disable any fine grained traps */
> > .macro __init_el2_fgt
> > mrs x1, id_aa64mmfr0_el1
> > @@ -196,16 +229,48 @@
> > cbz x1, .Lskip_fgt_\@
> >
> > mov x0, xzr
> > + mov x2, xzr
> > mrs x1, id_aa64dfr0_el1
> > ubfx x1, x1, #ID_AA64DFR0_EL1_PMSVer_SHIFT, #4
> > cmp x1, #3
> > b.lt .Lskip_spe_fgt_\@
> > +
> > /* Disable PMSNEVFR_EL1 read and write traps */
> > - orr x0, x0, #(1 << 62)
> > + orr x0, x0, #HDFGRTR_EL2_nPMSNEVFR_EL1_MASK
> > + orr x2, x2, #HDFGWTR_EL2_nPMSNEVFR_EL1_MASK
> >
> > .Lskip_spe_fgt_\@:
> > +#ifdef CONFIG_ARM64_BRBE
>
> Why is this gated on CONFIG_ARM64_BRBE?
Shrug. We don't do that anywhere else, so I'll drop it.
> > + mrs x1, id_aa64dfr0_el1
> > + ubfx x1, x1, #ID_AA64DFR0_EL1_BRBE_SHIFT, #4
> > + cbz x1, .Lskip_brbe_reg_fgt_\@
> > +
> > + /*
> > + * Disable read traps for the following registers
> > + *
> > + * [BRBSRC|BRBTGT|RBINF]_EL1
> > + * [BRBSRCINJ|BRBTGTINJ|BRBINFINJ|BRBTS]_EL1
> > + */
> > + orr x0, x0, #HDFGRTR_EL2_nBRBDATA_MASK
> > +
> > + /*
> > + * Disable write traps for the following registers
> > + *
> > + * [BRBSRCINJ|BRBTGTINJ|BRBINFINJ|BRBTS]_EL1
> > + */
> > + orr x2, x2, #HDFGWTR_EL2_nBRBDATA_MASK
> > +
> > + /* Disable read and write traps for [BRBCR|BRBFCR]_EL1 */
> > + orr x0, x0, #HDFGRTR_EL2_nBRBCTL_MASK
> > + orr x2, x2, #HDFGWTR_EL2_nBRBCTL_MASK
> > +
> > + /* Disable read traps for BRBIDR_EL1 */
> > + orr x0, x0, #HDFGRTR_EL2_nBRBIDR_MASK
> > +
> > +.Lskip_brbe_reg_fgt_\@:
>
> I think this label should become .Lset_debug_fgt_\@:. That way, we have
> a clear point at which we're done with HDFG*TR_EL2. We can zero x0 and
> x2 and from then on we can focus on HFG*TR + HFGITR.
>
> nit: the existing .Lskip_debug_fgt_\@ label looks to be misnamed -- it
> should probably be .Lskip_sme_fgt_\@ ?
>
> > +#endif /* CONFIG_ARM64_BRBE */
> > msr_s SYS_HDFGRTR_EL2, x0
> > - msr_s SYS_HDFGWTR_EL2, x0
> > + msr_s SYS_HDFGWTR_EL2, x2
>
> nit: It would be cleaner to use x0/x1 for the pair of trap registers
> but I can see that would be a more invasive change.
That would not be worth the churn IMO. We'd want to just swap x1 and x2
everywhere so we consistently use x2 for ID registers.
And then I'd be to blame for *all* this wonderful code. ;)
>
> > mov x0, xzr
> > mrs x1, id_aa64pfr1_el1
> > @@ -246,7 +311,21 @@
> > .Lset_fgt_\@:
> > msr_s SYS_HFGRTR_EL2, x0
> > msr_s SYS_HFGWTR_EL2, x0
> > - msr_s SYS_HFGITR_EL2, xzr
> > + mov x0, xzr
> > +#ifdef CONFIG_ARM64_BRBE
> > + mrs x1, id_aa64dfr0_el1
> > + ubfx x1, x1, #ID_AA64DFR0_EL1_BRBE_SHIFT, #4
> > + cbz x1, .Lskip_brbe_insn_fgt_\@
>
> It would probably scale better if we unconditionally stick x2 in
> HFGITR_EL2 and zero that register in '.Lset_debug_fgt_\@'. We still have
> two checks for BRBE, but at least '.Lset_fgt_\@' could just stick to
> writing the registers.
All that looks doable.
Rob
© 2016 - 2026 Red Hat, Inc.