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

Wed Sep 9 02:44:26 PDT 2026

On Tue, 08 Sep 2026 20:40:47 +0100,
Mark Brown <broonie at 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 at 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.