[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] xen/pvshim: fix GNTTABOP_query_size hypercall forwarding with SMAP



On Fri, Jan 26, 2018 at 04:28:27PM +0000, Wei Liu wrote:
> On Fri, Jan 26, 2018 at 04:22:41PM +0000, Roger Pau Monné wrote:
> > On Fri, Jan 26, 2018 at 03:52:38PM +0000, Wei Liu wrote:
> > > On Fri, Jan 26, 2018 at 03:29:10PM +0000, Roger Pau Monne wrote:
> > > > Disable SMAP in the shim before bouncing the hypercall, or else L0
> > > > will fail to get the hypercall buffer.
> > > > 
> > > > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > > > Reported-by: Fatih Acar <fatih.acar@xxxxxxxxx>
> > > > ---
> > > > Cc: Jan Beulich <jbeulich@xxxxxxxx>
> > > > Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > > > Cc: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
> > > > Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> > > > ---
> > > > Needs to be backported to the 4.10.0-shim-comet branch.
> > > > ---
> > > >  xen/arch/x86/pv/shim.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > > 
> > > > diff --git a/xen/arch/x86/pv/shim.c b/xen/arch/x86/pv/shim.c
> > > > index d5383dcfc7..eb1ee7d3c4 100644
> > > > --- a/xen/arch/x86/pv/shim.c
> > > > +++ b/xen/arch/x86/pv/shim.c
> > > > @@ -748,7 +748,10 @@ static long pv_shim_grant_table_op(unsigned int 
> > > > cmd,
> > > >      }
> > > >  
> > > >      case GNTTABOP_query_size:
> > > > +        /* Disable SMAP so L0 can access the buffer. */
> > > 
> > > Interesting.
> > > 
> > > > +        stac();
> > > >          rc = xen_hypercall_grant_table_op(GNTTABOP_query_size, uop.p, 
> > > > count);
> > > > +        clac();
> > > 
> > > There is so far only one instance of this, so doing this ad-hoc stac /
> > > clac is OK.
> > > 
> > > Probably another (more uniformed) way of fixing it is to have our own
> > > buffer (forbid passing through uop directly) and the copy back the
> > > result. Or we should invent a set of macros to deal with uop.
> > 
> > I guess we should figure out what's faster, disabling SMAP or copying
> > to a buffer?
> > 
> 
> It depends. If there are going to be instances that L0 needs to copy to
> a large buffer in L2 userspace, disabling SMAP is definitely better then
> bouncing.
> 
> In this particular case, it doesn't matter.
> 
> You can have my Rb on this patch if you think disabling SMAP is better.

It's less cumbersome than copying, and certainly faster if there's no
SMAP support, since that's just a nop in that case.

Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.