linux-kernel - Re: [PATCH] coresight: dynamic-replicator: Fix handling of multiple connections

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: <861f8ab0174d036cb1e49e34e4f81a92@codeaurora.org>
Date:   Wed, 13 May 2020 21:15:32 +0530
From:   Sai Prakash Ranjan <saiprakash.ranjan@...eaurora.org>
To:     Stephen Boyd <swboyd@...omium.org>
Cc:     Mike Leach <mike.leach@...aro.org>,
        Suzuki K Poulose <suzuki.poulose@....com>,
        Mathieu Poirier <mathieu.poirier@...aro.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-arm-msm@...r.kernel.org,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        Russell King <linux@...linux.org.uk>
Subject: Re: [PATCH] coresight: dynamic-replicator: Fix handling of multiple
 connections

On 2020-05-13 07:19, Stephen Boyd wrote:
> Quoting Mike Leach (2020-05-12 14:52:33)
>> HI Sai,
>> 
>> On Tue, 12 May 2020 at 18:46, Sai Prakash Ranjan
>> <saiprakash.ranjan@...eaurora.org> wrote:
>> >
>> > Hi Mike,
>> >
>> > On 2020-05-12 17:19, Mike Leach wrote:
>> > [...]
>> >
>> > >> >>
>> > >> >> Sorry for hurrying up and sending the patch -
>> > >> >> https://lore.kernel.org/patchwork/patch/1239923/.
>> > >> >> I will send v2 based on further feedbacks here or there.
>> > >> >>
>> > >> >>>
>> > >> >>> 1) does this replicator part have a unique ID that differs from the
>> > >> >>> standard ARM designed replicators?
>> > >> >>> If so perhaps link the modification into this. (even if the part no
>> > >> >>> in
>> > >> >>> PIDR0/1 is the same the UCI should be different for a different
>> > >> >>> implementation)
>> > >> >>>
>> > > I have reviewed the replicator driver, and compared to all the other CS
>> > > drivers.
>> > > This driver appears to be the only one that sets hardware values in
>> > > probe() and expects them to remain in place on enable, and uses that
>> > > state for programming decisions later, despite telling the PM
>> > > infrastructure that it is clear to suspend the device.
>> > >
>> > > Now we have a system where the replicator hardware is behaving
>> > > differently under the driver, but is it behaving unreasonably?
>> >
>> > Thanks for taking your time to review this. For new replicator behaving
>> > unreasonably, I think the assumption that the context is not lost on
>> > disabling clock is flawed since its implementation defined. Is such
>> > assumption documented in any TRM?
>> >
>> 
>> Looking at the AMBA driver there is a comment there that AMBA does not
>> lose state when clocks are removed. This is consistent with the AMBA
>> protocol spec which states that AMBA slaves can only be accessed /
>> read / write on various strobe signals,  or state reset on PRESET
>> signal, all timed by the rising edge of the bus clock. state changes
>> are not permitted on clock events alone. Given this static nature of
>> AMBA slaves then removing the clock should not have any effect.
> 
> I believe the "clock" that is being used here is actually a software
> message to the power manager hardware that the debug subsystem isn't
> being used anymore. When nothing is requesting that it be enabled the
> power manager turns off the power to the debug subsystem and then the
> register context is lost. It shouldn't be a clock in the clk subsystem.
> It should be a power domain and be attached to the amba devices in the
> usual ways. Then the normal runtime PM semantics would follow. If amba
> devices require a clk then we'll have to provide a dummy one that
> doesn't do anything on this platform.
> 

Note that there are 2 dynamic replicators and the behaviour is different 
only on
one of the replicators(swao_replicator) which is in AOSS domain. I don't 
see
how runtime PM would help us differentiate between them and handle PM 
differently
for different replicators.

Thanks,
Sai

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation