[PATCH RESEND v2 0/5] KVM: arm64: nv: Even more VNCR fixes

[PATCH RESEND v2 0/5] KVM: arm64: nv: Even more VNCR fixes

[Oliver Upton]

Argh, resending. Forgot to clean out my staging directory first. Sorry!

Yes, even more.

On top of the first pile:

 - KVM needs to fault in the PFN for write before mapping the L1 VNCR
   with write permissions in the host stage-1

 - Hack to prevent KVM from mapping a non-memory PFN with cacheable
   mappings. This doesn't align perfectly with the semantics at stage-2,
   boo hoo.

 - Bonus change to break VNCR abort loops in case by bugging the VM if
   KVM doesn't know how to handle the fault

Tested by running an L3 VM with kvmarm/next

v1: https://lore.kernel.org/kvmarm/20260608161446.718957-1-oupton@kernel.org/

Oliver Upton (5):
  KVM: arm64: nv: Respect read-only PFN when mapping L1 VNCR
  KVM: arm64: nv: Inject SEA if kvm_translate_vncr() can't resolve PFN
  KVM: arm64: nv: Re-translate VNCR before injecting abort
  KVM: arm64: nv: Inject SEA if guest VNCR isn't normal memory
  KVM: arm64: nv: Mark VM as bugged for unexpected VNCR abort

 arch/arm64/include/asm/kvm_nested.h |   8 ++
 arch/arm64/kvm/at.c                 |   8 --
 arch/arm64/kvm/nested.c             | 164 ++++++++++++++--------------
 3 files changed, 92 insertions(+), 88 deletions(-)


base-commit: 406f0c31f47877db036e885f15830106b89ca950
-- 
2.47.3

[PATCH RESEND v2 1/5] KVM: arm64: nv: Respect read-only PFN when mapping L1 VNCR

[Oliver Upton]

KVM currently maps the L1 VNCR into the host stage-1 by relying entirely
on the permissions of the guest stage-1. At the same time, it is
entirely possible that the backing PFN is read-only (e.g. RO memslot),
meaning that the L1 VNCR should use at most a read-only mapping.

Cache the writability of the PFN in the VNCR TLB and use it to constrain
the resulting fixmap permissions. Promote VNCR permission faults to an
SEA in the case where the guest attempts to write to a read-only
endpoint. Conveniently, this also plugs a page leak found by Sashiko [*]
resulting from the early return for a read-only PFN.

Cc: stable@vger.kernel.org
Fixes: 2a359e072596 ("KVM: arm64: nv: Handle mapping of VNCR_EL2 at EL2")
Link: https://lore.kernel.org/kvm/20260608082603.16AEC1F00893@smtp.kernel.org/
Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 arch/arm64/kvm/nested.c | 36 ++++++++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index 257813289c76..84b3bd528e11 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -24,6 +24,7 @@ struct vncr_tlb {
 	struct s1_walk_result	wr;
 
 	u64			hpa;
+	bool			hpa_writable;
 
 	/* -1 when not mapped on a CPU */
 	int			cpu;
@@ -1395,7 +1396,7 @@ static int kvm_translate_vncr(struct kvm_vcpu *vcpu, bool *is_gmem)
 	if (!*is_gmem) {
 		pfn = __kvm_faultin_pfn(memslot, gfn, write_fault ? FOLL_WRITE : 0,
 					&writable, &page);
-		if (is_error_noslot_pfn(pfn) || (write_fault && !writable))
+		if (is_error_noslot_pfn(pfn))
 			return -EFAULT;
 	} else {
 		ret = kvm_gmem_get_pfn(vcpu->kvm, memslot, gfn, &pfn, &page, NULL);
@@ -1404,6 +1405,8 @@ static int kvm_translate_vncr(struct kvm_vcpu *vcpu, bool *is_gmem)
 					      write_fault, false, false);
 			return ret;
 		}
+
+		writable = !(memslot->flags & KVM_MEM_READONLY);
 	}
 
 	scoped_guard(write_lock, &vcpu->kvm->mmu_lock) {
@@ -1414,28 +1417,41 @@ static int kvm_translate_vncr(struct kvm_vcpu *vcpu, bool *is_gmem)
 
 		vt->gva = va;
 		vt->hpa = pfn << PAGE_SHIFT;
+		vt->hpa_writable = writable;
 		vt->valid = true;
 		vt->cpu = -1;
 
 		kvm_make_request(KVM_REQ_MAP_L1_VNCR_EL2, vcpu);
-		kvm_release_faultin_page(vcpu->kvm, page, false, vt->wr.pw);
+		kvm_release_faultin_page(vcpu->kvm, page, false, vt->wr.pw && vt->hpa_writable);
 	}
 
-	if (vt->wr.pw)
+	if (vt->wr.pw && vt->hpa_writable)
 		mark_page_dirty(vcpu->kvm, gfn);
 
 	return 0;
 }
 
-static void inject_vncr_perm(struct kvm_vcpu *vcpu)
+static void handle_vncr_perm(struct kvm_vcpu *vcpu)
 {
 	struct vncr_tlb *vt = vcpu->arch.vncr_tlb;
 	u64 esr = kvm_vcpu_get_esr(vcpu);
+	u64 fsc;
+
+	/*
+	 * Promote to an external abort if the stage-1 permits writes but the
+	 * HPA is read-only (e.g. RO memslot).
+	 */
+	if (kvm_is_write_fault(vcpu) && vt->wr.pw && !vt->hpa_writable)
+		fsc = ESR_ELx_FSC_EXTABT;
+	/*
+	 * Otherwise, inject a permission fault using the guest's translation
+	 * level rather than the host's.
+	 */
+	else
+		fsc = ESR_ELx_FSC_PERM_L(vt->wr.level);
 
-	/* Adjust the fault level to reflect that of the guest's */
 	esr &= ~ESR_ELx_FSC;
-	esr |= FIELD_PREP(ESR_ELx_FSC,
-			  ESR_ELx_FSC_PERM_L(vt->wr.level));
+	esr |= FIELD_PREP(ESR_ELx_FSC, fsc);
 
 	kvm_inject_nested_sync(vcpu, esr);
 }
@@ -1469,7 +1485,7 @@ int kvm_handle_vncr_abort(struct kvm_vcpu *vcpu)
 		return kvm_handle_guest_sea(vcpu);
 
 	if (esr_fsc_is_permission_fault(esr)) {
-		inject_vncr_perm(vcpu);
+		handle_vncr_perm(vcpu);
 	} else if (esr_fsc_is_translation_fault(esr)) {
 		bool valid, is_gmem = false;
 		int ret;
@@ -1517,7 +1533,7 @@ int kvm_handle_vncr_abort(struct kvm_vcpu *vcpu)
 			break;
 		case -EPERM:
 			/* Hack to deal with POE until we get kernel support */
-			inject_vncr_perm(vcpu);
+			handle_vncr_perm(vcpu);
 			break;
 		case 0:
 			break;
@@ -1561,7 +1577,7 @@ static void kvm_map_l1_vncr(struct kvm_vcpu *vcpu)
 
 	vt->cpu = smp_processor_id();
 
-	if (vt->wr.pw && vt->wr.pr)
+	if (vt->hpa_writable && vt->wr.pw && vt->wr.pr)
 		prot = PAGE_KERNEL;
 	else if (vt->wr.pr)
 		prot = PAGE_KERNEL_RO;
-- 
2.47.3

[PATCH RESEND v2 2/5] KVM: arm64: nv: Inject SEA if kvm_translate_vncr() can't resolve PFN

[Oliver Upton]

kvm_handle_vncr_abort() assumes that s1_walk_result conveys an abort
when kvm_translate_vncr() returns -EFAULT. This is not always the case
as it's possible to encounter 'late' failures on the output of S1
translation, e.g. a GFN outside of the memslots.

Fix it by preparing an external abort before returning from
kvm_translate_vncr().

Cc: stable@vger.kernel.org
Fixes: 2a359e072596 ("KVM: arm64: nv: Handle mapping of VNCR_EL2 at EL2")
Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 arch/arm64/include/asm/kvm_nested.h | 8 ++++++++
 arch/arm64/kvm/at.c                 | 8 --------
 arch/arm64/kvm/nested.c             | 8 ++++++--
 3 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
index dc2957662ff2..cbdaaa2a2903 100644
--- a/arch/arm64/include/asm/kvm_nested.h
+++ b/arch/arm64/include/asm/kvm_nested.h
@@ -388,6 +388,14 @@ struct s1_walk_result {
 	bool	failed;
 };
 
+static inline void fail_s1_walk(struct s1_walk_result *wr, u8 fst, bool s1ptw)
+{
+	wr->fst		= fst;
+	wr->ptw		= s1ptw;
+	wr->s2		= s1ptw;
+	wr->failed	= true;
+}
+
 int __kvm_translate_va(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 		       struct s1_walk_result *wr, u64 va);
 int __kvm_find_s1_desc_level(struct kvm_vcpu *vcpu, u64 va, u64 ipa,
diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c
index 30e6fa8ac07c..8263c648207b 100644
--- a/arch/arm64/kvm/at.c
+++ b/arch/arm64/kvm/at.c
@@ -11,14 +11,6 @@
 #include <asm/kvm_mmu.h>
 #include <asm/lsui.h>
 
-static void fail_s1_walk(struct s1_walk_result *wr, u8 fst, bool s1ptw)
-{
-	wr->fst		= fst;
-	wr->ptw		= s1ptw;
-	wr->s2		= s1ptw;
-	wr->failed	= true;
-}
-
 #define S1_MMU_DISABLED		(-127)
 
 static int get_ia_size(struct s1_walk_info *wi)
diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index 84b3bd528e11..ebd7ccfeee99 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -1389,15 +1389,19 @@ static int kvm_translate_vncr(struct kvm_vcpu *vcpu, bool *is_gmem)
 
 	gfn = vt->wr.pa >> PAGE_SHIFT;
 	memslot = gfn_to_memslot(vcpu->kvm, gfn);
-	if (!memslot)
+	if (!memslot) {
+		fail_s1_walk(&vt->wr, ESR_ELx_FSC_EXTABT, false);
 		return -EFAULT;
+	}
 
 	*is_gmem = kvm_slot_has_gmem(memslot);
 	if (!*is_gmem) {
 		pfn = __kvm_faultin_pfn(memslot, gfn, write_fault ? FOLL_WRITE : 0,
 					&writable, &page);
-		if (is_error_noslot_pfn(pfn))
+		if (is_error_noslot_pfn(pfn)) {
+			fail_s1_walk(&vt->wr, ESR_ELx_FSC_EXTABT, false);
 			return -EFAULT;
+		}
 	} else {
 		ret = kvm_gmem_get_pfn(vcpu->kvm, memslot, gfn, &pfn, &page, NULL);
 		if (ret) {
-- 
2.47.3

[PATCH RESEND v2 3/5] KVM: arm64: nv: Re-translate VNCR before injecting abort

[Oliver Upton]

KVM faults in the VNCR page with FOLL_WRITE whenver the guest aborts for
a write, similar to how a regular stage-2 mapping is handled. It is
entirely possible that the guest reads from the VNCR before writing to
it, in which case the PFN could only be read-only.

Invalidate the VNCR TLB and re-fetch the translation upon taking a VNCR
abort, allowing the host mapping to be faulted in for write the second
time around. Interestingly enough, this also satisfies the ordering
requirements of FEAT_ETS2/3 between descriptor updates and MMU faults.

Cc: stable@vger.kernel.org
Fixes: 2a359e072596 ("KVM: arm64: nv: Handle mapping of VNCR_EL2 at EL2")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 arch/arm64/kvm/nested.c | 115 +++++++++++++++-------------------------
 1 file changed, 44 insertions(+), 71 deletions(-)

diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index ebd7ccfeee99..d5c4b57123a9 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -1460,92 +1460,65 @@ static void handle_vncr_perm(struct kvm_vcpu *vcpu)
 	kvm_inject_nested_sync(vcpu, esr);
 }
 
-static bool kvm_vncr_tlb_lookup(struct kvm_vcpu *vcpu)
-{
-	struct vncr_tlb *vt = vcpu->arch.vncr_tlb;
-
-	lockdep_assert_held_read(&vcpu->kvm->mmu_lock);
-
-	if (!vt->valid)
-		return false;
-
-	if (read_vncr_el2(vcpu) != vt->gva)
-		return false;
-
-	if (vt->wr.nG)
-		return get_asid_by_regime(vcpu, TR_EL20) == vt->wr.asid;
-
-	return true;
-}
-
 int kvm_handle_vncr_abort(struct kvm_vcpu *vcpu)
 {
 	struct vncr_tlb *vt = vcpu->arch.vncr_tlb;
 	u64 esr = kvm_vcpu_get_esr(vcpu);
+	bool is_gmem, perm;
+	int ret;
 
 	WARN_ON_ONCE(!(esr & ESR_ELx_VNCR));
 
 	if (kvm_vcpu_abt_issea(vcpu))
 		return kvm_handle_guest_sea(vcpu);
 
-	if (esr_fsc_is_permission_fault(esr)) {
-		handle_vncr_perm(vcpu);
-	} else if (esr_fsc_is_translation_fault(esr)) {
-		bool valid, is_gmem = false;
-		int ret;
-
-		scoped_guard(read_lock, &vcpu->kvm->mmu_lock)
-			valid = kvm_vncr_tlb_lookup(vcpu);
-
-		if (!valid)
-			ret = kvm_translate_vncr(vcpu, &is_gmem);
-		else
-			ret = -EPERM;
+	if (!esr_fsc_is_translation_fault(esr) && !esr_fsc_is_permission_fault(esr)) {
+		WARN_ONCE(1, "Unhandled VNCR abort, ESR=%llx\n", esr);
+		return 1;
+	}
 
-		switch (ret) {
-		case -EAGAIN:
-			/* Let's try again... */
-			break;
-		case -ENOMEM:
-			/*
-			 * For guest_memfd, this indicates that it failed to
-			 * create a folio to back the memory. Inform userspace.
-			 */
-			if (is_gmem)
-				return 0;
-			/* Otherwise, let's try again... */
-			break;
-		case -EFAULT:
-		case -EIO:
-		case -EHWPOISON:
-			if (is_gmem)
-				return 0;
-			fallthrough;
-		case -EINVAL:
-		case -ENOENT:
-		case -EACCES:
-			/*
-			 * Translation failed, inject the corresponding
-			 * exception back to EL2.
-			 */
-			BUG_ON(!vt->wr.failed);
+	ret = kvm_translate_vncr(vcpu, &is_gmem);
+	switch (ret) {
+	case -EAGAIN:
+		/* Let's try again... */
+		break;
+	case -ENOMEM:
+		/*
+		 * For guest_memfd, this indicates that it failed to
+		 * create a folio to back the memory. Inform userspace.
+		 */
+		if (is_gmem)
+			return 0;
+		/* Otherwise, let's try again... */
+		break;
+	case -EFAULT:
+	case -EIO:
+	case -EHWPOISON:
+		if (is_gmem)
+			return 0;
+		fallthrough;
+	case -EINVAL:
+	case -ENOENT:
+	case -EACCES:
+		/*
+		 * Translation failed, inject the corresponding
+		 * exception back to EL2.
+		 */
+		BUG_ON(!vt->wr.failed);
 
-			esr &= ~ESR_ELx_FSC;
-			esr |= FIELD_PREP(ESR_ELx_FSC, vt->wr.fst);
+		esr &= ~ESR_ELx_FSC;
+		esr |= FIELD_PREP(ESR_ELx_FSC, vt->wr.fst);
 
-			kvm_inject_nested_sync(vcpu, esr);
-			break;
-		case -EPERM:
-			/* Hack to deal with POE until we get kernel support */
-			handle_vncr_perm(vcpu);
-			break;
-		case 0:
-			break;
-		}
-	} else {
-		WARN_ONCE(1, "Unhandled VNCR abort, ESR=%llx\n", esr);
+		kvm_inject_nested_sync(vcpu, esr);
+		break;
+	case 0:
+		break;
 	}
 
+	perm = kvm_is_write_fault(vcpu) ? vt->wr.pw && vt->hpa_writable : vt->wr.pr;
+	if (!perm)
+		handle_vncr_perm(vcpu);
+
 	return 1;
 }
 
-- 
2.47.3

Re: [PATCH RESEND v2 3/5] KVM: arm64: nv: Re-translate VNCR before injecting abort

[Marc Zyngier]

On Tue, 09 Jun 2026 19:55:12 +0100,
Oliver Upton <oupton@kernel.org> wrote:
> 
> KVM faults in the VNCR page with FOLL_WRITE whenver the guest aborts for
> a write, similar to how a regular stage-2 mapping is handled. It is
> entirely possible that the guest reads from the VNCR before writing to
> it, in which case the PFN could only be read-only.
> 
> Invalidate the VNCR TLB and re-fetch the translation upon taking a VNCR
> abort, allowing the host mapping to be faulted in for write the second
> time around. Interestingly enough, this also satisfies the ordering
> requirements of FEAT_ETS2/3 between descriptor updates and MMU faults.
> 
> Cc: stable@vger.kernel.org
> Fixes: 2a359e072596 ("KVM: arm64: nv: Handle mapping of VNCR_EL2 at EL2")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Signed-off-by: Oliver Upton <oupton@kernel.org>
> ---
>  arch/arm64/kvm/nested.c | 115 +++++++++++++++-------------------------
>  1 file changed, 44 insertions(+), 71 deletions(-)
> 
> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index ebd7ccfeee99..d5c4b57123a9 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c
> @@ -1460,92 +1460,65 @@ static void handle_vncr_perm(struct kvm_vcpu *vcpu)
>  	kvm_inject_nested_sync(vcpu, esr);
>  }
>  
> -static bool kvm_vncr_tlb_lookup(struct kvm_vcpu *vcpu)
> -{
> -	struct vncr_tlb *vt = vcpu->arch.vncr_tlb;
> -
> -	lockdep_assert_held_read(&vcpu->kvm->mmu_lock);
> -
> -	if (!vt->valid)
> -		return false;
> -
> -	if (read_vncr_el2(vcpu) != vt->gva)
> -		return false;
> -
> -	if (vt->wr.nG)
> -		return get_asid_by_regime(vcpu, TR_EL20) == vt->wr.asid;
> -
> -	return true;
> -}
> -
>  int kvm_handle_vncr_abort(struct kvm_vcpu *vcpu)
>  {
>  	struct vncr_tlb *vt = vcpu->arch.vncr_tlb;
>  	u64 esr = kvm_vcpu_get_esr(vcpu);
> +	bool is_gmem, perm;
> +	int ret;
>  
>  	WARN_ON_ONCE(!(esr & ESR_ELx_VNCR));
>  
>  	if (kvm_vcpu_abt_issea(vcpu))
>  		return kvm_handle_guest_sea(vcpu);
>  
> -	if (esr_fsc_is_permission_fault(esr)) {
> -		handle_vncr_perm(vcpu);
> -	} else if (esr_fsc_is_translation_fault(esr)) {
> -		bool valid, is_gmem = false;
> -		int ret;
> -
> -		scoped_guard(read_lock, &vcpu->kvm->mmu_lock)
> -			valid = kvm_vncr_tlb_lookup(vcpu);
> -
> -		if (!valid)
> -			ret = kvm_translate_vncr(vcpu, &is_gmem);
> -		else
> -			ret = -EPERM;
> +	if (!esr_fsc_is_translation_fault(esr) && !esr_fsc_is_permission_fault(esr)) {
> +		WARN_ONCE(1, "Unhandled VNCR abort, ESR=%llx\n", esr);
> +		return 1;
> +	}
>  
> -		switch (ret) {
> -		case -EAGAIN:
> -			/* Let's try again... */
> -			break;
> -		case -ENOMEM:
> -			/*
> -			 * For guest_memfd, this indicates that it failed to
> -			 * create a folio to back the memory. Inform userspace.
> -			 */
> -			if (is_gmem)
> -				return 0;
> -			/* Otherwise, let's try again... */
> -			break;
> -		case -EFAULT:
> -		case -EIO:
> -		case -EHWPOISON:
> -			if (is_gmem)
> -				return 0;
> -			fallthrough;
> -		case -EINVAL:
> -		case -ENOENT:
> -		case -EACCES:
> -			/*
> -			 * Translation failed, inject the corresponding
> -			 * exception back to EL2.
> -			 */
> -			BUG_ON(!vt->wr.failed);
> +	ret = kvm_translate_vncr(vcpu, &is_gmem);
> +	switch (ret) {
> +	case -EAGAIN:
> +		/* Let's try again... */
> +		break;
> +	case -ENOMEM:
> +		/*
> +		 * For guest_memfd, this indicates that it failed to
> +		 * create a folio to back the memory. Inform userspace.
> +		 */
> +		if (is_gmem)
> +			return 0;
> +		/* Otherwise, let's try again... */
> +		break;
> +	case -EFAULT:
> +	case -EIO:
> +	case -EHWPOISON:
> +		if (is_gmem)
> +			return 0;
> +		fallthrough;
> +	case -EINVAL:
> +	case -ENOENT:
> +	case -EACCES:
> +		/*
> +		 * Translation failed, inject the corresponding
> +		 * exception back to EL2.
> +		 */
> +		BUG_ON(!vt->wr.failed);

Maybe we can lose this BUG_ON(). We rely on it being correct anyway.

>  
> -			esr &= ~ESR_ELx_FSC;
> -			esr |= FIELD_PREP(ESR_ELx_FSC, vt->wr.fst);
> +		esr &= ~ESR_ELx_FSC;
> +		esr |= FIELD_PREP(ESR_ELx_FSC, vt->wr.fst);
>  
> -			kvm_inject_nested_sync(vcpu, esr);
> -			break;
> -		case -EPERM:
> -			/* Hack to deal with POE until we get kernel support */
> -			handle_vncr_perm(vcpu);
> -			break;
> -		case 0:
> -			break;
> -		}
> -	} else {
> -		WARN_ONCE(1, "Unhandled VNCR abort, ESR=%llx\n", esr);
> +		kvm_inject_nested_sync(vcpu, esr);
> +		break;
> +	case 0:
> +		break;
>  	}
>  
> +	perm = kvm_is_write_fault(vcpu) ? vt->wr.pw && vt->hpa_writable : vt->wr.pr;
> +	if (!perm)
> +		handle_vncr_perm(vcpu);
> +

Isn't there a problem here, where anything that doesn't perform an
early return ends up evaluating the permissions, even if the
translation has failed? My hunch is that you'd want something like
this:

diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index 736fdd6e99cdc..0cefc73d97199 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -1460,13 +1460,12 @@ int kvm_handle_vncr_abort(struct kvm_vcpu *vcpu)
 		kvm_inject_nested_sync(vcpu, esr);
 		break;
 	case 0:
+		perm = kvm_is_write_fault(vcpu) ? vt->wr.pw && vt->hpa_writable : vt->wr.pr;
+		if (!perm)
+			handle_vncr_perm(vcpu);
 		break;
 	}
 
-	perm = kvm_is_write_fault(vcpu) ? vt->wr.pw && vt->hpa_writable : vt->wr.pr;
-	if (!perm)
-		handle_vncr_perm(vcpu);
-
 	return 1;
 }
 

Thoughts?

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH RESEND v2 3/5] KVM: arm64: nv: Re-translate VNCR before injecting abort

[Wei-Lin Chang]

Hi Oliver and Marc,

On Tue, Jun 09, 2026 at 11:55:12AM -0700, Oliver Upton wrote:
> KVM faults in the VNCR page with FOLL_WRITE whenver the guest aborts for
> a write, similar to how a regular stage-2 mapping is handled. It is
> entirely possible that the guest reads from the VNCR before writing to
> it, in which case the PFN could only be read-only.
> 
> Invalidate the VNCR TLB and re-fetch the translation upon taking a VNCR
> abort, allowing the host mapping to be faulted in for write the second
> time around. Interestingly enough, this also satisfies the ordering
> requirements of FEAT_ETS2/3 between descriptor updates and MMU faults.
> 
> Cc: stable@vger.kernel.org
> Fixes: 2a359e072596 ("KVM: arm64: nv: Handle mapping of VNCR_EL2 at EL2")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Signed-off-by: Oliver Upton <oupton@kernel.org>
> ---
>  arch/arm64/kvm/nested.c | 115 +++++++++++++++-------------------------
>  1 file changed, 44 insertions(+), 71 deletions(-)
> 
> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index ebd7ccfeee99..d5c4b57123a9 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c
> @@ -1460,92 +1460,65 @@ static void handle_vncr_perm(struct kvm_vcpu *vcpu)
>  	kvm_inject_nested_sync(vcpu, esr);
>  }
>  
> -static bool kvm_vncr_tlb_lookup(struct kvm_vcpu *vcpu)
> -{
> -	struct vncr_tlb *vt = vcpu->arch.vncr_tlb;
> -
> -	lockdep_assert_held_read(&vcpu->kvm->mmu_lock);
> -
> -	if (!vt->valid)
> -		return false;
> -
> -	if (read_vncr_el2(vcpu) != vt->gva)
> -		return false;
> -
> -	if (vt->wr.nG)
> -		return get_asid_by_regime(vcpu, TR_EL20) == vt->wr.asid;
> -
> -	return true;
> -}
> -
>  int kvm_handle_vncr_abort(struct kvm_vcpu *vcpu)
>  {
>  	struct vncr_tlb *vt = vcpu->arch.vncr_tlb;
>  	u64 esr = kvm_vcpu_get_esr(vcpu);
> +	bool is_gmem, perm;
> +	int ret;
>  
>  	WARN_ON_ONCE(!(esr & ESR_ELx_VNCR));
>  
>  	if (kvm_vcpu_abt_issea(vcpu))
>  		return kvm_handle_guest_sea(vcpu);
>  
> -	if (esr_fsc_is_permission_fault(esr)) {
> -		handle_vncr_perm(vcpu);
> -	} else if (esr_fsc_is_translation_fault(esr)) {
> -		bool valid, is_gmem = false;
> -		int ret;
> -
> -		scoped_guard(read_lock, &vcpu->kvm->mmu_lock)
> -			valid = kvm_vncr_tlb_lookup(vcpu);
> -
> -		if (!valid)
> -			ret = kvm_translate_vncr(vcpu, &is_gmem);
> -		else
> -			ret = -EPERM;
> +	if (!esr_fsc_is_translation_fault(esr) && !esr_fsc_is_permission_fault(esr)) {
> +		WARN_ONCE(1, "Unhandled VNCR abort, ESR=%llx\n", esr);
> +		return 1;
> +	}
>  
> -		switch (ret) {
> -		case -EAGAIN:
> -			/* Let's try again... */
> -			break;
> -		case -ENOMEM:
> -			/*
> -			 * For guest_memfd, this indicates that it failed to
> -			 * create a folio to back the memory. Inform userspace.
> -			 */
> -			if (is_gmem)
> -				return 0;
> -			/* Otherwise, let's try again... */
> -			break;
> -		case -EFAULT:
> -		case -EIO:
> -		case -EHWPOISON:
> -			if (is_gmem)
> -				return 0;
> -			fallthrough;
> -		case -EINVAL:
> -		case -ENOENT:
> -		case -EACCES:
> -			/*
> -			 * Translation failed, inject the corresponding
> -			 * exception back to EL2.
> -			 */
> -			BUG_ON(!vt->wr.failed);
> +	ret = kvm_translate_vncr(vcpu, &is_gmem);
> +	switch (ret) {
> +	case -EAGAIN:
> +		/* Let's try again... */
> +		break;
> +	case -ENOMEM:
> +		/*
> +		 * For guest_memfd, this indicates that it failed to
> +		 * create a folio to back the memory. Inform userspace.
> +		 */
> +		if (is_gmem)
> +			return 0;
> +		/* Otherwise, let's try again... */
> +		break;
> +	case -EFAULT:
> +	case -EIO:
> +	case -EHWPOISON:
> +		if (is_gmem)
> +			return 0;
> +		fallthrough;
> +	case -EINVAL:
> +	case -ENOENT:
> +	case -EACCES:
> +		/*
> +		 * Translation failed, inject the corresponding
> +		 * exception back to EL2.
> +		 */
> +		BUG_ON(!vt->wr.failed);
>  
> -			esr &= ~ESR_ELx_FSC;
> -			esr |= FIELD_PREP(ESR_ELx_FSC, vt->wr.fst);
> +		esr &= ~ESR_ELx_FSC;
> +		esr |= FIELD_PREP(ESR_ELx_FSC, vt->wr.fst);
>  
> -			kvm_inject_nested_sync(vcpu, esr);
> -			break;
> -		case -EPERM:
> -			/* Hack to deal with POE until we get kernel support */
> -			handle_vncr_perm(vcpu);
> -			break;
> -		case 0:
> -			break;
> -		}
> -	} else {
> -		WARN_ONCE(1, "Unhandled VNCR abort, ESR=%llx\n", esr);
> +		kvm_inject_nested_sync(vcpu, esr);
> +		break;
> +	case 0:
> +		break;
>  	}
>  
> +	perm = kvm_is_write_fault(vcpu) ? vt->wr.pw && vt->hpa_writable : vt->wr.pr;
> +	if (!perm)
> +		handle_vncr_perm(vcpu);
> +
>  	return 1;
>  }

Just a comment using this thread:

While reading this, I found this part of the code (not this patch in
particular) a little bit difficult to reason about. I think it's because
kvm_translate_vncr() is doing many things, and there are multiple
potential failure reasons e.g. s1 walk fault, no memslot, gmem/user mem
faultin errors, MMU notifier check, etc., and they are all mux'ed into
an error code with some context visible by the caller.

So in kvm_handle_vncr_abort() we demux the error code and handle the
errors with the help of the context (vt, is_gmem). We essentially have
to keep track of what error codes correspond to what error reasons.

Do you think it is better if we refactor and handle the errors when they
occur? Like inject the exception back to vEL2 right after getting the
results of __kvm_translate_va(), and finish up the abort handling there.
Same for other cases.

I can try it out and make it concrete if you also think this is
reasonable. Probably after this series gets applied when the comments
from Marc & Sashiko are addressed. (I reviewed and don't have additional
comments though.)

Thanks,
Wei-Lin Chang

>  
> -- 
> 2.47.3
> 

[PATCH RESEND v2 4/5] KVM: arm64: nv: Inject SEA if guest VNCR isn't normal memory

[Oliver Upton]

When constructing an L1 VNCR mapping, KVM unconditionally uses cacheable
memory attributes, even if the underlying PFN isn't memory. This gets
particularly hairy if the endpoint doesn't support cacheable memory
attributes, potentially throwing an SError on writeback...

While KVM does permit cacheable memory attributes on certain PFNMAP
VMAs, kvm_translate_vncr() isn't currently grabbing the VMA. So do the
simpler thing for now and just reject everything that isn't memory.

Cc: stable@vger.kernel.org
Fixes: 2a359e072596 ("KVM: arm64: nv: Handle mapping of VNCR_EL2 at EL2")
Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 arch/arm64/kvm/nested.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index d5c4b57123a9..a6bd60856fc3 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -1413,6 +1413,17 @@ static int kvm_translate_vncr(struct kvm_vcpu *vcpu, bool *is_gmem)
 		writable = !(memslot->flags & KVM_MEM_READONLY);
 	}
 
+	/*
+	 * FIXME: This check is too restrictive as KVM allows cacheable memory
+	 * attributes for PFNMAP VMAs that have cacheable attributes in host
+	 * stage-1.
+	 */
+	if (!pfn_is_map_memory(pfn)) {
+		kvm_release_faultin_page(vcpu->kvm, page, true, false);
+		fail_s1_walk(&vt->wr, ESR_ELx_FSC_EXTABT, false);
+		return -EFAULT;
+	}
+
 	scoped_guard(write_lock, &vcpu->kvm->mmu_lock) {
 		if (mmu_invalidate_retry(vcpu->kvm, mmu_seq)) {
 			kvm_release_faultin_page(vcpu->kvm, page, true, false);
-- 
2.47.3

[PATCH RESEND v2 5/5] KVM: arm64: nv: Mark VM as bugged for unexpected VNCR abort

[Oliver Upton]

KVM is unlikely to resolve an unexpected VNCR abort, meaning that
returning to the guest will likely leave the vCPU stuck in an abort
loop. Bug the VM and exit to userspace instead.

Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 arch/arm64/kvm/nested.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index a6bd60856fc3..e9bd4991f7a0 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -1484,8 +1484,8 @@ int kvm_handle_vncr_abort(struct kvm_vcpu *vcpu)
 		return kvm_handle_guest_sea(vcpu);
 
 	if (!esr_fsc_is_translation_fault(esr) && !esr_fsc_is_permission_fault(esr)) {
-		WARN_ONCE(1, "Unhandled VNCR abort, ESR=%llx\n", esr);
-		return 1;
+		KVM_BUG(1, vcpu->kvm, "Unhandled VNCR abort, ESR=%llx\n", esr);
+		return -EIO;
 	}
 
 	ret = kvm_translate_vncr(vcpu, &is_gmem);
-- 
2.47.3