[PATCH v2] KVM: arm64: Enable S1PIE for hVHE

[PATCH v2] KVM: arm64: Enable S1PIE for hVHE

[Mark Brown]

When FEAT_S1PIE (stage 1 permission indirection) is supported we
currently enable and use it in the hypervisor when running in VHE mode
but not when running in nVHE or hVHE mode.  While systems with
FEAT_S1PIE would normally use VHE users can configure them for nVHE or
hVHE.  Enable FEAT_S1PIE with hVHE only, hVHE is used for protected VMs
but there is no real use case for nVHE mode on hardware with this
feature.

AP[1] is one of the bits used to encode the indirected permissions.
Since for hVHE this is always 0 we only configure the subset of
indirected permissions that the system is expected to use.

With permission indirection read and write permissions must be encoded
in the bits used by S1PIE, set DBM for writable mappings.  Only do this
when using S1PIE, the hypervisor does not otherwise use DBM so no
existing hypervisor code sets that bit.  Since the meaning is assigned
via S1PIE this does not actually enable DBM, the mappings we configure
just grant write permission.

In order to enable S1PIE we also need to configure TCR2_EL2 which is
currently only done in __finalise_el2 which is VHE only, do so when the
register is present.  When running in nVHE we leave TCR2_EL2.PIE
disabled.  This ensures we have an explicit configuration for TCR2_EL2
when it is present in the system.

For simplicity we unconditionally initialise PIR_EL2 and PIRE0_EL2 if
FEAT_S1PIE is present, this will have no effect in nVHE mode since we
set TCR2_EL2.PIE to 0.

This should have no practical impact other than causing any unexpected
encodings to map to no permissions instead of their default meanings.
It will mean that the configuration is closer to that in VHE mode, and
will be required for future work enabling features like D128 and GCS
which are only available via indirection.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
Changes in v2:
- Rebase onto v7.3-rc2.
- Don't bother cleaning up TCR2_EL2 on hypervisor exit, and squash the
  handling into the S1PIE patch.
- Remove support for nVHE mode.
- Link to v1: https://patch.msgid.link/20260904-kvm-arm64-nvhe-pie-v1-0-29d59f245e6c@kernel.org
---
 arch/arm64/include/asm/kvm_arm.h     | 19 +++++++++++++++++++
 arch/arm64/include/asm/kvm_asm.h     |  1 +
 arch/arm64/include/asm/kvm_pgtable.h |  1 +
 arch/arm64/kernel/asm-offsets.c      |  1 +
 arch/arm64/kvm/arm.c                 |  8 +++++++-
 arch/arm64/kvm/hyp/nvhe/hyp-init.S   | 16 ++++++++++++++--
 arch/arm64/kvm/hyp/pgtable.c         |  8 ++++++++
 7 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index 4bfbd827c5aa..eecac91d41e4 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -345,4 +345,23 @@
 #define VCPU_RESET_PSTATE_SVC	(PSR_AA32_MODE_SVC | PSR_AA32_A_BIT | \
 				 PSR_AA32_I_BIT | PSR_AA32_F_BIT)
 
+/*
+ * Permission indirection configuration for the hVHE hypervisor when
+ * we have FEAT_S1PIE. Like the host kernel we configure a mapping
+ * mostly equivalent to the non-PIE meanings of the bits so the
+ * page table manipulation code needs minimal updates for PIE.
+ *
+ * These mappings are minimal with only things used from the
+ * hVHE hypervisor, nVHE is not supported.  Write permission is
+ * controlled via DBM.
+ */
+
+#define KVM_HYP_PIR_IDX(uxn, pxn, dbm, ap1) (((uxn) << 3) | ((pxn) << 2) | \
+					     ((dbm) << 1) | (ap1))
+
+#define KVM_HVHE_PIR_EL2 (						\
+	PIRx_ELx_PERM_PREP(KVM_HYP_PIR_IDX(0, 0, 0, 0), PIE_RX)	|	\
+	PIRx_ELx_PERM_PREP(KVM_HYP_PIR_IDX(1, 1, 0, 0), PIE_R)	|	\
+	PIRx_ELx_PERM_PREP(KVM_HYP_PIR_IDX(1, 1, 1, 0), PIE_RW))
+
 #endif /* __ARM64_KVM_ARM_H__ */
diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
index e5b92ac09e69..eb796436d6eb 100644
--- a/arch/arm64/include/asm/kvm_asm.h
+++ b/arch/arm64/include/asm/kvm_asm.h
@@ -208,6 +208,7 @@ extern void *__vhe_undefined_symbol;
 struct kvm_nvhe_init_params {
 	unsigned long mair_el2;
 	unsigned long tcr_el2;
+	unsigned long tcr2_el2;
 	unsigned long tpidr_el2;
 	unsigned long stack_hyp_va;
 	unsigned long stack_pa;
diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
index 41a8687938eb..7b1b7ab3e88e 100644
--- a/arch/arm64/include/asm/kvm_pgtable.h
+++ b/arch/arm64/include/asm/kvm_pgtable.h
@@ -93,6 +93,7 @@ typedef u64 kvm_pte_t;
 
 #define KVM_PTE_LEAF_ATTR_HI_S2_XN	GENMASK(54, 53)
 
+#define KVM_PTE_LEAF_ATTR_HI_S1_DBM	BIT(51)
 #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 | \
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index 9c853ed3ceab..baffe58015d6 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -118,6 +118,7 @@ int main(void)
   DEFINE(HOST_DATA_CONTEXT,	offsetof(struct kvm_host_data, host_ctxt));
   DEFINE(NVHE_INIT_MAIR_EL2,	offsetof(struct kvm_nvhe_init_params, mair_el2));
   DEFINE(NVHE_INIT_TCR_EL2,	offsetof(struct kvm_nvhe_init_params, tcr_el2));
+  DEFINE(NVHE_INIT_TCR2_EL2,	offsetof(struct kvm_nvhe_init_params, tcr2_el2));
   DEFINE(NVHE_INIT_TPIDR_EL2,	offsetof(struct kvm_nvhe_init_params, tpidr_el2));
   DEFINE(NVHE_INIT_STACK_HYP_VA,	offsetof(struct kvm_nvhe_init_params, stack_hyp_va));
   DEFINE(NVHE_INIT_PGD_PA,	offsetof(struct kvm_nvhe_init_params, pgd_pa));
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 8b080804bc90..607a6f808b1c 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -2158,7 +2158,7 @@ static int kvm_init_vector_slots(void)
 static void __init cpu_prepare_hyp_mode(int cpu, u32 hyp_va_bits)
 {
 	struct kvm_nvhe_init_params *params = per_cpu_ptr_nvhe_sym(kvm_init_params, cpu);
-	unsigned long tcr;
+	unsigned long tcr, tcr2;
 
 	/*
 	 * Calculate the raw per-cpu offset without a translation from the
@@ -2186,6 +2186,12 @@ static void __init cpu_prepare_hyp_mode(int cpu, u32 hyp_va_bits)
 	tcr |= TCR_T0SZ(hyp_va_bits);
 	params->tcr_el2 = tcr;
 
+	tcr2 = 0;
+	if (cpus_have_final_cap(ARM64_HAS_S1PIE) &&
+	    cpus_have_final_cap(ARM64_KVM_HVHE))
+		tcr2 |= TCR2_EL2_PIE;
+	params->tcr2_el2 = tcr2;
+
 	params->pgd_pa = kvm_mmu_get_httbr();
 	if (is_protected_kvm_enabled())
 		params->hcr_el2 = HCR_HOST_NVHE_PROTECTED_FLAGS;
diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-init.S b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
index 0b3e0b28dfc7..cd5b75c8776f 100644
--- a/arch/arm64/kvm/hyp/nvhe/hyp-init.S
+++ b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
@@ -137,8 +137,20 @@ alternative_if ARM64_HAS_CNP
 alternative_else_nop_endif
 	msr	ttbr0_el2, x2
 
-	ldr	x0, [x0, #NVHE_INIT_TCR_EL2]
-	msr	tcr_el2, x0
+	ldr	x1, [x0, #NVHE_INIT_TCR_EL2]
+	msr	tcr_el2, x1
+
+alternative_if ARM64_HAS_S1PIE
+	/* S1PIE is only enabled with TCR2_EL2.PIE if we are running hVHE */
+	mov_q	x1, KVM_HVHE_PIR_EL2
+	msr	REG_PIR_EL2, x1
+	msr	REG_PIRE0_EL2, xzr
+alternative_else_nop_endif
+
+alternative_if ARM64_HAS_TCR2
+	ldr	x1, [x0, #NVHE_INIT_TCR2_EL2]
+	msr	REG_TCR2_EL2, x1
+alternative_else_nop_endif
 
 	isb
 
diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index b74dd5ce1efd..5276c2874fe0 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -349,6 +349,14 @@ static int hyp_set_prot_attr(enum kvm_pgtable_prot prot, kvm_pte_t *ptep)
 
 		if (system_supports_bti_kernel())
 			attr |= KVM_PTE_LEAF_ATTR_HI_S1_GP;
+	} else if (cpus_have_final_cap(ARM64_HAS_S1PIE) &&
+		   cpus_have_final_cap(ARM64_KVM_HVHE) &&
+		   (prot & KVM_PGTABLE_PROT_W)) {
+		/*
+		 * When using S1PIE for hVHE set DBM for writable
+		 * mappings since AP[2] is ineffective.
+		 */
+		attr |= KVM_PTE_LEAF_ATTR_HI_S1_DBM;
 	}
 
 	if (cpus_have_final_cap(ARM64_KVM_HVHE)) {

---
base-commit: df2908090cda368b01ff43709f51890076c56157
change-id: 20260828-kvm-arm64-nvhe-pie-cb86c417f9ea

Best regards,
--  
Mark Brown <broonie@kernel.org>

Re: [PATCH v2] KVM: arm64: Enable S1PIE for hVHE

[Marc Zyngier]

On Tue, 08 Sep 2026 20:40:47 +0100,
Mark Brown <broonie@kernel.org> wrote:
> 
> When FEAT_S1PIE (stage 1 permission indirection) is supported we
> currently enable and use it in the hypervisor when running in VHE mode
> but not when running in nVHE or hVHE mode.  While systems with
> FEAT_S1PIE would normally use VHE users can configure them for nVHE or

Missing comma after VHE.

> hVHE.  Enable FEAT_S1PIE with hVHE only, hVHE is used for protected VMs
> but there is no real use case for nVHE mode on hardware with this
> feature.

That's not the reason. The reason is that there is no nVHE-only
hardware with PIE, and that on VHE-capable HW, nVHE and hVHE are
strictly equivalent. Therefore there is no need to add support for HW
that does not exist.

> 
> AP[1] is one of the bits used to encode the indirected permissions.
> Since for hVHE this is always 0 we only configure the subset of
> indirected permissions that the system is expected to use.
> 
> With permission indirection read and write permissions must be encoded
> in the bits used by S1PIE, set DBM for writable mappings.  Only do this
> when using S1PIE, the hypervisor does not otherwise use DBM so no
> existing hypervisor code sets that bit.  Since the meaning is assigned
> via S1PIE this does not actually enable DBM, the mappings we configure
> just grant write permission.
> 
> In order to enable S1PIE we also need to configure TCR2_EL2 which is
> currently only done in __finalise_el2 which is VHE only, do so when the
> register is present.  When running in nVHE we leave TCR2_EL2.PIE
> disabled.  This ensures we have an explicit configuration for TCR2_EL2
> when it is present in the system.
>
> For simplicity we unconditionally initialise PIR_EL2 and PIRE0_EL2 if
> FEAT_S1PIE is present, this will have no effect in nVHE mode since we
> set TCR2_EL2.PIE to 0.
> 
> This should have no practical impact other than causing any unexpected
> encodings to map to no permissions instead of their default
> meanings.

What default meanings?

> It will mean that the configuration is closer to that in VHE mode, and
> will be required for future work enabling features like D128 and GCS
> which are only available via indirection.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
> Changes in v2:
> - Rebase onto v7.3-rc2.
> - Don't bother cleaning up TCR2_EL2 on hypervisor exit, and squash the
>   handling into the S1PIE patch.
> - Remove support for nVHE mode.
> - Link to v1: https://patch.msgid.link/20260904-kvm-arm64-nvhe-pie-v1-0-29d59f245e6c@kernel.org
> ---
>  arch/arm64/include/asm/kvm_arm.h     | 19 +++++++++++++++++++
>  arch/arm64/include/asm/kvm_asm.h     |  1 +
>  arch/arm64/include/asm/kvm_pgtable.h |  1 +
>  arch/arm64/kernel/asm-offsets.c      |  1 +
>  arch/arm64/kvm/arm.c                 |  8 +++++++-
>  arch/arm64/kvm/hyp/nvhe/hyp-init.S   | 16 ++++++++++++++--
>  arch/arm64/kvm/hyp/pgtable.c         |  8 ++++++++
>  7 files changed, 51 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
> index 4bfbd827c5aa..eecac91d41e4 100644
> --- a/arch/arm64/include/asm/kvm_arm.h
> +++ b/arch/arm64/include/asm/kvm_arm.h
> @@ -345,4 +345,23 @@
>  #define VCPU_RESET_PSTATE_SVC	(PSR_AA32_MODE_SVC | PSR_AA32_A_BIT | \
>  				 PSR_AA32_I_BIT | PSR_AA32_F_BIT)
>  
> +/*
> + * Permission indirection configuration for the hVHE hypervisor when
> + * we have FEAT_S1PIE. Like the host kernel we configure a mapping
> + * mostly equivalent to the non-PIE meanings of the bits so the
> + * page table manipulation code needs minimal updates for PIE.
> + *
> + * These mappings are minimal with only things used from the
> + * hVHE hypervisor, nVHE is not supported.  Write permission is
> + * controlled via DBM.

They are controlled by PIIndex[1]. Yes, this is the same bit. No, this
doesn't mean the same thing. When PIE is enabled, DBM is in the PIE
register.

> + */
> +
> +#define KVM_HYP_PIR_IDX(uxn, pxn, dbm, ap1) (((uxn) << 3) | ((pxn) << 2) | \
> +					     ((dbm) << 1) | (ap1))
> +

This is not what these bits are called. They are just PIIndex[] bits.

> +#define KVM_HVHE_PIR_EL2 (						\
> +	PIRx_ELx_PERM_PREP(KVM_HYP_PIR_IDX(0, 0, 0, 0), PIE_RX)	|	\
> +	PIRx_ELx_PERM_PREP(KVM_HYP_PIR_IDX(1, 1, 0, 0), PIE_R)	|	\
> +	PIRx_ELx_PERM_PREP(KVM_HYP_PIR_IDX(1, 1, 1, 0), PIE_RW))
> +
>  #endif /* __ARM64_KVM_ARM_H__ */
> diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
> index e5b92ac09e69..eb796436d6eb 100644
> --- a/arch/arm64/include/asm/kvm_asm.h
> +++ b/arch/arm64/include/asm/kvm_asm.h
> @@ -208,6 +208,7 @@ extern void *__vhe_undefined_symbol;
>  struct kvm_nvhe_init_params {
>  	unsigned long mair_el2;
>  	unsigned long tcr_el2;
> +	unsigned long tcr2_el2;
>  	unsigned long tpidr_el2;
>  	unsigned long stack_hyp_va;
>  	unsigned long stack_pa;
> diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
> index 41a8687938eb..7b1b7ab3e88e 100644
> --- a/arch/arm64/include/asm/kvm_pgtable.h
> +++ b/arch/arm64/include/asm/kvm_pgtable.h
> @@ -93,6 +93,7 @@ typedef u64 kvm_pte_t;
>  
>  #define KVM_PTE_LEAF_ATTR_HI_S2_XN	GENMASK(54, 53)
>  
> +#define KVM_PTE_LEAF_ATTR_HI_S1_DBM	BIT(51)
>  #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 | \
> diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
> index 9c853ed3ceab..baffe58015d6 100644
> --- a/arch/arm64/kernel/asm-offsets.c
> +++ b/arch/arm64/kernel/asm-offsets.c
> @@ -118,6 +118,7 @@ int main(void)
>    DEFINE(HOST_DATA_CONTEXT,	offsetof(struct kvm_host_data, host_ctxt));
>    DEFINE(NVHE_INIT_MAIR_EL2,	offsetof(struct kvm_nvhe_init_params, mair_el2));
>    DEFINE(NVHE_INIT_TCR_EL2,	offsetof(struct kvm_nvhe_init_params, tcr_el2));
> +  DEFINE(NVHE_INIT_TCR2_EL2,	offsetof(struct kvm_nvhe_init_params, tcr2_el2));
>    DEFINE(NVHE_INIT_TPIDR_EL2,	offsetof(struct kvm_nvhe_init_params, tpidr_el2));
>    DEFINE(NVHE_INIT_STACK_HYP_VA,	offsetof(struct kvm_nvhe_init_params, stack_hyp_va));
>    DEFINE(NVHE_INIT_PGD_PA,	offsetof(struct kvm_nvhe_init_params, pgd_pa));
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 8b080804bc90..607a6f808b1c 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -2158,7 +2158,7 @@ static int kvm_init_vector_slots(void)
>  static void __init cpu_prepare_hyp_mode(int cpu, u32 hyp_va_bits)
>  {
>  	struct kvm_nvhe_init_params *params = per_cpu_ptr_nvhe_sym(kvm_init_params, cpu);
> -	unsigned long tcr;
> +	unsigned long tcr, tcr2;
>  
>  	/*
>  	 * Calculate the raw per-cpu offset without a translation from the
> @@ -2186,6 +2186,12 @@ static void __init cpu_prepare_hyp_mode(int cpu, u32 hyp_va_bits)
>  	tcr |= TCR_T0SZ(hyp_va_bits);
>  	params->tcr_el2 = tcr;
>  
> +	tcr2 = 0;
> +	if (cpus_have_final_cap(ARM64_HAS_S1PIE) &&
> +	    cpus_have_final_cap(ARM64_KVM_HVHE))
> +		tcr2 |= TCR2_EL2_PIE;
> +	params->tcr2_el2 = tcr2;
> +
>  	params->pgd_pa = kvm_mmu_get_httbr();
>  	if (is_protected_kvm_enabled())
>  		params->hcr_el2 = HCR_HOST_NVHE_PROTECTED_FLAGS;
> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-init.S b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> index 0b3e0b28dfc7..cd5b75c8776f 100644
> --- a/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> @@ -137,8 +137,20 @@ alternative_if ARM64_HAS_CNP
>  alternative_else_nop_endif
>  	msr	ttbr0_el2, x2
>  
> -	ldr	x0, [x0, #NVHE_INIT_TCR_EL2]
> -	msr	tcr_el2, x0
> +	ldr	x1, [x0, #NVHE_INIT_TCR_EL2]
> +	msr	tcr_el2, x1
> +
> +alternative_if ARM64_HAS_S1PIE
> +	/* S1PIE is only enabled with TCR2_EL2.PIE if we are running hVHE */
> +	mov_q	x1, KVM_HVHE_PIR_EL2
> +	msr	REG_PIR_EL2, x1
> +	msr	REG_PIRE0_EL2, xzr
> +alternative_else_nop_endif
> +
> +alternative_if ARM64_HAS_TCR2
> +	ldr	x1, [x0, #NVHE_INIT_TCR2_EL2]
> +	msr	REG_TCR2_EL2, x1
> +alternative_else_nop_endif

S1PIE implies TCR2. Why the additional alternatives?

>  
>  	isb
>  
> diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> index b74dd5ce1efd..5276c2874fe0 100644
> --- a/arch/arm64/kvm/hyp/pgtable.c
> +++ b/arch/arm64/kvm/hyp/pgtable.c
> @@ -349,6 +349,14 @@ static int hyp_set_prot_attr(enum kvm_pgtable_prot prot, kvm_pte_t *ptep)
>  
>  		if (system_supports_bti_kernel())
>  			attr |= KVM_PTE_LEAF_ATTR_HI_S1_GP;
> +	} else if (cpus_have_final_cap(ARM64_HAS_S1PIE) &&
> +		   cpus_have_final_cap(ARM64_KVM_HVHE) &&
> +		   (prot & KVM_PGTABLE_PROT_W)) {
> +		/*
> +		 * When using S1PIE for hVHE set DBM for writable
> +		 * mappings since AP[2] is ineffective.

It's not ineffective. It doesn't exist! See R_JJNHR.

This is totally obfuscating what is really going on, which is really
simple. We have 4 permission index bits in the PTE, and the both the
code and comments should reflect that.

> +		 */
> +		attr |= KVM_PTE_LEAF_ATTR_HI_S1_DBM;

So this sets PIIndex[1]. But It isn't obvious how this takes us from
R(0x8) to RW(0xC). Where is PIIndex[3] coming from? It is supplied by
the code following through the UXN/PXN bbits, but it is mind-boggling
that we should rely on something deals with non-execution to control
the *READ* permission.

TBH, I think this is completely going the wrong way. Why can't we
write this as a discrete enumeration of the permission combination we
support (all 3 of them), and map that to the correct index?

	M.

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

Re: [PATCH v2] KVM: arm64: Enable S1PIE for hVHE

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 2636 bytes --]

On Wed, Sep 09, 2026 at 10:44:26AM +0100, Marc Zyngier wrote:
> Mark Brown <broonie@kernel.org> wrote:

> > hVHE.  Enable FEAT_S1PIE with hVHE only, hVHE is used for protected VMs
> > but there is no real use case for nVHE mode on hardware with this
> > feature.

> That's not the reason. The reason is that there is no nVHE-only
> hardware with PIE, and that on VHE-capable HW, nVHE and hVHE are
> strictly equivalent. Therefore there is no need to add support for HW
> that does not exist.

Right, that's the reason why there is no real use case for nVHE on this
hardware - even where S1PIE capable hardware can run nVHE there is
always a preferable mode that delivers the same functionality.  I'll
make this more explicit.

> > This should have no practical impact other than causing any unexpected
> > encodings to map to no permissions instead of their default
> > meanings.

> What default meanings?

The meanings that are defined when S1PIE is not enabled.  I will make
this more explicit, or given your comments further down about not
sharing code possibly just renumber so it's not a thing any more.

> > +#define KVM_HYP_PIR_IDX(uxn, pxn, dbm, ap1) (((uxn) << 3) | ((pxn) << 2) | \
> > +					     ((dbm) << 1) | (ap1))
> > +

> This is not what these bits are called. They are just PIIndex[] bits.

I was trying to make it easier to map thing between non-S1PIE and S1PIE
encodings, I'll rename and add a comment to help people follow when
updating.

> > +alternative_if ARM64_HAS_S1PIE
> > +	/* S1PIE is only enabled with TCR2_EL2.PIE if we are running hVHE */
> > +	mov_q	x1, KVM_HVHE_PIR_EL2
> > +	msr	REG_PIR_EL2, x1
> > +	msr	REG_PIRE0_EL2, xzr
> > +alternative_else_nop_endif

> > +alternative_if ARM64_HAS_TCR2
> > +	ldr	x1, [x0, #NVHE_INIT_TCR2_EL2]
> > +	msr	REG_TCR2_EL2, x1
> > +alternative_else_nop_endif

> S1PIE implies TCR2. Why the additional alternatives?

That is true but TCR2 does not imply S1PIE, I wrote things this way so
that TCR2 is initialised even if we end up on a system where that is
present but S1PIE is not (or S1PIE is present but has been disabled by a
command line override).  This is during startup so it seemed reasonable
to write things in a straightforward and easy to read fashion.

> TBH, I think this is completely going the wrong way. Why can't we
> write this as a discrete enumeration of the permission combination we
> support (all 3 of them), and map that to the correct index?

I was deliberately following a similar pattern to that used for the host
kernel, intended to minimise code changes.  I will rework so we have an
alternative path for S1PIE rather than trying to share.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2] KVM: arm64: Enable S1PIE for hVHE

[Marc Zyngier]

On Wed, 09 Sep 2026 13:17:33 +0100,
Mark Brown <broonie@kernel.org> wrote:
> 
> > > +alternative_if ARM64_HAS_TCR2
> > > +	ldr	x1, [x0, #NVHE_INIT_TCR2_EL2]
> > > +	msr	REG_TCR2_EL2, x1
> > > +alternative_else_nop_endif
> 
> > S1PIE implies TCR2. Why the additional alternatives?
> 
> That is true but TCR2 does not imply S1PIE, I wrote things this way so
> that TCR2 is initialised even if we end up on a system where that is
> present but S1PIE is not (or S1PIE is present but has been disabled by a
> command line override).  This is during startup so it seemed reasonable
> to write things in a straightforward and easy to read fashion.

Fair enough.

> > TBH, I think this is completely going the wrong way. Why can't we
> > write this as a discrete enumeration of the permission combination we
> > support (all 3 of them), and map that to the correct index?
> 
> I was deliberately following a similar pattern to that used for the host
> kernel, intended to minimise code changes.  I will rework so we have an
> alternative path for S1PIE rather than trying to share.

I think it is fine to keep the same indices as the kernel for
permissions that map to something that can be expressed with direct
permissions.

But indirect permissions are not additive, and therefore shouldn't be
constructed as such. They are also more expressive, and there will
be a point where we will want to have other permissions that cannot be
expressed by "emulating" direct permissions (Execute-Only springs to
mind).

At this stage, this is not churn. This is an investment.

	M.

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