All of lore.kernel.org
 help / color / mirror / Atom feed
From: Halil Pasic <pasic@linux.ibm.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: "Toke Høiland-Jørgensen" <toke@toke.dk>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	Netdev <netdev@vger.kernel.org>, "Kalle Valo" <kvalo@kernel.org>,
	linux-wireless <linux-wireless@vger.kernel.org>,
	"Oleksandr Natalenko" <oleksandr@natalenko.name>,
	stable <stable@vger.kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	"Halil Pasic" <pasic@linux.ibm.com>,
	iommu <iommu@lists.linux-foundation.org>,
	"Olha Cherevyk" <olha.cherevyk@gmail.com>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Maxime Bizon" <mbizon@freebox.fr>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Robin Murphy" <robin.murphy@arm.com>,
	"Christoph Hellwig" <hch@lst.de>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>
Subject: Re: [REGRESSION] Recent swiotlb DMA_FROM_DEVICE fixes break ath9k-based AP
Date: Sun, 27 Mar 2022 05:48:48 +0200	[thread overview]
Message-ID: <20220327054848.1a545b12.pasic@linux.ibm.com> (raw)
In-Reply-To: <CAHk-=whUQCCaQXJt3KUeQ8mtnLeVXEScNXCp+_DYh2SNY7EcEA@mail.gmail.com>

On Thu, 24 Mar 2022 12:26:53 -0700
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> So I don't think the dma_sync_single_for_device() is *wrong* per se,
> because the CPU didn't actually do any modifications.
> 
> But yes, I think it's unnecessary - because any later CPU accesses
> would need that dma_sync_single_for_cpu() anyway, which should
> invalidate any stale caches.
> 
> And it clearly doesn't work in a bounce-buffer situation, but honestly
> I don't think a "CPU modified buffers concurrently with DMA" can
> *ever* work in that situation, so I think it's wrong for a bounce
> buffer model to ever do anything in the dma_sync_single_for_device()
> situation.

I agree it CPU modified buffers *concurrently* with DMA can never work,
and I believe the ownership model was conceived to prevent this
situation. But a CPU can modify the buffer *after* DMA has written to
it, while the mapping is still alive. For example one could do one
partial read from the device, *after* the DMA is done,
sync_for_cpu(DMA_FROM_DEVICE), examine, then zero out the entire buffer,
sync_for_device(DMA_FROM_DEVICE), make the device do partial DMA, do
dma_unmap and expect no residue from the fist DMA. That is expect that
the zeroing out was effective.

The point I'm trying to make is: if concurrent is even permitted (it
isn't because of ownership) swiotlb woudn't know if we are dealing with
the *concurrent* case, which is completely bogous, or with the
*sequential* case, which at least in theory could work. And if we don't
do anyting on the sync_for_device(DMA_FROM_DEVICE) we render
that zeroing out the entire buffer form my example ineffective, because
it would not reach the bounce buffer, and on dma_unmap we would overwrite
the original buffer with the content of the bounce buffer.

Regards,
Halil
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

WARNING: multiple messages have this Message-ID (diff)
From: Halil Pasic <pasic@linux.ibm.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: "Toke Høiland-Jørgensen" <toke@toke.dk>,
	"Robin Murphy" <robin.murphy@arm.com>,
	"Christoph Hellwig" <hch@lst.de>,
	"Maxime Bizon" <mbizon@freebox.fr>,
	"Oleksandr Natalenko" <oleksandr@natalenko.name>,
	"Marek Szyprowski" <m.szyprowski@samsung.com>,
	"Kalle Valo" <kvalo@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Olha Cherevyk" <olha.cherevyk@gmail.com>,
	iommu <iommu@lists.linux-foundation.org>,
	linux-wireless <linux-wireless@vger.kernel.org>,
	Netdev <netdev@vger.kernel.org>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	stable <stable@vger.kernel.org>,
	"Halil Pasic" <pasic@linux.ibm.com>
Subject: Re: [REGRESSION] Recent swiotlb DMA_FROM_DEVICE fixes break ath9k-based AP
Date: Sun, 27 Mar 2022 05:48:48 +0200	[thread overview]
Message-ID: <20220327054848.1a545b12.pasic@linux.ibm.com> (raw)
In-Reply-To: <CAHk-=whUQCCaQXJt3KUeQ8mtnLeVXEScNXCp+_DYh2SNY7EcEA@mail.gmail.com>

On Thu, 24 Mar 2022 12:26:53 -0700
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> So I don't think the dma_sync_single_for_device() is *wrong* per se,
> because the CPU didn't actually do any modifications.
> 
> But yes, I think it's unnecessary - because any later CPU accesses
> would need that dma_sync_single_for_cpu() anyway, which should
> invalidate any stale caches.
> 
> And it clearly doesn't work in a bounce-buffer situation, but honestly
> I don't think a "CPU modified buffers concurrently with DMA" can
> *ever* work in that situation, so I think it's wrong for a bounce
> buffer model to ever do anything in the dma_sync_single_for_device()
> situation.

I agree it CPU modified buffers *concurrently* with DMA can never work,
and I believe the ownership model was conceived to prevent this
situation. But a CPU can modify the buffer *after* DMA has written to
it, while the mapping is still alive. For example one could do one
partial read from the device, *after* the DMA is done,
sync_for_cpu(DMA_FROM_DEVICE), examine, then zero out the entire buffer,
sync_for_device(DMA_FROM_DEVICE), make the device do partial DMA, do
dma_unmap and expect no residue from the fist DMA. That is expect that
the zeroing out was effective.

The point I'm trying to make is: if concurrent is even permitted (it
isn't because of ownership) swiotlb woudn't know if we are dealing with
the *concurrent* case, which is completely bogous, or with the
*sequential* case, which at least in theory could work. And if we don't
do anyting on the sync_for_device(DMA_FROM_DEVICE) we render
that zeroing out the entire buffer form my example ineffective, because
it would not reach the bounce buffer, and on dma_unmap we would overwrite
the original buffer with the content of the bounce buffer.

Regards,
Halil

  parent reply	other threads:[~2022-03-27  3:49 UTC|newest]

