linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Will Deacon <will@kernel.org>
To: Thorsten Leemhuis <regressions@leemhuis.info>
Cc: Amit Pundir <amit.pundir@linaro.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Sibi Sankar <quic_sibis@quicinc.com>,
	Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
	Robin Murphy <robin.murphy@arm.com>,
	andersson@kernel.org, sumit.semwal@linaro.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, hch@lst.de,
	gregkh@linuxfoundation.org
Subject: Re: [PATCH] Revert "arm64: dma: Drop cache invalidation from arch_dma_prep_coherent()"
Date: Fri, 2 Dec 2022 10:03:58 +0000	[thread overview]
Message-ID: <20221202100357.GB29396@willie-the-truck> (raw)
In-Reply-To: <f98d163b-3410-9cf7-7d98-0f7640f4aa1f@leemhuis.info>

On Fri, Dec 02, 2022 at 09:54:05AM +0100, Thorsten Leemhuis wrote:
> On 02.12.22 09:26, Amit Pundir wrote:
> > On Thu, 1 Dec 2022 at 23:15, Catalin Marinas <catalin.marinas@arm.com> wrote:
> >>
> >> On Thu, Dec 01, 2022 at 10:29:39AM +0100, Thorsten Leemhuis wrote:
> >>> Has any progress been made to fix this regression? It afaics is not a
> >>> release critical issue, but well, it still would be nice to get this
> >>> fixed before 6.1 is released.
> >>
> >> The only (nearly) risk-free "fix" for 6.1 would be to revert the commit
> >> that exposed the driver bug. It doesn't fix the actual bug, it only
> >> makes it less likely to happen.
> >>
> >> I like the original commit removing the cache invalidation as it shows
> >> drivers not behaving properly
> 
> Yeah, I understand that, but I guess it's my job to ask at this point:
> "is continuing to live with the old behavior for one or two more cycles"
> that much of a problem"?

That wouldn't be a problem. The problem is that I haven't see any efforts
from the Qualcomm side to actually fix the drivers so what makes you think
the issue will be addressed in one or two more cycles? On the other hand, if
there were patches out there trying to fix the drivers and we just needed to
revert this change to buy them some time, then that would obviously be the
right thing to do.

> >> but, as a workaround, we could add a
> >> command line option to force back the old behaviour (defaulting to the
> >> new one) until the driver is fixed.
> 
> Well, sometimes that approach is fine to fix a regression, but I'm not
> sure this is one of those situations, as this...
> 
> > We use DB845c extensively for mainline and android-mainline[1] testing
> > with AOSP, and it is broken for weeks now. So be it a temporary
> > workaround or a proper driver fix in place, we'd really appreciate a
> > quick fix here.
> 
> ...doesn't sound like we are not talking about some odd corner case
> here. But in the end that would be up to Linus to decide.

The issue is that these drivers are abusing the DMA API to manage buffers
which are being transferred to trustzone. Even with the revert, this is
broken (the CPU can speculate from the kernel's cacheable linear mapping
of memory), it just appears to be less likely with the CPUs on this SoC.
So we end up in a situation where the kernel is flakey on these devices
but with even less incentive for the drivers to be fixed.

As well as broken drivers, the patch has also identified broken device-tree
files where DMA-coherent devices weher incorrectly being treated as
non-coherent:

https://lore.kernel.org/linux-arm-kernel/20221124142501.29314-1-johan+linaro@kernel.org/

so I do think it's something that's worth having as the default behaviour.

> I'll point him to this thread once more in my weekly report anyway.
> Maybe I'll even suggest to revert this change, not sure yet.

As I said above, I think the revert makes sense if the drivers are actually
being fixed, but I'm not seeing any movement at all on that front.

Will

  reply	other threads:[~2022-12-02 10:04 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-14 11:03 [PATCH] Revert "arm64: dma: Drop cache invalidation from arch_dma_prep_coherent()" Manivannan Sadhasivam
2022-11-14 11:29 ` Manivannan Sadhasivam
2022-11-14 14:11 ` Will Deacon
2022-11-14 15:14   ` Robin Murphy
2022-11-14 17:38     ` Catalin Marinas
2022-11-18 10:54       ` Manivannan Sadhasivam
2022-11-18 12:33         ` Will Deacon
2022-11-21  6:42           ` Manivannan Sadhasivam
2022-11-21 10:12             ` Sibi Sankar
2022-11-24 11:55               ` Catalin Marinas
2022-12-01  9:29                 ` Thorsten Leemhuis
2022-12-01 17:45                   ` Catalin Marinas
2022-12-02  8:26                     ` Amit Pundir
2022-12-02  8:54                       ` Thorsten Leemhuis
2022-12-02 10:03                         ` Will Deacon [this message]
2022-12-02 10:34                           ` Thorsten Leemhuis
2022-12-02 16:10                             ` Greg KH
2022-12-02 16:27                               ` Thorsten Leemhuis
2022-12-02 16:32                                 ` Greg KH
2022-12-02 17:14                                   ` Manivannan Sadhasivam
2022-12-05 14:24                                     ` Will Deacon
2022-12-06  9:21                                       ` Manivannan Sadhasivam
2022-12-06  9:58                                         ` Will Deacon
2022-12-02 10:54                           ` Manivannan Sadhasivam
2022-11-28  5:44 ` Thorsten Leemhuis
2022-11-28  8:15   ` Manivannan Sadhasivam
2022-12-08  4:59 ` Leonard Lausen

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=20221202100357.GB29396@willie-the-truck \
    --to=will@kernel.org \
    --cc=amit.pundir@linaro.org \
    --cc=andersson@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hch@lst.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=manivannan.sadhasivam@linaro.org \
    --cc=quic_sibis@quicinc.com \
    --cc=regressions@leemhuis.info \
    --cc=robin.murphy@arm.com \
    --cc=sumit.semwal@linaro.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).