[PATCH v2 0/2] KVM: arm64: nv: Fixes for FEAT_XNX (and the lack thereof)

[PATCH v2 0/2] KVM: arm64: nv: Fixes for FEAT_XNX (and the lack thereof)

[Oliver Upton]

Sending a v2 to address Wei-Lin's feedback and fix an additional issue
spotted by sashiko.

v1: https://lore.kernel.org/kvmarm/20260602072759.37885-1-oupton@kernel.org/

Oliver Upton (2):
  KVM: arm64: nv: Fix handling of XN[0] when !FEAT_XNX
  KVM: arm64: Correctly identify executable PTEs at stage-2

 arch/arm64/include/asm/kvm_nested.h | 4 ++--
 arch/arm64/kvm/hyp/pgtable.c        | 4 +++-
 2 files changed, 5 insertions(+), 3 deletions(-)


base-commit: 5d6919055dec134de3c40167a490f33c74c12581
-- 
2.47.3

[PATCH v2 1/2] KVM: arm64: nv: Fix handling of XN[0] when !FEAT_XNX

[Oliver Upton]

XN has already been extracted from its bitfield position so using
FIELD_PREP() on the mask that clears XN[0] is completely broken, having
the effect of unconditionally granting execute permissions...

Fix the obvious mistake by manipulating the right bit.

