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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9A48CC46467 for ; Sun, 8 Jan 2023 00:08:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231546AbjAHAIT (ORCPT ); Sat, 7 Jan 2023 19:08:19 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39362 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229627AbjAHAIQ (ORCPT ); Sat, 7 Jan 2023 19:08:16 -0500 Received: from out5-smtp.messagingengine.com (out5-smtp.messagingengine.com [66.111.4.29]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 10F271EEE4; Sat, 7 Jan 2023 16:08:14 -0800 (PST) Received: from compute6.internal (compute6.nyi.internal [10.202.2.47]) by mailout.nyi.internal (Postfix) with ESMTP id D339E5C007D; Sat, 7 Jan 2023 19:08:10 -0500 (EST) Received: from imap51 ([10.202.2.101]) by compute6.internal (MEProxy); Sat, 07 Jan 2023 19:08:10 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=arndb.de; h=cc :cc:content-type:date:date:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to; s=fm2; t=1673136490; x=1673222890; bh=w6+aDVc5tr 2uIs4oPoROJd4fXpjR6GDvnU6vXcUl8+E=; b=cp64SsHubAH99xcKXc31E50owr wlsfwwUfDFasgRONy24xanJJNwofyBrCQqq9yrDSayVGnfvqKHD8N5J8mJ1zsjMk m2HbIOo0QgUqKJDUoekJs0ZvySSR8ZynasP6zia1kY5qGTsWYi1JD1ipfD3GP9Q/ FAUQ1u9x7k/Ev1jVOG80iHbowRRMSeQkdpVvp22gh5e/69eW0NBGKFYS9V6+GHFJ C4hHjhy/3p80AYxUb/JFgEVxtQnRx6aCTDHQoILBz/NWhFRD/ho24bBB7lHGuVx/ chuK/pREqTqrM15AI5vS8397gk/YVLPsKflFCTO7f8NC3xOOyyAbFK9TEEQw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:date:date:feedback-id :feedback-id:from:from:in-reply-to:in-reply-to:message-id :mime-version:references:reply-to:sender:subject:subject:to:to :x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm2; t=1673136490; x=1673222890; bh=w6+aDVc5tr2uIs4oPoROJd4fXpjR 6GDvnU6vXcUl8+E=; b=SBqtoaNdbDoKgboEpLb+w8tienE1p0Z5u9EOif9FMYLL gjkQ2YAh+3v8zpK2Fksde3wHXh5Hnj6Z/dv/QEVXbG01tyV3ImVYSTr4DNa5ATnR 4mFmhWKLEnYJ3Ce8XIItTkjRrbEgm05DJMfFqkpMWIcCxz6huTNffz9sl6zevzd0 YyjqLKJHjVSAAxySj5Ow1OnicIN3aTVy2zu26qhvSGkh5bl5ggCloCCo+SCljGQs XvVbEv4ZuywZrF5TgOVlOWVQvtXX5aIvGkMhPmnWscQ/xV+0+lVyWd8pbUKt+B4R fdHYcpY/CSFVzOTPxnrNnrdR0IUtARA+w4kHIRbeDQ== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvhedrkeefgddukecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefofgggkfgjfhffhffvvefutgesthdtredtreertdenucfhrhhomhepfdetrhhn ugcuuegvrhhgmhgrnhhnfdcuoegrrhhnugesrghrnhgusgdruggvqeenucggtffrrghtth gvrhhnpeffheeugeetiefhgeethfejgfdtuefggeejleehjeeutefhfeeggefhkedtkeet ffenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpegrrh hnugesrghrnhgusgdruggv X-ME-Proxy: Feedback-ID: i56a14606:Fastmail Received: by mailuser.nyi.internal (Postfix, from userid 501) id 3FA10B60086; Sat, 7 Jan 2023 19:08:09 -0500 (EST) X-Mailer: MessagingEngine.com Webmail Interface User-Agent: Cyrus-JMAP/3.7.0-alpha0-1185-g841157300a-fm-20221208.002-g84115730 Mime-Version: 1.0 Message-Id: <9017adf0-acd4-4c43-8aea-3579b214b477@app.fastmail.com> In-Reply-To: References: <20230106185526.260163-1-prabhakar.mahadev-lad.rj@bp.renesas.com> <20230106185526.260163-2-prabhakar.mahadev-lad.rj@bp.renesas.com> <6f7d06ef-d74d-4dfc-9b77-6ae83e0d7816@app.fastmail.com> Date: Sun, 08 Jan 2023 01:07:48 +0100 From: "Arnd Bergmann" To: Prabhakar Cc: "Conor.Dooley" , "Geert Uytterhoeven" , =?UTF-8?Q?Heiko_St=C3=BCbner?= , guoren , "Andrew Jones" , "Paul Walmsley" , "Palmer Dabbelt" , "Albert Ou" , "open list:RISC-V ARCHITECTURE" , "open list" , devicetree@vger.kernel.org, Linux-Renesas , "Lad, Prabhakar" , "Philipp Tomsich" , "Nathan Chancellor" , "Atish Patra" , "Anup Patel" , "Tsukasa OI" , "Jisheng Zhang" , "Mayuresh Chitale" Subject: Re: [RFC PATCH v6 1/6] riscv: mm: dma-noncoherent: Switch using function pointers for cache management Content-Type: text/plain Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Jan 7, 2023, at 23:10, Lad, Prabhakar wrote: >> > + >> > + memset(&thead_cmo_ops, 0x0, sizeof(thead_cmo_ops)); >> > + if (IS_ENABLED(CONFIG_ERRATA_THEAD_CMO)) { >> > + thead_cmo_ops.clean_range = &thead_cmo_clean_range; >> > + thead_cmo_ops.inv_range = &thead_cmo_inval_range; >> > + thead_cmo_ops.flush_range = &thead_cmo_flush_range; >> > + riscv_noncoherent_register_cache_ops(&thead_cmo_ops); >> > + } >> >> The implementation here looks reasonable, just wonder whether >> the classification as an 'errata' makes sense. I would probably >> consider this a 'driver' at this point, but that's just >> a question of personal preference. >> > zicbom is a CPU feature that doesn't have any DT node and hence no > driver and similarly for T-HEAD SoC. A driver does not have to be a 'struct platform_driver' that matches to a device node, my point was more about what to name it, regardless of how the code is entered. > Also the arch_setup_dma_ops() > happens quite early before driver probing due to which we get WARN() > messages during bootup hence I have implemented it as errata; as > errata patching happens quite early. But there is no more patching here, just setting the function pointers, right? >> > +struct riscv_cache_ops { >> > + void (*clean_range)(unsigned long addr, unsigned long size); >> > + void (*inv_range)(unsigned long addr, unsigned long size); >> > + void (*flush_range)(unsigned long addr, unsigned long size); >> > + void (*riscv_dma_noncoherent_cmo_ops)(void *vaddr, size_t size, >> > + enum dma_data_direction dir, >> > + enum dma_noncoherent_ops ops); >> > +}; >> >> I don't quite see how the fourth operation is used here. >> Are there cache controllers that need something beyond >> clean/inv/flush? >> > This is for platforms that dont follow standard cache operations (like > done in patch 5/6) and there drivers decide on the operations > depending on the ops and dir. My feeling is that the set of operations that get called should not depend on the cache controller but at best the CPU. I tried to enumerate how zicbom and ax45 differ here, and how that compares to other architectures: zicbom ax45,mips,arc arm arm64 fromdevice clean/flush inval/inval inval/inval clean/inval todevice clean/- clean/- clean/- clean/- bidi flush/flush flush/inval clean/inval clean/inval So everyone does the same operation for DMA_TO_DEVICE, but they differ in the DMA_FROM_DEVICE handling, for reasons I don't quite see: Your ax45 code does the same as arc and mips. arm and arm64 skip invalidating the cache before bidi mappings, but arm has a FIXME comment about that. arm64 does a 'clean' instead of 'inval' when mapping a fromdevice page, which seems valid but slower than necessary. Could the zicbom operations be changed to do the same things as the ax45/mips/arc ones, or are there specific details in the zicbom spec that require this? Arnd 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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 721EDC54EBC for ; Sun, 8 Jan 2023 00:08:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Subject:Cc:To:From:Date:References: In-Reply-To:Message-Id:Mime-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=9ztl1ZQhhe/ChYgTxO8U9OXZi+NxlWmFZBN4DMo2eQg=; b=TpQPo7+tUDpd+U 2fI4X9vZf1IrHPUyPpb89DreO83x3O/ghm1fxcYQnptAhPhAZKxXpakeIwdSkEfrulfKWAALwz6Bj TclRgeaDWpvPlnxzB06JSFej/Wp+qkSO+XR5yFIXSCLmEc9nVhH4cf/rZGDrBuVFVdgLz9+xkxr4t fQNNJ6LxiLxx1b5t8F0ajw457ISgZ6r7vKMrUSl5mwn8GoInKYNc6z4hLoEw6nfDicAYstmDHUUY+ zyUJ+EvTYRGi9trLnJanT9IWytGl9ciESOs0+mTHTEAdv7yNgpNIvwmZh9GwDduyX+cC5DKKIxVng mGMBL8lp0wMY61AIEJHA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pEJEf-0093Ki-3V; Sun, 08 Jan 2023 00:08:25 +0000 Received: from out5-smtp.messagingengine.com ([66.111.4.29]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1pEJEV-0093HV-BZ for linux-riscv@lists.infradead.org; Sun, 08 Jan 2023 00:08:18 +0000 Received: from compute6.internal (compute6.nyi.internal [10.202.2.47]) by mailout.nyi.internal (Postfix) with ESMTP id D339E5C007D; Sat, 7 Jan 2023 19:08:10 -0500 (EST) Received: from imap51 ([10.202.2.101]) by compute6.internal (MEProxy); Sat, 07 Jan 2023 19:08:10 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=arndb.de; h=cc :cc:content-type:date:date:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to; s=fm2; t=1673136490; x=1673222890; bh=w6+aDVc5tr 2uIs4oPoROJd4fXpjR6GDvnU6vXcUl8+E=; b=cp64SsHubAH99xcKXc31E50owr wlsfwwUfDFasgRONy24xanJJNwofyBrCQqq9yrDSayVGnfvqKHD8N5J8mJ1zsjMk m2HbIOo0QgUqKJDUoekJs0ZvySSR8ZynasP6zia1kY5qGTsWYi1JD1ipfD3GP9Q/ FAUQ1u9x7k/Ev1jVOG80iHbowRRMSeQkdpVvp22gh5e/69eW0NBGKFYS9V6+GHFJ C4hHjhy/3p80AYxUb/JFgEVxtQnRx6aCTDHQoILBz/NWhFRD/ho24bBB7lHGuVx/ chuK/pREqTqrM15AI5vS8397gk/YVLPsKflFCTO7f8NC3xOOyyAbFK9TEEQw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:date:date:feedback-id :feedback-id:from:from:in-reply-to:in-reply-to:message-id :mime-version:references:reply-to:sender:subject:subject:to:to :x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm2; t=1673136490; x=1673222890; bh=w6+aDVc5tr2uIs4oPoROJd4fXpjR 6GDvnU6vXcUl8+E=; b=SBqtoaNdbDoKgboEpLb+w8tienE1p0Z5u9EOif9FMYLL gjkQ2YAh+3v8zpK2Fksde3wHXh5Hnj6Z/dv/QEVXbG01tyV3ImVYSTr4DNa5ATnR 4mFmhWKLEnYJ3Ce8XIItTkjRrbEgm05DJMfFqkpMWIcCxz6huTNffz9sl6zevzd0 YyjqLKJHjVSAAxySj5Ow1OnicIN3aTVy2zu26qhvSGkh5bl5ggCloCCo+SCljGQs XvVbEv4ZuywZrF5TgOVlOWVQvtXX5aIvGkMhPmnWscQ/xV+0+lVyWd8pbUKt+B4R fdHYcpY/CSFVzOTPxnrNnrdR0IUtARA+w4kHIRbeDQ== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvhedrkeefgddukecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefofgggkfgjfhffhffvvefutgesthdtredtreertdenucfhrhhomhepfdetrhhn ugcuuegvrhhgmhgrnhhnfdcuoegrrhhnugesrghrnhgusgdruggvqeenucggtffrrghtth gvrhhnpeffheeugeetiefhgeethfejgfdtuefggeejleehjeeutefhfeeggefhkedtkeet ffenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpegrrh hnugesrghrnhgusgdruggv X-ME-Proxy: Feedback-ID: i56a14606:Fastmail Received: by mailuser.nyi.internal (Postfix, from userid 501) id 3FA10B60086; Sat, 7 Jan 2023 19:08:09 -0500 (EST) X-Mailer: MessagingEngine.com Webmail Interface User-Agent: Cyrus-JMAP/3.7.0-alpha0-1185-g841157300a-fm-20221208.002-g84115730 Mime-Version: 1.0 Message-Id: <9017adf0-acd4-4c43-8aea-3579b214b477@app.fastmail.com> In-Reply-To: References: <20230106185526.260163-1-prabhakar.mahadev-lad.rj@bp.renesas.com> <20230106185526.260163-2-prabhakar.mahadev-lad.rj@bp.renesas.com> <6f7d06ef-d74d-4dfc-9b77-6ae83e0d7816@app.fastmail.com> Date: Sun, 08 Jan 2023 01:07:48 +0100 From: "Arnd Bergmann" To: Prabhakar Cc: "Conor.Dooley" , "Geert Uytterhoeven" , =?UTF-8?Q?Heiko_St=C3=BCbner?= , guoren , "Andrew Jones" , "Paul Walmsley" , "Palmer Dabbelt" , "Albert Ou" , "open list:RISC-V ARCHITECTURE" , "open list" , devicetree@vger.kernel.org, Linux-Renesas , "Lad, Prabhakar" , "Philipp Tomsich" , "Nathan Chancellor" , "Atish Patra" , "Anup Patel" , "Tsukasa OI" , "Jisheng Zhang" , "Mayuresh Chitale" Subject: Re: [RFC PATCH v6 1/6] riscv: mm: dma-noncoherent: Switch using function pointers for cache management X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230107_160816_411899_98C39A30 X-CRM114-Status: GOOD ( 20.57 ) X-BeenThere: linux-riscv@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-riscv" Errors-To: linux-riscv-bounces+linux-riscv=archiver.kernel.org@lists.infradead.org On Sat, Jan 7, 2023, at 23:10, Lad, Prabhakar wrote: >> > + >> > + memset(&thead_cmo_ops, 0x0, sizeof(thead_cmo_ops)); >> > + if (IS_ENABLED(CONFIG_ERRATA_THEAD_CMO)) { >> > + thead_cmo_ops.clean_range = &thead_cmo_clean_range; >> > + thead_cmo_ops.inv_range = &thead_cmo_inval_range; >> > + thead_cmo_ops.flush_range = &thead_cmo_flush_range; >> > + riscv_noncoherent_register_cache_ops(&thead_cmo_ops); >> > + } >> >> The implementation here looks reasonable, just wonder whether >> the classification as an 'errata' makes sense. I would probably >> consider this a 'driver' at this point, but that's just >> a question of personal preference. >> > zicbom is a CPU feature that doesn't have any DT node and hence no > driver and similarly for T-HEAD SoC. A driver does not have to be a 'struct platform_driver' that matches to a device node, my point was more about what to name it, regardless of how the code is entered. > Also the arch_setup_dma_ops() > happens quite early before driver probing due to which we get WARN() > messages during bootup hence I have implemented it as errata; as > errata patching happens quite early. But there is no more patching here, just setting the function pointers, right? >> > +struct riscv_cache_ops { >> > + void (*clean_range)(unsigned long addr, unsigned long size); >> > + void (*inv_range)(unsigned long addr, unsigned long size); >> > + void (*flush_range)(unsigned long addr, unsigned long size); >> > + void (*riscv_dma_noncoherent_cmo_ops)(void *vaddr, size_t size, >> > + enum dma_data_direction dir, >> > + enum dma_noncoherent_ops ops); >> > +}; >> >> I don't quite see how the fourth operation is used here. >> Are there cache controllers that need something beyond >> clean/inv/flush? >> > This is for platforms that dont follow standard cache operations (like > done in patch 5/6) and there drivers decide on the operations > depending on the ops and dir. My feeling is that the set of operations that get called should not depend on the cache controller but at best the CPU. I tried to enumerate how zicbom and ax45 differ here, and how that compares to other architectures: zicbom ax45,mips,arc arm arm64 fromdevice clean/flush inval/inval inval/inval clean/inval todevice clean/- clean/- clean/- clean/- bidi flush/flush flush/inval clean/inval clean/inval So everyone does the same operation for DMA_TO_DEVICE, but they differ in the DMA_FROM_DEVICE handling, for reasons I don't quite see: Your ax45 code does the same as arc and mips. arm and arm64 skip invalidating the cache before bidi mappings, but arm has a FIXME comment about that. arm64 does a 'clean' instead of 'inval' when mapping a fromdevice page, which seems valid but slower than necessary. Could the zicbom operations be changed to do the same things as the ax45/mips/arc ones, or are there specific details in the zicbom spec that require this? Arnd _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv