[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4a1f06c2-a120-56f5-37a7-e4625a5f275c@nvidia.com>
Date: Sun, 2 May 2021 15:34:17 -0700
From: John Hubbard <jhubbard@...dia.com>
To: Logan Gunthorpe <logang@...tatee.com>,
<linux-kernel@...r.kernel.org>, <linux-nvme@...ts.infradead.org>,
<linux-block@...r.kernel.org>, <linux-pci@...r.kernel.org>,
<linux-mm@...ck.org>, <iommu@...ts.linux-foundation.org>
CC: Stephen Bates <sbates@...thlin.com>,
Christoph Hellwig <hch@....de>,
Dan Williams <dan.j.williams@...el.com>,
Jason Gunthorpe <jgg@...pe.ca>,
Christian König <christian.koenig@....com>,
Don Dutile <ddutile@...hat.com>,
Matthew Wilcox <willy@...radead.org>,
Daniel Vetter <daniel.vetter@...ll.ch>,
Jakowski Andrzej <andrzej.jakowski@...el.com>,
Minturn Dave B <dave.b.minturn@...el.com>,
Jason Ekstrand <jason@...kstrand.net>,
Dave Hansen <dave.hansen@...ux.intel.com>,
Xiong Jianxin <jianxin.xiong@...el.com>,
Bjorn Helgaas <helgaas@...nel.org>,
Ira Weiny <ira.weiny@...el.com>,
Robin Murphy <robin.murphy@....com>
Subject: Re: [PATCH 06/16] lib/scatterlist: Add flag for indicating P2PDMA
segments in an SGL
On 4/8/21 10:01 AM, 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 P2PDMA.
>
> Using this bit requires adding an additional dependency on CONFIG_64BIT to
> CONFIG_PCI_P2PDMA. 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.
Totally agree with the CONFIG_64BIT call.
Also, I have failed to find anything wrong with this patch. :)
Reviewed-by: John Hubbard <jhubbard@...dia.com>
thanks,
--
John Hubbard
NVIDIA
>
> Signed-off-by: Logan Gunthorpe <logang@...tatee.com>
> ---
> drivers/pci/Kconfig | 2 +-
> include/linux/scatterlist.h | 49 ++++++++++++++++++++++++++++++++++---
> 2 files changed, 46 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index 0c473d75e625..90b4bddb3300 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -163,7 +163,7 @@ config PCI_PASID
>
> config PCI_P2PDMA
> bool "PCI peer-to-peer transfer support"
> - depends on ZONE_DEVICE
> + depends on ZONE_DEVICE && 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 6f70572b2938..5525d3ebf36f 100644
> --- a/include/linux/scatterlist.h
> +++ b/include/linux/scatterlist.h
> @@ -58,6 +58,21 @@ struct sg_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 PCI
> + * bus address when doing P2PDMA.
> + * Note: CONFIG_PCI_P2PDMA depends on CONFIG_64BIT because of this.
> + */
> +
> +#ifdef CONFIG_PCI_P2PDMA
> +#define SG_PCI_P2PDMA 0x04UL
> +#else
> +#define SG_PCI_P2PDMA 0x00UL
> +#endif
> +
> +#define SG_PAGE_LINK_MASK (SG_CHAIN | SG_END | SG_PCI_P2PDMA)
> +
> /*
> * 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.
> @@ -65,8 +80,9 @@ struct sg_table {
> */
> #define sg_is_chain(sg) ((sg)->page_link & SG_CHAIN)
> #define sg_is_last(sg) ((sg)->page_link & SG_END)
> +#define sg_is_pci_p2pdma(sg) ((sg)->page_link & SG_PCI_P2PDMA)
> #define sg_chain_ptr(sg) \
> - ((struct scatterlist *) ((sg)->page_link & ~(SG_CHAIN | SG_END)))
> + ((struct scatterlist *) ((sg)->page_link & ~SG_PAGE_LINK_MASK))
>
> /**
> * sg_assign_page - Assign a given page to an SG entry
> @@ -80,13 +96,13 @@ struct sg_table {
> **/
> static inline void sg_assign_page(struct scatterlist *sg, struct page *page)
> {
> - unsigned long page_link = sg->page_link & (SG_CHAIN | SG_END);
> + unsigned long page_link = sg->page_link & SG_PAGE_LINK_MASK;
>
> /*
> * In order for the low bit stealing approach to work, pages
> * must be aligned at a 32-bit boundary as a minimum.
> */
> - BUG_ON((unsigned long) page & (SG_CHAIN | SG_END));
> + BUG_ON((unsigned long) page & SG_PAGE_LINK_MASK);
> #ifdef CONFIG_DEBUG_SG
> BUG_ON(sg_is_chain(sg));
> #endif
> @@ -120,7 +136,7 @@ static inline struct page *sg_page(struct scatterlist *sg)
> #ifdef CONFIG_DEBUG_SG
> BUG_ON(sg_is_chain(sg));
> #endif
> - return (struct page *)((sg)->page_link & ~(SG_CHAIN | SG_END));
> + return (struct page *)((sg)->page_link & ~SG_PAGE_LINK_MASK);
> }
>
> /**
> @@ -222,6 +238,31 @@ static inline void sg_unmark_end(struct scatterlist *sg)
> sg->page_link &= ~SG_END;
> }
>
> +/**
> + * sg_mark_pci_p2pdma - Mark the scatterlist entry for PCI p2pdma
> + * @sg: SG entryScatterlist
> + *
> + * Description:
> + * Marks the passed in sg entry to indicate that the dma_address is
> + * a PCI bus address.
> + **/
> +static inline void sg_mark_pci_p2pdma(struct scatterlist *sg)
> +{
> + sg->page_link |= SG_PCI_P2PDMA;
> +}
> +
> +/**
> + * sg_unmark_pci_p2pdma - Unmark the scatterlist entry for PCI p2pdma
> + * @sg: SG entryScatterlist
> + *
> + * Description:
> + * Clears the PCI P2PDMA mark
> + **/
> +static inline void sg_unmark_pci_p2pdma(struct scatterlist *sg)
> +{
> + sg->page_link &= ~SG_PCI_P2PDMA;
> +}
> +
> /**
> * sg_phys - Return physical address of an sg entry
> * @sg: SG entry
>
Powered by blists - more mailing lists