On Mon, 28 Mar 2022 08:37:23 +0200
Christoph Hellwig wrote:
> On Fri, Mar 25, 2022 at 11:46:09AM -0700, Linus Torvalds wrote:
> > I think my list of three different sync cases (not just two! It's not
> > just about whether to sync for the CPU or the device, it's also about
> > what direction the
On Sun, 27 Mar 2022 17:30:01 -0700
Linus Torvalds wrote:
> On Sun, Mar 27, 2022 at 4:52 PM Halil Pasic wrote:
> >
> > I have no intention of pursuing this. When fixing the information leak,
> > I happened to realize, that a somewhat similar situation can emerge when
> > mappings are reused. It
On Mon, 2022-03-28 at 11:50 +0200, Johannes Berg wrote:
> No I worded that badly - the direction isn't useless, but thinking of it
> in terms of a buffer property rather than data movement is inaccurate.
> So then if we need something else to indicate how data was expected to
> be moved, the direct
On Mon, 2022-03-28 at 11:48 +0200, Johannes Berg wrote:
>
> However, this basically means that the direction argument to the flush
> APIs are completely useless, and we do have to define something
> new/else...
>
No I worded that badly - the direction isn't useless, but thinking of it
in terms o
On Sun, 2022-03-27 at 05:15 +0200, Halil Pasic wrote:
>
> 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.
>
Hah. So I wasn't imagining things after all.
However, as
From: Christoph Hellwig
> Sent: 28 March 2022 07:37
>
> On Fri, Mar 25, 2022 at 11:46:09AM -0700, Linus Torvalds wrote:
> > I think my list of three different sync cases (not just two! It's not
> > just about whether to sync for the CPU or the device, it's also about
> > what direction the data it
On Fri, Mar 25, 2022 at 11:46:09AM -0700, Linus Torvalds wrote:
> I think my list of three different sync cases (not just two! It's not
> just about whether to sync for the CPU or the device, it's also about
> what direction the data itself is taking) is correct.
>
> But maybe I'm wrong.
At the h
On Sun, Mar 27, 2022 at 4:37 PM Halil Pasic wrote:
>
>
> For the record, I believe that the partial revert proposed here
> https://www.spinics.net/lists/linux-wireless/msg222300.html
> would have been a wiser choice, than a complete revert of commit
> aa6f8dcbab47 ("swiotlb: rework "fix info leak
On Sun, Mar 27, 2022 at 4:52 PM Halil Pasic wrote:
>
> I have no intention of pursuing this. When fixing the information leak,
> I happened to realize, that a somewhat similar situation can emerge when
> mappings are reused. It seemed like an easy fix, so I asked the swiotlb
> maintainers, and th
On Sat, 26 Mar 2022 22:21:03 -0700
Linus Torvalds wrote:
> On Sat, Mar 26, 2022 at 10:06 PM Linus Torvalds
> wrote:
> >
> > On Sat, Mar 26, 2022 at 8:49 PM Halil Pasic wrote:
> > >
> > > I agree it CPU modified buffers *concurrently* with DMA can never work,
> > > and I believe the ownership
On Sat, 26 Mar 2022 22:06:15 -0700
Linus Torvalds wrote:
> On Sat, Mar 26, 2022 at 8:49 PM Halil Pasic wrote:
> >
> > 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 that just me
On Sun, Mar 27, 2022 at 12:23 PM Linus Torvalds
wrote:
>
> So I will propose that we really make it very much about practical
> concerns, and we document things as
>
> (a) the "sync" operation has by definition a "whose _future_ access
> do we sync for" notion.
>
> So "dma_sync_single_for_cp
On Sun, Mar 27, 2022 at 8:24 AM David Laight wrote:
>
> Aren't bounce buffers just a more extreme case on non-coherent
> memory accesses?
No.
In fact, this whoe change came about exactly because bounce buffers
are different.
The difference is that bounce buffers have that (wait for it) bounce
b
From: Linus Torvalds
> Sent: 27 March 2022 06:21
>
> On Sat, Mar 26, 2022 at 10:06 PM Linus Torvalds
> wrote:
> >
> > On Sat, Mar 26, 2022 at 8:49 PM Halil Pasic wrote:
> > >
> > > I agree it CPU modified buffers *concurrently* with DMA can never work,
> > > and I believe the ownership model was
On Sat, Mar 26, 2022 at 10:06 PM Linus Torvalds
wrote:
>
> On Sat, Mar 26, 2022 at 8:49 PM Halil Pasic wrote:
> >
> > 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 that just means
On Sat, Mar 26, 2022 at 8:49 PM Halil Pasic wrote:
>
> 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 that just means that the "ownership" model is garbage, and cannot
handle this REAL LIFE
On Thu, 24 Mar 2022 12:26:53 -0700
Linus Torvalds 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
On Fri, 25 Mar 2022 22:13:08 +0100
Johannes Berg 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,
On Sat, Mar 26, 2022 at 3:38 PM David Laight wrote:
>
> Is the idea of 'buffer ownership' even a good one?
I do think it might be best to not think in those terms, but literally
just in data movement terms.
Because the "buffer ownership" mental model is clearly confused, when
data transfer might
From: Linus Torvalds
> Sent: 26 March 2022 18:39
>
> On Sat, Mar 26, 2022 at 9:06 AM Toke Høiland-Jørgensen wrote:
> >
> > I was also toying with the idea of having a copy-based peek helper like:
> >
> > u32 data = dma_peek_word(buf, offset)
>
> I really don't think you can or want to have a wor
On Sat, Mar 26, 2022 at 9:06 AM Toke Høiland-Jørgensen wrote:
>
> I was also toying with the idea of having a copy-based peek helper like:
>
> u32 data = dma_peek_word(buf, offset)
I really don't think you can or want to have a word-based one.
That said, I like the *name* of that thing.
I think
Halil Pasic writes:
> On Fri, 25 Mar 2022 11:27:41 +
> Robin Murphy wrote:
>
>> What muddies the waters a bit is that the opposite combination
>> sync_for_cpu(DMA_TO_DEVICE) really *should* always be a no-op, and I for
>> one have already made the case for eliding that in code elsewhere, b
On Fri, 25 Mar 2022 11:27:41 +
Robin Murphy wrote:
> What muddies the waters a bit is that the opposite combination
> sync_for_cpu(DMA_TO_DEVICE) really *should* always be a no-op, and I for
> one have already made the case for eliding that in code elsewhere, but
> it doesn't necessarily h
From: Linus Torvalds
> Sent: 25 March 2022 21:57
>
> On Fri, Mar 25, 2022 at 2:13 PM Johannes Berg
> wrote:
> >
> > Well I see now that you said 'cache "writeback"' in (1), and 'flush' in
> > (2), so perhaps you were thinking of the same, and I'm just calling it
> > "flush" and "invalidate" resp
On Fri, Mar 25, 2022 at 2:13 PM Johannes Berg wrote:
>
> Well I see now that you said 'cache "writeback"' in (1), and 'flush' in
> (2), so perhaps you were thinking of the same, and I'm just calling it
> "flush" and "invalidate" respectively?
Yeah, so I mentally tend to think of the operations as
I've been thinking of the case where a descriptor ring has
to be in non-coherent memory (eg because that is all there is).
The receive ring processing isn't actually that difficult.
The driver has to fill a cache line full of new buffer
descriptors in memory but without assigning the first
buffer
On Fri, 2022-03-25 at 13:47 -0700, Linus Torvalds wrote:
> On Fri, Mar 25, 2022 at 1:38 PM Johannes Berg
> wrote:
> >
> > > (2) The CPU now wants to see any state written by the device since
> > > the last sync
> > >
> > > This is "dma_sync_single_for_cpu(DMA_FROM_DEVICE)".
> > >
> > >
On Fri, Mar 25, 2022 at 1:38 PM Johannes Berg wrote:
>
> > (2) The CPU now wants to see any state written by the device since
> > the last sync
> >
> > This is "dma_sync_single_for_cpu(DMA_FROM_DEVICE)".
> >
> > A bounce-buffer implementation needs to copy *from* the bounce buffer.
> >
>
So I've been watching this from the sidelines mostly, and discussing a
bit with Toke, but:
On Fri, 2022-03-25 at 11:30 -0700, Linus Torvalds wrote:
>
> (2) The CPU now wants to see any state written by the device since
> the last sync
>
> This is "dma_sync_single_for_cpu(DMA_FROM_DEVICE)".
On pátek 25. března 2022 20:27:43 CET Linus Torvalds wrote:
> On Fri, Mar 25, 2022 at 12:26 PM Oleksandr Natalenko
> wrote:
> >
> > On pátek 25. března 2022 19:30:21 CET Linus Torvalds wrote:
> > > The reason the ath9k issue was found quickly
> > > is very likely *NOT* because ath9k is the only th
On Fri, Mar 25, 2022 at 12:14 PM Robin Murphy wrote:
>
> Note "between the DMA transfers", and not "during the DMA transfers".
> The fundamental assumption of the streaming API is that only one thing
> is ever accessing the mapping at any given time, which is what the whole
> notion of ownership i
On Fri, Mar 25, 2022 at 12:26 PM Oleksandr Natalenko
wrote:
>
> On pátek 25. března 2022 19:30:21 CET Linus Torvalds wrote:
> > The reason the ath9k issue was found quickly
> > is very likely *NOT* because ath9k is the only thing affected. No,
> > it's because ath9k is relatively common.
>
> Indee
On pátek 25. března 2022 19:30:21 CET Linus Torvalds wrote:
> The reason the ath9k issue was found quickly
> is very likely *NOT* because ath9k is the only thing affected. No,
> it's because ath9k is relatively common.
Indeed. But having a wife who complains about non-working Wi-Fi printer
defini
On 2022-03-25 18:30, Linus Torvalds wrote:
On Fri, Mar 25, 2022 at 3:25 AM Maxime Bizon wrote:
In the non-cache-coherent scenario, and assuming dma_map() did an
initial cache invalidation, you can write this:
.. but the problem is that the dma mapping code is supposed to just
work, and the d
On Fri, Mar 25, 2022 at 11:42 AM Robin Murphy wrote:
>
> Note that the current code is already a violation of the DMA
> API (because the device keeps writing even when it doesn't have
> ownership), so there's not a very strong argument in that regard.
See my other email. I actually think that the
On 2022-03-25 18:15, Toke Høiland-Jørgensen wrote:
Christoph Hellwig writes:
On Thu, Mar 24, 2022 at 07:02:16PM +0100, Halil Pasic wrote:
If
ddbd89deb7d3 alone turns out to work OK then I'd be inclined to try a
partial revert of just that one hunk.
I'm not against being pragmatic and doing
On Fri, Mar 25, 2022 at 3:25 AM Maxime Bizon wrote:
>
> In the non-cache-coherent scenario, and assuming dma_map() did an
> initial cache invalidation, you can write this:
.. but the problem is that the dma mapping code is supposed to just
work, and the driver isn't supposed to know or care wheth
Christoph Hellwig writes:
> On Thu, Mar 24, 2022 at 07:02:16PM +0100, Halil Pasic wrote:
>> > If
>> > ddbd89deb7d3 alone turns out to work OK then I'd be inclined to try a
>> > partial revert of just that one hunk.
>> >
>>
>> I'm not against being pragmatic and doing the partial revert. But as
Robin Murphy writes:
> On 2022-03-25 16:25, Toke Høiland-Jørgensen wrote:
>> Maxime Bizon writes:
>>
>>> On Thu, 2022-03-24 at 12:26 -0700, Linus Torvalds wrote:
>>>
It's actually very natural in that situation to flush the caches from
the CPU side again. And so dma_sync_single_f
On 2022-03-25 16:25, Toke Høiland-Jørgensen wrote:
Maxime Bizon writes:
On Thu, 2022-03-24 at 12:26 -0700, Linus Torvalds wrote:
It's actually very natural in that situation to flush the caches from
the CPU side again. And so dma_sync_single_for_device() is a fairly
reasonable thing to do i
On Thu, Mar 24, 2022 at 07:02:16PM +0100, Halil Pasic wrote:
> > If
> > ddbd89deb7d3 alone turns out to work OK then I'd be inclined to try a
> > partial revert of just that one hunk.
> >
>
> I'm not against being pragmatic and doing the partial revert. But as
> explained above, I do believe for
On Thu, Mar 24, 2022 at 07:31:58PM +0100, Halil Pasic wrote:
> I agree with your analysis. Especially with the latter part (were you
> state that we don't have a good idiom for that use case).
>
> I believe, a stronger statement is also true: it is fundamentally
> impossible to accommodate use ca
Maxime Bizon writes:
> On Thu, 2022-03-24 at 12:26 -0700, Linus Torvalds wrote:
>
>>
>> It's actually very natural in that situation to flush the caches from
>> the CPU side again. And so dma_sync_single_for_device() is a fairly
>> reasonable thing to do in that situation.
>>
>
> In the non-cac
On 2022-03-25 15:25, Halil Pasic wrote:
On Thu, 24 Mar 2022 19:02:16 +0100
Halil Pasic wrote:
I'll admit I still never quite grasped the reason for also adding the
override to swiotlb_sync_single_for_device() in aa6f8dcbab47, but I
think by that point we were increasingly tired and confused an
On Thu, 24 Mar 2022 19:02:16 +0100
Halil Pasic wrote:
> > I'll admit I still never quite grasped the reason for also adding the
> > override to swiotlb_sync_single_for_device() in aa6f8dcbab47, but I
> > think by that point we were increasingly tired and confused and starting
> > to second-gue
On 2022-03-25 10:25, Maxime Bizon wrote:
On Thu, 2022-03-24 at 12:26 -0700, Linus Torvalds wrote:
It's actually very natural in that situation to flush the caches from
the CPU side again. And so dma_sync_single_for_device() is a fairly
reasonable thing to do in that situation.
In the non-c
On Thu, 2022-03-24 at 12:26 -0700, Linus Torvalds wrote:
>
> It's actually very natural in that situation to flush the caches from
> the CPU side again. And so dma_sync_single_for_device() is a fairly
> reasonable thing to do in that situation.
>
In the non-cache-coherent scenario, and assumi
On 25.03.22 08:12, Oleksandr Natalenko wrote:
> On čtvrtek 24. března 2022 18:07:29 CET Toke Høiland-Jørgensen wrote:
>> Right, but is that sync_for_device call really needed? AFAICT, that
>> ath9k_hw_process_rxdesc_edma() invocation doesn't actually modify any of
>> the data when it returns EINPRO
Hello.
On čtvrtek 24. března 2022 18:07:29 CET Toke Høiland-Jørgensen wrote:
> Right, but is that sync_for_device call really needed? AFAICT, that
> ath9k_hw_process_rxdesc_edma() invocation doesn't actually modify any of
> the data when it returns EINPROGRESS, so could we just skip it? Like
> the
Linus Torvalds writes:
> On Thu, Mar 24, 2022 at 10:07 AM Toke Høiland-Jørgensen wrote:
>>
>> Right, but is that sync_for_device call really needed?
>
> Well, imagine that you have a non-cache-coherent DMA (not bounce
> buffers - just bad hardware)...
>
> So the driver first does that dma_sync_s
On Thu, Mar 24, 2022 at 10:07 AM Toke Høiland-Jørgensen wrote:
>
> Right, but is that sync_for_device call really needed?
Well, imagine that you have a non-cache-coherent DMA (not bounce
buffers - just bad hardware)...
So the driver first does that dma_sync_single_for_cpu() for the CPU
see the c
On Thu, 24 Mar 2022 16:52:31 +
Robin Murphy wrote:
> > Creating a new mapping for the same buffer before unmapping the
> > previous one does looks rather bogus. But it does not fit the
> > pattern where revering the sync_single changes make the driver
> > work again.
>
> OK, you made me l
On Wed, 23 Mar 2022 20:54:08 +
Robin Murphy wrote:
> On 2022-03-23 19:16, Linus Torvalds wrote:
> > On Wed, Mar 23, 2022 at 12:06 PM Robin Murphy wrote:
> >
> >>
> >> On 2022-03-23 17:27, Linus Torvalds wrote:
> >>>
> >>> I'm assuming that the ath9k issue is that it gives DMA mapping a
Robin Murphy writes:
> On 2022-03-24 16:31, Christoph Hellwig wrote:
>> On Thu, Mar 24, 2022 at 05:29:12PM +0100, Maxime Bizon wrote:
I'm looking into this; but in the interest of a speedy resolution of
the regression I would be in favour of merging that partial revert
and reinstat
On 2022-03-24 16:31, Christoph Hellwig wrote:
On Thu, Mar 24, 2022 at 05:29:12PM +0100, Maxime Bizon wrote:
I'm looking into this; but in the interest of a speedy resolution of
the regression I would be in favour of merging that partial revert
and reinstating it if/when we identify (and fix) any
On Thu, 2022-03-24 at 15:27 +0100, Toke Høiland-Jørgensen wrote:
>
> I'm looking into this; but in the interest of a speedy resolution of
> the regression I would be in favour of merging that partial revert
> and reinstating it if/when we identify (and fix) any bugs in ath9k :)
This looks fishy
On Thu, Mar 24, 2022 at 05:29:12PM +0100, Maxime Bizon wrote:
> > I'm looking into this; but in the interest of a speedy resolution of
> > the regression I would be in favour of merging that partial revert
> > and reinstating it if/when we identify (and fix) any bugs in ath9k :)
>
> This looks fis
Robin Murphy writes:
> On 2022-03-24 10:25, Oleksandr Natalenko wrote:
>> Hello.
>>
>> On čtvrtek 24. března 2022 6:57:32 CET Christoph Hellwig wrote:
>>> On Wed, Mar 23, 2022 at 08:54:08PM +, Robin Murphy wrote:
I'll admit I still never quite grasped the reason for also adding the
On 2022-03-24 10:25, Oleksandr Natalenko wrote:
Hello.
On čtvrtek 24. března 2022 6:57:32 CET Christoph Hellwig wrote:
On Wed, Mar 23, 2022 at 08:54:08PM +, Robin Murphy wrote:
I'll admit I still never quite grasped the reason for also adding the
override to swiotlb_sync_single_for_device(
Hello.
On čtvrtek 24. března 2022 6:57:32 CET Christoph Hellwig wrote:
> On Wed, Mar 23, 2022 at 08:54:08PM +, Robin Murphy wrote:
> > I'll admit I still never quite grasped the reason for also adding the
> > override to swiotlb_sync_single_for_device() in aa6f8dcbab47, but I think
> > by th
Hello.
On středa 23. března 2022 18:27:21 CET Linus Torvalds wrote:
> On Wed, Mar 23, 2022 at 12:19 AM Oleksandr Natalenko
> wrote:
> > These commits appeared in v5.17 and v5.16.15, and both kernels are
> > broken for me. I'm pretty confident these commits make the difference
> > since I've built
On Wed, Mar 23, 2022 at 08:54:08PM +, Robin Murphy wrote:
> I'll admit I still never quite grasped the reason for also adding the
> override to swiotlb_sync_single_for_device() in aa6f8dcbab47, but I think
> by that point we were increasingly tired and confused and starting to
> second-guess
On 2022-03-23 19:16, Linus Torvalds wrote:
On Wed, Mar 23, 2022 at 12:06 PM Robin Murphy wrote:
On 2022-03-23 17:27, Linus Torvalds wrote:
I'm assuming that the ath9k issue is that it gives DMA mapping a big
enough area to handle any possible packet size, and just expects -
quite reasonably
On Wed, Mar 23, 2022 at 12:06 PM Robin Murphy wrote:
>
> On 2022-03-23 17:27, Linus Torvalds wrote:
> >
> > I'm assuming that the ath9k issue is that it gives DMA mapping a big
> > enough area to handle any possible packet size, and just expects -
> > quite reasonably - smaller packets to only fil
On 2022-03-23 17:27, Linus Torvalds wrote:
On Wed, Mar 23, 2022 at 12:19 AM Oleksandr Natalenko
wrote:
The following upstream commits:
aa6f8dcbab47 swiotlb: rework "fix info leak with DMA_FROM_DEVICE"
ddbd89deb7d3 swiotlb: fix info leak with DMA_FROM_DEVICE
break ath9k-based Wi-Fi access poi
On Wed, Mar 23, 2022 at 12:19 AM Oleksandr Natalenko
wrote:
>
> The following upstream commits:
>
> aa6f8dcbab47 swiotlb: rework "fix info leak with DMA_FROM_DEVICE"
> ddbd89deb7d3 swiotlb: fix info leak with DMA_FROM_DEVICE
>
> break ath9k-based Wi-Fi access point for me. The AP emits beacons, bu
Adding regressions list so that this can be tracked properly, including
the full report below.
Oleksandr Natalenko writes:
> Hello.
>
> The following upstream commits:
>
> aa6f8dcbab47 swiotlb: rework "fix info leak with DMA_FROM_DEVICE"
> ddbd89deb7d3 swiotlb: fix info leak with DMA_FROM_DEVICE
67 matches
Mail list logo
