| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <090c9792-bddd-e99e-92b0-724a6fbbef03@nvidia.com>
Date: Sun, 2 May 2021 15:52:12 -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 08/16] PCI/P2PDMA: Introduce helpers for dma_map_sg
implementations
On 4/8/21 10:01 AM, Logan Gunthorpe wrote:
> Add pci_p2pdma_map_segment() as a helper for simple dma_map_sg()
> implementations. It takes an scatterlist segment that must point to a
> pci_p2pdma struct page and will map it if the mapping requires a bus
> address.
>
> The return value indicates whether the mapping required a bus address
> or whether the caller still needs to map the segment normally. If the
> segment should not be mapped, -EREMOTEIO is returned.
>
> This helper uses a state structure to track the changes to the
> pgmap across calls and avoid needing to lookup into the xarray for
> every page.
>
> Also add pci_p2pdma_map_bus_segment() which is useful for IOMMU
> dma_map_sg() implementations where the sg segment containing the page
> differs from the sg segment containing the DMA address.
>
Hard to properly review this patch by itself, because it doesn't show
any callers of the new routine. If you end up shuffling patches and/or
refactoring for other reasons, it would be nice if the next version of
the series included a caller here. In particular, the new
pci_p2pdma_map_state concept is something I want to double-check, to
see if it hits any common pitfalls. I'm sure it doesn't, but still. :)
Meanwhile, I'll keep working through the series, and come back to this
one when I have seen the callers.
thanks,
--
John Hubbard
NVIDIA
> Signed-off-by: Logan Gunthorpe <logang@...tatee.com>
> ---
> drivers/pci/p2pdma.c | 65 ++++++++++++++++++++++++++++++++++++++
> include/linux/pci-p2pdma.h | 21 ++++++++++++
> 2 files changed, 86 insertions(+)
>
> diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
> index 38c93f57a941..44ad7664e875 100644
> --- a/drivers/pci/p2pdma.c
> +++ b/drivers/pci/p2pdma.c
> @@ -923,6 +923,71 @@ void pci_p2pdma_unmap_sg_attrs(struct device *dev, struct scatterlist *sg,
> }
> EXPORT_SYMBOL_GPL(pci_p2pdma_unmap_sg_attrs);
>
> +/**
> + * pci_p2pdma_map_segment - map an sg segment determining the mapping type
> + * @state: State structure that should be declared on the stack outside of
> + * the for_each_sg() loop and initialized to zero.
> + * @dev: DMA device that's doing the mapping operation
> + * @sg: scatterlist segment to map
> + * @attrs: dma mapping attributes
> + *
> + * This is a helper to be used by non-iommu dma_map_sg() implementations where
> + * the sg segment is the same for the page_link and the dma_address.
> + *
> + * Attempt to map a single segment in an SGL with the PCI bus address.
> + * The segment must point to a PCI P2PDMA page and thus must be
> + * wrapped in a is_pci_p2pdma_page(sg_page(sg)) check.
> + *
> + * Returns 1 if the segment was mapped, 0 if the segment should be mapped
> + * directly (or through the IOMMU) and -EREMOTEIO if the segment should not
> + * be mapped at all.
> + */
> +int pci_p2pdma_map_segment(struct pci_p2pdma_map_state *state,
> + struct device *dev, struct scatterlist *sg,
> + unsigned long dma_attrs)
> +{
> + if (state->pgmap != sg_page(sg)->pgmap) {
> + state->pgmap = sg_page(sg)->pgmap;
> + state->map = pci_p2pdma_map_type(state->pgmap, dev, dma_attrs);
> + state->bus_off = to_p2p_pgmap(state->pgmap)->bus_offset;
> + }
> +
> + switch (state->map) {
> + case PCI_P2PDMA_MAP_BUS_ADDR:
> + sg->dma_address = sg_phys(sg) + state->bus_off;
> + sg_dma_len(sg) = sg->length;
> + sg_mark_pci_p2pdma(sg);
> + return 1;
> + case PCI_P2PDMA_MAP_THRU_HOST_BRIDGE:
> + return 0;
> + default:
> + return -EREMOTEIO;
> + }
> +}
> +
> +/**
> + * pci_p2pdma_map_bus_segment - map an sg segment pre determined to
> + * be mapped with PCI_P2PDMA_MAP_BUS_ADDR
> + * @pg_sg: scatterlist segment with the page to map
> + * @dma_sg: scatterlist segment to assign a dma address to
> + *
> + * This is a helper for iommu dma_map_sg() implementations when the
> + * segment for the dma address differs from the segment containing the
> + * source page.
> + *
> + * pci_p2pdma_map_type() must have already been called on the pg_sg and
> + * returned PCI_P2PDMA_MAP_BUS_ADDR.
> + */
> +void pci_p2pdma_map_bus_segment(struct scatterlist *pg_sg,
> + struct scatterlist *dma_sg)
> +{
> + struct pci_p2pdma_pagemap *pgmap = to_p2p_pgmap(sg_page(pg_sg)->pgmap);
> +
> + dma_sg->dma_address = sg_phys(pg_sg) + pgmap->bus_offset;
> + sg_dma_len(dma_sg) = pg_sg->length;
> + sg_mark_pci_p2pdma(dma_sg);
> +}
> +
> /**
> * pci_p2pdma_enable_store - parse a configfs/sysfs attribute store
> * to enable p2pdma
> diff --git a/include/linux/pci-p2pdma.h b/include/linux/pci-p2pdma.h
> index a06072ac3a52..49e7679403cf 100644
> --- a/include/linux/pci-p2pdma.h
> +++ b/include/linux/pci-p2pdma.h
> @@ -13,6 +13,12 @@
>
> #include <linux/pci.h>
>
> +struct pci_p2pdma_map_state {
> + struct dev_pagemap *pgmap;
> + int map;
> + u64 bus_off;
> +};
> +
> struct block_device;
> struct scatterlist;
>
> @@ -43,6 +49,11 @@ int pci_p2pdma_map_sg_attrs(struct device *dev, struct scatterlist *sg,
> int nents, enum dma_data_direction dir, unsigned long attrs);
> void pci_p2pdma_unmap_sg_attrs(struct device *dev, struct scatterlist *sg,
> int nents, enum dma_data_direction dir, unsigned long attrs);
> +int pci_p2pdma_map_segment(struct pci_p2pdma_map_state *state,
> + struct device *dev, struct scatterlist *sg,
> + unsigned long dma_attrs);
> +void pci_p2pdma_map_bus_segment(struct scatterlist *pg_sg,
> + struct scatterlist *dma_sg);
> int pci_p2pdma_enable_store(const char *page, struct pci_dev **p2p_dev,
> bool *use_p2pdma);
> ssize_t pci_p2pdma_enable_show(char *page, struct pci_dev *p2p_dev,
> @@ -109,6 +120,16 @@ static inline void pci_p2pdma_unmap_sg_attrs(struct device *dev,
> unsigned long attrs)
> {
> }
> +static inline int pci_p2pdma_map_segment(struct pci_p2pdma_map_state *state,
> + struct device *dev, struct scatterlist *sg,
> + unsigned long dma_attrs)
> +{
> + return 0;
> +}
> +static inline void pci_p2pdma_map_bus_segment(struct scatterlist *pg_sg,
> + struct scatterlist *dma_sg)
> +{
> +}
> static inline int pci_p2pdma_enable_store(const char *page,
> struct pci_dev **p2p_dev, bool *use_p2pdma)
> {
>
Powered by blists - more mailing lists