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=-10.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable 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 87855C07E9E for ; Tue, 6 Jul 2021 14:40:20 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6FDAC6135C for ; Tue, 6 Jul 2021 14:40:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232602AbhGFOm5 (ORCPT ); Tue, 6 Jul 2021 10:42:57 -0400 Received: from foss.arm.com ([217.140.110.172]:43808 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232527AbhGFOmx (ORCPT ); Tue, 6 Jul 2021 10:42:53 -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 6E5E031B; Tue, 6 Jul 2021 07:01:18 -0700 (PDT) Received: from [10.57.40.45] (unknown [10.57.40.45]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 461163F73B; Tue, 6 Jul 2021 07:01:11 -0700 (PDT) Subject: Re: [PATCH v15 06/12] swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing To: Will Deacon , Christoph Hellwig Cc: heikki.krogerus@linux.intel.com, thomas.hellstrom@linux.intel.com, peterz@infradead.org, benh@kernel.crashing.org, joonas.lahtinen@linux.intel.com, dri-devel@lists.freedesktop.org, chris@chris-wilson.co.uk, grant.likely@arm.com, paulus@samba.org, Frank Rowand , mingo@kernel.org, Stefano Stabellini , Saravana Kannan , mpe@ellerman.id.au, "Rafael J . Wysocki" , Bartosz Golaszewski , bskeggs@redhat.com, linux-pci@vger.kernel.org, xen-devel@lists.xenproject.org, Thierry Reding , intel-gfx@lists.freedesktop.org, matthew.auld@intel.com, linux-devicetree , Jianxiong Gao , Daniel Vetter , Konrad Rzeszutek Wilk , maarten.lankhorst@linux.intel.com, airlied@linux.ie, Dan Williams , linuxppc-dev@lists.ozlabs.org, jani.nikula@linux.intel.com, Nathan Chancellor , Rob Herring , rodrigo.vivi@intel.com, Bjorn Helgaas , Claire Chang , boris.ostrovsky@oracle.com, Andy Shevchenko , jgross@suse.com, Nicolas Boichat , Greg KH , Randy Dunlap , Qian Cai , lkml , "list@263.net:IOMMU DRIVERS" , Jim Quinlan , xypron.glpk@gmx.de, Tom Lendacky , bauerman@linux.ibm.com References: <20210630114348.GA8383@willie-the-truck> <20210701074045.GA9436@willie-the-truck> <20210702135856.GB11132@willie-the-truck> <0f7bd903-e309-94a0-21d7-f0e8e9546018@arm.com> <20210705190352.GA19461@willie-the-truck> <20210706044848.GA13640@lst.de> <20210706132422.GA20327@willie-the-truck> From: Robin Murphy Message-ID: Date: Tue, 6 Jul 2021 15:01:04 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <20210706132422.GA20327@willie-the-truck> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2021-07-06 14:24, Will Deacon wrote: > On Tue, Jul 06, 2021 at 06:48:48AM +0200, Christoph Hellwig wrote: >> On Mon, Jul 05, 2021 at 08:03:52PM +0100, Will Deacon wrote: >>> So at this point, the AMD IOMMU driver does: >>> >>> swiotlb = (iommu_default_passthrough() || sme_me_mask) ? 1 : 0; >>> >>> where 'swiotlb' is a global variable indicating whether or not swiotlb >>> is in use. It's picked up a bit later on by pci_swiotlb_late_init(), which >>> will call swiotlb_exit() if 'swiotlb' is false. >>> >>> Now, that used to work fine, because swiotlb_exit() clears >>> 'io_tlb_default_mem' to NULL, but now with the restricted DMA changes, I >>> think that all the devices which have successfully probed beforehand will >>> have stale pointers to the freed structure in their 'dev->dma_io_tlb_mem' >>> field. >> >> Yeah. I don't think we can do that anymore, and I also think it is >> a bad idea to start with. > > I've had a crack at reworking things along the following lines: > > - io_tlb_default_mem now lives in the BSS, the flexible array member > is now a pointer and that part is allocated dynamically (downside of > this is an extra indirection to get at the slots). > > - io_tlb_default_mem.nslabs tells you whether the thing is valid > > - swiotlb_exit() frees the slots array and clears the rest of the > structure to 0. I also extended it to free the actual slabs, but I'm > not sure why it wasn't doing that before. > > So a non-NULL dev->dma_io_tlb_mem should always be valid to follow. FWIW I was pondering the question of whether to do something along those lines or just scrap the default assignment entirely, so since I hadn't got round to saying that I've gone ahead and hacked up the alternative (similarly untested) for comparison :) TBH I'm still not sure which one I prefer... Robin. ----->8----- diff --git a/drivers/base/core.c b/drivers/base/core.c index ea5b85354526..394abf184c1a 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -2847,9 +2847,6 @@ void device_initialize(struct device *dev) defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL) dev->dma_coherent = dma_default_coherent; #endif -#ifdef CONFIG_SWIOTLB - dev->dma_io_tlb_mem = io_tlb_default_mem; -#endif } EXPORT_SYMBOL_GPL(device_initialize); diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h index 39284ff2a6cd..620f16d89a98 100644 --- a/include/linux/swiotlb.h +++ b/include/linux/swiotlb.h @@ -107,16 +107,21 @@ struct io_tlb_mem { }; extern struct io_tlb_mem *io_tlb_default_mem; +static inline struct io_tlb_mem *dev_iotlb_mem(struct device *dev) +{ + return dev->dma_io_tlb_mem ?: io_tlb_default_mem; +} + static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); return mem && paddr >= mem->start && paddr < mem->end; } static inline bool is_swiotlb_force_bounce(struct device *dev) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); return mem && mem->force_bounce; } @@ -167,7 +172,7 @@ bool swiotlb_free(struct device *dev, struct page *page, size_t size); static inline bool is_swiotlb_for_alloc(struct device *dev) { - return dev->dma_io_tlb_mem->for_alloc; + return dev_iotlb_mem(dev)->for_alloc; } #else static inline struct page *swiotlb_alloc(struct device *dev, size_t size) diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c index b7f76bca89bf..f4942149f87d 100644 --- a/kernel/dma/swiotlb.c +++ b/kernel/dma/swiotlb.c @@ -359,7 +359,7 @@ static unsigned int swiotlb_align_offset(struct device *dev, u64 addr) static void swiotlb_bounce(struct device *dev, phys_addr_t tlb_addr, size_t size, enum dma_data_direction dir) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); int index = (tlb_addr - mem->start) >> IO_TLB_SHIFT; phys_addr_t orig_addr = mem->slots[index].orig_addr; size_t alloc_size = mem->slots[index].alloc_size; @@ -440,7 +440,7 @@ static unsigned int wrap_index(struct io_tlb_mem *mem, unsigned int index) static int swiotlb_find_slots(struct device *dev, phys_addr_t orig_addr, size_t alloc_size) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); unsigned long boundary_mask = dma_get_seg_boundary(dev); dma_addr_t tbl_dma_addr = phys_to_dma_unencrypted(dev, mem->start) & boundary_mask; @@ -522,7 +522,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr, size_t mapping_size, size_t alloc_size, enum dma_data_direction dir, unsigned long attrs) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); unsigned int offset = swiotlb_align_offset(dev, orig_addr); unsigned int i; int index; @@ -565,7 +565,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr, static void swiotlb_release_slots(struct device *dev, phys_addr_t tlb_addr) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); unsigned long flags; unsigned int offset = swiotlb_align_offset(dev, tlb_addr); int index = (tlb_addr - offset - mem->start) >> IO_TLB_SHIFT; @@ -682,7 +682,7 @@ size_t swiotlb_max_mapping_size(struct device *dev) bool is_swiotlb_active(struct device *dev) { - return dev->dma_io_tlb_mem != NULL; + return dev_iotlb_mem(dev) != NULL; } EXPORT_SYMBOL_GPL(is_swiotlb_active); @@ -729,7 +729,7 @@ static void rmem_swiotlb_debugfs_init(struct reserved_mem *rmem) struct page *swiotlb_alloc(struct device *dev, size_t size) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); phys_addr_t tlb_addr; int index; @@ -792,7 +792,7 @@ static int rmem_swiotlb_device_init(struct reserved_mem *rmem, static void rmem_swiotlb_device_release(struct reserved_mem *rmem, struct device *dev) { - dev->dma_io_tlb_mem = io_tlb_default_mem; + dev->dma_io_tlb_mem = NULL; } static const struct reserved_mem_ops rmem_swiotlb_ops = { 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=-10.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable 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 6012EC07E96 for ; Tue, 6 Jul 2021 14:01:50 +0000 (UTC) Received: from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (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 C5F0F619B4 for ; Tue, 6 Jul 2021 14:01:49 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C5F0F619B4 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4GK43w0c27z3bhn for ; Wed, 7 Jul 2021 00:01:48 +1000 (AEST) Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=arm.com (client-ip=217.140.110.172; helo=foss.arm.com; envelope-from=robin.murphy@arm.com; receiver=) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lists.ozlabs.org (Postfix) with ESMTP id 4GK43R1zqsz2yM5 for ; Wed, 7 Jul 2021 00:01:21 +1000 (AEST) 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 6E5E031B; Tue, 6 Jul 2021 07:01:18 -0700 (PDT) Received: from [10.57.40.45] (unknown [10.57.40.45]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 461163F73B; Tue, 6 Jul 2021 07:01:11 -0700 (PDT) Subject: Re: [PATCH v15 06/12] swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing To: Will Deacon , Christoph Hellwig References: <20210630114348.GA8383@willie-the-truck> <20210701074045.GA9436@willie-the-truck> <20210702135856.GB11132@willie-the-truck> <0f7bd903-e309-94a0-21d7-f0e8e9546018@arm.com> <20210705190352.GA19461@willie-the-truck> <20210706044848.GA13640@lst.de> <20210706132422.GA20327@willie-the-truck> From: Robin Murphy Message-ID: Date: Tue, 6 Jul 2021 15:01:04 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <20210706132422.GA20327@willie-the-truck> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Jim Quinlan , heikki.krogerus@linux.intel.com, linux-devicetree , peterz@infradead.org, joonas.lahtinen@linux.intel.com, dri-devel@lists.freedesktop.org, chris@chris-wilson.co.uk, grant.likely@arm.com, paulus@samba.org, Frank Rowand , mingo@kernel.org, Jianxiong Gao , Stefano Stabellini , Saravana Kannan , "Rafael J . Wysocki" , Bartosz Golaszewski , matthew.auld@intel.com, linux-pci@vger.kernel.org, xen-devel@lists.xenproject.org, Thierry Reding , bskeggs@redhat.com, Nicolas Boichat , thomas.hellstrom@linux.intel.com, jgross@suse.com, Konrad Rzeszutek Wilk , intel-gfx@lists.freedesktop.org, maarten.lankhorst@linux.intel.com, jani.nikula@linux.intel.com, Nathan Chancellor , Rob Herring , rodrigo.vivi@intel.com, Bjorn Helgaas , Claire Chang , Dan Williams , Andy Shevchenko , boris.ostrovsky@oracle.com, airlied@linux.ie, Greg KH , Randy Dunlap , Qian Cai , lkml , "list@263.net:IOMMU DRIVERS" , Daniel Vetter , xypron.glpk@gmx.de, Tom Lendacky , linuxppc-dev@lists.ozlabs.org, bauerman@linux.ibm.com Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On 2021-07-06 14:24, Will Deacon wrote: > On Tue, Jul 06, 2021 at 06:48:48AM +0200, Christoph Hellwig wrote: >> On Mon, Jul 05, 2021 at 08:03:52PM +0100, Will Deacon wrote: >>> So at this point, the AMD IOMMU driver does: >>> >>> swiotlb = (iommu_default_passthrough() || sme_me_mask) ? 1 : 0; >>> >>> where 'swiotlb' is a global variable indicating whether or not swiotlb >>> is in use. It's picked up a bit later on by pci_swiotlb_late_init(), which >>> will call swiotlb_exit() if 'swiotlb' is false. >>> >>> Now, that used to work fine, because swiotlb_exit() clears >>> 'io_tlb_default_mem' to NULL, but now with the restricted DMA changes, I >>> think that all the devices which have successfully probed beforehand will >>> have stale pointers to the freed structure in their 'dev->dma_io_tlb_mem' >>> field. >> >> Yeah. I don't think we can do that anymore, and I also think it is >> a bad idea to start with. > > I've had a crack at reworking things along the following lines: > > - io_tlb_default_mem now lives in the BSS, the flexible array member > is now a pointer and that part is allocated dynamically (downside of > this is an extra indirection to get at the slots). > > - io_tlb_default_mem.nslabs tells you whether the thing is valid > > - swiotlb_exit() frees the slots array and clears the rest of the > structure to 0. I also extended it to free the actual slabs, but I'm > not sure why it wasn't doing that before. > > So a non-NULL dev->dma_io_tlb_mem should always be valid to follow. FWIW I was pondering the question of whether to do something along those lines or just scrap the default assignment entirely, so since I hadn't got round to saying that I've gone ahead and hacked up the alternative (similarly untested) for comparison :) TBH I'm still not sure which one I prefer... Robin. ----->8----- diff --git a/drivers/base/core.c b/drivers/base/core.c index ea5b85354526..394abf184c1a 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -2847,9 +2847,6 @@ void device_initialize(struct device *dev) defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL) dev->dma_coherent = dma_default_coherent; #endif -#ifdef CONFIG_SWIOTLB - dev->dma_io_tlb_mem = io_tlb_default_mem; -#endif } EXPORT_SYMBOL_GPL(device_initialize); diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h index 39284ff2a6cd..620f16d89a98 100644 --- a/include/linux/swiotlb.h +++ b/include/linux/swiotlb.h @@ -107,16 +107,21 @@ struct io_tlb_mem { }; extern struct io_tlb_mem *io_tlb_default_mem; +static inline struct io_tlb_mem *dev_iotlb_mem(struct device *dev) +{ + return dev->dma_io_tlb_mem ?: io_tlb_default_mem; +} + static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); return mem && paddr >= mem->start && paddr < mem->end; } static inline bool is_swiotlb_force_bounce(struct device *dev) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); return mem && mem->force_bounce; } @@ -167,7 +172,7 @@ bool swiotlb_free(struct device *dev, struct page *page, size_t size); static inline bool is_swiotlb_for_alloc(struct device *dev) { - return dev->dma_io_tlb_mem->for_alloc; + return dev_iotlb_mem(dev)->for_alloc; } #else static inline struct page *swiotlb_alloc(struct device *dev, size_t size) diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c index b7f76bca89bf..f4942149f87d 100644 --- a/kernel/dma/swiotlb.c +++ b/kernel/dma/swiotlb.c @@ -359,7 +359,7 @@ static unsigned int swiotlb_align_offset(struct device *dev, u64 addr) static void swiotlb_bounce(struct device *dev, phys_addr_t tlb_addr, size_t size, enum dma_data_direction dir) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); int index = (tlb_addr - mem->start) >> IO_TLB_SHIFT; phys_addr_t orig_addr = mem->slots[index].orig_addr; size_t alloc_size = mem->slots[index].alloc_size; @@ -440,7 +440,7 @@ static unsigned int wrap_index(struct io_tlb_mem *mem, unsigned int index) static int swiotlb_find_slots(struct device *dev, phys_addr_t orig_addr, size_t alloc_size) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); unsigned long boundary_mask = dma_get_seg_boundary(dev); dma_addr_t tbl_dma_addr = phys_to_dma_unencrypted(dev, mem->start) & boundary_mask; @@ -522,7 +522,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr, size_t mapping_size, size_t alloc_size, enum dma_data_direction dir, unsigned long attrs) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); unsigned int offset = swiotlb_align_offset(dev, orig_addr); unsigned int i; int index; @@ -565,7 +565,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr, static void swiotlb_release_slots(struct device *dev, phys_addr_t tlb_addr) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); unsigned long flags; unsigned int offset = swiotlb_align_offset(dev, tlb_addr); int index = (tlb_addr - offset - mem->start) >> IO_TLB_SHIFT; @@ -682,7 +682,7 @@ size_t swiotlb_max_mapping_size(struct device *dev) bool is_swiotlb_active(struct device *dev) { - return dev->dma_io_tlb_mem != NULL; + return dev_iotlb_mem(dev) != NULL; } EXPORT_SYMBOL_GPL(is_swiotlb_active); @@ -729,7 +729,7 @@ static void rmem_swiotlb_debugfs_init(struct reserved_mem *rmem) struct page *swiotlb_alloc(struct device *dev, size_t size) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); phys_addr_t tlb_addr; int index; @@ -792,7 +792,7 @@ static int rmem_swiotlb_device_init(struct reserved_mem *rmem, static void rmem_swiotlb_device_release(struct reserved_mem *rmem, struct device *dev) { - dev->dma_io_tlb_mem = io_tlb_default_mem; + dev->dma_io_tlb_mem = NULL; } static const struct reserved_mem_ops rmem_swiotlb_ops = { 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=-10.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,URIBL_RED,USER_AGENT_SANE_1 autolearn=unavailable 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 B6EEEC07E96 for ; Tue, 6 Jul 2021 14:01:24 +0000 (UTC) Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (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 53293619B4 for ; Tue, 6 Jul 2021 14:01:24 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 53293619B4 Authentication-Results: mail.kernel.org; dmarc=fail (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 smtp3.osuosl.org (Postfix) with ESMTP id 18C4C6085F; Tue, 6 Jul 2021 14:01:24 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id r-O6Vgp6zu1O; Tue, 6 Jul 2021 14:01:23 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp3.osuosl.org (Postfix) with ESMTPS id B0E486084D; Tue, 6 Jul 2021 14:01:22 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 83A72C0010; Tue, 6 Jul 2021 14:01:22 +0000 (UTC) Received: from smtp2.osuosl.org (smtp2.osuosl.org [IPv6:2605:bc80:3010::133]) by lists.linuxfoundation.org (Postfix) with ESMTP id 8B475C000E for ; Tue, 6 Jul 2021 14:01:20 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 85D3540555 for ; Tue, 6 Jul 2021 14:01:20 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id DhUREyPcsVQP for ; Tue, 6 Jul 2021 14:01:19 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp2.osuosl.org (Postfix) with ESMTP id 65D74401B5 for ; Tue, 6 Jul 2021 14:01:19 +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 6E5E031B; Tue, 6 Jul 2021 07:01:18 -0700 (PDT) Received: from [10.57.40.45] (unknown [10.57.40.45]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 461163F73B; Tue, 6 Jul 2021 07:01:11 -0700 (PDT) Subject: Re: [PATCH v15 06/12] swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing To: Will Deacon , Christoph Hellwig References: <20210630114348.GA8383@willie-the-truck> <20210701074045.GA9436@willie-the-truck> <20210702135856.GB11132@willie-the-truck> <0f7bd903-e309-94a0-21d7-f0e8e9546018@arm.com> <20210705190352.GA19461@willie-the-truck> <20210706044848.GA13640@lst.de> <20210706132422.GA20327@willie-the-truck> From: Robin Murphy Message-ID: Date: Tue, 6 Jul 2021 15:01:04 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <20210706132422.GA20327@willie-the-truck> Content-Language: en-GB Cc: Jim Quinlan , heikki.krogerus@linux.intel.com, linux-devicetree , peterz@infradead.org, benh@kernel.crashing.org, joonas.lahtinen@linux.intel.com, dri-devel@lists.freedesktop.org, chris@chris-wilson.co.uk, grant.likely@arm.com, paulus@samba.org, Frank Rowand , mingo@kernel.org, Jianxiong Gao , Stefano Stabellini , Saravana Kannan , mpe@ellerman.id.au, "Rafael J . Wysocki" , Bartosz Golaszewski , matthew.auld@intel.com, linux-pci@vger.kernel.org, xen-devel@lists.xenproject.org, Thierry Reding , bskeggs@redhat.com, Nicolas Boichat , thomas.hellstrom@linux.intel.com, jgross@suse.com, Konrad Rzeszutek Wilk , intel-gfx@lists.freedesktop.org, maarten.lankhorst@linux.intel.com, jani.nikula@linux.intel.com, Nathan Chancellor , Rob Herring , rodrigo.vivi@intel.com, Bjorn Helgaas , Claire Chang , Dan Williams , Andy Shevchenko , boris.ostrovsky@oracle.com, airlied@linux.ie, Greg KH , Randy Dunlap , Qian Cai , lkml , "list@263.net:IOMMU DRIVERS" , Daniel Vetter , xypron.glpk@gmx.de, Tom Lendacky , linuxppc-dev@lists.ozlabs.org, bauerman@linux.ibm.com 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 2021-07-06 14:24, Will Deacon wrote: > On Tue, Jul 06, 2021 at 06:48:48AM +0200, Christoph Hellwig wrote: >> On Mon, Jul 05, 2021 at 08:03:52PM +0100, Will Deacon wrote: >>> So at this point, the AMD IOMMU driver does: >>> >>> swiotlb = (iommu_default_passthrough() || sme_me_mask) ? 1 : 0; >>> >>> where 'swiotlb' is a global variable indicating whether or not swiotlb >>> is in use. It's picked up a bit later on by pci_swiotlb_late_init(), which >>> will call swiotlb_exit() if 'swiotlb' is false. >>> >>> Now, that used to work fine, because swiotlb_exit() clears >>> 'io_tlb_default_mem' to NULL, but now with the restricted DMA changes, I >>> think that all the devices which have successfully probed beforehand will >>> have stale pointers to the freed structure in their 'dev->dma_io_tlb_mem' >>> field. >> >> Yeah. I don't think we can do that anymore, and I also think it is >> a bad idea to start with. > > I've had a crack at reworking things along the following lines: > > - io_tlb_default_mem now lives in the BSS, the flexible array member > is now a pointer and that part is allocated dynamically (downside of > this is an extra indirection to get at the slots). > > - io_tlb_default_mem.nslabs tells you whether the thing is valid > > - swiotlb_exit() frees the slots array and clears the rest of the > structure to 0. I also extended it to free the actual slabs, but I'm > not sure why it wasn't doing that before. > > So a non-NULL dev->dma_io_tlb_mem should always be valid to follow. FWIW I was pondering the question of whether to do something along those lines or just scrap the default assignment entirely, so since I hadn't got round to saying that I've gone ahead and hacked up the alternative (similarly untested) for comparison :) TBH I'm still not sure which one I prefer... Robin. ----->8----- diff --git a/drivers/base/core.c b/drivers/base/core.c index ea5b85354526..394abf184c1a 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -2847,9 +2847,6 @@ void device_initialize(struct device *dev) defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL) dev->dma_coherent = dma_default_coherent; #endif -#ifdef CONFIG_SWIOTLB - dev->dma_io_tlb_mem = io_tlb_default_mem; -#endif } EXPORT_SYMBOL_GPL(device_initialize); diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h index 39284ff2a6cd..620f16d89a98 100644 --- a/include/linux/swiotlb.h +++ b/include/linux/swiotlb.h @@ -107,16 +107,21 @@ struct io_tlb_mem { }; extern struct io_tlb_mem *io_tlb_default_mem; +static inline struct io_tlb_mem *dev_iotlb_mem(struct device *dev) +{ + return dev->dma_io_tlb_mem ?: io_tlb_default_mem; +} + static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); return mem && paddr >= mem->start && paddr < mem->end; } static inline bool is_swiotlb_force_bounce(struct device *dev) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); return mem && mem->force_bounce; } @@ -167,7 +172,7 @@ bool swiotlb_free(struct device *dev, struct page *page, size_t size); static inline bool is_swiotlb_for_alloc(struct device *dev) { - return dev->dma_io_tlb_mem->for_alloc; + return dev_iotlb_mem(dev)->for_alloc; } #else static inline struct page *swiotlb_alloc(struct device *dev, size_t size) diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c index b7f76bca89bf..f4942149f87d 100644 --- a/kernel/dma/swiotlb.c +++ b/kernel/dma/swiotlb.c @@ -359,7 +359,7 @@ static unsigned int swiotlb_align_offset(struct device *dev, u64 addr) static void swiotlb_bounce(struct device *dev, phys_addr_t tlb_addr, size_t size, enum dma_data_direction dir) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); int index = (tlb_addr - mem->start) >> IO_TLB_SHIFT; phys_addr_t orig_addr = mem->slots[index].orig_addr; size_t alloc_size = mem->slots[index].alloc_size; @@ -440,7 +440,7 @@ static unsigned int wrap_index(struct io_tlb_mem *mem, unsigned int index) static int swiotlb_find_slots(struct device *dev, phys_addr_t orig_addr, size_t alloc_size) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); unsigned long boundary_mask = dma_get_seg_boundary(dev); dma_addr_t tbl_dma_addr = phys_to_dma_unencrypted(dev, mem->start) & boundary_mask; @@ -522,7 +522,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr, size_t mapping_size, size_t alloc_size, enum dma_data_direction dir, unsigned long attrs) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); unsigned int offset = swiotlb_align_offset(dev, orig_addr); unsigned int i; int index; @@ -565,7 +565,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr, static void swiotlb_release_slots(struct device *dev, phys_addr_t tlb_addr) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); unsigned long flags; unsigned int offset = swiotlb_align_offset(dev, tlb_addr); int index = (tlb_addr - offset - mem->start) >> IO_TLB_SHIFT; @@ -682,7 +682,7 @@ size_t swiotlb_max_mapping_size(struct device *dev) bool is_swiotlb_active(struct device *dev) { - return dev->dma_io_tlb_mem != NULL; + return dev_iotlb_mem(dev) != NULL; } EXPORT_SYMBOL_GPL(is_swiotlb_active); @@ -729,7 +729,7 @@ static void rmem_swiotlb_debugfs_init(struct reserved_mem *rmem) struct page *swiotlb_alloc(struct device *dev, size_t size) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); phys_addr_t tlb_addr; int index; @@ -792,7 +792,7 @@ static int rmem_swiotlb_device_init(struct reserved_mem *rmem, static void rmem_swiotlb_device_release(struct reserved_mem *rmem, struct device *dev) { - dev->dma_io_tlb_mem = io_tlb_default_mem; + dev->dma_io_tlb_mem = NULL; } static const struct reserved_mem_ops rmem_swiotlb_ops = { _______________________________________________ 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=-10.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham 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 0E433C07E96 for ; Tue, 6 Jul 2021 14:01:21 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (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 C260A619B4 for ; Tue, 6 Jul 2021 14:01:20 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C260A619B4 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 453026E4CA; Tue, 6 Jul 2021 14:01:20 +0000 (UTC) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by gabe.freedesktop.org (Postfix) with ESMTP id 73B8D6E4CA; Tue, 6 Jul 2021 14:01:19 +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 6E5E031B; Tue, 6 Jul 2021 07:01:18 -0700 (PDT) Received: from [10.57.40.45] (unknown [10.57.40.45]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 461163F73B; Tue, 6 Jul 2021 07:01:11 -0700 (PDT) Subject: Re: [PATCH v15 06/12] swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing To: Will Deacon , Christoph Hellwig References: <20210630114348.GA8383@willie-the-truck> <20210701074045.GA9436@willie-the-truck> <20210702135856.GB11132@willie-the-truck> <0f7bd903-e309-94a0-21d7-f0e8e9546018@arm.com> <20210705190352.GA19461@willie-the-truck> <20210706044848.GA13640@lst.de> <20210706132422.GA20327@willie-the-truck> From: Robin Murphy Message-ID: Date: Tue, 6 Jul 2021 15:01:04 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <20210706132422.GA20327@willie-the-truck> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Jim Quinlan , heikki.krogerus@linux.intel.com, linux-devicetree , peterz@infradead.org, dri-devel@lists.freedesktop.org, chris@chris-wilson.co.uk, grant.likely@arm.com, paulus@samba.org, Frank Rowand , mingo@kernel.org, Jianxiong Gao , Stefano Stabellini , Saravana Kannan , mpe@ellerman.id.au, "Rafael J . Wysocki" , Bartosz Golaszewski , matthew.auld@intel.com, linux-pci@vger.kernel.org, xen-devel@lists.xenproject.org, Thierry Reding , bskeggs@redhat.com, Nicolas Boichat , thomas.hellstrom@linux.intel.com, jgross@suse.com, Konrad Rzeszutek Wilk , intel-gfx@lists.freedesktop.org, Nathan Chancellor , Rob Herring , rodrigo.vivi@intel.com, Bjorn Helgaas , Claire Chang , Dan Williams , Andy Shevchenko , boris.ostrovsky@oracle.com, airlied@linux.ie, Greg KH , Randy Dunlap , Qian Cai , lkml , "list@263.net:IOMMU DRIVERS" , xypron.glpk@gmx.de, Tom Lendacky , linuxppc-dev@lists.ozlabs.org, bauerman@linux.ibm.com Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On 2021-07-06 14:24, Will Deacon wrote: > On Tue, Jul 06, 2021 at 06:48:48AM +0200, Christoph Hellwig wrote: >> On Mon, Jul 05, 2021 at 08:03:52PM +0100, Will Deacon wrote: >>> So at this point, the AMD IOMMU driver does: >>> >>> swiotlb = (iommu_default_passthrough() || sme_me_mask) ? 1 : 0; >>> >>> where 'swiotlb' is a global variable indicating whether or not swiotlb >>> is in use. It's picked up a bit later on by pci_swiotlb_late_init(), which >>> will call swiotlb_exit() if 'swiotlb' is false. >>> >>> Now, that used to work fine, because swiotlb_exit() clears >>> 'io_tlb_default_mem' to NULL, but now with the restricted DMA changes, I >>> think that all the devices which have successfully probed beforehand will >>> have stale pointers to the freed structure in their 'dev->dma_io_tlb_mem' >>> field. >> >> Yeah. I don't think we can do that anymore, and I also think it is >> a bad idea to start with. > > I've had a crack at reworking things along the following lines: > > - io_tlb_default_mem now lives in the BSS, the flexible array member > is now a pointer and that part is allocated dynamically (downside of > this is an extra indirection to get at the slots). > > - io_tlb_default_mem.nslabs tells you whether the thing is valid > > - swiotlb_exit() frees the slots array and clears the rest of the > structure to 0. I also extended it to free the actual slabs, but I'm > not sure why it wasn't doing that before. > > So a non-NULL dev->dma_io_tlb_mem should always be valid to follow. FWIW I was pondering the question of whether to do something along those lines or just scrap the default assignment entirely, so since I hadn't got round to saying that I've gone ahead and hacked up the alternative (similarly untested) for comparison :) TBH I'm still not sure which one I prefer... Robin. ----->8----- diff --git a/drivers/base/core.c b/drivers/base/core.c index ea5b85354526..394abf184c1a 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -2847,9 +2847,6 @@ void device_initialize(struct device *dev) defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL) dev->dma_coherent = dma_default_coherent; #endif -#ifdef CONFIG_SWIOTLB - dev->dma_io_tlb_mem = io_tlb_default_mem; -#endif } EXPORT_SYMBOL_GPL(device_initialize); diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h index 39284ff2a6cd..620f16d89a98 100644 --- a/include/linux/swiotlb.h +++ b/include/linux/swiotlb.h @@ -107,16 +107,21 @@ struct io_tlb_mem { }; extern struct io_tlb_mem *io_tlb_default_mem; +static inline struct io_tlb_mem *dev_iotlb_mem(struct device *dev) +{ + return dev->dma_io_tlb_mem ?: io_tlb_default_mem; +} + static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); return mem && paddr >= mem->start && paddr < mem->end; } static inline bool is_swiotlb_force_bounce(struct device *dev) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); return mem && mem->force_bounce; } @@ -167,7 +172,7 @@ bool swiotlb_free(struct device *dev, struct page *page, size_t size); static inline bool is_swiotlb_for_alloc(struct device *dev) { - return dev->dma_io_tlb_mem->for_alloc; + return dev_iotlb_mem(dev)->for_alloc; } #else static inline struct page *swiotlb_alloc(struct device *dev, size_t size) diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c index b7f76bca89bf..f4942149f87d 100644 --- a/kernel/dma/swiotlb.c +++ b/kernel/dma/swiotlb.c @@ -359,7 +359,7 @@ static unsigned int swiotlb_align_offset(struct device *dev, u64 addr) static void swiotlb_bounce(struct device *dev, phys_addr_t tlb_addr, size_t size, enum dma_data_direction dir) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); int index = (tlb_addr - mem->start) >> IO_TLB_SHIFT; phys_addr_t orig_addr = mem->slots[index].orig_addr; size_t alloc_size = mem->slots[index].alloc_size; @@ -440,7 +440,7 @@ static unsigned int wrap_index(struct io_tlb_mem *mem, unsigned int index) static int swiotlb_find_slots(struct device *dev, phys_addr_t orig_addr, size_t alloc_size) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); unsigned long boundary_mask = dma_get_seg_boundary(dev); dma_addr_t tbl_dma_addr = phys_to_dma_unencrypted(dev, mem->start) & boundary_mask; @@ -522,7 +522,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr, size_t mapping_size, size_t alloc_size, enum dma_data_direction dir, unsigned long attrs) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); unsigned int offset = swiotlb_align_offset(dev, orig_addr); unsigned int i; int index; @@ -565,7 +565,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr, static void swiotlb_release_slots(struct device *dev, phys_addr_t tlb_addr) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); unsigned long flags; unsigned int offset = swiotlb_align_offset(dev, tlb_addr); int index = (tlb_addr - offset - mem->start) >> IO_TLB_SHIFT; @@ -682,7 +682,7 @@ size_t swiotlb_max_mapping_size(struct device *dev) bool is_swiotlb_active(struct device *dev) { - return dev->dma_io_tlb_mem != NULL; + return dev_iotlb_mem(dev) != NULL; } EXPORT_SYMBOL_GPL(is_swiotlb_active); @@ -729,7 +729,7 @@ static void rmem_swiotlb_debugfs_init(struct reserved_mem *rmem) struct page *swiotlb_alloc(struct device *dev, size_t size) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); phys_addr_t tlb_addr; int index; @@ -792,7 +792,7 @@ static int rmem_swiotlb_device_init(struct reserved_mem *rmem, static void rmem_swiotlb_device_release(struct reserved_mem *rmem, struct device *dev) { - dev->dma_io_tlb_mem = io_tlb_default_mem; + dev->dma_io_tlb_mem = NULL; } static const struct reserved_mem_ops rmem_swiotlb_ops = { 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=-10.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable 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 DC7CBC07E9B for ; Tue, 6 Jul 2021 18:52:37 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (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 A281261358 for ; Tue, 6 Jul 2021 18:52:37 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A281261358 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 4DB966E588; Tue, 6 Jul 2021 18:52:33 +0000 (UTC) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by gabe.freedesktop.org (Postfix) with ESMTP id 73B8D6E4CA; Tue, 6 Jul 2021 14:01:19 +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 6E5E031B; Tue, 6 Jul 2021 07:01:18 -0700 (PDT) Received: from [10.57.40.45] (unknown [10.57.40.45]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 461163F73B; Tue, 6 Jul 2021 07:01:11 -0700 (PDT) To: Will Deacon , Christoph Hellwig References: <20210630114348.GA8383@willie-the-truck> <20210701074045.GA9436@willie-the-truck> <20210702135856.GB11132@willie-the-truck> <0f7bd903-e309-94a0-21d7-f0e8e9546018@arm.com> <20210705190352.GA19461@willie-the-truck> <20210706044848.GA13640@lst.de> <20210706132422.GA20327@willie-the-truck> From: Robin Murphy Message-ID: Date: Tue, 6 Jul 2021 15:01:04 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <20210706132422.GA20327@willie-the-truck> Content-Language: en-GB X-Mailman-Approved-At: Tue, 06 Jul 2021 18:52:32 +0000 Subject: Re: [Intel-gfx] [PATCH v15 06/12] swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Jim Quinlan , heikki.krogerus@linux.intel.com, linux-devicetree , peterz@infradead.org, benh@kernel.crashing.org, dri-devel@lists.freedesktop.org, chris@chris-wilson.co.uk, grant.likely@arm.com, paulus@samba.org, Frank Rowand , mingo@kernel.org, Jianxiong Gao , Stefano Stabellini , Saravana Kannan , mpe@ellerman.id.au, "Rafael J . Wysocki" , Bartosz Golaszewski , matthew.auld@intel.com, linux-pci@vger.kernel.org, xen-devel@lists.xenproject.org, Thierry Reding , bskeggs@redhat.com, Nicolas Boichat , thomas.hellstrom@linux.intel.com, jgross@suse.com, Konrad Rzeszutek Wilk , intel-gfx@lists.freedesktop.org, Nathan Chancellor , Rob Herring , Bjorn Helgaas , Claire Chang , Dan Williams , Andy Shevchenko , boris.ostrovsky@oracle.com, airlied@linux.ie, Greg KH , Randy Dunlap , Qian Cai , lkml , "list@263.net:IOMMU DRIVERS" , xypron.glpk@gmx.de, Tom Lendacky , linuxppc-dev@lists.ozlabs.org, bauerman@linux.ibm.com Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On 2021-07-06 14:24, Will Deacon wrote: > On Tue, Jul 06, 2021 at 06:48:48AM +0200, Christoph Hellwig wrote: >> On Mon, Jul 05, 2021 at 08:03:52PM +0100, Will Deacon wrote: >>> So at this point, the AMD IOMMU driver does: >>> >>> swiotlb = (iommu_default_passthrough() || sme_me_mask) ? 1 : 0; >>> >>> where 'swiotlb' is a global variable indicating whether or not swiotlb >>> is in use. It's picked up a bit later on by pci_swiotlb_late_init(), which >>> will call swiotlb_exit() if 'swiotlb' is false. >>> >>> Now, that used to work fine, because swiotlb_exit() clears >>> 'io_tlb_default_mem' to NULL, but now with the restricted DMA changes, I >>> think that all the devices which have successfully probed beforehand will >>> have stale pointers to the freed structure in their 'dev->dma_io_tlb_mem' >>> field. >> >> Yeah. I don't think we can do that anymore, and I also think it is >> a bad idea to start with. > > I've had a crack at reworking things along the following lines: > > - io_tlb_default_mem now lives in the BSS, the flexible array member > is now a pointer and that part is allocated dynamically (downside of > this is an extra indirection to get at the slots). > > - io_tlb_default_mem.nslabs tells you whether the thing is valid > > - swiotlb_exit() frees the slots array and clears the rest of the > structure to 0. I also extended it to free the actual slabs, but I'm > not sure why it wasn't doing that before. > > So a non-NULL dev->dma_io_tlb_mem should always be valid to follow. FWIW I was pondering the question of whether to do something along those lines or just scrap the default assignment entirely, so since I hadn't got round to saying that I've gone ahead and hacked up the alternative (similarly untested) for comparison :) TBH I'm still not sure which one I prefer... Robin. ----->8----- diff --git a/drivers/base/core.c b/drivers/base/core.c index ea5b85354526..394abf184c1a 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -2847,9 +2847,6 @@ void device_initialize(struct device *dev) defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL) dev->dma_coherent = dma_default_coherent; #endif -#ifdef CONFIG_SWIOTLB - dev->dma_io_tlb_mem = io_tlb_default_mem; -#endif } EXPORT_SYMBOL_GPL(device_initialize); diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h index 39284ff2a6cd..620f16d89a98 100644 --- a/include/linux/swiotlb.h +++ b/include/linux/swiotlb.h @@ -107,16 +107,21 @@ struct io_tlb_mem { }; extern struct io_tlb_mem *io_tlb_default_mem; +static inline struct io_tlb_mem *dev_iotlb_mem(struct device *dev) +{ + return dev->dma_io_tlb_mem ?: io_tlb_default_mem; +} + static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); return mem && paddr >= mem->start && paddr < mem->end; } static inline bool is_swiotlb_force_bounce(struct device *dev) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); return mem && mem->force_bounce; } @@ -167,7 +172,7 @@ bool swiotlb_free(struct device *dev, struct page *page, size_t size); static inline bool is_swiotlb_for_alloc(struct device *dev) { - return dev->dma_io_tlb_mem->for_alloc; + return dev_iotlb_mem(dev)->for_alloc; } #else static inline struct page *swiotlb_alloc(struct device *dev, size_t size) diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c index b7f76bca89bf..f4942149f87d 100644 --- a/kernel/dma/swiotlb.c +++ b/kernel/dma/swiotlb.c @@ -359,7 +359,7 @@ static unsigned int swiotlb_align_offset(struct device *dev, u64 addr) static void swiotlb_bounce(struct device *dev, phys_addr_t tlb_addr, size_t size, enum dma_data_direction dir) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); int index = (tlb_addr - mem->start) >> IO_TLB_SHIFT; phys_addr_t orig_addr = mem->slots[index].orig_addr; size_t alloc_size = mem->slots[index].alloc_size; @@ -440,7 +440,7 @@ static unsigned int wrap_index(struct io_tlb_mem *mem, unsigned int index) static int swiotlb_find_slots(struct device *dev, phys_addr_t orig_addr, size_t alloc_size) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); unsigned long boundary_mask = dma_get_seg_boundary(dev); dma_addr_t tbl_dma_addr = phys_to_dma_unencrypted(dev, mem->start) & boundary_mask; @@ -522,7 +522,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr, size_t mapping_size, size_t alloc_size, enum dma_data_direction dir, unsigned long attrs) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); unsigned int offset = swiotlb_align_offset(dev, orig_addr); unsigned int i; int index; @@ -565,7 +565,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr, static void swiotlb_release_slots(struct device *dev, phys_addr_t tlb_addr) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); unsigned long flags; unsigned int offset = swiotlb_align_offset(dev, tlb_addr); int index = (tlb_addr - offset - mem->start) >> IO_TLB_SHIFT; @@ -682,7 +682,7 @@ size_t swiotlb_max_mapping_size(struct device *dev) bool is_swiotlb_active(struct device *dev) { - return dev->dma_io_tlb_mem != NULL; + return dev_iotlb_mem(dev) != NULL; } EXPORT_SYMBOL_GPL(is_swiotlb_active); @@ -729,7 +729,7 @@ static void rmem_swiotlb_debugfs_init(struct reserved_mem *rmem) struct page *swiotlb_alloc(struct device *dev, size_t size) { - struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + struct io_tlb_mem *mem = dev_iotlb_mem(dev); phys_addr_t tlb_addr; int index; @@ -792,7 +792,7 @@ static int rmem_swiotlb_device_init(struct reserved_mem *rmem, static void rmem_swiotlb_device_release(struct reserved_mem *rmem, struct device *dev) { - dev->dma_io_tlb_mem = io_tlb_default_mem; + dev->dma_io_tlb_mem = NULL; } static const struct reserved_mem_ops rmem_swiotlb_ops = { _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx