[boot-wrapper] aarch64: Enable access into FEAT_FGT2 registers from EL2 and below

[Anshuman Khandual]

On 2/24/25 18:00, Mark Rutland wrote:
> On Tue, Jul 30, 2024 at 11:16:43AM +0530, Anshuman Khandual wrote:
>> FEAT_FGT2 adds system registers HDFGRTR2_EL2, HDFGWTR2_EL2, HFGITR2_EL2,
>> HFGRTR2_EL2 and HFGWTR2_EL2. But access into these system registers from
>> EL2 and below exception levels, will trap into EL3 unless SCR_EL3.FGTEN2
>> is set.
>>
>> Enable access to FEAT_FGT2 registers when they are implemented. Given that
>> these new FEAT_FGT2 trap registers have fields that reset to UNKNOWN values
>> when resets are taken to EL3, this initialises all registers as cleared.
>>
>> Signed-off-by: Anshuman Khandual <anshuman.khandual at arm.com>
>> ---
>> This picks up from an earlier discussion regarding [PATCH 2/2].
>>
>> https://lore.kernel.org/all/20240404073726.947215-1-anshuman.khandual@arm.com/
>>
>>  arch/aarch64/include/asm/cpu.h | 7 +++++++
>>  arch/aarch64/init.c            | 9 +++++++++
>>  2 files changed, 16 insertions(+)
>>
>> diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h
>> index a5744e1..878d03d 100644
>> --- a/arch/aarch64/include/asm/cpu.h
>> +++ b/arch/aarch64/include/asm/cpu.h
>> @@ -23,6 +23,12 @@
>>  #define SCTLR2_EL1		s3_0_c1_c0_3
>>  #define SCTLR2_EL2		s3_4_c1_c0_3
>>  
>> +#define HDFGRTR2_EL2		s3_4_c3_c1_0
>> +#define HDFGWTR2_EL2		s3_4_c3_c1_1
>> +#define HFGRTR2_EL2		s3_4_c3_c1_2
>> +#define HFGWTR2_EL2		s3_4_c3_c1_3
>> +#define HFGITR2_EL2		s3_4_c3_c1_7
> 
> These all look correct per ARM DDI 0487 L.a, table D23-2.
> 
>> +
>>  /*
>>   * RES1 bit definitions definitions as of ARM DDI 0487G.b
>>   *
>> @@ -63,6 +69,7 @@
>>  #define SCR_EL3_SCTLR2En		BIT(44)
>>  #define SCR_EL3_PIEN			BIT(45)
>>  #define SCR_EL3_D128En			BIT(47)
>> +#define SCR_EL3_FGTEN2			BIT(59)
> 
> This looks correct per ARM DDI 0487 L.a, section D24.2.163.
> 
>>  
>>  #define HCR_EL2_RES1			BIT(1)
>>  
>> diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c
>> index c9fc7f1..b4d64a0 100644
>> --- a/arch/aarch64/init.c
>> +++ b/arch/aarch64/init.c
>> @@ -68,6 +68,15 @@ void cpu_init_el3(void)
>>  	if (mrs_field(ID_AA64MMFR0_EL1, FGT))
>>  		scr |= SCR_EL3_FGTEN;
>>  
>> +	if (mrs_field(ID_AA64MMFR0_EL1, FGT) >= 2) {
>> +		scr |= SCR_EL3_FGTEN2;
>> +		msr(HDFGRTR2_EL2, 0);
>> +		msr(HDFGWTR2_EL2, 0);
>> +		msr(HFGITR2_EL2, 0);
>> +		msr(HFGRTR2_EL2, 0);
>> +		msr(HFGWTR2_EL2, 0);
>> +	}
> 
> Superficially, this looks sane to me. I haven't audited all the bits, but I
> assume that you have gone through all of the bits and checked that zero is a
> sensible reset value for all of these.
> 
> I intend to apply this shortly.
> 
> As an aside, I think some necssary support is missing on the kernel side -- I'll reply
> on that front shortly.

Hello Mark,

Seems like neither this boot wrapper patch nor the more recent one which sets
MDCR_EL3.EnPM2 for FEAT_PMUv3p9 registers [1] (as you had pointed out earlier
on the kernel thread) has been applied.

[1] https://lore.kernel.org/all/20250227041603.2029058-1-anshuman.khandual@arm.com/

IIUC both these boot-wrapper patches are necessary for the kernel patch and
also for existing FEAT_PMUv3p9 feature.

https://lore.kernel.org/all/20250227035119.2025171-1-anshuman.khandual@arm.com/

FEAT_PMUv3p9 is the only current user with dependency on FEAT_FGT2 based traps,
beside upcoming FEAT_Debugv8p9 feature which is still under development.

Just wondering if there is something else that still needs to be addressed ?
Please do let me know, will accommodate the changes as required. Thank you.

- Anshuman

> 
> Mark.
> 
>> +
>>  	if (mrs_field(ID_AA64MMFR0_EL1, ECV) >= 2)
>>  		scr |= SCR_EL3_ECVEN;
>>  
>> -- 
>> 2.25.1
>>
>>