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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 09B02C433EF for ; Tue, 12 Apr 2022 22:01:42 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 264D46B0072; Tue, 12 Apr 2022 18:01:42 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 215366B0073; Tue, 12 Apr 2022 18:01:42 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0B6D66B0074; Tue, 12 Apr 2022 18:01:42 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (relay.hostedemail.com [64.99.140.27]) by kanga.kvack.org (Postfix) with ESMTP id E92566B0072 for ; Tue, 12 Apr 2022 18:01:41 -0400 (EDT) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id AE45924DB9 for ; Tue, 12 Apr 2022 22:01:41 +0000 (UTC) X-FDA: 79349599602.13.46D0434 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by imf21.hostedemail.com (Postfix) with ESMTP id CC0741C0004 for ; Tue, 12 Apr 2022 22:01:40 +0000 (UTC) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 7529DB8205E for ; Tue, 12 Apr 2022 22:01:39 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 3AE7CC385AD for ; Tue, 12 Apr 2022 22:01:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1649800898; bh=ydy77pfGsEmNO9TqK6T8vC6NHRGjR+yzvIsPTZVIUk0=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=pTUsq8HYKKgWnruu2r387ZlfiaNYe8bCUA04lKnWF1djMsvX3q7xmjwF97tRqQwKa aPjrWexfHLMGKy4Lxm39ynDkyimCrePiSv+LeSAWsNV9c3Pduj8DHlKQC+pnzrMHzw wUtpz2daFHJS7LfSTF9aCGH/p7BE8iW4hIsuIIFlguo+/vYCUcDsL6aTjvQcNnr0qg tR1Ozk2hWrO/jYzgGpt3LLsZw8ZXyEe8cEb2puJnBlyFuWbGKQMCVzoUl2sAzUrVaZ Nc6pggCkRz0HlyjAmYUGxOv051pl9nsmHJmTX33vqZNpy4ARJtav/vFYci5BO9vebo iNg5OtJiPuXuQ== Received: by mail-oi1-f169.google.com with SMTP id s1so162760oie.6 for ; Tue, 12 Apr 2022 15:01:38 -0700 (PDT) X-Gm-Message-State: AOAM531IF6BI2HPS0tFegAAqaUAaUh+df29POvEOFuq2IGZtvvAKrima MdMgxVH/l6cSW9amGQ5oFays2u27TUaaiU3t3QE= X-Google-Smtp-Source: ABdhPJwc13ovayq2njvov3t6qnbxbqC/vBF8C0cpvVrQN0L/QGvKCDndhyuRyhaeEnWznZibNtWFDf0lugthvaid4qg= X-Received: by 2002:aca:674c:0:b0:2d9:c460:707c with SMTP id b12-20020aca674c000000b002d9c460707cmr2813613oiy.126.1649800897252; Tue, 12 Apr 2022 15:01:37 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Ard Biesheuvel Date: Wed, 13 Apr 2022 00:01:25 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 07/10] crypto: Use ARCH_DMA_MINALIGN instead of ARCH_KMALLOC_MINALIGN To: Catalin Marinas Cc: Herbert Xu , Will Deacon , Marc Zyngier , Arnd Bergmann , Greg Kroah-Hartman , Andrew Morton , Linus Torvalds , Linux Memory Management List , Linux ARM , Linux Kernel Mailing List , "David S. Miller" Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam09 X-Rspam-User: X-Stat-Signature: 5rnk58gaijkjppg5aodimxow9ojsepbc Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=pTUsq8HY; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf21.hostedemail.com: domain of ardb@kernel.org designates 145.40.68.75 as permitted sender) smtp.mailfrom=ardb@kernel.org X-Rspamd-Queue-Id: CC0741C0004 X-HE-Tag: 1649800900-44155 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, 12 Apr 2022 at 14:31, Catalin Marinas wrote: > > On Tue, Apr 12, 2022 at 06:18:46PM +0800, Herbert Xu wrote: > > On Tue, Apr 12, 2022 at 11:02:54AM +0100, Catalin Marinas wrote: > > > This series does not penalise any architecture. It doesn't even make > > > arm64 any worse than it currently is. > > > > Right, the patch as it stands doesn't change anything. However, > > it is also broken as it stands. As I said before, CRYPTO_MINALIGN > > is not something that is guaranteed by the Crypto API, it is simply > > a statement of whatever kmalloc returns. > > I agree that CRYPTO_MINALIGN is not guaranteed by the Crypto API. What > I'm debating is the intended use for CRYPTO_MINALIGN in some (most?) of > the drivers. It's not just about kmalloc() but also a build-time offset > of buffers within structures to guarantee DMA safety. This can't be > fixed by cra_alignmask. > > We could leave CRYPTO_MINALIGN as ARCH_KMALLOC_MINALIGN and that matches > it being just a statement of the kmalloc() minimum alignment. But since > it is also overloaded with the DMA in-structure offset alignment, we'd > need a new CRYPTO_DMA_MINALIGN (and _ATTR) to annotate those structures. > I have a suspicion there'll be fewer of the original CRYPTO_MINALIGN > uses left, hence my approach to making this bigger from the start. > > There's also Ard's series introducing CRYPTO_REQ_MINALIGN while leaving > CRYPT_MINALIGN for DMA-safe offsets (IIUC): > > https://lore.kernel.org/r/20220406142715.2270256-1-ardb@kernel.org > > > So if kmalloc is no longer returning CRYPTO_MINALIGN-aligned > > memory, then those drivers that need this alignment for DMA > > will break anyway. > One thing to note here is that minimum DMA *alignment* is not the same as the padding to cache writeback granule (CWG) that is needed when dealing with non-cache coherent inbound DMA. The former is typically a property of the peripheral IP, and is something that the driver needs to deal with, potentially by setting cra_alignmask to ensure that the input and output buffers are placed such that they can accessed via DMA by the peripheral. The latter is a property of the CPU's cache hierarchy, not only the size of the CWG, but also whether or not DMA is cache coherent to begin with. This is not something the driver should usually have to care about if it uses the DMA API correctly. The reason why CRYPTO_MINALIGN matters for DMA in spite of this is that some drivers not only use DMA for processing the bulk of the data (typically presented using scatterlists) but sometimes also use DMA to map parts of the request and TFM structures, which carry control data used by the CPU to manage the crypto requests. Doing a non-coherent DMA write into such a structure may blow away 64 or 128 bytes of data, even if the write itself is much smaller, due to the fact that we need to perform cache invalidation in order for the CPU to be able to observe what the device wrote to that memory, and the invalidated cache lines may be shared with other data items, and may become dirty while the DMA mapping is active. This is what I am addressing with my patch series, i.e., padding out the driver owned parts of the struct to the CWG size so that cache invalidation does not affect data owned by other layers in the crypto cake, but only at runtime. By doing this consistently for TFM and request structures, we should be able to disregard ARCH_DMA_MINALIGN entirely when it comes to defining CRYPTO_MINALIGN, as it is highly unlikely that any of these peripherals would require more than 8 or 16 bytes of alignment for the DMA operations themselves. > No. As per one of my previous emails, kmalloc() will preserve the DMA > alignment for an SoC even if smaller than CRYPTO_MINALIGN (or a new > CRYPTO_DMA_MINALIGN). Since kmalloc() returns DMA-safe pointers and > CRYPTO_MINALIGN (or a new CRYPTO_DMA_MINALIGN) is DMA-safe, so would an > offset from a pointer returned by kmalloc(). > > > If you want the Crypto API to guarantee alignment over and above > > that returned by kmalloc, the correct way is to use cra_alignmask. > > For kmalloc(), this would work, but for the current CRYPTO_MINALIGN_ATTR > uses it won't. > > Thanks. > > -- > Catalin