Skip to content

Commit

Permalink
KVM: arm64: Only read HPFAR_EL2 when value is architecturally valid
Browse files Browse the repository at this point in the history
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.

Reviewed-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20250402201725.2963645-2-oliver.upton@linux.dev
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
  • Loading branch information
Oliver Upton committed Apr 3, 2025
1 parent 1f5bdd3 commit fb8a3eb
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 20 deletions.
22 changes: 20 additions & 2 deletions arch/arm64/include/asm/esr.h
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
{
Expand Down
46 changes: 28 additions & 18 deletions arch/arm64/kvm/hyp/include/hyp/fault.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit fb8a3eb

Please sign in to comment.