[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] RE: [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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |