[PATCH v2 6/6] KVM: arm64: Honor UX/PX attributes for EL2 S1 mappings

[Joey Gouly]

On Thu, Dec 11, 2025 at 04:21:20PM +0000, Marc Zyngier wrote:
> On Thu, 11 Dec 2025 15:18:51 +0000,
> Joey Gouly <joey.gouly at arm.com> wrote:
> > 
> > Question,
> > 
> > On Wed, Dec 10, 2025 at 05:30:24PM +0000, Marc Zyngier wrote:
> > > Now that we potentially have two bits to deal with when setting
> > > execution permissions, make sure we correctly handle them when both
> > > when building the page tables and when reading back from them.
> > > 
> > > Reported-by: Alexandru Elisei <alexandru.elisei at arm.com>
> > > Signed-off-by: Marc Zyngier <maz at kernel.org>
> > > ---
> > >  arch/arm64/include/asm/kvm_pgtable.h | 12 +++---------
> > >  arch/arm64/kvm/hyp/pgtable.c         | 24 +++++++++++++++++++++---
> > >  2 files changed, 24 insertions(+), 12 deletions(-)
> > > 
> > > diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
> > > index be68b89692065..095e6b73740a6 100644
> > > --- a/arch/arm64/include/asm/kvm_pgtable.h
> > > +++ b/arch/arm64/include/asm/kvm_pgtable.h
> > > @@ -87,15 +87,9 @@ typedef u64 kvm_pte_t;
> > >  
> > >  #define KVM_PTE_LEAF_ATTR_HI_SW		GENMASK(58, 55)
> > >  
> > > -#define __KVM_PTE_LEAF_ATTR_HI_S1_XN	BIT(54)
> > > -#define __KVM_PTE_LEAF_ATTR_HI_S1_UXN	BIT(54)
> > > -#define __KVM_PTE_LEAF_ATTR_HI_S1_PXN	BIT(53)
> > > -
> > > -#define KVM_PTE_LEAF_ATTR_HI_S1_XN					\
> > > -	({ cpus_have_final_cap(ARM64_KVM_HVHE) ?			\
> > > -			(__KVM_PTE_LEAF_ATTR_HI_S1_UXN |		\
> > > -			 __KVM_PTE_LEAF_ATTR_HI_S1_PXN) :		\
> > > -			__KVM_PTE_LEAF_ATTR_HI_S1_XN; })
> > > +#define KVM_PTE_LEAF_ATTR_HI_S1_XN	BIT(54)
> > > +#define KVM_PTE_LEAF_ATTR_HI_S1_UXN	BIT(54)
> > > +#define KVM_PTE_LEAF_ATTR_HI_S1_PXN	BIT(53)
> > >  
> > >  #define KVM_PTE_LEAF_ATTR_HI_S2_XN	GENMASK(54, 53)
> > >  
> > > diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> > > index e0bd6a0172729..97c0835d25590 100644
> > > --- a/arch/arm64/kvm/hyp/pgtable.c
> > > +++ b/arch/arm64/kvm/hyp/pgtable.c
> > > @@ -342,6 +342,9 @@ static int hyp_set_prot_attr(enum kvm_pgtable_prot prot, kvm_pte_t *ptep)
> > >  	if (!(prot & KVM_PGTABLE_PROT_R))
> > >  		return -EINVAL;
> > >  
> > > +	if (!cpus_have_final_cap(ARM64_KVM_HVHE))
> > > +		prot &= ~KVM_PGTABLE_PROT_UX;
> > 
> > Trying to understand this part. We don't consider KVM_PGTABLE_PROT_UX below
> > when !HVHE, and we don't set it in kvm_pgtable_hyp_pte_prot() when !HVHE
> > either, so can it ever actually be set?
> 
> Because KVM_PGTABLE_PROT_X, which is directly passed by the high-level
> code, is defined as such:
> 
> 	KVM_PGTABLE_PROT_X			= KVM_PGTABLE_PROT_PX	|
> 						  KVM_PGTABLE_PROT_UX,
> 

This was the main missing part!

> We *could* make that value dependent on HVHE, but since that's in an
> enum, it is pretty ugly to do (not impossible though).
> 
> But it is in the following code that this becomes useful...
> 
> >
> > Otherwise LGTM!
> > 
> > Thanks,
> > Joey
> > 
> > > +
> > >  	if (prot & KVM_PGTABLE_PROT_X) {
> > >  		if (prot & KVM_PGTABLE_PROT_W)
> > >  			return -EINVAL;
> 
> ... here. If you were passed UX (and only that), and that you're
> !HVHE, you won't have execution at all, and can allow writes.
> 
> Does that make sense?

Yes, thanks!

Reviewed-by: Joey Gouly <joey.gouly at arm.com>

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