From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, MENTIONS_GIT_HOSTING,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E0D2CC433E4 for ; Mon, 24 Aug 2020 06:56:16 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 77FE12067C for ; Mon, 24 Aug 2020 06:56:16 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=tcd-ie.20150623.gappssmtp.com header.i=@tcd-ie.20150623.gappssmtp.com header.b="ph3cmZei" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 77FE12067C Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=tcd.ie Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 8E5FD6EC7B; Mon, 24 Aug 2020 06:55:44 +0000 (UTC) Received: from mail-il1-x144.google.com (mail-il1-x144.google.com [IPv6:2607:f8b0:4864:20::144]) by gabe.freedesktop.org (Postfix) with ESMTPS id 2B3736E195 for ; Mon, 24 Aug 2020 00:04:46 +0000 (UTC) Received: by mail-il1-x144.google.com with SMTP id f75so5514901ilh.3 for ; Sun, 23 Aug 2020 17:04:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tcd-ie.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=2qvzvkep7ZlQHNyjrGsuXQAp+x7s+RjdNiOv81NxtoQ=; b=ph3cmZeijsQTsHnHnrr9eCGgxMPexaLwCGnu3F9Od9T7oE9SJ1xriseSecoaLR+A31 cxdttU99z8vHmtpSvs9wLGOOgPOY6GzwuKuovZh4UMjplaxKJu/wyAtKKGV99cTnuUCc b9jW667DJd78gpOSqL093TQrnCvZ9zpW4sDgc9e3s4gqdpHbIh0cqwIbSVLaQrTLqDAX thER0vKrZAtqkOJvYQeGZuITMeh4kPZ6AWC1kOvIEmnWvuxZr35lD2OOnlXMpkqB5GBo XlqFU7D/ggoXmysuTUVEiDWk1jvnBEMzHRuwpliqu/K2HYrSEPvB6WXxqYArt4mYifKa QdMw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=2qvzvkep7ZlQHNyjrGsuXQAp+x7s+RjdNiOv81NxtoQ=; b=dFussfB1KImtMoRNUKNALQH+tcIBM7PgfBmS+8CAyoCcs57wCNhtoBHqlsETFdAQ3A LTY3RryWzXE+pzVVweU0oRqmoY7ISY/aiQks4XKGaaV0FHeHMpQV50K/PL6E2LeUF0jW BcYNwDenftF1u7QesMHo/R1iyzY4hqNub8qLgnp6jW9e1CydO+WXzuxYfhSLg+pk7F2z i1gGv17NDkuV5I6YaNtPLmnbQDH3zblrP5rX/Emou+GgwPgQtEq1cLX3lQv3AcCtdDgS MWHYhJ8GSJjb8IhZl28ua4OvXE+CZRihwki66vQ6xU+idahQgsh3Bt00xnVeDR3Nz4Qg fsuQ== X-Gm-Message-State: AOAM533YpFvUA4cdUuSFAeo1GvuaFEzg9L2/k6yvPExRbOfRNxrGOhR6 a7oOmm5AtPT0BLxftbHvI9r8h1nwjQkg1rNjZ3DBUg== X-Google-Smtp-Source: ABdhPJx+Rb9Dh5afCkPV93ubbfa83YINwHuyBNL6aGGIC9gNFt/GVdQQ5DqMV2LQPpqLPd5cGrS+QMzugsAJ9rHE3h4= X-Received: by 2002:a92:4f:: with SMTP id 76mr2640989ila.11.1598227486317; Sun, 23 Aug 2020 17:04:46 -0700 (PDT) MIME-Version: 1.0 References: <20191221150402.13868-1-murphyt7@tcd.ie> <465815ae-9292-f37a-59b9-03949cb68460@deltatee.com> <20200529124523.GA11817@infradead.org> <33137cfb-603c-86e8-1091-f36117ecfaf3@deltatee.com> In-Reply-To: From: Tom Murphy Date: Mon, 24 Aug 2020 01:04:35 +0100 Message-ID: Subject: Re: [PATCH 0/8] Convert the intel iommu driver to the dma-iommu api To: Logan Gunthorpe X-Mailman-Approved-At: Mon, 24 Aug 2020 06:55:41 +0000 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kvm@vger.kernel.org, David Airlie , dri-devel@lists.freedesktop.org, Bjorn Andersson , linux-tegra@vger.kernel.org, Julien Grall , Thierry Reding , Will Deacon , Marek Szyprowski , Jean-Philippe Brucker , linux-samsung-soc@vger.kernel.org, Marc Zyngier , Krzysztof Kozlowski , Jonathan Hunter , Christoph Hellwig , linux-rockchip@lists.infradead.org, Andy Gross , linux-arm-kernel@lists.infradead.org, linux-s390@vger.kernel.org, linux-arm-msm@vger.kernel.org, intel-gfx@lists.freedesktop.org, Alex Williamson , linux-mediatek@lists.infradead.org, Rodrigo Vivi , Matthias Brugger , Thomas Gleixner , virtualization@lists.linux-foundation.org, Gerald Schaefer , David Woodhouse , Cornelia Huck , Linux Kernel Mailing List , iommu@lists.linux-foundation.org, Kukjin Kim , Robin Murphy Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Hi Logan/All, I have added a check for the sg_dma_len == 0 : """ } __sgt_iter(struct scatterlist *sgl, bool dma) { struct sgt_iter s = { .sgp = sgl }; + if (sgl && sg_dma_len(sgl) == 0) + s.sgp = NULL; if (s.sgp) { ..... """ at location [1]. but it doens't fix the problem. You're right though, this change does need to be made, this code doesn't handle pages of sg_dma_len(sg) == 0 correctly So my guess is that we have more bugs in other parts of the i915 driver (or there is a problem with my "sg_dma_len == 0" fix above). I have been trying to spot where else the code might be buggy but I haven't had any luck so far. I'm doing a microconfernce (at LPC 2020) this wednesdays [1] on this if you're interested in attending. I'm hoping I can chat about it with a few people and find how can reproduce and fix this issues. I don't have any more time I can give to this unfortunately and it would be a shame for the work to go to waste. [0] https://github.com/torvalds/linux/blob/d012a7190fc1fd72ed48911e77ca97ba4521bccd/drivers/gpu/drm/i915/i915_scatterlist.h#L28 [1] https://linuxplumbersconf.org/event/7/contributions/846/ On Fri, 29 May 2020 at 22:21, Logan Gunthorpe wrote: > > > > On 2020-05-29 3:11 p.m., Marek Szyprowski wrote: > > Patches are pending: > > https://lore.kernel.org/linux-iommu/20200513132114.6046-1-m.szyprowski@samsung.com/T/ > > Cool, nice! Though, I still don't think that fixes the issue in > i915_scatterlist.h given it still ignores sg_dma_len() and strictly > relies on sg_next()/sg_is_last() to stop iterating -- and I suspect this > is the bug that got in Tom's way. > > >> However, as Robin pointed out, there are other ugly tricks like stopping > >> iterating through the SGL when sg_dma_len() is zero. For example, the > >> AMD driver appears to use drm_prime_sg_to_page_addr_arrays() which does > >> this trick and thus likely isn't buggy (otherwise, I'd expect someone to > >> have complained by now seeing AMD has already switched to IOMMU-DMA. > > > > I'm not sure that this is a trick. Stopping at zero sg_dma_len() was > > somewhere documented. > > Well whatever you want to call it, it is ugly to have some drivers doing > one thing with the returned value and others assuming there's an extra > zero at the end. It just causes confusion for people reading/copying the > code. It would be better if they are all consistent. However, I concede > stopping at zero should not be broken, presently. > > Logan _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel