lkml.org 
[lkml]   [2016]   [Mar]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH 1/4] KVM: MMU: fix permission_fault()
From
Date


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

\
 
 \ /
  Last update: 2016-03-25 16:01    [W:0.532 / U:0.096 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site