Re: [PATCH V9 00/10] arm64/perf: Enable branch stack sampling

On 3/23/23 18:24, Mark Brown wrote:

On Thu, Mar 23, 2023 at 09:55:47AM +0530, Anshuman Khandual wrote:

quoted

On 3/22/23 00:32, Mark Brown wrote:

quoted

quoted

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.

quoted

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.

Right, for backwards compatibility all newly added fields are trap by
default.

Okay

quoted

SYS_HFGITR_EL2.nBRBIALL
SYS_HFGITR_EL2.nBRBINJ

quoted

By default entire SYS_HFGITR_EL2 is set as cleared during init and that would
prevent a guest from using BRBE.

It should prevent the host as well shouldn't it? 

In a EL2 host environment, BRBE is being enabled either in EL2 (kernel/hv) or
in EL0 (user space), it never gets enabled on EL1. Moreover BRBIALL/BRBINJ
instructions are always executed while being inside EL2 (kernel/hv). Hence how
could these instructions cause trap in EL2 ?

quoted

I guess something like the following (untested) needs to be done, to enable
BRBE in guests.

quoted

+       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_\@:

Yes, looks roughly what I'd expect.

I could send an stand alone patch after your latest series [1], which disables
BRBINJ/BRBIALL instruction trap in EL2 to enable BRBE usage in the guest.

https://lore.kernel.org/all/20230306-arm64-fgt-reg-gen-v3-2-decba93cbaab@kernel.org/T/ (local)

quoted

quoted

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.

quoted

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.

No fields at the minute but yes, like the other conversions.

Sure.

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