From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 21A9F6AB84; Tue, 5 Dec 2023 19:34:52 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 1F0F5C433C7; Tue, 5 Dec 2023 19:34:47 +0000 (UTC) Date: Tue, 5 Dec 2023 19:34:45 +0000 From: Catalin Marinas To: Jason Gunthorpe Cc: Niklas Schnelle , Mark Rutland , Leon Romanovsky , Arnd Bergmann , linux-arch@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-rdma@vger.kernel.org, llvm@lists.linux.dev, Michael Guralnik , Nathan Chancellor , Nick Desaulniers , Will Deacon Subject: Re: [PATCH rdma-next 1/2] arm64/io: add memcpy_toio_64 Message-ID: References: <20231124122352.GB436702@nvidia.com> <20231127134505.GI436702@nvidia.com> <20231204182330.GK1493156@nvidia.com> <20231205175127.GJ2692119@nvidia.com> Precedence: bulk X-Mailing-List: linux-arch@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20231205175127.GJ2692119@nvidia.com> On Tue, Dec 05, 2023 at 01:51:27PM -0400, Jason Gunthorpe wrote: > On Tue, Dec 05, 2023 at 05:21:27PM +0000, Catalin Marinas wrote: > > On Mon, Dec 04, 2023 at 02:23:30PM -0400, Jason Gunthorpe wrote: > > > On Mon, Dec 04, 2023 at 05:31:47PM +0000, Catalin Marinas wrote: > > > > Personally I'd optimise the mempcy_toio() arm64 implementation to do > > > > STPs if the alignment is right (like we do for classic memcpy()). > > > > There's a slight overhead for alignment checking but I suspect it would > > > > be lost as long as you can get the write-combining. Not sure whether the > > > > interspersed reads in memcpy_toio() would somehow prevent the > > > > write-combining. > > > > > > I understand on these new CPUs anything other than a block of > > > contiguous STPs is risky to break the WC. I was told we should not > > > have any loads between them. > > > > Classic memcpy does similar tricks with four LDPs in a row before > > starting to issue the STPs (though there are new LDPs for the next > > data in-between). But that was tuned for cacheable memory, not sure > > if something similar would behave well on Normal-NC memory. > > Can we conclude a direction here? > > 1) I don't want to mess with x86 so we keep a dedicated API > Are we agreed to call it __iowrite512_copy() and note its special > alignment limitation? Sounds fine to me. > 2) You want to #define __iowrite512_copy() to memcpy_toio() on ARM and > implement some quad STP optimization for this case? We can have the generic __iowrite512_copy() do memcpy_toio() and have the arm64 implement an optimised version. What I'm not entirely sure of is the DGH (whatever the io_* barrier name is). I'd put it in the same __iowrite512_copy() function and remove it from the driver code. Otherwise when ST64B is added, we have an unnecessary DGH in the driver. If this does not match the other __iowrite*_copy() semantics, we can come up with another name. But start with this for now and document the function. > 3) A future ST64B and the x86 version would be put under > __iowrite512_copy()? Yes, arch-specific override. > 4) A future ST64B would come with some kind of 'must do 64b copy or > oops' to support the future HW that must have this instruction? eg > we already see on Intel that HW must use ENQCMD and nothing else. I don't agree with the oops part. We can't guarantee it on arm64, ST64B I think is optional in the architecture. If you do need such guarantees, we'd need the driver to probe for the feature (e.g. arch_has_...()) and invoke a new macro. You can't have the __iowrite512_copy() that worked fine suddenly giving an error just because some driver wants a guaranteed atomic 64 byte write. -- Catalin 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 EABABC4167B for ; Tue, 5 Dec 2023 19:35:24 +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:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=ZYIfhDiirYpjDdOSQa0iztocScf0jq1NVifRgHUEGqk=; b=F+E3+DsM60Old4 zi6JIbkRdnX6jz/TzcAYutmesPnW5jViV5Aqv5zV0z5ztC/pxBxxWM6XMvV83r6qmi6Afg77BHd+v MZSvDDwZyrQSSegZtusFvsZepluGA391tS4snnPZUtMRXvf8pd7W9n6gD0twdgZihj4V47wcSohoN 2nFaF7gP4J5DverMR6qf6oygM6RRIws9fDMlTtgdZY44TcPuk8hStn4g1LatgXWsq8YDpTRqyS5PQ zJYgFa8q4wpUIvRrrb2CbS8RL1G/mpeT8Fa0iR00anx/gPDN2rLQskrcp9dW5b/tFA5NBcLhqVBCo 9fBD+LJatd2U/UliGVTw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1rAbC7-008ILH-0i; Tue, 05 Dec 2023 19:34:59 +0000 Received: from sin.source.kernel.org ([145.40.73.55]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1rAbC3-008IJn-2h for linux-arm-kernel@lists.infradead.org; Tue, 05 Dec 2023 19:34:57 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id 3E9A6CE11AB; Tue, 5 Dec 2023 19:34:53 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 1F0F5C433C7; Tue, 5 Dec 2023 19:34:47 +0000 (UTC) Date: Tue, 5 Dec 2023 19:34:45 +0000 From: Catalin Marinas To: Jason Gunthorpe Cc: Niklas Schnelle , Mark Rutland , Leon Romanovsky , Arnd Bergmann , linux-arch@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-rdma@vger.kernel.org, llvm@lists.linux.dev, Michael Guralnik , Nathan Chancellor , Nick Desaulniers , Will Deacon Subject: Re: [PATCH rdma-next 1/2] arm64/io: add memcpy_toio_64 Message-ID: References: <20231124122352.GB436702@nvidia.com> <20231127134505.GI436702@nvidia.com> <20231204182330.GK1493156@nvidia.com> <20231205175127.GJ2692119@nvidia.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20231205175127.GJ2692119@nvidia.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20231205_113456_330940_618F4E83 X-CRM114-Status: GOOD ( 31.78 ) X-BeenThere: linux-arm-kernel@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-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Dec 05, 2023 at 01:51:27PM -0400, Jason Gunthorpe wrote: > On Tue, Dec 05, 2023 at 05:21:27PM +0000, Catalin Marinas wrote: > > On Mon, Dec 04, 2023 at 02:23:30PM -0400, Jason Gunthorpe wrote: > > > On Mon, Dec 04, 2023 at 05:31:47PM +0000, Catalin Marinas wrote: > > > > Personally I'd optimise the mempcy_toio() arm64 implementation to do > > > > STPs if the alignment is right (like we do for classic memcpy()). > > > > There's a slight overhead for alignment checking but I suspect it would > > > > be lost as long as you can get the write-combining. Not sure whether the > > > > interspersed reads in memcpy_toio() would somehow prevent the > > > > write-combining. > > > > > > I understand on these new CPUs anything other than a block of > > > contiguous STPs is risky to break the WC. I was told we should not > > > have any loads between them. > > > > Classic memcpy does similar tricks with four LDPs in a row before > > starting to issue the STPs (though there are new LDPs for the next > > data in-between). But that was tuned for cacheable memory, not sure > > if something similar would behave well on Normal-NC memory. > > Can we conclude a direction here? > > 1) I don't want to mess with x86 so we keep a dedicated API > Are we agreed to call it __iowrite512_copy() and note its special > alignment limitation? Sounds fine to me. > 2) You want to #define __iowrite512_copy() to memcpy_toio() on ARM and > implement some quad STP optimization for this case? We can have the generic __iowrite512_copy() do memcpy_toio() and have the arm64 implement an optimised version. What I'm not entirely sure of is the DGH (whatever the io_* barrier name is). I'd put it in the same __iowrite512_copy() function and remove it from the driver code. Otherwise when ST64B is added, we have an unnecessary DGH in the driver. If this does not match the other __iowrite*_copy() semantics, we can come up with another name. But start with this for now and document the function. > 3) A future ST64B and the x86 version would be put under > __iowrite512_copy()? Yes, arch-specific override. > 4) A future ST64B would come with some kind of 'must do 64b copy or > oops' to support the future HW that must have this instruction? eg > we already see on Intel that HW must use ENQCMD and nothing else. I don't agree with the oops part. We can't guarantee it on arm64, ST64B I think is optional in the architecture. If you do need such guarantees, we'd need the driver to probe for the feature (e.g. arch_has_...()) and invoke a new macro. You can't have the __iowrite512_copy() that worked fine suddenly giving an error just because some driver wants a guaranteed atomic 64 byte write. -- Catalin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel