[PATCH v2 0/3] KVM: arm64: Fixes for resolving the fault IPA

[PATCH v2 0/3] KVM: arm64: Fixes for resolving the fault IPA

[Oliver Upton]

KVM's heuristics for determining the fault IPA are a bit shaky and don't
necessarily align with the letter of the architecture. On top of that,
HPFAR_EL2 is UNKNOWN if an SEA occurred during a table walk. Re-walking
the page tables will replay the SEA at EL2, becoming a panic in the hyp.

v1: https://lore.kernel.org/kvmarm/20250401224234.2906739-1-oliver.upton@linux.dev/

v1 -> v2 (addressed Marc's feedback):
 - Drop section reference in favor of the rule for HPFAR_EL2 validity
 - Fold test for Address size fault into helper, reuse existing
   definitions
 - Get rid of 'magic' shift for transforming FIPA field to an actual IPA
 - Add BUG_ON() to handle_host_mem_abort() if we fail to resolve the
   fault IPA.

Oliver Upton (3):
  KVM: arm64: Only read HPFAR_EL2 when value is architecturally valid
  arm64: Convert HPFAR_EL2 to sysreg table
  KVM: arm64: Don't translate FAR if invalid/unsafe

 arch/arm64/include/asm/esr.h           | 44 +++++++++++++++-
 arch/arm64/include/asm/kvm_emulate.h   |  7 ++-
 arch/arm64/include/asm/kvm_ras.h       |  2 +-
 arch/arm64/kvm/hyp/include/hyp/fault.h | 70 ++++++++++++++++++--------
 arch/arm64/kvm/hyp/nvhe/mem_protect.c  |  9 +++-
 arch/arm64/kvm/mmu.c                   | 31 +++++++-----
 arch/arm64/tools/sysreg                |  7 +++
 7 files changed, 131 insertions(+), 39 deletions(-)


base-commit: 369c0122682c4468a69f2454614eded71c5348f3
-- 
2.39.5

[PATCH v2 1/3] KVM: arm64: Only read HPFAR_EL2 when value is architecturally valid

[Oliver Upton]

KVM's logic for deciding when HPFAR_EL2 is UNKNOWN doesn't align with
the architecture. Most notably, KVM assumes HPFAR_EL2 contains the
faulting IPA even in the case of an SEA.

Align the logic with the architecture rather than attempting to
paraphrase it. Additionally, take the opportunity to improve the
language around ARM erratum #834220 such that it actually describes the
bug.

Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/include/asm/esr.h           | 22 ++++++++++--
 arch/arm64/kvm/hyp/include/hyp/fault.h | 46 ++++++++++++++++----------
 2 files changed, 48 insertions(+), 20 deletions(-)

diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
index d1b1a33f9a8b..92fb26e90840 100644
--- a/arch/arm64/include/asm/esr.h
+++ b/arch/arm64/include/asm/esr.h
@@ -121,6 +121,15 @@
 #define ESR_ELx_FSC_SEA_TTW(n)	(0x14 + (n))
 #define ESR_ELx_FSC_SECC	(0x18)
 #define ESR_ELx_FSC_SECC_TTW(n)	(0x1c + (n))
+#define ESR_ELx_FSC_ADDRSZ	(0x00)
+
+/*
+ * Annoyingly, the negative levels for Address size faults aren't laid out
+ * contiguously (or in the desired order)
+ */
+#define ESR_ELx_FSC_ADDRSZ_nL(n)	((n) == -1 ? 0x25 : 0x2C)
+#define ESR_ELx_FSC_ADDRSZ_L(n)		((n) < 0 ? ESR_ELx_FSC_ADDRSZ_nL(n) : \
+						   (ESR_ELx_FSC_ADDRSZ + (n)))
 
 /* Status codes for individual page table levels */
 #define ESR_ELx_FSC_ACCESS_L(n)	(ESR_ELx_FSC_ACCESS + (n))
@@ -161,8 +170,6 @@
 #define ESR_ELx_Xs_MASK		(GENMASK_ULL(4, 0))
 
 /* ISS field definitions for exceptions taken in to Hyp */
-#define ESR_ELx_FSC_ADDRSZ	(0x00)
-#define ESR_ELx_FSC_ADDRSZ_L(n)	(ESR_ELx_FSC_ADDRSZ + (n))
 #define ESR_ELx_CV		(UL(1) << 24)
 #define ESR_ELx_COND_SHIFT	(20)
 #define ESR_ELx_COND_MASK	(UL(0xF) << ESR_ELx_COND_SHIFT)
@@ -464,6 +471,17 @@ static inline bool esr_fsc_is_access_flag_fault(unsigned long esr)
 	       (esr == ESR_ELx_FSC_ACCESS_L(0));
 }
 
+static inline bool esr_fsc_is_addr_sz_fault(unsigned long esr)
+{
+	esr &= ESR_ELx_FSC;
+
+	return (esr == ESR_ELx_FSC_ADDRSZ_L(3))	||
+	       (esr == ESR_ELx_FSC_ADDRSZ_L(2))	||
+	       (esr == ESR_ELx_FSC_ADDRSZ_L(1)) ||
+	       (esr == ESR_ELx_FSC_ADDRSZ_L(0))	||
+	       (esr == ESR_ELx_FSC_ADDRSZ_L(-1));
+}
+
 /* Indicate whether ESR.EC==0x1A is for an ERETAx instruction */
 static inline bool esr_iss_is_eretax(unsigned long esr)
 {
diff --git a/arch/arm64/kvm/hyp/include/hyp/fault.h b/arch/arm64/kvm/hyp/include/hyp/fault.h
index 17df94570f03..59409685c14f 100644
--- a/arch/arm64/kvm/hyp/include/hyp/fault.h
+++ b/arch/arm64/kvm/hyp/include/hyp/fault.h
@@ -44,31 +44,41 @@ static inline bool __translate_far_to_hpfar(u64 far, u64 *hpfar)
 	return true;
 }
 
+/*
+ * Checks for the conditions when HPFAR_EL2 is written, per ARM ARM R_FKLWR.
+ */
+static inline bool __hpfar_valid(u64 esr)
+{
+	/*
+	 * CPUs affected by ARM erratum #834220 may incorrectly report a
+	 * stage-2 translation fault when a stage-1 permission fault occurs.
+	 *
+	 * Re-walk the page tables to determine if a stage-1 fault actually
+	 * occurred.
+	 */
+	if (cpus_have_final_cap(ARM64_WORKAROUND_834220) &&
+	    esr_fsc_is_translation_fault(esr))
+		return false;
+
+	if (esr_fsc_is_translation_fault(esr) || esr_fsc_is_access_flag_fault(esr))
+		return true;
+
+	if ((esr & ESR_ELx_S1PTW) && esr_fsc_is_permission_fault(esr))
+		return true;
+
+	return esr_fsc_is_addr_sz_fault(esr);
+}
+
 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
-	 * bit is clear) and one of the two following cases are true:
-	 *   1. The fault was due to a permission fault
-	 *   2. The processor carries errata 834220
-	 *
-	 * Therefore, for all non S1PTW faults where we either have a
-	 * 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_fsc_is_permission_fault(esr))) {
-		if (!__translate_far_to_hpfar(far, &hpfar))
-			return false;
-	} else {
+	if (__hpfar_valid(esr))
 		hpfar = read_sysreg(hpfar_el2);
-	}
+	else if (!__translate_far_to_hpfar(far, &hpfar))
+		return false;
 
 	fault->far_el2 = far;
 	fault->hpfar_el2 = hpfar;
-- 
2.39.5

Re: [PATCH v2 1/3] KVM: arm64: Only read HPFAR_EL2 when value is architecturally valid

[Marc Zyngier]

