diff mbox series

[v1,10/12] KVM: arm64: Rework logic to en/decode VTCR_EL2.{SL0, SL2} fields

Message ID 20221206135930.3277585-11-ryan.roberts@arm.com (mailing list archive)
State New, archived
Headers show
Series KVM: arm64: Support FEAT_LPA2 at hyp s1 and vm s2 | expand

Commit Message

Ryan Roberts Dec. 6, 2022, 1:59 p.m. UTC
In order to support 5 level translation, FEAT_LPA2 introduces the 1-bit
SL2 field within VTCR_EL2 to extend the existing 2-bit SL0 field. The
SL2[0]:SL0[1:0] encodings have no simple algorithmic relationship to the
start levels they represent (that I can find, at least), so replace the
existing macros with functions that do lookups to encode and decode the
values. These new functions no longer make hardcoded assumptions about
the maximum level and instead rely on KVM_PGTABLE_FIRST_LEVEL and
KVM_PGTABLE_LAST_LEVEL.

This is preparatory work for enabling 52-bit IPA for 4KB and 16KB pages
with FEAT_LPA2.

No functional change intended.

Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
---
 arch/arm64/include/asm/kvm_arm.h        | 75 ++++++++++++++-----------
 arch/arm64/include/asm/kvm_pgtable.h    | 33 +++++++++++
 arch/arm64/include/asm/stage2_pgtable.h | 13 ++++-
 arch/arm64/kvm/hyp/pgtable.c            | 67 +++++++++++++++++++++-
 4 files changed, 150 insertions(+), 38 deletions(-)

Comments

Oliver Upton Dec. 20, 2022, 12:06 a.m. UTC | #1
Hi Ryan,

On Tue, Dec 06, 2022 at 01:59:28PM +0000, Ryan Roberts wrote:
> In order to support 5 level translation, FEAT_LPA2 introduces the 1-bit
> SL2 field within VTCR_EL2 to extend the existing 2-bit SL0 field. The
> SL2[0]:SL0[1:0] encodings have no simple algorithmic relationship to the
> start levels they represent (that I can find, at least), so replace the
> existing macros with functions that do lookups to encode and decode the
> values. These new functions no longer make hardcoded assumptions about
> the maximum level and instead rely on KVM_PGTABLE_FIRST_LEVEL and
> KVM_PGTABLE_LAST_LEVEL.
> 
> This is preparatory work for enabling 52-bit IPA for 4KB and 16KB pages
> with FEAT_LPA2.
> 
> No functional change intended.
> 
> Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>

Why do we need to support 5-level paging at stage-2?

A configuration of start_level = 0, T0SZ = 12 with 4K paging would
result in 16 concatenated tables at level 0, avoiding the level -1
lookup altogether.

--
Thanks,
Oliver
Ryan Roberts Dec. 20, 2022, 9:01 a.m. UTC | #2
On 20/12/2022 00:06, Oliver Upton wrote:
> Hi Ryan,
> 
> On Tue, Dec 06, 2022 at 01:59:28PM +0000, Ryan Roberts wrote:
>> In order to support 5 level translation, FEAT_LPA2 introduces the 1-bit
>> SL2 field within VTCR_EL2 to extend the existing 2-bit SL0 field. The
>> SL2[0]:SL0[1:0] encodings have no simple algorithmic relationship to the
>> start levels they represent (that I can find, at least), so replace the
>> existing macros with functions that do lookups to encode and decode the
>> values. These new functions no longer make hardcoded assumptions about
>> the maximum level and instead rely on KVM_PGTABLE_FIRST_LEVEL and
>> KVM_PGTABLE_LAST_LEVEL.
>>
>> This is preparatory work for enabling 52-bit IPA for 4KB and 16KB pages
>> with FEAT_LPA2.
>>
>> No functional change intended.
>>
>> Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
> 
> Why do we need to support 5-level paging at stage-2?
> 
> A configuration of start_level = 0, T0SZ = 12 with 4K paging would
> result in 16 concatenated tables at level 0, avoiding the level -1
> lookup altogether.

Yes, agreed. And that's exactly what the code does. So we could remove this
patch from the series and everything would continue to function correctly. But I
was trying to make things more consistent and maintainable (this now works in
terms of KVM_PGTABLE_FIRST_LEVEL and KVM_PGTABLE_LAST_LEVEL for example).

That said, I haven't exactly been consistent in my refactoring; patch 11 just
adds a comment to kvm_vcpu_trap_get_fault_level() explaining that the new -1
level encodings will never be seen due to stage2 never using 5 levels of
translation.

So happy to remove this and replace with a comment describing the limitations if
that's your preference?

> 
> --
> Thanks,
> Oliver
Oliver Upton Dec. 20, 2022, 6:08 p.m. UTC | #3
On Tue, Dec 20, 2022 at 09:01:19AM +0000, Ryan Roberts wrote:
> On 20/12/2022 00:06, Oliver Upton wrote:
> > Hi Ryan,
> > 
> > On Tue, Dec 06, 2022 at 01:59:28PM +0000, Ryan Roberts wrote:
> >> In order to support 5 level translation, FEAT_LPA2 introduces the 1-bit
> >> SL2 field within VTCR_EL2 to extend the existing 2-bit SL0 field. The
> >> SL2[0]:SL0[1:0] encodings have no simple algorithmic relationship to the
> >> start levels they represent (that I can find, at least), so replace the
> >> existing macros with functions that do lookups to encode and decode the
> >> values. These new functions no longer make hardcoded assumptions about
> >> the maximum level and instead rely on KVM_PGTABLE_FIRST_LEVEL and
> >> KVM_PGTABLE_LAST_LEVEL.
> >>
> >> This is preparatory work for enabling 52-bit IPA for 4KB and 16KB pages
> >> with FEAT_LPA2.
> >>
> >> No functional change intended.
> >>
> >> Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
> > 
> > Why do we need to support 5-level paging at stage-2?
> > 
> > A configuration of start_level = 0, T0SZ = 12 with 4K paging would
> > result in 16 concatenated tables at level 0, avoiding the level -1
> > lookup altogether.
> 
> Yes, agreed. And that's exactly what the code does. So we could remove this
> patch from the series and everything would continue to function correctly. But I
> was trying to make things more consistent and maintainable (this now works in
> terms of KVM_PGTABLE_FIRST_LEVEL and KVM_PGTABLE_LAST_LEVEL for example).

My largest concern was the plumbing that was added for setting a start
level of -1 that is effectively dead code. I worry about it because it
can be confusing for newcomers and can be an open invitation to mess
things up later down the line.

> So happy to remove this and replace with a comment describing the limitations if
> that's your preference?

Marc, feel free to put me in line here if I'm not thinking about this
right, but adding support for an unused feature is likely less
maintainable. So, I'd prefer we drop the patch.

--
Thanks,
Oliver
diff mbox series

Patch

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index f9619a10d5d9..94bbb05e348f 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -150,58 +150,65 @@ 
 				 VTCR_EL2_IRGN0_WBWA | VTCR_EL2_RES1)
 
 /*
- * VTCR_EL2:SL0 indicates the entry level for Stage2 translation.
- * Interestingly, it depends on the page size.
- * See D.10.2.121, VTCR_EL2, in ARM DDI 0487C.a
+ * VTCR_EL2.{SL0, SL2} indicates the entry level for Stage2 translation.
+ * Interestingly, it depends on the page size. See D17.2.157, VTCR_EL2, in ARM
+ * DDI 0487I.a
  *
- *	-----------------------------------------
- *	| Entry level		|  4K  | 16K/64K |
- *	------------------------------------------
- *	| Level: 0		|  2   |   -     |
- *	------------------------------------------
- *	| Level: 1		|  1   |   2     |
- *	------------------------------------------
- *	| Level: 2		|  0   |   1     |
- *	------------------------------------------
- *	| Level: 3		|  -   |   0     |
- *	------------------------------------------
+ *      ----------------------------------------------------------
+ *      | Entry level           |    4K    |    16K   |    64K   |
+ *      |                       |  SL2:SL0 |  SL2:SL0 |  SL2:SL0 |
+ *      ----------------------------------------------------------
+ *      | Level: -1             |  0b100   |     -    |     -    |
+ *      ----------------------------------------------------------
+ *      | Level: 0              |  0b010   |  0b011   |     -    |
+ *      ----------------------------------------------------------
+ *      | Level: 1              |  0b001   |  0b010   |  0b010   |
+ *      ----------------------------------------------------------
+ *      | Level: 2              |  0b000   |  0b001   |  0b001   |
+ *      ----------------------------------------------------------
+ *      | Level: 3              |  0b011   |  0b000   |  0b000   |
+ *      ----------------------------------------------------------
  *
- * The table roughly translates to :
- *
- *	SL0(PAGE_SIZE, Entry_level) = TGRAN_SL0_BASE - Entry_Level
- *
- * Where TGRAN_SL0_BASE is a magic number depending on the page size:
- * 	TGRAN_SL0_BASE(4K) = 2
- *	TGRAN_SL0_BASE(16K) = 3
- *	TGRAN_SL0_BASE(64K) = 3
- * provided we take care of ruling out the unsupported cases and
- * Entry_Level = 4 - Number_of_levels.
+ * There is no concise algorithm to convert between the SLx encodings and the
+ * level numbers, so we implement 2 helpers kvm_vtcr_el2_sl_encode()
+ * kvm_vtcr_el2_sl_decode() which can convert between the representations. These
+ * helpers use a concatenated form of SLx: SL2[0]:SL0[1:0] as the 3 LSBs in u8.
+ * If an invalid input value is provided, VTCR_EL2_SLx_ENC_INVAL is returned. We
+ * declare the appropriate encoded values here for the compiled in page size.
  *
+ * See kvm_pgtable.h for documentation on the helpers.
  */
