From: Mark Rutland <mark.rutland@arm.com> To: Arnd Bergmann <arnd@arndb.de> Cc: Will Deacon <will@kernel.org>, emma@anholt.net, mripard@kernel.org, Marek Szyprowski <m.szyprowski@samsung.com>, linux-arm-kernel@lists.infradead.org, kernel-team@android.com, Catalin Marinas <catalin.marinas@arm.com>, Ard Biesheuvel <ardb@kernel.org>, Vincent Whitchurch <vincent.whitchurch@axis.com>, Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>, dri-devel <dri-devel@lists.freedesktop.org> Subject: Re: [PATCH] arm64: cache: Lower ARCH_DMA_MINALIGN to 64 (L1_CACHE_BYTES) Date: Mon, 7 Jun 2021 16:32:47 +0100 [thread overview] Message-ID: <20210607153247.GA6716@C02TD0UTHF1T.local> (raw) In-Reply-To: <CAK8P3a0sj0qtC0VpQv4+Ah-C8jyZaGgfqsx326mChuW+e5mvrQ@mail.gmail.com> On Mon, Jun 07, 2021 at 03:57:41PM +0200, Arnd Bergmann wrote: > On Mon, Jun 7, 2021 at 3:39 PM Will Deacon <will@kernel.org> wrote: > > > > [Adding VC4 folks -- please see the KASAN splat below!] > > > > Background here is that reducing ARCH_DMA_MINALIGN to 64 on arm64 (queued in > > -next) is causing vc4 to hang on Rpi3b due to a probable driver bug. > > The great news for the patch that caused it is that this has nothing to > do with DMA alignment. > > > On Mon, Jun 07, 2021 at 02:08:59PM +0100, Mark Rutland wrote: > > > On Mon, Jun 07, 2021 at 01:01:18PM +0100, Mark Rutland wrote: > > > > On Mon, Jun 07, 2021 at 11:58:32AM +0200, Marek Szyprowski wrote: > > > > [ 3.728042] BUG: KASAN: slab-out-of-bounds in vc4_atomic_commit_tail+0x1cc/0x910 > > > [ 3.728123] Read of size 8 at addr ffff000007360440 by task kworker/u8:0/7 > > This is offset 0x40 into struct vc4_hvs_state, which is the > 'pending_commit' pointer > for the array index 4, i.e. one after the end of the structure. > > > > [ 3.728495] kasan_report+0x1dc/0x240 > > > [ 3.728529] __asan_load8+0x98/0xd4 > > > [ 3.728565] vc4_atomic_commit_tail+0x1cc/0x910 > > It seems to be this loop: > > for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) { > struct vc4_crtc_state *vc4_crtc_state = > to_vc4_crtc_state(old_crtc_state); > unsigned int channel = vc4_crtc_state->assigned_channel; > int ret; > > if (channel == VC4_HVS_CHANNEL_DISABLED) > continue; > > if (!old_hvs_state->fifo_state[channel].in_use) > continue; > > ret = > drm_crtc_commit_wait(old_hvs_state->fifo_state[i].pending_commit); > if (ret) > drm_err(dev, "Timed out waiting for commit\n"); > } > > I notice that it checks index 'fifos_state[channel].in_use', but then > uses a different index 'i' for looking at the 'pending_commit' field > beyond the end of the array. FWIW, with that drm_crtc_commit_wait() call changed to: | ret = drm_crtc_commit_wait(old_hvs_state->fifo_state[channel].pending_commit); ... I no longer see a KASAN splat, and I no longer see a hang with ARCH_DMA_MINALIGN reduced to 64. Thanks, Mark. > > This code was introduced by Maxime Ripard in commit 9ec03d7f1ed3 > ("drm/vc4: kms: Wait on previous FIFO users before a commit"). > > Arnd _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: Mark Rutland <mark.rutland@arm.com> To: Arnd Bergmann <arnd@arndb.de> Cc: emma@anholt.net, Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>, Will Deacon <will@kernel.org>, Vincent Whitchurch <vincent.whitchurch@axis.com>, dri-devel <dri-devel@lists.freedesktop.org>, Catalin Marinas <catalin.marinas@arm.com>, kernel-team@android.com, Ard Biesheuvel <ardb@kernel.org>, linux-arm-kernel@lists.infradead.org, Marek Szyprowski <m.szyprowski@samsung.com> Subject: Re: [PATCH] arm64: cache: Lower ARCH_DMA_MINALIGN to 64 (L1_CACHE_BYTES) Date: Mon, 7 Jun 2021 16:32:47 +0100 [thread overview] Message-ID: <20210607153247.GA6716@C02TD0UTHF1T.local> (raw) In-Reply-To: <CAK8P3a0sj0qtC0VpQv4+Ah-C8jyZaGgfqsx326mChuW+e5mvrQ@mail.gmail.com> On Mon, Jun 07, 2021 at 03:57:41PM +0200, Arnd Bergmann wrote: > On Mon, Jun 7, 2021 at 3:39 PM Will Deacon <will@kernel.org> wrote: > > > > [Adding VC4 folks -- please see the KASAN splat below!] > > > > Background here is that reducing ARCH_DMA_MINALIGN to 64 on arm64 (queued in > > -next) is causing vc4 to hang on Rpi3b due to a probable driver bug. > > The great news for the patch that caused it is that this has nothing to > do with DMA alignment. > > > On Mon, Jun 07, 2021 at 02:08:59PM +0100, Mark Rutland wrote: > > > On Mon, Jun 07, 2021 at 01:01:18PM +0100, Mark Rutland wrote: > > > > On Mon, Jun 07, 2021 at 11:58:32AM +0200, Marek Szyprowski wrote: > > > > [ 3.728042] BUG: KASAN: slab-out-of-bounds in vc4_atomic_commit_tail+0x1cc/0x910 > > > [ 3.728123] Read of size 8 at addr ffff000007360440 by task kworker/u8:0/7 > > This is offset 0x40 into struct vc4_hvs_state, which is the > 'pending_commit' pointer > for the array index 4, i.e. one after the end of the structure. > > > > [ 3.728495] kasan_report+0x1dc/0x240 > > > [ 3.728529] __asan_load8+0x98/0xd4 > > > [ 3.728565] vc4_atomic_commit_tail+0x1cc/0x910 > > It seems to be this loop: > > for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) { > struct vc4_crtc_state *vc4_crtc_state = > to_vc4_crtc_state(old_crtc_state); > unsigned int channel = vc4_crtc_state->assigned_channel; > int ret; > > if (channel == VC4_HVS_CHANNEL_DISABLED) > continue; > > if (!old_hvs_state->fifo_state[channel].in_use) > continue; > > ret = > drm_crtc_commit_wait(old_hvs_state->fifo_state[i].pending_commit); > if (ret) > drm_err(dev, "Timed out waiting for commit\n"); > } > > I notice that it checks index 'fifos_state[channel].in_use', but then > uses a different index 'i' for looking at the 'pending_commit' field > beyond the end of the array. FWIW, with that drm_crtc_commit_wait() call changed to: | ret = drm_crtc_commit_wait(old_hvs_state->fifo_state[channel].pending_commit); ... I no longer see a KASAN splat, and I no longer see a hang with ARCH_DMA_MINALIGN reduced to 64. Thanks, Mark. > > This code was introduced by Maxime Ripard in commit 9ec03d7f1ed3 > ("drm/vc4: kms: Wait on previous FIFO users before a commit"). > > Arnd
next prev parent reply other threads:[~2021-06-07 15:34 UTC|newest] Thread overview: 73+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-05-27 12:43 [PATCH] arm64: cache: Lower ARCH_DMA_MINALIGN to 64 (L1_CACHE_BYTES) Will Deacon 2021-05-27 13:11 ` Catalin Marinas 2021-05-27 13:19 ` Mark Rutland 2021-05-28 9:35 ` Arnd Bergmann 2021-06-01 10:14 ` Catalin Marinas 2021-05-31 5:38 ` Ard Biesheuvel 2021-06-01 18:21 ` Will Deacon [not found] ` <CGME20210602132541eucas1p17127696041c26c00d1d2f50bef9cfaf0@eucas1p1.samsung.com> 2021-06-02 13:25 ` Marek Szyprowski 2021-06-02 13:51 ` Mark Rutland 2021-06-02 14:09 ` Marek Szyprowski 2021-06-02 14:14 ` Arnd Bergmann 2021-06-02 14:28 ` Marek Szyprowski 2021-06-02 14:52 ` Arnd Bergmann 2021-06-07 12:17 ` Arnd Bergmann 2021-06-04 10:01 ` Mark Rutland 2021-06-07 9:58 ` Marek Szyprowski 2021-06-07 12:01 ` Mark Rutland 2021-06-07 13:08 ` Mark Rutland 2021-06-07 13:39 ` Will Deacon 2021-06-07 13:39 ` Will Deacon 2021-06-07 13:56 ` Mark Rutland 2021-06-07 13:56 ` Mark Rutland 2021-06-07 13:57 ` Arnd Bergmann 2021-06-07 13:57 ` Arnd Bergmann 2021-06-07 15:17 ` Maxime Ripard 2021-06-07 15:17 ` Maxime Ripard 2021-06-07 15:50 ` Arnd Bergmann 2021-06-07 15:50 ` Arnd Bergmann 2021-06-08 8:57 ` Mark Rutland 2021-06-08 8:57 ` Mark Rutland 2021-06-07 15:32 ` Mark Rutland [this message] 2021-06-07 15:32 ` Mark Rutland 2021-06-02 14:11 ` Arnd Bergmann 2021-06-02 14:15 ` Marek Szyprowski 2021-07-06 9:26 ` Yassine Oudjana 2021-07-06 10:26 ` Catalin Marinas 2021-07-06 10:26 ` Catalin Marinas 2021-07-06 13:29 ` Robin Murphy 2021-07-06 13:29 ` Robin Murphy 2021-07-06 13:33 ` Will Deacon 2021-07-06 13:33 ` Will Deacon 2021-07-06 13:44 ` Marc Zyngier 2021-07-06 13:44 ` Marc Zyngier 2021-07-06 14:21 ` Robin Murphy 2021-07-06 14:21 ` Robin Murphy 2021-07-06 14:30 ` Arnd Bergmann 2021-07-06 14:30 ` Arnd Bergmann 2021-07-06 14:46 ` Marc Zyngier 2021-07-06 14:46 ` Marc Zyngier 2021-07-06 15:43 ` Arnd Bergmann 2021-07-06 15:43 ` Arnd Bergmann 2021-07-06 17:15 ` Yassine Oudjana 2021-07-06 17:15 ` Yassine Oudjana 2021-07-06 20:33 ` Arnd Bergmann 2021-07-06 20:33 ` Arnd Bergmann 2021-07-06 22:27 ` Bjorn Andersson 2021-07-06 22:27 ` Bjorn Andersson 2021-07-07 9:27 ` Will Deacon 2021-07-07 9:27 ` Will Deacon 2021-07-07 8:24 ` Yassine Oudjana 2021-07-07 8:24 ` Yassine Oudjana 2021-07-07 9:29 ` Arnd Bergmann 2021-07-07 9:29 ` Arnd Bergmann 2021-07-07 14:41 ` Jeffrey Hugo 2021-07-07 14:41 ` Jeffrey Hugo 2021-07-08 20:59 ` Jeffrey Hugo 2021-07-08 20:59 ` Jeffrey Hugo 2021-07-09 8:48 ` Will Deacon 2021-07-09 8:48 ` Will Deacon 2021-07-09 17:10 ` Catalin Marinas 2021-07-09 17:10 ` Catalin Marinas 2021-07-06 16:20 ` Will Deacon 2021-07-06 16:20 ` Will Deacon
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20210607153247.GA6716@C02TD0UTHF1T.local \ --to=mark.rutland@arm.com \ --cc=ardb@kernel.org \ --cc=arnd@arndb.de \ --cc=b.zolnierkie@samsung.com \ --cc=catalin.marinas@arm.com \ --cc=dri-devel@lists.freedesktop.org \ --cc=emma@anholt.net \ --cc=kernel-team@android.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=m.szyprowski@samsung.com \ --cc=mripard@kernel.org \ --cc=vincent.whitchurch@axis.com \ --cc=will@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.