Re: [Xen-devel] [PATCH v2] add SMEP support to HVM guest

[Tim Deegan <Tim.Deegan@xxxxxxxxxx>]

At 18:51 +0800 on 06 Jun (1307386302), Li, Xin wrote:
> > > -    if ( hvm_nx_enabled(current) )
> > > +    if ( hvm_nx_enabled(current) ||
> > > +         (!(pfec & PFEC_user_mode) && hvm_smep_enabled(current)) )
> > 
> > Shouldn't that be
> > "if ( hvm_nx_enabled(current) || hvm_smep_enabled(current) )"?
> 
> A smep fault happens when
> 1) current privilege level < 3

But this code is setting the PFEC_insn_fetch bit, not triggering a page
fault.   And the new PRM says (4.7): 

   I/D flag (bit 4).

   This flag is 1 if (1) the access causing the page-fault exception was
   an instruction fetch; and (2) either (a) CR4.SMEP = 1; or (b) both
   (i) CR4.PAE = 1 (either PAE paging or IA-32e paging is in use); and
   (ii) IA32_EFER.NXE = 1. Otherwise, the flag is 0. This flag describes
   the access causing the page-fault exception, not the access rights
   specified by paging.

> > There's no need to add SMEP-specific code at every level.  The existing
> > code already checks for flags that must be clear, so just arrange for
> > _PAGE_USER to be in both mflags and iflags whenever SMEP is enabled and
> > PFEC_user is clear.
> 
> 2) user flags in all page table level entries are set instead of clear.

Oh I see - yes, this introduces a whole new kind of logic in pagetable
walks.  I've attached a version of your patch that I think will do the
right thing, and that's tidied up in a few other places.  Can you check
to see that it does what you need?

Cheers,

Tim.

-- 
Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

smep
Description: Text document

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel