From: Halil Pasic <pasic@linux.ibm.com>
To: Johannes Berg <johannes@sipsolutions.net>
Cc: "Toke Høiland-Jørgensen" <toke@toke.dk>,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
"Robin Murphy" <robin.murphy@arm.com>,
"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>,
Netdev <netdev@vger.kernel.org>,
"Paolo Abeni" <pabeni@redhat.com>,
"Linus Torvalds" <torvalds@linux-foundation.org>,
"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:15:02 +0200 [thread overview]
Message-ID: <20220327051502.63fde20a.pasic@linux.ibm.com> (raw)
In-Reply-To: <e42e4c8bf35b62c671ec20ec6c21a43216e7daa6.camel@sipsolutions.net>
On Fri, 25 Mar 2022 22:13:08 +0100
Johannes Berg <johannes@sipsolutions.net> wrote:
> > > Then, however, we need to define what happens if you pass
> > > DMA_BIDIRECTIONAL to the sync_for_cpu() and sync_for_device() functions,
> > > which adds two more cases? Or maybe we eventually just think that's not
> > > valid at all, since you have to specify how you're (currently?) using
> > > the buffer, which can't be DMA_BIDIRECTIONAL?
> >
> > Ugh. Do we actually have cases that do it?
>
> Yes, a few.
>
> > That sounds really odd for
> > a "sync" operation. It sounds very reasonable for _allocating_ DMA,
> > but for syncing I'm left scratching my head what the semantics would
> > be.
>
> I agree.
>
> > But yes, if we do and people come up with semantics for it, those
> > semantics should be clearly documented.
>
> I'm not sure? I'm wondering if this isn't just because - like me
> initially - people misunderstood the direction argument, or didn't
> understand it well enough, and then just passed the same value as for
> the map()/unmap()?
I don't think you misunderstood the direction argument and its usage. I
didn't finish thinking about the proposal of Linus, I'm pretty confident
about my understanding of the current semantics of the direction.
According to the documentation, you do have to pass in the very same
direction, that was specified when the mapping was created. A qoute
from the documentation.
"""
void
dma_sync_single_for_cpu(struct device *dev, dma_addr_t dma_handle,
size_t size,
enum dma_data_direction direction)
void
dma_sync_single_for_device(struct device *dev, dma_addr_t dma_handle,
size_t size,
enum dma_data_direction direction)
void
dma_sync_sg_for_cpu(struct device *dev, struct scatterlist *sg,
int nents,
enum dma_data_direction direction)
void
dma_sync_sg_for_device(struct device *dev, struct scatterlist *sg,
int nents,
enum dma_data_direction direction)
Synchronise a single contiguous or scatter/gather mapping for the CPU
and device. With the sync_sg API, all the parameters must be the same
as those passed into the single mapping API. With the sync_single API,
you can use dma_handle and size parameters that aren't identical to
those passed into the single mapping API to do a partial sync.
"""
(Documentation/core-api/dma-api.rst)
The key here is "sync_sg API, all the parameters must be the same
as those passed into the single mapping API", but I have to admit,
I don't understand the *single* in here. The intended meaning of the
last sentence is that one can do partial sync by choose
dma_hande_sync, size_sync such that dma_handle_mapping <= dma_handle_sync
< dma_handle_mapping + size_mapping and dma_handle_sync + size_sync <=
dma_handle_mapping + size_mapping. But the direction has to remain the
same.
BTW, the current documented definition of the direction is about the
data transfer direction between memory and the device, and how the CPU
is interacting with the memory is not in scope. A quote form the
documentation.
"""
======================= =============================================
DMA_NONE no direction (used for debugging)
DMA_TO_DEVICE data is going from the memory to the device
DMA_FROM_DEVICE data is coming from the device to the memory
DMA_BIDIRECTIONAL direction isn't known
======================= =============================================
"""
(Documentation/core-api/dma-api.rst)
My feeling is, that re-defining the dma direction is not a good idea. But
I don't think my opinion has much weight here.
@Christoph, Robin: What do you think?
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: Johannes Berg <johannes@sipsolutions.net>
Cc: "Linus Torvalds" <torvalds@linux-foundation.org>,
"Maxime Bizon" <mbizon@freebox.fr>,
"Toke Høiland-Jørgensen" <toke@toke.dk>,
"Robin Murphy" <robin.murphy@arm.com>,
"Christoph Hellwig" <hch@lst.de>,
"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:15:02 +0200 [thread overview]
Message-ID: <20220327051502.63fde20a.pasic@linux.ibm.com> (raw)
In-Reply-To: <e42e4c8bf35b62c671ec20ec6c21a43216e7daa6.camel@sipsolutions.net>
On Fri, 25 Mar 2022 22:13:08 +0100
Johannes Berg <johannes@sipsolutions.net> wrote:
> > > Then, however, we need to define what happens if you pass
> > > DMA_BIDIRECTIONAL to the sync_for_cpu() and sync_for_device() functions,
> > > which adds two more cases? Or maybe we eventually just think that's not
> > > valid at all, since you have to specify how you're (currently?) using
> > > the buffer, which can't be DMA_BIDIRECTIONAL?
> >
> > Ugh. Do we actually have cases that do it?
>
> Yes, a few.
>
> > That sounds really odd for
> > a "sync" operation. It sounds very reasonable for _allocating_ DMA,
> > but for syncing I'm left scratching my head what the semantics would
> > be.
>
> I agree.
>
> > But yes, if we do and people come up with semantics for it, those
> > semantics should be clearly documented.
>
> I'm not sure? I'm wondering if this isn't just because - like me
> initially - people misunderstood the direction argument, or didn't
> understand it well enough, and then just passed the same value as for
> the map()/unmap()?
I don't think you misunderstood the direction argument and its usage. I
didn't finish thinking about the proposal of Linus, I'm pretty confident
about my understanding of the current semantics of the direction.
According to the documentation, you do have to pass in the very same
direction, that was specified when the mapping was created. A qoute
from the documentation.
"""
void
dma_sync_single_for_cpu(struct device *dev, dma_addr_t dma_handle,
size_t size,
enum dma_data_direction direction)
void
dma_sync_single_for_device(struct device *dev, dma_addr_t dma_handle,
size_t size,
enum dma_data_direction direction)
void
dma_sync_sg_for_cpu(struct device *dev, struct scatterlist *sg,
int nents,
enum dma_data_direction direction)
void
dma_sync_sg_for_device(struct device *dev, struct scatterlist *sg,
int nents,
enum dma_data_direction direction)
Synchronise a single contiguous or scatter/gather mapping for the CPU
and device. With the sync_sg API, all the parameters must be the same
as those passed into the single mapping API. With the sync_single API,
you can use dma_handle and size parameters that aren't identical to
those passed into the single mapping API to do a partial sync.
"""
(Documentation/core-api/dma-api.rst)
The key here is "sync_sg API, all the parameters must be the same
as those passed into the single mapping API", but I have to admit,
I don't understand the *single* in here. The intended meaning of the
last sentence is that one can do partial sync by choose
dma_hande_sync, size_sync such that dma_handle_mapping <= dma_handle_sync
< dma_handle_mapping + size_mapping and dma_handle_sync + size_sync <=
dma_handle_mapping + size_mapping. But the direction has to remain the
same.
BTW, the current documented definition of the direction is about the
data transfer direction between memory and the device, and how the CPU
is interacting with the memory is not in scope. A quote form the
documentation.
"""
======================= =============================================
DMA_NONE no direction (used for debugging)
DMA_TO_DEVICE data is going from the memory to the device
DMA_FROM_DEVICE data is coming from the device to the memory
DMA_BIDIRECTIONAL direction isn't known
======================= =============================================
"""
(Documentation/core-api/dma-api.rst)
My feeling is, that re-defining the dma direction is not a good idea. But
I don't think my opinion has much weight here.
@Christoph, Robin: What do you think?
Regards,
Halil
next prev parent reply other threads:[~2022-03-27 3:15 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 [this message]
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
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=20220327051502.63fde20a.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=johannes@sipsolutions.net \
--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.