| Message ID | 20210824132459.562923-1-alexandru.elisei@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: Do not trap PMSNEVFR_EL1 | expand |
Errr... somehow I forgot to add the arm64 maintainers. Fixing that. On 8/24/21 2:24 PM, 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. > > Fixes: 31c00d2aeaa2 ("arm64: Disable fine grained traps on boot") > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> > --- > 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
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: > >> 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! 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
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
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 <alexandru.elisei@arm.com> --- 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(-)