[PATCH v2 00/28] KVM: arm64: nv: Implement FEAT_HAFDBS, FEAT_HAFT

[PATCH v2 00/28] KVM: arm64: nv: Implement FEAT_HAFDBS, FEAT_HAFT

[Oliver Upton]

Here's v2 of the series adding dirty state and table access flags to
KVM's table walkers.

I spent quite a lot of time twiddling with CMO emulation to work around
the architecture. But I eventually got pissed at the whole problem and
tore it up. Ultimately, we shouldn't have to handle every ridiculous
hardware permutation in KVM; assuming FEAT_S2FWB is present on a system
with FEAT_NV2 is not a stretch of the imagination.

So I ran with that and decided to just NOP the CMOs when everything is
observable in the VM at the PoC, including instruction fetches and DMA.

Ultimately, the use of noncoherent DMA with forced writeback is
braindead. On top of that, noncoherent DMA isn't worth caring about in
future development of KVM when disabling FWB outright is a perfectly
viable alternative. Patches #19 and #20 enforce this and provide the
necessary escape valve.

v1 -> v2:
 - Limit FEAT_HAFDBS to VMs that are known to be fully coherent
 - Abuse the above dependency to treat CMOs as NOPs that use a
   writable-clean alias to cacheable memory
 - Pick up a fix for programming TCR_EL2.HA when vEL2 is in the EL2
   translation regime
 - Add command line switch for disabling FEAT_S2FWB, aliased in such a
   way that we can select the 'bypass' memory attributes when S2FWB page
   table formats become mandatory
 - Tracepoints for the MMU walk steps at stage-1 and stage-2
 - Address feedback from Leo, Wei-Lin and Sashiko
 - Collect tags
 - Plenty more I'm forgetting.

Oliver Upton (28):
  KVM: arm64: nv: Introduce struct for stage-2 walk step
  KVM: arm64: nv: Consolidate computation of stage-2 permissions
  KVM: arm64: nv: Get rid of kvm_s2_trans*() accessors
  KVM: arm64: nv: Only shadow writable-dirty guest descs as writable
  KVM: arm64: nv: Pass an access descriptor for stage-2 walks
  KVM: arm64: nv: Use a helper for stage-2 descriptor updates
  KVM: arm64: nv: Set dirty state at stage-2
  KVM: arm64: nv: Treat DBM as writable at stage-2
  KVM: arm64: Compute S1 permissions as part of s1_walk()
  KVM: arm64: Plumb through access descriptor for stage-1
  KVM: arm64: Use a struct for stage-1 walk context
  KVM: arm64: Create helper for stage-1 descriptor updates
  KVM: arm64: Set dirty state at stage-1
  KVM: arm64: nv: Respect HA when computing translated TCR
  KVM: arm64: nv: Respect HD when computing translated TCR
  KVM: arm64: Grant write permission when DBM is set at S1
  KVM: arm64: Don't update descriptors for "non-arch" access
  KVM: arm64: nv: Emulate CMOs via writable-clean translations as NOPs
  arm64: Add command line switch to disable FEAT_S2FWB
  KVM: arm64: Reject non-coherent DMA when using forced writeback
  KVM: arm64: nv: Expose FEAT_HAFDBS
  KVM: arm64: Set Access flag on table descriptors at stage-1
  KVM: arm64: nv: Set access flag on table descriptors at stage-2
  KVM: arm64: nv: Expose FEAT_HAFT
  KVM: arm64: selftests: Only test AF behavior for emulated AT insns
  KVM: arm64: selftests: Test AT emulation for FEAT_HAFT
  KVM: arm64: Add tracepoints for S1 page table walk
  KVM: arm64: Add tracepoints for S2 page table walk

 .../admin-guide/kernel-parameters.txt         |   4 +
 arch/arm64/include/asm/kvm_arm.h              |   1 +
 arch/arm64/include/asm/kvm_emulate.h          |   5 +
 arch/arm64/include/asm/kvm_host.h             |   2 +
 arch/arm64/include/asm/kvm_nested.h           | 110 +++----
 arch/arm64/include/asm/kvm_pgtable.h          |  17 +-
 arch/arm64/kernel/pi/idreg-override.c         |   2 +
 arch/arm64/kvm/arm.c                          |  14 +
 arch/arm64/kvm/at.c                           | 277 +++++++++++------
 arch/arm64/kvm/hyp/pgtable.c                  |  11 -
 arch/arm64/kvm/mmu.c                          |  45 ++-
 arch/arm64/kvm/nested.c                       | 288 +++++++++++++-----
 arch/arm64/kvm/trace_arm.h                    |  99 ++++++
 tools/testing/selftests/kvm/arm64/at.c        | 102 +++++--
 14 files changed, 704 insertions(+), 273 deletions(-)


base-commit: dd0bc00e3e9201fd49b62ff9cc07890c89003afe
-- 
2.47.3

[PATCH v2 01/28] KVM: arm64: nv: Introduce struct for stage-2 walk step

[Oliver Upton]

Factoring out helpers from walk_nested_s2_pgd() will require passing the
context of the current walk step. Prepare by reorganizing the relvant
information into a struct.

Reviewed-by: Leonardo Bras <leo.bras@arm.com>
Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 arch/arm64/kvm/nested.c | 109 +++++++++++++++++++++-------------------
 1 file changed, 57 insertions(+), 52 deletions(-)

diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index f0f473978d98..9ceb3a42767d 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -153,6 +153,12 @@ struct s2_walk_info {
 	bool		ha;
 };
 
+struct s2_walk_step {
+	u64		desc_pa;
+	u64		desc;
+	int		level;
+};
+
 static u32 compute_fsc(int level, u32 fsc)
 {
 	return fsc | (level & 0x3);
@@ -218,13 +224,13 @@ static int check_output_size(struct s2_walk_info *wi, phys_addr_t output)
 	return 0;
 }
 
