One last comment from Jan, he suggested to use
page_user = (read_cr4() & X86_CR4_SMEP) ? _PAEG_USER : 0;
thanks!
-Xin
> -----Original Message-----
> From: Keir Fraser [mailto:keir.xen@xxxxxxxxx] On Behalf Of Keir Fraser
> Sent: Saturday, June 04, 2011 3:22 AM
> To: Li, Xin
> Cc: xen-devel; Jan Beulich
> Subject: Re: [PATCH v2] Enable SMEP CPU feature support for XEN hypervisor
>
> Hi Xen,
>
> New patch attached and comments in-line.
>
> On 03/06/2011 19:49, "Li, Xin" <xin.li@xxxxxxxxx> wrote:
>
> >> 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().
>
> Oooo good point. Fixed in the attached patch by moving SMEP setup into
> setup.c, explicitly and immediately after identify_cpu(). I was actually in
> two minds whether to fix this by extending early_cpu_detect(). Overall I
> decided I don't want to grow e_c_d() if not absolutely necessary. If the CPU
> setup stuff in setup.c grows too big and ugly I'd rather refactor it another
> 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?
>
> An evil guest has to penetrate Xen before it can crash it in this way. If
> Xen has been subverted, and is lucky enough to notice, what should it do?
> The only sane answer is to shoot itself in the head. This kind of issue
> would require immediate developer attention to fix whatever Xen bug had been
> exploited to trigger SMEP.
>
> > 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.
>
> Haha, give over on this idea that unexplainable behaviour should make you
> only crash the process/guest. If your behaviour is unexplainable, and you
> have pretensions of security, you fail-stop.
>
> > We can't consistently handle it for 64bit and 32bit guest.
>
> Well yeah, but that ignores my actual question, which was...
> """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."""
>
> I mean, I know we may as well just hide the feature from PV 64b guests
> totally. That's obvious. Let's stop talking about PV 64b guests already! The
> question is: what to do about PV 32b guests?
>
> -- Keir
>
> > Thanks!
> > -Xin
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home