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 X-Spam-Level: X-Spam-Status: No, score=-6.6 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 93EA0C433DF for ; Wed, 19 Aug 2020 14:07:36 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7AB032078D for ; Wed, 19 Aug 2020 14:07:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728508AbgHSOHc (ORCPT ); Wed, 19 Aug 2020 10:07:32 -0400 Received: from foss.arm.com ([217.140.110.172]:38040 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728415AbgHSOHP (ORCPT ); Wed, 19 Aug 2020 10:07:15 -0400 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 671BB1045; Wed, 19 Aug 2020 07:07:14 -0700 (PDT) Received: from [10.57.40.122] (unknown [10.57.40.122]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A9DCD3F71F; Wed, 19 Aug 2020 07:07:07 -0700 (PDT) Subject: Re: [PATCH 05/28] media/v4l2: remove V4L2-FLAG-MEMORY-NON-CONSISTENT To: Tomasz Figa Cc: Christoph Hellwig , alsa-devel@alsa-project.org, linux-ia64@vger.kernel.org, Linux Doc Mailing List , nouveau@lists.freedesktop.org, linux-nvme@lists.infradead.org, Linux Kernel Mailing List , "James E.J. Bottomley" , linux-mm@kvack.org, Marek Szyprowski , linux-samsung-soc , Joonyoung Shim , linux-scsi@vger.kernel.org, Kyungmin Park , Ben Skeggs , Matt Porter , Linux Media Mailing List , Tom Lendacky , Pawel Osciak , Mauro Carvalho Chehab , "list@263.net:IOMMU DRIVERS" , Joerg Roedel , "list@263.net:IOMMU DRIVERS" , Joerg Roedel , linux-arm-kernel@lists.infradead.org, Thomas Bogendoerfer , linux-parisc@vger.kernel.org, netdev@vger.kernel.org, Seung-Woo Kim , linux-mips@vger.kernel.org References: <20200819065555.1802761-1-hch@lst.de> <20200819065555.1802761-6-hch@lst.de> <62e4f4fc-c8a5-3ee8-c576-fe7178cb4356@arm.com> From: Robin Murphy Message-ID: <2b32f1d8-16f7-3352-40a5-420993d52fb5@arm.com> Date: Wed, 19 Aug 2020 15:07:04 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit Sender: linux-parisc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-parisc@vger.kernel.org On 2020-08-19 13:49, Tomasz Figa wrote: > On Wed, Aug 19, 2020 at 1:51 PM Robin Murphy wrote: >> >> Hi Tomasz, >> >> On 2020-08-19 12:16, Tomasz Figa wrote: >>> Hi Christoph, >>> >>> On Wed, Aug 19, 2020 at 8:56 AM Christoph Hellwig wrote: >>>> >>>> The V4L2-FLAG-MEMORY-NON-CONSISTENT flag is entirely unused, >>> >>> Could you explain what makes you think it's unused? It's a feature of >>> the UAPI generally supported by the videobuf2 framework and relied on >>> by Chromium OS to get any kind of reasonable performance when >>> accessing V4L2 buffers in the userspace. >>> >>>> and causes >>>> weird gymanstics with the DMA_ATTR_NON_CONSISTENT flag, which is >>>> unimplemented except on PARISC and some MIPS configs, and about to be >>>> removed. >>> >>> It is implemented by the generic DMA mapping layer [1], which is used >>> by a number of architectures including ARM64 and supposed to be used >>> by new architectures going forward. >> >> AFAICS all that V4L2_FLAG_MEMORY_NON_CONSISTENT does is end up >> controling whether DMA_ATTR_NON_CONSISTENT is added to vb2_queue::dma_attrs. >> >> Please can you point to where DMA_ATTR_NON_CONSISTENT does anything at >> all on arm64? >> > > With the default config it doesn't, but with > CONFIG_DMA_NONCOHERENT_CACHE_SYNC enabled it makes dma_pgprot() keep > the pgprot value as is, without enforcing coherence attributes. How active are the PA-RISC and MIPS ports of Chromium OS? Hacking CONFIG_DMA_NONCOHERENT_CACHE_SYNC into an architecture that doesn't provide dma_cache_sync() is wrong, since at worst it may break other drivers. If downstream is wildly misusing an API then so be it, but it's hardly a strong basis for an upstream argument. >> Also, I posit that videobuf2 is not actually relying on >> DMA_ATTR_NON_CONSISTENT anyway, since it's clearly not using it properly: >> >> "By using this API, you are guaranteeing to the platform >> that you have all the correct and necessary sync points for this memory >> in the driver should it choose to return non-consistent memory." >> >> $ git grep dma_cache_sync drivers/media >> $ > > AFAIK dma_cache_sync() isn't the only way to perform the cache > synchronization. The earlier patch series that I reviewed relied on > dma_get_sgtable() and then dma_sync_sg_*() (which existed in the > vb2-dc since forever [1]). However, it looks like with the final code > the sgtable isn't acquired and the synchronization isn't happening, so > you have a point. Using the streaming sync calls on coherent allocations has also always been wrong per the API, regardless of the bodies of code that have happened to get away with it for so long. > FWIW, I asked back in time what the plan is for non-coherent > allocations and it seemed like DMA_ATTR_NON_CONSISTENT and > dma_sync_*() was supposed to be the right thing to go with. [2] The > same thread also explains why dma_alloc_pages() isn't suitable for the > users of dma_alloc_attrs() and DMA_ATTR_NON_CONSISTENT. AFAICS even back then Christoph was implying getting rid of NON_CONSISTENT and *replacing* it with something streaming-API-based - i.e. this series - not encouraging mixing the existing APIs. It doesn't seem impossible to implement a remapping version of this new dma_alloc_pages() for IOMMU-backed ops if it's really warranted (although at that point it seems like "non-coherent" vb2-dc starts to have significant conceptual overlap with vb2-sg). Robin. 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 X-Spam-Level: X-Spam-Status: No, score=-7.3 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4D0C2C433DF for ; Fri, 21 Aug 2020 07:51:42 +0000 (UTC) Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id C43032078D for ; Fri, 21 Aug 2020 07:51:41 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=alsa-project.org header.i=@alsa-project.org header.b="YDX5nazK" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C43032078D Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=alsa-devel-bounces@alsa-project.org Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id 5992E169E; Fri, 21 Aug 2020 09:50:50 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 5992E169E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1597996300; bh=jCqPZVe1OHj7I+fIqFxKRu6ZHElSS0ZmT4sOz/N6cCg=; h=Subject:To:References:From:Date:In-Reply-To:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=YDX5nazKFc95dRIi1jHqFmCRBijkcuMB+FIDPxBO8eB+7gpRr04WaLziRS0LG9Xia lvi/ci8VUja8rjXjMN3cALsE0Ak3kPlSwsn+qlQfhBJcYNTp226G939ueb5Ev/1b+g LotNuibWClBQfwLGKifbzw/CfcWT/BDg1OJxcnaI= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id C6145F803F3; Fri, 21 Aug 2020 09:36:26 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id A753BF80218; Wed, 19 Aug 2020 16:07:29 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by alsa1.perex.cz (Postfix) with ESMTP id 1DE4DF8011C for ; Wed, 19 Aug 2020 16:07:16 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz 1DE4DF8011C 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 671BB1045; Wed, 19 Aug 2020 07:07:14 -0700 (PDT) Received: from [10.57.40.122] (unknown [10.57.40.122]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A9DCD3F71F; Wed, 19 Aug 2020 07:07:07 -0700 (PDT) Subject: Re: [PATCH 05/28] media/v4l2: remove V4L2-FLAG-MEMORY-NON-CONSISTENT To: Tomasz Figa References: <20200819065555.1802761-1-hch@lst.de> <20200819065555.1802761-6-hch@lst.de> <62e4f4fc-c8a5-3ee8-c576-fe7178cb4356@arm.com> From: Robin Murphy Message-ID: <2b32f1d8-16f7-3352-40a5-420993d52fb5@arm.com> Date: Wed, 19 Aug 2020 15:07:04 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit X-Mailman-Approved-At: Fri, 21 Aug 2020 09:36:14 +0200 Cc: alsa-devel@alsa-project.org, linux-ia64@vger.kernel.org, Linux Doc Mailing List , nouveau@lists.freedesktop.org, linux-nvme@lists.infradead.org, linux-mips@vger.kernel.org, "James E.J. Bottomley" , linux-mm@kvack.org, Christoph Hellwig , Marek Szyprowski , linux-samsung-soc , Joonyoung Shim , linux-scsi@vger.kernel.org, Joerg Roedel , "list@263.net:IOMMU DRIVERS" , Ben Skeggs , Matt Porter , Linux Media Mailing List , Tom Lendacky , Pawel Osciak , Mauro Carvalho Chehab , linux-arm-kernel@lists.infradead.org, Thomas Bogendoerfer , linux-parisc@vger.kernel.org, netdev@vger.kernel.org, Seung-Woo Kim , Linux Kernel Mailing List , Kyungmin Park X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" On 2020-08-19 13:49, Tomasz Figa wrote: > On Wed, Aug 19, 2020 at 1:51 PM Robin Murphy wrote: >> >> Hi Tomasz, >> >> On 2020-08-19 12:16, Tomasz Figa wrote: >>> Hi Christoph, >>> >>> On Wed, Aug 19, 2020 at 8:56 AM Christoph Hellwig wrote: >>>> >>>> The V4L2-FLAG-MEMORY-NON-CONSISTENT flag is entirely unused, >>> >>> Could you explain what makes you think it's unused? It's a feature of >>> the UAPI generally supported by the videobuf2 framework and relied on >>> by Chromium OS to get any kind of reasonable performance when >>> accessing V4L2 buffers in the userspace. >>> >>>> and causes >>>> weird gymanstics with the DMA_ATTR_NON_CONSISTENT flag, which is >>>> unimplemented except on PARISC and some MIPS configs, and about to be >>>> removed. >>> >>> It is implemented by the generic DMA mapping layer [1], which is used >>> by a number of architectures including ARM64 and supposed to be used >>> by new architectures going forward. >> >> AFAICS all that V4L2_FLAG_MEMORY_NON_CONSISTENT does is end up >> controling whether DMA_ATTR_NON_CONSISTENT is added to vb2_queue::dma_attrs. >> >> Please can you point to where DMA_ATTR_NON_CONSISTENT does anything at >> all on arm64? >> > > With the default config it doesn't, but with > CONFIG_DMA_NONCOHERENT_CACHE_SYNC enabled it makes dma_pgprot() keep > the pgprot value as is, without enforcing coherence attributes. How active are the PA-RISC and MIPS ports of Chromium OS? Hacking CONFIG_DMA_NONCOHERENT_CACHE_SYNC into an architecture that doesn't provide dma_cache_sync() is wrong, since at worst it may break other drivers. If downstream is wildly misusing an API then so be it, but it's hardly a strong basis for an upstream argument. >> Also, I posit that videobuf2 is not actually relying on >> DMA_ATTR_NON_CONSISTENT anyway, since it's clearly not using it properly: >> >> "By using this API, you are guaranteeing to the platform >> that you have all the correct and necessary sync points for this memory >> in the driver should it choose to return non-consistent memory." >> >> $ git grep dma_cache_sync drivers/media >> $ > > AFAIK dma_cache_sync() isn't the only way to perform the cache > synchronization. The earlier patch series that I reviewed relied on > dma_get_sgtable() and then dma_sync_sg_*() (which existed in the > vb2-dc since forever [1]). However, it looks like with the final code > the sgtable isn't acquired and the synchronization isn't happening, so > you have a point. Using the streaming sync calls on coherent allocations has also always been wrong per the API, regardless of the bodies of code that have happened to get away with it for so long. > FWIW, I asked back in time what the plan is for non-coherent > allocations and it seemed like DMA_ATTR_NON_CONSISTENT and > dma_sync_*() was supposed to be the right thing to go with. [2] The > same thread also explains why dma_alloc_pages() isn't suitable for the > users of dma_alloc_attrs() and DMA_ATTR_NON_CONSISTENT. AFAICS even back then Christoph was implying getting rid of NON_CONSISTENT and *replacing* it with something streaming-API-based - i.e. this series - not encouraging mixing the existing APIs. It doesn't seem impossible to implement a remapping version of this new dma_alloc_pages() for IOMMU-backed ops if it's really warranted (although at that point it seems like "non-coherent" vb2-dc starts to have significant conceptual overlap with vb2-sg). Robin. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Robin Murphy Subject: Re: [PATCH 05/28] media/v4l2: remove V4L2-FLAG-MEMORY-NON-CONSISTENT Date: Wed, 19 Aug 2020 15:07:04 +0100 Message-ID: <2b32f1d8-16f7-3352-40a5-420993d52fb5@arm.com> References: <20200819065555.1802761-1-hch@lst.de> <20200819065555.1802761-6-hch@lst.de> <62e4f4fc-c8a5-3ee8-c576-fe7178cb4356@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Content-Language: en-GB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Sender: "iommu" To: Tomasz Figa Cc: alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org, linux-ia64-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux Doc Mailing List , nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org, linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-mips-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, "James E.J. Bottomley" , linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org, Christoph Hellwig , linux-samsung-soc , Joonyoung Shim , linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, "list-Y9sIeH5OGRo@public.gmane.org:IOMMU DRIVERS" , Ben Skeggs , Matt Porter , Linux Media Mailing List , Tom Lendacky , Pawel Osciak , Mauro Carvalho Chehab , linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, Thomas Bogendoerfer , linux-parisc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, netdev@vg List-Id: nouveau.vger.kernel.org On 2020-08-19 13:49, Tomasz Figa wrote: > On Wed, Aug 19, 2020 at 1:51 PM Robin Murphy wrote: >> >> Hi Tomasz, >> >> On 2020-08-19 12:16, Tomasz Figa wrote: >>> Hi Christoph, >>> >>> On Wed, Aug 19, 2020 at 8:56 AM Christoph Hellwig wrote: >>>> >>>> The V4L2-FLAG-MEMORY-NON-CONSISTENT flag is entirely unused, >>> >>> Could you explain what makes you think it's unused? It's a feature of >>> the UAPI generally supported by the videobuf2 framework and relied on >>> by Chromium OS to get any kind of reasonable performance when >>> accessing V4L2 buffers in the userspace. >>> >>>> and causes >>>> weird gymanstics with the DMA_ATTR_NON_CONSISTENT flag, which is >>>> unimplemented except on PARISC and some MIPS configs, and about to be >>>> removed. >>> >>> It is implemented by the generic DMA mapping layer [1], which is used >>> by a number of architectures including ARM64 and supposed to be used >>> by new architectures going forward. >> >> AFAICS all that V4L2_FLAG_MEMORY_NON_CONSISTENT does is end up >> controling whether DMA_ATTR_NON_CONSISTENT is added to vb2_queue::dma_attrs. >> >> Please can you point to where DMA_ATTR_NON_CONSISTENT does anything at >> all on arm64? >> > > With the default config it doesn't, but with > CONFIG_DMA_NONCOHERENT_CACHE_SYNC enabled it makes dma_pgprot() keep > the pgprot value as is, without enforcing coherence attributes. How active are the PA-RISC and MIPS ports of Chromium OS? Hacking CONFIG_DMA_NONCOHERENT_CACHE_SYNC into an architecture that doesn't provide dma_cache_sync() is wrong, since at worst it may break other drivers. If downstream is wildly misusing an API then so be it, but it's hardly a strong basis for an upstream argument. >> Also, I posit that videobuf2 is not actually relying on >> DMA_ATTR_NON_CONSISTENT anyway, since it's clearly not using it properly: >> >> "By using this API, you are guaranteeing to the platform >> that you have all the correct and necessary sync points for this memory >> in the driver should it choose to return non-consistent memory." >> >> $ git grep dma_cache_sync drivers/media >> $ > > AFAIK dma_cache_sync() isn't the only way to perform the cache > synchronization. The earlier patch series that I reviewed relied on > dma_get_sgtable() and then dma_sync_sg_*() (which existed in the > vb2-dc since forever [1]). However, it looks like with the final code > the sgtable isn't acquired and the synchronization isn't happening, so > you have a point. Using the streaming sync calls on coherent allocations has also always been wrong per the API, regardless of the bodies of code that have happened to get away with it for so long. > FWIW, I asked back in time what the plan is for non-coherent > allocations and it seemed like DMA_ATTR_NON_CONSISTENT and > dma_sync_*() was supposed to be the right thing to go with. [2] The > same thread also explains why dma_alloc_pages() isn't suitable for the > users of dma_alloc_attrs() and DMA_ATTR_NON_CONSISTENT. AFAICS even back then Christoph was implying getting rid of NON_CONSISTENT and *replacing* it with something streaming-API-based - i.e. this series - not encouraging mixing the existing APIs. It doesn't seem impossible to implement a remapping version of this new dma_alloc_pages() for IOMMU-backed ops if it's really warranted (although at that point it seems like "non-coherent" vb2-dc starts to have significant conceptual overlap with vb2-sg). Robin. 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 X-Spam-Level: X-Spam-Status: No, score=-6.6 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 11070C433E1 for ; Wed, 19 Aug 2020 14:07:21 +0000 (UTC) Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id C59F7204FD for ; Wed, 19 Aug 2020 14:07:20 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C59F7204FD Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=iommu-bounces@lists.linux-foundation.org Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 84F3185082; Wed, 19 Aug 2020 14:07:20 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ZU5gTYa33MPC; Wed, 19 Aug 2020 14:07:19 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id 5C4B584964; Wed, 19 Aug 2020 14:07:19 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 49032C0889; Wed, 19 Aug 2020 14:07:19 +0000 (UTC) Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 22204C0051 for ; Wed, 19 Aug 2020 14:07:17 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 134F120017 for ; Wed, 19 Aug 2020 14:07:17 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id XQZpiVtiqeLH for ; Wed, 19 Aug 2020 14:07:15 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by silver.osuosl.org (Postfix) with ESMTP id 544872000B for ; Wed, 19 Aug 2020 14:07:15 +0000 (UTC) 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 671BB1045; Wed, 19 Aug 2020 07:07:14 -0700 (PDT) Received: from [10.57.40.122] (unknown [10.57.40.122]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A9DCD3F71F; Wed, 19 Aug 2020 07:07:07 -0700 (PDT) Subject: Re: [PATCH 05/28] media/v4l2: remove V4L2-FLAG-MEMORY-NON-CONSISTENT To: Tomasz Figa References: <20200819065555.1802761-1-hch@lst.de> <20200819065555.1802761-6-hch@lst.de> <62e4f4fc-c8a5-3ee8-c576-fe7178cb4356@arm.com> From: Robin Murphy Message-ID: <2b32f1d8-16f7-3352-40a5-420993d52fb5@arm.com> Date: Wed, 19 Aug 2020 15:07:04 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-GB Cc: alsa-devel@alsa-project.org, linux-ia64@vger.kernel.org, Linux Doc Mailing List , nouveau@lists.freedesktop.org, linux-nvme@lists.infradead.org, linux-mips@vger.kernel.org, "James E.J. Bottomley" , linux-mm@kvack.org, Christoph Hellwig , linux-samsung-soc , Joonyoung Shim , linux-scsi@vger.kernel.org, "list@263.net:IOMMU DRIVERS" , Ben Skeggs , Matt Porter , Linux Media Mailing List , Tom Lendacky , Pawel Osciak , Mauro Carvalho Chehab , linux-arm-kernel@lists.infradead.org, Thomas Bogendoerfer , linux-parisc@vger.kernel.org, netdev@vger.kernel.org, Seung-Woo Kim , Linux Kernel Mailing List , Kyungmin Park X-BeenThere: iommu@lists.linux-foundation.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Development issues for Linux IOMMU support List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: iommu-bounces@lists.linux-foundation.org Sender: "iommu" On 2020-08-19 13:49, Tomasz Figa wrote: > On Wed, Aug 19, 2020 at 1:51 PM Robin Murphy wrote: >> >> Hi Tomasz, >> >> On 2020-08-19 12:16, Tomasz Figa wrote: >>> Hi Christoph, >>> >>> On Wed, Aug 19, 2020 at 8:56 AM Christoph Hellwig wrote: >>>> >>>> The V4L2-FLAG-MEMORY-NON-CONSISTENT flag is entirely unused, >>> >>> Could you explain what makes you think it's unused? It's a feature of >>> the UAPI generally supported by the videobuf2 framework and relied on >>> by Chromium OS to get any kind of reasonable performance when >>> accessing V4L2 buffers in the userspace. >>> >>>> and causes >>>> weird gymanstics with the DMA_ATTR_NON_CONSISTENT flag, which is >>>> unimplemented except on PARISC and some MIPS configs, and about to be >>>> removed. >>> >>> It is implemented by the generic DMA mapping layer [1], which is used >>> by a number of architectures including ARM64 and supposed to be used >>> by new architectures going forward. >> >> AFAICS all that V4L2_FLAG_MEMORY_NON_CONSISTENT does is end up >> controling whether DMA_ATTR_NON_CONSISTENT is added to vb2_queue::dma_attrs. >> >> Please can you point to where DMA_ATTR_NON_CONSISTENT does anything at >> all on arm64? >> > > With the default config it doesn't, but with > CONFIG_DMA_NONCOHERENT_CACHE_SYNC enabled it makes dma_pgprot() keep > the pgprot value as is, without enforcing coherence attributes. How active are the PA-RISC and MIPS ports of Chromium OS? Hacking CONFIG_DMA_NONCOHERENT_CACHE_SYNC into an architecture that doesn't provide dma_cache_sync() is wrong, since at worst it may break other drivers. If downstream is wildly misusing an API then so be it, but it's hardly a strong basis for an upstream argument. >> Also, I posit that videobuf2 is not actually relying on >> DMA_ATTR_NON_CONSISTENT anyway, since it's clearly not using it properly: >> >> "By using this API, you are guaranteeing to the platform >> that you have all the correct and necessary sync points for this memory >> in the driver should it choose to return non-consistent memory." >> >> $ git grep dma_cache_sync drivers/media >> $ > > AFAIK dma_cache_sync() isn't the only way to perform the cache > synchronization. The earlier patch series that I reviewed relied on > dma_get_sgtable() and then dma_sync_sg_*() (which existed in the > vb2-dc since forever [1]). However, it looks like with the final code > the sgtable isn't acquired and the synchronization isn't happening, so > you have a point. Using the streaming sync calls on coherent allocations has also always been wrong per the API, regardless of the bodies of code that have happened to get away with it for so long. > FWIW, I asked back in time what the plan is for non-coherent > allocations and it seemed like DMA_ATTR_NON_CONSISTENT and > dma_sync_*() was supposed to be the right thing to go with. [2] The > same thread also explains why dma_alloc_pages() isn't suitable for the > users of dma_alloc_attrs() and DMA_ATTR_NON_CONSISTENT. AFAICS even back then Christoph was implying getting rid of NON_CONSISTENT and *replacing* it with something streaming-API-based - i.e. this series - not encouraging mixing the existing APIs. It doesn't seem impossible to implement a remapping version of this new dma_alloc_pages() for IOMMU-backed ops if it's really warranted (although at that point it seems like "non-coherent" vb2-dc starts to have significant conceptual overlap with vb2-sg). Robin. _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu 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 X-Spam-Level: X-Spam-Status: No, score=-6.6 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8E823C433DF for ; Wed, 19 Aug 2020 14:08:44 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 57D61204FD for ; Wed, 19 Aug 2020 14:08:44 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="vZp8Lhom" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 57D61204FD Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Type: Content-Transfer-Encoding:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=UCnE6TUS4mXEil1TheE73+DP6egSUQdSTXKGOWApIIk=; b=vZp8LhomfbmA4O1Uti3+vWN56 W6s4VLe4jCs1MNg31v74nW7bkVkXOLf6fPRuWw+sop0bYnty/TnbkCltQHbb/u2/NVvRO/z7FUUhi UTnM0VMHFsSOuw9lNCeS9CmjDj11e7TYUC/L1c0H84PmtmxVIwUUwksLuH4F8MXphY2zdfxrQjdb8 kjnaG58hik0yVrqYJffcMlY2+f0RRYxzHDG54RAM/BLQ79cDwPza1hhw+xmracIjnzNfzuCNbo0HY SfX1Jpx1/oNWmISQDBfq/Gi4Vvep6QbxKXZkDcQn1sR35IvaFrQGS5B4XYEb0Zp7Wmfi7eRzDkWOI DX/Oq0QGw==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1k8OkK-0001Uk-7F; Wed, 19 Aug 2020 14:07:20 +0000 Received: from foss.arm.com ([217.140.110.172]) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1k8OkH-0001UB-86; Wed, 19 Aug 2020 14:07:18 +0000 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 671BB1045; Wed, 19 Aug 2020 07:07:14 -0700 (PDT) Received: from [10.57.40.122] (unknown [10.57.40.122]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A9DCD3F71F; Wed, 19 Aug 2020 07:07:07 -0700 (PDT) Subject: Re: [PATCH 05/28] media/v4l2: remove V4L2-FLAG-MEMORY-NON-CONSISTENT To: Tomasz Figa References: <20200819065555.1802761-1-hch@lst.de> <20200819065555.1802761-6-hch@lst.de> <62e4f4fc-c8a5-3ee8-c576-fe7178cb4356@arm.com> From: Robin Murphy Message-ID: <2b32f1d8-16f7-3352-40a5-420993d52fb5@arm.com> Date: Wed, 19 Aug 2020 15:07:04 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-GB X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200819_100717_377891_A81A0121 X-CRM114-Status: GOOD ( 29.38 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: alsa-devel@alsa-project.org, linux-ia64@vger.kernel.org, Linux Doc Mailing List , nouveau@lists.freedesktop.org, linux-nvme@lists.infradead.org, linux-mips@vger.kernel.org, "James E.J. Bottomley" , linux-mm@kvack.org, Christoph Hellwig , Marek Szyprowski , linux-samsung-soc , Joonyoung Shim , linux-scsi@vger.kernel.org, Joerg Roedel , "list@263.net:IOMMU DRIVERS" , Ben Skeggs , Matt Porter , Linux Media Mailing List , Tom Lendacky , Pawel Osciak , Mauro Carvalho Chehab , linux-arm-kernel@lists.infradead.org, Thomas Bogendoerfer , linux-parisc@vger.kernel.org, netdev@vger.kernel.org, Seung-Woo Kim , Linux Kernel Mailing List , Kyungmin Park Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 2020-08-19 13:49, Tomasz Figa wrote: > On Wed, Aug 19, 2020 at 1:51 PM Robin Murphy wrote: >> >> Hi Tomasz, >> >> On 2020-08-19 12:16, Tomasz Figa wrote: >>> Hi Christoph, >>> >>> On Wed, Aug 19, 2020 at 8:56 AM Christoph Hellwig wrote: >>>> >>>> The V4L2-FLAG-MEMORY-NON-CONSISTENT flag is entirely unused, >>> >>> Could you explain what makes you think it's unused? It's a feature of >>> the UAPI generally supported by the videobuf2 framework and relied on >>> by Chromium OS to get any kind of reasonable performance when >>> accessing V4L2 buffers in the userspace. >>> >>>> and causes >>>> weird gymanstics with the DMA_ATTR_NON_CONSISTENT flag, which is >>>> unimplemented except on PARISC and some MIPS configs, and about to be >>>> removed. >>> >>> It is implemented by the generic DMA mapping layer [1], which is used >>> by a number of architectures including ARM64 and supposed to be used >>> by new architectures going forward. >> >> AFAICS all that V4L2_FLAG_MEMORY_NON_CONSISTENT does is end up >> controling whether DMA_ATTR_NON_CONSISTENT is added to vb2_queue::dma_attrs. >> >> Please can you point to where DMA_ATTR_NON_CONSISTENT does anything at >> all on arm64? >> > > With the default config it doesn't, but with > CONFIG_DMA_NONCOHERENT_CACHE_SYNC enabled it makes dma_pgprot() keep > the pgprot value as is, without enforcing coherence attributes. How active are the PA-RISC and MIPS ports of Chromium OS? Hacking CONFIG_DMA_NONCOHERENT_CACHE_SYNC into an architecture that doesn't provide dma_cache_sync() is wrong, since at worst it may break other drivers. If downstream is wildly misusing an API then so be it, but it's hardly a strong basis for an upstream argument. >> Also, I posit that videobuf2 is not actually relying on >> DMA_ATTR_NON_CONSISTENT anyway, since it's clearly not using it properly: >> >> "By using this API, you are guaranteeing to the platform >> that you have all the correct and necessary sync points for this memory >> in the driver should it choose to return non-consistent memory." >> >> $ git grep dma_cache_sync drivers/media >> $ > > AFAIK dma_cache_sync() isn't the only way to perform the cache > synchronization. The earlier patch series that I reviewed relied on > dma_get_sgtable() and then dma_sync_sg_*() (which existed in the > vb2-dc since forever [1]). However, it looks like with the final code > the sgtable isn't acquired and the synchronization isn't happening, so > you have a point. Using the streaming sync calls on coherent allocations has also always been wrong per the API, regardless of the bodies of code that have happened to get away with it for so long. > FWIW, I asked back in time what the plan is for non-coherent > allocations and it seemed like DMA_ATTR_NON_CONSISTENT and > dma_sync_*() was supposed to be the right thing to go with. [2] The > same thread also explains why dma_alloc_pages() isn't suitable for the > users of dma_alloc_attrs() and DMA_ATTR_NON_CONSISTENT. AFAICS even back then Christoph was implying getting rid of NON_CONSISTENT and *replacing* it with something streaming-API-based - i.e. this series - not encouraging mixing the existing APIs. It doesn't seem impossible to implement a remapping version of this new dma_alloc_pages() for IOMMU-backed ops if it's really warranted (although at that point it seems like "non-coherent" vb2-dc starts to have significant conceptual overlap with vb2-sg). Robin. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel From mboxrd@z Thu Jan 1 00:00:00 1970 From: Robin Murphy Date: Wed, 19 Aug 2020 14:07:04 +0000 Subject: Re: [PATCH 05/28] media/v4l2: remove V4L2-FLAG-MEMORY-NON-CONSISTENT Message-Id: <2b32f1d8-16f7-3352-40a5-420993d52fb5@arm.com> List-Id: References: <20200819065555.1802761-1-hch@lst.de> <20200819065555.1802761-6-hch@lst.de> <62e4f4fc-c8a5-3ee8-c576-fe7178cb4356@arm.com> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Tomasz Figa Cc: Christoph Hellwig , alsa-devel@alsa-project.org, linux-ia64@vger.kernel.org, Linux Doc Mailing List , nouveau@lists.freedesktop.org, linux-nvme@lists.infradead.org, Linux Kernel Mailing List , "James E.J. Bottomley" , linux-mm@kvack.org, Marek Szyprowski , linux-samsung-soc , Joonyoung Shim , linux-scsi@vger.kernel.org, Kyungmin Park , Ben Skeggs , Matt Porter , Linux Media Mailing List , Tom Lendacky , Pawel Osciak , Mauro Carvalho Chehab , "list@263.net:IOMMU DRIVERS" , Joerg Roedel , "list@263.net:IOMMU DRIVERS" , Joerg Roedel , linux-arm-kernel@lists.infradead.org, Thomas Bogendoerfer , linux-parisc@vger.kernel.org, netdev@vger.kernel.org, Seung-Woo Kim , linux-mips@vger.kernel.org On 2020-08-19 13:49, Tomasz Figa wrote: > On Wed, Aug 19, 2020 at 1:51 PM Robin Murphy wrote: >> >> Hi Tomasz, >> >> On 2020-08-19 12:16, Tomasz Figa wrote: >>> Hi Christoph, >>> >>> On Wed, Aug 19, 2020 at 8:56 AM Christoph Hellwig wrote: >>>> >>>> The V4L2-FLAG-MEMORY-NON-CONSISTENT flag is entirely unused, >>> >>> Could you explain what makes you think it's unused? It's a feature of >>> the UAPI generally supported by the videobuf2 framework and relied on >>> by Chromium OS to get any kind of reasonable performance when >>> accessing V4L2 buffers in the userspace. >>> >>>> and causes >>>> weird gymanstics with the DMA_ATTR_NON_CONSISTENT flag, which is >>>> unimplemented except on PARISC and some MIPS configs, and about to be >>>> removed. >>> >>> It is implemented by the generic DMA mapping layer [1], which is used >>> by a number of architectures including ARM64 and supposed to be used >>> by new architectures going forward. >> >> AFAICS all that V4L2_FLAG_MEMORY_NON_CONSISTENT does is end up >> controling whether DMA_ATTR_NON_CONSISTENT is added to vb2_queue::dma_attrs. >> >> Please can you point to where DMA_ATTR_NON_CONSISTENT does anything at >> all on arm64? >> > > With the default config it doesn't, but with > CONFIG_DMA_NONCOHERENT_CACHE_SYNC enabled it makes dma_pgprot() keep > the pgprot value as is, without enforcing coherence attributes. How active are the PA-RISC and MIPS ports of Chromium OS? Hacking CONFIG_DMA_NONCOHERENT_CACHE_SYNC into an architecture that doesn't provide dma_cache_sync() is wrong, since at worst it may break other drivers. If downstream is wildly misusing an API then so be it, but it's hardly a strong basis for an upstream argument. >> Also, I posit that videobuf2 is not actually relying on >> DMA_ATTR_NON_CONSISTENT anyway, since it's clearly not using it properly: >> >> "By using this API, you are guaranteeing to the platform >> that you have all the correct and necessary sync points for this memory >> in the driver should it choose to return non-consistent memory." >> >> $ git grep dma_cache_sync drivers/media >> $ > > AFAIK dma_cache_sync() isn't the only way to perform the cache > synchronization. The earlier patch series that I reviewed relied on > dma_get_sgtable() and then dma_sync_sg_*() (which existed in the > vb2-dc since forever [1]). However, it looks like with the final code > the sgtable isn't acquired and the synchronization isn't happening, so > you have a point. Using the streaming sync calls on coherent allocations has also always been wrong per the API, regardless of the bodies of code that have happened to get away with it for so long. > FWIW, I asked back in time what the plan is for non-coherent > allocations and it seemed like DMA_ATTR_NON_CONSISTENT and > dma_sync_*() was supposed to be the right thing to go with. [2] The > same thread also explains why dma_alloc_pages() isn't suitable for the > users of dma_alloc_attrs() and DMA_ATTR_NON_CONSISTENT. AFAICS even back then Christoph was implying getting rid of NON_CONSISTENT and *replacing* it with something streaming-API-based - i.e. this series - not encouraging mixing the existing APIs. It doesn't seem impossible to implement a remapping version of this new dma_alloc_pages() for IOMMU-backed ops if it's really warranted (although at that point it seems like "non-coherent" vb2-dc starts to have significant conceptual overlap with vb2-sg). Robin.