Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1364688

Re: [PATCH 1/4] KVM: MMU: fix permission_fault()

From Paolo Bonzini <pbonzini@redhat.com>
Newsgroups linux.kernel
Subject Re: [PATCH 1/4] KVM: MMU: fix permission_fault()
Date 2016-03-25 15:30 +0100
Message-ID <rgAUq-2VM-15@gated-at.bofh.it> (permalink)
References <rgzYm-2fh-7@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On 25/03/2016 14:19, Xiao Guangrong wrote:
>  	WARN_ON(pfec & (PFERR_PK_MASK | PFERR_RSVD_MASK));
> -	pfec |= PFERR_PRESENT_MASK;
> +	errcode = PFERR_PRESENT_MASK;
>  
>  	if (unlikely(mmu->pkru_mask)) {
>  		u32 pkru_bits, offset;
> @@ -193,11 +193,11 @@ static inline u8 permission_fault(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
>  			((pte_access & PT_USER_MASK) << (PFERR_RSVD_BIT - PT_USER_SHIFT));
>  
>  		pkru_bits &= mmu->pkru_mask >> offset;
> -		pfec |= -pkru_bits & PFERR_PK_MASK;
> +		errcode |= -pkru_bits & PFERR_PK_MASK;
>  		fault |= (pkru_bits != 0);
>  	}
>  
> -	return -(uint32_t)fault & pfec;
> +	return -(uint32_t)fault & errcode;
>  }

I have another doubt here.

If you get a fault due to U=0, you would not get PFERR_PK_MASK.  This
is checked implicitly through the pte_user bit which we moved to
PFERR_RSVD_BIT.  However, if you get a fault due to W=0 _and_
PKRU.AD=1 or PKRU.WD=1 for the page's protection key, would the PK
bit be set in the error code?  If not, we would need something like
this:

diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
index 81bffd1524c4..6835a551a5c4 100644
--- a/arch/x86/kvm/mmu.h
+++ b/arch/x86/kvm/mmu.h
@@ -172,12 +172,11 @@ static inline u8 permission_fault(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
 	unsigned long smap = (cpl - 3) & (rflags & X86_EFLAGS_AC);
 	int index = (pfec >> 1) +
 		    (smap >> (X86_EFLAGS_AC_BIT - PFERR_RSVD_BIT + 1));
-	bool fault = (mmu->permissions[index] >> pte_access) & 1;
 
 	WARN_ON(pfec & (PFERR_PK_MASK | PFERR_RSVD_MASK));
-	errcode = PFERR_PRESENT_MASK;
+	errcode = (mmu->permissions[index] >> pte_access) & PFERR_PRESENT_MASK;
 
-	if (unlikely(mmu->pkru_mask)) {
+	if (unlikely(-errcode & mmu->pkru_mask)) {
 		u32 pkru_bits, offset;
 
 		/*
@@ -188,11 +187,10 @@ static inline u8 permission_fault(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
 			((pte_access & PT_USER_MASK) << (PFERR_RSVD_BIT - PT_USER_SHIFT));
 
 		pkru_bits &= mmu->pkru_mask >> offset;
-		errcode |= -pkru_bits & PFERR_PK_MASK;
-		fault |= (pkru_bits != 0);
+		errcode |= pkru_bits ? PFERR_PK_MASK | PFERR_PRESENT_MASK : 0;
 	}
 
-	return -(uint32_t)fault & errcode;
+	return errcode;
 }
 
 void kvm_mmu_invalidate_zap_all_pages(struct kvm *kvm);


Thanks,

Paolo

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 1/4] KVM: MMU: fix permission_fault() Xiao Guangrong <guangrong.xiao@linux.intel.com> - 2016-03-25 14:30 +0100
  [PATCH 3/4] KVM: MMU: reduce the size of mmu_page_path Xiao Guangrong <guangrong.xiao@linux.intel.com> - 2016-03-25 14:30 +0100
    Re: [PATCH 3/4] KVM: MMU: reduce the size of mmu_page_path Paolo Bonzini <pbonzini@redhat.com> - 2016-03-25 14:50 +0100
      Re: [PATCH 3/4] KVM: MMU: reduce the size of mmu_page_path Paolo Bonzini <pbonzini@redhat.com> - 2016-03-25 15:00 +0100
        Re: [PATCH 3/4] KVM: MMU: reduce the size of mmu_page_path Xiao Guangrong <guangrong.xiao@linux.intel.com> - 2016-03-25 15:10 +0100
          Re: [PATCH 3/4] KVM: MMU: reduce the size of mmu_page_path Paolo Bonzini <pbonzini@redhat.com> - 2016-03-25 15:30 +0100
      Re: [PATCH 3/4] KVM: MMU: reduce the size of mmu_page_path Xiao Guangrong <guangrong.xiao@linux.intel.com> - 2016-03-25 15:00 +0100
  Re: [PATCH 1/4] KVM: MMU: fix permission_fault() Paolo Bonzini <pbonzini@redhat.com> - 2016-03-25 14:40 +0100
    Re: [PATCH 1/4] KVM: MMU: fix permission_fault() Xiao Guangrong <guangrong.xiao@linux.intel.com> - 2016-03-25 14:50 +0100
      Re: [PATCH 1/4] KVM: MMU: fix permission_fault() Paolo Bonzini <pbonzini@redhat.com> - 2016-03-25 15:00 +0100
  [PATCH 2/4] KVM: MMU: simplify the logic of __mmu_unsync_walk() Xiao Guangrong <guangrong.xiao@linux.intel.com> - 2016-03-25 14:40 +0100
  Re: [PATCH 1/4] KVM: MMU: fix permission_fault() Paolo Bonzini <pbonzini@redhat.com> - 2016-03-25 15:30 +0100
    Re: [PATCH 1/4] KVM: MMU: fix permission_fault() Xiao Guangrong <guangrong.xiao@linux.intel.com> - 2016-03-29 19:50 +0200
      Re: [PATCH 1/4] KVM: MMU: fix permission_fault() Paolo Bonzini <pbonzini@redhat.com> - 2016-03-29 22:20 +0200
        Re: [PATCH 1/4] KVM: MMU: fix permission_fault() Xiao Guangrong <guangrong.xiao@linux.intel.com> - 2016-03-30 04:00 +0200
          Re: [PATCH 1/4] KVM: MMU: fix permission_fault() Paolo Bonzini <pbonzini@redhat.com> - 2016-03-30 08:40 +0200
            Re: [PATCH 1/4] KVM: MMU: fix permission_fault() Xiao Guangrong <guangrong.xiao@linux.intel.com> - 2016-03-30 08:50 +0200

csiph-web