linux-kernel - Re: [PATCH v2 6/9] coresight: Store in-connections as well as out-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: <eab64b08-34fb-996c-e57a-8f179c132de5@arm.com>
Date:   Wed, 29 Mar 2023 13:06:51 +0100
From:   James Clark <james.clark@....com>
To:     Jinlong Mao <quic_jinlmao@...cinc.com>, coresight@...ts.linaro.org
Cc:     Mathieu Poirier <mathieu.poirier@...aro.org>,
        Mike Leach <mike.leach@...aro.org>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 6/9] coresight: Store in-connections as well as
 out-connections



On 14/03/2023 05:35, Jinlong Mao wrote:
> 
> On 3/11/2023 12:06 AM, James Clark wrote:
>> This will allow CATU to get its associated ETR in a generic way where
>> currently the enable path has some hard coded searches which avoid
>> the need to store input connections.
>>
>> Signed-off-by: James Clark <james.clark@....com>
>> ---
>>   drivers/hwtracing/coresight/coresight-core.c  | 56 +++++++++++++++--
>>   .../hwtracing/coresight/coresight-platform.c  | 61 ++++++++++++++++---
>>   drivers/hwtracing/coresight/coresight-sysfs.c |  1 -
>>   include/linux/coresight.h                     | 25 ++++++++
>>   4 files changed, 130 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-core.c
>> b/drivers/hwtracing/coresight/coresight-core.c
>> index f457914e445e..a8ba7493c09a 100644
>> --- a/drivers/hwtracing/coresight/coresight-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>> @@ -59,6 +59,7 @@ const u32 coresight_barrier_pkt[4] = {0x7fffffff,
>> 0x7fffffff, 0x7fffffff, 0x7fff
>>   EXPORT_SYMBOL_GPL(coresight_barrier_pkt);
>>     static const struct cti_assoc_op *cti_assoc_ops;
>> +static int coresight_fixup_inputs(struct coresight_device *csdev);
>>     ssize_t coresight_simple_show_pair(struct device *_dev,
>>                     struct device_attribute *attr, char *buf)
>> @@ -1369,6 +1370,35 @@ static int coresight_fixup_orphan_conns(struct
>> coresight_device *csdev)
>>                csdev, coresight_orphan_match);
>>   }
>>   +/*
>> + * Device connections are discovered before one/both devices have
>> been created,
>> + * so inputs must be added later.
>> + */
>> +static int coresight_fixup_inputs(struct coresight_device *csdev)
>> +{
>> +    int i, ret = 0;
>> +    struct coresight_connection *out_conn;
>> +    struct coresight_connection in_conn;
>> +
>> +    for (i = 0; i < csdev->pdata->nr_outconns; i++) {
>> +        out_conn = &csdev->pdata->out_conns[i];
>> +        if (!out_conn->remote_dev || !out_conn->remote_dev->pdata)
>> +            continue;
> 
> Hi James,
> 
> If out_conn->remote_dev is null here,  the in_conn of
> out_conn->remote_dev->pdata will never be set.
> For example, device A is connected to in_port 0 of device B. If device A
> is probed first, the in_conn of device
> B will not be set.
> Do we need to add Defer probe return here ? I tested with defer probe
> return, it works.
> 
>         for (i = 0; i < csdev->pdata->nr_outconns; i++) {
>                 out_conn = &csdev->pdata->out_conns[i];
>                 if (!out_conn->remote_dev || !out_conn->remote_dev->pdata)
> -                       continue;
> +                        return -EPROBE_DEFER;
> 
> Thanks
> Jinlong Mao

I think you are right but I thought that EPROBE_DEFER was too big of a
change and that it might break something in some unexpected way.

In V3 I used the orphan mechanism for inputs in the same way as outputs
so the problem should be gone now but without having to defer loading.