diff mbox series

[V2,7/7] arm64/boot: Enable EL2 requirements for FEAT_PMUv3p9

Message ID 20250203050828.1049370-8-anshuman.khandual@arm.com (mailing list archive)
State New, archived
Headers show
Series arm64/boot: Enable EL2 requirements for FEAT_PMUv3p9 | expand

Commit Message

Anshuman Khandual Feb. 3, 2025, 5:08 a.m. UTC 
FEAT_PMUv3p9 registers such as PMICNTR_EL0, PMICFILTR_EL0, and PMUACR_EL1
access from EL1 requires appropriate EL2 fine grained trap configuration
via FEAT_FGT2 based trap control registers HDFGRTR2_EL2 and HDFGWTR2_EL2.
Otherwise such register accesses will result in traps into EL2.

Add a new helper __init_el2_fgt2() which initializes FEAT_FGT2 based fine
grained trap control registers HDFGRTR2_EL2 and HDFGWTR2_EL2 (setting the
bits nPMICNTR_EL0, nPMICFILTR_EL0 and nPMUACR_EL1) to enable access into
PMICNTR_EL0, PMICFILTR_EL0, and PMUACR_EL1 registers.

Also update booting.rst with SCR_EL3.FGTEn2 requirement for all FEAT_FGT2
based registers to be accessible in EL2.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Oliver Upton <oliver.upton@linux.dev>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: kvmarm@lists.linux.dev
Tested-by: Rob Herring (Arm) <robh@kernel.org>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 Documentation/arch/arm64/booting.rst | 18 ++++++++++++++++++
 arch/arm64/include/asm/el2_setup.h   | 25 +++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

Comments

Mark Rutland Feb. 24, 2025, 2:11 p.m. UTC | #1 
On Mon, Feb 03, 2025 at 10:38:28AM +0530, Anshuman Khandual wrote:
> FEAT_PMUv3p9 registers such as PMICNTR_EL0, PMICFILTR_EL0, and PMUACR_EL1
> access from EL1 requires appropriate EL2 fine grained trap configuration
> via FEAT_FGT2 based trap control registers HDFGRTR2_EL2 and HDFGWTR2_EL2.
> Otherwise such register accesses will result in traps into EL2.
> 
> Add a new helper __init_el2_fgt2() which initializes FEAT_FGT2 based fine
> grained trap control registers HDFGRTR2_EL2 and HDFGWTR2_EL2 (setting the
> bits nPMICNTR_EL0, nPMICFILTR_EL0 and nPMUACR_EL1) to enable access into
> PMICNTR_EL0, PMICFILTR_EL0, and PMUACR_EL1 registers.
> 
> Also update booting.rst with SCR_EL3.FGTEn2 requirement for all FEAT_FGT2
> based registers to be accessible in EL2.
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Oliver Upton <oliver.upton@linux.dev>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-doc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: kvmarm@lists.linux.dev
> Tested-by: Rob Herring (Arm) <robh@kernel.org>
> Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>  Documentation/arch/arm64/booting.rst | 18 ++++++++++++++++++
>  arch/arm64/include/asm/el2_setup.h   | 25 +++++++++++++++++++++++++
>  2 files changed, 43 insertions(+)

Three things to note here:

(1) I think this is missing some other necessary register configuration.

    From a quick scan, we also require MDCR_EL3.EnPM2 (bit [7]) to be
    configured, which is not described in mainline nor here. If that
    resets to 0, then EL{2,1,0} accesses to a number of registers such
    as PMUACR_EL1 may trap to EL3.

    AFAICT the boot-wrapper resets that bit to 0, so have we actually
    tested all of this with the boot-wrapper? Does TF-A set this bit?

    Are we sure we've cpatured *all* requirements for FEAT_PMUv3p9? i.e.
    is there anything else that we've missed?

(2) This is a fix for !VHE support for PMUACR and ICNTR, where the host
    may run at EL1 and consequently will be affected by fine grained
    traps.

    So this probably needs a CC stable and/or fixes tag, and backport.

