linux-hardening - Re: [PATCH 05/12] dmaengine: Add STM32 DMA3 support

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] [thread-next>] [day] [month] [year] [list]

Date: Tue, 7 May 2024 14:37:37 +0200
From: Amelie Delaunay <amelie.delaunay@...s.st.com>
To: Christophe JAILLET <christophe.jaillet@...adoo.fr>
CC: <alexandre.torgue@...s.st.com>, <conor+dt@...nel.org>,
        <devicetree@...r.kernel.org>, <dmaengine@...r.kernel.org>,
        <krzysztof.kozlowski+dt@...aro.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-hardening@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-stm32@...md-mailman.stormreply.com>,
        <mcoquelin.stm32@...il.com>, <robh+dt@...nel.org>, <vkoul@...nel.org>
Subject: Re: [PATCH 05/12] dmaengine: Add STM32 DMA3 support

Hi Christophe,

Thanks for the review.

On 5/4/24 16:27, Christophe JAILLET wrote:
> Le 23/04/2024 à 14:32, Amelie Delaunay a écrit :
>> STM32 DMA3 driver supports the 3 hardware configurations of the STM32 
>> DMA3
>> controller:
>> - LPDMA (Low Power): 4 channels, no FIFO
>> - GPDMA (General Purpose): 16 channels, FIFO from 8 to 32 bytes
>> - HPDMA (High Performance): 16 channels, FIFO from 8 to 256 bytes
>> Hardware configuration of the channels is retrieved from the hardware
>> configuration registers.
>> The client can specify its channel requirements through device tree.
>> STM32 DMA3 channels can be individually reserved either because they are
>> secure, or dedicated to another CPU.
>> Indeed, channels availability depends on Resource Isolation Framework
>> (RIF) configuration. RIF grants access to buses with Compartiment ID
>> (CIF) filtering, secure and privilege level. It also assigns DMA channels
>> to one or several processors.
>> DMA channels used by Linux should be CID-filtered and statically assigned
>> to CID1 or shared with other CPUs but using semaphore. In case CID
>> filtering is not configured, dma-channel-mask property can be used to
>> specify available DMA channels to the kernel, otherwise such channels
>> will be marked as reserved and can't be used by Linux.
>>
>> Signed-off-by: Amelie Delaunay 
>> <amelie.delaunay-rj0Iel/JR4NBDgjK7y7TUQ@...lic.gmane.org>
>> ---
> 
> ...
> 
>> +    pm_runtime_set_active(&pdev->dev);
>> +    pm_runtime_enable(&pdev->dev);
>> +    pm_runtime_get_noresume(&pdev->dev);
>> +    pm_runtime_put(&pdev->dev);
>> +
>> +    dev_info(&pdev->dev, "STM32 DMA3 registered rev:%lu.%lu\n",
>> +         FIELD_GET(VERR_MAJREV, verr), FIELD_GET(VERR_MINREV, verr));
>> +
>> +    return 0;
>> +
>> +err_clk_disable:
>> +    clk_disable_unprepare(ddata->clk);
>> +
>> +    return ret;
>> +}
>> +
>> +static void stm32_dma3_remove(struct platform_device *pdev)
>> +{
>> +    pm_runtime_disable(&pdev->dev);
> 
> Hi,
> 
> missing clk_disable_unprepare(ddata->clk);?
> 
> as in the error handling path on the probe just above?
> 
> CJ
> 

Clock is entirely managed by pm_runtime, except in error path of probe 
since pm_runtime is enabled only at the very end.
Clock is enabled with pm_runtime_resume_and_get() when a channel is 
requested or when an asynchronous register access occurs (filter_fn, 
debugfs, runtime_resume) and clock is disabled with 
pm_runtime_put_sync() when releasing a channel or at the end of 
asynchronous register access (filter_fn, debugfs, runtime_suspend).
Adding clk_disable_unprepare(ddata->clk); here leads to clock already 
disabled/unprepared warnings in drivers/clk/clk.c 
clk_core_disable()/clk_core_unprepare().

>> +}
> 
> ...
> 

Regards,
Amelie