Re: [RFC PATCH v2 07/11] dma-mapping: Add flags to dma_map_ops to indicate PCI P2PDMA support
On Thu, Mar 11, 2021 at 04:31:37PM -0700, Logan Gunthorpe wrote:
> +int dma_pci_p2pdma_supported(struct device *dev)
^^^
bool?
> +{
> + const struct dma_map_ops *ops = get_dma_ops(dev);
> +
> + return !ops || ops->flags & DMA_F_PCI_P2PDMA_SUPPORTED;
Is this logic correct? I would have expected.
return (ops && ops->flags & DMA_F_PCI_P2PDMA_SUPPORTED);
Ira
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH v2 04/11] PCI/P2PDMA: Introduce pci_p2pdma_should_map_bus() and pci_p2pdma_bus_offset()
On Thu, Mar 11, 2021 at 04:31:34PM -0700, Logan Gunthorpe wrote:
> Introduce pci_p2pdma_should_map_bus() which is meant to be called by
^
pci_p2pdma_dma_map_type() ???
FWIW I find this name confusing with pci_p2pdma_map_type() and looking at the
implementation I'm not clear why pci_p2pdma_dma_map_type() needs to exist?
Ira
[snip]
> +
> +/**
> + * pci_p2pdma_dma_map_type - determine if a DMA mapping should use the
> + * bus address, be mapped normally or fail
> + * @dev: device doing the DMA request
> + * @pgmap: dev_pagemap structure for the mapping
> + *
> + * Returns:
> + *1 - if the page should be mapped with a bus address,
> + *0 - if the page should be mapped normally through an IOMMU mapping or
> + *physical address; or
> + * -1 - if the device should not map the pages and an error should be
> + *returned
> + */
> +int pci_p2pdma_dma_map_type(struct device *dev, struct dev_pagemap *pgmap)
> +{
> + struct pci_p2pdma_pagemap *p2p_pgmap = to_p2p_pgmap(pgmap);
> + struct pci_dev *client;
> +
> + if (!dev_is_pci(dev))
> + return -1;
> +
> + client = to_pci_dev(dev);
> +
> + switch (pci_p2pdma_map_type(p2p_pgmap->provider, client)) {
> + case PCI_P2PDMA_MAP_THRU_HOST_BRIDGE:
> + return 0;
> + case PCI_P2PDMA_MAP_BUS_ADDR:
> + return 1;
> + default:
> + return -1;
> + }
> +}
> +EXPORT_SYMBOL_GPL(pci_p2pdma_dma_map_type);
I guess the main point here is to export this to the DMA layer?
Ira
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH v2 04/11] PCI/P2PDMA: Introduce pci_p2pdma_should_map_bus() and pci_p2pdma_bus_offset()
On Thu, Mar 11, 2021 at 04:31:34PM -0700, Logan Gunthorpe wrote:
> Introduce pci_p2pdma_should_map_bus() which is meant to be called by
> DMA map functions to determine how to map a given p2pdma page.
>
> pci_p2pdma_bus_offset() is also added to allow callers to get the bus
> offset if they need to map the bus address.
>
> Signed-off-by: Logan Gunthorpe
> ---
> drivers/pci/p2pdma.c | 50 ++
> include/linux/pci-p2pdma.h | 11 +
> 2 files changed, 61 insertions(+)
>
> diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
> index 7f6836732bce..66d16b7eb668 100644
> --- a/drivers/pci/p2pdma.c
> +++ b/drivers/pci/p2pdma.c
> @@ -912,6 +912,56 @@ void pci_p2pdma_unmap_sg_attrs(struct device *dev,
> struct scatterlist *sg,
> }
> EXPORT_SYMBOL_GPL(pci_p2pdma_unmap_sg_attrs);
>
> +/**
> + * pci_p2pdma_bus_offset - returns the bus offset for a given page
> + * @page: page to get the offset for
> + *
> + * Must be passed a PCI p2pdma page.
> + */
> +u64 pci_p2pdma_bus_offset(struct page *page)
> +{
> + struct pci_p2pdma_pagemap *p2p_pgmap = to_p2p_pgmap(page->pgmap);
> +
> + WARN_ON(!is_pci_p2pdma_page(page));
Shouldn't this check be before the to_p2p_pgmap() call? And I've been told not
to introduce WARN_ON's. Should this be?
if (!is_pci_p2pdma_page(page))
return -1;
???
Ira
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 4/5] dma-direct: provide a arch_dma_clear_uncached hook
On Mon, Feb 24, 2020 at 11:44:44AM -0800, Christoph Hellwig wrote: > This allows the arch code to reset the page tables to cached access when > freeing a dma coherent allocation that was set to uncached using > arch_dma_set_uncached. > > Signed-off-by: Christoph Hellwig > --- > arch/Kconfig| 7 +++ > include/linux/dma-noncoherent.h | 1 + > kernel/dma/direct.c | 2 ++ > 3 files changed, 10 insertions(+) > > diff --git a/arch/Kconfig b/arch/Kconfig > index 090cfe0c82a7..c26302f90c96 100644 > --- a/arch/Kconfig > +++ b/arch/Kconfig > @@ -255,6 +255,13 @@ config ARCH_HAS_SET_DIRECT_MAP > config ARCH_HAS_DMA_SET_UNCACHED > bool > > +# > +# Select if the architectures provides the arch_dma_clear_uncached symbol > +# to undo an in-place page table remap for uncached access. > +# > +config ARCH_HAS_DMA_CLEAR_UNCACHED > + bool > + > # Select if arch init_task must go in the __init_task_data section > config ARCH_TASK_STRUCT_ON_STACK > bool > diff --git a/include/linux/dma-noncoherent.h b/include/linux/dma-noncoherent.h > index 1a4039506673..b59f1b6be3e9 100644 > --- a/include/linux/dma-noncoherent.h > +++ b/include/linux/dma-noncoherent.h > @@ -109,5 +109,6 @@ static inline void arch_dma_prep_coherent(struct page > *page, size_t size) > #endif /* CONFIG_ARCH_HAS_DMA_PREP_COHERENT */ > > void *arch_dma_set_uncached(void *addr, size_t size); > +void arch_dma_clear_uncached(void *addr, size_t size); > > #endif /* _LINUX_DMA_NONCOHERENT_H */ > diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c > index f01a8191fd59..a8560052a915 100644 > --- a/kernel/dma/direct.c > +++ b/kernel/dma/direct.c > @@ -219,6 +219,8 @@ void dma_direct_free_pages(struct device *dev, size_t > size, void *cpu_addr, > > if (IS_ENABLED(CONFIG_DMA_REMAP) && is_vmalloc_addr(cpu_addr)) > vunmap(cpu_addr); > + else if (IS_ENABLED(CONFIG_ARCH_HAS_DMA_CLEAR_UNCACHED)) > + arch_dma_clear_uncached(cpu_addr, size); Isn't using arch_dma_clear_uncached() before patch 5 going to break bisectability? Ira > > dma_free_contiguous(dev, dma_direct_to_page(dev, dma_addr), size); > } > -- > 2.24.1 > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 2/2] dma-contiguous: Use fallback alloc_pages for single pages
On Thu, May 23, 2019 at 09:06:33PM -0700, Nicolin Chen wrote:
> The addresses within a single page are always contiguous, so it's
> not so necessary to always allocate one single page from CMA area.
> Since the CMA area has a limited predefined size of space, it may
> run out of space in heavy use cases, where there might be quite a
> lot CMA pages being allocated for single pages.
>
> However, there is also a concern that a device might care where a
> page comes from -- it might expect the page from CMA area and act
> differently if the page doesn't.
How does a device know, after this call, if a CMA area was used? From the
patches I figured a device should not care.
>
> This patch tries to use the fallback alloc_pages path, instead of
> one-page size allocations from the global CMA area in case that a
> device does not have its own CMA area. This'd save resources from
> the CMA global area for more CMA allocations, and also reduce CMA
> fragmentations resulted from trivial allocations.
>
> Signed-off-by: Nicolin Chen
> ---
> kernel/dma/contiguous.c | 11 ++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
> index 21f39a6cb04f..6914b92d5c88 100644
> --- a/kernel/dma/contiguous.c
> +++ b/kernel/dma/contiguous.c
> @@ -223,14 +223,23 @@ bool dma_release_from_contiguous(struct device *dev,
> struct page *pages,
> * This function allocates contiguous memory buffer for specified device. It
> * first tries to use device specific contiguous memory area if available or
> * the default global one, then tries a fallback allocation of normal pages.
> + *
> + * Note that it byapss one-page size of allocations from the global area as
> + * the addresses within one page are always contiguous, so there is no need
> + * to waste CMA pages for that kind; it also helps reduce fragmentations.
> */
> struct page *dma_alloc_contiguous(struct device *dev, size_t size, gfp_t gfp)
> {
> int node = dev ? dev_to_node(dev) : NUMA_NO_NODE;
> size_t count = PAGE_ALIGN(size) >> PAGE_SHIFT;
> size_t align = get_order(PAGE_ALIGN(size));
> - struct cma *cma = dev_get_cma_area(dev);
> struct page *page = NULL;
> + struct cma *cma = NULL;
> +
> + if (dev && dev->cma_area)
> + cma = dev->cma_area;
> + else if (count > 1)
> + cma = dma_contiguous_default_area;
Doesn't dev_get_dma_area() already do this?
Ira
>
> /* CMA can be used only in the context which permits sleeping */
> if (cma && gfpflags_allow_blocking(gfp)) {
> --
> 2.17.1
>
