From patchwork Wed Feb 15 07:25:56 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dan Williams X-Patchwork-Id: 9573455 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id D1CBF60493 for ; Wed, 15 Feb 2017 07:26:49 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id BE2A928417 for ; Wed, 15 Feb 2017 07:26:49 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B04A628446; Wed, 15 Feb 2017 07:26:49 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 14F5728417 for ; Wed, 15 Feb 2017 07:26:49 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:To:Subject:Message-ID:Date:From: References:In-Reply-To:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=3OweIgEpafgL5YN2aB4X+5Lk6v4TnDw6eTsr2jaKnqo=; b=SImX3VN6r2Jwy2 6YDBF4nuh4+P/e1QewpyzxZ4r9UlYxFMeRiavcJCszfbIiwO306LxlNCrPOXaWtgtIVvJdWkXnmZ3 eOm7c/COMaLGXH+i43pY05pvnWnjpahBbIvITbdDFbZTdRw++7arGwQGedVq2qixGqLwxD3GL+U0/ QFS9uc3xmhZtE4E240jvkI70OyFlmAnw9o4GBgpcHLVpVnX92LZ0VCpoVR+EyX3YfoT6ulG5cnEXB MKEOHtiCkhHZTzQzne3qyht3E/zDM798++djhnT0lAjq78MLRFKzxyJFV4B1d631s8PxmsoanXbrQ kTOv+1eTTblynFNllLag==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1cdtz8-0007vR-CR; Wed, 15 Feb 2017 07:26:42 +0000 Received: from mail-oi0-x22e.google.com ([2607:f8b0:4003:c06::22e]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1cdtyk-0007bS-Kb for linux-arm-kernel@lists.infradead.org; Wed, 15 Feb 2017 07:26:25 +0000 Received: by mail-oi0-x22e.google.com with SMTP id s203so82690657oie.1 for ; Tue, 14 Feb 2017 23:25:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=MntUGXDLJGg3v2M5AiyPPZxLehddhgykYF6frf/tEfs=; b=M68uagliqox1DBZ0RQFr45MDr2IupqO52LxI1TKFbYoOz49v3juB6l+z4Y/hTqa7xL 44vGLkIT0aBj3Ig40nnZg4OLib6x3nuibl7e2eOGRy3B4Oa4pOADsltyOrARqyqKJXMa WAm2kAVMRIdpPSn2/9W3VjM9Ee5+b2VoGOwoI0bxgokocmFcbQZN/IdRVhgF+vkDQxwU zE0FSgKsl9YUPJkjRoBwZDWB80y6JkSC87atq1cYelgxLDVx1qSkH5OiQuXlB2juPRy6 gtKahQSSpmk/aNqGOKtHt2LIwwEPDswgW9lUQXV7vhkl4CYri4c3kawkIOpTlWtAhHYW OiTw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=MntUGXDLJGg3v2M5AiyPPZxLehddhgykYF6frf/tEfs=; b=d547oy+b0qhZFTIX2ruU8Z4Zmq7IHwjKwyD9C5/4T2liu5rEXVpAoogWRxvCl8LioR BdIc345T3x9uBBfZtRk9/tBBi1ZApiuonsVVbS09KWkGZ5s2n/bGUrtTXDcIN8OjqL42 tvfBGPFHRXnIE1IE0tLFZjprOlEH1ZhK+a/72ROIip4R//dA7GLAZzEHM9Cul5VOR/FL lwvOgSdGmtouRRK9EC3fwjueGlJNizyFDKnNvnC8XGZeSGjmYy3MYMRI8wafrSsPf0Yd 171CqoNWTQi6AZmggsoVDBXVnkNVN9pebG/SBbrP7vpT9WBkpcrqFySAvSIpKDIMlQun pjFw== X-Gm-Message-State: AMke39k9nWVofoacUUiYR2VfhzgB5nHorEk277dpvfpFZuRdpbQMll8Nn4U0W6BoXdSaccph9XdmW5eajW4vg7Un X-Received: by 10.202.222.84 with SMTP id v81mr19099619oig.194.1487143557126; Tue, 14 Feb 2017 23:25:57 -0800 (PST) MIME-Version: 1.0 Received: by 10.157.27.228 with HTTP; Tue, 14 Feb 2017 23:25:56 -0800 (PST) In-Reply-To: References: <1487055112-5185-1-git-send-email-anup.patel@broadcom.com> <1487055112-5185-4-git-send-email-anup.patel@broadcom.com> From: Dan Williams Date: Tue, 14 Feb 2017 23:25:56 -0800 Message-ID: Subject: Re: [PATCH v4 3/4] dmaengine: Add Broadcom SBA RAID driver To: Anup Patel X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20170214_232619_637485_084896DB X-CRM114-Status: GOOD ( 23.18 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , Device Tree , Herbert Xu , Scott Branden , Vinod Koul , Ray Jui , Jassi Brar , "linux-kernel@vger.kernel.org" , linux-raid , Jon Mason , Rob Herring , BCM Kernel Feedback , linux-crypto@vger.kernel.org, Rob Rice , "dmaengine@vger.kernel.org" , "David S . Miller" , "linux-arm-kernel@lists.infradead.org" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP On Tue, Feb 14, 2017 at 11:03 PM, Anup Patel wrote: > On Wed, Feb 15, 2017 at 12:13 PM, Dan Williams wrote: >> On Tue, Feb 14, 2017 at 10:25 PM, Anup Patel wrote: >>> On Tue, Feb 14, 2017 at 10:04 PM, Dan Williams wrote: >>>> On Mon, Feb 13, 2017 at 10:51 PM, Anup Patel wrote: >>>>> The Broadcom stream buffer accelerator (SBA) provides offloading >>>>> capabilities for RAID operations. This SBA offload engine is >>>>> accessible via Broadcom SoC specific ring manager. >>>>> >>>>> This patch adds Broadcom SBA RAID driver which provides one >>>>> DMA device with RAID capabilities using one or more Broadcom >>>>> SoC specific ring manager channels. The SBA RAID driver in its >>>>> current shape implements memcpy, xor, and pq operations. >>>>> >>>>> Signed-off-by: Anup Patel >>>>> Reviewed-by: Ray Jui >>>>> --- >>>>> drivers/dma/Kconfig | 13 + >>>>> drivers/dma/Makefile | 1 + >>>>> drivers/dma/bcm-sba-raid.c | 1694 ++++++++++++++++++++++++++++++++++++++++++++ >>>>> 3 files changed, 1708 insertions(+) >>>>> create mode 100644 drivers/dma/bcm-sba-raid.c >>>>> >>>>> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig >>>>> index 263495d..bf8fb84 100644 >>>>> --- a/drivers/dma/Kconfig >>>>> +++ b/drivers/dma/Kconfig >>>>> @@ -99,6 +99,19 @@ config AXI_DMAC >>>>> controller is often used in Analog Device's reference designs for FPGA >>>>> platforms. >>>>> >>>>> +config BCM_SBA_RAID >>>>> + tristate "Broadcom SBA RAID engine support" >>>>> + depends on (ARM64 && MAILBOX && RAID6_PQ) || COMPILE_TEST >>>>> + select DMA_ENGINE >>>>> + select DMA_ENGINE_RAID >>>>> + select ASYNC_TX_ENABLE_CHANNEL_SWITCH >>>> >>>> I thought you agreed to drop this. Its usage is broken. >>> >>> If ASYNC_TX_ENABLE_CHANNEL_SWITCH is not selected >>> then async_dma_find_channel() will only try to find channel >>> with DMA_ASYNC_TX capability. >>> >>> The DMA_ASYNC_TX capability is set by >>> dma_async_device_register() when all Async Tx >>> capabilities are supported by a DMA devices namely >>> DMA_INTERRUPT, DMA_MEMCPY, DMA_XOR, >>> DMA_XOR_VAL, DMA_PQ, and DMA_PQ_VAL. >>> >>> We only support DMA_MEMCPY, DMA_XOR, and >>> DMA_PQ capabilities in BCM-SBA-RAID driver so >>> DMA_ASYNC_TX capability is never set for the >>> DMA device registered by BCM-SBA-RAID driver. >>> >>> Due to above, if ASYNC_TX_ENABLE_CHANNEL_SWITCH >>> is not selected then Async Tx APIs fail to find DMA >>> channel provided by BCM-SBA-RAID hence the >>> option ASYNC_TX_ENABLE_CHANNEL_SWITCH is >>> required for BCM-SBA-RAID. >>> >>> The DMA mappings are violated by channel switching >>> only if we switch form DMA channel A to DMA channel >>> B and both these DMA channels have different underlying >>> "struct device". In most of the cases DMA mappings >>> are not violated because DMA channels having >>> Async Tx capabilities are provided using same >>> underlying "struct device". >> >> No, fix the infrastructure. Do not put local hack in your driver for >> this global problem [1]. > > There is no hack in the driver. We need > ASYNC_TX_ENABLE_CHANNEL_SWITCH > based on current state of dmaengine framework. > > The framework should be fixed as separate patchset. > > We have other RAID drivers such as xgene-dma and > mv_xor_v2 who also require > ASYNC_TX_ENABLE_CHANNEL_SWITCH due > to same reason. > > Fixing the framework and improving framework is > a ongoing process. I don't see why that should > stop this patchset. > Because this driver is turning on a dangerous compile time option and is not using the functionality. If this silicon IP block appears in another product in the future paired with another DMA engine then the assumptions about a safe/single dma-device is violated. The realization of how async_tx was breaking DMA mapping api assumptions came after some of these dma-drivers were added to the kernel. We should stop making the problem worse. I should have submitted a patch like the below at the time we discovered this problem, but unfortunately it languished when I stopped maintaining the iop-adma and ioat drivers. diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig index 263495d0adbd..6b30eb9ad125 100644 --- a/drivers/dma/Kconfig +++ b/drivers/dma/Kconfig @@ -35,6 +35,7 @@ comment "DMA Devices" #core config ASYNC_TX_ENABLE_CHANNEL_SWITCH + depends on BROKEN bool config ARCH_HAS_ASYNC_TX_FIND_CHANNEL