Messages in this thread Patch in this message | | | Date | Thu, 23 Mar 2023 09:55:47 +0530 | Subject | Re: [PATCH V9 00/10] arm64/perf: Enable branch stack sampling | From | Anshuman Khandual <> |
| |
Hello Mark,
On 3/22/23 00:32, Mark Brown wrote: > On Wed, Mar 15, 2023 at 10:44:34AM +0530, Anshuman Khandual wrote: >> This series enables perf branch stack sampling support on arm64 platform >> via a new arch feature called Branch Record Buffer Extension (BRBE). All >> relevant register definitions could be accessed here. >> >> https://developer.arm.com/documentation/ddi0601/2021-12/AArch64-Registers > > While looking at another feature I noticed that HFGITR_EL2 has two traps > for BRBE instructions, nBRBINJ and nBRBIALL which trap BRB INJ and BRB > IALL. Even if we don't use those right now does it make sense to
Right, current branch stack sampling experiments have been on EL2 host itself.
> document a requirement for those traps to be disabled now in case we > need them later, and do so during EL2 setup for KVM guests? That could > always be done incrementally. Unlike all other instruction trap enable fields in SYS_HFGITR_EL2, these BRBE instructions ones are actually inverted in semantics i.e the particular fields need to be set for these traps to be disabled in EL2.
SYS_HFGITR_EL2.nBRBIALL SYS_HFGITR_EL2.nBRBINJ
By default entire SYS_HFGITR_EL2 is set as cleared during init and that would prevent a guest from using BRBE.
init_kernel_el() init_el2() init_el2_state() __init_el2_fgt() ........ msr_s SYS_HFGITR_EL2, xzr ........
I guess something like the following (untested) needs to be done, to enable BRBE in guests.
diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h index 037724b19c5c..309708127a2a 100644 --- a/arch/arm64/include/asm/el2_setup.h +++ b/arch/arm64/include/asm/el2_setup.h @@ -161,6 +161,15 @@ msr_s SYS_HFGWTR_EL2, x0 msr_s SYS_HFGITR_EL2, xzr + mrs x1, id_aa64dfr0_el1 + ubfx x1, x1, #ID_AA64DFR0_EL1_BRBE_SHIFT, #4 + cbz x1, .Lskip_brbe_\@ + mov x0, xzr + orr x0, x0, #HFGITR_EL2_nBRBIALL + orr x0, x0, #HFGITR_EL2_nBRBINJ + msr_s SYS_HFGITR_EL2, x0 + +.Lskip_brbe_\@: mrs x1, id_aa64pfr0_el1 // AMU traps UNDEF without AMU ubfx x1, x1, #ID_AA64PFR0_EL1_AMU_SHIFT, #4 cbz x1, .Lskip_fgt_\@ diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h index b3bc03ee22bd..3b939c42f3b8 100644 --- a/arch/arm64/include/asm/sysreg.h +++ b/arch/arm64/include/asm/sysreg.h @@ -527,6 +527,9 @@ #define SYS_HFGITR_EL2 sys_reg(3, 4, 1, 1, 6) #define SYS_HACR_EL2 sys_reg(3, 4, 1, 1, 7) +#define HFGITR_EL2_nBRBIALL (BIT(56)) +#define HFGITR_EL2_nBRBINJ (BIT(55)) + #define SYS_TTBR0_EL2 sys_reg(3, 4, 2, 0, 0) #define SYS_TTBR1_EL2 sys_reg(3, 4, 2, 0, 1) #define SYS_TCR_EL2 sys_reg(3, 4, 2, 0, 2)
> > I've got a patch adding the definition of that register to sysreg which > I should be sending shortly, no need to duplicate that effort.
Sure, I assume you are moving the existing definition for SYS_HFGITR_EL2 along with all its fields from ../include/asm/sysreg.h to ../tools/sysreg. Right, it makes sense.
- Anshuman
| |