[Xen-devel] RE: [PATCH v2] Enable SMEP CPU feature support for XEN hypervisor

["Li, Xin" <xin.li@xxxxxxxxx>]

> > Intel new CPU supports SMEP (Supervisor Mode Execution Protection). SMEP
> > prevents software operating with CPL < 3 (supervisor mode) from fetching
> > instructions from any linear address with a valid translation for which the
> > U/S
> > flag (bit 2) is 1 in every paging-structure entry controlling the 
> > translation
> > for the linear address.
> >
> > This patch enables SMEP in Xen to protect Xen hypervisor from executing pv
> > guest
> > instructions, whose translation paging-structure entries' U/S flags are all
> > set.
> 
> Xin,
> 
> I have attached a modified version of your patch which I believe deals with
> a number of issues. I briefly outline the major ones here:
> 
> 1. The initialisation in cpu/common.c is misguided. Features like
> set_in_cr4(), setup_clear_cpu_cap(), and cpu_has_xxx, are designed to allow
> the feature initialisation to happen close in code to where the feature is
> used. Hence I have moved the initialisation into traps.c:trap_init().

It's the right way to move.

But trap_init() is called before Xen does leaf 7.0 detection, thus we also need 
to
add leaf 7.0 detection in early_cpu_detect to get 
boot_cpu_data.x86_capability[7]
initialized before trap_init().

> 2. I have renamed the command-line option to 'smep'. Boolean options can be
> inverted by prepending 'no-', or appending '={0,false,off,no}'. We don't
> generally encode the negative within the base option name string.
> 
Thanks for doing in this better way.

> 3. I have pushed interpretation of the pf_type enumeration out to the
> callers of spurious_page_fault(). This is because a SMEP fault while Xen is
> executing should *crash Xen*. So that's what I do. Further, when it is a

I'm still wondering is it overkill to kill Xen?  An evil guest can crash Xen?

> guest fault, we should domain_crash() not domain_crash_synchronous() (which
> is generally a more dangerous function for Xen's own health).
> 
> More generally, I wonder whether SMEP should be enabled only for guests
> (even PV guests) which detect it via CPUID and proactively enable it via
> their virtualised CR4? I mean, it is off in real hardware by default for a
> reason: backward compatibility. Furthermore, we only detect spurious page
> faults for buggy old PV guests, the rest will get the SMEP fault punted up
> to them, which seems odd if they don't understand SMEP. What do you think?

64bit pv guest can't use SMEP as the kernel code is user accessible.  Thus
when guest executes it won't trigger SMEP fault. only when Xen executes
it will. It's okay to crash Xen (or just the guest).

32bit pv guest should be able to make use of SMEP.  When it is from Xen,
we crash Xen.  When it's is from guest kernel executing user code, we
can inject to guest to let it kill the current process.  Of course such cases
the guest should be able to do SMEP handling.

We can't consistently handle it for 64bit and 32bit guest.

Thanks!
-Xin

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