From patchwork Tue Dec 20 16:50:28 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Konrad Rzeszutek Wilk X-Patchwork-Id: 9481835 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id BC7C4601C0 for ; Tue, 20 Dec 2016 16:51:14 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id AC7B421BED for ; Tue, 20 Dec 2016 16:51:14 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id A08F928346; Tue, 20 Dec 2016 16:51:14 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 787A327F8C for ; Tue, 20 Dec 2016 16:51:13 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 880806E9F5; Tue, 20 Dec 2016 16:51:06 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from aserp1040.oracle.com (aserp1040.oracle.com [141.146.126.69]) by gabe.freedesktop.org (Postfix) with ESMTPS id 03AAB6E9F5; Tue, 20 Dec 2016 16:51:04 +0000 (UTC) Received: from userv0021.oracle.com (userv0021.oracle.com [156.151.31.71]) by aserp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id uBKGoUNM015974 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 20 Dec 2016 16:50:31 GMT Received: from userv0122.oracle.com (userv0122.oracle.com [156.151.31.75]) by userv0021.oracle.com (8.14.4/8.14.4) with ESMTP id uBKGoUqW022335 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 20 Dec 2016 16:50:30 GMT Received: from abhmp0007.oracle.com (abhmp0007.oracle.com [141.146.116.13]) by userv0122.oracle.com (8.14.4/8.14.4) with ESMTP id uBKGoT2E008414; Tue, 20 Dec 2016 16:50:29 GMT Received: from char.us.oracle.com (/10.137.176.158) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Tue, 20 Dec 2016 08:50:29 -0800 Received: by char.us.oracle.com (Postfix, from userid 1000) id 288876A00EB; Tue, 20 Dec 2016 11:50:28 -0500 (EST) Date: Tue, 20 Dec 2016 11:50:28 -0500 From: Konrad Rzeszutek Wilk To: Juergen Gross Subject: Re: i915 regression in kernel 4.10 Message-ID: <20161220165028.GE22440@char.us.oracle.com> References: <7abf8559-3aa7-af3a-8dc1-1dee42019fcd@suse.com> <20161219122934.GM29871@nuc-i3427.alporthouse.com> <3de0be86-c0bc-6bfd-defa-745b589d7bd9@suse.com> <20161220144246.GA23668@char.us.oracle.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20161220144246.GA23668@char.us.oracle.com> User-Agent: Mutt/1.7.1 (2016-10-04) X-Source-IP: userv0021.oracle.com [156.151.31.71] Cc: intel-gfx , Linux Kernel Mailing List , dri-devel@lists.freedesktop.org, daniel.vetter@intel.com, Boris Ostrovsky X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP On Tue, Dec 20, 2016 at 09:42:46AM -0500, Konrad Rzeszutek Wilk wrote: > On Mon, Dec 19, 2016 at 03:16:44PM +0100, Juergen Gross wrote: > > On 19/12/16 13:29, Chris Wilson wrote: > > > On Mon, Dec 19, 2016 at 12:39:16PM +0100, Juergen Gross wrote: > > >> With recent 4.10 kernel the graphics isn't coming up under Xen. First > > >> failure message is: > > >> > > >> [ 46.656649] i915 0000:00:02.0: swiotlb buffer is full (sz: 1630208 bytes) > > > > > > Do we get a silent failure? i915_gem_gtt_prepare_pages() is where we > > > call dma_map_sg() and pass the sg to swiotlb (in this case) for > > > remapping, and we do check for an error value of 0. After that error, > > > SWIOTLB_MAP_ERROR is propagated back and converted to 0 for > > > dma_map_sg(). That looks valid, and we should report ENOMEM back to the > > > caller. > > > > > >> Later I see splats like: > > >> > > >> [ 49.393583] general protection fault: 0000 [#1] SMP > > > > > > What was the faulting address? RAX is particularly non-pointer-like so I > > > wonder if we walked onto an uninitialised portion of the sgtable. We may > > > have tripped over a bug in our sg_page iterator. > > > > During the bisect process there have been either GP or NULL pointer > > dereferences or other page faults. Typical addresses where: > > > > xen_swiotlb_unmap_sg_attrs+0x1f/0x50: access to 0000000000000018 > > xen_swiotlb_unmap_sg_attrs+0x1f/0x50: access to 0000000003020118 > > > > > > > > The attached patch should prevent an early ENOMEM following the swiotlb > > > allocation failure. But I suspect that we will still be tripping up the > > > failure in the sg walker when binding to the GPU. > > > -Chris > > > > > > > The patch is working not too bad. :-) > > > > Still several "swiotlb buffer is full" messages (some with sz:, most > > without), but no faults any more (neither GP nor NULL pointer > > dereference). Graphical login is working now. > > > I think I know why. The optimization that was added assumes that > bus addresses is the same as physical address. Hence it packs all > of the virtual addresses in the sg, and hands it off to SWIOTLB > which walks each one and realizes that it has to use the bounce > buffer. > > I am wondering if would make sense to pull 'swiotlb_max_size' inside > of SWIOTLB and make it an library-ish - so Xen-SWIOTLB can register > as well and report say that it can only provide one page > (unless it is running under baremtal). > > Or make the usage of 'max_segement' and 'page_to_pfn(page) != last_pfn + 1' > in i915_gem_object_Get_pages_gtt use something similar to xen_biovec_phys_mergeable? Chris, I was thinking of something like this (which Juergen has already tested). From f196f1294fd25f1402c3dd1231babb8d7f5db2e7 Mon Sep 17 00:00:00 2001 From: Konrad Rzeszutek Wilk Date: Tue, 20 Dec 2016 10:02:02 -0500 Subject: [PATCH] swiotlb: Export swiotlb_max_segment to users So they can figure out what is the optimal number of pages that can be contingously stitched together without fear of bounce buffer. We also expose an mechanism for sub-users of SWIOTLB API, such as Xen-SWIOTLB to set the max segment value. And lastly if swiotlb=force is set (which mandates we bounce buffer everything) we set max_segment so at least we can bounce buffer one 4K page instead of a giant 512KB one for which we may not have space. Reported-and-Tested-by: Juergen Gross Signed-off-by: Konrad Rzeszutek Wilk --- drivers/gpu/drm/i915/i915_gem.c | 11 +---------- drivers/xen/swiotlb-xen.c | 4 ++++ include/linux/swiotlb.h | 3 +++ lib/swiotlb.c | 26 ++++++++++++++++++++++++++ 4 files changed, 34 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index d0dcaf3..115fa39 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2290,15 +2290,6 @@ void __i915_gem_object_put_pages(struct drm_i915_gem_object *obj, mutex_unlock(&obj->mm.lock); } -static unsigned int swiotlb_max_size(void) -{ -#if IS_ENABLED(CONFIG_SWIOTLB) - return rounddown(swiotlb_nr_tbl() << IO_TLB_SHIFT, PAGE_SIZE); -#else - return 0; -#endif -} - static void i915_sg_trim(struct sg_table *orig_st) { struct sg_table new_st; @@ -2345,7 +2336,7 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj) GEM_BUG_ON(obj->base.read_domains & I915_GEM_GPU_DOMAINS); GEM_BUG_ON(obj->base.write_domain & I915_GEM_GPU_DOMAINS); - max_segment = swiotlb_max_size(); + max_segment = swiotlb_max_segment(); if (!max_segment) max_segment = rounddown(UINT_MAX, PAGE_SIZE); diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c index aba1200..f905d6e 100644 --- a/drivers/xen/swiotlb-xen.c +++ b/drivers/xen/swiotlb-xen.c @@ -275,6 +275,10 @@ int __ref xen_swiotlb_init(int verbose, bool early) rc = 0; } else rc = swiotlb_late_init_with_tbl(xen_io_tlb_start, xen_io_tlb_nslabs); + + if (!rc) + swiotlb_set_max_segment(PAGE_SIZE); + return rc; error: if (repeat--) { diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h index d9c84a9..d7eacd8 100644 --- a/include/linux/swiotlb.h +++ b/include/linux/swiotlb.h @@ -114,11 +114,14 @@ swiotlb_dma_supported(struct device *hwdev, u64 mask); #ifdef CONFIG_SWIOTLB extern void __init swiotlb_free(void); +unsigned int swiotlb_max_segment(void); #else static inline void swiotlb_free(void) { } +unsigned int swiotlb_max_segment(void) { return 0; } #endif extern void swiotlb_print_info(void); extern int is_swiotlb_buffer(phys_addr_t paddr); +extern void swiotlb_set_max_segment(unsigned int); #endif /* __LINUX_SWIOTLB_H */ diff --git a/lib/swiotlb.c b/lib/swiotlb.c index 9def738..975b8fc 100644 --- a/lib/swiotlb.c +++ b/lib/swiotlb.c @@ -83,6 +83,12 @@ static unsigned int *io_tlb_list; static unsigned int io_tlb_index; /* + * Max segment that we can provide which (if pages are contingous) will + * not be bounced (unless SWIOTLB_FORCE is set). + */ +unsigned int max_segment; + +/* * We need to save away the original address corresponding to a mapped entry * for the sync operations. */ @@ -124,6 +130,20 @@ unsigned long swiotlb_nr_tbl(void) } EXPORT_SYMBOL_GPL(swiotlb_nr_tbl); +unsigned int swiotlb_max_segment(void) +{ + return max_segment; +} +EXPORT_SYMBOL_GPL(swiotlb_max_segment); + +void swiotlb_set_max_segment(unsigned int val) +{ + if (swiotlb_force == SWIOTLB_FORCE) + max_segment = 1; + else + max_segment = rounddown(val, PAGE_SIZE); +} + /* default to 64MB */ #define IO_TLB_DEFAULT_SIZE (64UL<<20) unsigned long swiotlb_size_or_default(void) @@ -205,6 +225,7 @@ int __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose) if (verbose) swiotlb_print_info(); + swiotlb_set_max_segment(io_tlb_nslabs << IO_TLB_SHIFT); return 0; } @@ -283,6 +304,7 @@ swiotlb_late_init_with_default_size(size_t default_size) rc = swiotlb_late_init_with_tbl(vstart, io_tlb_nslabs); if (rc) free_pages((unsigned long)vstart, order); + return rc; } @@ -337,6 +359,8 @@ swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs) late_alloc = 1; + swiotlb_set_max_segment(io_tlb_nslabs << IO_TLB_SHIFT); + return 0; cleanup4: @@ -351,6 +375,7 @@ swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs) io_tlb_end = 0; io_tlb_start = 0; io_tlb_nslabs = 0; + max_segment = 0; return -ENOMEM; } @@ -379,6 +404,7 @@ void __init swiotlb_free(void) PAGE_ALIGN(io_tlb_nslabs << IO_TLB_SHIFT)); } io_tlb_nslabs = 0; + max_segment = 0; } int is_swiotlb_buffer(phys_addr_t paddr)