[1/2] spi: spi-qcom-qspi: Fallback to PIO for xfers that aren't multiples of 4 bytes
Message ID | 20230725110226.1.Ia2f980fc7cd0b831e633391f0bb1272914d8f381@changeid |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:9010:0:b0:3e4:2afc:c1 with SMTP id l16csp2661879vqg; Tue, 25 Jul 2023 11:36:52 -0700 (PDT) X-Google-Smtp-Source: APBJJlETHI10tuSmQgBCmSL7IMkTA0CaFz7dzIaN+YJri+NRAk1MdwNPtGWzdmhAVtMvP9PN92nx X-Received: by 2002:a17:90b:1648:b0:263:514:5eda with SMTP id il8-20020a17090b164800b0026305145edamr57185pjb.29.1690310212526; Tue, 25 Jul 2023 11:36:52 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1690310212; cv=none; d=google.com; s=arc-20160816; b=HgpdeSlIIDO4vJjQAac3OrNQnpR2VWkLuFmkpRIUn3W8Efhl9EvSwa9+J6cYtqRlhC SE63Rz4LglqGxIEE+d9hElR8cHoZ/sxr9W63RsiiB9N8PxA+ZHXBuNaHnytXw8sSlR1R h/bbOZ161AOSb1uyOQXgfHS6YuNdPt3r7qO3TvgEM5XYcNHFdyyw+oEJgMCS5fiJBXBr Ua8kvanvivtn1zT2CsiaPUYfalGp3vOiC8GAg7LqCbM9e3r7vCZilORmzR2P6269djxW 3JJMDz34dE+h7vMAMVWYL1m95bI8MYebLdeoE0w7UZMlTuHjSVYp1MT/1C57XZCNFout G2Yw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=A3wk98CPhmXaucfW3yFy0X9veK7c7V/LOvkjR4lZXDk=; fh=4R/7fcfjyq4vZldVemZQCIej8j1XJnUMWCJiCyhU2oY=; b=pE5wTt80X9PgiDnQBrB5SyTNaw1xYIv9w/yxEkISReUvRFBO4BufM93Ru6VvJj3yfl h1xMh5IO1PkGpzV3BClTBcT4SMZ3MMvrdf04j1w0HD5IErYqYy4lngsEG+ekMkn4/y50 WFOI/rbBU3badV8WlYpXIcVw5MVhO7IlD0/mhS4kZx2PDHZezLasFyBtXQpThEmlZ7H3 CNsLung52VABksoPlnvHFWraUtY1TkYxHNghm8Q6CW/EPSLTDoR076qezAmL6VxKLypV RgauIK/tp/mHbhx1KuhpXqcXCxGsh4Lk8qUzQY5Z3RKPg/f5PFa7VuHGNiM2M1plNigL 3uOg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=mdgqGoEy; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id me15-20020a17090b17cf00b00263fc986e28si15913189pjb.24.2023.07.25.11.36.39; Tue, 25 Jul 2023 11:36:52 -0700 (PDT) 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; Authentication-Results: mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=mdgqGoEy; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232289AbjGYSDk (ORCPT <rfc822;kautuk.consul.80@gmail.com> + 99 others); Tue, 25 Jul 2023 14:03:40 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55058 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230313AbjGYSDi (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 25 Jul 2023 14:03:38 -0400 Received: from mail-pl1-x632.google.com (mail-pl1-x632.google.com [IPv6:2607:f8b0:4864:20::632]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 611361FDE for <linux-kernel@vger.kernel.org>; Tue, 25 Jul 2023 11:03:36 -0700 (PDT) Received: by mail-pl1-x632.google.com with SMTP id d9443c01a7336-1bbc2e1c6b2so4550875ad.3 for <linux-kernel@vger.kernel.org>; Tue, 25 Jul 2023 11:03:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1690308216; x=1690913016; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=A3wk98CPhmXaucfW3yFy0X9veK7c7V/LOvkjR4lZXDk=; b=mdgqGoEyde5ufR0v0DwdbxwxdHa372uxZ0WCuZCPFnP2tWnW3cD+aRMsF3UhDkW2W8 JRNRLtjBkoPm/8J+2v8HL3qRmFef49+Bom+JO5Ec/knUI7wtUTiCn7WGM+yUu0pprK9T lf5uswnmIi0XpGMFOBgRcHhnhqrYFfANyRFJY= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690308216; x=1690913016; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=A3wk98CPhmXaucfW3yFy0X9veK7c7V/LOvkjR4lZXDk=; b=TbLYKz23yAIFe0FTjYthClu4bmY8uO5Gvu2IYnMaPz4w5wuyN2uSf4DzWjcAOYuaL2 r3GdbT9KByRUBe5X7BFwKFu2QNNuGkDHIOJur+cijzsTbQeWHBLFTqjRQegB20WIjUjD u2ZbqV2QVOOcAPQnR01pcxQVC2vaIi2mEPLcYWDa0Xk/HQwC/mEgsw6nwTLu66fWhQ0e QhTuagWh41KSQ1Cu+zXfpY41o6iZfqTE21M/2QM4IperMb7xrPpJxjLts9n+Nm2p8Z93 QFeBkY84hDuPBKLxndhhIpGAv+yQ4iVnsGeb9woHcUGVwslZa0RQVnpi9ErkKFpGzHo0 EX0Q== X-Gm-Message-State: ABy/qLZda9OtWv2hG6Pc3Y+iEJDSILaKcvgOvdaVBncBi0VVYv2+PGgj 59L9NzBDzPzFYcrlkgRxa6VXlg== X-Received: by 2002:a17:902:d487:b0:1b8:8069:d432 with SMTP id c7-20020a170902d48700b001b88069d432mr23474plg.16.1690308215805; Tue, 25 Jul 2023 11:03:35 -0700 (PDT) Received: from tictac2.mtv.corp.google.com ([2620:15c:9d:2:c363:4681:f5b8:301]) by smtp.gmail.com with ESMTPSA id jl14-20020a170903134e00b001b54a88e4a6sm11305254plb.51.2023.07.25.11.03.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 25 Jul 2023 11:03:35 -0700 (PDT) From: Douglas Anderson <dianders@chromium.org> To: Mark Brown <broonie@kernel.org> Cc: Vijaya Krishna Nivarthi <quic_vnivarth@quicinc.com>, Douglas Anderson <dianders@chromium.org>, Andy Gross <agross@kernel.org>, Bjorn Andersson <andersson@kernel.org>, Konrad Dybcio <konrad.dybcio@linaro.org>, linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-spi@vger.kernel.org Subject: [PATCH 1/2] spi: spi-qcom-qspi: Fallback to PIO for xfers that aren't multiples of 4 bytes Date: Tue, 25 Jul 2023 11:02:26 -0700 Message-ID: <20230725110226.1.Ia2f980fc7cd0b831e633391f0bb1272914d8f381@changeid> X-Mailer: git-send-email 2.41.0.487.g6d72f3e995-goog MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1772418721145286497 X-GMAIL-MSGID: 1772418721145286497 |
Series |
[1/2] spi: spi-qcom-qspi: Fallback to PIO for xfers that aren't multiples of 4 bytes
|
|
Commit Message
Doug Anderson
July 25, 2023, 6:02 p.m. UTC
The Qualcomm QSPI driver appears to require that any reads using DMA
are a mutliple of 4 bytes. If this isn't true then the controller will
clobber any extra bytes in memory following the last word. Let's
detect this and falback to PIO.
This fixes problems reported by slub_debug=FZPUA, which would complain
about "kmalloc Redzone overwritten". One such instance said:
0xffffff80c29d541a-0xffffff80c29d541b @offset=21530. First byte 0x0 instead of 0xcc
Allocated in mtd_kmalloc_up_to+0x98/0xac age=36 cpu=3 pid=6658
Tracing through what was happening I saw that, while we often did DMA
tranfers of 0x1000 bytes, sometimes we'd end up doing ones of 0x41a
bytes. Those 0x41a byte transfers were the problem.
NOTE: a future change will enable the SPI "mem ops" to help avoid this
case, but it still seems good to add the extra check in the transfer.
Fixes: b5762d95607e ("spi: spi-qcom-qspi: Add DMA mode support")
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
drivers/spi/spi-qcom-qspi.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
Comments
On Tue, Jul 25, 2023 at 11:02:27AM -0700, Douglas Anderson wrote: > In the patch ("spi: spi-qcom-qspi: Fallback to PIO for xfers that > aren't multiples of 4 bytes") we detect reads that we can't handle > properly and fallback to PIO mode. While that's correct behavior, we > can do better by adding "spi_controller_mem_ops" for our > controller. Once we do this then the caller will give us a transfer > that's a multiple of 4-bytes so we can DMA. This is more of an optimisation for the case where we're using flash - if someone has hung some other hardware off the controller (which seems reasonable enough if they don't need it for flash) then we'll not use the mem_ops.
Hi, On Tue, Jul 25, 2023 at 11:24 AM Mark Brown <broonie@kernel.org> wrote: > > On Tue, Jul 25, 2023 at 11:02:27AM -0700, Douglas Anderson wrote: > > In the patch ("spi: spi-qcom-qspi: Fallback to PIO for xfers that > > aren't multiples of 4 bytes") we detect reads that we can't handle > > properly and fallback to PIO mode. While that's correct behavior, we > > can do better by adding "spi_controller_mem_ops" for our > > controller. Once we do this then the caller will give us a transfer > > that's a multiple of 4-bytes so we can DMA. > > This is more of an optimisation for the case where we're using flash - > if someone has hung some other hardware off the controller (which seems > reasonable enough if they don't need it for flash) then we'll not use > the mem_ops. Right. That's why I also have the first patch in the series too. The first patch ensures correctness and the second patch makes things more optimal for when we're using flash. Do you want me to re-submit the patch with wording that makes this more obvious? Note that it's pretty likely someone wouldn't use this SPI controller for anything other than SPI flash. While it's theoretically possible, the controller is intended for SPI flash (supports dual and quad SPI modes) and is only half duplex. -Doug
On Tue, Jul 25, 2023 at 11:30:30AM -0700, Doug Anderson wrote: > Note that it's pretty likely someone wouldn't use this SPI controller > for anything other than SPI flash. While it's theoretically possible, > the controller is intended for SPI flash (supports dual and quad SPI > modes) and is only half duplex. TBH most devices are half duplex so it's not *that* big a restriction, and dual/quad mode are obviously attractive if you need to transfer large quantities of data.
On Tue, Jul 25, 2023 at 11:02:26AM -0700, Douglas Anderson wrote: > The Qualcomm QSPI driver appears to require that any reads using DMA > are a mutliple of 4 bytes. If this isn't true then the controller will > clobber any extra bytes in memory following the last word. Let's > detect this and falback to PIO. > > This fixes problems reported by slub_debug=FZPUA, which would complain > about "kmalloc Redzone overwritten". One such instance said: > > 0xffffff80c29d541a-0xffffff80c29d541b @offset=21530. First byte 0x0 instead of 0xcc > Allocated in mtd_kmalloc_up_to+0x98/0xac age=36 cpu=3 pid=6658 > > Tracing through what was happening I saw that, while we often did DMA > tranfers of 0x1000 bytes, sometimes we'd end up doing ones of 0x41a > bytes. Those 0x41a byte transfers were the problem. > > NOTE: a future change will enable the SPI "mem ops" to help avoid this > case, but it still seems good to add the extra check in the transfer. > > Fixes: b5762d95607e ("spi: spi-qcom-qspi: Add DMA mode support") > Signed-off-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Bjorn Andersson <andersson@kernel.org> Regards, Bjorn
On 7/25/2023 11:32 PM, Douglas Anderson wrote: > The Qualcomm QSPI driver appears to require that any reads using DMA > are a mutliple of 4 bytes. If this isn't true then the controller will > clobber any extra bytes in memory following the last word. Let's > detect this and falback to PIO. > > This fixes problems reported by slub_debug=FZPUA, which would complain > about "kmalloc Redzone overwritten". One such instance said: > > 0xffffff80c29d541a-0xffffff80c29d541b @offset=21530. First byte 0x0 instead of 0xcc > Allocated in mtd_kmalloc_up_to+0x98/0xac age=36 cpu=3 pid=6658 > > Tracing through what was happening I saw that, while we often did DMA > tranfers of 0x1000 bytes, sometimes we'd end up doing ones of 0x41a > bytes. Those 0x41a byte transfers were the problem. > > NOTE: a future change will enable the SPI "mem ops" to help avoid this > case, but it still seems good to add the extra check in the transfer. > > Fixes: b5762d95607e ("spi: spi-qcom-qspi: Add DMA mode support") > Signed-off-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Vijaya Krishna Nivarthi <quic_vnivarth@quicinc.com> Thank you for the fix, Vijay/ > --- > > drivers/spi/spi-qcom-qspi.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/spi/spi-qcom-qspi.c b/drivers/spi/spi-qcom-qspi.c > index a0ad9802b606..39b4d8a8107a 100644 > --- a/drivers/spi/spi-qcom-qspi.c > +++ b/drivers/spi/spi-qcom-qspi.c > @@ -355,10 +355,22 @@ static int qcom_qspi_setup_dma_desc(struct qcom_qspi *ctrl, > > for (i = 0; i < sgt->nents; i++) { > dma_ptr_sg = sg_dma_address(sgt->sgl + i); > + dma_len_sg = sg_dma_len(sgt->sgl + i); > if (!IS_ALIGNED(dma_ptr_sg, QSPI_ALIGN_REQ)) { > dev_warn_once(ctrl->dev, "dma_address not aligned to %d\n", QSPI_ALIGN_REQ); > return -EAGAIN; > } > + /* > + * When reading with DMA the controller writes to memory 1 word > + * at a time. If the length isn't a multiple of 4 bytes then > + * the controller can clobber the things later in memory. > + * Fallback to PIO to be safe. > + */ > + if (ctrl->xfer.dir == QSPI_READ && (dma_len_sg & 0x03)) { > + dev_warn_once(ctrl->dev, "fallback to PIO for read of size %#010x\n", > + dma_len_sg); > + return -EAGAIN; > + } > } > > for (i = 0; i < sgt->nents; i++) {
On Tue, 25 Jul 2023 11:02:26 -0700, Douglas Anderson wrote: > The Qualcomm QSPI driver appears to require that any reads using DMA > are a mutliple of 4 bytes. If this isn't true then the controller will > clobber any extra bytes in memory following the last word. Let's > detect this and falback to PIO. > > This fixes problems reported by slub_debug=FZPUA, which would complain > about "kmalloc Redzone overwritten". One such instance said: > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next Thanks! [1/2] spi: spi-qcom-qspi: Fallback to PIO for xfers that aren't multiples of 4 bytes commit: 138d73b627c71bf2b2f61502dc6c1137b9656434 [2/2] spi: spi-qcom-qspi: Add mem_ops to avoid PIO for badly sized reads commit: cc71c42b3dc1085d3e72dfa5603e827b9eb59da1 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
diff --git a/drivers/spi/spi-qcom-qspi.c b/drivers/spi/spi-qcom-qspi.c index a0ad9802b606..39b4d8a8107a 100644 --- a/drivers/spi/spi-qcom-qspi.c +++ b/drivers/spi/spi-qcom-qspi.c @@ -355,10 +355,22 @@ static int qcom_qspi_setup_dma_desc(struct qcom_qspi *ctrl, for (i = 0; i < sgt->nents; i++) { dma_ptr_sg = sg_dma_address(sgt->sgl + i); + dma_len_sg = sg_dma_len(sgt->sgl + i); if (!IS_ALIGNED(dma_ptr_sg, QSPI_ALIGN_REQ)) { dev_warn_once(ctrl->dev, "dma_address not aligned to %d\n", QSPI_ALIGN_REQ); return -EAGAIN; } + /* + * When reading with DMA the controller writes to memory 1 word + * at a time. If the length isn't a multiple of 4 bytes then + * the controller can clobber the things later in memory. + * Fallback to PIO to be safe. + */ + if (ctrl->xfer.dir == QSPI_READ && (dma_len_sg & 0x03)) { + dev_warn_once(ctrl->dev, "fallback to PIO for read of size %#010x\n", + dma_len_sg); + return -EAGAIN; + } } for (i = 0; i < sgt->nents; i++) {