linux-kernel - RE: [PATCH 1/2] dmaengine: tegra: Add terminate() for Tegra234

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 7 Jul 2022 10:47:41 +0000
From:   Akhil R <akhilrajeev@...dia.com>
To:     Jonathan Hunter <jonathanh@...dia.com>,
        "dmaengine@...r.kernel.org" <dmaengine@...r.kernel.org>,
        Laxman Dewangan <ldewangan@...dia.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
        "p.zabel@...gutronix.de" <p.zabel@...gutronix.de>,
        "thierry.reding@...il.com" <thierry.reding@...il.com>,
        "vkoul@...nel.org" <vkoul@...nel.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: RE: [PATCH 1/2] dmaengine: tegra: Add terminate() for Tegra234

> On 07/07/2022 11:27, Akhil R wrote:
> > In certain cases where the DMA client bus gets corrupted or if the end
> > device ceases to send/receive data, DMA can wait indefinitely for the
> > data to be received/sent. Attempting to terminate the transfer will
> > put the DMA in pause flush mode and it remains there.
> >
> > The channel is irrecoverable once this pause times out in Tegra194 and
> > earlier chips. Whereas, from Tegra234, it can be recovered by
> > disabling the channel and reprograming it.
> >
> > Hence add a new terminate() function that ignores the outcome of
> > dma_pause() and disables the channel.
> >
> > Signed-off-by: Akhil R <akhilrajeev@...dia.com>
> > ---
> >   arch/arm64/boot/dts/nvidia/tegra234.dtsi |  5 +++--
> >   drivers/dma/tegra186-gpc-dma.c           | 26 ++++++++++++++++++++++--
> >   2 files changed, 27 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> > b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> > index cf611eff7f6b..83d1ad7d3c8c 100644
> > --- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> > +++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> > @@ -22,8 +22,9 @@
> >   		ranges = <0x0 0x0 0x0 0x40000000>;
> >
> >   		gpcdma: dma-controller@...0000 {
> > -			compatible = "nvidia,tegra194-gpcdma",
> > -				      "nvidia,tegra186-gpcdma";
> > +			compatible = "nvidia,tegra234-gpcdma",
> > +				     "nvidia,tegra194-gpcdma",
> > +				     "nvidia,tegra186-gpcdma";
> >   			reg = <0x2600000 0x210000>;
> >   			resets = <&bpmp TEGRA234_RESET_GPCDMA>;
> >   			reset-names = "gpcdma";
> 
> I think that this should be split into two patches.
> 
> > diff --git a/drivers/dma/tegra186-gpc-dma.c
> > b/drivers/dma/tegra186-gpc-dma.c index 05cd451f541d..fa9bda4a2bc6
> > 100644
> > --- a/drivers/dma/tegra186-gpc-dma.c
> > +++ b/drivers/dma/tegra186-gpc-dma.c
> > @@ -157,8 +157,8 @@
> >    * If any burst is in flight and DMA paused then this is the time to complete
> >    * on-flight burst and update DMA status register.
> >    */
> > -#define TEGRA_GPCDMA_BURST_COMPLETE_TIME	20
> > -#define TEGRA_GPCDMA_BURST_COMPLETION_TIMEOUT	100
> > +#define TEGRA_GPCDMA_BURST_COMPLETE_TIME	10
> > +#define TEGRA_GPCDMA_BURST_COMPLETION_TIMEOUT	5000 /* 5
> msec */
> >
> >   /* Channel base address offset from GPCDMA base address */
> >   #define TEGRA_GPCDMA_CHANNEL_BASE_ADD_OFFSET	0x20000
> > @@ -432,6 +432,17 @@ static int tegra_dma_device_resume(struct
> dma_chan *dc)
> >   	return 0;
> >   }
> >
> > +static inline int tegra_dma_pause_noerr(struct tegra_dma_channel
> > +*tdc) {
> > +	/* Return 0 irrespective of PAUSE status.
> > +	 * This is useful to recover channels that can exit out of flush
> > +	 * state when the channel is disabled.
> > +	 */
> > +
> > +	tegra_dma_pause(tdc);
> > +	return 0;
> > +}
> 
> The commit message says that "add a new terminate() function that ignores the
> outcome of dma_pause() and disables the channel". But I only see pause being
> done here.
The function is set as .terminate() function in chip_data and is called during
terminate_all(). Since this return 0, tegra_dma_terminate_all() will proceed
and calls tegra_dma_disable() in the next step.

Regards,
Akhil

--
nvpublic