linux-kernel - Re: [PATCH] coresight: cti: Check if the CPU activated for the CPU CTI

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

Message-ID: <8a2dc999-2bed-0f32-4210-bc3d8f223581@arm.com>
Date:   Mon, 27 Mar 2023 15:57:25 +0100
From:   Suzuki K Poulose <suzuki.poulose@....com>
To:     Tao Zhang <quic_taozha@...cinc.com>,
        Mathieu Poirier <mathieu.poirier@...aro.org>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Konrad Dybcio <konradybcio@...il.com>,
        Mike Leach <mike.leach@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>
Cc:     Jinlong Mao <quic_jinlmao@...cinc.com>,
        Leo Yan <leo.yan@...aro.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        coresight@...ts.linaro.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
        Tingwei Zhang <quic_tingweiz@...cinc.com>,
        Yuanfang Zhang <quic_yuanfang@...cinc.com>,
        Trilok Soni <quic_tsoni@...cinc.com>,
        Hao Zhang <quic_hazha@...cinc.com>,
        linux-arm-msm@...r.kernel.org, andersson@...nel.org
Subject: Re: [PATCH] coresight: cti: Check if the CPU activated for the CPU
 CTI

On 27/03/2023 15:28, Tao Zhang wrote:
> Hi Suzuki,
> 
> On 3/27/2023 5:52 PM, Suzuki K Poulose wrote:
>> On 27/03/2023 10:49, Tao Zhang wrote:
>>> Check whether the CPU corresponding to the CPU CTI is activated.
>>> If it is not activated, the CPU CTI node should not exist, and
>>> an error will be returned in the initialization function.
>>>
>>> Signed-off-by: Tao Zhang <quic_taozha@...cinc.com>
>>> ---
>>>   drivers/hwtracing/coresight/coresight-cti-core.c | 6 ++++--
>>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c 
>>> b/drivers/hwtracing/coresight/coresight-cti-core.c
>>> index 277c890..aaa83ae 100644
>>> --- a/drivers/hwtracing/coresight/coresight-cti-core.c
>>> +++ b/drivers/hwtracing/coresight/coresight-cti-core.c
>>> @@ -899,10 +899,12 @@ static int cti_probe(struct amba_device *adev, 
>>> const struct amba_id *id)
>>>       drvdata->config.hw_powered = true;
>>>         /* set up device name - will depend if cpu bound or otherwise */
>>> -    if (drvdata->ctidev.cpu >= 0)
>>> +    if (drvdata->ctidev.cpu >= 0) {
>>> +        if (!cpu_active(drvdata->ctidev.cpu))
>>> +            return -ENXIO;
>>>           cti_desc.name = devm_kasprintf(dev, GFP_KERNEL, "cti_cpu%d",
>>>                              drvdata->ctidev.cpu);
>>
>> But why ? As long as we do not enable or touch any CPU specific bits 
>> in the probe, why do we need to fail this ? What are you trying to fix ?
>>
>> Please could you share the log if you are hitting something ? This looks
>> like masking a problem.
>>
>> Suzuki
> 
> We found that when the CPU core is disabled, for example, CPU3 is 
> disabled, but
> 
> CPU3 CTI node corresponding to CPU3 still exists. In fact, in this case, 
> CPU3 CTI
> 
> has been unable to trigger CPU3 properly since CPU3 is in an inactive 
> state. This change
> 
> is to avoid configuring the CPU CTI of the CPU that has been disabled in 
> this case.

Who is configuring the trigger ? Shouldn't we skip "enabling" the CTI
when the associated CPU is inactive instead ? Disabling the probe with
an error doesn't solve the problem. What if the CPU becomes active later 
? What makes sure that the CTI is probed then ?

Suzuki


> 
> Tao
> 
>>
>>
>>> -    else
>>> +    } else
>>>           cti_desc.name = coresight_alloc_device_name(&cti_sys_devs, 
>>> dev);
>>>       if (!cti_desc.name)
>>>           return -ENOMEM;
>>