linux-kernel - RE: [PATCH v5 2/6] dmaengine: sh: rz-dmac: Move all CHCTRL updates under spinlock

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID:
 <TYCPR01MB11332CA51AE3F9792883FD9A386A8A@TYCPR01MB11332.jpnprd01.prod.outlook.com>
Date: Thu, 18 Dec 2025 08:30:17 +0000
From: Biju Das <biju.das.jz@...renesas.com>
To: Claudiu.Beznea <claudiu.beznea@...on.dev>, "vkoul@...nel.org"
	<vkoul@...nel.org>, Fabrizio Castro <fabrizio.castro.jz@...esas.com>,
	"geert+renesas@...der.be" <geert+renesas@...der.be>, Prabhakar Mahadev Lad
	<prabhakar.mahadev-lad.rj@...renesas.com>
CC: "dmaengine@...r.kernel.org" <dmaengine@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Claudiu Beznea
	<claudiu.beznea.uj@...renesas.com>, "stable@...r.kernel.org"
	<stable@...r.kernel.org>
Subject: RE: [PATCH v5 2/6] dmaengine: sh: rz-dmac: Move all CHCTRL updates
 under spinlock

Hi Claudiu,

> -----Original Message-----
> From: Claudiu Beznea <claudiu.beznea@...on.dev>
> Sent: 18 December 2025 07:44
> Subject: Re: [PATCH v5 2/6] dmaengine: sh: rz-dmac: Move all CHCTRL updates under spinlock
> 
> Hi, Biju,
> 
> On 12/17/25 18:16, Biju Das wrote:
> > Hi Claudiu,
> >
> >> -----Original Message-----
> >> From: Claudiu <claudiu.beznea@...on.dev>
> >> Sent: 17 December 2025 13:52
> >> To: vkoul@...nel.org; Fabrizio Castro
> >> <fabrizio.castro.jz@...esas.com>; Biju Das
> >> <biju.das.jz@...renesas.com>; geert+renesas@...der.be; Prabhakar
> >> Mahadev Lad <prabhakar.mahadev- lad.rj@...renesas.com>
> >> Cc: Claudiu.Beznea <claudiu.beznea@...on.dev>;
> >> dmaengine@...r.kernel.org; linux- kernel@...r.kernel.org; Claudiu
> >> Beznea <claudiu.beznea.uj@...renesas.com>; stable@...r.kernel.org
> >> Subject: [PATCH v5 2/6] dmaengine: sh: rz-dmac: Move all CHCTRL
> >> updates under spinlock
> >>
> >> From: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
> >>
> >> Both rz_dmac_disable_hw() and rz_dmac_irq_handle_channel() update the
> >> CHCTRL registers. To avoid concurrency issues when updating these
> >> registers, take the virtual channel lock. All other CHCTRL updates were already protected by the
> same lock.
> >>
> >> Previously, rz_dmac_disable_hw() disabled and re-enabled local IRQs,
> >> before accessing CHCTRL registers but this does not ensure race-free access.
> >
> > Maybe I am missing some thing here about race-access:
> >
> > 	local_irq_save(flags);
> >    	rz_dmac_ch_writel(channel, CHCTRL_DEFAULT, CHCTRL, 1);
> >
> > After local_irq_save there won't be any IRQ. So how there can be a
> > race in IRQ handler.
> 
> My point was to address races that may happen b/w different cores trying to set CHCTRL. E.g.:
> 
> core0: take the IRQ and set CHCTRL
> core1: call rz_dmac_issue_pending() -> rz_dmac_xfer_desc() ->
> rz_dmac_enable_hw() -> set CHCTRL

For that you could use a new lock. Currently all the calls are serialized using vc_lock
In irqhandler and irqhandler unnecessarily keeps spinning as the chctrl register is the only shared
Data in interrupt handler.

> 
> However, looking again though the HW manual, the CHCTRL returns zero when it is read, for each
> individual bit. Thus, there is no need for any kind of locking around this register. Also, read-
> modify-write approach when updating settings though it is not needed.

But still you need a lock other than vc_lock as CHCTRL is shared register between IRQhandler and tasklet/process context


Cheers,
Biju