linux-kernel - Re: [PATCH] drm/i915: Call dma_set_max_seg_size() in i915_ggtt_probe

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <c90348c9b569bebe2efbf21c53c155756f592988.camel@redhat.com>
Date:   Thu, 22 Aug 2019 16:08:32 -0400
From:   Lyude Paul <lyude@...hat.com>
To:     intel-gfx@...ts.freedesktop.org
Cc:     Chris Wilson <chris@...is-wilson.co.uk>, stable@...r.kernel.org,
        Jani Nikula <jani.nikula@...ux.intel.com>,
        Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
        Rodrigo Vivi <rodrigo.vivi@...el.com>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/i915: Call dma_set_max_seg_size() in
 i915_ggtt_probe_hw()

...whoops.

Ignore this patch-Chris Wilson had requested that I send a patch along with
this one to enable CONFIG_DMA_API_DEBUG_SG in CI, but I completely forgot to
do that before sending. Will send out a reroll with that in just a moment

On Thu, 2019-08-22 at 16:04 -0400, Lyude Paul wrote:
> Currently, we don't call dma_set_max_seg_size() for i915 because we
> intentionally do not limit the segment length that the device supports.
> However, this results in a warning being emitted if we try to map
> anything larger than SZ_64K on a kernel with CONFIG_DMA_API_DEBUG_SG
> enabled:
> 
> [    7.751926] DMA-API: i915 0000:00:02.0: mapping sg segment longer
> than device claims to support [len=98304] [max=65536]
> [    7.751934] WARNING: CPU: 5 PID: 474 at kernel/dma/debug.c:1220
> debug_dma_map_sg+0x20f/0x340
> 
> This was originally brought up on
> https://bugs.freedesktop.org/show_bug.cgi?id=108517 , and the consensus
> there was it wasn't really useful to set a limit (and that dma-debug
> isn't really all that useful for i915 in the first place). Unfortunately
> though, CONFIG_DMA_API_DEBUG_SG is enabled in the debug configs for
> various distro kernels. Since a WARN_ON() will disable automatic problem
> reporting (and cause any CI with said option enabled to start
> complaining), we really should just fix the problem.
> 
> Note that as me and Chris Wilson discussed, the other solution for this
> would be to make DMA-API not make such assumptions when a driver hasn't
> explicitly set a maximum segment size. But, taking a look at the commit
> which originally introduced this behavior, commit 78c47830a5cb
> ("dma-debug: check scatterlist segments"), there is an explicit mention
> of this assumption and how it applies to devices with no segment size:
> 
> 	Conversely, devices which are less limited than the rather
> 	conservative defaults, or indeed have no limitations at all
> 	(e.g. GPUs with their own internal MMU), should be encouraged to
> 	set appropriate dma_parms, as they may get more efficient DMA
> 	mapping performance out of it.
> 
> So unless there's any concerns (I'm open to discussion!), let's just
> follow suite and call dma_set_max_seg_size() with UINT_MAX as our limit
> to silence any warnings.
> 
> Signed-off-by: Lyude Paul <lyude@...hat.com>
> Cc: Chris Wilson <chris@...is-wilson.co.uk>
> Cc: <stable@...r.kernel.org> # v4.18+
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 0b81e0b64393..a1475039d182 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -3152,6 +3152,11 @@ static int ggtt_probe_hw(struct i915_ggtt *ggtt,
> struct intel_gt *gt)
>  	if (ret)
>  		return ret;
>  
> +	/* We don't have a max segment size, so set it to the max so sg's
> +	 * debugging layer doesn't complain
> +	 */
> +	dma_set_max_seg_size(ggtt->vm.dma, UINT_MAX);
> +
>  	if ((ggtt->vm.total - 1) >> 32) {
>  		DRM_ERROR("We never expected a Global GTT with more than
> 32bits"
>  			  " of address space! Found %lldM!\n",
-- 
Cheers,
	Lyude Paul