Re: [PATCH 8/9] drm/xen-front: Implement GEM operations

[Oleksandr Andrushchenko]

On 02/28/2018 09:46 PM, Boris Ostrovsky wrote:
> On 02/27/2018 01:52 AM, Oleksandr Andrushchenko wrote:
> > On 02/27/2018 01:47 AM, Boris Ostrovsky wrote:
> > > On 02/23/2018 10:35 AM, Oleksandr Andrushchenko wrote:
> > > > On 02/23/2018 05:26 PM, Boris Ostrovsky wrote:
> > > > > On 02/21/2018 03:03 AM, Oleksandr Andrushchenko wrote:
> > > > > > +ÂÂÂÂÂÂÂ ret = gem_alloc_pages_array(xen_obj, size);
> > > > > > +ÂÂÂÂÂÂÂ if (ret < 0) {
> > > > > > +ÂÂÂÂÂÂÂÂÂÂÂ gem_free_pages_array(xen_obj);
> > > > > > +ÂÂÂÂÂÂÂÂÂÂÂ goto fail;
> > > > > > +ÂÂÂÂÂÂÂ }
> > > > > > +
> > > > > > +ÂÂÂÂÂÂÂ ret = alloc_xenballooned_pages(xen_obj->num_pages,
> > > > > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ xen_obj->pages);
> > > > > Why are you allocating balloon pages?
> > > > in this use-case we map pages provided by the backend
> > > > (yes, I know this can be a problem from both security
> > > > POV and that DomU can die holding pages of Dom0 forever:
> > > > but still it is a configuration option, so user decides
> > > > if her use-case needs this and takes responsibility for
> > > > such a decision).
> > > Perhaps I am missing something here but when you say "I know this can be
> > > a problem from both security POV ..." then there is something wrong with
> > > your solution.
> > well, in this scenario there are actually 2 concerns:
> > 1. If DomU dies the pages/grants from Dom0/DomD cannot be
> > reclaimed back
> > 2. Misbehaving guest may send too many requests to the
> > backend exhausting grant references and memory of Dom0/DomD
> > (this is the only concern from security POV). Please see [1]
> >
> > But, we are focusing on embedded use-cases,
> > so those systems we use are not that "dynamic" with respect to 2).
> > Namely: we have fixed number of domains and their functionality
> > is well known, so we can do rather precise assumption on resource
> > usage. This is why I try to warn on such a use-case and rely on
> > the end user who understands the caveats
>
> How will dom0/backend know whether or not to trust the front end (and
> thus whether or not to provide provide pages to it)? Will there be
> something in xenstore, for example, to indicate such trusted frontends?
Exactly, there is a dedicated xl configuration option available [1] for 
vdispl:

"be-alloc=BOOLEAN
Indicates if backend can be a buffer provider/allocator for this domain. 
See display protocol for details."

Thus, one can configure this per domain for trusted ones in corresponding
xl configuration files
> -boris
>
>
> > I'll probably add more precise description of this use-case
> > clarifying what is that security POV, so there is no confusion
> >
> > Hope this explanation answers your questions
> > > -boris
> > >
> > > > Please see description of the buffering modes in xen_drm_front.h
> > > > specifically for backend allocated buffers:
> > > >   
> > > > *******************************************************************************
> > > >
> > > >  Â * 2. Buffers allocated by the backend
> > > >   
> > > > *******************************************************************************
> > > >
> > > >  Â *
> > > >  Â * This mode of operation is run-time configured via guest domain
> > > > configuration
> > > >  Â * through XenStore entries.
> > > >  Â *
> > > >  Â * For systems which do not provide IOMMU support, but having specific
> > > >  Â * requirements for display buffers it is possible to allocate such
> > > > buffers
> > > >  Â * at backend side and share those with the frontend.
> > > >  Â * For example, if host domain is 1:1 mapped and has DRM/GPU hardware
> > > > expecting
> > > >  Â * physically contiguous memory, this allows implementing zero-copying
> > > >  Â * use-cases.
> > > >
> > > > > -boris
> > > > >
> > > > > > +ÂÂÂÂÂÂÂ if (ret < 0) {
> > > > > > +ÂÂÂÂÂÂÂÂÂÂÂ DRM_ERROR("Cannot allocate %zu ballooned pages: %d\n",
> > > > > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ xen_obj->num_pages, ret);
> > > > > > +ÂÂÂÂÂÂÂÂÂÂÂ goto fail;
> > > > > > +ÂÂÂÂÂÂÂ }
> > > > > > +
> > > > > > +ÂÂÂÂÂÂÂ return xen_obj;
> > > > > > +ÂÂÂ }
> > > > > > +ÂÂÂ /*
> > > > > > +ÂÂÂÂ * need to allocate backing pages now, so we can share those
> > > > > > +ÂÂÂÂ * with the backend
> > > > > > +ÂÂÂÂ */
> > > > > > +ÂÂÂ xen_obj->num_pages = DIV_ROUND_UP(size, PAGE_SIZE);
> > > > > > +ÂÂÂ xen_obj->pages = drm_gem_get_pages(&xen_obj->base);
> > > > > > +ÂÂÂ if (IS_ERR_OR_NULL(xen_obj->pages)) {
> > > > > > +ÂÂÂÂÂÂÂ ret = PTR_ERR(xen_obj->pages);
> > > > > > +ÂÂÂÂÂÂÂ xen_obj->pages = NULL;
> > > > > > +ÂÂÂÂÂÂÂ goto fail;
> > > > > > +ÂÂÂ }
> > > > > > +
> > > > > > +ÂÂÂ return xen_obj;
> > > > > > +
> > > > > > +fail:
> > > > > > +ÂÂÂ DRM_ERROR("Failed to allocate buffer with size %zu\n", size);
> > > > > > +ÂÂÂ return ERR_PTR(ret);
> > > > > > +}
> > > > > > +
> > Thank you,
> > Oleksandr
> >
> > [1]
> > https://lists.xenproject.org/archives/html/xen-devel/2017-07/msg03100.html
[1] https://xenbits.xen.org/docs/4.10-testing/man/xl.cfg.5.html

   Indicates if backend can be a buffer provider/allocator for this
   domain. See display protocol for details.