On Thu, Feb 13, 2025 at 11:03 AM Leo Yan <leo.yan@xxxxxxx> wrote: > > On Sun, Feb 02, 2025 at 06:43:04PM -0600, Rob Herring (Arm) wrote: > > [...] > > > +static void __debug_save_brbe(u64 *brbcr_el1) > > +{ > > + *brbcr_el1 = 0; > > + > > + /* Check if the BRBE is enabled */ > > + if (!(read_sysreg_el1(SYS_BRBCR) & (BRBCR_ELx_E0BRE | BRBCR_ELx_ExBRE))) > > + return; > > + > > + /* > > + * Prohibit branch record generation while we are in guest. > > + * Since access to BRBCR_EL1 is trapped, the guest can't > > + * modify the filtering set by the host. > > + */ > > + *brbcr_el1 = read_sysreg_el1(SYS_BRBCR); > > + write_sysreg_el1(0, SYS_BRBCR); > > +} > > Should flush branch record and use isb() before exit host kernel? I don't think so. The isb()'s in the other cases appear to be related to ordering WRT memory buffers. BRBE is just registers. I would assume that there's some barrier before we switch to the guest. > I see inconsistence between the function above and BRBE's disable > function. Here it clears E0BRE / ExBRE bits for disabling BRBE, but the > BRBE driver sets the PAUSED bit in BRBFCR_EL1 for disabling BRBE. Indeed. This works, but the enabled check won't work. I'm going to add clearing BRBCR to brbe_disable(), and this part will stay the same. Rob