All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ard Biesheuvel <ardb@kernel.org>
To: "Russell King (Oracle)" <linux@armlinux.org.uk>
Cc: Will Deacon <will@kernel.org>,
	linux-arch <linux-arch@vger.kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Marc Zyngier <maz@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Robin Murphy <robin.murphy@arm.com>,
	Christoph Hellwig <hch@lst.de>,
	vgupta@kernel.org, Arnd Bergmann <arnd@arndb.de>,
	bcain@quicinc.com, Geert Uytterhoeven <geert@linux-m68k.org>,
	Michal Simek <monstr@monstr.eu>,
	Dinh Nguyen <dinguyen@kernel.org>,
	Stafford Horne <shorne@gmail.com>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Rich Felker <dalias@libc.org>
Subject: Re: Cache maintenance for non-coherent DMA in arch_sync_dma_for_device()
Date: Mon, 6 Jun 2022 18:15:13 +0200	[thread overview]
Message-ID: <CAMj1kXGyO0Dtws-qy=3=D-QhR9jV83n3g7CjJwy99YAMejh5Fg@mail.gmail.com> (raw)
In-Reply-To: <Yp4eqzHKyV64/Nxc@shell.armlinux.org.uk>

On Mon, 6 Jun 2022 at 17:36, Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Mon, Jun 06, 2022 at 04:21:50PM +0100, Will Deacon wrote:
> >   (1) What if the DMA transfer doesn't write to every byte in the buffer?
>
> The data that is in RAM gets pulled into the cache and is visible to
> the CPU - but if DMA doesn't write to every byte in the buffer, isn't
> that a DMA failure? Should a buffer that suffers DMA failure be passed
> to the user?
>
> >   (2) What if the buffer has a virtual alias in userspace (e.g. because
> >       the kernel has GUP'd the buffer?
>
> Then userspace needs to avoid writing to cachelines that overlap the
> buffer to avoid destroying the action of the DMA. It shouldn't be doing
> this anyway (what happens if userspace writes to the same location that
> is being DMA'd to... who wins?)
>
> However, you're right that invalidating in this case could expose data
> that userspace shouldn't see, and I'd suggest in this case that DMA
> buffers should be cleaned in this circumstance before they're exposed
> to userspace - so userspace only ever gets to see the data that was
> there at the point they're mapped, or is subsequently written to
> afterwards by DMA.
>
> I don't think there's anything to be worried about if the invalidation
> reveals stale data provided the stale data is not older than the data
> that was there on first mapping.
>

Given that cache invalidate without clean could potentially nullify
the effect of, e.g., a memzero_explicit() call, I think a clean is
definitely safer, but  OTOH, it is also more costly, and not strictly
necessary for correctness of the DMA operation itself.

So I agree with the suggested change, as long as it annotated
sufficiently clearly to make the above distinction.

> > Finally, on arm(64), the DMA mapping code tries to deal with buffers
> > that are not cacheline aligned by issuing clean-and-invalidate
> > operations for the overlapping portions at each end of the buffer. I
> > don't think this makes a tonne of sense, as inevitably one of the
> > writers (either the CPU or the DMA) is going to win and somebody is
> > going to run into silent data loss. Having the caller receive
> > DMA_MAPPING_ERROR in this case would probably be better.
>
> Sadly unavoidable - people really like passing unaligned buffers to the
> DMA API, sometimes those buffers contain information that needs to be
> preserved. I really wish it wasn't that way, because it would make life
> a lot better, but it's what we've had to deal with over the years with
> the likes of the SCSI subsystem (and e.g. it's sense buffer that was
> embedded non-cacheline aligned into other structures that had to be
> DMA'd to.)
>

As discussed in the thread related to Catalin's DMA granularity
series, this is something I think we should be addressing with bounce
buffering for inbound DMA. This would allow us to reduce the kmalloc
alignment as well.

  parent reply	other threads:[~2022-06-06 16:15 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-06 15:21 Cache maintenance for non-coherent DMA in arch_sync_dma_for_device() Will Deacon
2022-06-06 15:35 ` Russell King (Oracle)
2022-06-06 16:02   ` Robin Murphy
2022-06-06 16:16     ` Russell King (Oracle)
2022-06-08  8:49     ` Christoph Hellwig
2022-06-06 16:13   ` Catalin Marinas
2022-06-06 16:15   ` Ard Biesheuvel [this message]
2022-06-08 16:51     ` Catalin Marinas
2022-06-08  8:48 ` Christoph Hellwig
2022-06-08 12:02   ` Russell King (Oracle)
2022-06-08 15:14     ` Christoph Hellwig
2022-06-09 13:59   ` Will Deacon
2022-06-09 14:18     ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAMj1kXGyO0Dtws-qy=3=D-QhR9jV83n3g7CjJwy99YAMejh5Fg@mail.gmail.com' \
    --to=ardb@kernel.org \
    --cc=arnd@arndb.de \
    --cc=bcain@quicinc.com \
    --cc=catalin.marinas@arm.com \
    --cc=dalias@libc.org \
    --cc=dinguyen@kernel.org \
    --cc=geert@linux-m68k.org \
    --cc=hch@lst.de \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=mark.rutland@arm.com \
    --cc=maz@kernel.org \
    --cc=monstr@monstr.eu \
    --cc=mpe@ellerman.id.au \
    --cc=robin.murphy@arm.com \
    --cc=shorne@gmail.com \
    --cc=vgupta@kernel.org \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.