linux-kernel - Re: [PATCH v4 2/4] dmaengine: gpi: Add Lock and Unlock TRE support to access I2C exclusively

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]

Message-ID: <12337d5f-63ad-43f1-a9e0-87eb2e3abf61@quicinc.com>
Date: Mon, 25 Nov 2024 10:31:43 +0530
From: Mukesh Kumar Savaliya <quic_msavaliy@...cinc.com>
To: Konrad Dybcio <konrad.dybcio@....qualcomm.com>, <konrad.dybcio@...aro.org>,
        <andersson@...nel.org>, <andi.shyti@...nel.org>,
        <linux-arm-msm@...r.kernel.org>, <dmaengine@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <linux-i2c@...r.kernel.org>,
        <conor+dt@...nel.org>, <agross@...nel.org>,
        <devicetree@...r.kernel.org>, <vkoul@...nel.org>, <linux@...blig.org>,
        <dan.carpenter@...aro.org>, <Frank.Li@....com>,
        <konradybcio@...nel.org>, <bryan.odonoghue@...aro.org>,
        <krzk+dt@...nel.org>, <robh@...nel.org>
CC: <quic_vdadhani@...cinc.com>
Subject: Re: [PATCH v4 2/4] dmaengine: gpi: Add Lock and Unlock TRE support to
 access I2C exclusively

Hi Konrad, Thanks !

On 11/22/2024 7:10 PM, Konrad Dybcio wrote:
> On 18.11.2024 6:46 AM, Mukesh Kumar Savaliya wrote:
>> Thanks Konrad for the review !
>>
>> On 11/16/2024 12:53 AM, Konrad Dybcio wrote:
>>> On 13.11.2024 5:14 PM, Mukesh Kumar Savaliya wrote:
>>>> GSI DMA provides specific TREs(Transfer ring element) namely Lock and
>>>> Unlock TRE. It provides mutually exclusive access to I2C controller from
>>>> any of the processor(Apps,ADSP). Lock prevents other subsystems from
>>>> concurrently performing DMA transfers and avoids disturbance to data path.
>>>> Basically for shared I2C usecase, lock the SE(Serial Engine) for one of
>>>> the processor, complete the transfer, unlock the SE.
>>>>
>>>> Apply Lock TRE for the first transfer of shared SE and Apply Unlock
>>>> TRE for the last transfer.
>>>>
>>>> Also change MAX_TRE macro to 5 from 3 because of the two additional TREs.
>>>>
>>>> Signed-off-by: Mukesh Kumar Savaliya <quic_msavaliy@...cinc.com>
>>>> ---
> 
> [...]
> 
>>>>    +    /* create lock tre for first tranfser */
>>>> +    if (i2c->shared_se && i2c->first_msg) {
>>>
>>> Does the first/last logic handle errors well? i.e. what if we
>>> have >= 3 transfers and:
>>>
>>> 1) the first transfer succeeds but the last doesn't
>>> 2) the first transfer succeeds, the second one doesn't and the lock
>>>      is submitted again
>>> 3) the unlock never suceeds
>>>
>> geni_i2c_gpi_xfer() takes care of any of the error. Upon error, it does dma_engine_terminate_sync() which resets all the pipes. Internal downstream also has same implementation.
> 
> Okay, this sounds reassuring.
> 
> Since the TRE would be locked to APSS, I'm guessing we don't ever need
> to worry about gpi_terminate_all() being executed halfway through a
> non-APSS transaction?
> 
Right.
> Konrad