On Wed, 02 Apr 2025 21:17:23 +0100,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> KVM's logic for deciding when HPFAR_EL2 is UNKNOWN doesn't align with
> the architecture. Most notably, KVM assumes HPFAR_EL2 contains the
> faulting IPA even in the case of an SEA.
> 
> Align the logic with the architecture rather than attempting to
> paraphrase it. Additionally, take the opportunity to improve the
> language around ARM erratum #834220 such that it actually describes the
> bug.
> 
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>  arch/arm64/include/asm/esr.h           | 22 ++++++++++--
>  arch/arm64/kvm/hyp/include/hyp/fault.h | 46 ++++++++++++++++----------
>  2 files changed, 48 insertions(+), 20 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
> index d1b1a33f9a8b..92fb26e90840 100644
> --- a/arch/arm64/include/asm/esr.h
> +++ b/arch/arm64/include/asm/esr.h
> @@ -121,6 +121,15 @@
>  #define ESR_ELx_FSC_SEA_TTW(n)	(0x14 + (n))
>  #define ESR_ELx_FSC_SECC	(0x18)
>  #define ESR_ELx_FSC_SECC_TTW(n)	(0x1c + (n))
> +#define ESR_ELx_FSC_ADDRSZ	(0x00)
> +
> +/*
> + * Annoyingly, the negative levels for Address size faults aren't laid out
> + * contiguously (or in the desired order)
> + */
> +#define ESR_ELx_FSC_ADDRSZ_nL(n)	((n) == -1 ? 0x25 : 0x2C)
> +#define ESR_ELx_FSC_ADDRSZ_L(n)		((n) < 0 ? ESR_ELx_FSC_ADDRSZ_nL(n) : \
> +						   (ESR_ELx_FSC_ADDRSZ + (n)))

Oh gawd, D128 and its level -2. Do we really want to add this now? I'd
rather leave it to whoever already lives in th future. Specially given
that the helper you introduce below doesn't make any use of it.

Otherwise looks OK.

	M.

-- 
Jazz isn't dead. It just smells funny.

Re: [PATCH v2 1/3] KVM: arm64: Only read HPFAR_EL2 when value is architecturally valid

[Oliver Upton]

On Wed, Apr 02, 2025 at 10:48:43PM +0100, Marc Zyngier wrote:
> On Wed, 02 Apr 2025 21:17:23 +0100,
> Oliver Upton <oliver.upton@linux.dev> wrote:
> > 
> > KVM's logic for deciding when HPFAR_EL2 is UNKNOWN doesn't align with
> > the architecture. Most notably, KVM assumes HPFAR_EL2 contains the
> > faulting IPA even in the case of an SEA.
> > 
> > Align the logic with the architecture rather than attempting to
> > paraphrase it. Additionally, take the opportunity to improve the
> > language around ARM erratum #834220 such that it actually describes the
> > bug.
> > 
> > Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> > ---
> >  arch/arm64/include/asm/esr.h           | 22 ++++++++++--
> >  arch/arm64/kvm/hyp/include/hyp/fault.h | 46 ++++++++++++++++----------
> >  2 files changed, 48 insertions(+), 20 deletions(-)
> > 
> > diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
> > index d1b1a33f9a8b..92fb26e90840 100644
> > --- a/arch/arm64/include/asm/esr.h
> > +++ b/arch/arm64/include/asm/esr.h
> > @@ -121,6 +121,15 @@
> >  #define ESR_ELx_FSC_SEA_TTW(n)	(0x14 + (n))
> >  #define ESR_ELx_FSC_SECC	(0x18)
> >  #define ESR_ELx_FSC_SECC_TTW(n)	(0x1c + (n))
> > +#define ESR_ELx_FSC_ADDRSZ	(0x00)
> > +
> > +/*
> > + * Annoyingly, the negative levels for Address size faults aren't laid out
> > + * contiguously (or in the desired order)
> > + */
> > +#define ESR_ELx_FSC_ADDRSZ_nL(n)	((n) == -1 ? 0x25 : 0x2C)
> > +#define ESR_ELx_FSC_ADDRSZ_L(n)		((n) < 0 ? ESR_ELx_FSC_ADDRSZ_nL(n) : \
> > +						   (ESR_ELx_FSC_ADDRSZ + (n)))
> 
> Oh gawd, D128 and its level -2. Do we really want to add this now?

I certainly don't want to, but the other FSC level macros (whether
intentional or not) already cope with D128. So I did the same for the
sake of consisency.

Thanks,
Oliver

Re: [PATCH v2 1/3] KVM: arm64: Only read HPFAR_EL2 when value is architecturally valid

[Marc Zyngier]

