[PATCH v2] arm64: Avoid repeated AA64MMFR1_EL1 register read on pagefault path

[Anshuman Khandual]

On 1/9/23 20:49, Gabriel Krisman Bertazi wrote:
> Accessing AA64MMFR1_EL1 is expensive in KVM guests, since it is emulated
> in the hypervisor.  In fact, ARM documentation mentions some feature
> registers are not supposed to be accessed frequently by the OS, and
> therefore should be emulated for guests [1].

I am just curious, is this the only system register access (AA64MMFR1_EL1)
causing such performance problems ?

> 
> Commit 0388f9c74330 ("arm64: mm: Implement
> arch_wants_old_prefaulted_pte()") introduced a read of this register in
> the page fault path.  But, even when the feature of setting faultaround

Right, although cpu_has_hw_af() was added earlier via commit 47d7b15b88f9
("arm64: cpufeature: introduce helper cpu_has_hw_af()"), but above commit
did add this on regular page fault path via do_set_pte().


> pages with the old flag is disabled for a given cpu, we are still paying
> the cost of checking the register on every pagefault. This results in an

Right, accessing a system register on each page fault is not optimal.

> explosion of vmexit events in KVM guests, which directly impacts the
> performance of virtualized workloads.  For instance, running kernbench
> yields a 15% increase in system time solely due to the increased vmexit
> cycles.
> 
> This patch avoids the extra cost by using the sanitized cached value.
> It should be safe to do so, since this register mustn't change for a
> given cpu.
> 
> [1] https://developer.arm.com/-/media/Arm%20Developer%20Community/PDF/Learn%20the%20Architecture/Armv8-A%20virtualization.pdf?revision=a765a7df-1a00-434d-b241-357bfda2dd31
> 
> Signed-off-by: Gabriel Krisman Bertazi <krisman at suse.de>
> 
> ---
> Changes since v1:
>   - Rely on read_sanitised_ftr_reg instead of static var (will)
> ---
>  arch/arm64/include/asm/cpufeature.h | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index 03d1c9d7af82..3d1a5677321e 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -864,7 +864,11 @@ static inline bool cpu_has_hw_af(void)
>  	if (!IS_ENABLED(CONFIG_ARM64_HW_AFDBM))
>  		return false;
>  
> -	mmfr1 = read_cpuid(ID_AA64MMFR1_EL1);
> +	/*
> +	 * Use cached version to avoid emulated msr operation on KVM
> +	 * guests.
> +	 */
> +	mmfr1 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR1_EL1);
>  	return cpuid_feature_extract_unsigned_field(mmfr1,
>  						ID_AA64MMFR1_EL1_HAFDBS_SHIFT);
>  }

LGTM but as mentioned earlier, are there not other similar instances or this
is just more problematic being on direct page fault path ?