(3) As there's no KVM changes, this is only safe provided that the
    registers affected by these fine grained traps are already
    unconditionally trapped by other traps when running a vCPU.

    It looks like PMICNTR_EL0, PMICFILTR_EL0, and PMUACR_EL1 are all
    trapped by MDCR_EL2.TPM, so that should work as long as we emulate
    the PMU. For direct access, FGT2 support will be a prerequisite.

Ideally, we'd have added support for FGT2 before the PMU functionality
that implicitly depends upon it. We should pay more attention to trap
controls in future.

Given (1) and (2) I think someone needs to look into this a bit more and
figure out if this needs a fixup or a respin.

Mark.

> diff --git a/Documentation/arch/arm64/booting.rst b/Documentation/arch/arm64/booting.rst
> index cad6fdc96b98..04d97a1d5ffa 100644
> --- a/Documentation/arch/arm64/booting.rst
> +++ b/Documentation/arch/arm64/booting.rst
> @@ -288,6 +288,12 @@ Before jumping into the kernel, the following conditions must be met:
>  
>      - SCR_EL3.FGTEn (bit 27) must be initialised to 0b1.
>  
> +  For CPUs with the Fine Grained Traps 2 (FEAT_FGT2) extension present:
> +
> +  - If EL3 is present and the kernel is entered at EL2:
> +
> +    - SCR_EL3.FGTEn2 (bit 59) must be initialised to 0b1.
> +
>    For CPUs with support for HCRX_EL2 (FEAT_HCX) present:
>  
>    - If EL3 is present and the kernel is entered at EL2:
> @@ -382,6 +388,18 @@ Before jumping into the kernel, the following conditions must be met:
>  
>      - SMCR_EL2.EZT0 (bit 30) must be initialised to 0b1.
>  
> +  For CPUs with the Performance Monitors Extension (FEAT_PMUv3p9):
> +
> + - If the kernel is entered at EL1 and EL2 is present:
> +
> +    - HDFGRTR2_EL2.nPMICNTR_EL0 (bit 2) must be initialised to 0b1.
> +    - HDFGRTR2_EL2.nPMICFILTR_EL0 (bit 3) must be initialised to 0b1.
> +    - HDFGRTR2_EL2.nPMUACR_EL1 (bit 4) must be initialised to 0b1.
> +
> +    - HDFGWTR2_EL2.nPMICNTR_EL0 (bit 2) must be initialised to 0b1.
> +    - HDFGWTR2_EL2.nPMICFILTR_EL0 (bit 3) must be initialised to 0b1.
> +    - HDFGWTR2_EL2.nPMUACR_EL1 (bit 4) must be initialised to 0b1.
> +
>    For CPUs with Memory Copy and Memory Set instructions (FEAT_MOPS):
>  
>    - If the kernel is entered at EL1 and EL2 is present:
> diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h
> index 25e162651750..1a0071faf57e 100644
> --- a/arch/arm64/include/asm/el2_setup.h
> +++ b/arch/arm64/include/asm/el2_setup.h
> @@ -233,6 +233,30 @@
>  .Lskip_fgt_\@:
>  .endm
>  
> +.macro __init_el2_fgt2
> +	mrs	x1, id_aa64mmfr0_el1
> +	ubfx	x1, x1, #ID_AA64MMFR0_EL1_FGT_SHIFT, #4
> +	cmp	x1, #ID_AA64MMFR0_EL1_FGT_FGT2
> +	b.lt	.Lskip_fgt2_\@
> +
> +	mov	x0, xzr
> +	mrs	x1, id_aa64dfr0_el1
> +	ubfx	x1, x1, #ID_AA64DFR0_EL1_PMUVer_SHIFT, #4
> +	cmp	x1, #ID_AA64DFR0_EL1_PMUVer_V3P9
> +	b.lt	.Lskip_pmuv3p9_\@
> +
> +	orr	x0, x0, #HDFGRTR2_EL2_nPMICNTR_EL0
> +	orr	x0, x0, #HDFGRTR2_EL2_nPMICFILTR_EL0
> +	orr	x0, x0, #HDFGRTR2_EL2_nPMUACR_EL1
> +.Lskip_pmuv3p9_\@:
> +	msr_s   SYS_HDFGRTR2_EL2, x0
> +	msr_s   SYS_HDFGWTR2_EL2, x0
> +	msr_s   SYS_HFGRTR2_EL2, xzr
> +	msr_s   SYS_HFGWTR2_EL2, xzr
> +	msr_s   SYS_HFGITR2_EL2, xzr
> +.Lskip_fgt2_\@:
> +.endm
> +
>  .macro __init_el2_gcs
>  	mrs_s	x1, SYS_ID_AA64PFR1_EL1
>  	ubfx	x1, x1, #ID_AA64PFR1_EL1_GCS_SHIFT, #4
> @@ -283,6 +307,7 @@
>  	__init_el2_nvhe_idregs
>  	__init_el2_cptr
>  	__init_el2_fgt
> +	__init_el2_fgt2
>          __init_el2_gcs
>  .endm
>  
> -- 
> 2.25.1
>
Anshuman Khandual Feb. 25, 2025, 6:17 a.m. UTC | #2 
On 2/24/25 19:41, Mark Rutland wrote:
> On Mon, Feb 03, 2025 at 10:38:28AM +0530, Anshuman Khandual wrote:
>> FEAT_PMUv3p9 registers such as PMICNTR_EL0, PMICFILTR_EL0, and PMUACR_EL1
>> access from EL1 requires appropriate EL2 fine grained trap configuration
>> via FEAT_FGT2 based trap control registers HDFGRTR2_EL2 and HDFGWTR2_EL2.
>> Otherwise such register accesses will result in traps into EL2.
>>
>> Add a new helper __init_el2_fgt2() which initializes FEAT_FGT2 based fine
>> grained trap control registers HDFGRTR2_EL2 and HDFGWTR2_EL2 (setting the
>> bits nPMICNTR_EL0, nPMICFILTR_EL0 and nPMUACR_EL1) to enable access into
>> PMICNTR_EL0, PMICFILTR_EL0, and PMUACR_EL1 registers.
>>
>> Also update booting.rst with SCR_EL3.FGTEn2 requirement for all FEAT_FGT2
>> based registers to be accessible in EL2.
>>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Rob Herring <robh@kernel.org>
>> Cc: Jonathan Corbet <corbet@lwn.net>
>> Cc: Marc Zyngier <maz@kernel.org>
>> Cc: Oliver Upton <oliver.upton@linux.dev>
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-doc@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> Cc: kvmarm@lists.linux.dev
>> Tested-by: Rob Herring (Arm) <robh@kernel.org>
>> Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>>  Documentation/arch/arm64/booting.rst | 18 ++++++++++++++++++
>>  arch/arm64/include/asm/el2_setup.h   | 25 +++++++++++++++++++++++++
>>  2 files changed, 43 insertions(+)
> 
> Three things to note here:
> 
> (1) I think this is missing some other necessary register configuration.
> 
>     From a quick scan, we also require MDCR_EL3.EnPM2 (bit [7]) to be
>     configured, which is not described in mainline nor here. If that

