Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <a71371c5-8b56-a8a6-64eb-180180edfb89@linux.intel.com>
Date:   Fri, 22 Jul 2022 11:13:59 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [PATCH] drm/i915: stop using swiotlb
Content-Language: en-US
To:     Christoph Hellwig <hch@lst.de>
Cc:     Robert Beckett <bob.beckett@collabora.com>,
        Jani Nikula <jani.nikula@linux.intel.com>,
        Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
        Rodrigo Vivi <rodrigo.vivi@intel.com>,
        David Airlie <airlied@linux.ie>,
        Daniel Vetter <daniel@ffwll.ch>, kernel@collabora.com,
        Thomas Hellstrom <thomas.hellstrom@intel.com>,
        Matthew Auld <matthew.auld@intel.com>,
        intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
        linux-kernel@vger.kernel.org
References: <20220721174307.1085741-1-bob.beckett@collabora.com>
 <a0a7a734-d3dd-960e-f130-39f86b04b24d@linux.intel.com>
 <20220722095859.GB14113@lst.de>
From:   Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Organization: Intel Corporation UK Plc
In-Reply-To: <20220722095859.GB14113@lst.de>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 22/07/2022 10:58, Christoph Hellwig wrote:
> On Fri, Jul 22, 2022 at 10:45:54AM +0100, Tvrtko Ursulin wrote:
>>> -	unsigned int size = swiotlb_max_segment();
>>> -
>>> -	if (size == 0)
>>> -		size = UINT_MAX;
>>
>> On a more detailed look, there was a CI failure which makes me think this
>> cap might need to stay. Because max sg segment is unsigned int. So I wonder
>> if sg contstruction overflows without it.
>>
>> If this quick analysis is right, you could leave i915_sg_segment_size
>> helper and cap the return from dma_max_mapping_size to UINT_MAX in it.
> 
> As dma_max_mapping_size retuns a size_t it would be good to make
> all variables using it a size_t as well.  In places where that gets
> lower to an unsigned int your probably want this cap.

Yep. Problem we have is struct scatterlist length field, which is 
unsigned int, since all our backing storage handling is built on top of 
it. Therefore I think capping in this helper should be good.

Regards,

Tvrtko