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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 84E1EC433F5 for ; Thu, 30 Sep 2021 16:09:44 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A0173613A9 for ; Thu, 30 Sep 2021 16:09:43 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org A0173613A9 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 36DF880F3B; Thu, 30 Sep 2021 18:09:41 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: by phobos.denx.de (Postfix, from userid 109) id 673A080F6A; Thu, 30 Sep 2021 18:09:39 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by phobos.denx.de (Postfix) with ESMTP id 1B23480644 for ; Thu, 30 Sep 2021 18:09:36 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=andre.przywara@arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 480A3101E; Thu, 30 Sep 2021 09:09:35 -0700 (PDT) Received: from donnerap.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 468433F70D; Thu, 30 Sep 2021 09:09:34 -0700 (PDT) Date: Thu, 30 Sep 2021 17:09:10 +0100 From: Andre Przywara To: Stefan Agner Cc: u-boot@lists.denx.de, bmeng.cn@gmail.com, nsaenz@kernel.org, mbrugger@suse.com, m.szyprowski@samsung.com, s.nawrocki@samsung.com, Tom Rini Subject: Re: [PATCH] nvme: invalidate correct memory range after read Message-ID: <20210930170910.2d6dd7b7@donnerap.cambridge.arm.com> In-Reply-To: References: Organization: ARM X-Mailer: Claws Mail 3.17.3 (GTK+ 2.24.32; aarch64-unknown-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean On Tue, 28 Sep 2021 10:01:40 +0200 Stefan Agner wrote: > The current code invalidates the range after the read buffer since the > buffer pointer gets incremented in the read loop. Use a temporary > pointer to make sure we have a pristine pointer to invalidate the > correct memory range after read. Ah, good catch! This looks good, just one nit below: > Fixes: 704e040a51d2 ("nvme: Apply cache operations on the DMA buffers") > Signed-off-by: Stefan Agner > --- > > drivers/nvme/nvme.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c > index f6465ea7f4..354513ad30 100644 > --- a/drivers/nvme/nvme.c > +++ b/drivers/nvme/nvme.c > @@ -743,6 +743,7 @@ static ulong nvme_blk_rw(struct udevice *udev, lbaint_t blknr, > u64 prp2; > u64 total_len = blkcnt << desc->log2blksz; > u64 temp_len = total_len; > + void *temp_buffer = buffer; You could make this an unsigned long (or better uintptr_t), then lose all the casts below and avoid the void ptr arithmetic. But it works either way, so: Reviewed-by: Andre Przywara Cheers, Andre > > u64 slba = blknr; > u16 lbas = 1 << (dev->max_transfer_shift - ns->lba_shift); > @@ -770,19 +771,19 @@ static ulong nvme_blk_rw(struct udevice *udev, lbaint_t blknr, > } > > if (nvme_setup_prps(dev, &prp2, > - lbas << ns->lba_shift, (ulong)buffer)) > + lbas << ns->lba_shift, (ulong)temp_buffer)) > return -EIO; > c.rw.slba = cpu_to_le64(slba); > slba += lbas; > c.rw.length = cpu_to_le16(lbas - 1); > - c.rw.prp1 = cpu_to_le64((ulong)buffer); > + c.rw.prp1 = cpu_to_le64((ulong)temp_buffer); > c.rw.prp2 = cpu_to_le64(prp2); > status = nvme_submit_sync_cmd(dev->queues[NVME_IO_Q], > &c, NULL, IO_TIMEOUT); > if (status) > break; > temp_len -= (u32)lbas << ns->lba_shift; > - buffer += lbas << ns->lba_shift; > + temp_buffer += lbas << ns->lba_shift; > } > > if (read)