On Wed, 02 Apr 2025 22:52:02 +0100,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> On Wed, Apr 02, 2025 at 10:48:43PM +0100, Marc Zyngier wrote:
> > On Wed, 02 Apr 2025 21:17:23 +0100,
> > Oliver Upton <oliver.upton@linux.dev> wrote:
> > > 
> > > KVM's logic for deciding when HPFAR_EL2 is UNKNOWN doesn't align with
> > > the architecture. Most notably, KVM assumes HPFAR_EL2 contains the
> > > faulting IPA even in the case of an SEA.
> > > 
> > > Align the logic with the architecture rather than attempting to
> > > paraphrase it. Additionally, take the opportunity to improve the
> > > language around ARM erratum #834220 such that it actually describes the
> > > bug.
> > > 
> > > Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> > > ---
> > >  arch/arm64/include/asm/esr.h           | 22 ++++++++++--
> > >  arch/arm64/kvm/hyp/include/hyp/fault.h | 46 ++++++++++++++++----------
> > >  2 files changed, 48 insertions(+), 20 deletions(-)
> > > 
> > > diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
> > > index d1b1a33f9a8b..92fb26e90840 100644
> > > --- a/arch/arm64/include/asm/esr.h
> > > +++ b/arch/arm64/include/asm/esr.h
> > > @@ -121,6 +121,15 @@
> > >  #define ESR_ELx_FSC_SEA_TTW(n)	(0x14 + (n))
> > >  #define ESR_ELx_FSC_SECC	(0x18)
> > >  #define ESR_ELx_FSC_SECC_TTW(n)	(0x1c + (n))
> > > +#define ESR_ELx_FSC_ADDRSZ	(0x00)
> > > +
> > > +/*
> > > + * Annoyingly, the negative levels for Address size faults aren't laid out
> > > + * contiguously (or in the desired order)
> > > + */
> > > +#define ESR_ELx_FSC_ADDRSZ_nL(n)	((n) == -1 ? 0x25 : 0x2C)
> > > +#define ESR_ELx_FSC_ADDRSZ_L(n)		((n) < 0 ? ESR_ELx_FSC_ADDRSZ_nL(n) : \
> > > +						   (ESR_ELx_FSC_ADDRSZ + (n)))
> > 
> > Oh gawd, D128 and its level -2. Do we really want to add this now?
> 
> I certainly don't want to, but the other FSC level macros (whether
> intentional or not) already cope with D128. So I did the same for the
> sake of consisency.

Ah, fair enough. Let's have it then.

Thanks,

	M.

-- 
Jazz isn't dead. It just smells funny.

[PATCH v2 2/3] arm64: Convert HPFAR_EL2 to sysreg table

[Oliver Upton]

Switch over to the typical sysreg table for HPFAR_EL2 as we're about to
start using more fields in the register.

Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/include/asm/kvm_emulate.h  | 4 +++-
 arch/arm64/kvm/hyp/nvhe/mem_protect.c | 2 +-
 arch/arm64/tools/sysreg               | 7 +++++++
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index d7cf66573aca..44e3fc6483c8 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -305,7 +305,9 @@ static __always_inline unsigned long kvm_vcpu_get_hfar(const struct kvm_vcpu *vc
 
 static __always_inline phys_addr_t kvm_vcpu_get_fault_ipa(const struct kvm_vcpu *vcpu)
 {
-	return ((phys_addr_t)vcpu->arch.fault.hpfar_el2 & HPFAR_MASK) << 8;
+	u64 hpfar = vcpu->arch.fault.hpfar_el2;
+
+	return FIELD_GET(HPFAR_EL2_FIPA, hpfar) << 12;
 }
 
 static inline u64 kvm_vcpu_get_disr(const struct kvm_vcpu *vcpu)
diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
index f34f11c720d7..5ce2230054d9 100644
--- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c
+++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
@@ -578,7 +578,7 @@ void handle_host_mem_abort(struct kvm_cpu_context *host_ctxt)
 		return;
 	}
 
-	addr = (fault.hpfar_el2 & HPFAR_MASK) << 8;
+	addr = FIELD_GET(HPFAR_EL2_FIPA, fault.hpfar_el2) << 12;
 	ret = host_stage2_idmap(addr);
 	BUG_ON(ret && ret != -EAGAIN);
 }
diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
index 2c63662c1a48..31ad9ce2b91c 100644
--- a/arch/arm64/tools/sysreg
+++ b/arch/arm64/tools/sysreg
@@ -3433,3 +3433,10 @@ Field	5	F
 Field	4	P
 Field	3:0	Align
 EndSysreg
+
+Sysreg	HPFAR_EL2	3	4	6	0	4
+Field	63	NS
+Res0	62:48
+Field	47:4	FIPA
+Res0	3:0
+EndSysreg
-- 
2.39.5

[PATCH v2 3/3] KVM: arm64: Don't translate FAR if invalid/unsafe

[Oliver Upton]

Don't re-walk the page tables if an SEA occurred during the faulting
page table walk to avoid taking a fatal exception in the hyp.
Additionally, check that FAR_EL2 is valid for SEAs not taken on PTW
as the architecture doesn't guarantee it contains the fault VA.

Finally, fix up the rest of the abort path by checking for SEAs early
and bugging the VM if we get further along with an UNKNOWN fault IPA.

Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/include/asm/esr.h           | 22 ++++++++++++++++++
 arch/arm64/include/asm/kvm_emulate.h   |  3 +++
 arch/arm64/include/asm/kvm_ras.h       |  2 +-
 arch/arm64/kvm/hyp/include/hyp/fault.h | 26 ++++++++++++++++-----
 arch/arm64/kvm/hyp/nvhe/mem_protect.c  |  7 ++++++
 arch/arm64/kvm/mmu.c                   | 31 ++++++++++++++++----------
 6 files changed, 73 insertions(+), 18 deletions(-)

diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
index 92fb26e90840..e4f77757937e 100644
--- a/arch/arm64/include/asm/esr.h
+++ b/arch/arm64/include/asm/esr.h
@@ -482,6 +482,28 @@ static inline bool esr_fsc_is_addr_sz_fault(unsigned long esr)
 	       (esr == ESR_ELx_FSC_ADDRSZ_L(-1));
 }
 
