Re: [PATCH 4/5] KVM: MMU: Optimize pte permission checks

[Avi Kivity <avi@redhat.com>]

On 09/13/2012 03:41 PM, Xiao Guangrong wrote:
> On 09/12/2012 10:29 PM, Avi Kivity wrote:
> 
>> +		pte_access = pt_access & gpte_access(vcpu, pte);
>> +		eperm |= (mmu->permissions[access >> 1] >> pte_access) & 1;
>> 
>>  		last_gpte = FNAME(is_last_gpte)(walker, vcpu, mmu, pte);
>> -		if (last_gpte) {
>> -			pte_access = pt_access & gpte_access(vcpu, pte);
>> -			/* check if the kernel is fetching from user page */
>> -			if (unlikely(pte_access & PT_USER_MASK) &&
>> -			    kvm_read_cr4_bits(vcpu, X86_CR4_SMEP))
>> -				if (fetch_fault && !user_fault)
>> -					eperm = true;
>> -		}
> 
> I see this in the SDM:
> 
> If CR4.SMEP = 1, instructions may be fetched from any linear
> address with a valid translation for which the U/S flag (bit 2) is 0 in at
> least one of the paging-structure entries controlling the translation.

Another good catch.

> 
> This patch checks smep on every levels, breaks this rule.
> (current code checks smep on the last level).
> 

We can just move the permission check to the end of the loop.  We used
to terminate the loop on a permission error, but now we do the whole
thing anyway.

It does mean that we'll need to set accessed bits after the loop is
complete.

-- 
error compiling committee.c: too many arguments to function