Re: [PATCH 06/28] KVM: x86/mmu: merge make_spte_{non,}executable

[mlevitsk]

On Tue, 2026-05-05 at 21:52 +0200, Paolo Bonzini wrote:
> As the logic will become more complicated with the introduction
> of MBEC, at least write it only once.
> 
> Tested-by: David Riley <d.riley@xxxxxxxxxxx>
> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> ---
>  arch/x86/kvm/mmu/spte.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/x86/kvm/mmu/spte.c b/arch/x86/kvm/mmu/spte.c
> index 85a0473809b0..849a1c1c92b5 100644
> --- a/arch/x86/kvm/mmu/spte.c
> +++ b/arch/x86/kvm/mmu/spte.c
> @@ -317,14 +317,16 @@ static u64 modify_spte_protections(u64 spte, u64 set, u64 clear)
>   return spte;
>  }
>  
> -static u64 make_spte_executable(u64 spte)
> +static u64 change_spte_executable(u64 spte, u8 access)

Minor nitpick: to be honest this is a bit less readable, but overall, I am not against this change.
Can we add a comment though about what values the 'access' can take?
or even better, add an assert?

>  {
> - return modify_spte_protections(spte, shadow_x_mask, shadow_nx_mask);
> -}
> + u64 set, clear;
>  
> -static u64 make_spte_nonexecutable(u64 spte)
> -{
> - return modify_spte_protections(spte, shadow_nx_mask, shadow_x_mask);
> + if (access & ACC_EXEC_MASK)
> + set = shadow_x_mask;
> + else
> + set = shadow_nx_mask;
> + clear = set ^ (shadow_nx_mask | shadow_x_mask);
> + return modify_spte_protections(spte, set, clear);
>  }
>  
>  /*
> @@ -356,8 +358,8 @@ u64 make_small_spte(struct kvm *kvm, u64 huge_spte,
>   * the page executable as the NX hugepage mitigation no longer
>   * applies.
>   */
> - if ((role.access & ACC_EXEC_MASK) && is_nx_huge_page_enabled(kvm))
> - child_spte = make_spte_executable(child_spte);
> + if (is_nx_huge_page_enabled(kvm))
> + child_spte = change_spte_executable(child_spte, role.access);
>   }
>  
>   return child_spte;
> @@ -379,7 +381,7 @@ u64 make_huge_spte(struct kvm *kvm, u64 small_spte, int level)
>   huge_spte &= KVM_HPAGE_MASK(level) | ~PAGE_MASK;
>  
>   if (is_nx_huge_page_enabled(kvm))
> - huge_spte = make_spte_nonexecutable(huge_spte);
> + huge_spte = change_spte_executable(huge_spte, 0);
>  
>   return huge_spte;
>  }

Reviewed-by: Maxim Levitsky <mlevitsk@xxxxxxxxxx>
Best regards,
	Maxim Levitsky