+#define VTCR_EL2_SLx_ENC_INVAL		255
+
 #ifdef CONFIG_ARM64_64K_PAGES
 
 #define VTCR_EL2_TGRAN			VTCR_EL2_TG0_64K
-#define VTCR_EL2_TGRAN_SL0_BASE		3UL
+#define VTCR_EL2_SLx_ENC_Lm1		VTCR_EL2_SLx_ENC_INVAL
+#define VTCR_EL2_SLx_ENC_L0		VTCR_EL2_SLx_ENC_INVAL
+#define VTCR_EL2_SLx_ENC_Lp1		2
+#define VTCR_EL2_SLx_ENC_Lp2		1
+#define VTCR_EL2_SLx_ENC_Lp3		0
 
 #elif defined(CONFIG_ARM64_16K_PAGES)
 
 #define VTCR_EL2_TGRAN			VTCR_EL2_TG0_16K
-#define VTCR_EL2_TGRAN_SL0_BASE		3UL
+#define VTCR_EL2_SLx_ENC_Lm1		VTCR_EL2_SLx_ENC_INVAL
+#define VTCR_EL2_SLx_ENC_L0		3
+#define VTCR_EL2_SLx_ENC_Lp1		2
+#define VTCR_EL2_SLx_ENC_Lp2		1
+#define VTCR_EL2_SLx_ENC_Lp3		0
 
 #else	/* 4K */
 
 #define VTCR_EL2_TGRAN			VTCR_EL2_TG0_4K
-#define VTCR_EL2_TGRAN_SL0_BASE		2UL
+#define VTCR_EL2_SLx_ENC_Lm1		4
+#define VTCR_EL2_SLx_ENC_L0		2
+#define VTCR_EL2_SLx_ENC_Lp1		1
+#define VTCR_EL2_SLx_ENC_Lp2		0
+#define VTCR_EL2_SLx_ENC_Lp3		3
 
 #endif
 
-#define VTCR_EL2_LVLS_TO_SL0(levels)	\
-	((VTCR_EL2_TGRAN_SL0_BASE - (4 - (levels))) << VTCR_EL2_SL0_SHIFT)
-#define VTCR_EL2_SL0_TO_LVLS(sl0)	\
-	((sl0) + 4 - VTCR_EL2_TGRAN_SL0_BASE)
-#define VTCR_EL2_LVLS(vtcr)		\
-	VTCR_EL2_SL0_TO_LVLS(((vtcr) & VTCR_EL2_SL0_MASK) >> VTCR_EL2_SL0_SHIFT)
-
 #define VTCR_EL2_FLAGS			(VTCR_EL2_COMMON_BITS | VTCR_EL2_TGRAN)
 #define VTCR_EL2_IPA(vtcr)		(64 - ((vtcr) & VTCR_EL2_T0SZ_MASK))
 
diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
index a282a3d5ddbc..3e0b64052c51 100644
--- a/arch/arm64/include/asm/kvm_pgtable.h
+++ b/arch/arm64/include/asm/kvm_pgtable.h
@@ -328,6 +328,39 @@  int kvm_pgtable_hyp_map(struct kvm_pgtable *pgt, u64 addr, u64 size, u64 phys,
  */
 u64 kvm_pgtable_hyp_unmap(struct kvm_pgtable *pgt, u64 addr, u64 size);
 
+/**
+ * kvm_vtcr_el2_sl_encode() - Helper to encode start level for vtcr_el2.
+ * @sl_dec:     Start level to be encoded.
+ *
+ * Takes an unencoded translation start level value and returns it encoded for
+ * use in vtcr_el2 register. The returned value has SL0 (a 2 bit field) in bits
+ * [1:0] and SL2 (a 1 bit field) in bit [2]. The user is responsible for
+ * extracting and packing in the correct locations of vctr_el2.
+ *
+ * Do not call this function with a value that is out of range for the page size
+ * in operation. A warning will be output if this is detected and the function
+ * returns VTCR_EL2_SLx_ENC_INVAL. See comment in kvm_arm.h for more info.
+ *
+ * Return: 3 bit value containing SL2[0]:SL0[1:0], or VTCR_EL2_SLx_ENC_INVAL.
+ */
+u8 kvm_vtcr_el2_sl_encode(s8 sl_dec);
+
+/**
+ * kvm_vtcr_el2_sl_decode() - Helper to decode start level for vtcr_el2.
+ * @sl_enc:     Start level encoded as SL2[0]:SL0[1:0].
+ *
+ * Takes an encoded translation start level value, as used in the vtcr_el2
+ * register and returns it decoded. See kvm_vtcr_el2_sl_encode() for description
+ * of input encoding.
+ *
+ * Do not call this function with a value that is invalid for the page size in
+ * operation. A warning will be output if this is detected and the function
+ * returns VTCR_EL2_SLx_ENC_INVAL. See comment in kvm_arm.h for more info.
+ *
+ * Return: Decoded start level, or VTCR_EL2_SLx_ENC_INVAL.
+ */
+s8 kvm_vtcr_el2_sl_decode(u8 sl_enc);
+
 /**
  * kvm_get_vtcr() - Helper to construct VTCR_EL2
  * @mmfr0:	Sanitized value of SYS_ID_AA64MMFR0_EL1 register.
diff --git a/arch/arm64/include/asm/stage2_pgtable.h b/arch/arm64/include/asm/stage2_pgtable.h
index c8dca8ae359c..02c5e04d4958 100644
--- a/arch/arm64/include/asm/stage2_pgtable.h
+++ b/arch/arm64/include/asm/stage2_pgtable.h
@@ -21,7 +21,18 @@ 
  * (IPA_SHIFT - 4).
  */
 #define stage2_pgtable_levels(ipa)	ARM64_HW_PGTABLE_LEVELS((ipa) - 4)
-#define kvm_stage2_levels(kvm)		VTCR_EL2_LVLS(kvm->arch.vtcr)
+static inline s8 kvm_stage2_levels(struct kvm *kvm)
+{
+	u64 vtcr = kvm->arch.vtcr;
+	u8 slx;
+	s8 start_level;
+
+	slx = FIELD_GET(VTCR_EL2_SL0_MASK, vtcr);
+	slx |= FIELD_GET(VTCR_EL2_SL2_MASK, vtcr) << 2;
+	start_level = kvm_vtcr_el2_sl_decode(slx);
+
+	return KVM_PGTABLE_LAST_LEVEL + 1 - start_level;
+}
 
 /*
  * kvm_mmmu_cache_min_pages() is the number of pages required to install
diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index 274f839bd0d7..8ebd9aaed2c4 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -595,12 +595,67 @@  struct stage2_map_data {
 	bool				force_pte;
 };
 
+u8 kvm_vtcr_el2_sl_encode(s8 sl_dec)
+{
+	u8 sl_enc = VTCR_EL2_SLx_ENC_INVAL;
+
+	BUILD_BUG_ON(KVM_PGTABLE_FIRST_LEVEL < -1);
+	BUILD_BUG_ON(KVM_PGTABLE_LAST_LEVEL > 3);
+
+	switch (sl_dec) {
+	case -1:
+		sl_enc = VTCR_EL2_SLx_ENC_Lm1;
+		break;
+	case 0:
+		sl_enc = VTCR_EL2_SLx_ENC_L0;
+		break;
+	case 1:
+		sl_enc = VTCR_EL2_SLx_ENC_Lp1;
+		break;
+	case 2:
+		sl_enc = VTCR_EL2_SLx_ENC_Lp2;
+		break;
+	case 3:
+		sl_enc = VTCR_EL2_SLx_ENC_Lp3;
+		break;
+	}
+
+	WARN_ON_ONCE(sl_enc == VTCR_EL2_SLx_ENC_INVAL);
+	return sl_enc;
+}
+
+s8 kvm_vtcr_el2_sl_decode(u8 sl_enc)
+{
+	s8 sl_dec = VTCR_EL2_SLx_ENC_INVAL;
+
+	BUILD_BUG_ON(KVM_PGTABLE_FIRST_LEVEL < -1);
+	BUILD_BUG_ON(KVM_PGTABLE_LAST_LEVEL > 3);
+
+	if (sl_enc == VTCR_EL2_SLx_ENC_Lm1)
+		sl_dec = -1;
+	else if (sl_enc == VTCR_EL2_SLx_ENC_L0)
+		sl_dec = 0;
+	else if (sl_enc == VTCR_EL2_SLx_ENC_Lp1)
+		sl_dec = 1;
+	else if (sl_enc == VTCR_EL2_SLx_ENC_Lp2)
+		sl_dec = 2;
+	else if (sl_enc == VTCR_EL2_SLx_ENC_Lp3)
+		sl_dec = 3;
+
+	if (WARN_ON_ONCE(sl_dec == VTCR_EL2_SLx_ENC_INVAL ||
+			 sl_enc == VTCR_EL2_SLx_ENC_INVAL))
+		sl_dec = VTCR_EL2_SLx_ENC_INVAL;
+
+	return sl_dec;
+}
+
 u64 kvm_get_vtcr(u64 mmfr0, u64 mmfr1, u32 phys_shift)
 {
 	u64 vtcr = VTCR_EL2_FLAGS;
 	s8 levels;
 	u64 parange;
 	bool lpa2_ena = false;
+	u8 slx;
 
 	/*
 	 * If stage 2 reports that it supports FEAT_LPA2 for our page size, then
@@ -625,7 +680,9 @@  u64 kvm_get_vtcr(u64 mmfr0, u64 mmfr1, u32 phys_shift)
 	levels = stage2_pgtable_levels(phys_shift);
 	if (levels < 2)
 		levels = 2;
-	vtcr |= VTCR_EL2_LVLS_TO_SL0(levels);
+	slx = kvm_vtcr_el2_sl_encode(KVM_PGTABLE_LAST_LEVEL + 1 - levels);
+	vtcr |= FIELD_PREP(VTCR_EL2_SL0_MASK, slx);
+	vtcr |= FIELD_PREP(VTCR_EL2_SL2_MASK, slx >> 2);
 
 	/*
 	 * Enable the Hardware Access Flag management, unconditionally
@@ -1215,10 +1272,14 @@  int __kvm_pgtable_stage2_init(struct kvm_pgtable *pgt, struct kvm_s2_mmu *mmu,
 	size_t pgd_sz;
 	u64 vtcr = mmu->arch->vtcr;
 	u32 ia_bits = VTCR_EL2_IPA(vtcr);
-	u32 sl0 = FIELD_GET(VTCR_EL2_SL0_MASK, vtcr);
-	s8 start_level = VTCR_EL2_TGRAN_SL0_BASE - sl0;
+	u8 slx;
+	s8 start_level;
 	bool lpa2_ena = (vtcr & VTCR_EL2_DS) != 0;
 
+	slx = FIELD_GET(VTCR_EL2_SL0_MASK, vtcr);
+	slx |= FIELD_GET(VTCR_EL2_SL2_MASK, vtcr) << 2;
+	start_level = kvm_vtcr_el2_sl_decode(slx);
+
 	pgd_sz = kvm_pgd_pages(ia_bits, start_level) * PAGE_SIZE;
 	pgt->pgd = mm_ops->zalloc_pages_exact(pgd_sz);
 	if (!pgt->pgd)