linux-kernel - [PATCH] KVM: arm64: Cleanup the __get_fault_info() to take out the code that validates HPFAR

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220927002715.2142353-1-mizhang@google.com>
Date:   Tue, 27 Sep 2022 00:27:15 +0000
From:   Mingwei Zhang <mizhang@...gle.com>
To:     Marc Zyngier <maz@...nel.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>
Cc:     James Morse <james.morse@....com>,
        Alexandru Elisei <alexandru.elisei@....com>,
        Suzuki K Poulose <suzuki.poulose@....com>,
        Oliver Upton <oliver.upton@...ux.dev>,
        linux-arm-kernel@...ts.infradead.org, kvmarm@...ts.cs.columbia.edu,
        linux-kernel@...r.kernel.org, Oliver Upton <oupton@...gle.com>,
        Mingwei Zhang <mizhang@...gle.com>
Subject: [PATCH] KVM: arm64: Cleanup the __get_fault_info() to take out the
 code that validates HPFAR

Cleanup __get_fault_info() to take out the code that checks HPFAR. The
conditions in __get_fault_info() that checks if HPFAR contains a valid IPA
is slightly messy in that several conditions are written within one IF
statement acrossing multiple lines and are connected with different logical
operators. Among them, some conditions come from ARM Spec, while others
come from CPU erratum. This makes the code hard to read and difficult to
extend.

So, cleanup the function to improve the readability. In particular,
explicitly specify each condition separately within a newly created inline
function.

No functional changes are intended.

Suggested-by: Oliver Upton <oupton@...gle.com>
Signed-off-by: Mingwei Zhang <mizhang@...gle.com>
---
 arch/arm64/kvm/hyp/include/hyp/fault.h | 36 ++++++++++++++++----------
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/arch/arm64/kvm/hyp/include/hyp/fault.h b/arch/arm64/kvm/hyp/include/hyp/fault.h
index 1b8a2dcd712f..4575500d26ff 100644
--- a/arch/arm64/kvm/hyp/include/hyp/fault.h
+++ b/arch/arm64/kvm/hyp/include/hyp/fault.h
@@ -41,12 +41,6 @@ static inline bool __translate_far_to_hpfar(u64 far, u64 *hpfar)
 	return true;
 }
 
-static inline bool __get_fault_info(u64 esr, struct kvm_vcpu_fault_info *fault)
-{
-	u64 hpfar, far;
-
-	far = read_sysreg_el2(SYS_FAR);
-
 	/*
 	 * The HPFAR can be invalid if the stage 2 fault did not
 	 * happen during a stage 1 page table walk (the ESR_EL2.S1PTW
@@ -58,14 +52,30 @@ static inline bool __get_fault_info(u64 esr, struct kvm_vcpu_fault_info *fault)
 	 * permission fault or the errata workaround is enabled, we
 	 * resolve the IPA using the AT instruction.
 	 */
-	if (!(esr & ESR_ELx_S1PTW) &&
-	    (cpus_have_final_cap(ARM64_WORKAROUND_834220) ||
-	     (esr & ESR_ELx_FSC_TYPE) == FSC_PERM)) {
-		if (!__translate_far_to_hpfar(far, &hpfar))
-			return false;
-	} else {
+static inline bool __hpfar_is_valid(u64 esr)
+{
+	if (esr & ESR_ELx_S1PTW)
+		return true;
+
+	if ((esr & ESR_ELx_FSC_TYPE) == FSC_PERM)
+		return false;
+
+	if (cpus_have_final_cap(ARM64_WORKAROUND_834220))
+		return false;
+
+	return true;
+}
+
+static inline bool __get_fault_info(u64 esr, struct kvm_vcpu_fault_info *fault)
+{
+	u64 hpfar, far;
+
+	far = read_sysreg_el2(SYS_FAR);
+
+	if (!__hpfar_is_valid(esr) && !__translate_far_to_hpfar(far, &hpfar))
+		return false;
+	else
 		hpfar = read_sysreg(hpfar_el2);
-	}
 
 	fault->far_el2 = far;
 	fault->hpfar_el2 = hpfar;

base-commit: c59fb127583869350256656b7ed848c398bef879
-- 
2.37.3.998.g577e59143f-goog