Re: [Xen-devel] [RFC 4/4] HVM x86 deprivileged mode: Trap handlers for deprivileged mode

[Ben Catterall <Ben.Catterall@xxxxxxxxxx>]

On 11/08/15 11:33, Ben Catterall wrote:
> On 10/08/15 11:07, Tim Deegan wrote:
> > Hi,
> >
> > > @@ -685,8 +685,17 @@ static int hap_page_fault(struct vcpu *v,
> > > unsigned long va,
> > >   {
> > >       struct domain *d = v->domain;
> > >
> > > +    /* If we get a page fault whilst in HVM security user mode */
> > > +    if( v->user_mode == 1 )
> > > +    {
> > > +        printk("HVM: #PF (%u:%u) whilst in user mode\n",
> > > +                 d->domain_id, v->vcpu_id);
> > > +        domain_crash_synchronous();
> > > +    }
> > > +
> > This should happen in paging_fault() so it can guard the
> > shadow-pagetable paths too.  Once it's there, it'll need a check for
> > is_hvm_vcpu() as well as for user_mode.  Maybe have a helper function
> > 'is_hvm_deprivileged_vcpu()' to do both checks, also used in
> > hvm_deprivileged_check_trap() &c.
I've moved this and now need to add shadow page table support as this 
currently only supports HAP.
> >
> Ok, I'll make this change.
> > >       HAP_ERROR("Intercepted a guest #PF (%u:%u) with HAP enabled.\n",
> > >                 d->domain_id, v->vcpu_id);
> > > +
> > >       domain_crash(d);
> > >       return 0;
> > >   }
> > > diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
> > > index 9f5a6c6..19d465f 100644
> > > --- a/xen/arch/x86/traps.c
> > > +++ b/xen/arch/x86/traps.c
> > > @@ -74,6 +74,7 @@
> > >   #include <asm/vpmu.h>
> > >   #include <public/arch-x86/cpuid.h>
> > >   #include <xsm/xsm.h>
> > > +#include <xen/hvm/deprivileged.h>
> > >
> > >   /*
> > >    * opt_nmi: one of 'ignore', 'dom0', or 'fatal'.
> > > @@ -500,6 +501,11 @@ static void do_guest_trap(
> > >       struct trap_bounce *tb;
> > >       const struct trap_info *ti;
> > >
> > > +    /* If we take the trap whilst in HVM deprivileged mode
> > > +     * then we should crash the domain.
> > > +     */
> > > +    hvm_deprivileged_check_trap(__FUNCTION__);
> > I wonder whether it would be better to switch to an IDT with all
> > unacceptable traps stubbed out, rather than have to blacklist them all
> > separately.  Probably not - this check is cheap, and maintaining the
> > parallel tables would be a pain.
> >
> > Or maybe there's some single point upstream of here, in the asm
> > handlers, that would catch all the cases where this check is needed?
> Yep, I think this can be done.
Had a deeper look at this. There is a point where all exceptions come in 
in the asm (handle_exception in entry.S) and we could branch out at this 
point. It does mean that we'd treat _all_ exceptions that occur in this 
mode the same way whereas the current way means that we can treat them 
differently (e.g. get __func__). So, should I make all exceptions go to 
the same point or keep as is?
Thanks!
> > In any case, the check needs to return an error code so the caller
> > knows to return without running the rest of the handler (and likewise
> > elsewhere).
> understood.
> > Cheers,
> >
> > Tim.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel