| 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
| ||
|
Date: Fri, 30 Sep 2022 12:35:27 +0530
From: Souradeep Chowdhury <quic_schowdhu@...cinc.com>
To: Krzysztof Kozlowski <krzk@...nel.org>,
Andy Gross <agross@...nel.org>,
Bjorn Andersson <andersson@...nel.org>,
Rob Herring <robh+dt@...nel.org>, Alex Elder <elder@...e.org>
CC: <linux-arm-kernel@...ts.infradead.org>,
<linux-kernel@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>,
<devicetree@...r.kernel.org>,
"Sai Prakash Ranjan" <quic_saipraka@...cinc.com>,
Sibi Sankar <quic_sibis@...cinc.com>,
Rajendra Nayak <quic_rjendra@...cinc.com>, <vkoul@...nel.org>
Subject: Re: [PATCH V14 2/7] soc: qcom: dcc: Add driver support for Data
Capture and Compare unit(DCC)
On 9/28/2022 11:05 PM, Krzysztof Kozlowski wrote:
> On 28/09/2022 19:11, Souradeep Chowdhury wrote:
>> The DCC is a DMA Engine designed to capture and store data
>> during system crash or software triggers. The DCC operates
>> based on user inputs via the debugfs interface. The user gives
>> addresses as inputs and these addresses are stored in the
>> dcc sram. In case of a system crash or a manual software
>> trigger by the user through the debugfs interface,
>> the dcc captures and stores the values at these addresses.
>> This patch contains the driver which has all the methods
>> pertaining to the debugfs interface, auxiliary functions to
>> support all the four fundamental operations of dcc namely
>> read, write, read/modify/write and loop. The probe method
>> here instantiates all the resources necessary for dcc to
>> operate mainly the dedicated dcc sram where it stores the
>> values. The DCC driver can be used for debugging purposes
>> without going for a reboot since it can perform software
>> triggers as well based on user inputs.
>>
>> Also added the documentation for debugfs entries and explained
>> the functionalities of each debugfs file that has been created
>> for dcc.
>>
>> The following is the justification of using debugfs interface
>> over the other alternatives like sysfs/ioctls
>>
>> i) As can be seen from the debugfs attribute descriptions,
>> some of the debugfs attribute files here contains multiple
>> arguments which needs to be accepted from the user. This goes
>> against the design style of sysfs.
>>
>> ii) The user input patterns have been made simple and convenient
>> in this case with the use of debugfs interface as user doesn't
>> need to shuffle between different files to execute one instruction
>> as was the case on using other alternatives.
>>
>> Signed-off-by: Souradeep Chowdhury <quic_schowdhu@...cinc.com>
>> ---
>> Documentation/ABI/testing/debugfs-driver-dcc | 98 ++
>> drivers/soc/qcom/Kconfig | 8 +
>> drivers/soc/qcom/Makefile | 1 +
>> drivers/soc/qcom/dcc.c | 1352 ++++++++++++++++++++++++++
>> 4 files changed, 1459 insertions(+)
>> create mode 100644 Documentation/ABI/testing/debugfs-driver-dcc
>> create mode 100644 drivers/soc/qcom/dcc.c
>>
>> diff --git a/Documentation/ABI/testing/debugfs-driver-dcc b/Documentation/ABI/testing/debugfs-driver-dcc
>> new file mode 100644
>> index 0000000..387f67e
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/debugfs-driver-dcc
>> @@ -0,0 +1,98 @@
>> +What: /sys/kernel/debug/dcc/.../ready
>> +Date: September 2022
>> +Contact: Souradeep Chowdhury <quic_schowdhu@...cinc.com>
>> +Description:
>> + This file is used to check the status of the dcc
>> + hardware if it's ready to take the inputs. A 'Y'
>> + here indicates dcc is in a ready condition.
>> + Example:
>> + cat /sys/kernel/debug/dcc/.../ready
>> +
>> +What: /sys/kernel/debug/dcc/.../trigger
>> +Date: September 2022
>> +Contact: Souradeep Chowdhury <quic_schowdhu@...cinc.com>
>> +Description:
>> + This is the debugfs interface for manual software
>> + triggers. The user can simply enter a 1 against
>> + the debugfs file and enable a manual trigger.
>> + Example:
>> + echo 1 > /sys/kernel/debug/dcc/.../trigger
>> +
>> +What: /sys/kernel/debug/dcc/.../config_reset
>> +Date: September 2022
>> +Contact: Souradeep Chowdhury <quic_schowdhu@...cinc.com>
>> +Description:
>> + This file is used to reset the configuration of
>> + a dcc driver to the default configuration. This
>> + means that all the previous addresses stored in
>> + the driver gets removed and user needs to enter
>> + the address values from the start.
>> + Example:
>> + echo 1 > /sys/kernel/debug/dcc/../config_reset
>> +
>> +What: /sys/kernel/debug/dcc/.../[list-number]/config
>> +Date: September 2022
>> +Contact: Souradeep Chowdhury <quic_schowdhu@...cinc.com>
>> +Description:
>> + This stores the addresses of the registers which
>> + should be read in case of a hardware crash or
>> + manual software triggers. The addresses entered here
>> + are considered under all the 4 types of dcc
>> + instructions Read type, Write type, Read Modify Write
>> + type and Loop type. The lists need to be configured
>> + sequentially and not in a overlapping manner. As an
>> + example user can jump to list x only after list y is
>> + configured and enabled. The format for entering all
>> + types of instructions are explained in examples as
>> + follows.
>> + Example:
>> + i)Read Type Instruction
>> + echo R <1> <2> <3> >/sys/kernel/debug/dcc/../[list-number]/config
>> + 1->Address to be considered for reading the value.
>> + 2->The word count of the addresses, read n words
>> + starting from address <1>. Each word is of 32 bits.
>> + If not entered 1 is considered.
>> + 3->Can be 'apb' or 'ahb' which indicates if it is apb or ahb
>> + bus respectively. If not entered ahb is considered.
>> + ii)Write Type Instruction
>> + echo W <1> <2> <3> > /sys/kernel/debug/dcc/../[list-number]/config
>> + 1->Address to be considered for writing the value.
>> + 2->The value that needs to be written at the location.
>> + 3->Can be a 'apb' or 'ahb' which indicates if it is apb or ahb
>> + but respectively.
>> + iii)Read Modify Write type instruction
>> + echo RW <1> <2> <3> > /sys/kernel/debug/dcc/../[list-number]/config
>> + 1->The address which needs to be considered for read then write.
>> + 2->The value that needs to be written on the address.
>> + 3->The mask of the value to be written.
>> + iv)Loop Type Instruction
>> + echo L <1> <2> <3> > /sys/kernel/debug/dcc/../[list-number]/config
>> + 1->The loop count, the number of times the value of the addresses will be
>> + captured.
>> + 2->The address count, total number of addresses to be entered in this
>> + instruction.
>> + 3->The series of addresses to be entered separated by a space like <addr1>
>> + <addr2>... and so on.
>> +
>> +What: /sys/kernel/debug/dcc/.../[list-number]/enable
>> +Date: September 2022
>> +Contact: Souradeep Chowdhury <quic_schowdhu@...cinc.com>
>> +Description:
>> + This debugfs interface is used for enabling the
>> + the dcc hardware. Enable file is kept under the
>> + directory list number for which the user wants
>> + to enable it. For example if the user wants to
>> + enable list 1, then he should go for
>> + echo 1 > /sys/kernel/debug/dcc/.../1/enable.
>> + On enabling the dcc, all the addresses entered
>> + by the user for the corresponding list is written
>> + into dcc sram which is read by the dcc hardware
>> + on manual or crash induced triggers. Lists should
>> + be enabled sequentially.For example after configuring
>> + addresses for list 1 and enabling it, a user can
>> + proceed to enable list 2 or vice versa.
>> + Example:
>> + echo 0 > /sys/kernel/debug/dcc/.../[list-number]/enable
>> + (disable dcc for the corresponding list number)
>> + echo 1 > /sys/kernel/debug/dcc/.../[list-number]/enable
>> + (enable dcc for the corresponding list number)
>> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
>> index 024e420..d5730bf 100644
>> --- a/drivers/soc/qcom/Kconfig
>> +++ b/drivers/soc/qcom/Kconfig
>> @@ -69,6 +69,14 @@ config QCOM_LLCC
>> SDM845. This provides interfaces to clients that use the LLCC.
>> Say yes here to enable LLCC slice driver.
>>
>> +config QCOM_DCC
>> + tristate "Qualcomm Technologies, Inc. Data Capture and Compare(DCC) engine driver"
>> + depends on ARCH_QCOM || COMPILE_TEST
>> + help
>> + This option enables driver for Data Capture and Compare engine. DCC
>> + driver provides interface to configure DCC block and read back
>> + captured data from DCC's internal SRAM.
>> +
>> config QCOM_KRYO_L2_ACCESSORS
>> bool
>> depends on ARCH_QCOM && ARM64 || COMPILE_TEST
>> diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
>> index d66604a..b1fe812 100644
>> --- a/drivers/soc/qcom/Makefile
>> +++ b/drivers/soc/qcom/Makefile
>> @@ -4,6 +4,7 @@ obj-$(CONFIG_QCOM_AOSS_QMP) += qcom_aoss.o
>> obj-$(CONFIG_QCOM_GENI_SE) += qcom-geni-se.o
>> obj-$(CONFIG_QCOM_COMMAND_DB) += cmd-db.o
>> obj-$(CONFIG_QCOM_CPR) += cpr.o
>> +obj-$(CONFIG_QCOM_DCC) += dcc.o
>> obj-$(CONFIG_QCOM_GSBI) += qcom_gsbi.o
>> obj-$(CONFIG_QCOM_MDT_LOADER) += mdt_loader.o
>> obj-$(CONFIG_QCOM_OCMEM) += ocmem.o
>> diff --git a/drivers/soc/qcom/dcc.c b/drivers/soc/qcom/dcc.c
>> new file mode 100644
>> index 0000000..e16c074
>> --- /dev/null
>> +++ b/drivers/soc/qcom/dcc.c
>> @@ -0,0 +1,1352 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) 2015-2021, The Linux Foundation. All rights reserved.
>> + * Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights reserved.
>> + */
>> +
>> +#include <linux/bitfield.h>
>> +#include <linux/bitops.h>
>> +#include <linux/debugfs.h>
>> +#include <linux/delay.h>
>> +#include <linux/fs.h>
>> +#include <linux/io.h>
>> +#include <linux/iopoll.h>
>> +#include <linux/miscdevice.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/slab.h>
>> +#include <linux/uaccess.h>
>> +
>> +#define STATUS_READY_TIMEOUT 5000 /*microseconds*/
>> +
>> +#define DCC_SRAM_NODE "dcc_sram"
>> +
>> +/* DCC registers */
>> +#define DCC_HW_INFO 0x04
>> +#define DCC_LL_NUM_INFO 0x10
>> +#define DCC_STATUS(vers) ((vers) == 1 ? 0x0c : 0x1c)
>> +#define DCC_LL_LOCK 0x00
>> +#define DCC_LL_CFG 0x04
>> +#define DCC_LL_BASE 0x08
>> +#define DCC_FD_BASE 0x0c
>> +#define DCC_LL_TIMEOUT 0x10
>> +#define DCC_LL_INT_ENABLE 0x18
>> +#define DCC_LL_INT_STATUS 0x1c
>> +#define DCC_LL_SW_TRIGGER 0x2c
>> +#define DCC_LL_BUS_ACCESS_STATUS 0x30
>> +
>> +#define DCC_MAP_LEVEL1 0x18
>> +#define DCC_MAP_LEVEL2 0x34
>> +#define DCC_MAP_LEVEL3 0x4C
>> +
>> +#define DCC_MAP_OFFSET1 0x10
>> +#define DCC_MAP_OFFSET2 0x18
>> +#define DCC_MAP_OFFSET3 0x1C
>> +#define DCC_MAP_OFFSET4 0x8
>> +
>> +/*Default value used if a bit 6 in the HW_INFO register is set.*/
>> +#define DCC_FIX_LOOP_OFFSET 16
>> +
>> +/*Mask to find version info from HW_Info register*/
>> +#define DCC_VER_INFO_MASK BIT(9)
>> +
>> +#define DCC_READ 0
>> +#define DCC_WRITE 1
>> +#define DCC_LOOP 2
>> +#define DCC_READ_WRITE 3
>> +
>> +#define MAX_DCC_OFFSET GENMASK(9, 2)
>> +#define MAX_DCC_LEN GENMASK(6, 0)
>> +#define MAX_LOOP_CNT GENMASK(7, 0)
>> +#define MAX_LOOP_ADDR 10
>> +
>> +#define DCC_ADDR_DESCRIPTOR 0x00
>> +#define DCC_ADDR_LIMIT 27
>> +#define DCC_ADDR_OFF_RANGE 8
>> +#define DCC_ADDR_RANGE_MASK GENMASK(31, 4)
>> +#define DCC_LOOP_DESCRIPTOR BIT(30)
>> +#define DCC_RD_MOD_WR_DESCRIPTOR BIT(31)
>> +#define DCC_LINK_DESCRIPTOR GENMASK(31, 30)
>> +#define DCC_STATUS_MASK GENMASK(1, 0)
>> +#define DCC_LOCK_MASK BIT(0)
>> +#define DCC_LOOP_OFFSET_MASK BIT(6)
>> +#define DCC_TRIGGER_MASK BIT(9)
>> +
>> +#define DCC_WRITE_MASK BIT(15)
>> +#define DCC_WRITE_OFF_MASK GENMASK(7, 0)
>> +#define DCC_WRITE_LEN_MASK GENMASK(14, 8)
>> +
>> +#define DCC_READ_IND 0x00
>> +#define DCC_WRITE_IND (BIT(28))
>> +
>> +#define DCC_AHB_IND 0x00
>> +#define DCC_APB_IND BIT(29)
>> +
>> +#define DCC_MAX_LINK_LIST 8
>> +#define DCC_INVALID_LINK_LIST GENMASK(7, 0)
>> +
>> +#define DCC_VER_MASK1 GENMASK(6, 0)
>> +#define DCC_VER_MASK2 GENMASK(5, 0)
>> +
>> +#define DCC_SRAM_WORD_LENGTH 4
>> +
>> +#define DCC_RD_MOD_WR_ADDR 0xC105E
>> +
>> +/*DCC debugfs directory*/
>> +static struct dentry *dcc_dbg;
>> +
>> +enum dcc_descriptor_type {
>> + DCC_READ_TYPE,
>> + DCC_LOOP_TYPE,
>> + DCC_READ_WRITE_TYPE,
>> + DCC_WRITE_TYPE
>> +};
>> +
>> +struct dcc_config_entry {
>> + u32 base;
>> + u32 offset;
>> + u32 len;
>> + u32 loop_cnt;
>> + u32 write_val;
>> + u32 mask;
>> + bool apb_bus;
>> + enum dcc_descriptor_type desc_type;
>> + struct list_head list;
>> +};
>> +
>> +/**
>> + * struct dcc_drvdata - configuration information related to a dcc device
>> + * @base: Base Address of the dcc device
>> + * @dev: The device attached to the driver data
>> + * @mutex: Lock to protect access and manipulation of dcc_drvdata
>> + * @ram_base: Base address for the SRAM dedicated for the dcc device
>> + * @ram_size: Total size of the SRAM dedicated for the dcc device
>> + * @ram_offset: Offset to the SRAM dedicated for dcc device
>> + * @mem_map_ver: Memory map version of DCC hardware
>> + * @ram_cfg: Used for address limit calculation for dcc
>> + * @ram_start: Starting address of DCC SRAM
>> + * @sram_dev: Miscellaneous device equivalent of dcc SRAM
>> + * @cfg_head: Points to the head of the linked list of addresses
> So where is the list item? Is it dcc_config_entry?
Yes that's right.
>
>> + * @dbg_dir: The dcc debugfs directory under which all the debugfs files are placed
>> + * @nr_link_list: Total number of linkedlists supported by the DCC configuration
>> + * @loopoff: Loop offset bits range for the addresses
>> + * @enable; This contains an array of linkedlist enable flags
> Typo: ;
>
> Compile it with W=1 and fix all the warnings.
>
> The description of this field still looks wrong. It does not have array
> of linked lists. Linked list is list_head, this is a bool.
Ack
>
> Best regards,
> Krzysztof
>
Powered by blists - more mailing lists