WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Subject: RE: [Xen-devel] [PATCH v2] add SMEP support to HVM guest
From: "Li, Xin" <xin.li@xxxxxxxxx>
Date: Mon, 6 Jun 2011 18:51:42 +0800
Accept-language: zh-CN, en-US
Acceptlanguage: zh-CN, en-US
Cc: Jan, xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxx>, Keir Fraser <keir@xxxxxxx>, "'Keir Fraser \(keir.xen@xxxxxxxxx\)'" <keir.xen@xxxxxxxxx>, Beulich <JBeulich@xxxxxxxxxx>
Delivery-date: Mon, 06 Jun 2011 03:53:54 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20110606083750.GU5098@xxxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <FC2FB65B4D919844ADE4BE3C2BB739AD5AB9EE1D@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> <20110606083750.GU5098@xxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcwkJQjRKE5HbSOrRIqSjoi0JPEMTwACuT6g
Thread-topic: [Xen-devel] [PATCH v2] add SMEP support to HVM guest
> > -    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

> > +         (!(pfec & PFEC_user_mode) && hvm_smep_enabled(current)) )
> 
> Likewise.
> 
> >      l4p = (guest_l4e_t *) top_map;
> >      gw->l4e = l4p[guest_l4_table_offset(va)];
> >      gflags = guest_l4e_get_flags(gw->l4e) ^ iflags;
> > +    user_flag &= gflags;
> 
> 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.

In current code, what I read is that it assumes some permission(s) is missing
in guest_walk_tables(). NX is special, but your code wisely used inverted logic
to seamlessly cover it. I did want to reuse the existing logic, but seems it 
can't
accommodate SMEP logic easily.

Please advise!


> > @@ -277,6 +281,11 @@ guest_walk_tables(struct vcpu *v, struct
> >       * walkers behave this way. */
> >      if ( rc == 0 )
> >      {
> > +        if ( guest_supports_smep(v) && user_flag &&
> > +             ((pfec & (PFEC_insn_fetch|PFEC_user_mode)) ==
> PFEC_insn_fetch) ) {
> > +            rc = _PAGE_SMEP;
> > +            goto out;
> > +        }
> 
> I think this hunk will probably go away entirely, but if not, please
> don't put it between the code below and the comment above that describes
> it.

I didn't figure out a better way than this, will add comments if we have to do
this way.  Or do you have any other suggestion?

> > +#define _PAGE_SMEP    0x8000U
> 
> What does this new code mean?  You added code that returns it but not
> any that checks for it.

Actually the current logic doesn't check what is missing returned from
guest_walk_tables unless it needs to change error code, and the callers
mostly return INVALID_GFN.
I introduced _PAGE_SMEP just to let the caller know it is not 0.  Probably
I can reuse one of the _PAGE_ macros, but the memory virtualization code
is subtle, I'm afraid I'll introduce bugs in other parts. That's to say, I do 
want
to hear from you :)
Thanks!
-Xin

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