[PATCH] arm64: Do not trap PMSNEVFR_EL1

Commit 31c00d2aeaa2 ("arm64: Disable fine grained traps on boot") zeroed
the fine grained trap registers to prevent unwanted register traps from
occuring. However, for the PMSNEVFR_EL1 register, the corresponding
HDFGRTR_EL2.nPMSNEVFR_EL1 field must be 1 to disable trapping. Set the
field to 1 if FEAT_SPEv1p2 is detected.

Fixes: 31c00d2aeaa2 ("arm64: Disable fine grained traps on boot")
Signed-off-by: Alexandru Elisei <redacted>
---
Based on v5.14-rc7. Also, we could write 1 << 62 to HDFGRTR_EL2 unconditionally
since the field is RAZ/WI if !FEAT_SPEv1p2. I don't have a strong preference for
either approaches, but I chose this implementation because it's clearer (even
though it's more verbose and it's one extra trap on NV).

Tested on the model, using boot-wrapper built from commit 5cd6238ec4ef
("aarch32: fix .globl replacement"). Without this patch, in NVHE mode, the model
freezes when I try to read PMSNEVFR_EL1. With this patch, the model doesn't hang
when I read the register, but it hangs when I write to it. I've gone throught
the pseudocode for reading and writing to PMSNEVFR_EL1 and from what I can tell
nothing should be trapping the accesses. On top of that, this is what I tried on
the model with this patch applied:

1. VHE mode, I can read and write to PMSNEVFR_EL1 without any issues, so the
hang is not caused by an incorrect EL3 configuration.

2. NVHE mode, I can read and write just fine to *PMSEVFR_EL1*, so the hang is
not caused by an EL2 trap that affects the rest of the profiling control
registers. I have tried printing the HDFGRTR_EL2 value in this situation using
semihosting, the value is what it is programmed by __init_el2_fgt (that is,
1 << 62).

At this point, I am inclined to think it's a model bug because reading works,
but writing causes a hang and that looks very suspicious to me. I'm going to
open a model bug internally and see what comes of it.

 arch/arm64/include/asm/el2_setup.h | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h
index b83fb24954b7..8a9adb2039fd 100644
--- a/arch/arm64/include/asm/el2_setup.h
+++ b/arch/arm64/include/asm/el2_setup.h

@@ -149,7 +149,16 @@
 	ubfx	x1, x1, #ID_AA64MMFR0_FGT_SHIFT, #4
 	cbz	x1, .Lskip_fgt_\@
 
-	msr_s	SYS_HDFGRTR_EL2, xzr
+	mov	x0, xzr
+	mrs	x1, id_aa64dfr0_el1
+	ubfx	x1, x1, #ID_AA64DFR0_PMSVER_SHIFT, #4
+	cmp	x1, #3
+	b.lt	.Lset_fgt_\@
+	/* Set HDFGRTR_EL2.nPMSNEVFR_EL1 to disable the register trap */
+	orr	x0, x0, #(1 << 62)
+
+.Lset_fgt_\@:
+	msr_s	SYS_HDFGRTR_EL2, x0
 	msr_s	SYS_HDFGWTR_EL2, xzr
 	msr_s	SYS_HFGRTR_EL2, xzr
 	msr_s	SYS_HFGWTR_EL2, xzr

-- 
2.33.0


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

Errr... somehow I forgot to add the arm64 maintainers. Fixing that.

On 8/24/21 2:24 PM, Alexandru Elisei wrote:

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

On Tue, Aug 24, 2021 at 02:24:59PM +0100, Alexandru Elisei wrote:

Commit 31c00d2aeaa2 ("arm64: Disable fine grained traps on boot") zeroed
the fine grained trap registers to prevent unwanted register traps from
occuring. However, for the PMSNEVFR_EL1 register, the corresponding
HDFGRTR_EL2.nPMSNEVFR_EL1 field must be 1 to disable trapping. Set the
field to 1 if FEAT_SPEv1p2 is detected.

Oh, that's a shame :/  I wonder why this feature is different to the
others, I just had a quick check and didn't see any other issues but...

Based on v5.14-rc7. Also, we could write 1 << 62 to HDFGRTR_EL2 unconditionally
since the field is RAZ/WI if !FEAT_SPEv1p2. I don't have a strong preference for
either approaches, but I chose this implementation because it's clearer (even
though it's more verbose and it's one extra trap on NV).

Yes, the explicit feature check is both clearer and more conservative -
it's unlikely to have a practical impact but 

At this point, I am inclined to think it's a model bug because reading works,
but writing causes a hang and that looks very suspicious to me. I'm going to
open a model bug internally and see what comes of it.

...are you sure this isn't that the same issue also exists with the
equivalent field HDFGWTR_EL2 - glancing at the XML it appears to have
the same issue?  One of the fine grained aspects of fine grained traps
is that there's separate read and write traps!

Hi Mark,

On 8/24/21 4:10 PM, Mark Brown wrote:

On Tue, Aug 24, 2021 at 02:24:59PM +0100, Alexandru Elisei wrote:

quoted

Commit 31c00d2aeaa2 ("arm64: Disable fine grained traps on boot") zeroed
the fine grained trap registers to prevent unwanted register traps from
occuring. However, for the PMSNEVFR_EL1 register, the corresponding
HDFGRTR_EL2.nPMSNEVFR_EL1 field must be 1 to disable trapping. Set the
field to 1 if FEAT_SPEv1p2 is detected.

Oh, that's a shame :/  I wonder why this feature is different to the
others, I just had a quick check and didn't see any other issues but...

quoted

Based on v5.14-rc7. Also, we could write 1 << 62 to HDFGRTR_EL2 unconditionally
since the field is RAZ/WI if !FEAT_SPEv1p2. I don't have a strong preference for
either approaches, but I chose this implementation because it's clearer (even
though it's more verbose and it's one extra trap on NV).

Yes, the explicit feature check is both clearer and more conservative -
it's unlikely to have a practical impact but 

quoted

At this point, I am inclined to think it's a model bug because reading works,
but writing causes a hang and that looks very suspicious to me. I'm going to
open a model bug internally and see what comes of it.

...are you sure this isn't that the same issue also exists with the
equivalent field HDFGWTR_EL2 - glancing at the XML it appears to have
the same issue?  One of the fine grained aspects of fine grained traps
is that there's separate read and write traps!

Yes, you are right, when I read the MSR PMSNEVFR_EL1, <Xt> pseudocode I missed the
fact that for writes the trap is controlled by HDFG*W*TR_EL1, not by
HDFG*R*TR_EL1. I set  the nPMSNEVFR_EL1 bit in HDFGWRTR_EL1, and Linux doesn't
hang anymore when I try to write to the register. Will send v2 shortly.

Thanks,

Alex


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