From patchwork Sat May 13 09:07:28 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Wahren X-Patchwork-Id: 9725057 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 250D460146 for ; Sat, 13 May 2017 09:08:29 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 178C7288B1 for ; Sat, 13 May 2017 09:08:29 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 09B64288CB; Sat, 13 May 2017 09:08:29 +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=-1.4 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,RCVD_IN_SORBS_SPAM autolearn=no version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 2DF26288B1 for ; Sat, 13 May 2017 09:08:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Subject:References: In-Reply-To:Message-ID:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=oCMPYo6jVbA6owZ/UAr2ZRRRG0T1KgtwnuEIX/WKgVs=; b=VHyvXoHASqoTVM DUbq5uZNjHdlBjaAkzBXD8eWT3tiRP0FWGJyUukaE8qIllvcLf9u6sEeSLkwig5DHs6OV8A6GZ4Qn 5UQmGYq/CXOcVDbGHIGDCN158kyU/G2KhBG9c9pz5dfy+IgLsXku7lnAjCq44FvKyUWrE1TBeie4N I0oyKuB/oMRPUEgIZoXyW6PJzfeWD4Aqnq/MMPkTVd97rK+kddOsJh18yndAq5TwzZYSdAvUGWkYX l+uhAk3BFU4N0SZj9J/jQjhkieNeKwgiqiFZV9duuyiLRKvy1chZMLk3RBwALLav1EU4ZXpcy56lF KnYoDA4K6NjRoiR1Z2Yg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1d9T2H-0000vm-82; Sat, 13 May 2017 09:08:25 +0000 Received: from mout.kundenserver.de ([212.227.126.133]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1d9T2C-0000u3-Ac; Sat, 13 May 2017 09:08:23 +0000 Received: from oxbsltgw09.schlund.de ([172.19.249.26]) by mrelayeu.kundenserver.de (mreue001 [212.227.15.167]) with ESMTPSA (Nemesis) id 0Ly6OT-1e3gDB047p-015dqx; Sat, 13 May 2017 11:07:32 +0200 Date: Sat, 13 May 2017 11:07:28 +0200 (CEST) From: Stefan Wahren To: Russell King - ARM Linux Message-ID: <952745481.440383.1494666448727@email.1und1.de> In-Reply-To: <1381182989.263636.1493062504676@email.1und1.de> References: <331235003.222371.1491934205807@email.1und1.de> <685482264.60263.1492105308873@email.1und1.de> <20170413222915.GF17774@n2100.armlinux.org.uk> <20170414074104.GA8704@laptop> <87pog7x7c5.fsf@eliezer.anholt.net> <20170420195822.GA22677@laptop> <696fb1c1-7751-caf0-e0d9-9c67b059b4cc@i2se.com> <20170424164015.GP17774@n2100.armlinux.org.uk> <1058502523.15283.1493055747531@email.1und1.de> <20170424185931.GQ17774@n2100.armlinux.org.uk> <1381182989.263636.1493062504676@email.1und1.de> Subject: Re: [Bug] VCHIQ functional test broken MIME-Version: 1.0 X-Priority: 3 Importance: Medium X-Mailer: Open-Xchange Mailer v7.8.1-Rev34 X-Originating-Client: open-xchange-appsuite X-Provags-ID: V03:K0:dVgcOZkaxZA9pm8+vwB6ifl14I2MiJ3fXq+Sv8ttVaGWPyEX6mN Yn3cVneGv8hAT2xBdASvb4uVv3hIWC31V9NmutccgKoyrMp7wNvPwKuXxApGg+f2WAGK2xw aMFl2+l4KT96mm8GXuB6ulHH5uSZVD+j86GMDifIEJJ6gIiNjhtVen4oJwJAVnjNQlG7Z9C RSW4icJykj/xP4vm4nysg== X-UI-Out-Filterresults: notjunk:1; V01:K0:CeJEtoCPuuw=:MNHNCf+hqLqsto6OInHFZ/ MjdW0TbUNrEo/qXU9Ju0DpIDKObPUKNtb0D3chcI//F2DlXKEZsvoSJsCwL7VRWWxZew4Zgqj tt2jn0ggJmDlDJMj+rC1v0CTjgfFe0LJSeFoL6KMl6Im/qIrR7DfDo5z3KmUa7jliRtAIiQIa WSrKEJ1WOSS8GUEX6WBaoY2X6xpfpOVSZswgTj9op7gatLs1RZRIhqaeBjUuuX9B0QAqSUuyP ICT4Npl6ap+j7S47MzJUZvwZZZDDcWXlubgA6KuyBKqQ8nIVGHmAjM9ScQhQDutXyF1RY9Bqz NU+Ul1HcYwPQzueCsds4GSvJ+5M4PPCtp23fymbdbCuHEJ3BRo7+O4A39RS4VQ8MgyhwcYx35 9fKUReatSfhkcV1lVDuy5jayJfGr2cxgkoQIZyd8Ldfn/zxj+rwZgA8cpYWOp8x6XCItJJqz0 wvQmjJfGLF0pdTeo7J878wtuhC7svgbHFdKm+v/QIqFBiLJp5G0DzsCWM2GsxQMiXYcESh9fb medaIc9OpRm0/h9Gxvew7GTj0sg2XD8gsk7IKoMnfdNAywbspOMuB3AYB/leFMHXeyF3qCsOf IaOymP+HH8GumvMv3yz0CZX8lk0LcLD9pQQZNIQLEsn8Lbqh8rlL86mHb/SN5lBZruic1CRrg QbxpVQO1yB5Xo+19emhHSivfxfeH93dQQUK6BlZsAcpqM3rlsOr7nS96T3M0bzT0IelxbeOsD BkxVpAr+/94pHSUm X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20170513_020820_712289_2DD52CD0 X-CRM114-Status: GOOD ( 32.86 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: devel@driverdev.osuosl.org, Catalin Marinas , phil@raspberrypi.org, Rabin Vincent , Eric Anholt , linux-rpi-kernel@lists.infradead.org, linux-arm-kernel@lists.infradead.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP > Stefan Wahren hat am 24. April 2017 um 21:35 geschrieben: > > > > > Russell King - ARM Linux hat am 24. April 2017 um 20:59 geschrieben: > > > > > > On Mon, Apr 24, 2017 at 07:42:27PM +0200, Stefan Wahren wrote: > > > > What I can't see is how changing flush_dcache_page() has possibly broken > > > > this: it seems to imply that you're getting flush_dcache_page() somehow > > > > called inbetween mapping it for DMA, using it for DMA and CPU accesses > > > > occuring. However, the driver doesn't call flush_dcache_page() other > > > > than through get_user_pages() - and since dma_map_sg() is used in that > > > > path, it should be fine. > > > > > > > > So, I don't have much to offer. > > > > > > > > However, flush_dcache_page() is probably still a tad heavy - it currently > > > > flushes to the point of coherency, but it's really about making sure that > > > > the user vs kernel mappings are coherent, not about device coherency. > > > > That's the role of the DMA API. > > > > > > Currently i don't care too much about performance. More important is to > > > fix this regression, because i'm not able to verify the function of this > > > driver efficiently. > > > > This is a driver in staging, and the reason its in staging is because it > > has problems. This is just another example of another problem it has... > > Yes, the regression is regrettable, but I don't think it's something to > > get hung up on. > > > > > I'm thinking about 2 options: > > > > > > 1) add a force parameter to flush_dcache_page() which make it possible > > > to override the new logic > > > 2) create a second version of flush_dcache_page() with the old behavior > > > > Neither, really, because, as I've already explained, flush_dcache_page() > > has nothing what so ever to do with DMA coherence - and if a driver > > breaks because we change its semantic slightly (but still in a compliant > > way) then it's uncovered a latent bug in _that_ driver. > > > > Rather than trying to "fix" flush_dcache_page() to work how this driver > > wants it to (which is a totally crazy thing to propose - we can't go > > shoe-horning driver specific behaviour into these generic flushing > > functions), it would be much better to work out why the driver has > > broken. > > I totally agree. I had the hope we could make a workaround within the driver until we found the real issue. > > > > > I see the kernel driver has its own logging (using vchiq_log_info() etc?) > > maybe a starting point would be to compare the output from a working > > kernel with a non-working kernel to get more of an idea where the problem > > actually is? > > I will try ... > > > > > What I'm willing to do is to temporarily drop the offending patch for > > the next merge window to give more time for this driver to be debugged, > > but I will want to re-apply it after that merge window closes. > > Since this is already in 4.11, please keep it. At least this makes it easier to reproduce. No need for more confusion and effort. > In the meantime this issue has been fixed by Phil [1]. Unfortunately i found another issue. If i enable CONFIG_HIGHMEM in the kernel config, the data during functional test gets corrupted. Phil said it's caused by the usage of get_user_pages() [2]. That makes me wonder, because there are a lot of gup users and it's hard to believe that they are also affected. Would that be hard to fix? In that case i tend to fix at least the dependency: Thanks Stefan [1] - http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2017-May/105443.html [2] - https://github.com/raspberrypi/linux/issues/1996 diff --git a/drivers/staging/vc04_services/Kconfig b/drivers/staging/vc04_servic index 9e27636..6572903 100644 --- a/drivers/staging/vc04_services/Kconfig +++ b/drivers/staging/vc04_services/Kconfig @@ -2,7 +2,7 @@ menuconfig BCM_VIDEOCORE tristate "Broadcom VideoCore support" depends on HAS_DMA depends on OF - depends on RASPBERRYPI_FIRMWARE || (COMPILE_TEST && !RASPBERRYPI_FIRMWAR + depends on (RASPBERRYPI_FIRMWARE && !HIGHMEM) || (COMPILE_TEST && !RASPB default y help Support for Broadcom VideoCore services including