| 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: <CAAFQd5Bbp-eAVKS1MKS8xtrT4ZoOmBPfZyw9mys=eOmDb6r8Lw@mail.gmail.com>
Date: Wed, 19 Aug 2020 17:03:52 +0200
From: Tomasz Figa <tfiga@...omium.org>
To: Christoph Hellwig <hch@....de>
Cc: Mauro Carvalho Chehab <mchehab@...nel.org>,
Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
"James E.J. Bottomley" <James.Bottomley@...senpartnership.com>,
Joonyoung Shim <jy0922.shim@...sung.com>,
Seung-Woo Kim <sw0312.kim@...sung.com>,
Kyungmin Park <kyungmin.park@...sung.com>,
Ben Skeggs <bskeggs@...hat.com>,
Pawel Osciak <pawel@...iak.com>,
Marek Szyprowski <m.szyprowski@...sung.com>,
Matt Porter <mporter@...nel.crashing.org>,
"list@....net:IOMMU DRIVERS <iommu@...ts.linux-foundation.org>, Joerg
Roedel <joro@...tes.org>," <iommu@...ts.linux-foundation.org>,
Tom Lendacky <thomas.lendacky@....com>,
alsa-devel@...a-project.org,
linux-samsung-soc <linux-samsung-soc@...r.kernel.org>,
linux-ia64@...r.kernel.org, linux-scsi@...r.kernel.org,
linux-parisc@...r.kernel.org,
Linux Doc Mailing List <linux-doc@...r.kernel.org>,
nouveau@...ts.freedesktop.org,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
linux-nvme@...ts.infradead.org, linux-mips@...r.kernel.org,
linux-mm@...ck.org, netdev@...r.kernel.org,
"list@....net:IOMMU DRIVERS <iommu@...ts.linux-foundation.org>, Joerg
Roedel <joro@...tes.org>," <linux-arm-kernel@...ts.infradead.org>,
Linux Media Mailing List <linux-media@...r.kernel.org>
Subject: Re: [PATCH 19/28] dma-mapping: replace DMA_ATTR_NON_CONSISTENT with
dma_{alloc, free}_pages
Hi Christoph,
On Wed, Aug 19, 2020 at 8:57 AM Christoph Hellwig <hch@....de> wrote:
>
> Add a new API to allocate and free pages that are guaranteed to be
> addressable by a device, but otherwise behave like pages allocated by
> alloc_pages. The intended APIs to sync them for use with the device
> and cpu are dma_sync_single_for_{device,cpu} that are also used for
> streaming mappings.
>
> Switch all drivers over to this new API, but keep the usage of the
> crufty dma_cache_sync API for now, which will be cleaned up on a driver
> by driver basis.
>
> Signed-off-by: Christoph Hellwig <hch@....de>
> ---
> Documentation/core-api/dma-api.rst | 68 +++++++++++------------
> Documentation/core-api/dma-attributes.rst | 8 ---
> arch/alpha/kernel/pci_iommu.c | 2 +
> arch/arm/mm/dma-mapping-nommu.c | 2 +
> arch/arm/mm/dma-mapping.c | 4 ++
> arch/ia64/hp/common/sba_iommu.c | 2 +
> arch/mips/jazz/jazzdma.c | 7 +--
> arch/powerpc/kernel/dma-iommu.c | 2 +
> arch/powerpc/platforms/ps3/system-bus.c | 4 ++
> arch/powerpc/platforms/pseries/vio.c | 2 +
> arch/s390/pci/pci_dma.c | 2 +
> arch/x86/kernel/amd_gart_64.c | 2 +
> drivers/iommu/dma-iommu.c | 2 +
> drivers/iommu/intel/iommu.c | 4 ++
> drivers/net/ethernet/i825xx/lasi_82596.c | 13 ++---
> drivers/net/ethernet/seeq/sgiseeq.c | 12 ++--
> drivers/parisc/ccio-dma.c | 2 +
> drivers/parisc/sba_iommu.c | 2 +
> drivers/scsi/53c700.c | 8 +--
> drivers/scsi/sgiwd93.c | 12 ++--
> drivers/xen/swiotlb-xen.c | 2 +
> include/linux/dma-direct.h | 5 ++
> include/linux/dma-mapping.h | 29 ++++++++--
> include/linux/dma-noncoherent.h | 3 -
> kernel/dma/direct.c | 51 ++++++++++++++++-
> kernel/dma/mapping.c | 43 +++++++++++++-
> kernel/dma/ops_helpers.c | 35 ++++++++++++
> kernel/dma/virt.c | 2 +
> sound/mips/hal2.c | 20 +++----
> 29 files changed, 254 insertions(+), 96 deletions(-)
>
Thanks for the patch. The general design looks quite nice, but please
see my comments inline.
> diff --git a/Documentation/core-api/dma-api.rst b/Documentation/core-api/dma-api.rst
> index 90239348b30f6f..047fcfffa0e5cf 100644
> --- a/Documentation/core-api/dma-api.rst
> +++ b/Documentation/core-api/dma-api.rst
> @@ -516,48 +516,53 @@ routines, e.g.:::
> }
>
>
> -Part II - Advanced dma usage
> -----------------------------
> +Part II - Non-coherent DMA allocations
> +--------------------------------------
>
> -Warning: These pieces of the DMA API should not be used in the
> -majority of cases, since they cater for unlikely corner cases that
> -don't belong in usual drivers.
> +These APIs allow to allocate pages that can be used like normal pages
> +in the kernel direct mapping, but are guaranteed to be DMA addressable.
Could we elaborate a bit more on what "like normal pages in kernel
direct mapping" mean from the driver perspective?
>
> If you don't understand how cache line coherency works between a
> processor and an I/O device, you should not be using this part of the
> -API at all.
> +API.
>
> ::
>
> void *
> - dma_alloc_attrs(struct device *dev, size_t size, dma_addr_t *dma_handle,
> - gfp_t flag, unsigned long attrs)
> + dma_alloc_pages(struct device *dev, size_t size, dma_addr_t *dma_handle,
> + enum dma_data_direction dir, gfp_t gfp)
> +
> +This routine allocates a region of <size> bytes of consistent memory. It
> +returns a pointer to the allocated region (in the processor's virtual address
> +space) or NULL if the allocation failed. The returned memory is guanteed to
> +behave like memory allocated using alloc_pages.
There is one aspect that the existing dma_alloc_attrs() handles, but
this new function doesn't: IOMMU support. The function will always
allocate a physically-contiguous block memory, which is a costly
operation and not even guaranteed to succeed, even if enough free
memory is available.
Modern SoCs employ IOMMUs to avoid the need to allocate
physically-contiguous memory and those happen to be also the devices
that could benefit from non-coherent allocations a lot. One of the
tasks of the DMA API was making it possible to allocate suitable
memory for a given device, without having the driver know about the
SoC integration details, such as the presence of an IOMMU.
Today, dma_alloc_attrs() uses the .alloc callback of the dma_ops
struct and the IOMMU-aware implementations, like the dma-iommu helpers
[1], would allocate discontiguous pages. Therefore, while I see the
DMA-aware page allocation functionality as a useful functionality on
its own for scatter-gather-capable hardware, I believe it is not a
complete replacement for dma_alloc_attrs() with the
DMA_ATTR_NON_CONSISTENT attribute.
[1] https://elixir.bootlin.com/linux/v5.9-rc1/source/drivers/iommu/dma-iommu.c#L510
Best regards,
Tomasz
Powered by blists - more mailing lists