linux-kernel - Re: [Patch v7 1/8] memory: tegra: add interconnect support for DRAM scaling in 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <10e2d3b4-d66f-947f-4643-97b1f04e4b31@nvidia.com>
Date:   Tue, 9 May 2023 20:17:41 +0530
From:   Sumit Gupta <sumitg@...dia.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        <treding@...dia.com>, <dmitry.osipenko@...labora.com>,
        <viresh.kumar@...aro.org>, <rafael@...nel.org>,
        <jonathanh@...dia.com>, <robh+dt@...nel.org>,
        <lpieralisi@...nel.org>, <helgaas@...nel.org>
CC:     <linux-kernel@...r.kernel.org>, <linux-tegra@...r.kernel.org>,
        <linux-pm@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-pci@...r.kernel.org>, <mmaddireddy@...dia.com>,
        <kw@...ux.com>, <bhelgaas@...gle.com>, <vidyas@...dia.com>,
        <sanjayc@...dia.com>, <ksitaraman@...dia.com>, <ishah@...dia.com>,
        <bbasu@...dia.com>, Sumit Gupta <sumitg@...dia.com>
Subject: Re: [Patch v7 1/8] memory: tegra: add interconnect support for DRAM
 scaling in Tegra234



On 09/05/23 19:06, Krzysztof Kozlowski wrote:
> External email: Use caution opening links or attachments
> 
> 
> On 09/05/2023 15:17, Sumit Gupta wrote:
>>>>>>> +                     /*
>>>>>>> +                      * MC driver probe can't get BPMP reference as
>>>>>>> it gets probed
>>>>>>> +                      * earlier than BPMP. So, save the BPMP ref got
>>>>>>> from the EMC
>>>>>>> +                      * DT node in the mc->bpmp and use it in MC's
>>>>>>> icc_set hook.
>>>>>>> +                      */
>>>>>>> +                     mc->bpmp = emc->bpmp;
>>>>>>
>>>>>> This (and ()) are called without any locking. You register first the
>>>>>> interconnect, so set() callback can be used, right? Then set() could be
>>>>>> called anytime between tegra_emc_interconnect_init() and assignment
>>>>>> above. How do you synchronize these?
>>>>>>
>>>>>> Best regards,
>>>>>> Krzysztof
>>>>>>
>>>>>
>>>>> Currently, the tegra234_mc_icc_set() has NULL check. So, it will give
>>>>> this error.
>>>>>     if (!mc->bpmp) {
>>>
>>> How does it solve concurrent accesses and re-ordering of instructions by
>>> compiler or CPU?
>>>
>>
>> Now, the "mc->bpmp" is set before tegra_emc_interconnect_init().
>> So, until the EMC interconnect initializes, set() won't be
>> called as the devm_of_icc_get() call will fail.
> 
> What if compiler puts "mc->bpmp" assignment after
> tegra_emc_interconnect_init()?
> 
> What if CPU executes above assignment also after
> tegra_emc_interconnect_init()?
> 
> Considering amount of code inside tegra_emc_interconnect_init() second
> case is rather unlikely, but first possible, right?
> 
> Best regards,
> Krzysztof
> 

Yes, for protection from compiler re-ordering I can add
barrier() after the assignment.

   mc->bpmp = emc->bpmp;
   barrier();

Thank you,
Sumit Gupta