Will update the Documentation/arch/arm64/booting.rst.

>     resets to 0, then EL{2,1,0} accesses to a number of registers such
>     as PMUACR_EL1 may trap to EL3> 
>     AFAICT the boot-wrapper resets that bit to 0, so have we actually
>     tested all of this with the boot-wrapper? Does TF-A set this bit?

Right, boot wrapper resets the bit to 0. We will need the following changes
to set that up when PMUv3p9 is available. MDCR_EL3.EnPM2 also needs to be
set when FEAT_SPMU, FEAT_EBEP, FEAT_PMUv3_SS or FEAT_SPMU2 are implemented.
Should those features be checked here as well ?

--- a/arch/aarch64/include/asm/cpu.h
+++ b/arch/aarch64/include/asm/cpu.h
@@ -56,6 +56,7 @@
 #define MDCR_EL3_SBRBE_NOTRAP_NOPROHIBIT       (UL(3) << 32)
 #define MDCR_EL3_ENPMSN                                BIT(36)
 #define MDCR_EL3_EBWE                          BIT(43)
+#define MDCR_EL3_EnPM2                         BIT(7)
 
 #define SCR_EL3_RES1                   BITS(5, 4)
 #define SCR_EL3_NS                     BIT(0)
@@ -87,6 +88,7 @@
 #define ID_AA64DFR0_EL1_PMSVER         BITS(35, 32)
 #define ID_AA64DFR0_EL1_TRACEBUFFER    BITS(47, 44)
 #define ID_AA64DFR0_EL1_BRBE           BITS(55, 52)
