Re: [PATCH 1/2] arm64: Add support for FEAT_HAFT

[Marc Zyngier <maz@kernel.org>]

On Fri, 02 Aug 2024 10:34:57 +0100,
Yicong Yang <yangyicong@huawei.com> wrote:
> 
> From: Yicong Yang <yangyicong@hisilicon.com>
> 
> Armv8.9/v9.4 introduces the feature Hardware managed Access Flag
> for Table descriptors (FEAT_HAFT). The feature is indicated by
> ID_AA64MMFR1_EL1.HAFDBS == 0b0011 and can be enabled by
> TCR2_EL1.HAFT so it has a dependency on FEAT_TCR2.
> 
> This patch adds the Kconfig for FEAT_HAFT and support detecting
> and enabling the feature.
> 
> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> ---
>  arch/arm64/Kconfig                     | 20 ++++++++++++++
>  arch/arm64/include/asm/pgtable-hwdef.h |  5 ++++
>  arch/arm64/kernel/cpufeature.c         | 38 ++++++++++++++++++++++++++
>  arch/arm64/tools/cpucaps               |  1 +
>  arch/arm64/tools/sysreg                |  1 +
>  5 files changed, 65 insertions(+)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index b3fc891f1544..f263ae4139a5 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -2127,6 +2127,26 @@ config ARM64_EPAN
>  	  if the cpu does not implement the feature.
>  endmenu # "ARMv8.7 architectural features"
>  
> +menu "ARMv8.9 architectural features"
> +
> +config ARM64_HAFT
> +	bool "Support for Hardware managed Access Flag for Table Descriptor"
> +	depends on ARM64_HW_AFDBM
> +	default y
> +	help
> +	  The ARMv8.9/ARMv9.5 introduces the feature Hardware managed Access
> +	  Flag for Table descriptors. When enabled in TCR_EL1 (HAFT bit) on

TCR2_EL{1,2}. But I don't think we need to details registers and bit
layout in the help section.

> +	  capable processors, an architectural executed memory access will
> +	  update the Access Flag in each Table descriptor which is accessed
> +	  during the translation table walk and for which the Access Flag is
> +	  0. The Access Flag of the Table descriptor use the same bit of
> +	  PTE_AF.
> +
> +	  The feature will only be enabled on supported CPUs. If unsure,
> +	  say Y.
> +
> +endmenu # "ARMv8.9 architectural features"
> +
>  config ARM64_SVE
>  	bool "ARM Scalable Vector Extension support"
>  	default y
> diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h
> index 1f60aa1bc750..47bd29874e62 100644
> --- a/arch/arm64/include/asm/pgtable-hwdef.h
> +++ b/arch/arm64/include/asm/pgtable-hwdef.h
> @@ -308,6 +308,11 @@
>  #define TCR_TCMA1		(UL(1) << 58)
>  #define TCR_DS			(UL(1) << 59)
>  
> +/*
> + * TCR2 Flags
> + */
> +#define TCR2_HAFT		(UL(1) << 11)
> +

TCR2_ELx is already fully described in arch/arm64/tools/sysreg.

>  /*
>   * TTBR.
>   */
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 646ecd3069fd..99402fd00f16 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -2044,6 +2044,29 @@ static bool has_hw_dbm(const struct arm64_cpu_capabilities *cap,
>  
>  #endif
>  
> +#if CONFIG_ARM64_HAFT
> +
> +static void cpu_enable_haft(struct arm64_cpu_capabilities const *cap)
> +{
> +	u64 reg = read_sysreg_s(SYS_TCR2_EL1);
> +
> +	reg |= TCR2_HAFT;
> +	write_sysreg_s(reg, SYS_TCR2_EL1);

Probably more elegantly written as

	sysreg_clear_set_s(SYS_TCR2_EL1, 0, TCR2_EL1x_HAFT);

> +	isb();
> +	local_flush_tlb_all();
> +}
> +
> +static bool has_haft(const struct arm64_cpu_capabilities *cap, int scope)
> +{
> +	/* FEAT_HAFT relies on FEAT_TCR2 */
> +	if (!this_cpu_has_cap(ARM64_HAS_TCR2))
> +		return false;

Why do we need this? If FEAT_TCR2 isn't implemented, this is a HW bug.

> +
> +	return has_cpuid_feature(cap, scope);
> +}
> +
> +#endif
> +
>  #ifdef CONFIG_ARM64_AMU_EXTN
>  
>  /*
> @@ -2580,6 +2603,21 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
>  		.cpus = &dbm_cpus,
>  		ARM64_CPUID_FIELDS(ID_AA64MMFR1_EL1, HAFDBS, DBM)
>  	},
> +#endif
> +#ifdef CONFIG_ARM64_HAFT
> +	{
> +		.desc = "Hardware managed Access Flag for Table Descriptor",
> +		/*
> +		 * Per Spec, software management of Access Flag for Table
> +		 * descriptor is not supported, so make this feature system
> +		 * wide.
> +		 */

I don't understand what you mean by this. Can you please clarify?

> +		.type = ARM64_CPUCAP_BOOT_CPU_FEATURE,
> +		.capability = ARM64_HAFT,
> +		.matches = has_haft,
> +		.cpu_enable = cpu_enable_haft,
> +		ARM64_CPUID_FIELDS(ID_AA64MMFR1_EL1, HAFDBS, HAFT)
> +	},
>  #endif
>  	{
>  		.desc = "CRC32 instructions",
> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
> index ac3429d892b9..0b7a3a237e5d 100644
> --- a/arch/arm64/tools/cpucaps
> +++ b/arch/arm64/tools/cpucaps
> @@ -55,6 +55,7 @@ HAS_TLB_RANGE
>  HAS_VA52
>  HAS_VIRT_HOST_EXTN
>  HAS_WFXT
> +HAFT
>  HW_DBM
>  KVM_HVHE
>  KVM_PROTECTED_MODE
> diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
> index 7ceaa1e0b4bc..9b3d15ea8a63 100644
> --- a/arch/arm64/tools/sysreg
> +++ b/arch/arm64/tools/sysreg
> @@ -1688,6 +1688,7 @@ UnsignedEnum	3:0	HAFDBS
>  	0b0000	NI
>  	0b0001	AF
>  	0b0010	DBM
> +	0b0011	HAFT
>  EndEnum
>  EndSysreg
>  

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.