Cc: stable@vger.kernel.org
Fixes: d93febe2ed2e ("KVM: arm64: nv: Forward FEAT_XNX permissions to the shadow stage-2")
Reviewed-by: Wei-Lin Chang <weilin.chang@arm.com>
Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 arch/arm64/include/asm/kvm_nested.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
index 091544e6af44..a0eb83319c2e 100644
--- a/arch/arm64/include/asm/kvm_nested.h
+++ b/arch/arm64/include/asm/kvm_nested.h
@@ -131,7 +131,7 @@ static inline bool kvm_s2_trans_exec_el0(struct kvm *kvm, struct kvm_s2_trans *t
 	u8 xn = FIELD_GET(KVM_PTE_LEAF_ATTR_HI_S2_XN, trans->desc);
 
 	if (!kvm_has_xnx(kvm))
-		xn &= FIELD_PREP(KVM_PTE_LEAF_ATTR_HI_S2_XN, 0b10);
+		xn &= 0b10;
 
 	switch (xn) {
 	case 0b00:
@@ -147,7 +147,7 @@ static inline bool kvm_s2_trans_exec_el1(struct kvm *kvm, struct kvm_s2_trans *t
 	u8 xn = FIELD_GET(KVM_PTE_LEAF_ATTR_HI_S2_XN, trans->desc);
 
 	if (!kvm_has_xnx(kvm))
-		xn &= FIELD_PREP(KVM_PTE_LEAF_ATTR_HI_S2_XN, 0b10);
+		xn &= 0b10;
 
 	switch (xn) {
 	case 0b00:
-- 
2.47.3

Re: [PATCH v2 1/2] KVM: arm64: nv: Fix handling of XN[0] when !FEAT_XNX

[Wei-Lin Chang]

Hi Oliver,

On Tue, Jun 02, 2026 at 09:59:00AM -0700, Oliver Upton wrote:
> XN has already been extracted from its bitfield position so using
> FIELD_PREP() on the mask that clears XN[0] is completely broken, having
> the effect of unconditionally granting execute permissions...
> 
> Fix the obvious mistake by manipulating the right bit.
> 
> Cc: stable@vger.kernel.org
> Fixes: d93febe2ed2e ("KVM: arm64: nv: Forward FEAT_XNX permissions to the shadow stage-2")
> Reviewed-by: Wei-Lin Chang <weilin.chang@arm.com>
> Signed-off-by: Oliver Upton <oupton@kernel.org>
> ---
>  arch/arm64/include/asm/kvm_nested.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
> index 091544e6af44..a0eb83319c2e 100644
> --- a/arch/arm64/include/asm/kvm_nested.h
> +++ b/arch/arm64/include/asm/kvm_nested.h
> @@ -131,7 +131,7 @@ static inline bool kvm_s2_trans_exec_el0(struct kvm *kvm, struct kvm_s2_trans *t
>  	u8 xn = FIELD_GET(KVM_PTE_LEAF_ATTR_HI_S2_XN, trans->desc);
>  
>  	if (!kvm_has_xnx(kvm))
> -		xn &= FIELD_PREP(KVM_PTE_LEAF_ATTR_HI_S2_XN, 0b10);
> +		xn &= 0b10;
>  
>  	switch (xn) {
>  	case 0b00:
> @@ -147,7 +147,7 @@ static inline bool kvm_s2_trans_exec_el1(struct kvm *kvm, struct kvm_s2_trans *t
>  	u8 xn = FIELD_GET(KVM_PTE_LEAF_ATTR_HI_S2_XN, trans->desc);
>  
>  	if (!kvm_has_xnx(kvm))
> -		xn &= FIELD_PREP(KVM_PTE_LEAF_ATTR_HI_S2_XN, 0b10);
> +		xn &= 0b10;
>  

Now that the other patch brings up kvm_pgtable_stage2_pte_prot(), what
do you think about also using that here? It can save a little bit of
duplicated decode logic.

Other than this being in a header and we'll have to move the code
around for this to work, I'm curious are there any other issues with
this idea?

Thanks,
Wei-Lin Chang

>  	switch (xn) {
>  	case 0b00:
> -- 
> 2.47.3
> 

Re: [PATCH v2 1/2] KVM: arm64: nv: Fix handling of XN[0] when !FEAT_XNX

[Oliver Upton]

Hey Wei-Lin,

Thanks for the review.

On Wed, Jun 03, 2026 at 11:57:20PM +0100, Wei-Lin Chang wrote:
> Hi Oliver,
> 
> On Tue, Jun 02, 2026 at 09:59:00AM -0700, Oliver Upton wrote:
> > XN has already been extracted from its bitfield position so using
> > FIELD_PREP() on the mask that clears XN[0] is completely broken, having
> > the effect of unconditionally granting execute permissions...
> > 
> > Fix the obvious mistake by manipulating the right bit.
> > 
> > Cc: stable@vger.kernel.org
> > Fixes: d93febe2ed2e ("KVM: arm64: nv: Forward FEAT_XNX permissions to the shadow stage-2")
> > Reviewed-by: Wei-Lin Chang <weilin.chang@arm.com>
> > Signed-off-by: Oliver Upton <oupton@kernel.org>
> > ---
> >  arch/arm64/include/asm/kvm_nested.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
> > index 091544e6af44..a0eb83319c2e 100644
> > --- a/arch/arm64/include/asm/kvm_nested.h
> > +++ b/arch/arm64/include/asm/kvm_nested.h
> > @@ -131,7 +131,7 @@ static inline bool kvm_s2_trans_exec_el0(struct kvm *kvm, struct kvm_s2_trans *t
> >  	u8 xn = FIELD_GET(KVM_PTE_LEAF_ATTR_HI_S2_XN, trans->desc);
> >  
> >  	if (!kvm_has_xnx(kvm))
> > -		xn &= FIELD_PREP(KVM_PTE_LEAF_ATTR_HI_S2_XN, 0b10);
> > +		xn &= 0b10;
> >  
> >  	switch (xn) {
> >  	case 0b00:
> > @@ -147,7 +147,7 @@ static inline bool kvm_s2_trans_exec_el1(struct kvm *kvm, struct kvm_s2_trans *t
> >  	u8 xn = FIELD_GET(KVM_PTE_LEAF_ATTR_HI_S2_XN, trans->desc);
> >  
> >  	if (!kvm_has_xnx(kvm))
> > -		xn &= FIELD_PREP(KVM_PTE_LEAF_ATTR_HI_S2_XN, 0b10);
> > +		xn &= 0b10;
> >  
> 
> Now that the other patch brings up kvm_pgtable_stage2_pte_prot(), what
> do you think about also using that here? It can save a little bit of
> duplicated decode logic.
> 
> Other than this being in a header and we'll have to move the code
> around for this to work, I'm curious are there any other issues with
> this idea?

No issues with your suggestion but I plan on nuking the kvm_s2_trans*()
accessors soon :)

Ultimately kvm_s2_trans should just contain pre-computed permissions,
which matters more when dealing with descriptor fields that require the
MMU context to make sense of (like DBM). On top of that, getters for
obviously named fields isn't adding a whole lot.

Any concerns with leaving as-is for now?

-- 
Thanks,
Oliver

Re: [PATCH v2 1/2] KVM: arm64: nv: Fix handling of XN[0] when !FEAT_XNX

[Wei-Lin Chang]

On Wed, Jun 03, 2026 at 04:06:09PM -0700, Oliver Upton wrote:
> Hey Wei-Lin,
> 
> Thanks for the review.
> 
> On Wed, Jun 03, 2026 at 11:57:20PM +0100, Wei-Lin Chang wrote:
> > Hi Oliver,
> > 
> > On Tue, Jun 02, 2026 at 09:59:00AM -0700, Oliver Upton wrote:
> > > XN has already been extracted from its bitfield position so using
> > > FIELD_PREP() on the mask that clears XN[0] is completely broken, having
> > > the effect of unconditionally granting execute permissions...
> > > 
> > > Fix the obvious mistake by manipulating the right bit.
> > > 
> > > Cc: stable@vger.kernel.org
> > > Fixes: d93febe2ed2e ("KVM: arm64: nv: Forward FEAT_XNX permissions to the shadow stage-2")
> > > Reviewed-by: Wei-Lin Chang <weilin.chang@arm.com>
> > > Signed-off-by: Oliver Upton <oupton@kernel.org>
> > > ---
> > >  arch/arm64/include/asm/kvm_nested.h | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
> > > index 091544e6af44..a0eb83319c2e 100644
> > > --- a/arch/arm64/include/asm/kvm_nested.h
> > > +++ b/arch/arm64/include/asm/kvm_nested.h
> > > @@ -131,7 +131,7 @@ static inline bool kvm_s2_trans_exec_el0(struct kvm *kvm, struct kvm_s2_trans *t
> > >  	u8 xn = FIELD_GET(KVM_PTE_LEAF_ATTR_HI_S2_XN, trans->desc);
> > >  
> > >  	if (!kvm_has_xnx(kvm))
> > > -		xn &= FIELD_PREP(KVM_PTE_LEAF_ATTR_HI_S2_XN, 0b10);
> > > +		xn &= 0b10;
> > >  
> > >  	switch (xn) {
> > >  	case 0b00:
> > > @@ -147,7 +147,7 @@ static inline bool kvm_s2_trans_exec_el1(struct kvm *kvm, struct kvm_s2_trans *t
> > >  	u8 xn = FIELD_GET(KVM_PTE_LEAF_ATTR_HI_S2_XN, trans->desc);
> > >  
> > >  	if (!kvm_has_xnx(kvm))
> > > -		xn &= FIELD_PREP(KVM_PTE_LEAF_ATTR_HI_S2_XN, 0b10);
> > > +		xn &= 0b10;
> > >  
> > 
> > Now that the other patch brings up kvm_pgtable_stage2_pte_prot(), what
> > do you think about also using that here? It can save a little bit of
> > duplicated decode logic.
> > 
> > Other than this being in a header and we'll have to move the code
> > around for this to work, I'm curious are there any other issues with
> > this idea?
> 
> No issues with your suggestion but I plan on nuking the kvm_s2_trans*()
> accessors soon :)

Ah, cool cool.

> 
> Ultimately kvm_s2_trans should just contain pre-computed permissions,
> which matters more when dealing with descriptor fields that require the
> MMU context to make sense of (like DBM). On top of that, getters for
> obviously named fields isn't adding a whole lot.

I see, makes sense to me.

> 
> Any concerns with leaving as-is for now?

No, and thanks for sharing!

Thanks,
Wei-Lin Chang

> 
> -- 
> Thanks,
> Oliver

[PATCH v2 2/2] KVM: arm64: Correctly identify executable PTEs at stage-2

[Oliver Upton]

KVM invalidates the I-cache before installing an executable PTE on
implementations without DIC. Unfortunately, support for FEAT_XNX
broke this check as KVM_PTE_LEAF_ATTR_HI_S2_XN was expanded to a
bitfield.

Fix it by reusing kvm_pgtable_stage2_pte_prot() and testing the abstract
permission bits instead.

Fixes: 2608563b466b ("KVM: arm64: Add support for FEAT_XNX stage-2 permissions")
Reported-by: Sashiko (gemini/gemini-3.1-pro-preview)
Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 arch/arm64/kvm/hyp/pgtable.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index 0c1defa5fb0f..91a7dfad6686 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -925,7 +925,9 @@ static bool stage2_pte_cacheable(struct kvm_pgtable *pgt, kvm_pte_t pte)
 
 static bool stage2_pte_executable(kvm_pte_t pte)
 {
-	return kvm_pte_valid(pte) && !(pte & KVM_PTE_LEAF_ATTR_HI_S2_XN);
+	enum kvm_pgtable_prot prot = kvm_pgtable_stage2_pte_prot(pte);
+
+	return prot & (KVM_PGTABLE_PROT_UX | KVM_PGTABLE_PROT_PX);
 }
 
 static u64 stage2_map_walker_phys_addr(const struct kvm_pgtable_visit_ctx *ctx,
-- 
2.47.3

Re: [PATCH v2 2/2] KVM: arm64: Correctly identify executable PTEs at stage-2

[Wei-Lin Chang]

On Tue, Jun 02, 2026 at 09:59:01AM -0700, Oliver Upton wrote:
> KVM invalidates the I-cache before installing an executable PTE on
> implementations without DIC. Unfortunately, support for FEAT_XNX
> broke this check as KVM_PTE_LEAF_ATTR_HI_S2_XN was expanded to a
> bitfield.
> 
> Fix it by reusing kvm_pgtable_stage2_pte_prot() and testing the abstract
> permission bits instead.
> 
> Fixes: 2608563b466b ("KVM: arm64: Add support for FEAT_XNX stage-2 permissions")
> Reported-by: Sashiko (gemini/gemini-3.1-pro-preview)
> Signed-off-by: Oliver Upton <oupton@kernel.org>
> ---
>  arch/arm64/kvm/hyp/pgtable.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> index 0c1defa5fb0f..91a7dfad6686 100644
> --- a/arch/arm64/kvm/hyp/pgtable.c
> +++ b/arch/arm64/kvm/hyp/pgtable.c
> @@ -925,7 +925,9 @@ static bool stage2_pte_cacheable(struct kvm_pgtable *pgt, kvm_pte_t pte)
>  
>  static bool stage2_pte_executable(kvm_pte_t pte)
>  {
> -	return kvm_pte_valid(pte) && !(pte & KVM_PTE_LEAF_ATTR_HI_S2_XN);
> +	enum kvm_pgtable_prot prot = kvm_pgtable_stage2_pte_prot(pte);
> +
> +	return prot & (KVM_PGTABLE_PROT_UX | KVM_PGTABLE_PROT_PX);
>  }
>  
>  static u64 stage2_map_walker_phys_addr(const struct kvm_pgtable_visit_ctx *ctx,
> -- 
> 2.47.3
> 

Reviewed-by: Wei-Lin Chang <weilin.chang@arm.com>

Thanks,
Wei-Lin Chang