Re: [PATCH 3/6] KVM: MMU: Add EPT support

["Yang, Sheng" <sheng.yang@intel.com>]

On Friday 18 April 2008 21:30:14 Anthony Liguori wrote:
> Yang, Sheng wrote:
> > @@ -1048,17 +1071,18 @@ static void mmu_set_spte(struct kvm_vcpu *vcpu,
> > u64 *shadow_pte,
> >  	 * whether the guest actually used the pte (in order to detect
> >  	 * demand paging).
> >  	 */
> > -	spte = PT_PRESENT_MASK | PT_DIRTY_MASK;
> > +	spte = shadow_base_present_pte | shadow_dirty_mask;
> >  	if (!speculative)
> >  		pte_access |= PT_ACCESSED_MASK;
> >  	if (!dirty)
> >  		pte_access &= ~ACC_WRITE_MASK;
> > -	if (!(pte_access & ACC_EXEC_MASK))
> > -		spte |= PT64_NX_MASK;
> > -
> > -	spte |= PT_PRESENT_MASK;
> > +	if (pte_access & ACC_EXEC_MASK) {
> > +		if (shadow_x_mask)
> > +			spte |= shadow_x_mask;
> > +	} else if (shadow_nx_mask)
> > +		spte |= shadow_nx_mask;
>
> This looks like it may be a bug.  The old behavior sets NX if
> (pte_access & ACC_EXEC_MASK).  The new behavior unconditionally sets NX
> and never sets PRESENT.  Also, the if (shadow_x_mas k) checks are
> unnecessary.  spte |= 0 is a nop.

Thanks for the comment! I realized two judgments of shadow_nx/x_mask is 
unnecessary... In fact, the correct behavior is either set shadow_x_mask or 
shadow_nx_mask, may be there is a better approach for this. The logic assured 
by program itself is always safer. But I will remove the redundant code at 
first.

But I don't think it's a bug. The old behavior set NX if (!(pte_access & 
ACC_EXEC_MASK)), the same as the new one. And I also curious about the 
PRESENT bit. You see, the PRESENT bit was set at the beginning of the code, 
and I really don't know why the duplicate one exists there... 

>
> >  	if (pte_access & ACC_USER_MASK)
> > -		spte |= PT_USER_MASK;
> > +		spte |= shadow_user_mask;
> >  	if (largepage)
> >  		spte |= PT_PAGE_SIZE_MASK;

-- 
Thanks
Yang, Sheng

>
> Regards,
>
> Anthony Liguori



-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone