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: <148b684e-b32f-4159-8163-5e360bc36ed3@quicinc.com>
Date: Thu, 10 Apr 2025 13:44:14 +0800
From: songchai <quic_songchai@...cinc.com>
To: Mike Leach <mike.leach@...aro.org>
CC: Suzuki K Poulose <suzuki.poulose@....com>,
        James Clark
	<james.clark@....com>,
        Alexander Shishkin
	<alexander.shishkin@...ux.intel.com>,
        Andy Gross <agross@...nel.org>,
        Bjorn
 Andersson <andersson@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof
 Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>, <linux-kernel@...r.kernel.org>,
        <coresight@...ts.linaro.org>, <linux-arm-kernel@...ts.infradead.org>,
        <linux-arm-msm@...r.kernel.org>, <devicetree@...r.kernel.org>
Subject: Re: [PATCH v3 3/7] coresight-tgu: Add signal priority support


On 3/7/2025 8:17 PM, Mike Leach wrote:
> Hi
>
> On Thu, 27 Feb 2025 at 09:27, songchai <quic_songchai@...cinc.com> wrote:
>> From: Songwei Chai <quic_songchai@...cinc.com>
>>
>> Like circuit of a Logic analyzer, in TGU, the requirement could be
>> configured in each step and the trigger will be created once the
>> requirements are met. Add priority functionality here to sort the
>> signals into different priorities. The signal which is wanted could
>> be configured in each step's priority node, the larger number means
>> the higher priority and the signal with higher priority will be sensed
>> more preferentially.
>>
>> Signed-off-by: Songwei Chai <quic_songchai@...cinc.com>
>> Signed-off-by: songchai <quic_songchai@...cinc.com>
>> ---
>>   .../testing/sysfs-bus-coresight-devices-tgu   |   7 +
>>   drivers/hwtracing/coresight/coresight-tgu.c   | 139 ++++++++++++++++++
>>   drivers/hwtracing/coresight/coresight-tgu.h   | 110 ++++++++++++++
>>   3 files changed, 256 insertions(+)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tgu b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tgu
>> index 741bc9fd9df5..af7332153833 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tgu
>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tgu
>> @@ -7,3 +7,10 @@ Description:
>>                  Accepts only one of the 2 values -  0 or 1.
>>                  0 : disable TGU.
>>                  1 : enable TGU.
>> +
>> +What:           /sys/bus/coresight/devices/<tgu-name>/step[0:7]_priority[0:3]/reg[0:17]
>> +Date:           February 2025
>> +KernelVersion   6.15
>> +Contact:        Jinlong Mao (QUIC) <quic_jinlmao@...cinc.com>, Sam Chai (QUIC) <quic_songchai@...cinc.com>
>> +Description:
>> +                (RW) Set/Get the sensed siganal with specific step and priority for TGU.
> sp/siganal/signal
Done.
>
>> diff --git a/drivers/hwtracing/coresight/coresight-tgu.c b/drivers/hwtracing/coresight/coresight-tgu.c
>> index da4c04ac1097..f28761619ebe 100644
>> --- a/drivers/hwtracing/coresight/coresight-tgu.c
>> +++ b/drivers/hwtracing/coresight/coresight-tgu.c
>> @@ -17,9 +17,92 @@
>>
>>   DEFINE_CORESIGHT_DEVLIST(tgu_devs, "tgu");
>>
>> +static int calculate_array_location(struct tgu_drvdata *drvdata, int step_index,
>> +                         int operation_index, int reg_index)
>> +{
>> +       int ret = -EINVAL;
>> +
>> +       ret = operation_index * (drvdata->max_step) *
>> +                     (drvdata->max_reg) + step_index * (drvdata->max_reg)
>> +                               + reg_index;
>> +
>> +       return ret;
>> +}
>> +
>> +static ssize_t tgu_dataset_show(struct device *dev,
>> +                               struct device_attribute *attr, char *buf)
>> +{
>> +       struct tgu_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +       struct tgu_attribute *tgu_attr =
>> +               container_of(attr, struct tgu_attribute, attr);
>> +
>> +       return sysfs_emit(buf, "0x%x\n",
>> +                         drvdata->value_table->priority[calculate_array_location(
>> +                                 drvdata, tgu_attr->step_index,
>> +                                 tgu_attr->operation_index, tgu_attr->reg_num)]);
>> +
>> +}
>> +
>> +static ssize_t tgu_dataset_store(struct device *dev,
>> +                                struct device_attribute *attr, const char *buf,
>> +                                size_t size)
>> +{
>> +       unsigned long val;
>> +       ssize_t ret = -EINVAL;
>> +
>> +       struct tgu_drvdata *tgu_drvdata = dev_get_drvdata(dev->parent);
>> +       struct tgu_attribute *tgu_attr =
>> +               container_of(attr, struct tgu_attribute, attr);
>> +
>> +       if (kstrtoul(buf, 0, &val))
>> +               return ret;
>> +
>> +       guard(spinlock)(&tgu_drvdata->spinlock);
>> +       tgu_drvdata->value_table->priority[calculate_array_location(
>> +               tgu_drvdata, tgu_attr->step_index, tgu_attr->operation_index,
>> +               tgu_attr->reg_num)] = val;
>> +       ret = size;
>> +
>> +       return ret;
> ret is unneeded - directly return either size or -EINVAL.
Done.
>
>> +}
>> +
>> +static umode_t tgu_node_visible(struct kobject *kobject, struct attribute *attr,
>> +                               int n)
>> +{
>> +       struct device *dev = kobj_to_dev(kobject);
>> +       struct tgu_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +       int ret = 0;
>> +
>> +       struct device_attribute *dev_attr =
>> +               container_of(attr, struct device_attribute, attr);
>> +       struct tgu_attribute *tgu_attr =
>> +               container_of(dev_attr, struct tgu_attribute, attr);
>> +
>> +       if (tgu_attr->step_index < drvdata->max_step) {
>> +               ret = (tgu_attr->reg_num < drvdata->max_reg) ?
>> +                                           attr->mode : 0;
>> +               return ret;
>> +       }
>> +       return SYSFS_GROUP_INVISIBLE;
>> +}
> default ret as SYSFS_GROUP_INVISIBLE, and returnret at end
Done.
>
>> +
>>   static void tgu_write_all_hw_regs(struct tgu_drvdata *drvdata)
>>   {
>> +       int i, j, k;
>> +
>>          CS_UNLOCK(drvdata->base);
>> +
>> +       for (i = 0; i < drvdata->max_step; i++) {
>> +               for (j = 0; j < MAX_PRIORITY; j++) {
>> +                       for (k = 0; k < drvdata->max_reg; k++) {
>> +                               tgu_writel(drvdata,
>> +                                          drvdata->value_table->priority
>> +                                                  [calculate_array_location(drvdata, i, j, k)],
>> +                                          PRIORITY_REG_STEP(i, j, k));
>> +                       }
>> +               }
>> +       }
>> +
>>          /* Enable TGU to program the triggers */
>>          tgu_writel(drvdata, 1, TGU_CONTROL);
>>          CS_LOCK(drvdata->base);
>> @@ -130,6 +213,38 @@ static const struct attribute_group tgu_common_grp = {
>>
>>   static const struct attribute_group *tgu_attr_groups[] = {
>>          &tgu_common_grp,
>> +       PRIORITY_ATTRIBUTE_GROUP_INIT(0, 0),
>> +       PRIORITY_ATTRIBUTE_GROUP_INIT(0, 1),
>> +       PRIORITY_ATTRIBUTE_GROUP_INIT(0, 2),
>> +       PRIORITY_ATTRIBUTE_GROUP_INIT(0, 3),
>> +       PRIORITY_ATTRIBUTE_GROUP_INIT(1, 0),
>> +       PRIORITY_ATTRIBUTE_GROUP_INIT(1, 1),
>> +       PRIORITY_ATTRIBUTE_GROUP_INIT(1, 2),
>> +       PRIORITY_ATTRIBUTE_GROUP_INIT(1, 3),
>> +       PRIORITY_ATTRIBUTE_GROUP_INIT(2, 0),
>> +       PRIORITY_ATTRIBUTE_GROUP_INIT(2, 1),
>> +       PRIORITY_ATTRIBUTE_GROUP_INIT(2, 2),
>> +       PRIORITY_ATTRIBUTE_GROUP_INIT(2, 3),
>> +       PRIORITY_ATTRIBUTE_GROUP_INIT(3, 0),
>> +       PRIORITY_ATTRIBUTE_GROUP_INIT(3, 1),
>> +       PRIORITY_ATTRIBUTE_GROUP_INIT(3, 2),
>> +       PRIORITY_ATTRIBUTE_GROUP_INIT(3, 3),
>> +       PRIORITY_ATTRIBUTE_GROUP_INIT(4, 0),
>> +       PRIORITY_ATTRIBUTE_GROUP_INIT(4, 1),
>> +       PRIORITY_ATTRIBUTE_GROUP_INIT(4, 2),
>> +       PRIORITY_ATTRIBUTE_GROUP_INIT(4, 3),
>> +       PRIORITY_ATTRIBUTE_GROUP_INIT(5, 0),
>> +       PRIORITY_ATTRIBUTE_GROUP_INIT(5, 1),
>> +       PRIORITY_ATTRIBUTE_GROUP_INIT(5, 2),
>> +       PRIORITY_ATTRIBUTE_GROUP_INIT(5, 3),
>> +       PRIORITY_ATTRIBUTE_GROUP_INIT(6, 0),
>> +       PRIORITY_ATTRIBUTE_GROUP_INIT(6, 1),
>> +       PRIORITY_ATTRIBUTE_GROUP_INIT(6, 2),
>> +       PRIORITY_ATTRIBUTE_GROUP_INIT(6, 3),
>> +       PRIORITY_ATTRIBUTE_GROUP_INIT(7, 0),
>> +       PRIORITY_ATTRIBUTE_GROUP_INIT(7, 1),
>> +       PRIORITY_ATTRIBUTE_GROUP_INIT(7, 2),
>> +       PRIORITY_ATTRIBUTE_GROUP_INIT(7, 3),
>>          NULL,
>>   };
>>
>> @@ -164,6 +279,30 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id)
>>
>>          spin_lock_init(&drvdata->spinlock);
>>
>> +       ret = of_property_read_u32(adev->dev.of_node, "tgu-regs",
>> +                                  &drvdata->max_reg);
>> +       if (ret)
>> +               return -EINVAL;
>> +
>> +       ret = of_property_read_u32(adev->dev.of_node, "tgu-steps",
>> +                                  &drvdata->max_step);
>> +       if (ret)
>> +               return -EINVAL;
>> +
>> +       drvdata->value_table =
>> +               devm_kzalloc(dev, sizeof(*drvdata->value_table), GFP_KERNEL);
>> +       if (!drvdata->value_table)
>> +               return -ENOMEM;
>> +
>> +       drvdata->value_table->priority = devm_kzalloc(
>> +               dev,
>> +               MAX_PRIORITY * drvdata->max_reg * drvdata->max_step *
>> +                       sizeof(*(drvdata->value_table->priority)),
>> +               GFP_KERNEL);
>> +
>> +       if (!drvdata->value_table->priority)
>> +               return -ENOMEM;
>> +
>>          drvdata->enable = false;
>>          desc.type = CORESIGHT_DEV_TYPE_HELPER;
>>          desc.pdata = adev->dev.platform_data;
>> diff --git a/drivers/hwtracing/coresight/coresight-tgu.h b/drivers/hwtracing/coresight/coresight-tgu.h
>> index 380686f94130..6e5d465117df 100644
>> --- a/drivers/hwtracing/coresight/coresight-tgu.h
>> +++ b/drivers/hwtracing/coresight/coresight-tgu.h
>> @@ -13,6 +13,110 @@
>>   #define tgu_writel(drvdata, val, off) __raw_writel((val), drvdata->base + off)
>>   #define tgu_readl(drvdata, off) __raw_readl(drvdata->base + off)
>>
>> +/*
>> + *  TGU configuration space                              Step configuration
>> + *  offset table                                         space layout
>> + * x-------------------------x$                          x-------------x$
>> + * |                         |$                          |             |$
>> + * |                         |                           |   reserve   |$
>> + * |                         |                           |             |$
>> + * |coresight management     |                           |-------------|base+n*0x1D8+0x1F4$
>> + * |     registe             |                     |---> |prioroty[3]  |$
>> + * |                         |                     |     |-------------|base+n*0x1D8+0x194$
>> + * |                         |                     |     |prioroty[2]  |$
>> + * |-------------------------|                     |     |-------------|base+n*0x1D8+0x134$
>> + * |                         |                     |     |prioroty[1]  |$
>> + * |         step[7]         |                     |     |-------------|base+n*0x1D8+0xD4$
>> + * |-------------------------|->base+0x40+7*0x1D8  |     |prioroty[0]  |$
>> + * |                         |                     |     |-------------|base+n*0x1D8+0x74$
>> + * |         ...             |                     |     |  condition  |$
>> + * |                         |                     |     |   select    |$
>> + * |-------------------------|->base+0x40+1*0x1D8  |     |-------------|base+n*0x1D8+0x60$
>> + * |                         |                     |     |  condition  |$
>> + * |         step[0]         |-------------------->      |   decode    |$
>> + * |-------------------------|-> base+0x40               |-------------|base+n*0x1D8+0x50$
>> + * |                         |                           |             |$
>> + * | Control and status space|                           |Timer/Counter|$
>> + * |        space            |                           |             |$
>> + * x-------------------------x->base                     x-------------x base+n*0x1D8+0x40$
>> + *
>> + */
>> +
>> +/* Calculate compare step addresses */
>> +#define PRIORITY_REG_STEP(step, priority, reg)\
>> +       (0x0074 + 0x60 * priority + 0x4 * reg + 0x1D8 * step)
>> +
> use #defines + explanation instead of arbitrary magic numbers
Done.
>
>> +#define tgu_dataset_ro(name, step_index, type, reg_num)     \
>> +       (&((struct tgu_attribute[]){ {                      \
>> +               __ATTR(name, 0444, tgu_dataset_show, NULL), \
>> +               step_index,                                 \
>> +               type,                                       \
>> +               reg_num,                                    \
>> +       } })[0].attr.attr)
>> +
>
> This define unused in this patch, Drop till it is used.
Done.
>
>> +#define tgu_dataset_rw(name, step_index, type, reg_num)                  \
>> +       (&((struct tgu_attribute[]){ {                                   \
>> +               __ATTR(name, 0644, tgu_dataset_show, tgu_dataset_store), \
>> +               step_index,                                              \
>> +               type,                                                    \
>> +               reg_num,                                                 \
>> +       } })[0].attr.attr)
>> +
>> +#define STEP_PRIORITY(step_index, reg_num, priority)                     \
>> +       tgu_dataset_rw(reg##reg_num, step_index, TGU_PRIORITY##priority, \
>> +                      reg_num)
>> +
>> +#define STEP_PRIORITY_LIST(step_index, priority)  \
>> +       {STEP_PRIORITY(step_index, 0, priority),  \
>> +        STEP_PRIORITY(step_index, 1, priority),  \
>> +        STEP_PRIORITY(step_index, 2, priority),  \
>> +        STEP_PRIORITY(step_index, 3, priority),  \
>> +        STEP_PRIORITY(step_index, 4, priority),  \
>> +        STEP_PRIORITY(step_index, 5, priority),  \
>> +        STEP_PRIORITY(step_index, 6, priority),  \
>> +        STEP_PRIORITY(step_index, 7, priority),  \
>> +        STEP_PRIORITY(step_index, 8, priority),  \
>> +        STEP_PRIORITY(step_index, 9, priority),  \
>> +        STEP_PRIORITY(step_index, 10, priority), \
>> +        STEP_PRIORITY(step_index, 11, priority), \
>> +        STEP_PRIORITY(step_index, 12, priority), \
>> +        STEP_PRIORITY(step_index, 13, priority), \
>> +        STEP_PRIORITY(step_index, 14, priority), \
>> +        STEP_PRIORITY(step_index, 15, priority), \
>> +        STEP_PRIORITY(step_index, 16, priority), \
>> +        STEP_PRIORITY(step_index, 17, priority), \
>> +        NULL                   \
>> +       }
>> +
>> +#define PRIORITY_ATTRIBUTE_GROUP_INIT(step, priority)\
>> +       (&(const struct attribute_group){\
>> +               .attrs = (struct attribute*[])STEP_PRIORITY_LIST(step, priority),\
>> +               .is_visible = tgu_node_visible,\
>> +               .name = "step" #step "_priority" #priority \
>> +       })
>> +
>> +enum operation_index {
>> +       TGU_PRIORITY0,
>> +       TGU_PRIORITY1,
>> +       TGU_PRIORITY2,
>> +       TGU_PRIORITY3
>> +
>> +};
>> +
>> +/* Maximum priority that TGU supports */
>> +#define MAX_PRIORITY 4
>> +
>> +struct tgu_attribute {
>> +       struct device_attribute attr;
>> +       u32 step_index;
>> +       enum operation_index operation_index;
>> +       u32 reg_num;
>> +};
>> +
>> +struct value_table {
>> +       unsigned int *priority;
>> +};
>> +
>>   /**
>>    * struct tgu_drvdata - Data structure for a TGU (Trigger Generator Unit) device
>>    * @base: Memory-mapped base address of the TGU device
>> @@ -20,6 +124,9 @@
>>    * @csdev: Pointer to the associated coresight device
>>    * @spinlock: Spinlock for handling concurrent access
>>    * @enable: Flag indicating whether the TGU device is enabled
>> + * @value_table: Store given value based on relevant parameters.
>> + * @max_reg: Maximum number of registers
>> + * @max_step: Maximum step size
>>    *
>>    * This structure defines the data associated with a TGU device, including its base
>>    * address, device pointers, clock, spinlock for synchronization, trigger data pointers,
>> @@ -31,6 +138,9 @@ struct tgu_drvdata {
>>          struct coresight_device *csdev;
>>          spinlock_t spinlock;
>>          bool enable;
>> +       struct value_table *value_table;
>> +       int max_reg;
>> +       int max_step;
>>   };
>>
>>   #endif
> Regards
>
> Mike
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