Thread overview: 139+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-23  7:19 [REGRESSION] Recent swiotlb DMA_FROM_DEVICE fixes break ath9k-based AP Oleksandr Natalenko
2022-03-23  7:19 ` Oleksandr Natalenko via iommu
2022-03-23  7:28 ` Kalle Valo
2022-03-23  7:28   ` Kalle Valo
2022-03-23 17:27 ` Linus Torvalds
2022-03-23 17:27   ` Linus Torvalds
2022-03-23 19:06   ` Robin Murphy
2022-03-23 19:06     ` Robin Murphy
2022-03-23 19:16     ` Linus Torvalds
2022-03-23 19:16       ` Linus Torvalds
2022-03-23 20:54       ` Robin Murphy
2022-03-23 20:54         ` Robin Murphy
2022-03-24  5:57         ` Christoph Hellwig
2022-03-24  5:57           ` Christoph Hellwig
2022-03-24 10:25           ` Oleksandr Natalenko
2022-03-24 10:25             ` Oleksandr Natalenko via iommu
2022-03-24 11:05             ` Robin Murphy
2022-03-24 11:05               ` Robin Murphy
2022-03-24 14:27               ` Toke Høiland-Jørgensen
2022-03-24 14:27                 ` Toke Høiland-Jørgensen via iommu
2022-03-24 16:29                 ` Maxime Bizon
2022-03-24 16:29                   ` Maxime Bizon
2022-03-24 16:31                   ` Christoph Hellwig
2022-03-24 16:31                     ` Christoph Hellwig
2022-03-24 16:52                     ` Robin Murphy
2022-03-24 16:52                       ` Robin Murphy
2022-03-24 17:07                       ` Toke Høiland-Jørgensen
2022-03-24 17:07                         ` Toke Høiland-Jørgensen via iommu
2022-03-24 19:26                         ` Linus Torvalds
2022-03-24 19:26                           ` Linus Torvalds
2022-03-24 21:14                           ` Toke Høiland-Jørgensen
2022-03-24 21:14                             ` Toke Høiland-Jørgensen via iommu
2022-03-25 10:25                           ` Maxime Bizon
2022-03-25 10:25                             ` Maxime Bizon
2022-03-25 11:27                             ` Robin Murphy
2022-03-25 11:27                               ` Robin Murphy
2022-03-25 23:38                               ` Halil Pasic
2022-03-25 23:38                                 ` Halil Pasic
2022-03-26 16:05                                 ` Toke Høiland-Jørgensen
2022-03-26 16:05                                   ` Toke Høiland-Jørgensen via iommu
2022-03-26 18:38                                   ` Linus Torvalds
2022-03-26 18:38                                     ` Linus Torvalds
2022-03-26 22:38                                     ` David Laight
2022-03-26 22:38                                       ` David Laight
2022-03-26 22:41                                       ` Linus Torvalds
2022-03-26 22:41                                         ` Linus Torvalds
2022-03-25 16:25                             ` Toke Høiland-Jørgensen
2022-03-25 16:25                               ` Toke Høiland-Jørgensen via iommu
2022-03-25 16:45                               ` Robin Murphy
2022-03-25 16:45                                 ` Robin Murphy
2022-03-25 18:13                                 ` Toke Høiland-Jørgensen via iommu
2022-03-25 18:13                                   ` Toke Høiland-Jørgensen
2022-03-25 18:30                             ` Linus Torvalds
2022-03-25 18:30                               ` Linus Torvalds
2022-03-25 19:14                               ` Robin Murphy
2022-03-25 19:14                                 ` Robin Murphy
2022-03-25 19:21                                 ` Linus Torvalds
2022-03-25 19:21                                   ` Linus Torvalds
2022-03-25 19:26                               ` Oleksandr Natalenko via iommu
2022-03-25 19:26                                 ` Oleksandr Natalenko
2022-03-25 19:27                                 ` Linus Torvalds
2022-03-25 19:27                                   ` Linus Torvalds
2022-03-25 19:35                                   ` Oleksandr Natalenko via iommu
2022-03-25 19:35                                     ` Oleksandr Natalenko
2022-03-25 20:37                               ` Johannes Berg
2022-03-25 20:37                                 ` Johannes Berg
2022-03-25 20:47                                 ` Linus Torvalds
2022-03-25 20:47                                   ` Linus Torvalds
2022-03-25 21:13                                   ` Johannes Berg
2022-03-25 21:13                                     ` Johannes Berg
2022-03-25 21:40                                     ` David Laight
2022-03-25 21:40                                       ` David Laight
2022-03-25 21:56                                     ` Linus Torvalds
2022-03-25 21:56                                       ` Linus Torvalds
2022-03-25 22:41                                       ` David Laight
2022-03-25 22:41                                         ` David Laight
2022-03-27  3:15                                     ` Halil Pasic
2022-03-27  3:15                                       ` Halil Pasic
2022-03-28  9:48                                       ` Johannes Berg
2022-03-28  9:48                                         ` Johannes Berg
2022-03-28  9:50                                         ` Johannes Berg
2022-03-28  9:50                                           ` Johannes Berg
2022-03-28  9:57                                           ` Johannes Berg
2022-03-28  9:57                                             ` Johannes Berg
2022-03-27  3:48                           ` Halil Pasic [this message]
2022-03-27  3:48                             ` Halil Pasic
2022-03-27  5:06                             ` Linus Torvalds
2022-03-27  5:06                               ` Linus Torvalds
2022-03-27  5:21                               ` Linus Torvalds
2022-03-27  5:21                                 ` Linus Torvalds
2022-03-27 15:24                                 ` David Laight
2022-03-27 15:24                                   ` David Laight
2022-03-27 19:23                                   ` Linus Torvalds
2022-03-27 19:23                                     ` Linus Torvalds
2022-03-27 20:04                                     ` Linus Torvalds
2022-03-27 20:04                                       ` Linus Torvalds
2022-03-27 23:52                                 ` Halil Pasic
2022-03-27 23:52                                   ` Halil Pasic
2022-03-28  0:30                                   ` Linus Torvalds
2022-03-28  0:30                                     ` Linus Torvalds
2022-03-28 12:02                                     ` Halil Pasic
2022-03-28 12:02                                       ` Halil Pasic
2022-03-27 23:37                               ` Halil Pasic
2022-03-27 23:37                                 ` Halil Pasic
2022-03-28  0:37                                 ` Linus Torvalds
2022-03-28  0:37                                   ` Linus Torvalds
2022-03-25  7:12                         ` Oleksandr Natalenko
2022-03-25  7:12                           ` Oleksandr Natalenko via iommu
2022-03-25  9:21                           ` Thorsten Leemhuis
2022-03-25  9:21                             ` Thorsten Leemhuis
2022-03-24 18:31                       ` Halil Pasic
2022-03-24 18:31                         ` Halil Pasic
2022-03-25 16:31                         ` Christoph Hellwig
2022-03-25 16:31                           ` Christoph Hellwig
2022-03-24 18:02         ` Halil Pasic
2022-03-24 18:02           ` Halil Pasic
2022-03-25 15:25           ` Halil Pasic
2022-03-25 15:25             ` Halil Pasic
2022-03-25 16:23             ` Robin Murphy
2022-03-25 16:23               ` Robin Murphy
2022-03-25 16:32           ` Christoph Hellwig
2022-03-25 16:32             ` Christoph Hellwig
2022-03-25 18:15             ` Toke Høiland-Jørgensen via iommu
2022-03-25 18:15               ` Toke Høiland-Jørgensen
2022-03-25 18:42               ` Robin Murphy
2022-03-25 18:42                 ` Robin Murphy
2022-03-25 18:46                 ` Linus Torvalds
2022-03-25 18:46                   ` Linus Torvalds
2022-03-28  6:37                   ` Christoph Hellwig
2022-03-28  6:37                     ` Christoph Hellwig
2022-03-28  8:15                     ` David Laight
2022-03-28  8:15                       ` David Laight
2022-03-30 12:11                     ` Halil Pasic
2022-03-30 12:11                       ` Halil Pasic
2022-03-24  8:55   ` Oleksandr Natalenko
2022-03-24  8:55     ` Oleksandr Natalenko via iommu
2022-03-24 12:32 ` [REGRESSION] Recent swiotlb DMA_FROM_DEVICE fixes break ath9k-based AP #forregzbot Thorsten Leemhuis
2022-03-25  9:24   ` Thorsten Leemhuis
2022-03-27  9:00     ` Thorsten Leemhuis

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=20220327054848.1a545b12.pasic@linux.ibm.com \
    --to=pasic@linux.ibm.com \
    --cc=davem@davemloft.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=hch@lst.de \
    --cc=iommu@lists.linux-foundation.org \
    --cc=kuba@kernel.org \
    --cc=kvalo@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=mbizon@freebox.fr \
    --cc=netdev@vger.kernel.org \
    --cc=oleksandr@natalenko.name \
    --cc=olha.cherevyk@gmail.com \
    --cc=pabeni@redhat.com \
    --cc=robin.murphy@arm.com \
    --cc=stable@vger.kernel.org \
    --cc=toke@toke.dk \
    --cc=torvalds@linux-foundation.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.