[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].
> 
> 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
> 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
> 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

A small nit. This URL length causes a checkpatch.pl warning.

WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)

Just wondering, if this URL can be replaced with a document identification
number or something similar for a more cleaner commit message, not sure if
such a thing is even feasible.

> 
> 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);
>  }

FWIW

Reviewed-by: Anshuman Khandual <anshuman.khandual at arm.com>