+#define ID_AA64DFR0_EL1_PMUVER         BITS(11, 8)
 #define ID_AA64DFR0_EL1_DEBUGVER       BITS(3, 0)
 
 #define ID_AA64ISAR0_EL1_TME           BITS(27, 24)
diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c
index 54e4cc4..fe7ed5f 100644
--- a/arch/aarch64/init.c
+++ b/arch/aarch64/init.c
@@ -152,6 +152,9 @@ static void cpu_init_el3(void)
        if (mrs_field(ID_AA64DFR0_EL1, DEBUGVER) >= 11)
                mdcr |= MDCR_EL3_EBWE;
 
+       if (mrs_field(ID_AA64DFR0_EL1, PMUVER) >= 0b1001)
+               mdcr |= MDCR_EL3_EnPM2;
+
        msr(MDCR_EL3, mdcr);
 
        if (mrs_field(ID_AA64PFR0_EL1, SVE)) {

MDCR_EL2.EnPM2 does not seem to be set on TFA either, will double check and
get it enabled.

> 
>     Are we sure we've cpatured *all* requirements for FEAT_PMUv3p9? i.e.
>     is there anything else that we've missed?


> 
> (2) This is a fix for !VHE support for PMUACR and ICNTR, where the host
>     may run at EL1 and consequently will be affected by fine grained
>     traps.
> 
>     So this probably needs a CC stable and/or fixes tag, and backport.

Fixes: 0bbff9ed8165 ("perf/arm_pmuv3: Add PMUv3.9 per counter EL0 access control")
Fixes: d8226d8cfbaf ("perf: arm_pmuv3: Add support for Armv9.4 PMU instruction counter")
Cc: stable@vger.kernel.org

But is there a particular stable tree this patch should be addressed ?

> 
> (3) As there's no KVM changes, this is only safe provided that the
>     registers affected by these fine grained traps are already
>     unconditionally trapped by other traps when running a vCPU.
> 
>     It looks like PMICNTR_EL0, PMICFILTR_EL0, and PMUACR_EL1 are all
>     trapped by MDCR_EL2.TPM, so that should work as long as we emulate
>     the PMU. For direct access, FGT2 support will be a prerequisite.
> 
> Ideally, we'd have added support for FGT2 before the PMU functionality
> that implicitly depends upon it. We should pay more attention to trap
> controls in future.
> 
> Given (1) and (2) I think someone needs to look into this a bit more and
> figure out if this needs a fixup or a respin.

To summarize

- Update arm64/booting.rst regarding MDCR_EL3.EnPM2
- Add above mentioned "Fixes:" tag and "CC: stable"
- But should respin this patch or send a fix up instead ?

- Boot wrapper patch setting MDCR_EL3.EnPM2
- TFA patch setting MDCR_EL3.EnPM2

Is there anything else missing ?

> 
> Mark.
> 
>> diff --git a/Documentation/arch/arm64/booting.rst b/Documentation/arch/arm64/booting.rst
>> index cad6fdc96b98..04d97a1d5ffa 100644
>> --- a/Documentation/arch/arm64/booting.rst
>> +++ b/Documentation/arch/arm64/booting.rst
>> @@ -288,6 +288,12 @@ Before jumping into the kernel, the following conditions must be met:
>>  
>>      - SCR_EL3.FGTEn (bit 27) must be initialised to 0b1.
>>  
>> +  For CPUs with the Fine Grained Traps 2 (FEAT_FGT2) extension present:
>> +
>> +  - If EL3 is present and the kernel is entered at EL2:
>> +
>> +    - SCR_EL3.FGTEn2 (bit 59) must be initialised to 0b1.
>> +
>>    For CPUs with support for HCRX_EL2 (FEAT_HCX) present:
>>  
>>    - If EL3 is present and the kernel is entered at EL2:
>> @@ -382,6 +388,18 @@ Before jumping into the kernel, the following conditions must be met:
>>  
>>      - SMCR_EL2.EZT0 (bit 30) must be initialised to 0b1.
>>  
>> +  For CPUs with the Performance Monitors Extension (FEAT_PMUv3p9):
>> +
>> + - If the kernel is entered at EL1 and EL2 is present:
>> +
>> +    - HDFGRTR2_EL2.nPMICNTR_EL0 (bit 2) must be initialised to 0b1.
>> +    - HDFGRTR2_EL2.nPMICFILTR_EL0 (bit 3) must be initialised to 0b1.
>> +    - HDFGRTR2_EL2.nPMUACR_EL1 (bit 4) must be initialised to 0b1.
>> +
>> +    - HDFGWTR2_EL2.nPMICNTR_EL0 (bit 2) must be initialised to 0b1.
>> +    - HDFGWTR2_EL2.nPMICFILTR_EL0 (bit 3) must be initialised to 0b1.
>> +    - HDFGWTR2_EL2.nPMUACR_EL1 (bit 4) must be initialised to 0b1.
>> +
>>    For CPUs with Memory Copy and Memory Set instructions (FEAT_MOPS):
>>  
>>    - If the kernel is entered at EL1 and EL2 is present:
>> diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h
>> index 25e162651750..1a0071faf57e 100644
>> --- a/arch/arm64/include/asm/el2_setup.h
>> +++ b/arch/arm64/include/asm/el2_setup.h
>> @@ -233,6 +233,30 @@
>>  .Lskip_fgt_\@:
>>  .endm
>>  
>> +.macro __init_el2_fgt2
>> +	mrs	x1, id_aa64mmfr0_el1
>> +	ubfx	x1, x1, #ID_AA64MMFR0_EL1_FGT_SHIFT, #4
>> +	cmp	x1, #ID_AA64MMFR0_EL1_FGT_FGT2
>> +	b.lt	.Lskip_fgt2_\@
>> +
>> +	mov	x0, xzr
>> +	mrs	x1, id_aa64dfr0_el1
>> +	ubfx	x1, x1, #ID_AA64DFR0_EL1_PMUVer_SHIFT, #4
>> +	cmp	x1, #ID_AA64DFR0_EL1_PMUVer_V3P9
>> +	b.lt	.Lskip_pmuv3p9_\@
>> +
>> +	orr	x0, x0, #HDFGRTR2_EL2_nPMICNTR_EL0
>> +	orr	x0, x0, #HDFGRTR2_EL2_nPMICFILTR_EL0
>> +	orr	x0, x0, #HDFGRTR2_EL2_nPMUACR_EL1
>> +.Lskip_pmuv3p9_\@:
>> +	msr_s   SYS_HDFGRTR2_EL2, x0
>> +	msr_s   SYS_HDFGWTR2_EL2, x0
>> +	msr_s   SYS_HFGRTR2_EL2, xzr
>> +	msr_s   SYS_HFGWTR2_EL2, xzr
>> +	msr_s   SYS_HFGITR2_EL2, xzr
>> +.Lskip_fgt2_\@:
>> +.endm
>> +
>>  .macro __init_el2_gcs
>>  	mrs_s	x1, SYS_ID_AA64PFR1_EL1
>>  	ubfx	x1, x1, #ID_AA64PFR1_EL1_GCS_SHIFT, #4
>> @@ -283,6 +307,7 @@
>>  	__init_el2_nvhe_idregs
>>  	__init_el2_cptr
>>  	__init_el2_fgt
>> +	__init_el2_fgt2
>>          __init_el2_gcs
>>  .endm
>>  
>> -- 
>> 2.25.1
>>
Rob Herring (Arm) Feb. 26, 2025, 5:14 p.m. UTC | #3 
On Tue, Feb 25, 2025 at 12:17 AM Anshuman Khandual
<anshuman.khandual@arm.com> wrote:
>
>
>
> On 2/24/25 19:41, Mark Rutland wrote:
> > On Mon, Feb 03, 2025 at 10:38:28AM +0530, Anshuman Khandual wrote:
> >> FEAT_PMUv3p9 registers such as PMICNTR_EL0, PMICFILTR_EL0, and PMUACR_EL1
> >> access from EL1 requires appropriate EL2 fine grained trap configuration
> >> via FEAT_FGT2 based trap control registers HDFGRTR2_EL2 and HDFGWTR2_EL2.
> >> Otherwise such register accesses will result in traps into EL2.
> >>
> >> Add a new helper __init_el2_fgt2() which initializes FEAT_FGT2 based fine
> >> grained trap control registers HDFGRTR2_EL2 and HDFGWTR2_EL2 (setting the
> >> bits nPMICNTR_EL0, nPMICFILTR_EL0 and nPMUACR_EL1) to enable access into
> >> PMICNTR_EL0, PMICFILTR_EL0, and PMUACR_EL1 registers.
> >>
> >> Also update booting.rst with SCR_EL3.FGTEn2 requirement for all FEAT_FGT2
> >> based registers to be accessible in EL2.
> >>
> >> Cc: Catalin Marinas <catalin.marinas@arm.com>
> >> Cc: Will Deacon <will@kernel.org>
> >> Cc: Mark Rutland <mark.rutland@arm.com>
> >> Cc: Rob Herring <robh@kernel.org>
> >> Cc: Jonathan Corbet <corbet@lwn.net>
> >> Cc: Marc Zyngier <maz@kernel.org>
> >> Cc: Oliver Upton <oliver.upton@linux.dev>
> >> Cc: linux-arm-kernel@lists.infradead.org
> >> Cc: linux-doc@vger.kernel.org
> >> Cc: linux-kernel@vger.kernel.org
> >> Cc: kvmarm@lists.linux.dev
> >> Tested-by: Rob Herring (Arm) <robh@kernel.org>
> >> Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
> >> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> >> ---
> >>  Documentation/arch/arm64/booting.rst | 18 ++++++++++++++++++
> >>  arch/arm64/include/asm/el2_setup.h   | 25 +++++++++++++++++++++++++
> >>  2 files changed, 43 insertions(+)
> >
> > Three things to note here:
> >
> > (1) I think this is missing some other necessary register configuration.
> >
> >     From a quick scan, we also require MDCR_EL3.EnPM2 (bit [7]) to be
> >     configured, which is not described in mainline nor here. If that
>
> Will update the Documentation/arch/arm64/booting.rst.
>
> >     resets to 0, then EL{2,1,0} accesses to a number of registers such
> >     as PMUACR_EL1 may trap to EL3>
> >     AFAICT the boot-wrapper resets that bit to 0, so have we actually
> >     tested all of this with the boot-wrapper? Does TF-A set this bit?
>
> Right, boot wrapper resets the bit to 0. We will need the following changes
> to set that up when PMUv3p9 is available. MDCR_EL3.EnPM2 also needs to be
> set when FEAT_SPMU, FEAT_EBEP, FEAT_PMUv3_SS or FEAT_SPMU2 are implemented.
> Should those features be checked here as well ?

From Arm ARM:
If FEAT_SPMU is implemented, then FEAT_PMUv3p9 is implemented.
If FEAT_SPMU2 is implemented, then FEAT_SPMU is implemented.
If FEAT_PMUv3_SS is implemented, then FEAT_PMUv3p9 is implemented.

FEAT_EBEP is v9.3 and later. if FEAT_PMUv3p9 is implemented, FEAT_EBEP
is implemented.

So I don't think we need to check anything else.

>
> --- a/arch/aarch64/include/asm/cpu.h
> +++ b/arch/aarch64/include/asm/cpu.h
> @@ -56,6 +56,7 @@
>  #define MDCR_EL3_SBRBE_NOTRAP_NOPROHIBIT       (UL(3) << 32)
>  #define MDCR_EL3_ENPMSN                                BIT(36)
>  #define MDCR_EL3_EBWE                          BIT(43)
> +#define MDCR_EL3_EnPM2                         BIT(7)
>
>  #define SCR_EL3_RES1                   BITS(5, 4)
>  #define SCR_EL3_NS                     BIT(0)
> @@ -87,6 +88,7 @@
>  #define ID_AA64DFR0_EL1_PMSVER         BITS(35, 32)
>  #define ID_AA64DFR0_EL1_TRACEBUFFER    BITS(47, 44)
>  #define ID_AA64DFR0_EL1_BRBE           BITS(55, 52)
> +#define ID_AA64DFR0_EL1_PMUVER         BITS(11, 8)
>  #define ID_AA64DFR0_EL1_DEBUGVER       BITS(3, 0)
>
>  #define ID_AA64ISAR0_EL1_TME           BITS(27, 24)
> diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c
> index 54e4cc4..fe7ed5f 100644
> --- a/arch/aarch64/init.c
> +++ b/arch/aarch64/init.c
> @@ -152,6 +152,9 @@ static void cpu_init_el3(void)
>         if (mrs_field(ID_AA64DFR0_EL1, DEBUGVER) >= 11)
>                 mdcr |= MDCR_EL3_EBWE;
>
> +       if (mrs_field(ID_AA64DFR0_EL1, PMUVER) >= 0b1001)
> +               mdcr |= MDCR_EL3_EnPM2;
> +
>         msr(MDCR_EL3, mdcr);
>
>         if (mrs_field(ID_AA64PFR0_EL1, SVE)) {
>
> MDCR_EL2.EnPM2 does not seem to be set on TFA either, will double check and
> get it enabled.
>
> >
> >     Are we sure we've cpatured *all* requirements for FEAT_PMUv3p9? i.e.
> >     is there anything else that we've missed?

For FEAT_PMUv3_SS, there's MDCR_EL3.EnPMSS, MDCR_EL3.PMSSE, and MDCR_EL2.PMSSE.

For FEAT_EBEP (and SEBEP), there's  MDCR_EL3.PMEE and MDCR_EL2.PMEE

I would assume those become requirements when and if the kernel uses
those features.

>
> >
> > (2) This is a fix for !VHE support for PMUACR and ICNTR, where the host
> >     may run at EL1 and consequently will be affected by fine grained
> >     traps.
> >
> >     So this probably needs a CC stable and/or fixes tag, and backport.
>
> Fixes: 0bbff9ed8165 ("perf/arm_pmuv3: Add PMUv3.9 per counter EL0 access control")
> Fixes: d8226d8cfbaf ("perf: arm_pmuv3: Add support for Armv9.4 PMU instruction counter")
> Cc: stable@vger.kernel.org
>
> But is there a particular stable tree this patch should be addressed ?

It's 6.12+, but the Fixes tags are enough for the tooling to decide.

>
> >
> > (3) As there's no KVM changes, this is only safe provided that the
> >     registers affected by these fine grained traps are already
> >     unconditionally trapped by other traps when running a vCPU.
> >
> >     It looks like PMICNTR_EL0, PMICFILTR_EL0, and PMUACR_EL1 are all
> >     trapped by MDCR_EL2.TPM, so that should work as long as we emulate
> >     the PMU. For direct access, FGT2 support will be a prerequisite.
> >
> > Ideally, we'd have added support for FGT2 before the PMU functionality
> > that implicitly depends upon it. We should pay more attention to trap
> > controls in future.
> >
> > Given (1) and (2) I think someone needs to look into this a bit more and
> > figure out if this needs a fixup or a respin.
>
> To summarize
>
> - Update arm64/booting.rst regarding MDCR_EL3.EnPM2
> - Add above mentioned "Fixes:" tag and "CC: stable"
> - But should respin this patch or send a fix up instead ?

It is not applied yet, right? So respin.

Rob
Catalin Marinas Feb. 26, 2025, 5:44 p.m. UTC | #4 
On Wed, Feb 26, 2025 at 11:14:57AM -0600, Rob Herring wrote:
> On Tue, Feb 25, 2025 at 12:17 AM Anshuman Khandual
> <anshuman.khandual@arm.com> wrote:
> > To summarize
> >
> > - Update arm64/booting.rst regarding MDCR_EL3.EnPM2
> > - Add above mentioned "Fixes:" tag and "CC: stable"
> > - But should respin this patch or send a fix up instead ?
> 
> It is not applied yet, right? So respin.

It's applied but I'm fine with a respin, especially if we need a cc
stable + Fixes, it's easier to track.

Just don't repost the whole series, the first 6 patches are on the arm64
for-next/sysreg branch.

Thanks.
diff mbox series

Patch

diff --git a/Documentation/arch/arm64/booting.rst b/Documentation/arch/arm64/booting.rst
index cad6fdc96b98..04d97a1d5ffa 100644
--- a/Documentation/arch/arm64/booting.rst
+++ b/Documentation/arch/arm64/booting.rst
@@ -288,6 +288,12 @@  Before jumping into the kernel, the following conditions must be met:
 
     - SCR_EL3.FGTEn (bit 27) must be initialised to 0b1.
 
+  For CPUs with the Fine Grained Traps 2 (FEAT_FGT2) extension present:
+
+  - If EL3 is present and the kernel is entered at EL2:
+
+    - SCR_EL3.FGTEn2 (bit 59) must be initialised to 0b1.
+
   For CPUs with support for HCRX_EL2 (FEAT_HCX) present:
 
   - If EL3 is present and the kernel is entered at EL2:
@@ -382,6 +388,18 @@  Before jumping into the kernel, the following conditions must be met:
 
     - SMCR_EL2.EZT0 (bit 30) must be initialised to 0b1.
 
+  For CPUs with the Performance Monitors Extension (FEAT_PMUv3p9):
+
+ - If the kernel is entered at EL1 and EL2 is present:
+
+    - HDFGRTR2_EL2.nPMICNTR_EL0 (bit 2) must be initialised to 0b1.
+    - HDFGRTR2_EL2.nPMICFILTR_EL0 (bit 3) must be initialised to 0b1.
+    - HDFGRTR2_EL2.nPMUACR_EL1 (bit 4) must be initialised to 0b1.
+
+    - HDFGWTR2_EL2.nPMICNTR_EL0 (bit 2) must be initialised to 0b1.
+    - HDFGWTR2_EL2.nPMICFILTR_EL0 (bit 3) must be initialised to 0b1.
+    - HDFGWTR2_EL2.nPMUACR_EL1 (bit 4) must be initialised to 0b1.
+
   For CPUs with Memory Copy and Memory Set instructions (FEAT_MOPS):
 
   - If the kernel is entered at EL1 and EL2 is present:
diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h
index 25e162651750..1a0071faf57e 100644
--- a/arch/arm64/include/asm/el2_setup.h
+++ b/arch/arm64/include/asm/el2_setup.h
@@ -233,6 +233,30 @@ 
 .Lskip_fgt_\@:
 .endm
 
+.macro __init_el2_fgt2
+	mrs	x1, id_aa64mmfr0_el1
+	ubfx	x1, x1, #ID_AA64MMFR0_EL1_FGT_SHIFT, #4
+	cmp	x1, #ID_AA64MMFR0_EL1_FGT_FGT2
+	b.lt	.Lskip_fgt2_\@
+
+	mov	x0, xzr
+	mrs	x1, id_aa64dfr0_el1
+	ubfx	x1, x1, #ID_AA64DFR0_EL1_PMUVer_SHIFT, #4
+	cmp	x1, #ID_AA64DFR0_EL1_PMUVer_V3P9
+	b.lt	.Lskip_pmuv3p9_\@
+
+	orr	x0, x0, #HDFGRTR2_EL2_nPMICNTR_EL0
+	orr	x0, x0, #HDFGRTR2_EL2_nPMICFILTR_EL0
+	orr	x0, x0, #HDFGRTR2_EL2_nPMUACR_EL1
+.Lskip_pmuv3p9_\@:
+	msr_s   SYS_HDFGRTR2_EL2, x0
+	msr_s   SYS_HDFGWTR2_EL2, x0
+	msr_s   SYS_HFGRTR2_EL2, xzr
+	msr_s   SYS_HFGWTR2_EL2, xzr
+	msr_s   SYS_HFGITR2_EL2, xzr
+.Lskip_fgt2_\@:
+.endm
+
 .macro __init_el2_gcs
 	mrs_s	x1, SYS_ID_AA64PFR1_EL1
 	ubfx	x1, x1, #ID_AA64PFR1_EL1_GCS_SHIFT, #4
@@ -283,6 +307,7 @@ 
 	__init_el2_nvhe_idregs
 	__init_el2_cptr
 	__init_el2_fgt
+	__init_el2_fgt2
         __init_el2_gcs
 .endm