From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 96E0EC433EF for ; Wed, 29 Jun 2022 09:05:53 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DAC086B0074; Wed, 29 Jun 2022 05:05:52 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D34FB8E0002; Wed, 29 Jun 2022 05:05:52 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BD5D38E0001; Wed, 29 Jun 2022 05:05:52 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id A76716B0074 for ; Wed, 29 Jun 2022 05:05:52 -0400 (EDT) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 68E872116A for ; Wed, 29 Jun 2022 09:05:52 +0000 (UTC) X-FDA: 79630690944.26.E55D215 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf26.hostedemail.com (Postfix) with ESMTP id 7385B140008 for ; Wed, 29 Jun 2022 09:05:51 +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 B53E41477; Wed, 29 Jun 2022 02:05:50 -0700 (PDT) Received: from [10.57.85.71] (unknown [10.57.85.71]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id BA2B53F792; Wed, 29 Jun 2022 02:05:45 -0700 (PDT) Message-ID: Date: Wed, 29 Jun 2022 10:05:40 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: [PATCH v7 01/21] lib/scatterlist: add flag for indicating P2PDMA segments in an SGL Content-Language: en-GB To: Logan Gunthorpe , linux-kernel@vger.kernel.org, linux-nvme@lists.infradead.org, linux-block@vger.kernel.org, linux-pci@vger.kernel.org, linux-mm@kvack.org, iommu@lists.linux-foundation.org Cc: Stephen Bates , Christoph Hellwig , Dan Williams , Jason Gunthorpe , =?UTF-8?Q?Christian_K=c3=b6nig?= , John Hubbard , Don Dutile , Matthew Wilcox , Daniel Vetter , Minturn Dave B , Jason Ekstrand , Dave Hansen , Xiong Jianxin , Bjorn Helgaas , Ira Weiny , Martin Oliveira , Chaitanya Kulkarni , Ralph Campbell , Chaitanya Kulkarni References: <20220615161233.17527-1-logang@deltatee.com> <20220615161233.17527-2-logang@deltatee.com> From: Robin Murphy In-Reply-To: <20220615161233.17527-2-logang@deltatee.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1656493552; a=rsa-sha256; cv=none; b=KCZ2FC1oF8q/PFaEMP1WJUnt3vn8u4L6e7y0XFhhyDIV09yXyECJavhOR0kIFES3uG8tVC Z9/yatQJI+jq8HUaYSxBlx1V3PBr/Ch5+nCfcidHZIS3pl+0uaMO322dHjsxd466PJExqm ah04xcZfZuSwqxj82CFrUhppxQpDqWE= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=none; spf=pass (imf26.hostedemail.com: domain of robin.murphy@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=robin.murphy@arm.com; dmarc=pass (policy=none) header.from=arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1656493552; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=e/AX9LcU4en+nJewDe5gXnjNunwfU9eHtMHBKdj4UCI=; b=jEWJJxdLFi0siD6twSjJLd8AoOS3a7Sjmv9GZnScZaWMu9yOq1A1ozG3kqAxUGv0Zks0ho 8Jq7bF78B6Gq7g4tlNjvK07kBnqN0mirZzLXZX+8fLhiw1tDv30zC8jjybmbRgtq+riUjG h1EwCDCs3lgEbfIR7IMY8Rs87OCaIpY= X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 7385B140008 X-Rspam-User: X-Stat-Signature: tepb63dytqo448fdd94xqan5kk649twr Authentication-Results: imf26.hostedemail.com; dkim=none; spf=pass (imf26.hostedemail.com: domain of robin.murphy@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=robin.murphy@arm.com; dmarc=pass (policy=none) header.from=arm.com X-HE-Tag: 1656493551-984572 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On 2022-06-15 17:12, Logan Gunthorpe wrote: > Make use of the third free LSB in scatterlist's page_link on 64bit systems. > > The extra bit will be used by dma_[un]map_sg_p2pdma() to determine when a > given SGL segments dma_address points to a PCI bus address. > dma_unmap_sg_p2pdma() will need to perform different cleanup when a > segment is marked as a bus address. > > The new bit will only be used when CONFIG_PCI_P2PDMA is set; this means > PCI P2PDMA will require CONFIG_64BIT. This should be acceptable as the > majority of P2PDMA use cases are restricted to newer root complexes and > roughly require the extra address space for memory BARs used in the > transactions. > > Signed-off-by: Logan Gunthorpe > Reviewed-by: Chaitanya Kulkarni > --- > drivers/pci/Kconfig | 5 +++++ > include/linux/scatterlist.h | 44 ++++++++++++++++++++++++++++++++++++- > 2 files changed, 48 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig > index 133c73207782..5cc7cba1941f 100644 > --- a/drivers/pci/Kconfig > +++ b/drivers/pci/Kconfig > @@ -164,6 +164,11 @@ config PCI_PASID > config PCI_P2PDMA > bool "PCI peer-to-peer transfer support" > depends on ZONE_DEVICE > + # > + # The need for the scatterlist DMA bus address flag means PCI P2PDMA > + # requires 64bit > + # > + depends on 64BIT > select GENERIC_ALLOCATOR > help > Enableѕ drivers to do PCI peer-to-peer transactions to and from > diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h > index 7ff9d6386c12..6561ca8aead8 100644 > --- a/include/linux/scatterlist.h > +++ b/include/linux/scatterlist.h > @@ -64,12 +64,24 @@ struct sg_append_table { > #define SG_CHAIN 0x01UL > #define SG_END 0x02UL > > +/* > + * bit 2 is the third free bit in the page_link on 64bit systems which > + * is used by dma_unmap_sg() to determine if the dma_address is a > + * bus address when doing P2PDMA. > + */ > +#ifdef CONFIG_PCI_P2PDMA > +#define SG_DMA_BUS_ADDRESS 0x04UL > +static_assert(__alignof__(struct page) >= 8); > +#else > +#define SG_DMA_BUS_ADDRESS 0x00UL > +#endif > + > /* > * We overload the LSB of the page pointer to indicate whether it's > * a valid sg entry, or whether it points to the start of a new scatterlist. > * Those low bits are there for everyone! (thanks mason :-) > */ > -#define SG_PAGE_LINK_MASK (SG_CHAIN | SG_END) > +#define SG_PAGE_LINK_MASK (SG_CHAIN | SG_END | SG_DMA_BUS_ADDRESS) > > static inline unsigned int __sg_flags(struct scatterlist *sg) > { > @@ -91,6 +103,11 @@ static inline bool sg_is_last(struct scatterlist *sg) > return __sg_flags(sg) & SG_END; > } > > +static inline bool sg_is_dma_bus_address(struct scatterlist *sg) > +{ > + return __sg_flags(sg) & SG_DMA_BUS_ADDRESS; > +} > + > /** > * sg_assign_page - Assign a given page to an SG entry > * @sg: SG entry > @@ -245,6 +262,31 @@ static inline void sg_unmark_end(struct scatterlist *sg) > sg->page_link &= ~SG_END; > } > > +/** > + * sg_dma_mark_bus address - Mark the scatterlist entry as a bus address > + * @sg: SG entryScatterlist entryScatterlist? > + * > + * Description: > + * Marks the passed in sg entry to indicate that the dma_address is > + * a bus address and doesn't need to be unmapped. > + **/ > +static inline void sg_dma_mark_bus_address(struct scatterlist *sg) > +{ > + sg->page_link |= SG_DMA_BUS_ADDRESS; > +} > + > +/** > + * sg_unmark_pci_p2pdma - Unmark the scatterlist entry as a bus address > + * @sg: SG entryScatterlist > + * > + * Description: > + * Clears the bus address mark. > + **/ > +static inline void sg_dma_unmark_bus_address(struct scatterlist *sg) > +{ > + sg->page_link &= ~SG_DMA_BUS_ADDRESS; > +} Does this serve any useful purpose? If a page is determined to be device memory, it's not going to suddenly stop being device memory, and if the underlying sg is recycled to point elsewhere then sg_assign_page() will still (correctly) clear this flag anyway. Trying to reason about this beyond superficial API symmetry - i.e. why exactly would a caller need to call it, and what would the implications be of failing to do so - seems to lead straight to confusion. In fact I'd be inclined to have sg_assign_page() be responsible for setting the flag automatically as well, and thus not need sg_dma_mark_bus_address() either, however I can see the argument for doing it this way round to not entangle the APIs too much, so I don't have any great objection to that. Thanks, Robin. > + > /** > * sg_phys - Return physical address of an sg entry > * @sg: SG entry