From patchwork Mon Feb 8 13:31:54 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andre Przywara X-Patchwork-Id: 1437658 X-Patchwork-Delegate: trini@ti.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.denx.de (client-ip=2a01:238:438b:c500:173d:9f52:ddab:ee01; helo=phobos.denx.de; envelope-from=u-boot-bounces@lists.denx.de; receiver=) Received: from phobos.denx.de (phobos.denx.de [IPv6:2a01:238:438b:c500:173d:9f52:ddab:ee01]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4DZ6Qv56vNz9sCD for ; Tue, 9 Feb 2021 00:32:55 +1100 (AEDT) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 68C1882843; Mon, 8 Feb 2021 14:32:51 +0100 (CET) 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 BE755828B3; Mon, 8 Feb 2021 14:32:49 +0100 (CET) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on phobos.denx.de X-Spam-Level: X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_MED, SPF_HELO_NONE autolearn=ham autolearn_force=no version=3.4.2 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by phobos.denx.de (Postfix) with ESMTP id 1166582829 for ; Mon, 8 Feb 2021 14:32:47 +0100 (CET) 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 2E84831B; Mon, 8 Feb 2021 05:32:46 -0800 (PST) Received: from localhost.localdomain (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 225583F73D; Mon, 8 Feb 2021 05:32:45 -0800 (PST) From: Andre Przywara To: Bin Meng , Marek Vasut Cc: Tom Rini , Mark Rutland , u-boot@lists.denx.de Subject: [RFC PATCH] nvme: Always invalidate whole cqes[] array Date: Mon, 8 Feb 2021 13:31:54 +0000 Message-Id: <20210208133154.12645-1-andre.przywara@arm.com> X-Mailer: git-send-email 2.14.1 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.102.3 at phobos.denx.de X-Virus-Status: Clean At the moment nvme_read_completion_status() tries to invidate a single member of the cqes[] array, which is shady as just a single entry is not cache line aligned. The structure is dictated by hardware, and with 16 bytes is smaller than any cache line we usually deal with. Also multiple entries need to be consecutive in memory, so we can't pad them to cover a whole cache line. As a consequence we can only always invalidate all of them - U-Boot just uses two of them anyway. This is fine, as they are only ever read by the CPU (apart from the initial zeroing), so they can't become dirty. Make this obvious by always invalidating the whole array, regardless of the entry number we are about to read. Also blow up the allocation size to cover whole cache lines, to avoid other heap allocations to sneak in. Signed-off-by: Andre Przywara Reviewed-by: Bin Meng Reviewed-by: Michael Trimarchi Tested-by: Neil Armstrong --- Hi, this is just compile tested, and should fix the only questionable cache invalidate call in this driver. Please verify if this fixes any issues! Cheers, Andre drivers/nvme/nvme.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c index 5d6331ad346..c9efeff4bc9 100644 --- a/drivers/nvme/nvme.c +++ b/drivers/nvme/nvme.c @@ -22,6 +22,8 @@ #define NVME_AQ_DEPTH 2 #define NVME_SQ_SIZE(depth) (depth * sizeof(struct nvme_command)) #define NVME_CQ_SIZE(depth) (depth * sizeof(struct nvme_completion)) +#define NVME_CQ_ALLOCATION ALIGN(NVME_CQ_SIZE(NVME_Q_DEPTH), \ + ARCH_DMA_MINALIGN) #define ADMIN_TIMEOUT 60 #define IO_TIMEOUT 30 #define MAX_PRP_POOL 512 @@ -144,8 +146,14 @@ static __le16 nvme_get_cmd_id(void) static u16 nvme_read_completion_status(struct nvme_queue *nvmeq, u16 index) { - u64 start = (ulong)&nvmeq->cqes[index]; - u64 stop = start + sizeof(struct nvme_completion); + /* + * Single CQ entries are always smaller than a cache line, so we + * can't invalidate them individually. However CQ entries are + * read only by the CPU, so it's safe to always invalidate all of them, + * as the cache line should never become dirty. + */ + ulong start = (ulong)&nvmeq->cqes[0]; + ulong stop = start + NVME_CQ_ALLOCATION; invalidate_dcache_range(start, stop); @@ -241,7 +249,7 @@ static struct nvme_queue *nvme_alloc_queue(struct nvme_dev *dev, return NULL; memset(nvmeq, 0, sizeof(*nvmeq)); - nvmeq->cqes = (void *)memalign(4096, NVME_CQ_SIZE(depth)); + nvmeq->cqes = (void *)memalign(4096, NVME_CQ_ALLOCATION); if (!nvmeq->cqes) goto free_nvmeq; memset((void *)nvmeq->cqes, 0, NVME_CQ_SIZE(depth)); @@ -339,7 +347,7 @@ static void nvme_init_queue(struct nvme_queue *nvmeq, u16 qid) nvmeq->q_db = &dev->dbs[qid * 2 * dev->db_stride]; memset((void *)nvmeq->cqes, 0, NVME_CQ_SIZE(nvmeq->q_depth)); flush_dcache_range((ulong)nvmeq->cqes, - (ulong)nvmeq->cqes + NVME_CQ_SIZE(nvmeq->q_depth)); + (ulong)nvmeq->cqes + NVME_CQ_ALLOCATION); dev->online_queues++; }