From patchwork Tue Oct 8 14:14:40 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Maarten Lankhorst X-Patchwork-Id: 3003891 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 829BBBF924 for ; Tue, 8 Oct 2013 14:15:09 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 237412014A for ; Tue, 8 Oct 2013 14:15:08 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id A58E620136 for ; Tue, 8 Oct 2013 14:15:06 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 4E159E6DE0 for ; Tue, 8 Oct 2013 07:15:06 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from youngberry.canonical.com (youngberry.canonical.com [91.189.89.112]) by gabe.freedesktop.org (Postfix) with ESMTP id E26F2E5CD5; Tue, 8 Oct 2013 07:14:46 -0700 (PDT) Received: from 5ed49945.cm-7-5c.dynamic.ziggo.nl ([94.212.153.69] helo=[192.168.1.128]) by youngberry.canonical.com with esmtpsa (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1VTY3d-0001tD-C6; Tue, 08 Oct 2013 14:14:41 +0000 Message-ID: <52541350.5060807@canonical.com> Date: Tue, 08 Oct 2013 16:14:40 +0200 From: Maarten Lankhorst User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.0 MIME-Version: 1.0 To: Thomas Hellstrom , Alex Deucher References: <20130912150645.GZ31370@twins.programming.kicks-ass.net> <5231E18D.7070306@canonical.com> <5231EF5A.7010901@vmware.com> <52323734.4070908@canonical.com> <5232A39B.5040205@vmware.com> In-Reply-To: <5232A39B.5040205@vmware.com> X-Enigmail-Version: 1.5.2 Cc: Peter Zijlstra , Daniel Vetter , intel-gfx , Linux Kernel Mailing List , dri-devel , Dave Airlie , Thomas Gleixner , Ingo Molnar Subject: [Intel-gfx] [RFC PATCH] drm/radeon: fixup locking inversion between mmap_sem and reservations X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org X-Spam-Status: No, score=-4.5 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Allocate and copy all kernel memory before doing reservations. This prevents a locking inversion between mmap_sem and reservation_class, and allows us to drop the trylocking in ttm_bo_vm_fault without upsetting lockdep. Signed-off-by: Maarten Lankhorst diff --git a/drivers/gpu/drm/radeon/r600_cs.c b/drivers/gpu/drm/radeon/r600_cs.c index 01a3ec8..efa9bca 100644 --- a/drivers/gpu/drm/radeon/r600_cs.c +++ b/drivers/gpu/drm/radeon/r600_cs.c @@ -2391,18 +2391,13 @@ int r600_cs_legacy(struct drm_device *dev, void *data, struct drm_file *filp, ib_chunk = &parser.chunks[parser.chunk_ib_idx]; parser.ib.length_dw = ib_chunk->length_dw; *l = parser.ib.length_dw; + memcpy(ib, ib_chunk->kdata, ib_chunk->length_dw * 4); r = r600_cs_parse(&parser); if (r) { DRM_ERROR("Invalid command stream !\n"); r600_cs_parser_fini(&parser, r); return r; } - r = radeon_cs_finish_pages(&parser); - if (r) { - DRM_ERROR("Invalid command stream !\n"); - r600_cs_parser_fini(&parser, r); - return r; - } r600_cs_parser_fini(&parser, r); return r; } diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index e067024..c52bb5e 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -983,12 +983,7 @@ struct radeon_cs_reloc { struct radeon_cs_chunk { uint32_t chunk_id; uint32_t length_dw; - int kpage_idx[2]; - uint32_t *kpage[2]; uint32_t *kdata; - void __user *user_ptr; - int last_copied_page; - int last_page_index; }; struct radeon_cs_parser { @@ -1027,8 +1022,13 @@ struct radeon_cs_parser { struct radeon_sa_bo *cpu_sema; }; -extern int radeon_cs_finish_pages(struct radeon_cs_parser *p); -extern u32 radeon_get_ib_value(struct radeon_cs_parser *p, int idx); +static inline u32 radeon_get_ib_value(struct radeon_cs_parser *p, int idx) +{ + struct radeon_cs_chunk *ibc = &p->chunks[p->chunk_ib_idx]; + + return ibc->kdata[idx]; +} + struct radeon_cs_packet { unsigned idx; diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c index 7d7322e..98d8898 100644 --- a/drivers/gpu/drm/radeon/radeon_cs.c +++ b/drivers/gpu/drm/radeon/radeon_cs.c @@ -217,9 +217,7 @@ int radeon_cs_parser_init(struct radeon_cs_parser *p, void *data) return -EFAULT; } p->chunks[i].length_dw = user_chunk.length_dw; - p->chunks[i].kdata = NULL; p->chunks[i].chunk_id = user_chunk.chunk_id; - p->chunks[i].user_ptr = (void __user *)(unsigned long)user_chunk.chunk_data; if (p->chunks[i].chunk_id == RADEON_CHUNK_ID_RELOCS) { p->chunk_relocs_idx = i; } @@ -242,25 +240,22 @@ int radeon_cs_parser_init(struct radeon_cs_parser *p, void *data) return -EINVAL; } - cdata = (uint32_t *)(unsigned long)user_chunk.chunk_data; - if ((p->chunks[i].chunk_id == RADEON_CHUNK_ID_RELOCS) || - (p->chunks[i].chunk_id == RADEON_CHUNK_ID_FLAGS)) { - size = p->chunks[i].length_dw * sizeof(uint32_t); - p->chunks[i].kdata = kmalloc(size, GFP_KERNEL); - if (p->chunks[i].kdata == NULL) { - return -ENOMEM; - } - if (DRM_COPY_FROM_USER(p->chunks[i].kdata, - p->chunks[i].user_ptr, size)) { - return -EFAULT; - } - if (p->chunks[i].chunk_id == RADEON_CHUNK_ID_FLAGS) { - p->cs_flags = p->chunks[i].kdata[0]; - if (p->chunks[i].length_dw > 1) - ring = p->chunks[i].kdata[1]; - if (p->chunks[i].length_dw > 2) - priority = (s32)p->chunks[i].kdata[2]; - } + size = p->chunks[i].length_dw; + p->chunks[i].kdata = drm_malloc_ab(size, sizeof(uint32_t)); + size *= sizeof(uint32_t); + if (p->chunks[i].kdata == NULL) { + return -ENOMEM; + } + cdata = (void __user *)(unsigned long)user_chunk.chunk_data; + if (DRM_COPY_FROM_USER(p->chunks[i].kdata, cdata, size)) { + return -EFAULT; + } + if (p->chunks[i].chunk_id == RADEON_CHUNK_ID_FLAGS) { + p->cs_flags = p->chunks[i].kdata[0]; + if (p->chunks[i].length_dw > 1) + ring = p->chunks[i].kdata[1]; + if (p->chunks[i].length_dw > 2) + priority = (s32)p->chunks[i].kdata[2]; } } @@ -283,34 +278,6 @@ int radeon_cs_parser_init(struct radeon_cs_parser *p, void *data) } } - /* deal with non-vm */ - if ((p->chunk_ib_idx != -1) && - ((p->cs_flags & RADEON_CS_USE_VM) == 0) && - (p->chunks[p->chunk_ib_idx].chunk_id == RADEON_CHUNK_ID_IB)) { - if (p->chunks[p->chunk_ib_idx].length_dw > (16 * 1024)) { - DRM_ERROR("cs IB too big: %d\n", - p->chunks[p->chunk_ib_idx].length_dw); - return -EINVAL; - } - if (p->rdev && (p->rdev->flags & RADEON_IS_AGP)) { - p->chunks[p->chunk_ib_idx].kpage[0] = kmalloc(PAGE_SIZE, GFP_KERNEL); - p->chunks[p->chunk_ib_idx].kpage[1] = kmalloc(PAGE_SIZE, GFP_KERNEL); - if (p->chunks[p->chunk_ib_idx].kpage[0] == NULL || - p->chunks[p->chunk_ib_idx].kpage[1] == NULL) { - kfree(p->chunks[p->chunk_ib_idx].kpage[0]); - kfree(p->chunks[p->chunk_ib_idx].kpage[1]); - p->chunks[p->chunk_ib_idx].kpage[0] = NULL; - p->chunks[p->chunk_ib_idx].kpage[1] = NULL; - return -ENOMEM; - } - } - p->chunks[p->chunk_ib_idx].kpage_idx[0] = -1; - p->chunks[p->chunk_ib_idx].kpage_idx[1] = -1; - p->chunks[p->chunk_ib_idx].last_copied_page = -1; - p->chunks[p->chunk_ib_idx].last_page_index = - ((p->chunks[p->chunk_ib_idx].length_dw * 4) - 1) / PAGE_SIZE; - } - return 0; } @@ -450,13 +417,8 @@ static void radeon_cs_parser_fini(struct radeon_cs_parser *parser, int error, bo kfree(parser->track); kfree(parser->relocs); kfree(parser->relocs_ptr); - for (i = 0; i < parser->nchunks; i++) { - kfree(parser->chunks[i].kdata); - if ((parser->rdev->flags & RADEON_IS_AGP)) { - kfree(parser->chunks[i].kpage[0]); - kfree(parser->chunks[i].kpage[1]); - } - } + for (i = 0; i < parser->nchunks; i++) + drm_free_large(parser->chunks[i].kdata); kfree(parser->chunks); kfree(parser->chunks_array); radeon_ib_free(parser->rdev, &parser->ib); @@ -483,17 +445,15 @@ static int radeon_cs_ib_chunk(struct radeon_device *rdev, DRM_ERROR("Failed to get ib !\n"); return r; } + + memcpy(parser->ib.ptr, ib_chunk->kdata, ib_chunk->length_dw * 4); parser->ib.length_dw = ib_chunk->length_dw; + r = radeon_cs_parse(rdev, parser->ring, parser); if (r || parser->parser_error) { DRM_ERROR("Invalid command stream !\n"); return r; } - r = radeon_cs_finish_pages(parser); - if (r) { - DRM_ERROR("Invalid command stream !\n"); - return r; - } if (parser->ring == R600_RING_TYPE_UVD_INDEX) radeon_uvd_note_usage(rdev); @@ -555,10 +515,8 @@ static int radeon_cs_ib_vm_chunk(struct radeon_device *rdev, parser->const_ib.is_const_ib = true; parser->const_ib.length_dw = ib_chunk->length_dw; /* Copy the packet into the IB */ - if (DRM_COPY_FROM_USER(parser->const_ib.ptr, ib_chunk->user_ptr, - ib_chunk->length_dw * 4)) { - return -EFAULT; - } + memcpy(parser->const_ib.ptr, ib_chunk->kdata, + ib_chunk->length_dw * 4); r = radeon_ring_ib_parse(rdev, parser->ring, &parser->const_ib); if (r) { return r; @@ -578,10 +536,7 @@ static int radeon_cs_ib_vm_chunk(struct radeon_device *rdev, } parser->ib.length_dw = ib_chunk->length_dw; /* Copy the packet into the IB */ - if (DRM_COPY_FROM_USER(parser->ib.ptr, ib_chunk->user_ptr, - ib_chunk->length_dw * 4)) { - return -EFAULT; - } + memcpy(parser->ib.ptr, ib_chunk->kdata, ib_chunk->length_dw * 4); r = radeon_ring_ib_parse(rdev, parser->ring, &parser->ib); if (r) { return r; @@ -704,97 +659,6 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) return r; } -int radeon_cs_finish_pages(struct radeon_cs_parser *p) -{ - struct radeon_cs_chunk *ibc = &p->chunks[p->chunk_ib_idx]; - int i; - int size = PAGE_SIZE; - - for (i = ibc->last_copied_page + 1; i <= ibc->last_page_index; i++) { - if (i == ibc->last_page_index) { - size = (ibc->length_dw * 4) % PAGE_SIZE; - if (size == 0) - size = PAGE_SIZE; - } - - if (DRM_COPY_FROM_USER(p->ib.ptr + (i * (PAGE_SIZE/4)), - ibc->user_ptr + (i * PAGE_SIZE), - size)) - return -EFAULT; - } - return 0; -} - -static int radeon_cs_update_pages(struct radeon_cs_parser *p, int pg_idx) -{ - int new_page; - struct radeon_cs_chunk *ibc = &p->chunks[p->chunk_ib_idx]; - int i; - int size = PAGE_SIZE; - bool copy1 = (p->rdev && (p->rdev->flags & RADEON_IS_AGP)) ? - false : true; - - for (i = ibc->last_copied_page + 1; i < pg_idx; i++) { - if (DRM_COPY_FROM_USER(p->ib.ptr + (i * (PAGE_SIZE/4)), - ibc->user_ptr + (i * PAGE_SIZE), - PAGE_SIZE)) { - p->parser_error = -EFAULT; - return 0; - } - } - - if (pg_idx == ibc->last_page_index) { - size = (ibc->length_dw * 4) % PAGE_SIZE; - if (size == 0) - size = PAGE_SIZE; - } - - new_page = ibc->kpage_idx[0] < ibc->kpage_idx[1] ? 0 : 1; - if (copy1) - ibc->kpage[new_page] = p->ib.ptr + (pg_idx * (PAGE_SIZE / 4)); - - if (DRM_COPY_FROM_USER(ibc->kpage[new_page], - ibc->user_ptr + (pg_idx * PAGE_SIZE), - size)) { - p->parser_error = -EFAULT; - return 0; - } - - /* copy to IB for non single case */ - if (!copy1) - memcpy((void *)(p->ib.ptr+(pg_idx*(PAGE_SIZE/4))), ibc->kpage[new_page], size); - - ibc->last_copied_page = pg_idx; - ibc->kpage_idx[new_page] = pg_idx; - - return new_page; -} - -u32 radeon_get_ib_value(struct radeon_cs_parser *p, int idx) -{ - struct radeon_cs_chunk *ibc = &p->chunks[p->chunk_ib_idx]; - u32 pg_idx, pg_offset; - u32 idx_value = 0; - int new_page; - - pg_idx = (idx * 4) / PAGE_SIZE; - pg_offset = (idx * 4) % PAGE_SIZE; - - if (ibc->kpage_idx[0] == pg_idx) - return ibc->kpage[0][pg_offset/4]; - if (ibc->kpage_idx[1] == pg_idx) - return ibc->kpage[1][pg_offset/4]; - - new_page = radeon_cs_update_pages(p, pg_idx); - if (new_page < 0) { - p->parser_error = new_page; - return 0; - } - - idx_value = ibc->kpage[new_page][pg_offset/4]; - return idx_value; -} - /** * radeon_cs_packet_parse() - parse cp packet and point ib index to next packet * @parser: parser structure holding parsing context.