+static inline bool esr_fsc_is_sea_ttw(unsigned long esr)
+{
+	esr = esr & ESR_ELx_FSC;
+
+	return (esr == ESR_ELx_FSC_SEA_TTW(3)) ||
+	       (esr == ESR_ELx_FSC_SEA_TTW(2)) ||
+	       (esr == ESR_ELx_FSC_SEA_TTW(1)) ||
+	       (esr == ESR_ELx_FSC_SEA_TTW(0)) ||
+	       (esr == ESR_ELx_FSC_SEA_TTW(-1));
+}
+
+static inline bool esr_fsc_is_secc_ttw(unsigned long esr)
+{
+	esr = esr & ESR_ELx_FSC;
+
+	return (esr == ESR_ELx_FSC_SECC_TTW(3)) ||
+	       (esr == ESR_ELx_FSC_SECC_TTW(2)) ||
+	       (esr == ESR_ELx_FSC_SECC_TTW(1)) ||
+	       (esr == ESR_ELx_FSC_SECC_TTW(0)) ||
+	       (esr == ESR_ELx_FSC_SECC_TTW(-1));
+}
+
 /* Indicate whether ESR.EC==0x1A is for an ERETAx instruction */
 static inline bool esr_iss_is_eretax(unsigned long esr)
 {
diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index 44e3fc6483c8..bd020fc28aa9 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -307,6 +307,9 @@ static __always_inline phys_addr_t kvm_vcpu_get_fault_ipa(const struct kvm_vcpu
 {
 	u64 hpfar = vcpu->arch.fault.hpfar_el2;
 
+	if (unlikely(!(hpfar & HPFAR_EL2_NS)))
+		return INVALID_GPA;
+
 	return FIELD_GET(HPFAR_EL2_FIPA, hpfar) << 12;
 }
 
diff --git a/arch/arm64/include/asm/kvm_ras.h b/arch/arm64/include/asm/kvm_ras.h
index 87e10d9a635b..9398ade632aa 100644
--- a/arch/arm64/include/asm/kvm_ras.h
+++ b/arch/arm64/include/asm/kvm_ras.h
@@ -14,7 +14,7 @@
  * Was this synchronous external abort a RAS notification?
  * Returns '0' for errors handled by some RAS subsystem, or -ENOENT.
  */
-static inline int kvm_handle_guest_sea(phys_addr_t addr, u64 esr)
+static inline int kvm_handle_guest_sea(void)
 {
 	/* apei_claim_sea(NULL) expects to mask interrupts itself */
 	lockdep_assert_irqs_enabled();
diff --git a/arch/arm64/kvm/hyp/include/hyp/fault.h b/arch/arm64/kvm/hyp/include/hyp/fault.h
index 59409685c14f..fc573fc767b0 100644
--- a/arch/arm64/kvm/hyp/include/hyp/fault.h
+++ b/arch/arm64/kvm/hyp/include/hyp/fault.h
@@ -12,6 +12,16 @@
 #include <asm/kvm_hyp.h>
 #include <asm/kvm_mmu.h>
 
+static inline bool __fault_safe_to_translate(u64 esr)
+{
+	u64 fsc = esr & ESR_ELx_FSC;
+
+	if (esr_fsc_is_sea_ttw(esr) || esr_fsc_is_secc_ttw(esr))
+		return false;
+
+	return !(fsc == ESR_ELx_FSC_EXTABT && (esr & ESR_ELx_FnV));
+}
+
 static inline bool __translate_far_to_hpfar(u64 far, u64 *hpfar)
 {
 	int ret;
@@ -71,17 +81,23 @@ static inline bool __hpfar_valid(u64 esr)
 
 static inline bool __get_fault_info(u64 esr, struct kvm_vcpu_fault_info *fault)
 {
-	u64 hpfar, far;
+	u64 hpfar;
 
-	far = read_sysreg_el2(SYS_FAR);
+	fault->far_el2		= read_sysreg_el2(SYS_FAR);
+	fault->hpfar_el2	= 0;
 
 	if (__hpfar_valid(esr))
 		hpfar = read_sysreg(hpfar_el2);
-	else if (!__translate_far_to_hpfar(far, &hpfar))
+	else if (unlikely(!__fault_safe_to_translate(esr)))
+		return true;
+	else if (!__translate_far_to_hpfar(fault->far_el2, &hpfar))
 		return false;
 
-	fault->far_el2 = far;
-	fault->hpfar_el2 = hpfar;
+	/*
+	 * Hijack HPFAR_EL2.NS (RES0 in Non-secure) to indicate a valid
+	 * HPFAR value.
+	 */
+	fault->hpfar_el2 = hpfar | HPFAR_EL2_NS;
 	return true;
 }
 
diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
index 5ce2230054d9..2a5284f749b4 100644
--- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c
+++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
@@ -578,7 +578,14 @@ void handle_host_mem_abort(struct kvm_cpu_context *host_ctxt)
 		return;
 	}
 
+
+	/*
+	 * Yikes, we couldn't resolve the fault IPA. This should reinject an
+	 * abort into the host when we figure out how to do that.
+	 */
+	BUG_ON(!(fault.hpfar_el2 & HPFAR_EL2_NS));
 	addr = FIELD_GET(HPFAR_EL2_FIPA, fault.hpfar_el2) << 12;
+
 	ret = host_stage2_idmap(addr);
 	BUG_ON(ret && ret != -EAGAIN);
 }
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 2feb6c6b63af..754f2fe0cc67 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -1794,9 +1794,28 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu)
 	gfn_t gfn;
 	int ret, idx;
 
+	/* Synchronous External Abort? */
+	if (kvm_vcpu_abt_issea(vcpu)) {
+		/*
+		 * For RAS the host kernel may handle this abort.
+		 * There is no need to pass the error into the guest.
+		 */
+		if (kvm_handle_guest_sea())
+			kvm_inject_vabt(vcpu);
+
+		return 1;
+	}
+
 	esr = kvm_vcpu_get_esr(vcpu);
 
+	/*
+	 * The fault IPA should be reliable at this point as we're not dealing
+	 * with an SEA.
+	 */
 	ipa = fault_ipa = kvm_vcpu_get_fault_ipa(vcpu);
+	if (KVM_BUG_ON(ipa == INVALID_GPA, vcpu->kvm))
+		return -EFAULT;
+
 	is_iabt = kvm_vcpu_trap_is_iabt(vcpu);
 
 	if (esr_fsc_is_translation_fault(esr)) {
@@ -1818,18 +1837,6 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu)
 		}
 	}
 
-	/* Synchronous External Abort? */
-	if (kvm_vcpu_abt_issea(vcpu)) {
-		/*
-		 * For RAS the host kernel may handle this abort.
-		 * There is no need to pass the error into the guest.
-		 */
-		if (kvm_handle_guest_sea(fault_ipa, kvm_vcpu_get_esr(vcpu)))
-			kvm_inject_vabt(vcpu);
-
-		return 1;
-	}
-
 	trace_kvm_guest_fault(*vcpu_pc(vcpu), kvm_vcpu_get_esr(vcpu),
 			      kvm_vcpu_get_hfar(vcpu), fault_ipa);
 
-- 
2.39.5

Re: [PATCH v2 0/3] KVM: arm64: Fixes for resolving the fault IPA

[Marc Zyngier]

On Wed, 02 Apr 2025 21:17:22 +0100,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> KVM's heuristics for determining the fault IPA are a bit shaky and don't
> necessarily align with the letter of the architecture. On top of that,
> HPFAR_EL2 is UNKNOWN if an SEA occurred during a table walk. Re-walking
> the page tables will replay the SEA at EL2, becoming a panic in the hyp.
> 
> v1: https://lore.kernel.org/kvmarm/20250401224234.2906739-1-oliver.upton@linux.dev/
> 
> v1 -> v2 (addressed Marc's feedback):
>  - Drop section reference in favor of the rule for HPFAR_EL2 validity
>  - Fold test for Address size fault into helper, reuse existing
>    definitions
>  - Get rid of 'magic' shift for transforming FIPA field to an actual IPA
>  - Add BUG_ON() to handle_host_mem_abort() if we fail to resolve the
>    fault IPA.
> 
> Oliver Upton (3):
>   KVM: arm64: Only read HPFAR_EL2 when value is architecturally valid
>   arm64: Convert HPFAR_EL2 to sysreg table
>   KVM: arm64: Don't translate FAR if invalid/unsafe
> 
>  arch/arm64/include/asm/esr.h           | 44 +++++++++++++++-
>  arch/arm64/include/asm/kvm_emulate.h   |  7 ++-
>  arch/arm64/include/asm/kvm_ras.h       |  2 +-
>  arch/arm64/kvm/hyp/include/hyp/fault.h | 70 ++++++++++++++++++--------
>  arch/arm64/kvm/hyp/nvhe/mem_protect.c  |  9 +++-
>  arch/arm64/kvm/mmu.c                   | 31 +++++++-----
>  arch/arm64/tools/sysreg                |  7 +++
>  7 files changed, 131 insertions(+), 39 deletions(-)
> 
> 
> base-commit: 369c0122682c4468a69f2454614eded71c5348f3

My comment on patch #1 notwithstanding, this looks pretty good.

Reviewed-by: Marc Zyngier <maz@kernel.org>

	M.

-- 
Jazz isn't dead. It just smells funny.

Re: [PATCH v2 0/3] KVM: arm64: Fixes for resolving the fault IPA

[Oliver Upton]

On Wed, 02 Apr 2025 13:17:22 -0700, Oliver Upton wrote:
> KVM's heuristics for determining the fault IPA are a bit shaky and don't
> necessarily align with the letter of the architecture. On top of that,
> HPFAR_EL2 is UNKNOWN if an SEA occurred during a table walk. Re-walking
> the page tables will replay the SEA at EL2, becoming a panic in the hyp.
> 
> v1: https://lore.kernel.org/kvmarm/20250401224234.2906739-1-oliver.upton@linux.dev/
> 
> [...]

Applied to fixes, thanks!

[1/3] KVM: arm64: Only read HPFAR_EL2 when value is architecturally valid
      https://git.kernel.org/kvmarm/kvmarm/c/fb8a3eba9c81
[2/3] arm64: Convert HPFAR_EL2 to sysreg table
      https://git.kernel.org/kvmarm/kvmarm/c/1cf3e126f152
[3/3] KVM: arm64: Don't translate FAR if invalid/unsafe
      https://git.kernel.org/kvmarm/kvmarm/c/26fbdf369227

--
Best,
Oliver