| 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
| ||
|
Message-ID: <df010fb9-44f7-44f4-b242-f7f06eb0adcb@arm.com>
Date: Thu, 19 Dec 2024 16:40:46 +0000
From: Suzuki K Poulose <suzuki.poulose@....com>
To: Mike Leach <mike.leach@...aro.org>
Cc: Mike Leach <Mike.Leach@....com>, Mao Jinlong <quic_jinlmao@...cinc.com>,
James Clark <James.Clark@....com>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Mathieu Poirier <mathieu.poirier@...aro.org>,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konradybcio@...nel.org>,
"coresight@...ts.linaro.org" <coresight@...ts.linaro.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>
Subject: Re: [PATCH v6 2/2] coresight: Add label sysfs node support
On 19/12/2024 11:17, Mike Leach wrote:
> Hi,
>
> On Wed, 18 Dec 2024 at 18:16, Suzuki K Poulose <suzuki.poulose@....com> wrote:
>>
>> Hi Mike
>>
>> On 18/12/2024 09:56, Mike Leach wrote:
>>> Hi
>>>
>>>> -----Original Message-----
>>>> From: Suzuki K Poulose <suzuki.poulose@....com>
>>>> Sent: Wednesday, December 18, 2024 9:38 AM
>>>> To: Mao Jinlong <quic_jinlmao@...cinc.com>; Mike Leach
>>>> <mike.leach@...aro.org>; James Clark <James.Clark@....com>; Rob Herring
>>>> <robh@...nel.org>; Krzysztof Kozlowski <krzk+dt@...nel.org>; Conor Dooley
>>>> <conor+dt@...nel.org>; Mathieu Poirier <mathieu.poirier@...aro.org>; Bjorn
>>>> Andersson <andersson@...nel.org>; Konrad Dybcio
>>>> <konradybcio@...nel.org>
>>>> Cc: coresight@...ts.linaro.org; linux-arm-kernel@...ts.infradead.org;
>>>> devicetree@...r.kernel.org; linux-kernel@...r.kernel.org; linux-arm-
>>>> msm@...r.kernel.org
>>>> Subject: Re: [PATCH v6 2/2] coresight: Add label sysfs node support
>>>>
>>>> On 17/12/2024 06:33, Mao Jinlong wrote:
>>>>> For some coresight components like CTI and TPDM, there could be
>>>>> numerous of them. From the node name, we can only get the type and
>>>>> register address of the component. We can't identify the HW or the
>>>>> system the component belongs to. Add label sysfs node support for
>>>>> showing the intuitive name of the device.
>>>>>
>>>>> Signed-off-by: Mao Jinlong <quic_jinlmao@...cinc.com>
>>>>> ---
>>>>> .../testing/sysfs-bus-coresight-devices-cti | 6 ++++
>>>>> .../sysfs-bus-coresight-devices-funnel | 6 ++++
>>>>> .../testing/sysfs-bus-coresight-devices-tpdm | 6 ++++
>>>>> drivers/hwtracing/coresight/coresight-sysfs.c | 32
>>>> +++++++++++++++++++
>>>>> 4 files changed, 50 insertions(+)
>>>>
>>>> Do you think we need to name the devices using the label ?
>>>>
>>>
>>> No - absolutely not. If we use label to name devices then we have to validate that the string is a correctly formed device name and that it has not been previously used.
>>
>> Anything that doesn't contain '/' can be a device name ? And it is very
>> easy to find if the device name has been used in the coresight bus,
>> after all these devices only appear there.
>>
>> It is as good as :
>>
>> bus_find_device_by_name(coresight_bus_type, NULL, name) == NULL
>>
>> Of course with coresight_mutex() held.
>>
>> Suzuki
>>
>
> DTS label property (DT spec 4.1.2) is a freeform string, specified to
> be a human readable description of the device, e.g. :-
>
> cti0@...000 {
> reg = <0x1000>;
> label = "main system CTI";
> };
>
> Arguably the label is completely unnecessary - as the @0x1000 tells
> the knowledgeable user, with a hardware specification of the device
> precisely what this CTI does and is related to.
>
> The point of this patchset is to add context to the name@...r to make
> the identification of the devices easier.
>
> The DT compiler should ensure that the device tree is well formed.
> Using driver selected names (cti_cpu0 ... etc) guarantees that every
> device found in the DT has a unique representation in sysfs.
>
> Once a freeform string is used, then not only are duplicates possible,
> illegal device names are possible, all of which can result in missing
> nodes or worse. This requires handling / complications that are
> unnecessary for the purpose.
>
> Yes of course it is easy to look for duplicate names, reject bad ones,
> emit errors - but that could end up with a partially working system
> with missing components.
>
> Why add potential for breakage when it is not necessary?
Fair enough, we can do away with exporting the label.
Cheers
Suzuki
>
> Regards
>
> Mike
>
>>
>>>
>>> Using the canonical driver selected names works best as we are guaranteed a unique name and the information label can be used to provide flexible context information that best matches the users requirements.
>>>
>>> Mike
>>>
>>>> Or is this enough ?
>>>
>>>> Suzuki
>>>>
>>>>
>>>>>
>>>>> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-cti
>>>>> b/Documentation/ABI/testing/sysfs-bus-coresight-devices-cti
>>>>> index bf2869c413e7..909670e0451a 100644
>>>>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-cti
>>>>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-cti
>>>>> @@ -239,3 +239,9 @@ Date: March 2020
>>>>> KernelVersion 5.7
>>>>> Contact: Mike Leach or Mathieu Poirier
>>>>> Description: (Write) Clear all channel / trigger programming.
>>>>> +
>>>>> +What: /sys/bus/coresight/devices/<cti-name>/label
>>>>> +Date: Dec 2024
>>>>> +KernelVersion 6.14
>>>>> +Contact: Mao Jinlong <quic_jinlmao@...cinc.com>
>>>>> +Description: (Read) Show hardware context information of device.
>>>>> diff --git
>>>>> a/Documentation/ABI/testing/sysfs-bus-coresight-devices-funnel
>>>>> b/Documentation/ABI/testing/sysfs-bus-coresight-devices-funnel
>>>>> index d75acda5e1b3..944aad879aeb 100644
>>>>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-funnel
>>>>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-funnel
>>>>> @@ -10,3 +10,9 @@ Date: November 2014
>>>>> KernelVersion: 3.19
>>>>> Contact: Mathieu Poirier <mathieu.poirier@...aro.org>
>>>>> Description: (RW) Defines input port priority order.
>>>>> +
>>>>> +What: /sys/bus/coresight/devices/<memory_map>.funnel/label
>>>>> +Date: Dec 2024
>>>>> +KernelVersion 6.14
>>>>> +Contact: Mao Jinlong <quic_jinlmao@...cinc.com>
>>>>> +Description: (Read) Show hardware context information of device.
>>>>> diff --git
>>>>> a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>>> b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>>> index bf710ea6e0ef..309802246398 100644
>>>>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>>> @@ -257,3 +257,9 @@ Contact: Jinlong Mao (QUIC)
>>>> <quic_jinlmao@...cinc.com>, Tao Zhang (QUIC) <quic_t
>>>>> Description:
>>>>> (RW) Set/Get the MSR(mux select register) for the CMB
>>>> subunit
>>>>> TPDM.
>>>>> +
>>>>> +What: /sys/bus/coresight/devices/<tpdm-name>/label
>>>>> +Date: Dec 2024
>>>>> +KernelVersion 6.14
>>>>> +Contact: Mao Jinlong <quic_jinlmao@...cinc.com>
>>>>> +Description: (Read) Show hardware context information of device.
>>>>> diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c
>>>>> b/drivers/hwtracing/coresight/coresight-sysfs.c
>>>>> index a01c9e54e2ed..4af40cd7d75a 100644
>>>>> --- a/drivers/hwtracing/coresight/coresight-sysfs.c
>>>>> +++ b/drivers/hwtracing/coresight/coresight-sysfs.c
>>>>> @@ -7,6 +7,7 @@
>>>>> #include <linux/device.h>
>>>>> #include <linux/idr.h>
>>>>> #include <linux/kernel.h>
>>>>> +#include <linux/property.h>
>>>>>
>>>>> #include "coresight-priv.h"
>>>>> #include "coresight-trace-id.h"
>>>>> @@ -366,18 +367,47 @@ static ssize_t enable_source_store(struct device
>>>> *dev,
>>>>> }
>>>>> static DEVICE_ATTR_RW(enable_source);
>>>>>
>>>>> +static ssize_t label_show(struct device *dev,
>>>>> + struct device_attribute *attr, char *buf) {
>>>>> +
>>>>> + const char *str;
>>>>> + int ret = 0;
>>>>> +
>>>>> + ret = fwnode_property_read_string(dev_fwnode(dev), "label", &str);
>>>>> + if (ret == 0)
>>>>> + return scnprintf(buf, PAGE_SIZE, "%s\n", str);
>>>>> + else
>>>>> + return ret;
>>>>> +}
>>>>> +static DEVICE_ATTR_RO(label);
>>>>> +
>>>>> static struct attribute *coresight_sink_attrs[] = {
>>>>> &dev_attr_enable_sink.attr,
>>>>> + &dev_attr_label.attr,
>>>>> NULL,
>>>>> };
>>>>> ATTRIBUTE_GROUPS(coresight_sink);
>>>>>
>>>>> static struct attribute *coresight_source_attrs[] = {
>>>>> &dev_attr_enable_source.attr,
>>>>> + &dev_attr_label.attr,
>>>>> NULL,
>>>>> };
>>>>> ATTRIBUTE_GROUPS(coresight_source);
>>>>>
>>>>> +static struct attribute *coresight_link_attrs[] = {
>>>>> + &dev_attr_label.attr,
>>>>> + NULL,
>>>>> +};
>>>>> +ATTRIBUTE_GROUPS(coresight_link);
>>>>> +
>>>>> +static struct attribute *coresight_helper_attrs[] = {
>>>>> + &dev_attr_label.attr,
>>>>> + NULL,
>>>>> +};
>>>>> +ATTRIBUTE_GROUPS(coresight_helper);
>>>>> +
>>>>> const struct device_type coresight_dev_type[] = {
>>>>> [CORESIGHT_DEV_TYPE_SINK] = {
>>>>> .name = "sink",
>>>>> @@ -385,6 +415,7 @@ const struct device_type coresight_dev_type[] = {
>>>>> },
>>>>> [CORESIGHT_DEV_TYPE_LINK] = {
>>>>> .name = "link",
>>>>> + .groups = coresight_link_groups,
>>>>> },
>>>>> [CORESIGHT_DEV_TYPE_LINKSINK] = {
>>>>> .name = "linksink",
>>>>> @@ -396,6 +427,7 @@ const struct device_type coresight_dev_type[] = {
>>>>> },
>>>>> [CORESIGHT_DEV_TYPE_HELPER] = {
>>>>> .name = "helper",
>>>>> + .groups = coresight_helper_groups,
>>>>> }
>>>>> };
>>>>> /* Ensure the enum matches the names and groups */
>>>>
>>>> _______________________________________________
>>>> CoreSight mailing list -- coresight@...ts.linaro.org To unsubscribe send an
>>>> email to coresight-leave@...ts.linaro.org
>>
>
>
Powered by blists - more mailing lists