From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752036AbeB0Gwh (ORCPT ); Tue, 27 Feb 2018 01:52:37 -0500 Received: from mail-lf0-f65.google.com ([209.85.215.65]:40460 "EHLO mail-lf0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751128AbeB0Gwg (ORCPT ); Tue, 27 Feb 2018 01:52:36 -0500 X-Google-Smtp-Source: AG47ELsmKRrQZDQGi5w7s8tZ7HXa3sU9snIzt0KnvCkKD3mkk7Kylix+U0TmI7pSg4JxwN8BJtIk0g== Subject: Re: [PATCH 8/9] drm/xen-front: Implement GEM operations To: Boris Ostrovsky , Oleksandr Andrushchenko , xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, airlied@linux.ie, daniel.vetter@intel.com, seanpaul@chromium.org, gustavo@padovan.org, jgross@suse.com, konrad.wilk@oracle.com References: <1519200222-20623-1-git-send-email-andr2000@gmail.com> <1519200222-20623-9-git-send-email-andr2000@gmail.com> <2f2c6fea-c0cb-e244-41f3-269db07986fc@oracle.com> <56c4a78b-356a-fb35-a97e-187581ae45ad@epam.com> <71ab9d03-dc07-f7f2-c9f8-463cc926e573@oracle.com> From: Oleksandr Andrushchenko Message-ID: Date: Tue, 27 Feb 2018 08:52:32 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <71ab9d03-dc07-f7f2-c9f8-463cc926e573@oracle.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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: >>>> +static struct xen_gem_object *gem_create(struct drm_device *dev, >>>> size_t size) >>>> +{ >>>> + struct xen_drm_front_drm_info *drm_info = dev->dev_private; >>>> + struct xen_gem_object *xen_obj; >>>> + int ret; >>>> + >>>> + size = round_up(size, PAGE_SIZE); >>>> + xen_obj = gem_create_obj(dev, size); >>>> + if (IS_ERR_OR_NULL(xen_obj)) >>>> + return xen_obj; >>>> + >>>> + if (drm_info->cfg->be_alloc) { >>>> + /* >>>> + * backend will allocate space for this buffer, so >>>> + * only allocate array of pointers to pages >>>> + */ >>>> + xen_obj->be_alloc = true; >>> If be_alloc is a flag (which I am not sure about) --- should it be set >>> to true *after* you've successfully allocated your things? >> this is a configuration option telling about the way >> the buffer gets allocated: either by the frontend or >> backend (be_alloc -> buffer allocated by the backend) > > I can see how drm_info->cfg->be_alloc might be a configuration option > but xen_obj->be_alloc is set here and that's not how configuration > options typically behave. you are right, I will put be_alloc down the code and will slightly rework error handling for this function > >>>> + 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 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Oleksandr Andrushchenko Subject: Re: [PATCH 8/9] drm/xen-front: Implement GEM operations Date: Tue, 27 Feb 2018 08:52:32 +0200 Message-ID: References: <1519200222-20623-1-git-send-email-andr2000@gmail.com> <1519200222-20623-9-git-send-email-andr2000@gmail.com> <2f2c6fea-c0cb-e244-41f3-269db07986fc@oracle.com> <56c4a78b-356a-fb35-a97e-187581ae45ad@epam.com> <71ab9d03-dc07-f7f2-c9f8-463cc926e573@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: Received: from mail-lf0-x244.google.com (mail-lf0-x244.google.com [IPv6:2a00:1450:4010:c07::244]) by gabe.freedesktop.org (Postfix) with ESMTPS id 230186E5D0 for ; Tue, 27 Feb 2018 06:52:36 +0000 (UTC) Received: by mail-lf0-x244.google.com with SMTP id m69so25906246lfe.8 for ; Mon, 26 Feb 2018 22:52:35 -0800 (PST) In-Reply-To: <71ab9d03-dc07-f7f2-c9f8-463cc926e573@oracle.com> Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Boris Ostrovsky , Oleksandr Andrushchenko , xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, airlied@linux.ie, daniel.vetter@intel.com, seanpaul@chromium.org, gustavo@padovan.org, jgross@suse.com, konrad.wilk@oracle.com List-Id: dri-devel@lists.freedesktop.org T24gMDIvMjcvMjAxOCAwMTo0NyBBTSwgQm9yaXMgT3N0cm92c2t5IHdyb3RlOgo+IE9uIDAyLzIz LzIwMTggMTA6MzUgQU0sIE9sZWtzYW5kciBBbmRydXNoY2hlbmtvIHdyb3RlOgo+PiBPbiAwMi8y My8yMDE4IDA1OjI2IFBNLCBCb3JpcyBPc3Ryb3Zza3kgd3JvdGU6Cj4+PiBPbiAwMi8yMS8yMDE4 IDAzOjAzIEFNLCBPbGVrc2FuZHIgQW5kcnVzaGNoZW5rbyB3cm90ZToKPj4+PiArc3RhdGljIHN0 cnVjdCB4ZW5fZ2VtX29iamVjdCAqZ2VtX2NyZWF0ZShzdHJ1Y3QgZHJtX2RldmljZSAqZGV2LAo+ Pj4+IHNpemVfdCBzaXplKQo+Pj4+ICt7Cj4+Pj4gKyAgICBzdHJ1Y3QgeGVuX2RybV9mcm9udF9k cm1faW5mbyAqZHJtX2luZm8gPSBkZXYtPmRldl9wcml2YXRlOwo+Pj4+ICsgICAgc3RydWN0IHhl bl9nZW1fb2JqZWN0ICp4ZW5fb2JqOwo+Pj4+ICsgICAgaW50IHJldDsKPj4+PiArCj4+Pj4gKyAg ICBzaXplID0gcm91bmRfdXAoc2l6ZSwgUEFHRV9TSVpFKTsKPj4+PiArICAgIHhlbl9vYmogPSBn ZW1fY3JlYXRlX29iaihkZXYsIHNpemUpOwo+Pj4+ICsgICAgaWYgKElTX0VSUl9PUl9OVUxMKHhl bl9vYmopKQo+Pj4+ICsgICAgICAgIHJldHVybiB4ZW5fb2JqOwo+Pj4+ICsKPj4+PiArICAgIGlm IChkcm1faW5mby0+Y2ZnLT5iZV9hbGxvYykgewo+Pj4+ICsgICAgICAgIC8qCj4+Pj4gKyAgICAg ICAgICogYmFja2VuZCB3aWxsIGFsbG9jYXRlIHNwYWNlIGZvciB0aGlzIGJ1ZmZlciwgc28KPj4+ PiArICAgICAgICAgKiBvbmx5IGFsbG9jYXRlIGFycmF5IG9mIHBvaW50ZXJzIHRvIHBhZ2VzCj4+ Pj4gKyAgICAgICAgICovCj4+Pj4gKyAgICAgICAgeGVuX29iai0+YmVfYWxsb2MgPSB0cnVlOwo+ Pj4gSWYgYmVfYWxsb2MgaXMgYSBmbGFnICh3aGljaCBJIGFtIG5vdCBzdXJlIGFib3V0KSAtLS0g c2hvdWxkIGl0IGJlIHNldAo+Pj4gdG8gdHJ1ZSAqYWZ0ZXIqIHlvdSd2ZSBzdWNjZXNzZnVsbHkg YWxsb2NhdGVkIHlvdXIgdGhpbmdzPwo+PiB0aGlzIGlzIGEgY29uZmlndXJhdGlvbiBvcHRpb24g dGVsbGluZyBhYm91dCB0aGUgd2F5Cj4+IHRoZSBidWZmZXIgZ2V0cyBhbGxvY2F0ZWQ6IGVpdGhl ciBieSB0aGUgZnJvbnRlbmQgb3IKPj4gYmFja2VuZCAoYmVfYWxsb2MgLT4gYnVmZmVyIGFsbG9j YXRlZCBieSB0aGUgYmFja2VuZCkKPgo+IEkgY2FuIHNlZSBob3cgZHJtX2luZm8tPmNmZy0+YmVf YWxsb2MgbWlnaHQgYmUgYSBjb25maWd1cmF0aW9uIG9wdGlvbgo+IGJ1dCB4ZW5fb2JqLT5iZV9h bGxvYyBpcyBzZXQgaGVyZSBhbmQgdGhhdCdzIG5vdCBob3cgY29uZmlndXJhdGlvbgo+IG9wdGlv bnMgdHlwaWNhbGx5IGJlaGF2ZS4KeW91IGFyZSByaWdodCwgSSB3aWxsIHB1dCBiZV9hbGxvYyBk b3duIHRoZSBjb2RlIGFuZCB3aWxsIHNsaWdodGx5CnJld29yayBlcnJvciBoYW5kbGluZyBmb3Ig dGhpcyBmdW5jdGlvbgo+Cj4+Pj4gKyAgICAgICAgcmV0ID0gZ2VtX2FsbG9jX3BhZ2VzX2FycmF5 KHhlbl9vYmosIHNpemUpOwo+Pj4+ICsgICAgICAgIGlmIChyZXQgPCAwKSB7Cj4+Pj4gKyAgICAg ICAgICAgIGdlbV9mcmVlX3BhZ2VzX2FycmF5KHhlbl9vYmopOwo+Pj4+ICsgICAgICAgICAgICBn b3RvIGZhaWw7Cj4+Pj4gKyAgICAgICAgfQo+Pj4+ICsKPj4+PiArICAgICAgICByZXQgPSBhbGxv Y194ZW5iYWxsb29uZWRfcGFnZXMoeGVuX29iai0+bnVtX3BhZ2VzLAo+Pj4+ICsgICAgICAgICAg ICAgICAgeGVuX29iai0+cGFnZXMpOwo+Pj4gV2h5IGFyZSB5b3UgYWxsb2NhdGluZyBiYWxsb29u IHBhZ2VzPwo+PiBpbiB0aGlzIHVzZS1jYXNlIHdlIG1hcCBwYWdlcyBwcm92aWRlZCBieSB0aGUg YmFja2VuZAo+PiAoeWVzLCBJIGtub3cgdGhpcyBjYW4gYmUgYSBwcm9ibGVtIGZyb20gYm90aCBz ZWN1cml0eQo+PiBQT1YgYW5kIHRoYXQgRG9tVSBjYW4gZGllIGhvbGRpbmcgcGFnZXMgb2YgRG9t MCBmb3JldmVyOgo+PiBidXQgc3RpbGwgaXQgaXMgYSBjb25maWd1cmF0aW9uIG9wdGlvbiwgc28g dXNlciBkZWNpZGVzCj4+IGlmIGhlciB1c2UtY2FzZSBuZWVkcyB0aGlzIGFuZCB0YWtlcyByZXNw b25zaWJpbGl0eSBmb3IKPj4gc3VjaCBhIGRlY2lzaW9uKS4KPgo+IFBlcmhhcHMgSSBhbSBtaXNz aW5nIHNvbWV0aGluZyBoZXJlIGJ1dCB3aGVuIHlvdSBzYXkgIkkga25vdyB0aGlzIGNhbiBiZQo+ IGEgcHJvYmxlbSBmcm9tIGJvdGggc2VjdXJpdHkgUE9WIC4uLiIgdGhlbiB0aGVyZSBpcyBzb21l dGhpbmcgd3Jvbmcgd2l0aAo+IHlvdXIgc29sdXRpb24uCndlbGwsIGluIHRoaXMgc2NlbmFyaW8g dGhlcmUgYXJlIGFjdHVhbGx5IDIgY29uY2VybnM6CjEuIElmIERvbVUgZGllcyB0aGUgcGFnZXMv Z3JhbnRzIGZyb20gRG9tMC9Eb21EIGNhbm5vdCBiZQpyZWNsYWltZWQgYmFjawoyLiBNaXNiZWhh dmluZyBndWVzdCBtYXkgc2VuZCB0b28gbWFueSByZXF1ZXN0cyB0byB0aGUKYmFja2VuZCBleGhh dXN0aW5nIGdyYW50IHJlZmVyZW5jZXMgYW5kIG1lbW9yeSBvZiBEb20wL0RvbUQKKHRoaXMgaXMg dGhlIG9ubHkgY29uY2VybiBmcm9tIHNlY3VyaXR5IFBPVikuIFBsZWFzZSBzZWUgWzFdCgpCdXQs IHdlIGFyZSBmb2N1c2luZyBvbiBlbWJlZGRlZCB1c2UtY2FzZXMsCnNvIHRob3NlIHN5c3RlbXMg d2UgdXNlIGFyZSBub3QgdGhhdCAiZHluYW1pYyIgd2l0aCByZXNwZWN0IHRvIDIpLgpOYW1lbHk6 IHdlIGhhdmUgZml4ZWQgbnVtYmVyIG9mIGRvbWFpbnMgYW5kIHRoZWlyIGZ1bmN0aW9uYWxpdHkK aXMgd2VsbCBrbm93biwgc28gd2UgY2FuIGRvIHJhdGhlciBwcmVjaXNlIGFzc3VtcHRpb24gb24g cmVzb3VyY2UKdXNhZ2UuIFRoaXMgaXMgd2h5IEkgdHJ5IHRvIHdhcm4gb24gc3VjaCBhIHVzZS1j YXNlIGFuZCByZWx5IG9uCnRoZSBlbmQgdXNlciB3aG8gdW5kZXJzdGFuZHMgdGhlIGNhdmVhdHMK CkknbGwgcHJvYmFibHkgYWRkIG1vcmUgcHJlY2lzZSBkZXNjcmlwdGlvbiBvZiB0aGlzIHVzZS1j YXNlCmNsYXJpZnlpbmcgd2hhdCBpcyB0aGF0IHNlY3VyaXR5IFBPViwgc28gdGhlcmUgaXMgbm8g Y29uZnVzaW9uCgpIb3BlIHRoaXMgZXhwbGFuYXRpb24gYW5zd2VycyB5b3VyIHF1ZXN0aW9ucwo+ IC1ib3Jpcwo+Cj4+IFBsZWFzZSBzZWUgZGVzY3JpcHRpb24gb2YgdGhlIGJ1ZmZlcmluZyBtb2Rl cyBpbiB4ZW5fZHJtX2Zyb250LmgKPj4gc3BlY2lmaWNhbGx5IGZvciBiYWNrZW5kIGFsbG9jYXRl ZCBidWZmZXJzOgo+PiAgICoqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioq KioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioKPj4KPj4gICAqIDIuIEJ1ZmZl cnMgYWxsb2NhdGVkIGJ5IHRoZSBiYWNrZW5kCj4+ICAgKioqKioqKioqKioqKioqKioqKioqKioq KioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKgo+ Pgo+PiAgICoKPj4gICAqIFRoaXMgbW9kZSBvZiBvcGVyYXRpb24gaXMgcnVuLXRpbWUgY29uZmln dXJlZCB2aWEgZ3Vlc3QgZG9tYWluCj4+IGNvbmZpZ3VyYXRpb24KPj4gICAqIHRocm91Z2ggWGVu U3RvcmUgZW50cmllcy4KPj4gICAqCj4+ICAgKiBGb3Igc3lzdGVtcyB3aGljaCBkbyBub3QgcHJv dmlkZSBJT01NVSBzdXBwb3J0LCBidXQgaGF2aW5nIHNwZWNpZmljCj4+ICAgKiByZXF1aXJlbWVu dHMgZm9yIGRpc3BsYXkgYnVmZmVycyBpdCBpcyBwb3NzaWJsZSB0byBhbGxvY2F0ZSBzdWNoCj4+ IGJ1ZmZlcnMKPj4gICAqIGF0IGJhY2tlbmQgc2lkZSBhbmQgc2hhcmUgdGhvc2Ugd2l0aCB0aGUg ZnJvbnRlbmQuCj4+ICAgKiBGb3IgZXhhbXBsZSwgaWYgaG9zdCBkb21haW4gaXMgMToxIG1hcHBl ZCBhbmQgaGFzIERSTS9HUFUgaGFyZHdhcmUKPj4gZXhwZWN0aW5nCj4+ICAgKiBwaHlzaWNhbGx5 IGNvbnRpZ3VvdXMgbWVtb3J5LCB0aGlzIGFsbG93cyBpbXBsZW1lbnRpbmcgemVyby1jb3B5aW5n Cj4+ICAgKiB1c2UtY2FzZXMuCj4+Cj4+PiAtYm9yaXMKPj4+Cj4+Pj4gKyAgICAgICAgaWYgKHJl dCA8IDApIHsKPj4+PiArICAgICAgICAgICAgRFJNX0VSUk9SKCJDYW5ub3QgYWxsb2NhdGUgJXp1 IGJhbGxvb25lZCBwYWdlczogJWRcbiIsCj4+Pj4gKyAgICAgICAgICAgICAgICAgICAgeGVuX29i ai0+bnVtX3BhZ2VzLCByZXQpOwo+Pj4+ICsgICAgICAgICAgICBnb3RvIGZhaWw7Cj4+Pj4gKyAg ICAgICAgfQo+Pj4+ICsKPj4+PiArICAgICAgICByZXR1cm4geGVuX29iajsKPj4+PiArICAgIH0K Pj4+PiArICAgIC8qCj4+Pj4gKyAgICAgKiBuZWVkIHRvIGFsbG9jYXRlIGJhY2tpbmcgcGFnZXMg bm93LCBzbyB3ZSBjYW4gc2hhcmUgdGhvc2UKPj4+PiArICAgICAqIHdpdGggdGhlIGJhY2tlbmQK Pj4+PiArICAgICAqLwo+Pj4+ICsgICAgeGVuX29iai0+bnVtX3BhZ2VzID0gRElWX1JPVU5EX1VQ KHNpemUsIFBBR0VfU0laRSk7Cj4+Pj4gKyAgICB4ZW5fb2JqLT5wYWdlcyA9IGRybV9nZW1fZ2V0 X3BhZ2VzKCZ4ZW5fb2JqLT5iYXNlKTsKPj4+PiArICAgIGlmIChJU19FUlJfT1JfTlVMTCh4ZW5f b2JqLT5wYWdlcykpIHsKPj4+PiArICAgICAgICByZXQgPSBQVFJfRVJSKHhlbl9vYmotPnBhZ2Vz KTsKPj4+PiArICAgICAgICB4ZW5fb2JqLT5wYWdlcyA9IE5VTEw7Cj4+Pj4gKyAgICAgICAgZ290 byBmYWlsOwo+Pj4+ICsgICAgfQo+Pj4+ICsKPj4+PiArICAgIHJldHVybiB4ZW5fb2JqOwo+Pj4+ ICsKPj4+PiArZmFpbDoKPj4+PiArICAgIERSTV9FUlJPUigiRmFpbGVkIHRvIGFsbG9jYXRlIGJ1 ZmZlciB3aXRoIHNpemUgJXp1XG4iLCBzaXplKTsKPj4+PiArICAgIHJldHVybiBFUlJfUFRSKHJl dCk7Cj4+Pj4gK30KPj4+PiArCj4+Pj4KVGhhbmsgeW91LApPbGVrc2FuZHIKClsxXSAKaHR0cHM6 Ly9saXN0cy54ZW5wcm9qZWN0Lm9yZy9hcmNoaXZlcy9odG1sL3hlbi1kZXZlbC8yMDE3LTA3L21z ZzAzMTAwLmh0bWwKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3Jn Cmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVs Cg==