-static int read_guest_s2_desc(struct kvm_vcpu *vcpu, phys_addr_t pa, u64 *desc,
+static int read_guest_s2_desc(struct kvm_vcpu *vcpu, struct s2_walk_step *ws,
 			      struct s2_walk_info *wi)
 {
 	u64 val;
 	int r;
 
-	r = kvm_read_guest(vcpu->kvm, pa, &val, sizeof(val));
+	r = kvm_read_guest(vcpu->kvm, ws->desc_pa, &val, sizeof(val));
 	if (r)
 		return r;
 
@@ -233,9 +239,9 @@ static int read_guest_s2_desc(struct kvm_vcpu *vcpu, phys_addr_t pa, u64 *desc,
 	 * host and the guest hypervisor.
 	 */
 	if (wi->be)
-		*desc = be64_to_cpu((__force __be64)val);
+		ws->desc = be64_to_cpu((__force __be64)val);
 	else
-		*desc = le64_to_cpu((__force __le64)val);
+		ws->desc = le64_to_cpu((__force __le64)val);
 
 	return 0;
 }
@@ -264,22 +270,22 @@ static int swap_guest_s2_desc(struct kvm_vcpu *vcpu, phys_addr_t pa, u64 old, u6
 static int walk_nested_s2_pgd(struct kvm_vcpu *vcpu, phys_addr_t ipa,
 			      struct s2_walk_info *wi, struct kvm_s2_trans *out)
 {
-	int first_block_level, level, stride, input_size, base_lower_bound;
+	int first_block_level, stride, input_size, base_lower_bound;
+	struct s2_walk_step ws = {};
 	phys_addr_t base_addr;
 	unsigned int addr_top, addr_bottom;
-	u64 desc, new_desc;  /* page table entry */
+	u64 new_desc;  /* page table entry */
 	int ret;
-	phys_addr_t paddr;
 
 	switch (BIT(wi->pgshift)) {
 	default:
 	case SZ_64K:
 	case SZ_16K:
-		level = 3 - wi->sl;
+		ws.level = 3 - wi->sl;
 		first_block_level = 2;
 		break;
 	case SZ_4K:
-		level = 2 - wi->sl;
+		ws.level = 2 - wi->sl;
 		first_block_level = 1;
 		break;
 	}
@@ -289,13 +295,13 @@ static int walk_nested_s2_pgd(struct kvm_vcpu *vcpu, phys_addr_t ipa,
 	if (input_size > 48 || input_size < 25)
 		return -EFAULT;
 
-	ret = check_base_s2_limits(vcpu, wi, level, input_size, stride);
+	ret = check_base_s2_limits(vcpu, wi, ws.level, input_size, stride);
 	if (WARN_ON(ret)) {
 		out->esr = compute_fsc(0, ESR_ELx_FSC_FAULT);
 		return ret;
 	}
 
-	base_lower_bound = 3 + input_size - ((3 - level) * stride +
+	base_lower_bound = 3 + input_size - ((3 - ws.level) * stride +
 			   wi->pgshift);
 	base_addr = wi->baddr & GENMASK_ULL(47, base_lower_bound);
 
@@ -310,96 +316,95 @@ static int walk_nested_s2_pgd(struct kvm_vcpu *vcpu, phys_addr_t ipa,
 	while (1) {
 		phys_addr_t index;
 
-		addr_bottom = (3 - level) * stride + wi->pgshift;
+		addr_bottom = (3 - ws.level) * stride + wi->pgshift;
 		index = (ipa & GENMASK_ULL(addr_top, addr_bottom))
 			>> (addr_bottom - 3);
 
-		paddr = base_addr | index;
-		ret = read_guest_s2_desc(vcpu, paddr, &desc, wi);
+		ws.desc_pa = base_addr | index;
+		ret = read_guest_s2_desc(vcpu, &ws, wi);
 		if (ret < 0) {
-			out->esr = ESR_ELx_FSC_SEA_TTW(level);
+			out->esr = ESR_ELx_FSC_SEA_TTW(ws.level);
 			return ret;
 		}
 
-		new_desc = desc;
+		new_desc = ws.desc;
 
 		/* Check for valid descriptor at this point */
-		if (!(desc & KVM_PTE_VALID)) {
-			out->esr = compute_fsc(level, ESR_ELx_FSC_FAULT);
-			out->desc = desc;
+		if (!(ws.desc & KVM_PTE_VALID)) {
+			out->esr = compute_fsc(ws.level, ESR_ELx_FSC_FAULT);
+			out->desc = ws.desc;
 			return 1;
 		}
 
-		if (FIELD_GET(KVM_PTE_TYPE, desc) == KVM_PTE_TYPE_BLOCK) {
-			if (level < 3)
+		if (FIELD_GET(KVM_PTE_TYPE, ws.desc) == KVM_PTE_TYPE_BLOCK) {
+			if (ws.level < 3)
 				break;
 
-			out->esr = compute_fsc(level, ESR_ELx_FSC_FAULT);
-			out->desc = desc;
+			out->esr = compute_fsc(ws.level, ESR_ELx_FSC_FAULT);
+			out->desc = ws.desc;
 			return 1;
 		}
 
 		/* We're at the final level */
-		if (level == 3)
+		if (ws.level == 3)
 			break;
 
-		if (check_output_size(wi, desc)) {
-			out->esr = compute_fsc(level, ESR_ELx_FSC_ADDRSZ);
-			out->desc = desc;
+		if (check_output_size(wi, ws.desc)) {
+			out->esr = compute_fsc(ws.level, ESR_ELx_FSC_ADDRSZ);
+			out->desc = ws.desc;
 			return 1;
 		}
 
-		base_addr = desc & GENMASK_ULL(47, wi->pgshift);
+		base_addr = ws.desc & GENMASK_ULL(47, wi->pgshift);
 
-		level += 1;
+		ws.level += 1;
 		addr_top = addr_bottom - 1;
 	}
 
-	if (level < first_block_level) {
-		out->esr = compute_fsc(level, ESR_ELx_FSC_FAULT);
-		out->desc = desc;
+	if (ws.level < first_block_level) {
+		out->esr = compute_fsc(ws.level, ESR_ELx_FSC_FAULT);
+		out->desc = ws.desc;
 		return 1;
 	}
 
-	if (check_output_size(wi, desc)) {
-		out->esr = compute_fsc(level, ESR_ELx_FSC_ADDRSZ);
-		out->desc = desc;
+	if (check_output_size(wi, ws.desc)) {
+		out->esr = compute_fsc(ws.level, ESR_ELx_FSC_ADDRSZ);
+		out->desc = ws.desc;
 		return 1;
 	}
 
 	if (wi->ha)
 		new_desc |= KVM_PTE_LEAF_ATTR_LO_S2_AF;
 
-	if (new_desc != desc) {
-		ret = swap_guest_s2_desc(vcpu, paddr, desc, new_desc, wi);
+	if (new_desc != ws.desc) {
+		ret = swap_guest_s2_desc(vcpu, ws.desc_pa, ws.desc, new_desc, wi);
 		if (ret == -EAGAIN)
 			return ret;
 		if (ret) {
-			out->esr = ESR_ELx_FSC_SEA_TTW(level);
-			out->desc = desc;
+			out->esr = ESR_ELx_FSC_SEA_TTW(ws.level);
+			out->desc = ws.desc;
 			return 1;
 		}
 
-		desc = new_desc;
+		ws.desc = new_desc;
 	}
 
-	if (!(desc & KVM_PTE_LEAF_ATTR_LO_S2_AF)) {
-		out->esr = compute_fsc(level, ESR_ELx_FSC_ACCESS);
-		out->desc = desc;
+	if (!(ws.desc & KVM_PTE_LEAF_ATTR_LO_S2_AF)) {
+		out->esr = compute_fsc(ws.level, ESR_ELx_FSC_ACCESS);
+		out->desc = ws.desc;
 		return 1;
 	}
 
-	addr_bottom += contiguous_bit_shift(desc, wi, level);
+	addr_bottom += contiguous_bit_shift(ws.desc, wi, ws.level);
 
 	/* Calculate and return the result */
-	paddr = (desc & GENMASK_ULL(47, addr_bottom)) |
-		(ipa & GENMASK_ULL(addr_bottom - 1, 0));
-	out->output = paddr;
-	out->block_size = 1UL << ((3 - level) * stride + wi->pgshift);
-	out->readable = desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
-	out->writable = desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
-	out->level = level;
-	out->desc = desc;
+	out->output = (ws.desc & GENMASK_ULL(47, addr_bottom)) |
+		      (ipa & GENMASK_ULL(addr_bottom - 1, 0));
+	out->block_size = 1UL << ((3 - ws.level) * stride + wi->pgshift);
+	out->readable = ws.desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
+	out->writable = ws.desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
+	out->level = ws.level;
+	out->desc = ws.desc;
 	return 0;
 }
 
-- 
2.47.3

[PATCH v2 02/28] KVM: arm64: nv: Consolidate computation of stage-2 permissions

[Oliver Upton]

Computing the output permissions from a translation requires
contextualization, as fields in the descriptors may change depending on
the MMU context. Centralize the computation of stage-2 permissions
within the table walk rather as opposed to inspecting the descriptor in
the case of pX and uX permissions.

Reviewed-by: Leonardo Bras <leo.bras@arm.com>
Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 arch/arm64/include/asm/kvm_nested.h  | 42 ++++++++--------------------
 arch/arm64/include/asm/kvm_pgtable.h |  5 ++--
 arch/arm64/kvm/nested.c              | 33 ++++++++++++++++++++--
 3 files changed, 45 insertions(+), 35 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
index 0f899addafa8..ba628d5bbe6e 100644
--- a/arch/arm64/include/asm/kvm_nested.h
+++ b/arch/arm64/include/asm/kvm_nested.h
@@ -93,13 +93,15 @@ extern void kvm_nested_sync_hwstate(struct kvm_vcpu *vcpu);
 extern void kvm_nested_setup_mdcr_el2(struct kvm_vcpu *vcpu);
 
 struct kvm_s2_trans {
-	phys_addr_t output;
-	unsigned long block_size;
-	bool writable;
-	bool readable;
-	int level;
-	u32 esr;
-	u64 desc;
+	u64		desc;
+	phys_addr_t	output;
+	unsigned long	block_size;
+	int		level;
+	u32		esr;
+	bool		writable;
+	bool		readable;
+	bool		px;
+	bool		ux;
 };
 
 static inline phys_addr_t kvm_s2_trans_output(struct kvm_s2_trans *trans)
@@ -135,34 +137,12 @@ static inline bool kvm_has_xnx(struct kvm *kvm)
 
 static inline bool kvm_s2_trans_exec_el0(struct kvm *kvm, struct kvm_s2_trans *trans)
 {
-	u8 xn = FIELD_GET(KVM_PTE_LEAF_ATTR_HI_S2_XN, trans->desc);
-
-	if (!kvm_has_xnx(kvm))
-		xn &= 0b10;
-
-	switch (xn) {
-	case 0b00:
-	case 0b01:
-		return true;
-	default:
-		return false;
-	}
+	return trans->ux;
 }
 
 static inline bool kvm_s2_trans_exec_el1(struct kvm *kvm, struct kvm_s2_trans *trans)
 {
-	u8 xn = FIELD_GET(KVM_PTE_LEAF_ATTR_HI_S2_XN, trans->desc);
-
-	if (!kvm_has_xnx(kvm))
-		xn &= 0b10;
-
-	switch (xn) {
-	case 0b00:
-	case 0b11:
-		return true;
-	default:
-		return false;
-	}
+	return trans->px;
 }
 
 extern int kvm_walk_nested_s2(struct kvm_vcpu *vcpu, phys_addr_t gipa,
diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
index 1c9c57cd6ae4..edfae3a6f362 100644
--- a/arch/arm64/include/asm/kvm_pgtable.h
+++ b/arch/arm64/include/asm/kvm_pgtable.h
@@ -87,6 +87,8 @@ typedef u64 kvm_pte_t;
 #define KVM_PTE_LEAF_ATTR_LO_S2_MEMATTR	GENMASK(5, 2)
 #define KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R	BIT(6)
 #define KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W	BIT(7)
+#define KVM_PTE_LEAF_ATTR_LO_S2_S2AP	(KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R |	\
+					 KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W)
 #define KVM_PTE_LEAF_ATTR_LO_S2_SH	GENMASK(9, 8)
 #define KVM_PTE_LEAF_ATTR_LO_S2_SH_IS	3
 #define KVM_PTE_LEAF_ATTR_LO_S2_AF	BIT(10)
@@ -103,8 +105,7 @@ typedef u64 kvm_pte_t;
 
 #define KVM_PTE_LEAF_ATTR_HI_S1_GP	BIT(50)
 
-#define KVM_PTE_LEAF_ATTR_S2_PERMS	(KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R | \
-					 KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W | \
+#define KVM_PTE_LEAF_ATTR_S2_PERMS	(KVM_PTE_LEAF_ATTR_LO_S2_S2AP |	\
 					 KVM_PTE_LEAF_ATTR_HI_S2_XN)
 
 /* pKVM invalid pte encodings */
diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index 9ceb3a42767d..7bed32c43ea2 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -260,6 +260,36 @@ static int swap_guest_s2_desc(struct kvm_vcpu *vcpu, phys_addr_t pa, u64 old, u6
 	return __kvm_at_swap_desc(vcpu->kvm, pa, old, new);
 }
 
+static void compute_s2_permissions(struct kvm_vcpu *vcpu, struct s2_walk_info *wi,
+				   struct s2_walk_step *ws, struct kvm_s2_trans *trans)
+{
+	u8 s2ap = FIELD_GET(KVM_PTE_LEAF_ATTR_LO_S2_S2AP, ws->desc);
+	u8 xn = FIELD_GET(KVM_PTE_LEAF_ATTR_HI_S2_XN, ws->desc);
+
+	if (!kvm_has_xnx(vcpu->kvm))
+		xn &= 0b10;
+
+	switch (xn) {
+	case 0b00:
+		trans->px = trans->ux = true;
+		break;
+	case 0b01:
+		trans->px = false;
+		trans->ux = true;
+		break;
+	case 0b10:
+		trans->px = trans->ux = false;
+		break;
+	case 0b11:
+		trans->px = true;
+		trans->ux = false;
+		break;
+	}
+
+	trans->readable = s2ap & BIT(0);
+	trans->writable = s2ap & BIT(1);
+}
+
 /*
  * This is essentially a C-version of the pseudo code from the ARM ARM
  * AArch64.TranslationTableWalk  function.  I strongly recommend looking at
@@ -401,8 +431,7 @@ static int walk_nested_s2_pgd(struct kvm_vcpu *vcpu, phys_addr_t ipa,
 	out->output = (ws.desc & GENMASK_ULL(47, addr_bottom)) |
 		      (ipa & GENMASK_ULL(addr_bottom - 1, 0));
 	out->block_size = 1UL << ((3 - ws.level) * stride + wi->pgshift);
-	out->readable = ws.desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
-	out->writable = ws.desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
+	compute_s2_permissions(vcpu, wi, &ws, out);
 	out->level = ws.level;
 	out->desc = ws.desc;
 	return 0;
-- 
2.47.3

[PATCH v2 03/28] KVM: arm64: nv: Get rid of kvm_s2_trans*() accessors

[Oliver Upton]

Access the aptly named fields in kvm_s2_trans directly rather than
indirecting through useless getters.

Reviewed-by: Leonardo Bras <leo.bras@arm.com>
Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 arch/arm64/include/asm/kvm_nested.h | 35 -----------------------------
 arch/arm64/kvm/at.c                 |  6 ++---
 arch/arm64/kvm/mmu.c                | 25 ++++++++++-----------
 arch/arm64/kvm/nested.c             |  4 ++--
 4 files changed, 17 insertions(+), 53 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
index ba628d5bbe6e..01645a5b224f 100644
--- a/arch/arm64/include/asm/kvm_nested.h
+++ b/arch/arm64/include/asm/kvm_nested.h
@@ -104,47 +104,12 @@ struct kvm_s2_trans {
 	bool		ux;
 };
 
-static inline phys_addr_t kvm_s2_trans_output(struct kvm_s2_trans *trans)
-{
-	return trans->output;
-}
-
-static inline unsigned long kvm_s2_trans_size(struct kvm_s2_trans *trans)
-{
-	return trans->block_size;
-}
-
-static inline u32 kvm_s2_trans_esr(struct kvm_s2_trans *trans)
-{
-	return trans->esr;
-}
-
-static inline bool kvm_s2_trans_readable(struct kvm_s2_trans *trans)
-{
-	return trans->readable;
-}
-
-static inline bool kvm_s2_trans_writable(struct kvm_s2_trans *trans)
-{
-	return trans->writable;
-}
-
 static inline bool kvm_has_xnx(struct kvm *kvm)
 {
 	return cpus_have_final_cap(ARM64_HAS_XNX) &&
 		kvm_has_feat(kvm, ID_AA64MMFR1_EL1, XNX, IMP);
 }
 
-static inline bool kvm_s2_trans_exec_el0(struct kvm *kvm, struct kvm_s2_trans *trans)
-{
-	return trans->ux;
-}
-
-static inline bool kvm_s2_trans_exec_el1(struct kvm *kvm, struct kvm_s2_trans *trans)
-{
-	return trans->px;
-}
-
 extern int kvm_walk_nested_s2(struct kvm_vcpu *vcpu, phys_addr_t gipa,
 			      struct kvm_s2_trans *result);
 extern int kvm_s2_handle_perm_fault(struct kvm_vcpu *vcpu,
diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c
index 0926426b8798..12ffd54be2b8 100644
--- a/arch/arm64/kvm/at.c
+++ b/arch/arm64/kvm/at.c
@@ -492,14 +492,14 @@ static int walk_s1(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 				return ret;
 			}
 
-			if (!kvm_s2_trans_readable(&s2_trans)) {
+			if (!s2_trans.readable) {
 				fail_s1_walk(wr, ESR_ELx_FSC_PERM_L(level),
 					     true);
 
 				return -EPERM;
 			}
 
-			ipa = kvm_s2_trans_output(&s2_trans);
+			ipa = s2_trans.output;
 		}
 
 		if (wi->filter) {
@@ -580,7 +580,7 @@ static int walk_s1(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 		new_desc |= PTE_AF;
 
 	if (new_desc != desc) {
-		if (wi->s2 && !kvm_s2_trans_writable(&s2_trans)) {
+		if (wi->s2 && !s2_trans.writable) {
 			fail_s1_walk(wr, ESR_ELx_FSC_PERM_L(level), true);
 			return -EPERM;
 		}
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index a48f3f531e82..cbf3cfb426ba 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -1703,21 +1703,20 @@ static int topup_mmu_memcache(struct kvm_s2_mmu *mmu, void *memcache)
 static enum kvm_pgtable_prot adjust_nested_fault_perms(struct kvm_s2_trans *nested,
 						       enum kvm_pgtable_prot prot)
 {
-	if (!kvm_s2_trans_writable(nested))
+	if (!nested->writable)
 		prot &= ~KVM_PGTABLE_PROT_W;
-	if (!kvm_s2_trans_readable(nested))
+	if (!nested->readable)
 		prot &= ~KVM_PGTABLE_PROT_R;
 
 	return prot | kvm_encode_nested_level(nested);
 }
 
-static enum kvm_pgtable_prot adjust_nested_exec_perms(struct kvm *kvm,
-						      struct kvm_s2_trans *nested,
+static enum kvm_pgtable_prot adjust_nested_exec_perms(struct kvm_s2_trans *nested,
 						      enum kvm_pgtable_prot prot)
 {
-	if (!kvm_s2_trans_exec_el0(kvm, nested))
+	if (!nested->ux)
 		prot &= ~KVM_PGTABLE_PROT_UX;
-	if (!kvm_s2_trans_exec_el1(kvm, nested))
+	if (!nested->px)
 		prot &= ~KVM_PGTABLE_PROT_PX;
 
 	return prot;
@@ -1814,7 +1813,7 @@ static int gmem_abort(const struct kvm_s2_fault_desc *s2fd,
 		prot |= KVM_PGTABLE_PROT_X;
 
 	if (s2fd->nested)
-		prot = adjust_nested_exec_perms(kvm, s2fd->nested, prot);
+		prot = adjust_nested_exec_perms(s2fd->nested, prot);
 
 	kvm_fault_lock(kvm);
 	if (mmu_invalidate_retry(kvm, s2vi.mmu_seq)) {
@@ -1985,7 +1984,7 @@ static short kvm_s2_resolve_vma_size(const struct kvm_s2_fault_desc *s2fd,
 		 * can only create a block mapping if the guest stage 2 page
 		 * table uses at least as big a mapping.
 		 */
-		max_map_size = min(kvm_s2_trans_size(s2fd->nested), max_map_size);
+		max_map_size = min(s2fd->nested->block_size, max_map_size);
 
 		/*
 		 * Be careful that if the mapping size falls between
@@ -2054,7 +2053,7 @@ static gfn_t get_canonical_gfn(const struct kvm_s2_fault_desc *s2fd,
 	if (!s2fd->nested)
 		return s2vi->gfn;
 
-	ipa = kvm_s2_trans_output(s2fd->nested);
+	ipa = s2fd->nested->output;
 	return ALIGN_DOWN(ipa, s2vi->vma_pagesize) >> PAGE_SHIFT;
 }
 
@@ -2158,7 +2157,7 @@ static int kvm_s2_fault_compute_prot(const struct kvm_s2_fault_desc *s2fd,
 		*prot |= KVM_PGTABLE_PROT_X;
 
 	if (s2fd->nested)
-		*prot = adjust_nested_exec_perms(kvm, s2fd->nested, *prot);
+		*prot = adjust_nested_exec_perms(s2fd->nested, *prot);
 
 	if (!kvm_s2_fault_is_perm(s2fd) && !s2vi->map_non_cacheable && kvm_has_mte(kvm)) {
 		/* Check the VMM hasn't introduced a new disallowed VMA */
@@ -2529,19 +2528,19 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu)
 		}
 
 		if (ret) {
-			esr = kvm_s2_trans_esr(&nested_trans);
+			esr = nested_trans.esr;
 			kvm_inject_s2_fault(vcpu, esr);
 			goto out_unlock;
 		}
 
 		ret = kvm_s2_handle_perm_fault(vcpu, &nested_trans);
 		if (ret) {
-			esr = kvm_s2_trans_esr(&nested_trans);
+			esr = nested_trans.esr;
 			kvm_inject_s2_fault(vcpu, esr);
 			goto out_unlock;
 		}
 
-		ipa = kvm_s2_trans_output(&nested_trans);
+		ipa = nested_trans.output;
 		nested = &nested_trans;
 	}
 
diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index 7bed32c43ea2..15c22a218609 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -1089,9 +1089,9 @@ int kvm_s2_handle_perm_fault(struct kvm_vcpu *vcpu, struct kvm_s2_trans *trans)
 
 	if (kvm_vcpu_trap_is_iabt(vcpu)) {
 		if (vcpu_mode_priv(vcpu))
-			forward_fault = !kvm_s2_trans_exec_el1(vcpu->kvm, trans);
+			forward_fault = !trans->px;
 		else
-			forward_fault = !kvm_s2_trans_exec_el0(vcpu->kvm, trans);
+			forward_fault = !trans->ux;
 	} else {
 		bool write_fault = kvm_is_write_fault(vcpu);
 
-- 
2.47.3

[PATCH v2 04/28] KVM: arm64: nv: Only shadow writable-dirty guest descs as writable

[Oliver Upton]

KVM will soon add support for hardware dirty state management for nested
guests. In order to emulate dirty state transitions on the guest
desciptor, KVM will need to use read-only hardware mappings and re-walk
the guest page tables upon taking a permission fault.

Prepare by limiting shadow stage-2 and shadow VNCR translations to
read-only for writable-clean guest translations.

Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 arch/arm64/include/asm/kvm_nested.h |  2 ++
 arch/arm64/kvm/at.c                 |  2 ++
 arch/arm64/kvm/mmu.c                |  2 +-
 arch/arm64/kvm/nested.c             | 15 +++++++++++----
 4 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
index 01645a5b224f..e8d5ae7afb9a 100644
--- a/arch/arm64/include/asm/kvm_nested.h
+++ b/arch/arm64/include/asm/kvm_nested.h
@@ -100,6 +100,7 @@ struct kvm_s2_trans {
 	u32		esr;
 	bool		writable;
 	bool		readable;
+	bool		dirty;
 	bool		px;
 	bool		ux;
 };
@@ -344,6 +345,7 @@ struct s1_walk_result {
 			bool	pr;
 			bool	pw;
 			bool	px;
+			bool	dirty;
 		};
 		struct {
 			u8	fst;
diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c
index 12ffd54be2b8..4e505433299b 100644
--- a/arch/arm64/kvm/at.c
+++ b/arch/arm64/kvm/at.c
@@ -1315,6 +1315,7 @@ static void compute_s1_permissions(struct kvm_vcpu *vcpu,
 			  (pan3_enabled(vcpu, wi->regime) && wr->ux));
 	wr->pw &= !pan;
 	wr->pr &= !pan;
+	wr->dirty = !(wr->desc & BIT(7));
 }
 
 static int handle_at_slow(struct kvm_vcpu *vcpu, u32 op, u64 vaddr, u64 *par)
@@ -1673,6 +1674,7 @@ int __kvm_translate_va(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 	if (wr->level == S1_MMU_DISABLED) {
 		wr->ur = wr->uw = wr->ux = true;
 		wr->pr = wr->pw = wr->px = true;
+		wr->dirty = true;
 	} else {
 		ret = walk_s1(vcpu, wi, wr, va);
 		if (ret)
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index cbf3cfb426ba..84aa1bb31095 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -1703,7 +1703,7 @@ static int topup_mmu_memcache(struct kvm_s2_mmu *mmu, void *memcache)
 static enum kvm_pgtable_prot adjust_nested_fault_perms(struct kvm_s2_trans *nested,
 						       enum kvm_pgtable_prot prot)
 {
-	if (!nested->writable)
+	if (!(nested->writable && nested->dirty))
 		prot &= ~KVM_PGTABLE_PROT_W;
 	if (!nested->readable)
 		prot &= ~KVM_PGTABLE_PROT_R;
diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index 15c22a218609..9b2ea83aeccd 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -288,6 +288,8 @@ static void compute_s2_permissions(struct kvm_vcpu *vcpu, struct s2_walk_info *w
 
 	trans->readable = s2ap & BIT(0);
 	trans->writable = s2ap & BIT(1);
+
+	trans->dirty = ws->desc & BIT(7);
 }
 
 /*
@@ -1587,6 +1589,11 @@ static u64 read_vncr_el2(struct kvm_vcpu *vcpu)
 	return (u64)sign_extend64(__vcpu_sys_reg(vcpu, VNCR_EL2), 48);
 }
 
+static bool vncr_can_map_writable(struct vncr_tlb *vt)
+{
+	return vt->wr.pw && vt->wr.dirty && vt->hpa_writable;
+}
+
 static int kvm_translate_vncr(struct kvm_vcpu *vcpu, bool *is_gmem)
 {
 	struct kvm_memory_slot *memslot;
@@ -1683,10 +1690,10 @@ static int kvm_translate_vncr(struct kvm_vcpu *vcpu, bool *is_gmem)
 		atomic_set(&vt->cpu, -1);
 
 		kvm_make_request(KVM_REQ_MAP_L1_VNCR_EL2, vcpu);
-		kvm_release_faultin_page(vcpu->kvm, page, false, vt->wr.pw && vt->hpa_writable);
+		kvm_release_faultin_page(vcpu->kvm, page, false, vncr_can_map_writable(vt));
 	}
 
-	if (vt->wr.pw && vt->hpa_writable)
+	if (vncr_can_map_writable(vt))
 		mark_page_dirty(vcpu->kvm, gfn);
 
 	return 0;
@@ -1781,7 +1788,7 @@ 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;
+		perm = kvm_is_write_fault(vcpu) ? vncr_can_map_writable(vt) : vt->wr.pr;
 		if (!perm)
 			handle_vncr_perm(vcpu);
 		break;
@@ -1824,7 +1831,7 @@ static void kvm_map_l1_vncr(struct kvm_vcpu *vcpu)
 	if (vt->wr.nG && get_asid_by_regime(vcpu, TR_EL20) != vt->wr.asid)
 		return;
 
-	if (vt->hpa_writable && vt->wr.pw && vt->wr.pr)
+	if (vncr_can_map_writable(vt) && vt->wr.pr)
 		prot = PAGE_KERNEL;
 	else if (vt->wr.pr)
 		prot = PAGE_KERNEL_RO;
-- 
2.47.3

Re: [PATCH v2 04/28] KVM: arm64: nv: Only shadow writable-dirty guest descs as writable

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] KVM: arm64: nv: missing wr.dirty check in handle_vncr_perm()
--

commit cfa39628f154da2407f90720dcc9a53017a25a78
Author: Oliver Upton <oupton@kernel.org>

KVM: arm64: nv: Only shadow writable-dirty guest descs as writable

This commit restricts shadow stage-2 and shadow VNCR translations to
read-only for writable-clean guest descriptors in preparation for emulating
hardware dirty state management for nested guests.

> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index 15c22a2186095..9b2ea83aeccd2 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c

[ ... ]

> @@ -1587,6 +1589,11 @@ static u64 read_vncr_el2(struct kvm_vcpu *vcpu)
>  	return (u64)sign_extend64(__vcpu_sys_reg(vcpu, VNCR_EL2), 48);
>  }
>  
> +static bool vncr_can_map_writable(struct vncr_tlb *vt)
> +{
> +	return vt->wr.pw && vt->wr.dirty && vt->hpa_writable;
> +}
> +
>  static int kvm_translate_vncr(struct kvm_vcpu *vcpu, bool *is_gmem)
>  {
>  	struct kvm_memory_slot *memslot;

[ ... ]

> @@ -1781,7 +1788,7 @@ 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;
> +		perm = kvm_is_write_fault(vcpu) ? vncr_can_map_writable(vt) : vt->wr.pr;
>  		if (!perm)
>  			handle_vncr_perm(vcpu);
>  		break;

[Severity: Medium]
Does handle_vncr_perm() in arch/arm64/kvm/nested.c also need to check
vt->wr.dirty when deciding whether to promote a write fault to an external
abort?

With FEAT_S1PIE enabled at guest EL2, compute_s1_indirect_permissions()
computes vt->wr.pw = true from PIR_EL2 and the descriptor's permission index
independently of BIT(7), while compute_s1_permissions() sets vt->wr.dirty =
false when BIT(7) is set on a writable-clean stage-1 descriptor.

When a nested guest takes a VNCR write fault on such a writable-clean
descriptor backed by a read-only memslot or HPA (!vt->hpa_writable),
kvm_handle_vncr_abort() sees !vncr_can_map_writable(vt) and calls
handle_vncr_perm():

arch/arm64/kvm/nested.c:handle_vncr_perm() {
	...
	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);
	...
}

Because handle_vncr_perm() only checks vt->wr.pw && !vt->hpa_writable
without checking vt->wr.dirty, won't it inject ESR_ELx_FSC_EXTABT instead of
the architectural stage-1 permission fault ESR_ELx_FSC_PERM_L(vt->wr.level)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023205610.1044532-1-oupton@kernel.org?part=4

Re: [PATCH v2 04/28] KVM: arm64: nv: Only shadow writable-dirty guest descs as writable

[Oliver Upton]

On Fri, Oct 23, 2026 at 09:25:39PM +0000, sashiko-bot@kernel.org wrote:
> > @@ -1781,7 +1788,7 @@ 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;
> > +		perm = kvm_is_write_fault(vcpu) ? vncr_can_map_writable(vt) : vt->wr.pr;
> >  		if (!perm)
> >  			handle_vncr_perm(vcpu);
> >  		break;
> 
> [Severity: Medium]
> Does handle_vncr_perm() in arch/arm64/kvm/nested.c also need to check
> vt->wr.dirty when deciding whether to promote a write fault to an external
> abort?

Yep, what we have 'works' for the direct permission model but S1PIE
style software dirty state makes a mess of it.

> With FEAT_S1PIE enabled at guest EL2, compute_s1_indirect_permissions()
> computes vt->wr.pw = true from PIR_EL2 and the descriptor's permission index
> independently of BIT(7), while compute_s1_permissions() sets vt->wr.dirty =
> false when BIT(7) is set on a writable-clean stage-1 descriptor.

Hey, look at that! You got the architecture right this time.

Thanks,
Oliver

> When a nested guest takes a VNCR write fault on such a writable-clean
> descriptor backed by a read-only memslot or HPA (!vt->hpa_writable),
> kvm_handle_vncr_abort() sees !vncr_can_map_writable(vt) and calls
> handle_vncr_perm():
> 
> arch/arm64/kvm/nested.c:handle_vncr_perm() {
> 	...
> 	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);
> 	...
> }
> 
> Because handle_vncr_perm() only checks vt->wr.pw && !vt->hpa_writable
> without checking vt->wr.dirty, won't it inject ESR_ELx_FSC_EXTABT instead of
> the architectural stage-1 permission fault ESR_ELx_FSC_PERM_L(vt->wr.level)?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261023205610.1044532-1-oupton@kernel.org?part=4

[PATCH v2 05/28] KVM: arm64: nv: Pass an access descriptor for stage-2 walks

[Oliver Upton]

Pass sufficient context to the stage-2 PTW such that access-dependent
features like FEAT_HAFDBS can determine the correct behavior of the
walk.

Reinstate the recently deleted kvm_vcpu_dabt_is_cm() to correctly
annotate CMO data aborts.

Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 arch/arm64/include/asm/kvm_emulate.h |  5 +++++
 arch/arm64/include/asm/kvm_nested.h  | 15 ++++++++++++++-
 arch/arm64/kvm/at.c                  | 22 ++++++++++++++++++----
 arch/arm64/kvm/mmu.c                 | 16 +++++++++++++++-
 arch/arm64/kvm/nested.c              | 10 +++++-----
 5 files changed, 57 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index 85289a4b8d52..0aabe592be16 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -457,6 +457,11 @@ static __always_inline bool kvm_vcpu_dabt_iswrite(const struct kvm_vcpu *vcpu)
 	return esr_dabt_is_write(kvm_vcpu_get_esr(vcpu));
 }
 
+static __always_inline bool kvm_vcpu_dabt_is_cm(const struct kvm_vcpu *vcpu)
+{
+	return esr_dabt_is_cm(kvm_vcpu_get_esr(vcpu));
+}
+
 static __always_inline unsigned int kvm_vcpu_dabt_get_as(const struct kvm_vcpu *vcpu)
 {
 	return 1 << ((kvm_vcpu_get_esr(vcpu) & ESR_ELx_SAS) >> ESR_ELx_SAS_SHIFT);
diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
index e8d5ae7afb9a..89a860b92826 100644
--- a/arch/arm64/include/asm/kvm_nested.h
+++ b/arch/arm64/include/asm/kvm_nested.h
@@ -111,7 +111,20 @@ static inline bool kvm_has_xnx(struct kvm *kvm)
 		kvm_has_feat(kvm, ID_AA64MMFR1_EL1, XNX, IMP);
 }
 
-extern int kvm_walk_nested_s2(struct kvm_vcpu *vcpu, phys_addr_t gipa,
+struct kvm_walk_access {
+	enum {
+		WALK_ACCESS_IFETCH,
+		WALK_ACCESS_LDST,
+		WALK_ACCESS_CMO,
+		WALK_ACCESS_AT,
+		WALK_ACCESS_S1PTW,
+	} type;
+
+	u64	ia;
+	bool	write;
+};
+
+extern int kvm_walk_nested_s2(struct kvm_vcpu *vcpu, struct kvm_walk_access *access,
 			      struct kvm_s2_trans *result);
 extern int kvm_s2_handle_perm_fault(struct kvm_vcpu *vcpu,
 				    struct kvm_s2_trans *trans);
diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c
index 4e505433299b..32d2e57b017c 100644
--- a/arch/arm64/kvm/at.c
+++ b/arch/arm64/kvm/at.c
@@ -481,7 +481,19 @@ static int walk_s1(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 		ipa = baddr | index;
 
 		if (wi->s2) {
-			ret = kvm_walk_nested_s2(vcpu, ipa, &s2_trans);
+			struct kvm_walk_access s2_access = {
+				.type	= WALK_ACCESS_S1PTW,
+				.ia	= ipa,
+
+				/*
+				 * R_JCXVS, stage-2 dirty state can be updated
+				 * for an S1PTW even if the stage-1 descriptor
+				 * isn't updated.
+				 */
+				.write	= wi->ha,
+			};
+
+			ret = kvm_walk_nested_s2(vcpu, &s2_access, &s2_trans);
 			if (ret == -EAGAIN)
 				return ret;
 
@@ -1595,8 +1607,9 @@ int __kvm_at_s1e2(struct kvm_vcpu *vcpu, u32 op, u64 vaddr)
 
 int __kvm_at_s12(struct kvm_vcpu *vcpu, u32 op, u64 vaddr)
 {
+	struct kvm_walk_access access = {};
 	struct kvm_s2_trans out = {};
-	u64 ipa, par;
+	u64 par;
 	bool write;
 	int ret;
 
@@ -1640,10 +1653,11 @@ int __kvm_at_s12(struct kvm_vcpu *vcpu, u32 op, u64 vaddr)
 		return 0;
 
 	/* Do the stage-2 translation */
-	ipa = (par & GENMASK_ULL(47, 12)) | (vaddr & GENMASK_ULL(11, 0));
+	access.type = WALK_ACCESS_AT;
+	access.ia = (par & GENMASK_ULL(47, 12)) | (vaddr & GENMASK_ULL(11, 0));
 	out.esr = 0;
 	scoped_guard(srcu, &vcpu->kvm->srcu)
-		ret = kvm_walk_nested_s2(vcpu, ipa, &out);
+		ret = kvm_walk_nested_s2(vcpu, &access, &out);
 	if (ret < 0)
 		return ret;
 
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 84aa1bb31095..7b29be33a1ea 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -2519,9 +2519,23 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu)
 	 * canonical translation.
 	 */
 	if (kvm_is_nested_s2_mmu(kvm, mmu) && mmu->nested_stage2_enabled) {
+		struct kvm_walk_access access = {
+			.ia	= fault_ipa,
+		};
 		u32 esr;
 
-		ret = kvm_walk_nested_s2(vcpu, fault_ipa, &nested_trans);
+		if (kvm_vcpu_abt_iss1tw(vcpu))
+			access.type = WALK_ACCESS_S1PTW;
+		else if (is_iabt)
+			access.type = WALK_ACCESS_IFETCH;
+		else if (kvm_vcpu_dabt_is_cm(vcpu))
+			access.type = WALK_ACCESS_CMO;
+		else
+			access.type = WALK_ACCESS_LDST;
+
+		access.write = kvm_is_write_fault(vcpu);
+
+		ret = kvm_walk_nested_s2(vcpu, &access, &nested_trans);
 		if (ret == -EAGAIN) {
 			ret = 1;
 			goto out_unlock;
diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index 9b2ea83aeccd..a3cbf37dddc9 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -299,7 +299,7 @@ static void compute_s2_permissions(struct kvm_vcpu *vcpu, struct s2_walk_info *w
  *
  * Must be called with the kvm->srcu read lock held
  */
-static int walk_nested_s2_pgd(struct kvm_vcpu *vcpu, phys_addr_t ipa,
+static int walk_nested_s2_pgd(struct kvm_vcpu *vcpu, struct kvm_walk_access *access,
 			      struct s2_walk_info *wi, struct kvm_s2_trans *out)
 {
 	int first_block_level, stride, input_size, base_lower_bound;
@@ -349,7 +349,7 @@ static int walk_nested_s2_pgd(struct kvm_vcpu *vcpu, phys_addr_t ipa,
 		phys_addr_t index;
 
 		addr_bottom = (3 - ws.level) * stride + wi->pgshift;
-		index = (ipa & GENMASK_ULL(addr_top, addr_bottom))
+		index = (access->ia & GENMASK_ULL(addr_top, addr_bottom))
 			>> (addr_bottom - 3);
 
 		ws.desc_pa = base_addr | index;
@@ -431,7 +431,7 @@ static int walk_nested_s2_pgd(struct kvm_vcpu *vcpu, phys_addr_t ipa,
 
 	/* Calculate and return the result */
 	out->output = (ws.desc & GENMASK_ULL(47, addr_bottom)) |
-		      (ipa & GENMASK_ULL(addr_bottom - 1, 0));
+		      (access->ia & GENMASK_ULL(addr_bottom - 1, 0));
 	out->block_size = 1UL << ((3 - ws.level) * stride + wi->pgshift);
 	compute_s2_permissions(vcpu, wi, &ws, out);
 	out->level = ws.level;
@@ -534,7 +534,7 @@ static void setup_s2_walk(struct kvm_vcpu *vcpu, struct s2_walk_info *wi)
 	wi->be = vcpu_read_sys_reg(vcpu, SCTLR_EL2) & SCTLR_ELx_EE;
 }
 
-int kvm_walk_nested_s2(struct kvm_vcpu *vcpu, phys_addr_t gipa,
+int kvm_walk_nested_s2(struct kvm_vcpu *vcpu, struct kvm_walk_access *access,
 		       struct kvm_s2_trans *result)
 {
 	struct s2_walk_info wi;
@@ -547,7 +547,7 @@ int kvm_walk_nested_s2(struct kvm_vcpu *vcpu, phys_addr_t gipa,
 
 	setup_s2_walk(vcpu, &wi);
 
-	ret = walk_nested_s2_pgd(vcpu, gipa, &wi, result);
+	ret = walk_nested_s2_pgd(vcpu, access, &wi, result);
 	if (ret)
 		result->esr |= (kvm_vcpu_get_esr(vcpu) & ~ESR_ELx_FSC);
 
-- 
2.47.3

[PATCH v2 06/28] KVM: arm64: nv: Use a helper for stage-2 descriptor updates

[Oliver Upton]

Use a helper for handling stage-2 descriptor updates so the
implementation can be shared with updates to table descriptors.

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

diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index a3cbf37dddc9..dea5c31f10df 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -246,9 +246,23 @@ static int read_guest_s2_desc(struct kvm_vcpu *vcpu, struct s2_walk_step *ws,
 	return 0;
 }
 
-static int swap_guest_s2_desc(struct kvm_vcpu *vcpu, phys_addr_t pa, u64 old, u64 new,
-			      struct s2_walk_info *wi)
+static int handle_desc_update(struct kvm_vcpu *vcpu, struct s2_walk_info *wi,
+			      struct s2_walk_step *ws, struct kvm_s2_trans *out,
+			      struct kvm_walk_access *access)
 {
+	u64 old, new;
+	int ret;
+
+	old = new = ws->desc;
+
+	if (wi->ha)
+		new |= KVM_PTE_LEAF_ATTR_LO_S2_AF;
+
+	if (old == new)
+		return 0;
+
+	ws->desc = new;
+
 	if (wi->be) {
 		old = (__force u64)cpu_to_be64(old);
 		new = (__force u64)cpu_to_be64(new);
@@ -257,7 +271,13 @@ static int swap_guest_s2_desc(struct kvm_vcpu *vcpu, phys_addr_t pa, u64 old, u6
 		new = (__force u64)cpu_to_le64(new);
 	}
 
-	return __kvm_at_swap_desc(vcpu->kvm, pa, old, new);
+	ret = __kvm_at_swap_desc(vcpu->kvm, ws->desc_pa, old, new);
+	if (!ret || ret == -EAGAIN)
+		return ret;
+
+	out->esr = ESR_ELx_FSC_SEA_TTW(ws->level);
+	out->desc = ws->desc;
+	return 1;
 }
 
 static void compute_s2_permissions(struct kvm_vcpu *vcpu, struct s2_walk_info *wi,
@@ -306,7 +326,6 @@ static int walk_nested_s2_pgd(struct kvm_vcpu *vcpu, struct kvm_walk_access *acc
 	struct s2_walk_step ws = {};
 	phys_addr_t base_addr;
 	unsigned int addr_top, addr_bottom;
-	u64 new_desc;  /* page table entry */
 	int ret;
 
 	switch (BIT(wi->pgshift)) {
@@ -359,8 +378,6 @@ static int walk_nested_s2_pgd(struct kvm_vcpu *vcpu, struct kvm_walk_access *acc
 			return ret;
 		}
 
-		new_desc = ws.desc;
-
 		/* Check for valid descriptor at this point */
 		if (!(ws.desc & KVM_PTE_VALID)) {
 			out->esr = compute_fsc(ws.level, ESR_ELx_FSC_FAULT);
@@ -405,21 +422,9 @@ static int walk_nested_s2_pgd(struct kvm_vcpu *vcpu, struct kvm_walk_access *acc
 		return 1;
 	}
 
-	if (wi->ha)
-		new_desc |= KVM_PTE_LEAF_ATTR_LO_S2_AF;
-
-	if (new_desc != ws.desc) {
-		ret = swap_guest_s2_desc(vcpu, ws.desc_pa, ws.desc, new_desc, wi);
-		if (ret == -EAGAIN)
-			return ret;
-		if (ret) {
-			out->esr = ESR_ELx_FSC_SEA_TTW(ws.level);
-			out->desc = ws.desc;
-			return 1;
-		}
-
-		ws.desc = new_desc;
-	}
+	ret = handle_desc_update(vcpu, wi, &ws, out, access);
+	if (ret)
+		return ret;
 
 	if (!(ws.desc & KVM_PTE_LEAF_ATTR_LO_S2_AF)) {
 		out->esr = compute_fsc(ws.level, ESR_ELx_FSC_ACCESS);
-- 
2.47.3

[PATCH v2 07/28] KVM: arm64: nv: Set dirty state at stage-2

[Oliver Upton]

Set the dirty state on descriptors at stage-2 for write accesses when
DBM is set.

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

diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index dea5c31f10df..1d581d3bd557 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -151,6 +151,7 @@ struct s2_walk_info {
 	unsigned int	t0sz;
 	bool		be;
 	bool		ha;
+	bool		hd;
 };
 
 struct s2_walk_step {
@@ -246,6 +247,20 @@ static int read_guest_s2_desc(struct kvm_vcpu *vcpu, struct s2_walk_step *ws,
 	return 0;
 }
 
+static bool should_set_dirty_state(struct s2_walk_info *wi, struct s2_walk_step *ws,
+				   struct kvm_s2_trans *out, struct kvm_walk_access *access)
+{
+	switch (access->type) {
+	/* R_ZYGHJ */
+	case WALK_ACCESS_CMO:
+	case WALK_ACCESS_AT:
+		return false;
+	default:
+		/* R_NSXRD */
+		return access->write && wi->hd && out->writable;
+	}
+}
+
 static int handle_desc_update(struct kvm_vcpu *vcpu, struct s2_walk_info *wi,
 			      struct s2_walk_step *ws, struct kvm_s2_trans *out,
 			      struct kvm_walk_access *access)
@@ -258,6 +273,9 @@ static int handle_desc_update(struct kvm_vcpu *vcpu, struct s2_walk_info *wi,
 	if (wi->ha)
 		new |= KVM_PTE_LEAF_ATTR_LO_S2_AF;
 
+	if (should_set_dirty_state(wi, ws, out, access))
+		new |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
+
 	if (old == new)
 		return 0;
 
@@ -272,11 +290,15 @@ static int handle_desc_update(struct kvm_vcpu *vcpu, struct s2_walk_info *wi,
 	}
 
 	ret = __kvm_at_swap_desc(vcpu->kvm, ws->desc_pa, old, new);
-	if (!ret || ret == -EAGAIN)
+	if (ret == -EAGAIN)
 		return ret;
+	if (!ret) {
+		out->desc = ws->desc;
+		out->dirty = out->desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
+		return 0;
+	}
 
 	out->esr = ESR_ELx_FSC_SEA_TTW(ws->level);
-	out->desc = ws->desc;
 	return 1;
 }
 
@@ -422,16 +444,6 @@ static int walk_nested_s2_pgd(struct kvm_vcpu *vcpu, struct kvm_walk_access *acc
 		return 1;
 	}
 
-	ret = handle_desc_update(vcpu, wi, &ws, out, access);
-	if (ret)
-		return ret;
-
-	if (!(ws.desc & KVM_PTE_LEAF_ATTR_LO_S2_AF)) {
-		out->esr = compute_fsc(ws.level, ESR_ELx_FSC_ACCESS);
-		out->desc = ws.desc;
-		return 1;
-	}
-
 	addr_bottom += contiguous_bit_shift(ws.desc, wi, ws.level);
 
 	/* Calculate and return the result */
@@ -441,6 +453,20 @@ static int walk_nested_s2_pgd(struct kvm_vcpu *vcpu, struct kvm_walk_access *acc
 	compute_s2_permissions(vcpu, wi, &ws, out);
 	out->level = ws.level;
 	out->desc = ws.desc;
+
+	ret = handle_desc_update(vcpu, wi, &ws, out, access);
+	if (ret)
+		return ret;
+
+	if (!(ws.desc & KVM_PTE_LEAF_ATTR_LO_S2_AF)) {
+		*out = (struct kvm_s2_trans) {
+			.esr	= compute_fsc(ws.level, ESR_ELx_FSC_ACCESS),
+			.desc	= ws.desc,
+		};
+
+		return 1;
+	}
+
 	return 0;
 }
 
@@ -537,6 +563,7 @@ static void setup_s2_walk(struct kvm_vcpu *vcpu, struct s2_walk_info *wi)
 			      ps_to_output_size(FIELD_GET(VTCR_EL2_PS_MASK, vtcr), false));
 	wi->ha = vtcr & VTCR_EL2_HA;
 	wi->be = vcpu_read_sys_reg(vcpu, SCTLR_EL2) & SCTLR_ELx_EE;
+	wi->hd = wi->ha && (vtcr & VTCR_EL2_HD);
 }
 
 int kvm_walk_nested_s2(struct kvm_vcpu *vcpu, struct kvm_walk_access *access,
-- 
2.47.3

[PATCH v2 08/28] KVM: arm64: nv: Treat DBM as writable at stage-2

[Oliver Upton]

When using direct permissions, the DBM bit has the effect of granting
the write permission. And, depending on the access, it could grant write
permission without a dirty state update.

Reviewed-by: Leonardo Bras <leo.bras@arm.com>
Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 arch/arm64/include/asm/kvm_pgtable.h |  1 +
 arch/arm64/kvm/nested.c              | 11 +++++++++++
 2 files changed, 12 insertions(+)

diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
index edfae3a6f362..23e93f14af7b 100644
--- a/arch/arm64/include/asm/kvm_pgtable.h
+++ b/arch/arm64/include/asm/kvm_pgtable.h
@@ -102,6 +102,7 @@ typedef u64 kvm_pte_t;
 #define KVM_PTE_LEAF_ATTR_HI_S1_PXN	BIT(53)
 
 #define KVM_PTE_LEAF_ATTR_HI_S2_XN	GENMASK(54, 53)
+#define KVM_PTE_LEAF_ATTR_HI_S2_DBM	BIT(51)
 
 #define KVM_PTE_LEAF_ATTR_HI_S1_GP	BIT(50)
 
diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index 1d581d3bd557..e388cefad9e5 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -328,6 +328,17 @@ static void compute_s2_permissions(struct kvm_vcpu *vcpu, struct s2_walk_info *w
 		break;
 	}
 
+	/*
+	 * Descriptors with the DBM bit set while hardware dirty state are
+	 * considered writable, even though certain accesses (like AT instructions)
+	 * don't actually update the dirty state.
+	 *
+	 * Assume that walk_nestd_s2_pgd() made the necessary descriptor updates
+	 * for the access and just treat DBM as writable here.
+	 */
+	if (wi->hd && ws->desc & KVM_PTE_LEAF_ATTR_HI_S2_DBM)
+		s2ap |= BIT(1);
+
 	trans->readable = s2ap & BIT(0);
 	trans->writable = s2ap & BIT(1);
 
-- 
2.47.3

[PATCH v2 09/28] KVM: arm64: Compute S1 permissions as part of s1_walk()

[Oliver Upton]

Implementing support for hardware dirty state means that the table
walker needs to have visibility into the permissions on the final
translation.

Compute the S1 permissions as part of s1_walk() and initialize
s1_walk_result before checking all fault conditions. The appropriate
fields will be reinitialized if the walk happens to fail at a later
point.

Reviewed-by: Leonardo Bras <leo.bras@arm.com>
Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 arch/arm64/kvm/at.c | 41 ++++++++++++++++++++---------------------
 1 file changed, 20 insertions(+), 21 deletions(-)

diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c
index 32d2e57b017c..3031f5855316 100644
--- a/arch/arm64/kvm/at.c
+++ b/arch/arm64/kvm/at.c
@@ -459,6 +459,10 @@ static int kvm_swap_s1_desc(struct kvm_vcpu *vcpu, u64 pa, u64 old, u64 new,
 	return __kvm_at_swap_desc(vcpu->kvm, pa, old, new);
 }
 
+static void compute_s1_permissions(struct kvm_vcpu *vcpu,
+				   struct s1_walk_info *wi,
+				   struct s1_walk_result *wr);
+
 static int walk_s1(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 		   struct s1_walk_result *wr, u64 va)
 {
@@ -588,6 +592,20 @@ static int walk_s1(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 	if (check_output_size(baddr & GENMASK(52, va_bottom), wi))
 		goto addrsz;
 
+	va_bottom += contiguous_bit_shift(desc, wi, level);
+
+	wr->failed = false;
+	wr->level = level;
+	wr->desc = desc;
+	wr->pa = baddr & GENMASK(52, va_bottom);
+	wr->pa |= va & GENMASK_ULL(va_bottom - 1, 0);
+
+	wr->nG = (wi->regime != TR_EL2) && (desc & PTE_NG);
+	if (wr->nG)
+		wr->asid = get_asid_by_regime(vcpu, wi->regime);
+
+	compute_s1_permissions(vcpu, wi, wr);
+
 	if (wi->ha)
 		new_desc |= PTE_AF;
 
@@ -613,18 +631,6 @@ static int walk_s1(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 		return -EACCES;
 	}
 
-	va_bottom += contiguous_bit_shift(desc, wi, level);
-
-	wr->failed = false;
-	wr->level = level;
-	wr->desc = desc;
-	wr->pa = baddr & GENMASK(52, va_bottom);
-	wr->pa |= va & GENMASK_ULL(va_bottom - 1, 0);
-
-	wr->nG = (wi->regime != TR_EL2) && (desc & PTE_NG);
-	if (wr->nG)
-		wr->asid = get_asid_by_regime(vcpu, wi->regime);
-
 	return 0;
 
 addrsz:
@@ -1363,8 +1369,6 @@ static int handle_at_slow(struct kvm_vcpu *vcpu, u32 op, u64 vaddr, u64 *par)
 	if (ret)
 		goto compute_par;
 
-	compute_s1_permissions(vcpu, &wi, &wr);
-
 	switch (op) {
 	case OP_AT_S1E1RP:
 	case OP_AT_S1E1R:
@@ -1689,15 +1693,10 @@ int __kvm_translate_va(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 		wr->ur = wr->uw = wr->ux = true;
 		wr->pr = wr->pw = wr->px = true;
 		wr->dirty = true;
-	} else {
-		ret = walk_s1(vcpu, wi, wr, va);
-		if (ret)
-			return ret;
-
-		compute_s1_permissions(vcpu, wi, wr);
+		return 0;
 	}
 
-	return 0;
+	return walk_s1(vcpu, wi, wr, va);
 }
 
 struct desc_match {
-- 
2.47.3

[PATCH v2 10/28] KVM: arm64: Plumb through access descriptor for stage-1

[Oliver Upton]

Pass sufficient context to the stage-1 walk such that access-dependent
features like FEAT_HAFDBS can implement the correct behavior.

Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 arch/arm64/include/asm/kvm_nested.h |  4 +++-
 arch/arm64/kvm/at.c                 | 32 ++++++++++++++++++++++-------
 arch/arm64/kvm/nested.c             | 10 ++++++---
 3 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
index 89a860b92826..d7c1fa66f029 100644
--- a/arch/arm64/include/asm/kvm_nested.h
+++ b/arch/arm64/include/asm/kvm_nested.h
@@ -118,6 +118,8 @@ struct kvm_walk_access {
 		WALK_ACCESS_CMO,
 		WALK_ACCESS_AT,
 		WALK_ACCESS_S1PTW,
+		WALK_ACCESS_NV2,
+		WALK_ACCESS_NONARCH,
 	} type;
 
 	u64	ia;
@@ -385,7 +387,7 @@ static inline bool s1_walk_translated(struct s1_walk_result *wr)
 }
 
 int __kvm_translate_va(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
-		       struct s1_walk_result *wr, u64 va);
+		       struct s1_walk_result *wr, struct kvm_walk_access *access);
 int __kvm_find_s1_desc_level(struct kvm_vcpu *vcpu, u64 va, u64 ipa,
 			     int *level);
 
diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c
index 3031f5855316..fe1d8a674d64 100644
--- a/arch/arm64/kvm/at.c
+++ b/arch/arm64/kvm/at.c
@@ -464,12 +464,13 @@ static void compute_s1_permissions(struct kvm_vcpu *vcpu,
 				   struct s1_walk_result *wr);
 
 static int walk_s1(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
-		   struct s1_walk_result *wr, u64 va)
+		   struct s1_walk_result *wr, struct kvm_walk_access *access)
 {
-	u64 va_top, va_bottom, baddr, desc, new_desc, ipa;
+	u64 va_top, va_bottom, baddr, desc, new_desc, ipa, va;
 	struct kvm_s2_trans s2_trans = {};
 	int level, stride, ret;
 
+	va = access->ia;
 	level = wi->sl;
 	stride = wi->pgshift - 3;
 	baddr = wi->baddr;
@@ -1338,6 +1339,7 @@ static void compute_s1_permissions(struct kvm_vcpu *vcpu,
 
 static int handle_at_slow(struct kvm_vcpu *vcpu, u32 op, u64 vaddr, u64 *par)
 {
+	struct kvm_walk_access access = {};
 	struct s1_walk_result wr = {};
 	struct s1_walk_info wi = {};
 	bool perm_fail = false;
@@ -1355,9 +1357,21 @@ static int handle_at_slow(struct kvm_vcpu *vcpu, u32 op, u64 vaddr, u64 *par)
 	if (wr.level == S1_MMU_DISABLED)
 		goto compute_par;
 
+	access.type = WALK_ACCESS_AT;
+	access.ia = vaddr;
+	switch (op) {
+	case OP_AT_S1E1WP:
+	case OP_AT_S1E1W:
+	case OP_AT_S1E2W:
+	case OP_AT_S1E0W:
+		access.write = true;
+		break;
+	default:
+	}
+
 	idx = srcu_read_lock(&vcpu->kvm->srcu);
 
-	ret = walk_s1(vcpu, &wi, &wr, vaddr);
+	ret = walk_s1(vcpu, &wi, &wr, &access);
 
 	srcu_read_unlock(&vcpu->kvm->srcu, idx);
 
@@ -1681,11 +1695,11 @@ int __kvm_at_s12(struct kvm_vcpu *vcpu, u32 op, u64 vaddr)
  * set. The rest of the wi and wr should be 0-initialised.
  */
 int __kvm_translate_va(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
-		       struct s1_walk_result *wr, u64 va)
+		       struct s1_walk_result *wr, struct kvm_walk_access *access)
 {
 	int ret;
 
-	ret = setup_s1_walk(vcpu, wi, wr, va);
+	ret = setup_s1_walk(vcpu, wi, wr, access->ia);
 	if (ret)
 		return ret;
 
@@ -1696,7 +1710,7 @@ int __kvm_translate_va(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 		return 0;
 	}
 
-	return walk_s1(vcpu, wi, wr, va);
+	return walk_s1(vcpu, wi, wr, access);
 }
 
 struct desc_match {
@@ -1734,6 +1748,10 @@ int __kvm_find_s1_desc_level(struct kvm_vcpu *vcpu, u64 va, u64 ipa, int *level)
 		.as_el0	= false,
 		.pan	= false,
 	};
+	struct kvm_walk_access access = {
+		.type	= WALK_ACCESS_NONARCH,
+		.ia	= va,
+	};
 	struct s1_walk_result wr = {};
 	int ret;
 
@@ -1754,7 +1772,7 @@ int __kvm_find_s1_desc_level(struct kvm_vcpu *vcpu, u64 va, u64 ipa, int *level)
 
 	/* Walk the guest's PT, looking for a match along the way */
 	scoped_guard(srcu, &vcpu->kvm->srcu)
-		ret = walk_s1(vcpu, &wi, &wr, va);
+		ret = walk_s1(vcpu, &wi, &wr, &access);
 	switch (ret) {
 	case -EINTR:
 		/* We interrupted the walk on a match, return the level */
diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index e388cefad9e5..964331dbf475 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -1639,6 +1639,7 @@ static bool vncr_can_map_writable(struct vncr_tlb *vt)
 
 static int kvm_translate_vncr(struct kvm_vcpu *vcpu, bool *is_gmem)
 {
+	struct kvm_walk_access access = {};
 	struct kvm_memory_slot *memslot;
 	bool write_fault, writable;
 	unsigned long mmu_seq;
@@ -1648,6 +1649,7 @@ static int kvm_translate_vncr(struct kvm_vcpu *vcpu, bool *is_gmem)
 	int ret;
 
 	vt = vcpu->arch.vncr_tlb;
+	write_fault = kvm_is_write_fault(vcpu);
 
 	/*
 	 * If we're about to walk the EL2 S1 PTs, we must invalidate the
@@ -1674,15 +1676,17 @@ static int kvm_translate_vncr(struct kvm_vcpu *vcpu, bool *is_gmem)
 
 	va =  read_vncr_el2(vcpu);
 
+	access.type = WALK_ACCESS_NV2;
+	access.ia = va;
+	access.write = write_fault;
+
 	mmu_seq = vcpu->kvm->mmu_invalidate_seq;
 	smp_rmb();
 
-	ret = __kvm_translate_va(vcpu, &vt->wi, &vt->wr, va);
+	ret = __kvm_translate_va(vcpu, &vt->wi, &vt->wr, &access);
 	if (ret)
 		return ret;
 
-	write_fault = kvm_is_write_fault(vcpu);
-
 	gfn = vt->wr.pa >> PAGE_SHIFT;
 	memslot = gfn_to_memslot(vcpu->kvm, gfn);
 	if (!memslot) {
-- 
2.47.3

Re: [PATCH v2 10/28] KVM: arm64: Plumb through access descriptor for stage-1

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] KVM: arm64: empty default label without statement in handle_at_slow()
--

commit 2d01817a35b4565eced6f12aa2e735cfdeeae327
Author: Oliver Upton <oupton@kernel.org>

KVM: arm64: Plumb through access descriptor for stage-1

This commit plumbs struct kvm_walk_access through the stage-1 translation
walk helpers so access-dependent features can inspect the access type and
write intent.

> diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c
> index 3031f58553160..fe1d8a674d64d 100644
> --- a/arch/arm64/kvm/at.c
> +++ b/arch/arm64/kvm/at.c

[ ... ]

> @@ -1355,9 +1357,21 @@ static int handle_at_slow(struct kvm_vcpu *vcpu, u32 op, u64 vaddr, u64 *par)
>  	if (wr.level == S1_MMU_DISABLED)
>  		goto compute_par;
>  
> +	access.type = WALK_ACCESS_AT;
> +	access.ia = vaddr;
> +	switch (op) {
> +	case OP_AT_S1E1WP:
> +	case OP_AT_S1E1W:
> +	case OP_AT_S1E2W:
> +	case OP_AT_S1E0W:
> +		access.write = true;
> +		break;
> +	default:
> +	}

[Severity: Low]
Will this bare default: label at the end of the switch block in
handle_at_slow() in arch/arm64/kvm/at.c cause a build regression on older
compilers?

When compiling with -std=gnu11, a label at the end of a compound statement
without a following break; or null statement is a c23 extension only
supported starting in gcc 11, which triggers a "label at end of compound
statement" syntax error on gcc 8 through 10.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023205610.1044532-1-oupton@kernel.org?part=10

[PATCH v2 11/28] KVM: arm64: Use a struct for stage-1 walk context

[Oliver Upton]

Consolidate the current walk step in a struct such that helpers can be
spun off from the core implementation.

Reviewed-by: Leonardo Bras <leo.bras@arm.com>
Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 arch/arm64/kvm/at.c | 94 ++++++++++++++++++++++++---------------------
 1 file changed, 51 insertions(+), 43 deletions(-)

diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c
index fe1d8a674d64..dcbe5563baa2 100644
--- a/arch/arm64/kvm/at.c
+++ b/arch/arm64/kvm/at.c
@@ -427,6 +427,14 @@ static int setup_s1_walk(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 	return -EFAULT;
 }
 
+struct s1_walk_step {
+	u64			desc;
+	u64			desc_ipa;
+	u64			desc_pa;
+	struct kvm_s2_trans	s2_trans;
+	int			level;
+};
+
 static int kvm_read_s1_desc(struct kvm_vcpu *vcpu, u64 pa, u64 *desc,
 			    struct s1_walk_info *wi)
 {
@@ -466,12 +474,12 @@ static void compute_s1_permissions(struct kvm_vcpu *vcpu,
 static int walk_s1(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 		   struct s1_walk_result *wr, struct kvm_walk_access *access)
 {
-	u64 va_top, va_bottom, baddr, desc, new_desc, ipa, va;
-	struct kvm_s2_trans s2_trans = {};
-	int level, stride, ret;
+	u64 va_top, va_bottom, baddr, new_desc, va;
+	struct s1_walk_step ws = {};
+	int stride, ret;
 
 	va = access->ia;
-	level = wi->sl;
+	ws.level = wi->sl;
 	stride = wi->pgshift - 3;
 	baddr = wi->baddr;
 
@@ -480,15 +488,15 @@ static int walk_s1(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 	while (1) {
 		u64 index;
 
-		va_bottom = (3 - level) * stride + wi->pgshift;
+		va_bottom = (3 - ws.level) * stride + wi->pgshift;
 		index = (va & GENMASK_ULL(va_top, va_bottom)) >> (va_bottom - 3);
 
-		ipa = baddr | index;
+		ws.desc_ipa = ws.desc_pa = baddr | index;
 
 		if (wi->s2) {
 			struct kvm_walk_access s2_access = {
 				.type	= WALK_ACCESS_S1PTW,
-				.ia	= ipa,
+				.ia	= ws.desc_ipa,
 
 				/*
 				 * R_JCXVS, stage-2 dirty state can be updated
@@ -498,25 +506,25 @@ static int walk_s1(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 				.write	= wi->ha,
 			};
 
-			ret = kvm_walk_nested_s2(vcpu, &s2_access, &s2_trans);
+			ret = kvm_walk_nested_s2(vcpu, &s2_access, &ws.s2_trans);
 			if (ret == -EAGAIN)
 				return ret;
 
 			if (ret) {
 				fail_s1_walk(wr,
-					     (s2_trans.esr & ~ESR_ELx_FSC_LEVEL) | level,
+					     (ws.s2_trans.esr & ~ESR_ELx_FSC_LEVEL) | ws.level,
 					     true);
 				return ret;
 			}
 
-			if (!s2_trans.readable) {
-				fail_s1_walk(wr, ESR_ELx_FSC_PERM_L(level),
+			if (!ws.s2_trans.readable) {
+				fail_s1_walk(wr, ESR_ELx_FSC_PERM_L(ws.level),
 					     true);
 
 				return -EPERM;
 			}
 
-			ipa = s2_trans.output;
+			ws.desc_pa = ws.s2_trans.output;
 		}
 
 		if (wi->filter) {
@@ -524,40 +532,40 @@ static int walk_s1(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 					     {
 						     .wi	= wi,
 						     .table_ipa	= baddr,
-						     .level	= level,
+						     .level	= ws.level,
 					     }, wi->filter->priv);
 			if (ret)
 				return ret;
 		}
 
-		ret = kvm_read_s1_desc(vcpu, ipa, &desc, wi);
+		ret = kvm_read_s1_desc(vcpu, ws.desc_pa, &ws.desc, wi);
 		if (ret) {
-			fail_s1_walk(wr, ESR_ELx_FSC_SEA_TTW(level), false);
+			fail_s1_walk(wr, ESR_ELx_FSC_SEA_TTW(ws.level), false);
 			return ret;
 		}
 
-		new_desc = desc;
+		new_desc = ws.desc;
 
 		/* Invalid descriptor */
-		if (!(desc & BIT(0)))
+		if (!(ws.desc & BIT(0)))
 			goto transfault;
 
 		/* Block mapping, check validity down the line */
-		if (!(desc & BIT(1)))
+		if (!(ws.desc & BIT(1)))
 			break;
 
 		/* Page mapping */
-		if (level == 3)
+		if (ws.level == 3)
 			break;
 
 		/* Table handling */
 		if (!wi->hpd) {
-			wr->APTable  |= FIELD_GET(S1_TABLE_AP, desc);
-			wr->UXNTable |= FIELD_GET(PMD_TABLE_UXN, desc);
-			wr->PXNTable |= FIELD_GET(PMD_TABLE_PXN, desc);
+			wr->APTable  |= FIELD_GET(S1_TABLE_AP, ws.desc);
+			wr->UXNTable |= FIELD_GET(PMD_TABLE_UXN, ws.desc);
+			wr->PXNTable |= FIELD_GET(PMD_TABLE_PXN, ws.desc);
 		}
 
-		baddr = desc_to_oa(wi, desc);
+		baddr = desc_to_oa(wi, ws.desc);
 
 		/* Check for out-of-range OA */
 		if (check_output_size(baddr, wi))
@@ -565,23 +573,23 @@ static int walk_s1(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 
 		/* Prepare for next round */
 		va_top = va_bottom - 1;
-		level++;
+		ws.level++;
 	}
 
 	/* Block mapping, check the validity of the level */
-	if (!(desc & BIT(1))) {
+	if (!(ws.desc & BIT(1))) {
 		bool valid_block = false;
 		bool lpa = kvm_has_feat_enum(vcpu->kvm, ID_AA64MMFR0_EL1, PARANGE, 52);
 
 		switch (BIT(wi->pgshift)) {
 		case SZ_4K:
-			valid_block = level == 1 || level == 2 || (wi->pa52bit && level == 0);
+			valid_block = ws.level == 1 || ws.level == 2 || (wi->pa52bit && ws.level == 0);
 			break;
 		case SZ_16K:
-			valid_block = level == 2 || (wi->pa52bit && level == 1);
+			valid_block = ws.level == 2 || (wi->pa52bit && ws.level == 1);
 			break;
 		case SZ_64K:
-			valid_block = level == 2 || (lpa && level == 1);
+			valid_block = ws.level == 2 || (lpa && ws.level == 1);
 			break;
 		}
 
@@ -589,19 +597,19 @@ static int walk_s1(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 			goto transfault;
 	}
 
-	baddr = desc_to_oa(wi, desc);
+	baddr = desc_to_oa(wi, ws.desc);
 	if (check_output_size(baddr & GENMASK(52, va_bottom), wi))
 		goto addrsz;
 
-	va_bottom += contiguous_bit_shift(desc, wi, level);
+	va_bottom += contiguous_bit_shift(ws.desc, wi, ws.level);
 
 	wr->failed = false;
-	wr->level = level;
-	wr->desc = desc;
+	wr->level = ws.level;
+	wr->desc = ws.desc;
 	wr->pa = baddr & GENMASK(52, va_bottom);
 	wr->pa |= va & GENMASK_ULL(va_bottom - 1, 0);
 
-	wr->nG = (wi->regime != TR_EL2) && (desc & PTE_NG);
+	wr->nG = (wi->regime != TR_EL2) && (ws.desc & PTE_NG);
 	if (wr->nG)
 		wr->asid = get_asid_by_regime(vcpu, wi->regime);
 
@@ -610,35 +618,35 @@ static int walk_s1(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 	if (wi->ha)
 		new_desc |= PTE_AF;
 
-	if (new_desc != desc) {
-		if (wi->s2 && !s2_trans.writable) {
-			fail_s1_walk(wr, ESR_ELx_FSC_PERM_L(level), true);
+	if (new_desc != ws.desc) {
+		if (wi->s2 && !ws.s2_trans.writable) {
+			fail_s1_walk(wr, ESR_ELx_FSC_PERM_L(ws.level), true);
 			return -EPERM;
 		}
 
-		ret = kvm_swap_s1_desc(vcpu, ipa, desc, new_desc, wi);
+		ret = kvm_swap_s1_desc(vcpu, ws.desc_pa, ws.desc, new_desc, wi);
 		if (ret == -EAGAIN)
 			return ret;
 		if (ret) {
-			fail_s1_walk(wr, ESR_ELx_FSC_SEA_TTW(level), false);
+			fail_s1_walk(wr, ESR_ELx_FSC_SEA_TTW(ws.level), false);
 			return ret;
 		}
 
-		desc = new_desc;
+		ws.desc = new_desc;
 	}
 
-	if (!(desc & PTE_AF)) {
-		fail_s1_walk(wr, ESR_ELx_FSC_ACCESS_L(level), false);
+	if (!(ws.desc & PTE_AF)) {
+		fail_s1_walk(wr, ESR_ELx_FSC_ACCESS_L(ws.level), false);
 		return -EACCES;
 	}
 
 	return 0;
 
 addrsz:
-	fail_s1_walk(wr, ESR_ELx_FSC_ADDRSZ_L(level), false);
+	fail_s1_walk(wr, ESR_ELx_FSC_ADDRSZ_L(ws.level), false);
 	return -EINVAL;
 transfault:
-	fail_s1_walk(wr, ESR_ELx_FSC_FAULT_L(level), false);
+	fail_s1_walk(wr, ESR_ELx_FSC_FAULT_L(ws.level), false);
 	return -ENOENT;
 }
 
-- 
2.47.3

[PATCH v2 12/28] KVM: arm64: Create helper for stage-1 descriptor updates

[Oliver Upton]

Prepare for FEAT_HAFT support by creating a helper for descriptor
updates to be used for both leaf and table descriptors.

Reviewed-by: Leonardo Bras <leo.bras@arm.com>
Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 arch/arm64/kvm/at.c | 56 +++++++++++++++++++++++++--------------------
 1 file changed, 31 insertions(+), 25 deletions(-)

diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c
index dcbe5563baa2..eba51c6e01af 100644
--- a/arch/arm64/kvm/at.c
+++ b/arch/arm64/kvm/at.c
@@ -453,9 +453,28 @@ static int kvm_read_s1_desc(struct kvm_vcpu *vcpu, u64 pa, u64 *desc,
 	return 0;
 }
 
-static int kvm_swap_s1_desc(struct kvm_vcpu *vcpu, u64 pa, u64 old, u64 new,
-			    struct s1_walk_info *wi)
+static int handle_desc_update(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
+			      struct s1_walk_step *ws, struct s1_walk_result *wr,
+			      struct kvm_walk_access *access)
 {
+	u64 old, new;
+	int ret;
+
+	old = new = ws->desc;
+
+	if (wi->ha)
+		new |= PTE_AF;
+
+	if (new == old)
+		return 0;
+
+	if (wi->s2 && !ws->s2_trans.writable) {
+		fail_s1_walk(wr, ESR_ELx_FSC_PERM_L(ws->level), true);
+		return -EPERM;
+	}
+
+	ws->desc = new;
+
 	if (wi->be) {
 		old = (__force u64)cpu_to_be64(old);
 		new = (__force u64)cpu_to_be64(new);
@@ -464,7 +483,12 @@ static int kvm_swap_s1_desc(struct kvm_vcpu *vcpu, u64 pa, u64 old, u64 new,
 		new = (__force u64)cpu_to_le64(new);
 	}
 
-	return __kvm_at_swap_desc(vcpu->kvm, pa, old, new);
+	ret = __kvm_at_swap_desc(vcpu->kvm, ws->desc_pa, old, new);
+	if (!ret || ret == -EAGAIN)
+		return ret;
+
+	fail_s1_walk(wr, ESR_ELx_FSC_SEA_TTW(ws->level), false);
+	return ret;
 }
 
 static void compute_s1_permissions(struct kvm_vcpu *vcpu,
@@ -474,7 +498,7 @@ static void compute_s1_permissions(struct kvm_vcpu *vcpu,
 static int walk_s1(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 		   struct s1_walk_result *wr, struct kvm_walk_access *access)
 {
-	u64 va_top, va_bottom, baddr, new_desc, va;
+	u64 va_top, va_bottom, baddr, va;
 	struct s1_walk_step ws = {};
 	int stride, ret;
 
@@ -544,8 +568,6 @@ static int walk_s1(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 			return ret;
 		}
 
-		new_desc = ws.desc;
-
 		/* Invalid descriptor */
 		if (!(ws.desc & BIT(0)))
 			goto transfault;
@@ -615,25 +637,9 @@ static int walk_s1(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 
 	compute_s1_permissions(vcpu, wi, wr);
 
-	if (wi->ha)
-		new_desc |= PTE_AF;
-
-	if (new_desc != ws.desc) {
-		if (wi->s2 && !ws.s2_trans.writable) {
-			fail_s1_walk(wr, ESR_ELx_FSC_PERM_L(ws.level), true);
-			return -EPERM;
-		}
-
-		ret = kvm_swap_s1_desc(vcpu, ws.desc_pa, ws.desc, new_desc, wi);
-		if (ret == -EAGAIN)
-			return ret;
-		if (ret) {
-			fail_s1_walk(wr, ESR_ELx_FSC_SEA_TTW(ws.level), false);
-			return ret;
-		}
-
-		ws.desc = new_desc;
-	}
+	ret = handle_desc_update(vcpu, wi, &ws, wr, access);
+	if (ret)
+		return ret;
 
 	if (!(ws.desc & PTE_AF)) {
 		fail_s1_walk(wr, ESR_ELx_FSC_ACCESS_L(ws.level), false);
-- 
2.47.3

[PATCH v2 13/28] KVM: arm64: Set dirty state at stage-1

[Oliver Upton]

Mark the descriptor as dirty if the permissions are sufficient to grant
write access. Note that compute_s1_direct_permissions() already
considers the DBM bit as writable.

Reviewed-by: Leonardo Bras <leo.bras@arm.com>
Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 arch/arm64/include/asm/kvm_arm.h    |  1 +
 arch/arm64/include/asm/kvm_nested.h |  1 +
 arch/arm64/kvm/at.c                 | 34 ++++++++++++++++++++++++++---
 3 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index 678f0daed445..ad897dd71382 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -113,6 +113,7 @@
 #define TCR_EL2_DS		(1UL << 32)
 #define TCR_EL2_RES1		((1U << 31) | (1 << 23))
 #define TCR_EL2_HPD		(1 << 24)
+#define TCR_EL2_HD		(1 << 22)
 #define TCR_EL2_HA		(1 << 21)
 #define TCR_EL2_TBI		(1 << 20)
 #define TCR_EL2_PS_SHIFT	16
diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
index d7c1fa66f029..f0ac36fff652 100644
--- a/arch/arm64/include/asm/kvm_nested.h
+++ b/arch/arm64/include/asm/kvm_nested.h
@@ -337,6 +337,7 @@ struct s1_walk_info {
 	bool	     		s2;
 	bool			pa52bit;
 	bool			ha;
+	bool			hd;
 };
 
 struct s1_walk_result {
diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c
index eba51c6e01af..583fba1da73b 100644
--- a/arch/arm64/kvm/at.c
+++ b/arch/arm64/kvm/at.c
@@ -410,6 +410,10 @@ static int setup_s1_walk(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 	wi->ha &= (wi->regime == TR_EL2 ?
 		  FIELD_GET(TCR_EL2_HA, tcr) :
 		  FIELD_GET(TCR_HA, tcr));
+	wi->hd  = kvm_has_feat(vcpu->kvm, ID_AA64MMFR1_EL1, HAFDBS, DBM) && wi->ha;
+	wi->hd &= (wi->regime == TR_EL2 ?
+		  FIELD_GET(TCR_EL2_HD, tcr) :
+		  FIELD_GET(TCR_HD, tcr));
 
 	return 0;
 
@@ -453,6 +457,22 @@ static int kvm_read_s1_desc(struct kvm_vcpu *vcpu, u64 pa, u64 *desc,
 	return 0;
 }
 
+static bool should_set_dirty_state(struct s1_walk_info *wi, struct s1_walk_step *ws,
+				   struct s1_walk_result *wr, struct kvm_walk_access *access)
+{
+	bool perm = wi->as_el0 ? wr->uw : wr->pw;
+
+	switch (access->type) {
+	/* R_RKMHW */
+	case WALK_ACCESS_CMO:
+	case WALK_ACCESS_AT:
+		return false;
+	default:
+		/* R_NSXRD */
+		return access->write && wi->hd && perm;
+	}
+}
+
 static int handle_desc_update(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 			      struct s1_walk_step *ws, struct s1_walk_result *wr,
 			      struct kvm_walk_access *access)
@@ -465,6 +485,9 @@ static int handle_desc_update(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 	if (wi->ha)
 		new |= PTE_AF;
 
+	if (should_set_dirty_state(wi, ws, wr, access))
+		new &= ~PTE_RDONLY;
+
 	if (new == old)
 		return 0;
 
@@ -484,11 +507,16 @@ static int handle_desc_update(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 	}
 
 	ret = __kvm_at_swap_desc(vcpu->kvm, ws->desc_pa, old, new);
-	if (!ret || ret == -EAGAIN)
+	if (ret == -EAGAIN)
+		return ret;
+	if (ret) {
+		fail_s1_walk(wr, ESR_ELx_FSC_SEA_TTW(ws->level), false);
 		return ret;
+	}
 
-	fail_s1_walk(wr, ESR_ELx_FSC_SEA_TTW(ws->level), false);
-	return ret;
+	wr->desc = ws->desc;
+	wr->dirty = !(wr->desc & PTE_RDONLY);
+	return 0;
 }
 
 static void compute_s1_permissions(struct kvm_vcpu *vcpu,
-- 
2.47.3

[PATCH v2 14/28] KVM: arm64: nv: Respect HA when computing translated TCR

[Oliver Upton]

Add the missing forwarding of TCR_EL2.HA to TCR_EL1.HA, which are
different bit indices, to ensure that FEAT_HAF actually works for an L1
hypervisor in the EL2 translation regime.

Cc: stable@vger.kernel.org
Fixes: d5bbb76f4474 ("KVM: arm64: nv: Expose hardware access flag management to NV guests")
Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 arch/arm64/include/asm/kvm_nested.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
index f0ac36fff652..d51d011fc68a 100644
--- a/arch/arm64/include/asm/kvm_nested.h
+++ b/arch/arm64/include/asm/kvm_nested.h
@@ -29,7 +29,8 @@ static inline u64 translate_tcr_el2_to_tcr_el1(u64 tcr)
 	       (tcr & TCR_EL2_TG0_MASK) |
 	       (tcr & TCR_EL2_ORGN0_MASK) |
 	       (tcr & TCR_EL2_IRGN0_MASK) |
-	       (tcr & TCR_EL2_T0SZ_MASK);
+	       (tcr & TCR_EL2_T0SZ_MASK) |
+	       ((tcr & TCR_EL2_HA) ? TCR_HA : 0);
 }
 
 static inline u64 translate_cptr_el2_to_cpacr_el1(u64 cptr_el2)
-- 
2.47.3

[PATCH v2 15/28] KVM: arm64: nv: Respect HD when computing translated TCR

[Oliver Upton]

In a similar vein to the recent fix for FEAT_HAF, propagate the L1
hypervisor's setting for HD to the translated TCR_EL1 value, allowing
FEAT_HAFDBS to be used by an L1 hypervisor in the EL2 translation
regime.

Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 arch/arm64/include/asm/kvm_nested.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
index d51d011fc68a..11944dfb204c 100644
--- a/arch/arm64/include/asm/kvm_nested.h
+++ b/arch/arm64/include/asm/kvm_nested.h
@@ -30,7 +30,8 @@ static inline u64 translate_tcr_el2_to_tcr_el1(u64 tcr)
 	       (tcr & TCR_EL2_ORGN0_MASK) |
 	       (tcr & TCR_EL2_IRGN0_MASK) |
 	       (tcr & TCR_EL2_T0SZ_MASK) |
-	       ((tcr & TCR_EL2_HA) ? TCR_HA : 0);
+	       ((tcr & TCR_EL2_HA) ? TCR_HA : 0) |
+	       ((tcr & TCR_EL2_HD) ? TCR_HD : 0);
 }
 
 static inline u64 translate_cptr_el2_to_cpacr_el1(u64 cptr_el2)
-- 
2.47.3

[PATCH v2 16/28] KVM: arm64: Grant write permission when DBM is set at S1

[Oliver Upton]

The DBM bit effectively behaves as a permission bit when using direct
permissions, especially so in the context of accesses that do not update
the dirty state (like AT).

Treat AP[1] == 0 for the purposes of computing permissions, relying on
the walker to abort if it fails to set the dirty state where required.

Reviewed-by: Leonardo Bras <leo.bras@arm.com>
Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 arch/arm64/kvm/at.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c
index 583fba1da73b..20cc50c0f694 100644
--- a/arch/arm64/kvm/at.c
+++ b/arch/arm64/kvm/at.c
@@ -1078,11 +1078,23 @@ static void compute_s1_direct_permissions(struct kvm_vcpu *vcpu,
 					  struct s1_walk_info *wi,
 					  struct s1_walk_result *wr)
 {
+	u8 ap = FIELD_GET(PTE_USER | PTE_RDONLY, wr->desc);
 	bool wxn;
 
+	/*
+	 * Descriptors with the DBM bit set while hardware dirty state are
+	 * considered writable, even though certain accesses (like AT instructions)
+	 * don't actually update the dirty state.
+	 *
+	 * Assume that walk_s1() made the necessary descriptor updates for the
+	 * access and just treat DBM as writable here.
+	 */
+	if (wi->hd && (wr->desc & PTE_DBM))
+		ap &= ~BIT(1);
+
 	/* Non-hierarchical part of AArch64.S1DirectBasePermissions() */
 	if (wi->regime != TR_EL2) {
-		switch (FIELD_GET(PTE_USER | PTE_RDONLY, wr->desc)) {
+		switch (ap) {
 		case 0b00:
 			wr->pr = wr->pw = true;
 			wr->ur = wr->uw = false;
@@ -1106,7 +1118,7 @@ static void compute_s1_direct_permissions(struct kvm_vcpu *vcpu,
 	} else {
 		wr->ur = wr->uw = wr->ux = false;
 
-		if (!(wr->desc & PTE_RDONLY)) {
+		if (!(ap & BIT(1))) {
 			wr->pr = wr->pw = true;
 		} else {
 			wr->pr = true;
-- 
2.47.3

[PATCH v2 17/28] KVM: arm64: Don't update descriptors for "non-arch" access

[Oliver Upton]

WALK_ACCESS_NONARCH is a special indicator that the page table walk
request is not due to an architectural behavior and instead used
internally for KVM. Currently, the only user of this is SEA injection to
identify the correct translation level that generated the abort.

Ensure "non-arch" table walks are not made visible to the guest and
explicitly avoid descriptor updates. These accesses are not expected to
be used for pseudo-TLB fills, nor are they used to dirty any memory.

Reviewed-by: Leonardo Bras <leo.bras@arm.com>
Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 arch/arm64/kvm/at.c     | 15 ++++++++++++++-
 arch/arm64/kvm/nested.c | 12 +++++++++++-
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c
index 20cc50c0f694..2ce34af3b9f2 100644
--- a/arch/arm64/kvm/at.c
+++ b/arch/arm64/kvm/at.c
@@ -457,6 +457,15 @@ static int kvm_read_s1_desc(struct kvm_vcpu *vcpu, u64 pa, u64 *desc,
 	return 0;
 }
 
+static bool should_set_access_flag(struct s1_walk_info *wi, struct s1_walk_step *ws,
+				   struct kvm_walk_access *access)
+{
+	if (access->type == WALK_ACCESS_NONARCH)
+		return false;
+
+	return wi->ha;
+}
+
 static bool should_set_dirty_state(struct s1_walk_info *wi, struct s1_walk_step *ws,
 				   struct s1_walk_result *wr, struct kvm_walk_access *access)
 {
@@ -466,6 +475,7 @@ static bool should_set_dirty_state(struct s1_walk_info *wi, struct s1_walk_step
 	/* R_RKMHW */
 	case WALK_ACCESS_CMO:
 	case WALK_ACCESS_AT:
+	case WALK_ACCESS_NONARCH:
 		return false;
 	default:
 		/* R_NSXRD */
@@ -482,7 +492,7 @@ static int handle_desc_update(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 
 	old = new = ws->desc;
 
-	if (wi->ha)
+	if (should_set_access_flag(wi, ws, access))
 		new |= PTE_AF;
 
 	if (should_set_dirty_state(wi, ws, wr, access))
@@ -558,6 +568,9 @@ static int walk_s1(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 				.write	= wi->ha,
 			};
 
+			if (access->type == WALK_ACCESS_NONARCH)
+				s2_access.type = WALK_ACCESS_NONARCH;
+
 			ret = kvm_walk_nested_s2(vcpu, &s2_access, &ws.s2_trans);
 			if (ret == -EAGAIN)
 				return ret;
diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index 964331dbf475..7dda95d37925 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -247,6 +247,15 @@ static int read_guest_s2_desc(struct kvm_vcpu *vcpu, struct s2_walk_step *ws,
 	return 0;
 }
 
+static bool should_set_access_flag(struct s2_walk_info *wi, struct s2_walk_step *ws,
+				   struct kvm_walk_access *access)
+{
+	if (access->type == WALK_ACCESS_NONARCH)
+		return false;
+
+	return wi->ha;
+}
+
 static bool should_set_dirty_state(struct s2_walk_info *wi, struct s2_walk_step *ws,
 				   struct kvm_s2_trans *out, struct kvm_walk_access *access)
 {
@@ -254,6 +263,7 @@ static bool should_set_dirty_state(struct s2_walk_info *wi, struct s2_walk_step
 	/* R_ZYGHJ */
 	case WALK_ACCESS_CMO:
 	case WALK_ACCESS_AT:
+	case WALK_ACCESS_NONARCH:
 		return false;
 	default:
 		/* R_NSXRD */
@@ -270,7 +280,7 @@ static int handle_desc_update(struct kvm_vcpu *vcpu, struct s2_walk_info *wi,
 
 	old = new = ws->desc;
 
-	if (wi->ha)
+	if (should_set_access_flag(wi, ws, access))
 		new |= KVM_PTE_LEAF_ATTR_LO_S2_AF;
 
 	if (should_set_dirty_state(wi, ws, out, access))
-- 
2.47.3

[PATCH v2 18/28] KVM: arm64: nv: Emulate CMOs via writable-clean translations as NOPs

[Oliver Upton]

Virtualizing FEAT_HAFDBS for an L1 hypervisor has a pretty large
architectural deficiency by way of cache maintenance instructions.
CMOs (e.g. DC IVAC) require write permission but do not set the dirty
state on the descriptor.

There's no way to shadow a writable-clean translation in such a way that
these CMOs pass the permission check and subsequent writes generate a
permission fault. Worse yet, data aborts taken on CMOs do not have
sufficient syndrome information to perform instruction emulation and
trapping all CMOs is unpalatable.

So what do we do? Absolutely nothing. Treat CMOs to writable-clean
translations as NOPs in anticipation of KVM only exposing FEAT_HAFDBS
when the VM is fully coherent (FEAT_S2FWB, CTR_EL0.{DIC,IDC}={1,1}, no
noncoherent DMA).

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

diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 7b29be33a1ea..a21b674a1981 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -2548,6 +2548,10 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu)
 		}
 
 		ret = kvm_s2_handle_perm_fault(vcpu, &nested_trans);
+		if (ret == -EAGAIN) {
+			ret = 1;
+			goto out_unlock;
+		}
 		if (ret) {
 			esr = nested_trans.esr;
 			kvm_inject_s2_fault(vcpu, esr);
diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index 7dda95d37925..46fcc75f4c20 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -1150,6 +1150,26 @@ int kvm_s2_handle_perm_fault(struct kvm_vcpu *vcpu, struct kvm_s2_trans *trans)
 	} else {
 		bool write_fault = kvm_is_write_fault(vcpu);
 
+		/*
+		 * Plug your nose for a moment.
+		 *
+		 * Writable-clean translations are shadowed using a read-only
+		 * mapping so KVM can set the dirty state on the L1 descriptor
+		 * before installing a writable mapping.
+		 *
+		 * Certain cache maintenance instructions require a write
+		 * permission check but leave the translation in a writable-clean
+		 * state, meaning KVM is on the hook to emulate CMOs that use a
+		 * writable-clean translation.
+		 *
+		 * "Emulate" the CMO as a NOP, as KVM only exposes FEAT_HAFDBS
+		 * when FEAT_S2FWB is present and the I-cache is coherent.
+		 */
+		if (kvm_vcpu_dabt_is_cm(vcpu) && trans->writable && !trans->dirty) {
+			kvm_incr_pc(vcpu);
+			return -EAGAIN;
+		}
+
 		forward_fault = ((write_fault && !trans->writable) ||
 				 (!write_fault && !trans->readable));
 	}
-- 
2.47.3

[PATCH v2 19/28] arm64: Add command line switch to disable FEAT_S2FWB

[Oliver Upton]

Assigning noncoherent devices to KVM guests does not work when guest
memory is forced writeback. There's absolutely no reason to support
this configuration, especially when disabling FWB is a valid fallback.

Add a command line switch to disable FWB in anticipation of KVM
mandating it for noncoherent DMA. Use a kvm_arm alias that can be
remapped in the distant future to select passthough S2 attributes
when FEAT_S2FWB becomes mandatory in the architecture.

Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 Documentation/admin-guide/kernel-parameters.txt | 4 ++++
 arch/arm64/kernel/pi/idreg-override.c           | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 33cd30996e47..1c0e3b9e726d 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3265,6 +3265,10 @@ Kernel parameters
 			"nested" and "protected" are experimental and should be
 			used with extreme caution.
 
+	kvm-arm.nofwb
+			[KVM,ARM,EARLY] Disable the use of stage-2 forced
+			writeback in KVM.
+
 	kvm-arm.vgic_v3_group0_trap=
 			[KVM,ARM,EARLY] Trap guest accesses to GICv3 group-0
 			system registers
diff --git a/arch/arm64/kernel/pi/idreg-override.c b/arch/arm64/kernel/pi/idreg-override.c
index 274bc72ba98d..188e1fb2eb2c 100644
--- a/arch/arm64/kernel/pi/idreg-override.c
+++ b/arch/arm64/kernel/pi/idreg-override.c
@@ -103,6 +103,7 @@ static const struct ftr_set_desc mmfr2 __prel64_initconst = {
 	.override	= &id_aa64mmfr2_override,
 	.fields		= {
 		FIELD("varange", ID_AA64MMFR2_EL1_VARange_SHIFT, mmfr2_varange_filter),
+		FIELD("fwb", ID_AA64MMFR2_EL1_FWB_SHIFT, NULL),
 		{}
 	},
 };
@@ -245,6 +246,7 @@ static const struct {
 } aliases[] __initconst = {
 	{ "kvm_arm.mode=nvhe",		"arm64_sw.hvhe=0 id_aa64mmfr1.vh=0" },
 	{ "kvm_arm.mode=protected",	"arm64_sw.hvhe=1" },
+	{ "kvm_arm.nofwb",		"id_aa64mmfr2.fwb=0" },
 	{ "arm64.nosve",		"id_aa64pfr0.sve=0" },
 	{ "arm64.nosme",		"id_aa64pfr1.sme=0" },
 	{ "arm64.nobti",		"id_aa64pfr1.bt=0" },
-- 
2.47.3

[PATCH v2 20/28] KVM: arm64: Reject non-coherent DMA when using forced writeback

[Oliver Upton]

Assigning non-coherent devices to a KVM guest subject to forced
writeback is broken. DMA through cacheable aliases and CMOs work, but
using non-cacheable aliases for buffers on the CPU does not due to the
implicit promotion to Normal writeback.

Non-coherent DMA is fundamentally incompatible with the direction of
travel on KVM/arm64. Draw the line in the sand and bug the VM if FWB is
in use. Users can use the newly-added kernel commandline to fall back to
a non-FWB configuration that *actually works*.

Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 arch/arm64/include/asm/kvm_host.h |  2 ++
 arch/arm64/kvm/arm.c              | 14 ++++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 1c6632216e32..5672721416a4 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -1565,6 +1565,8 @@ struct kvm *kvm_arch_alloc_vm(void);
 
 #define __KVM_HAVE_ARCH_FLUSH_REMOTE_TLBS_RANGE
 
+#define __KVM_HAVE_ARCH_NONCOHERENT_DMA
+
 #ifdef __KVM_NVHE_HYPERVISOR__
 
 #define kvm_vm_is_protected(kvm)			\
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index d593ba928093..f08a450184d9 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -133,6 +133,20 @@ int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu)
 	return kvm_vcpu_exiting_guest_mode(vcpu) == IN_GUEST_MODE;
 }
 
+void kvm_arch_register_noncoherent_dma(struct kvm *kvm)
+{
+	if (!cpus_have_final_cap(ARM64_HAS_STAGE2_FWB))
+		return;
+
+	pr_err_once("kvm: noncoherent DMA unsupported with FWB, see kvm_arm.nofwb\n");
+	kvm_vm_bugged(kvm);
+}
+
+void kvm_arch_unregister_noncoherent_dma(struct kvm *kvm)
+{
+
+}
+
 int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
 			    struct kvm_enable_cap *cap)
 {
-- 
2.47.3

Re: [PATCH v2 20/28] KVM: arm64: Reject non-coherent DMA when using forced writeback

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] KVM: arm64: kvm_arch_register_noncoherent_dma() bugs VM for coherent VFIO devs
--

commit e65fa223b5ab689dd29e0940ed61947cf8f5ac8a
Author: Oliver Upton <oupton@kernel.org>

KVM: arm64: Reject non-coherent DMA when using forced writeback

This commit implements kvm_arch_register_noncoherent_dma() on arm64 to mark a
VM as bugged when non-coherent DMA is registered on a system using stage-2
forced writeback (ARM64_HAS_STAGE2_FWB).

> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index d593ba9280933..f08a450184d96 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -133,6 +133,20 @@ int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu)
>  	return kvm_vcpu_exiting_guest_mode(vcpu) == IN_GUEST_MODE;
>  }
>  
> +void kvm_arch_register_noncoherent_dma(struct kvm *kvm)
> +{
> +	if (!cpus_have_final_cap(ARM64_HAS_STAGE2_FWB))
> +		return;
> +
> +	pr_err_once("kvm: noncoherent DMA unsupported with FWB, see kvm_arm.nofwb\n");
> +	kvm_vm_bugged(kvm);
> +}

[Severity: High]
Will calling kvm_vm_bugged() here break VFIO passthrough for cache-coherent
devices on systems with ARM64_HAS_STAGE2_FWB that do not advertise
IOMMU_CAP_ENFORCE_CACHE_COHERENCY?

When userspace attaches a VFIO group or device file to a VM via
KVM_DEV_VFIO_FILE_ADD, virt/kvm/vfio.c:kvm_vfio_update_coherency() checks
kvm_vfio_file_enforced_coherent():

	list_for_each_entry(kvf, &kv->file_list, node) {
		if (!kvm_vfio_file_enforced_coherent(kvf->file)) {
			noncoherent = true;
			break;
		}
	}

	if (noncoherent != kv->noncoherent) {
		kv->noncoherent = noncoherent;

		if (kv->noncoherent)
			kvm_arch_register_noncoherent_dma(dev->kvm);

That helper calls drivers/vfio/vfio_main.c:vfio_file_enforced_coherent(),
which queries IOMMU_CAP_ENFORCE_CACHE_COHERENCY:

	device = vfio_device_from_file(file);
	if (device)
		return device_iommu_capable(device->dev,
					    IOMMU_CAP_ENFORCE_CACHE_COHERENCY);

While VFIO and iommufd already require IOMMU_CAP_CACHE_COHERENCY at device
registration and bind time (__vfio_register_dev() and iommufd_bind_iommu()),
IOMMU_CAP_ENFORCE_CACHE_COHERENCY is much narrower on arm64. In
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c:arm_smmu_capable(), it is gated
on arm_smmu_master_canwbs():

	case IOMMU_CAP_CACHE_COHERENCY:
		/* Assume that a coherent TCU implies coherent TBUs */
		return master->smmu->features & ARM_SMMU_FEAT_COHERENCY;
	case IOMMU_CAP_ENFORCE_CACHE_COHERENCY:
		return arm_smmu_master_canwbs(master);

And IOMMU_FWSPEC_PCI_RC_CANWBS is only set in
drivers/acpi/arm64/iort.c:iort_iommu_configure_id() for ACPI PCI root
complexes whose IORT node sets ACPI_IORT_MF_CANWBS:

		if (fwspec && iort_pci_rc_supports_canwbs(node))
			fwspec->flags |= IOMMU_FWSPEC_PCI_RC_CANWBS;

As a result, device_iommu_capable(dev, IOMMU_CAP_ENFORCE_CACHE_COHERENCY)
returns false on Device Tree systems, ACPI systems with IORT tables that only
set CCA=1 (ACPI_IORT_MF_COHERENCY) without ACPI_IORT_MF_CANWBS, non-PCI
platform devices, and systems using other arm64 IOMMU drivers (such as
arm-smmu, virtio-iommu, apple-dart, or qcom_iommu).

On any of those systems with ARM64_HAS_STAGE2_FWB, won't attaching a normal
cache-coherent VFIO device invoke kvm_arch_register_noncoherent_dma() and
immediately terminate the VM via kvm_vm_bugged()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023205610.1044532-1-oupton@kernel.org?part=20

[PATCH v2 21/28] KVM: arm64: nv: Expose FEAT_HAFDBS

[Oliver Upton]

Everything is in place now to support hardware dirty state management.
Flip the corresponding bit and advertise the feature to the guest.

Reviewed-by: Leonardo Bras <leo.bras@arm.com>
Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 arch/arm64/kvm/nested.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index 46fcc75f4c20..eb20409a14b1 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -2040,7 +2040,12 @@ u64 limit_nv_id_reg(struct kvm *kvm, u32 reg, u64 val)
 		if (test_bit(KVM_ARM_VCPU_HAS_EL2_E2H0, kvm->arch.vcpu_features))
 			val &= ~ID_AA64MMFR1_EL1_VH;
 
-		val = ID_REG_LIMIT_FIELD_ENUM(val, ID_AA64MMFR1_EL1, HAFDBS, AF);
+		/* FEAT_S2FWB implies IDC */
+		if (cpus_have_final_cap(ARM64_HAS_STAGE2_FWB) &&
+		    cpus_have_final_cap(ARM64_HAS_CACHE_DIC))
+			val = ID_REG_LIMIT_FIELD_ENUM(val, ID_AA64MMFR1_EL1, HAFDBS, DBM);
+		else
+			val = ID_REG_LIMIT_FIELD_ENUM(val, ID_AA64MMFR1_EL1, HAFDBS, AF);
 		break;
 
 	case SYS_ID_AA64MMFR2_EL1:
-- 
2.47.3

[PATCH v2 22/28] KVM: arm64: Set Access flag on table descriptors at stage-1

[Oliver Upton]

Implement access flag updates on table descriptors as required by
FEAT_HAFT. Note that unlike leaf descriptors, access flags on table
descriptors never generate a fault.

Reviewed-by: Leonardo Bras <leo.bras@arm.com>
Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 arch/arm64/include/asm/kvm_nested.h  |  1 +
 arch/arm64/include/asm/kvm_pgtable.h | 11 +++++++++++
 arch/arm64/kvm/at.c                  | 23 ++++++++++++++++-------
 arch/arm64/kvm/hyp/pgtable.c         | 11 -----------
 4 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
index 11944dfb204c..81d8b1676fe4 100644
--- a/arch/arm64/include/asm/kvm_nested.h
+++ b/arch/arm64/include/asm/kvm_nested.h
@@ -340,6 +340,7 @@ struct s1_walk_info {
 	bool			pa52bit;
 	bool			ha;
 	bool			hd;
+	bool			haft;
 };
 
 struct s1_walk_result {
diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
index 23e93f14af7b..0912987ea21a 100644
--- a/arch/arm64/include/asm/kvm_pgtable.h
+++ b/arch/arm64/include/asm/kvm_pgtable.h
@@ -139,6 +139,17 @@ static inline bool kvm_pte_valid(kvm_pte_t pte)
 	return pte & KVM_PTE_VALID;
 }
 
+static inline bool kvm_pte_table(kvm_pte_t pte, s8 level)
+{
+	if (level == KVM_PGTABLE_LAST_LEVEL)
+		return false;
+
+	if (!kvm_pte_valid(pte))
+		return false;
+
+	return FIELD_GET(KVM_PTE_TYPE, pte) == KVM_PTE_TYPE_TABLE;
+}
+
 static inline u64 kvm_pte_to_phys(kvm_pte_t pte)
 {
 	u64 pa;
diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c
index 2ce34af3b9f2..8f1be3af64e8 100644
--- a/arch/arm64/kvm/at.c
+++ b/arch/arm64/kvm/at.c
@@ -414,6 +414,8 @@ static int setup_s1_walk(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 	wi->hd &= (wi->regime == TR_EL2 ?
 		  FIELD_GET(TCR_EL2_HD, tcr) :
 		  FIELD_GET(TCR_HD, tcr));
+	wi->haft  = kvm_has_feat(vcpu->kvm, ID_AA64MMFR1_EL1, HAFDBS, HAFT) && wi->ha &&
+		    FIELD_GET(TCR2_EL1_HAFT, effective_tcr2(vcpu, wi->regime));
 
 	return 0;
 
@@ -463,7 +465,7 @@ static bool should_set_access_flag(struct s1_walk_info *wi, struct s1_walk_step
 	if (access->type == WALK_ACCESS_NONARCH)
 		return false;
 
-	return wi->ha;
+	return kvm_pte_table(ws->desc, ws->level) ? wi->haft : wi->ha;
 }
 
 static bool should_set_dirty_state(struct s1_walk_info *wi, struct s1_walk_step *ws,
@@ -471,6 +473,9 @@ static bool should_set_dirty_state(struct s1_walk_info *wi, struct s1_walk_step
 {
 	bool perm = wi->as_el0 ? wr->uw : wr->pw;
 
+	if (kvm_pte_table(ws->desc, ws->level))
+		return false;
+
 	switch (access->type) {
 	/* R_RKMHW */
 	case WALK_ACCESS_CMO:
@@ -621,6 +626,16 @@ static int walk_s1(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 		if (ws.level == 3)
 			break;
 
+		baddr = desc_to_oa(wi, ws.desc);
+
+		/* Check for out-of-range OA */
+		if (check_output_size(baddr, wi))
+			goto addrsz;
+
+		ret = handle_desc_update(vcpu, wi, &ws, wr, access);
+		if (ret)
+			return ret;
+
 		/* Table handling */
 		if (!wi->hpd) {
 			wr->APTable  |= FIELD_GET(S1_TABLE_AP, ws.desc);
@@ -628,12 +643,6 @@ static int walk_s1(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 			wr->PXNTable |= FIELD_GET(PMD_TABLE_PXN, ws.desc);
 		}
 
-		baddr = desc_to_oa(wi, ws.desc);
-
-		/* Check for out-of-range OA */
-		if (check_output_size(baddr, wi))
-			goto addrsz;
-
 		/* Prepare for next round */
 		va_top = va_bottom - 1;
 		ws.level++;
diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index 9c83837b184d..fc22dc70f167 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -72,17 +72,6 @@ static u32 kvm_pgd_pages(u32 ia_bits, s8 start_level)
 	return kvm_pgd_page_idx(&pgt, -1ULL) + 1;
 }
 
-static bool kvm_pte_table(kvm_pte_t pte, s8 level)
-{
-	if (level == KVM_PGTABLE_LAST_LEVEL)
-		return false;
-
-	if (!kvm_pte_valid(pte))
-		return false;
-
-	return FIELD_GET(KVM_PTE_TYPE, pte) == KVM_PTE_TYPE_TABLE;
-}
-
 static kvm_pte_t *kvm_pte_follow(kvm_pte_t pte, struct kvm_pgtable_mm_ops *mm_ops)
 {
 	return mm_ops->phys_to_virt(kvm_pte_to_phys(pte));
-- 
2.47.3

Re: [PATCH v2 22/28] KVM: arm64: Set Access flag on table descriptors at stage-1

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] KVM: arm64: missing kvm_has_tcr2() check in effective_tcr2()
--

commit bcde4120f508adf06668822f4f97b91e98f978ff
Author: Oliver Upton <oupton@kernel.org>

KVM: arm64: Set Access flag on table descriptors at stage-1

This commit implements FEAT_HAFT stage-1 access flag updates on table
descriptors during software page table walks.

> diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c
> index 2ce34af3b9f20..8f1be3af64e88 100644
> --- a/arch/arm64/kvm/at.c
> +++ b/arch/arm64/kvm/at.c
> @@ -414,6 +414,8 @@ static int setup_s1_walk(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
>  	wi->hd &= (wi->regime == TR_EL2 ?
>  		  FIELD_GET(TCR_EL2_HD, tcr) :
>  		  FIELD_GET(TCR_HD, tcr));
> +	wi->haft  = kvm_has_feat(vcpu->kvm, ID_AA64MMFR1_EL1, HAFDBS, HAFT) && wi->ha &&
> +		    FIELD_GET(TCR2_EL1_HAFT, effective_tcr2(vcpu, wi->regime));

[Severity: Medium]
Can this read a stale physical SYS_TCR2_EL12 register value if userspace
disables FEAT_TCR2 (ID_AA64MMFR3_EL1.TCRX = 0) while leaving
ID_AA64MMFR1_EL1.HAFDBS set to HAFT?

ID_AA64MMFR1_EL1 and ID_AA64MMFR3_EL1 are independently writable from
userspace via KVM_SET_ONE_REG and are not cross-sanitized in
kvm_vm_finalize_sys_regs(). When kvm_has_tcr2(vcpu->kvm) is false,
ctxt_has_tcrx() returns false in arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h:

static inline bool ctxt_has_tcrx(struct kvm_cpu_context *ctxt)
{
	struct kvm_vcpu *vcpu;

	if (!cpus_have_final_cap(ARM64_HAS_TCR2))
		return false;

	vcpu = ctxt_to_vcpu(ctxt);
	return kvm_has_tcr2(kern_hyp_va(vcpu->kvm));
}

As a result, __sysreg_restore_el1_state() and __sysreg_restore_vel2_state()
do not context-switch the physical SYS_TCR2_EL12 register when loading the
vCPU.

However, effective_tcr2() in arch/arm64/kvm/at.c does not check
kvm_has_tcr2(vcpu->kvm):

static u64 effective_tcr2(struct kvm_vcpu *vcpu, enum trans_regime regime)
{
	if (regime == TR_EL10) {
		if (vcpu_has_nv(vcpu) &&
		    !(__vcpu_sys_reg(vcpu, HCRX_EL2) & HCRX_EL2_TCR2En))
			return 0;

		return vcpu_read_sys_reg(vcpu, TCR2_EL1);
	}

	return vcpu_read_sys_reg(vcpu, TCR2_EL2);
}

When setup_s1_walk() runs while SYSREGS_ON_CPU is set (for example, in
TR_EL2/TR_EL20 via handle_at_s1e2() -> __kvm_at_s1e2() -> handle_at_slow() ->
setup_s1_walk() or kvm_s2_handle_perm_fault() -> __kvm_translate_va() ->
setup_s1_walk(), or for a non-NV guest in TR_EL10 via kvm_inject_sea() ->
inject_abt64() -> __kvm_find_s1_desc_level() -> setup_s1_walk()),
vcpu_read_sys_reg() resolves to read_sr_from_cpu() in
arch/arm64/kvm/sys_regs.c:

	case TCR2_EL1:		val = read_sysreg_s(SYS_TCR2_EL12);	break;

This reads whatever SYS_TCR2_EL12 value was left on the physical CPU by a
previous vCPU, which could set wi->haft and cause walk_s1() ->
handle_desc_update() to set PTE_AF on guest stage-1 table descriptors.

Should effective_tcr2() check kvm_has_tcr2(vcpu->kvm) first (similar to
__kvm_at_s1e01_fast() and __effective_sctlr2_bit()), or should setup_s1_walk()
gate wi->haft on kvm_has_tcr2(vcpu->kvm)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023205610.1044532-1-oupton@kernel.org?part=22

Re: [PATCH v2 22/28] KVM: arm64: Set Access flag on table descriptors at stage-1

[Oliver Upton]

On Fri, Oct 23, 2026 at 09:22:33PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] KVM: arm64: missing kvm_has_tcr2() check in effective_tcr2()
> --
> 
> commit bcde4120f508adf06668822f4f97b91e98f978ff
> Author: Oliver Upton <oupton@kernel.org>
> 
> KVM: arm64: Set Access flag on table descriptors at stage-1
> 
> This commit implements FEAT_HAFT stage-1 access flag updates on table
> descriptors during software page table walks.
> 
> > diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c
> > index 2ce34af3b9f20..8f1be3af64e88 100644
> > --- a/arch/arm64/kvm/at.c
> > +++ b/arch/arm64/kvm/at.c
> > @@ -414,6 +414,8 @@ static int setup_s1_walk(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
> >  	wi->hd &= (wi->regime == TR_EL2 ?
> >  		  FIELD_GET(TCR_EL2_HD, tcr) :
> >  		  FIELD_GET(TCR_HD, tcr));
> > +	wi->haft  = kvm_has_feat(vcpu->kvm, ID_AA64MMFR1_EL1, HAFDBS, HAFT) && wi->ha &&
> > +		    FIELD_GET(TCR2_EL1_HAFT, effective_tcr2(vcpu, wi->regime));
> 
> [Severity: Medium]
> Can this read a stale physical SYS_TCR2_EL12 register value if userspace
> disables FEAT_TCR2 (ID_AA64MMFR3_EL1.TCRX = 0) while leaving
> ID_AA64MMFR1_EL1.HAFDBS set to HAFT?
> 
> ID_AA64MMFR1_EL1 and ID_AA64MMFR3_EL1 are independently writable from
> userspace via KVM_SET_ONE_REG and are not cross-sanitized in
> kvm_vm_finalize_sys_regs(). When kvm_has_tcr2(vcpu->kvm) is false,
> ctxt_has_tcrx() returns false in arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h:

Hrm, having effective_tcr2() return 0 in the case that FEAT_TCRX isn't
implemented seems to be the best way out here.

Unlike the enforcement we need to do for context save/restore
dependencies, KVM emulation assumes that system registers take an
effective value that's consistent with the feature being NI.

Thanks,
Oliver

> static inline bool ctxt_has_tcrx(struct kvm_cpu_context *ctxt)
> {
> 	struct kvm_vcpu *vcpu;
> 
> 	if (!cpus_have_final_cap(ARM64_HAS_TCR2))
> 		return false;
> 
> 	vcpu = ctxt_to_vcpu(ctxt);
> 	return kvm_has_tcr2(kern_hyp_va(vcpu->kvm));
> }
> 
> As a result, __sysreg_restore_el1_state() and __sysreg_restore_vel2_state()
> do not context-switch the physical SYS_TCR2_EL12 register when loading the
> vCPU.
> 
> However, effective_tcr2() in arch/arm64/kvm/at.c does not check
> kvm_has_tcr2(vcpu->kvm):
> 
> static u64 effective_tcr2(struct kvm_vcpu *vcpu, enum trans_regime regime)
> {
> 	if (regime == TR_EL10) {
> 		if (vcpu_has_nv(vcpu) &&
> 		    !(__vcpu_sys_reg(vcpu, HCRX_EL2) & HCRX_EL2_TCR2En))
> 			return 0;
> 
> 		return vcpu_read_sys_reg(vcpu, TCR2_EL1);
> 	}
> 
> 	return vcpu_read_sys_reg(vcpu, TCR2_EL2);
> }
> 
> When setup_s1_walk() runs while SYSREGS_ON_CPU is set (for example, in
> TR_EL2/TR_EL20 via handle_at_s1e2() -> __kvm_at_s1e2() -> handle_at_slow() ->
> setup_s1_walk() or kvm_s2_handle_perm_fault() -> __kvm_translate_va() ->
> setup_s1_walk(), or for a non-NV guest in TR_EL10 via kvm_inject_sea() ->
> inject_abt64() -> __kvm_find_s1_desc_level() -> setup_s1_walk()),
> vcpu_read_sys_reg() resolves to read_sr_from_cpu() in
> arch/arm64/kvm/sys_regs.c:
> 
> 	case TCR2_EL1:		val = read_sysreg_s(SYS_TCR2_EL12);	break;
> 
> This reads whatever SYS_TCR2_EL12 value was left on the physical CPU by a
> previous vCPU, which could set wi->haft and cause walk_s1() ->
> handle_desc_update() to set PTE_AF on guest stage-1 table descriptors.
> 
> Should effective_tcr2() check kvm_has_tcr2(vcpu->kvm) first (similar to
> __kvm_at_s1e01_fast() and __effective_sctlr2_bit()), or should setup_s1_walk()
> gate wi->haft on kvm_has_tcr2(vcpu->kvm)?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261023205610.1044532-1-oupton@kernel.org?part=22

[PATCH v2 23/28] KVM: arm64: nv: Set access flag on table descriptors at stage-2

[Oliver Upton]

Implement access flag updates on table descriptors as required by
FEAT_HAFT.

Reviewed-by: Leonardo Bras <leo.bras@arm.com>
Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 arch/arm64/kvm/nested.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index eb20409a14b1..5ae818a70ae2 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -152,6 +152,7 @@ struct s2_walk_info {
 	bool		be;
 	bool		ha;
 	bool		hd;
+	bool		haft;
 };
 
 struct s2_walk_step {
@@ -253,12 +254,15 @@ static bool should_set_access_flag(struct s2_walk_info *wi, struct s2_walk_step
 	if (access->type == WALK_ACCESS_NONARCH)
 		return false;
 
-	return wi->ha;
+	return kvm_pte_table(ws->desc, ws->level) ? wi->haft : wi->ha;
 }
 
 static bool should_set_dirty_state(struct s2_walk_info *wi, struct s2_walk_step *ws,
 				   struct kvm_s2_trans *out, struct kvm_walk_access *access)
 {
+	if (kvm_pte_table(ws->desc, ws->level))
+		return false;
+
 	switch (access->type) {
 	/* R_ZYGHJ */
 	case WALK_ACCESS_CMO:
@@ -447,6 +451,10 @@ static int walk_nested_s2_pgd(struct kvm_vcpu *vcpu, struct kvm_walk_access *acc
 			return 1;
 		}
 
+		ret = handle_desc_update(vcpu, wi, &ws, out, access);
+		if (ret)
+			return ret;
+
 		base_addr = ws.desc & GENMASK_ULL(47, wi->pgshift);
 
 		ws.level += 1;
@@ -585,6 +593,7 @@ static void setup_s2_walk(struct kvm_vcpu *vcpu, struct s2_walk_info *wi)
 	wi->ha = vtcr & VTCR_EL2_HA;
 	wi->be = vcpu_read_sys_reg(vcpu, SCTLR_EL2) & SCTLR_ELx_EE;
 	wi->hd = wi->ha && (vtcr & VTCR_EL2_HD);
+	wi->haft = wi->ha && (vtcr & VTCR_EL2_HAFT);
 }
 
 int kvm_walk_nested_s2(struct kvm_vcpu *vcpu, struct kvm_walk_access *access,
-- 
2.47.3

[PATCH v2 24/28] KVM: arm64: nv: Expose FEAT_HAFT

[Oliver Upton]

Both stages of translation now support the access flag on table
descriptors, advertise the feature.

Reviewed-by: Leonardo Bras <leo.bras@arm.com>
Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 arch/arm64/kvm/nested.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index 5ae818a70ae2..0c468235f123 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -2052,7 +2052,7 @@ u64 limit_nv_id_reg(struct kvm *kvm, u32 reg, u64 val)
 		/* FEAT_S2FWB implies IDC */
 		if (cpus_have_final_cap(ARM64_HAS_STAGE2_FWB) &&
 		    cpus_have_final_cap(ARM64_HAS_CACHE_DIC))
-			val = ID_REG_LIMIT_FIELD_ENUM(val, ID_AA64MMFR1_EL1, HAFDBS, DBM);
+			val = ID_REG_LIMIT_FIELD_ENUM(val, ID_AA64MMFR1_EL1, HAFDBS, HAFT);
 		else
 			val = ID_REG_LIMIT_FIELD_ENUM(val, ID_AA64MMFR1_EL1, HAFDBS, AF);
 		break;
-- 
2.47.3

[PATCH v2 25/28] KVM: arm64: selftests: Only test AF behavior for emulated AT insns

[Oliver Upton]

The original intent of the test was to test KVM's slowpath AT emulation,
specifically including a test for KVM's IMP DEF choice that AT
instructions set the access flag.

While it is possible to force AT instructions to take the slow path in
KVM, this is only effective for an out-of-context regime wherein the
stage-2 translations are guaranteed to be invalid. As such, the
inclusion of AT S1E2* instructions muddied this a bit, as they operate
on the in-context EL2&0 regime.

Restore the original intent of the test and get rid of the AT S1E2* test
cases entirely.

Reviewed-by: Leonardo Bras <leo.bras@arm.com>
Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 tools/testing/selftests/kvm/arm64/at.c | 38 ++++++++++++++++----------
 1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/tools/testing/selftests/kvm/arm64/at.c b/tools/testing/selftests/kvm/arm64/at.c
index ce5d312ef6ba..d7289f3df04f 100644
--- a/tools/testing/selftests/kvm/arm64/at.c
+++ b/tools/testing/selftests/kvm/arm64/at.c
@@ -13,6 +13,7 @@
 
 enum {
 	CLEAR_ACCESS_FLAG,
+	TEST_ACCESS_FLAG,
 };
 
 static u64 *ptep_hva;
@@ -48,21 +49,12 @@ do {											\
 		GUEST_ASSERT_EQ(FIELD_GET(SYS_PAR_EL1_ATTR, par), MAIR_ATTR_NORMAL);	\
 		GUEST_ASSERT_EQ(FIELD_GET(SYS_PAR_EL1_SH, par), PTE_SHARED >> 8);	\
 		GUEST_ASSERT_EQ(par & SYS_PAR_EL1_PA, TEST_ADDR);			\
+		GUEST_SYNC(TEST_ACCESS_FLAG);						\
 	}										\
 } while (0)
 
 static void test_at(bool expect_fault)
 {
-	test_at_insn(S1E2R, expect_fault);
-	test_at_insn(S1E2W, expect_fault);
-
-	/* Reuse the stage-1 MMU context from EL2 at EL1 */
-	copy_el2_to_el1(SCTLR);
-	copy_el2_to_el1(MAIR);
-	copy_el2_to_el1(TCR);
-	copy_el2_to_el1(TTBR0);
-	copy_el2_to_el1(TTBR1);
-
 	/* Disable stage-2 translation and enter a non-host context */
 	write_sysreg(0, vtcr_el2);
 	write_sysreg(0, vttbr_el2);
@@ -71,11 +63,21 @@ static void test_at(bool expect_fault)
 
 	test_at_insn(S1E1R, expect_fault);
 	test_at_insn(S1E1W, expect_fault);
+
+	sysreg_clear_set(hcr_el2, 0, HCR_EL2_TGE | HCR_EL2_VM);
+	isb();
 }
 
 static void guest_code(void)
 {
-	sysreg_clear_set(tcr_el1, TCR_HA, 0);
+	/* Reuse the stage-1 MMU context from EL2 at EL1 */
+	copy_el2_to_el1(SCTLR);
+	copy_el2_to_el1(MAIR);
+	copy_el2_to_el1(TCR);
+	copy_el2_to_el1(TTBR0);
+	copy_el2_to_el1(TTBR1);
+
+	sysreg_clear_set_s(SYS_TCR_EL12, TCR_HA, 0);
 	isb();
 
 	test_at(true);
@@ -83,7 +85,11 @@ static void guest_code(void)
 	if (!SYS_FIELD_GET(ID_AA64MMFR1_EL1, HAFDBS, read_sysreg(id_aa64mmfr1_el1)))
 		GUEST_DONE();
 
-	sysreg_clear_set(tcr_el1, 0, TCR_HA);
+	/*
+	 * KVM's software PTW makes the implementation choice that the AT
+	 * instruction sets the access flag.
+	 */
+	sysreg_clear_set_s(SYS_TCR_EL12, 0, TCR_HA);
 	isb();
 	test_at(false);
 
@@ -96,8 +102,8 @@ static void handle_sync(struct kvm_vcpu *vcpu, struct ucall *uc)
 	case CLEAR_ACCESS_FLAG:
 		/*
 		 * Delete + reinstall the memslot to invalidate stage-2
-		 * mappings of the stage-1 page tables, allowing KVM to
-		 * potentially use the 'slow' AT emulation path.
+		 * mappings of the stage-1 page tables, forcing KVM to
+		 * use the 'slow' AT emulation path.
 		 *
 		 * This and clearing the access flag from host userspace
 		 * ensures that the access flag cannot be set speculatively
@@ -106,6 +112,10 @@ static void handle_sync(struct kvm_vcpu *vcpu, struct ucall *uc)
 		clear_bit(__ffs(PTE_AF), ptep_hva);
 		vm_mem_region_reload(vcpu->vm, vcpu->vm->memslots[MEM_REGION_PT]);
 		break;
+	case TEST_ACCESS_FLAG:
+		TEST_ASSERT(test_bit(__ffs(PTE_AF), ptep_hva),
+			    "Expected access flag to be set (desc: %lu)", *ptep_hva);
+		break;
 	default:
 		TEST_FAIL("Unexpected SYNC arg: %lu", uc->args[1]);
 	}
-- 
2.47.3

[PATCH v2 26/28] KVM: arm64: selftests: Test AT emulation for FEAT_HAFT

[Oliver Upton]

Test that KVM's AT emulation sets the access flag on table descriptors
when FEAT_HAFT is enabled at stage-1. Additionally, add test coverage
that asserts the access flag is clear on table/page descriptors when the
AT instruction generates an access flag fault.

Reviewed-by: Leonardo Bras <leo.bras@arm.com>
Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 tools/testing/selftests/kvm/arm64/at.c | 74 +++++++++++++++++++++++---
 1 file changed, 66 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/kvm/arm64/at.c b/tools/testing/selftests/kvm/arm64/at.c
index d7289f3df04f..3212b18bd309 100644
--- a/tools/testing/selftests/kvm/arm64/at.c
+++ b/tools/testing/selftests/kvm/arm64/at.c
@@ -13,10 +13,11 @@
 
 enum {
 	CLEAR_ACCESS_FLAG,
-	TEST_ACCESS_FLAG,
+	ASSERT_ACCESS_FLAG_SET,
+	ASSERT_ACCESS_FLAG_CLEAR,
 };
 
-static u64 *ptep_hva;
+static u64 *page_ptep, *table_ptep;
 
 #define copy_el2_to_el1(reg)						\
 	write_sysreg_s(read_sysreg_s(SYS_##reg##_EL1), SYS_##reg##_EL12)
@@ -45,11 +46,12 @@ do {											\
 		__GUEST_ASSERT(fsc == ESR_ELx_FSC_ACCESS_L(3),				\
 			       "AT "#op": expected access flag fault (par: %lx)",	\
 			       par);							\
+		GUEST_SYNC(ASSERT_ACCESS_FLAG_CLEAR);					\
 	} else {									\
 		GUEST_ASSERT_EQ(FIELD_GET(SYS_PAR_EL1_ATTR, par), MAIR_ATTR_NORMAL);	\
 		GUEST_ASSERT_EQ(FIELD_GET(SYS_PAR_EL1_SH, par), PTE_SHARED >> 8);	\
 		GUEST_ASSERT_EQ(par & SYS_PAR_EL1_PA, TEST_ADDR);			\
-		GUEST_SYNC(TEST_ACCESS_FLAG);						\
+		GUEST_SYNC(ASSERT_ACCESS_FLAG_SET);					\
 	}										\
 } while (0)
 
@@ -68,6 +70,14 @@ static void test_at(bool expect_fault)
 	isb();
 }
 
+static bool guest_has_haft(void)
+{
+	u64 mmfr1 = read_sysreg(id_aa64mmfr1_el1);
+
+	return SYS_FIELD_GET(ID_AA64MMFR1_EL1, HAFDBS, mmfr1) >=
+		ID_AA64MMFR1_EL1_HAFDBS_HAFT;
+}
+
 static void guest_code(void)
 {
 	/* Reuse the stage-1 MMU context from EL2 at EL1 */
@@ -93,9 +103,43 @@ static void guest_code(void)
 	isb();
 	test_at(false);
 
+	if (!guest_has_haft())
+		GUEST_DONE();
+
+	sysreg_clear_set_s(SYS_HCRX_EL2, 0, HCRX_EL2_TCR2En);
+	sysreg_clear_set_s(SYS_TCR2_EL12, 0, TCR2_EL1_HAFT);
+	isb();
+	test_at(false);
+
+	/* The effective value of HAFT is 0 if HA is 0 */
+	sysreg_clear_set_s(SYS_TCR_EL12, TCR_HA, 0);
+	isb();
+	test_at(true);
+
+	/* The effective value of HAFT is 0 if HCRX_EL2.TCR2En is 0 */
+	sysreg_clear_set_s(SYS_HCRX_EL2, HCRX_EL2_TCR2En, 0);
+	sysreg_clear_set_s(SYS_TCR_EL12, 0, TCR_HA);
+	isb();
+	test_at(false);
+
 	GUEST_DONE();
 }
 
+static bool vcpu_haft_enabled(struct kvm_vcpu *vcpu)
+{
+	u64 mmfr1 = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_ID_AA64MMFR1_EL1));
+	u8 hafdbs = SYS_FIELD_GET(ID_AA64MMFR1_EL1, HAFDBS, mmfr1);
+	u64 tcr2, hcrx;
+
+	/* FEAT_HAFT implies FEAT_TCRX, FEAT_HCX */
+	if (hafdbs < ID_AA64MMFR1_EL1_HAFDBS_HAFT)
+		return false;
+
+	hcrx = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_HCRX_EL2));
+	tcr2 = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_TCR2_EL1));
+	return hcrx & HCRX_EL2_TCR2En && tcr2 & TCR2_EL1_HAFT;
+}
+
 static void handle_sync(struct kvm_vcpu *vcpu, struct ucall *uc)
 {
 	switch (uc->args[1]) {
@@ -109,12 +153,25 @@ static void handle_sync(struct kvm_vcpu *vcpu, struct ucall *uc)
 		 * ensures that the access flag cannot be set speculatively
 		 * and is reliably cleared at the time of the AT instruction.
 		 */
-		clear_bit(__ffs(PTE_AF), ptep_hva);
+		clear_bit(__ffs(PTE_AF), page_ptep);
+		clear_bit(__ffs(PTE_AF), table_ptep);
 		vm_mem_region_reload(vcpu->vm, vcpu->vm->memslots[MEM_REGION_PT]);
 		break;
-	case TEST_ACCESS_FLAG:
-		TEST_ASSERT(test_bit(__ffs(PTE_AF), ptep_hva),
-			    "Expected access flag to be set (desc: %lu)", *ptep_hva);
+	case ASSERT_ACCESS_FLAG_SET:
+		TEST_ASSERT(test_bit(__ffs(PTE_AF), page_ptep),
+			    "Expected access flag to be set (desc: %lu)", *page_ptep);
+		if (!vcpu_haft_enabled(vcpu))
+			TEST_ASSERT(!test_bit(__ffs(PTE_AF), table_ptep),
+				    "Expected access flag to be clear (desc: %lu)", *table_ptep);
+		else
+			TEST_ASSERT(test_bit(__ffs(PTE_AF), table_ptep),
+				    "Expected access flag to be set (desc: %lu)", *table_ptep);
+		break;
+	case ASSERT_ACCESS_FLAG_CLEAR:
+		TEST_ASSERT(!test_bit(__ffs(PTE_AF), page_ptep),
+			    "Expected access flag to be clear (desc: %lu)", *page_ptep);
+		TEST_ASSERT(!test_bit(__ffs(PTE_AF), table_ptep),
+			    "Expected access flag to be clear (desc: %lu)", *table_ptep);
 		break;
 	default:
 		TEST_FAIL("Unexpected SYNC arg: %lu", uc->args[1]);
@@ -158,7 +215,8 @@ int main(void)
 	kvm_arch_vm_finalize_vcpus(vm);
 
 	virt_map(vm, TEST_ADDR, TEST_ADDR, 1);
-	ptep_hva = virt_get_pte_hva_at_level(vm, TEST_ADDR, 3);
+	page_ptep = virt_get_pte_hva_at_level(vm, TEST_ADDR, 3);
+	table_ptep = virt_get_pte_hva_at_level(vm, TEST_ADDR, 2);
 	run_test(vcpu);
 
 	kvm_vm_free(vm);
-- 
2.47.3

[PATCH v2 27/28] KVM: arm64: Add tracepoints for S1 page table walk

[Oliver Upton]

MMU walk steps are a useful tidbit of information for KVM hackers. As
these events are far closer to modelling CPU architecture than exposing
KVM internals it clears the bar for adding tracepoints.

Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 arch/arm64/include/asm/kvm_nested.h |  8 +++++
 arch/arm64/kvm/at.c                 | 18 +++++-----
 arch/arm64/kvm/trace_arm.h          | 52 +++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
index 81d8b1676fe4..341604b619cc 100644
--- a/arch/arm64/include/asm/kvm_nested.h
+++ b/arch/arm64/include/asm/kvm_nested.h
@@ -343,6 +343,14 @@ struct s1_walk_info {
 	bool			haft;
 };
 
+struct s1_walk_step {
+	u64			desc;
+	u64			desc_ipa;
+	u64			desc_pa;
+	struct kvm_s2_trans	s2_trans;
+	int			level;
+};
+
 struct s1_walk_result {
 	union {
 		struct {
diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c
index 8f1be3af64e8..1a49eb3c288a 100644
--- a/arch/arm64/kvm/at.c
+++ b/arch/arm64/kvm/at.c
@@ -11,6 +11,8 @@
 #include <asm/kvm_mmu.h>
 #include <asm/lsui.h>
 
+#include "trace.h"
+
 static int get_ia_size(struct s1_walk_info *wi)
 {
 	return 64 - wi->txsz;
@@ -433,14 +435,6 @@ static int setup_s1_walk(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 	return -EFAULT;
 }
 
-struct s1_walk_step {
-	u64			desc;
-	u64			desc_ipa;
-	u64			desc_pa;
-	struct kvm_s2_trans	s2_trans;
-	int			level;
-};
-
 static int kvm_read_s1_desc(struct kvm_vcpu *vcpu, u64 pa, u64 *desc,
 			    struct s1_walk_info *wi)
 {
@@ -492,10 +486,10 @@ static int handle_desc_update(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 			      struct s1_walk_step *ws, struct s1_walk_result *wr,
 			      struct kvm_walk_access *access)
 {
-	u64 old, new;
+	u64 old, old_cpu, new;
 	int ret;
 
-	old = new = ws->desc;
+	old = old_cpu = new = ws->desc;
 
 	if (should_set_access_flag(wi, ws, access))
 		new |= PTE_AF;
@@ -529,6 +523,8 @@ static int handle_desc_update(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 		return ret;
 	}
 
+	trace_kvm_s1_desc_update(ws, access->ia, old_cpu);
+
 	wr->desc = ws->desc;
 	wr->dirty = !(wr->desc & PTE_RDONLY);
 	return 0;
@@ -614,6 +610,8 @@ static int walk_s1(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 			return ret;
 		}
 
+		trace_kvm_s1_walk_step(&ws, access->ia);
+
 		/* Invalid descriptor */
 		if (!(ws.desc & BIT(0)))
 			goto transfault;
diff --git a/arch/arm64/kvm/trace_arm.h b/arch/arm64/kvm/trace_arm.h
index 9c60f6465c78..9fe9b57f6fa8 100644
--- a/arch/arm64/kvm/trace_arm.h
+++ b/arch/arm64/kvm/trace_arm.h
@@ -415,6 +415,58 @@ TRACE_EVENT(kvm_forward_sysreg_trap,
 		      sys_reg_Op2(__entry->sysreg))
 );
 
+TRACE_EVENT(kvm_s1_walk_step,
+	    TP_PROTO(struct s1_walk_step *ws, u64 ia),
+	    TP_ARGS(ws, ia),
+
+	    TP_STRUCT__entry(
+		__field(u64,	ia)
+		__field(u64,	desc)
+		__field(u64,	ipa)
+		__field(u64,	pa)
+		__field(int,	level)
+	    ),
+
+	    TP_fast_assign(
+		__entry->ia	= ia;
+		__entry->desc	= ws->desc;
+		__entry->ipa	= ws->desc_ipa;
+		__entry->pa	= ws->desc_pa;
+		__entry->level	= ws->level;
+	    ),
+
+	    TP_printk("ia: 0x%llx desc: 0x%llx ipa: 0x%llx pa: 0x%llx level: %d",
+		      __entry->ia, __entry->desc, __entry->ipa, __entry->pa,
+		      __entry->level)
+);
+
+TRACE_EVENT(kvm_s1_desc_update,
+	    TP_PROTO(struct s1_walk_step *ws, u64 ia, u64 old),
+	    TP_ARGS(ws, ia, old),
+
+	    TP_STRUCT__entry(
+		__field(u64,	ia)
+		__field(u64,	new)
+		__field(u64,	old)
+		__field(u64,	ipa)
+		__field(u64,	pa)
+		__field(int,	level)
+	    ),
+
+	    TP_fast_assign(
+		__entry->ia	= ia;
+		__entry->new	= ws->desc;
+		__entry->old	= old;
+		__entry->ipa	= ws->desc_ipa;
+		__entry->pa	= ws->desc_pa;
+		__entry->level	= ws->level;
+	    ),
+
+	    TP_printk("ia: 0x%llx new: 0x%llx old: 0x%llx ipa: 0x%llx pa: 0x%llx level: %d",
+		      __entry->ia, __entry->new, __entry->old, __entry->ipa,
+		      __entry->pa, __entry->level)
+);
+
 #endif /* _TRACE_ARM_ARM64_KVM_H */
 
 #undef TRACE_INCLUDE_PATH
-- 
2.47.3

[PATCH v2 28/28] KVM: arm64: Add tracepoints for S2 page table walk

[Oliver Upton]

MMU walk steps are a useful tidbit of information for KVM hackers. As
these events are far closer to modelling CPU architecture than exposing
KVM internals it clears the bar for adding tracepoints.

Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 arch/arm64/include/asm/kvm_nested.h |  6 ++++
 arch/arm64/kvm/nested.c             | 14 ++++-----
 arch/arm64/kvm/trace_arm.h          | 47 +++++++++++++++++++++++++++++
 3 files changed, 59 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
index 341604b619cc..65dc411188cb 100644
--- a/arch/arm64/include/asm/kvm_nested.h
+++ b/arch/arm64/include/asm/kvm_nested.h
@@ -128,6 +128,12 @@ struct kvm_walk_access {
 	bool	write;
 };
 
+struct s2_walk_step {
+	u64		desc_pa;
+	u64		desc;
+	int		level;
+};
+
 extern int kvm_walk_nested_s2(struct kvm_vcpu *vcpu, struct kvm_walk_access *access,
 			      struct kvm_s2_trans *result);
 extern int kvm_s2_handle_perm_fault(struct kvm_vcpu *vcpu,
diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index 0c468235f123..a04ab2200d36 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -17,6 +17,7 @@
 #include <asm/sysreg.h>
 
 #include "sys_regs.h"
+#include "trace.h"
 #include "vgic/vgic.h"
 
 struct vncr_tlb {
@@ -155,12 +156,6 @@ struct s2_walk_info {
 	bool		haft;
 };
 
-struct s2_walk_step {
-	u64		desc_pa;
-	u64		desc;
-	int		level;
-};
-
 static u32 compute_fsc(int level, u32 fsc)
 {
 	return fsc | (level & 0x3);
@@ -279,10 +274,10 @@ static int handle_desc_update(struct kvm_vcpu *vcpu, struct s2_walk_info *wi,
 			      struct s2_walk_step *ws, struct kvm_s2_trans *out,
 			      struct kvm_walk_access *access)
 {
-	u64 old, new;
+	u64 old, old_cpu, new;
 	int ret;
 
-	old = new = ws->desc;
+	old = old_cpu = new = ws->desc;
 
 	if (should_set_access_flag(wi, ws, access))
 		new |= KVM_PTE_LEAF_ATTR_LO_S2_AF;
@@ -307,6 +302,7 @@ static int handle_desc_update(struct kvm_vcpu *vcpu, struct s2_walk_info *wi,
 	if (ret == -EAGAIN)
 		return ret;
 	if (!ret) {
+		trace_kvm_s2_desc_update(ws, access->ia, old_cpu);
 		out->desc = ws->desc;
 		out->dirty = out->desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
 		return 0;
@@ -425,6 +421,8 @@ static int walk_nested_s2_pgd(struct kvm_vcpu *vcpu, struct kvm_walk_access *acc
 			return ret;
 		}
 
+		trace_kvm_s2_walk_step(&ws, access->ia);
+
 		/* Check for valid descriptor at this point */
 		if (!(ws.desc & KVM_PTE_VALID)) {
 			out->esr = compute_fsc(ws.level, ESR_ELx_FSC_FAULT);
diff --git a/arch/arm64/kvm/trace_arm.h b/arch/arm64/kvm/trace_arm.h
index 9fe9b57f6fa8..0bf1a0be2636 100644
--- a/arch/arm64/kvm/trace_arm.h
+++ b/arch/arm64/kvm/trace_arm.h
@@ -467,6 +467,53 @@ TRACE_EVENT(kvm_s1_desc_update,
 		      __entry->pa, __entry->level)
 );
 
+TRACE_EVENT(kvm_s2_walk_step,
+	    TP_PROTO(struct s2_walk_step *ws, u64 ia),
+	    TP_ARGS(ws, ia),
+
+	    TP_STRUCT__entry(
+		__field(u64,	ia)
+		__field(u64,	desc)
+		__field(u64,	pa)
+		__field(int,	level)
+	    ),
+
+	    TP_fast_assign(
+		__entry->ia	= ia;
+		__entry->desc	= ws->desc;
+		__entry->pa	= ws->desc_pa;
+		__entry->level	= ws->level;
+	    ),
+
+	    TP_printk("ia: 0x%llx desc: 0x%llx pa: 0x%llx level: %d",
+		      __entry->ia, __entry->desc, __entry->pa, __entry->level)
+);
+
+TRACE_EVENT(kvm_s2_desc_update,
+	    TP_PROTO(struct s2_walk_step *ws, u64 ia, u64 old),
+	    TP_ARGS(ws, ia, old),
+
+	    TP_STRUCT__entry(
+		__field(u64,	ia)
+		__field(u64,	new)
+		__field(u64,	old)
+		__field(u64,	pa)
+		__field(int,	level)
+	    ),
+
+	    TP_fast_assign(
+		__entry->ia	= ia;
+		__entry->new	= ws->desc;
+		__entry->old	= old;
+		__entry->pa	= ws->desc_pa;
+		__entry->level	= ws->level;
+	    ),
+
+	    TP_printk("ia: 0x%llx new: 0x%llx old: 0x%llx pa: 0x%llx level: %d",
+		      __entry->ia, __entry->new, __entry->old, __entry->pa,
+		      __entry->level)
+);
+
 #endif /* _TRACE_ARM_ARM64_KVM_H */
 
 #undef TRACE_INCLUDE_PATH
-- 
2.47.3