Re: [PATCH 08/28] KVM: x86/mmu: separate more EPT/non-EPT permission_fault()

[Sean Christopherson <seanjc@google.com>]

On Tue, May 05, 2026, Paolo Bonzini wrote:
> Move more of EPT handling entirely in the existing "if (!ept)"
> conditional.  Use a new "rf" variable instead of uf for read permissions
> for clarity.
> 
> Merge smepf and ff into a single variable because EPT's "SMEP" (actually
> MBEC) is defined differently and does not need smepf.
> 
> Tested-by: David Riley <d.riley@proxmox.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/kvm/mmu/mmu.c | 26 +++++++++++++++-----------
>  1 file changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index d94a488db79d..fc34536c536b 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -5584,24 +5584,28 @@ static void update_permission_bitmask(struct kvm_mmu *mmu, bool ept)
>  		 * that causes a fault with the given PFEC.
>  		 */
>  
> +		/* Faults from reads to non-readable pages */
> +		u8 rf = 0;
>  		/* Faults from writes to non-writable pages */
>  		u8 wf = (pfec & PFERR_WRITE_MASK) ? (u8)~w : 0;
>  		/* Faults from user mode accesses to supervisor pages */
> -		u8 uf = (pfec & PFERR_USER_MASK) ? (u8)~u : 0;
> -		/* Faults from fetches of non-executable pages*/
> -		u8 ff = (pfec & PFERR_FETCH_MASK) ? (u8)~x : 0;
> -		/* Faults from kernel mode fetches of user pages */
> -		u8 smepf = 0;
> +		u8 uf = 0;
> +		/* Faults from fetches of non-executable pages */
> +		u8 ff = 0;
>  		/* Faults from kernel mode accesses of user pages */
>  		u8 smapf = 0;
>  
> -		if (!ept) {
> +		if (ept) {
> +			rf = (pfec & PFERR_USER_MASK) ? (u8)~u : 0;
> +			ff = (pfec & PFERR_FETCH_MASK) ? (u8)~x : 0;
> +		} else {
>  			/* Faults from kernel mode accesses to user pages */
>  			u8 kf = (pfec & PFERR_USER_MASK) ? 0 : u;
>  
> -			/* Not really needed: !nx will cause pte.nx to fault */
> -			if (!efer_nx)
> -				ff = 0;
> +			uf = (pfec & PFERR_USER_MASK) ? (u8)~u : 0;
> +
> +			if (efer_nx)
> +				ff = (pfec & PFERR_FETCH_MASK) ? (u8)~x : 0;

I still think we should do:

			if (efer_nx)
				ff |= (pfec & PFERR_FETCH_MASK) ? (u8)~x : 0;

so that the order of the efer_nx and cr4_smep logic doesn't matter.

>  
>  			/* Allow supervisor writes if !cr0.wp */
>  			if (!cr0_wp)
> @@ -5609,7 +5613,7 @@ static void update_permission_bitmask(struct kvm_mmu *mmu, bool ept)
>  
>  			/* Disallow supervisor fetches of user code if cr4.smep */
>  			if (cr4_smep)
> -				smepf = (pfec & PFERR_FETCH_MASK) ? kf : 0;
> +				ff |= (pfec & PFERR_FETCH_MASK) ? kf : 0;