[v12,1/2] drivers/coresight: Add UltraSoc System Memory Buffer driver

Message ID 20221109135008.9485-2-hejunhao3@huawei.com
State New
Headers
Series Add support for UltraSoc System Memory Buffer |

Commit Message

hejunhao Nov. 9, 2022, 1:50 p.m. UTC
  From: Qi Liu <liuqi115@huawei.com>

This patch adds driver for UltraSoc SMB(System Memory Buffer)
device. SMB provides a way to buffer messages from ETM, and
store these "CPU instructions trace" in system memory.

SMB is developed by UltraSoc technology, which is acquired by
Siemens, and we still use "UltraSoc" to name driver.

Signed-off-by: Qi Liu <liuqi115@huawei.com>
Signed-off-by: Junhao He <hejunhao3@huawei.com>
Tested-by: JunHao He <hejunhao3@huawei.com>
---
 drivers/hwtracing/coresight/Kconfig        |  11 +
 drivers/hwtracing/coresight/Makefile       |   1 +
 drivers/hwtracing/coresight/ultrasoc-smb.c | 635 +++++++++++++++++++++
 drivers/hwtracing/coresight/ultrasoc-smb.h | 116 ++++
 4 files changed, 763 insertions(+)
 create mode 100644 drivers/hwtracing/coresight/ultrasoc-smb.c
 create mode 100644 drivers/hwtracing/coresight/ultrasoc-smb.h
  

Comments

Jonathan Cameron Nov. 9, 2022, 4:56 p.m. UTC | #1
On Wed, 9 Nov 2022 21:50:07 +0800
Junhao He <hejunhao3@huawei.com> wrote:

> From: Qi Liu <liuqi115@huawei.com>
> 
> This patch adds driver for UltraSoc SMB(System Memory Buffer)
> device. SMB provides a way to buffer messages from ETM, and
> store these "CPU instructions trace" in system memory.
> 
> SMB is developed by UltraSoc technology, which is acquired by
> Siemens, and we still use "UltraSoc" to name driver.
> 
> Signed-off-by: Qi Liu <liuqi115@huawei.com>
> Signed-off-by: Junhao He <hejunhao3@huawei.com>
> Tested-by: JunHao He <hejunhao3@huawei.com>
Hi JunHao,

One trivial side effect of dropping the ACPI dependency.

Also, I think (at the cost of a slightly lengthening of lines)
you can rename the register fields to avoid any potential
confusion between GLB and LB registers.

With those fixed feel free to add

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>



...

> diff --git a/drivers/hwtracing/coresight/ultrasoc-smb.c b/drivers/hwtracing/coresight/ultrasoc-smb.c
> new file mode 100644
> index 000000000000..ea2552a98d28
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/ultrasoc-smb.c

...

> +static const struct acpi_device_id ultrasoc_smb_acpi_match[] = {
> +	{"HISI03A1", 0},
> +	{}
> +};
> +MODULE_DEVICE_TABLE(acpi, ultrasoc_smb_acpi_match);
> +
> +static struct platform_driver smb_driver = {
> +	.driver = {
> +		.name = "ultrasoc-smb",
> +		.acpi_match_table = ACPI_PTR(ultrasoc_smb_acpi_match),
Now the driver build isn't dependent on CONFIG_ACPI
if !CONFIG_ACPI ACPI_PTR() doesn't reference the parameter.
As such you'll get unused warnings.

1 options to fix this
a) Drop ACPI_PTR() and just have .acpi_match_data = ultrasoc_smb_acpi_match
b) ifdef magic around the acpi_match table.

In theory the first option results in bloat, but in this case I doubt we care.

> +		.suppress_bind_attrs = true,
> +	},
> +	.probe = smb_probe,
> +	.remove = smb_remove,
> +};
> +module_platform_driver(smb_driver);
> +
> +MODULE_DESCRIPTION("UltraSoc SMB CoreSight driver");
> +MODULE_LICENSE("Dual MIT/GPL");
> +MODULE_AUTHOR("Jonathan Zhou <jonathan.zhouwen@huawei.com>");
> +MODULE_AUTHOR("Qi Liu <liuqi115@huawei.com>");
> diff --git a/drivers/hwtracing/coresight/ultrasoc-smb.h b/drivers/hwtracing/coresight/ultrasoc-smb.h
> new file mode 100644
> index 000000000000..2e2f9f8fe54b
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/ultrasoc-smb.h
> @@ -0,0 +1,116 @@
> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
> +/*
> + * Siemens System Memory Buffer driver.
> + * Copyright(c) 2022, HiSilicon Limited.
> + */
> +
> +#ifndef _ULTRASOC_SMB_H
> +#define _ULTRASOC_SMB_H
> +
> +#include <linux/miscdevice.h>
> +#include <linux/mutex.h>
> +
> +/* Offset of SMB global registers */
> +#define SMB_GLB_CFG_REG		0x00
> +#define SMB_GLB_EN_REG		0x04
> +#define SMB_GLB_INT_REG		0x08
> +
> +/* Offset of SMB logical buffer registers */
> +#define SMB_LB_CFG_LO_REG	0x40
> +#define SMB_LB_CFG_HI_REG	0x44
> +#define SMB_LB_INT_CTRL_REG	0x48
> +#define SMB_LB_INT_STS_REG	0x4c
> +#define SMB_LB_RD_ADDR_REG	0x5c
> +#define SMB_LB_WR_ADDR_REG	0x60
> +#define SMB_LB_PURGE_REG	0x64
> +
> +/* Set global config register */
> +#define SMB_CFG_BURST_LEN_MSK	GENMASK(11, 4)

Given there are several CFG registers, possibly worth
prefix of SMB_GLB_CFG_ ...

> +#define SMB_CFG_IDLE_PRD_MSK	GENMASK(15, 12)
> +#define SMB_CFG_MEM_WR_MSK	GENMASK(21, 16)
> +#define SMB_CFG_MEM_RD_MSK	GENMASK(27, 22)
> +#define SMB_GLB_CFG_DEFAULT	(FIELD_PREP(SMB_CFG_BURST_LEN_MSK, 0xf) | \
> +				 FIELD_PREP(SMB_CFG_IDLE_PRD_MSK, 0xf) | \
> +				 FIELD_PREP(SMB_CFG_MEM_WR_MSK, 0x3) | \
> +				 FIELD_PREP(SMB_CFG_MEM_RD_MSK, 0x1b))
> +
> +/* Set global interrupt control register */
> +#define SMB_INT_EN		BIT(0)

Again, multiple INT registers, so SMB_INT_GLB_* perhaps?

> +#define SMB_INT_PULSE		BIT(1) /* Interrupt type: 1 - Pulse */
> +#define SMB_INT_ACT_H		BIT(2) /* Interrupt polarity: 1 - Active high */
> +#define SMB_GLB_INT_CFG		(SMB_INT_EN | SMB_INT_PULSE | SMB_INT_ACT_H)
> +
> +/* Set logical buffer config register lower 32 bits */
> +#define SMB_CFG_LO_EN		BIT(0)

SMB_LB_CFG_...

etc for other cases.

> +#define SMB_CFG_LO_SINGLE_END	BIT(1)
> +#define SMB_CFG_LO_INIT		BIT(8)
> +#define SMB_CFG_LO_CONT		BIT(11)
> +#define SMB_CFG_LO_FLOW_MSK	GENMASK(19, 16)
> +#define SMB_LB_CFG_LO_DEFAULT	(SMB_CFG_LO_EN | SMB_CFG_LO_SINGLE_END | \
> +				 SMB_CFG_LO_INIT | SMB_CFG_LO_CONT | \
> +				 FIELD_PREP(SMB_CFG_LO_FLOW_MSK, 0xf))
> +
> +/* Set logical buffer config register upper 32 bits */
> +#define SMB_CFG_HI_RANGE_UP_MSK	GENMASK(15, 8)
> +#define SMB_LB_CFG_HI_DEFAULT	FIELD_PREP(SMB_CFG_HI_RANGE_UP_MSK, 0xff)
> +
> +/* Set logical buffer interrupt control register */
> +#define SMB_INT_CTRL_EN			BIT(0)
> +#define SMB_INT_CTRL_BUF_NOTE_MSK	GENMASK(11, 8)
> +#define SMB_LB_INT_CTRL_CFG	(SMB_INT_CTRL_EN | \
> +				 FIELD_PREP(SMB_INT_CTRL_BUF_NOTE_MSK, 0xf))
> +
> +#define SMB_LB_INT_STS_NOT_EMPTY_MSK	BIT(0)
> +#define SMB_LB_STS_RESET_MSK		GENMASK(3, 0)
> +#define SMB_LB_INT_BUF_STS_RESET	FIELD_PREP(SMB_LB_STS_RESET_MSK, 0xf)
> +#define SMB_LB_PURGE_PURGED	BIT(0)
> +#define SMB_GLB_EN_HW_ENABLE	BIT(0)
> +
> +#define SMB_REG_ADDR_RES	0
> +#define SMB_BUF_ADDR_RES	1
> +#define SMB_BUF_ADDR_LO_MSK	GENMASK(31, 0)

...

> +
> +/**
> + * struct smb_drv_data - specifics associated to an SMB component
> + * @base:	Memory mapped base address for SMB component.
> + * @csdev:	Component vitals needed by the framework.
> + * @sdb:	Data buffer for SMB.
> + * @miscdev:	Specifics to handle "/dev/xyz.smb" entry.
> + * @mutex:	Control data access to one at a time.
> + * @reading:	Synchronise user space access to SMB buffer.
> + * @pid:	Process ID of the process being monitored by the
> + *		session that is using this component.
> + * @mode:	how this SMB is being used, perf mode or sysfs mode.
Trivial, but for consistency should be: How this...

> + */
> +struct smb_drv_data {
> +	void __iomem *base;
> +	struct coresight_device	*csdev;
> +	struct smb_data_buffer sdb;
> +	struct miscdevice miscdev;
> +	struct mutex mutex;
> +	local_t reading;
> +	pid_t pid;
> +	u32 mode;
> +};
> +
> +#endif
  
hejunhao Nov. 10, 2022, 11:13 a.m. UTC | #2
On 2022/11/10 0:56, Jonathan Cameron wrote:
> On Wed, 9 Nov 2022 21:50:07 +0800
> Junhao He <hejunhao3@huawei.com> wrote:
>
>> From: Qi Liu <liuqi115@huawei.com>
>>
>> This patch adds driver for UltraSoc SMB(System Memory Buffer)
>> device. SMB provides a way to buffer messages from ETM, and
>> store these "CPU instructions trace" in system memory.
>>
>> SMB is developed by UltraSoc technology, which is acquired by
>> Siemens, and we still use "UltraSoc" to name driver.
>>
>> Signed-off-by: Qi Liu <liuqi115@huawei.com>
>> Signed-off-by: Junhao He <hejunhao3@huawei.com>
>> Tested-by: JunHao He <hejunhao3@huawei.com>
> Hi JunHao,
>
> One trivial side effect of dropping the ACPI dependency.
>
> Also, I think (at the cost of a slightly lengthening of lines)
> you can rename the register fields to avoid any potential
> confusion between GLB and LB registers.
>
> With those fixed feel free to add
>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
Hi Jonathan,

Thanks for you comments!
I will fix these in the next version.

Thanks.
>
> ...
>
>> diff --git a/drivers/hwtracing/coresight/ultrasoc-smb.c b/drivers/hwtracing/coresight/ultrasoc-smb.c
>> new file mode 100644
>> index 000000000000..ea2552a98d28
>> --- /dev/null
>> +++ b/drivers/hwtracing/coresight/ultrasoc-smb.c
> ...
>
>> +static const struct acpi_device_id ultrasoc_smb_acpi_match[] = {
>> +	{"HISI03A1", 0},
>> +	{}
>> +};
>> +MODULE_DEVICE_TABLE(acpi, ultrasoc_smb_acpi_match);
>> +
>> +static struct platform_driver smb_driver = {
>> +	.driver = {
>> +		.name = "ultrasoc-smb",
>> +		.acpi_match_table = ACPI_PTR(ultrasoc_smb_acpi_match),
> Now the driver build isn't dependent on CONFIG_ACPI
> if !CONFIG_ACPI ACPI_PTR() doesn't reference the parameter.
> As such you'll get unused warnings.
>
> 1 options to fix this
> a) Drop ACPI_PTR() and just have .acpi_match_data = ultrasoc_smb_acpi_match
> b) ifdef magic around the acpi_match table.
>
> In theory the first option results in bloat, but in this case I doubt we care.
Ok, will fix it, as follows

```
#ifdef CONFIG_ACPI
static const struct acpi_device_id ultrasoc_smb_acpi_match[] = {
     {"HISI03A1", 0},
     {}
};
MODULE_DEVICE_TABLE(acpi, ultrasoc_smb_acpi_match);
#endif
```

>> +		.suppress_bind_attrs = true,
>> +	},
>> +	.probe = smb_probe,
>> +	.remove = smb_remove,
>> +};
>> +module_platform_driver(smb_driver);
>> +
>> +MODULE_DESCRIPTION("UltraSoc SMB CoreSight driver");
>> +MODULE_LICENSE("Dual MIT/GPL");
>> +MODULE_AUTHOR("Jonathan Zhou <jonathan.zhouwen@huawei.com>");
>> +MODULE_AUTHOR("Qi Liu <liuqi115@huawei.com>");
>> diff --git a/drivers/hwtracing/coresight/ultrasoc-smb.h b/drivers/hwtracing/coresight/ultrasoc-smb.h
>> new file mode 100644
>> index 000000000000..2e2f9f8fe54b
>> --- /dev/null
>> +++ b/drivers/hwtracing/coresight/ultrasoc-smb.h
>> @@ -0,0 +1,116 @@
>> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
>> +/*
>> + * Siemens System Memory Buffer driver.
>> + * Copyright(c) 2022, HiSilicon Limited.
>> + */
>> +
>> +#ifndef _ULTRASOC_SMB_H
>> +#define _ULTRASOC_SMB_H
>> +
>> +#include <linux/miscdevice.h>
>> +#include <linux/mutex.h>
>> +
>> +/* Offset of SMB global registers */
>> +#define SMB_GLB_CFG_REG		0x00
>> +#define SMB_GLB_EN_REG		0x04
>> +#define SMB_GLB_INT_REG		0x08
>> +
>> +/* Offset of SMB logical buffer registers */
>> +#define SMB_LB_CFG_LO_REG	0x40
>> +#define SMB_LB_CFG_HI_REG	0x44
>> +#define SMB_LB_INT_CTRL_REG	0x48
>> +#define SMB_LB_INT_STS_REG	0x4c
>> +#define SMB_LB_RD_ADDR_REG	0x5c
>> +#define SMB_LB_WR_ADDR_REG	0x60
>> +#define SMB_LB_PURGE_REG	0x64
>> +
>> +/* Set global config register */
>> +#define SMB_CFG_BURST_LEN_MSK	GENMASK(11, 4)
> Given there are several CFG registers, possibly worth
> prefix of SMB_GLB_CFG_ ...
Sure, I will do that.

>> +#define SMB_CFG_IDLE_PRD_MSK	GENMASK(15, 12)
>> +#define SMB_CFG_MEM_WR_MSK	GENMASK(21, 16)
>> +#define SMB_CFG_MEM_RD_MSK	GENMASK(27, 22)
>> +#define SMB_GLB_CFG_DEFAULT	(FIELD_PREP(SMB_CFG_BURST_LEN_MSK, 0xf) | \
>> +				 FIELD_PREP(SMB_CFG_IDLE_PRD_MSK, 0xf) | \
>> +				 FIELD_PREP(SMB_CFG_MEM_WR_MSK, 0x3) | \
>> +				 FIELD_PREP(SMB_CFG_MEM_RD_MSK, 0x1b))
>> +
>> +/* Set global interrupt control register */
>> +#define SMB_INT_EN		BIT(0)
> Again, multiple INT registers, so SMB_INT_GLB_* perhaps?
Ok, will fix it.

>> +#define SMB_INT_PULSE		BIT(1) /* Interrupt type: 1 - Pulse */
>> +#define SMB_INT_ACT_H		BIT(2) /* Interrupt polarity: 1 - Active high */
>> +#define SMB_GLB_INT_CFG		(SMB_INT_EN | SMB_INT_PULSE | SMB_INT_ACT_H)
>> +
>> +/* Set logical buffer config register lower 32 bits */
>> +#define SMB_CFG_LO_EN		BIT(0)
> SMB_LB_CFG_...
>
> etc for other cases.
Sure, I will do that.

>> +#define SMB_CFG_LO_SINGLE_END	BIT(1)
>> +#define SMB_CFG_LO_INIT		BIT(8)
>> +#define SMB_CFG_LO_CONT		BIT(11)
>> +#define SMB_CFG_LO_FLOW_MSK	GENMASK(19, 16)
>> +#define SMB_LB_CFG_LO_DEFAULT	(SMB_CFG_LO_EN | SMB_CFG_LO_SINGLE_END | \
>> +				 SMB_CFG_LO_INIT | SMB_CFG_LO_CONT | \
>> +				 FIELD_PREP(SMB_CFG_LO_FLOW_MSK, 0xf))
>> +
>> +/* Set logical buffer config register upper 32 bits */
>> +#define SMB_CFG_HI_RANGE_UP_MSK	GENMASK(15, 8)
>> +#define SMB_LB_CFG_HI_DEFAULT	FIELD_PREP(SMB_CFG_HI_RANGE_UP_MSK, 0xff)
>> +
>> +/* Set logical buffer interrupt control register */
>> +#define SMB_INT_CTRL_EN			BIT(0)
>> +#define SMB_INT_CTRL_BUF_NOTE_MSK	GENMASK(11, 8)
>> +#define SMB_LB_INT_CTRL_CFG	(SMB_INT_CTRL_EN | \
>> +				 FIELD_PREP(SMB_INT_CTRL_BUF_NOTE_MSK, 0xf))
>> +
>> +#define SMB_LB_INT_STS_NOT_EMPTY_MSK	BIT(0)
>> +#define SMB_LB_STS_RESET_MSK		GENMASK(3, 0)
>> +#define SMB_LB_INT_BUF_STS_RESET	FIELD_PREP(SMB_LB_STS_RESET_MSK, 0xf)
>> +#define SMB_LB_PURGE_PURGED	BIT(0)
>> +#define SMB_GLB_EN_HW_ENABLE	BIT(0)
>> +
>> +#define SMB_REG_ADDR_RES	0
>> +#define SMB_BUF_ADDR_RES	1
>> +#define SMB_BUF_ADDR_LO_MSK	GENMASK(31, 0)
> ...
>
>> +
>> +/**
>> + * struct smb_drv_data - specifics associated to an SMB component
>> + * @base:	Memory mapped base address for SMB component.
>> + * @csdev:	Component vitals needed by the framework.
>> + * @sdb:	Data buffer for SMB.
>> + * @miscdev:	Specifics to handle "/dev/xyz.smb" entry.
>> + * @mutex:	Control data access to one at a time.
>> + * @reading:	Synchronise user space access to SMB buffer.
>> + * @pid:	Process ID of the process being monitored by the
>> + *		session that is using this component.
>> + * @mode:	how this SMB is being used, perf mode or sysfs mode.
> Trivial, but for consistency should be: How this...
Yes, I will do that.

>> + */
>> +struct smb_drv_data {
>> +	void __iomem *base;
>> +	struct coresight_device	*csdev;
>> +	struct smb_data_buffer sdb;
>> +	struct miscdevice miscdev;
>> +	struct mutex mutex;
>> +	local_t reading;
>> +	pid_t pid;
>> +	u32 mode;
>> +};
>> +
>> +#endif
> .
>
Best regards,
Junhao.
  
Yicong Yang Nov. 10, 2022, 12:05 p.m. UTC | #3
Hi Junhao,

On 2022/11/9 21:50, Junhao He wrote:
> From: Qi Liu <liuqi115@huawei.com>
> 
> This patch adds driver for UltraSoc SMB(System Memory Buffer)
> device. SMB provides a way to buffer messages from ETM, and
> store these "CPU instructions trace" in system memory.
> 
> SMB is developed by UltraSoc technology, which is acquired by
> Siemens, and we still use "UltraSoc" to name driver.
> 

Some comments inline.

> Signed-off-by: Qi Liu <liuqi115@huawei.com>
> Signed-off-by: Junhao He <hejunhao3@huawei.com>
> Tested-by: JunHao He <hejunhao3@huawei.com>
> ---
>  drivers/hwtracing/coresight/Kconfig        |  11 +
>  drivers/hwtracing/coresight/Makefile       |   1 +
>  drivers/hwtracing/coresight/ultrasoc-smb.c | 635 +++++++++++++++++++++
>  drivers/hwtracing/coresight/ultrasoc-smb.h | 116 ++++
>  4 files changed, 763 insertions(+)
>  create mode 100644 drivers/hwtracing/coresight/ultrasoc-smb.c
>  create mode 100644 drivers/hwtracing/coresight/ultrasoc-smb.h
> 
> diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
> index 45c1eb5dfcb7..cb17c207a728 100644
> --- a/drivers/hwtracing/coresight/Kconfig
> +++ b/drivers/hwtracing/coresight/Kconfig
> @@ -201,4 +201,15 @@ config CORESIGHT_TRBE
>  
>  	  To compile this driver as a module, choose M here: the module will be
>  	  called coresight-trbe.
> +
> +config ULTRASOC_SMB
> +	tristate "Ultrasoc system memory buffer drivers"
> +	depends on ARM64 && CORESIGHT_LINKS_AND_SINKS
> +	help
> +	  This driver provides support for the Ultrasoc system memory buffer (SMB).
> +	  SMB is responsible for receiving the trace data from Coresight ETM devices
> +	  and storing them to a system buffer.
> +
> +	  To compile this driver as a module, choose M here: the module will be
> +	  called ultrasoc-smb.
>  endif
> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
> index b6c4a48140ec..344dba8d6ff8 100644
> --- a/drivers/hwtracing/coresight/Makefile
> +++ b/drivers/hwtracing/coresight/Makefile
> @@ -27,3 +27,4 @@ obj-$(CONFIG_CORESIGHT_CTI) += coresight-cti.o
>  obj-$(CONFIG_CORESIGHT_TRBE) += coresight-trbe.o
>  coresight-cti-y := coresight-cti-core.o	coresight-cti-platform.o \
>  		   coresight-cti-sysfs.o
> +obj-$(CONFIG_ULTRASOC_SMB) += ultrasoc-smb.o
> diff --git a/drivers/hwtracing/coresight/ultrasoc-smb.c b/drivers/hwtracing/coresight/ultrasoc-smb.c
> new file mode 100644
> index 000000000000..ea2552a98d28
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/ultrasoc-smb.c
> @@ -0,0 +1,635 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * Siemens System Memory Buffer driver.
> + * Copyright(c) 2022, HiSilicon Limited.
> + */
> +
> +#include <linux/atomic.h>
> +#include <linux/acpi.h>
> +#include <linux/circ_buf.h>
> +#include <linux/err.h>
> +#include <linux/fs.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/platform_device.h>
> +
> +#include "coresight-etm-perf.h"
> +#include "coresight-priv.h"
> +#include "ultrasoc-smb.h"
> +
> +DEFINE_CORESIGHT_DEVLIST(sink_devs, "ultra_smb");
> +
> +#define ULTRASOC_SMB_DSM_UUID	"82ae1283-7f6a-4cbe-aa06-53e8fb24db18"
> +
> +static bool smb_buffer_is_empty(struct smb_drv_data *drvdata)
> +{
> +	u32 buf_status = readl(drvdata->base + SMB_LB_INT_STS_REG);
> +
> +	return !FIELD_PREP(SMB_LB_INT_STS_NOT_EMPTY_MSK, buf_status);

Should be FIELD_GET() here?

> +}
> +
> +static void smb_buffer_sync_status(struct smb_drv_data *drvdata)
> +{
> +	struct smb_data_buffer *sdb = &drvdata->sdb;
> +
> +	sdb->wr_offset = readl(drvdata->base + SMB_LB_WR_ADDR_REG) - sdb->start_addr;
> +	sdb->rd_offset = readl(drvdata->base + SMB_LB_RD_ADDR_REG) - sdb->start_addr;
> +	sdb->full = sdb->wr_offset == sdb->rd_offset && !smb_buffer_is_empty(drvdata);

Mistake here? Why to overwrite wr_offset?

> +}
> +
> +static void smb_reset_buffer_status(struct smb_drv_data *drvdata)
> +{
> +	writel(SMB_LB_INT_BUF_STS_RESET, drvdata->base + SMB_LB_INT_STS_REG);

You'll reset other fields of SMB_LB_INT_STS_REG since SMB_LB_INT_BUF_STS_RESET is only
BIT [3, 0], if it's not intended I'd suggest to only update the necessary fields. So is
other places.

> +}
> +
> +/* Purge data remaining in hardware path in case them influence next trace */
> +static void smb_purge_data(struct smb_drv_data *drvdata)
> +{
> +	writel(SMB_LB_PURGE_PURGED, drvdata->base + SMB_LB_PURGE_REG);
> +}
> +
> +static void smb_update_data_size(struct smb_drv_data *drvdata)
> +{
> +	struct smb_data_buffer *sdb = &drvdata->sdb;
> +
> +	smb_purge_data(drvdata);
> +	smb_buffer_sync_status(drvdata);
> +	if (sdb->full) {
> +		sdb->data_size = sdb->buf_size;
> +		return;
> +	}
> +
> +	sdb->data_size = CIRC_CNT(sdb->wr_offset, sdb->rd_offset, sdb->buf_size);
> +}
> +
> +static int smb_open(struct inode *inode, struct file *file)
> +{
> +	struct smb_drv_data *drvdata = container_of(file->private_data,
> +					struct smb_drv_data, miscdev);
> +	int ret = 0;
> +
> +	mutex_lock(&drvdata->mutex);
> +
> +	if (local_read(&drvdata->reading)) {
> +		ret = -EBUSY;
> +		goto out;
> +	}
> +
> +	if (atomic_read(drvdata->csdev->refcnt)) {
> +		ret = -EBUSY;
> +		goto out;
> +	}
> +
> +	local_set(&drvdata->reading, 1);
> +out:
> +	mutex_unlock(&drvdata->mutex);
> +
> +	return ret;
> +}
> +
> +static ssize_t smb_read(struct file *file, char __user *data, size_t len,
> +			loff_t *ppos)
> +{
> +	struct smb_drv_data *drvdata = container_of(file->private_data,
> +					struct smb_drv_data, miscdev);
> +	struct smb_data_buffer *sdb = &drvdata->sdb;
> +	struct device *dev = &drvdata->csdev->dev;
> +	ssize_t to_copy = 0;
> +
> +	mutex_lock(&drvdata->mutex);
> +
> +	if (!sdb->data_size) {
> +		smb_update_data_size(drvdata);
> +		if (!sdb->data_size)
> +			goto out;
> +	}
> +

It'd be better to check this earlier in open().

> +	to_copy = min(sdb->data_size, len);
> +
> +	/* Copy parts of trace data when read pointer wrap around SMB buffer */
> +	if (sdb->rd_offset + to_copy > sdb->buf_size)
> +		to_copy = sdb->buf_size - sdb->rd_offset;
> +
> +	if (copy_to_user(data, (void *)sdb->buf_base + sdb->rd_offset,
> +			 to_copy)) {
> +		dev_dbg(dev, "Failed to copy data to user\n");
> +		to_copy = -EFAULT;
> +		goto out;
> +	}
> +
> +	*ppos += to_copy;
> +	sdb->data_size -= to_copy;
> +	sdb->rd_offset += to_copy;
> +	sdb->rd_offset %= sdb->buf_size;
> +	writel(sdb->start_addr + sdb->rd_offset,
> +	       drvdata->base + SMB_LB_RD_ADDR_REG);
> +	dev_dbg(dev, "%zu bytes copied\n", to_copy);
> +out:
> +	if (!sdb->data_size)
> +		smb_reset_buffer_status(drvdata);
> +	mutex_unlock(&drvdata->mutex);
> +
> +	return to_copy;
> +}
> +
> +static int smb_release(struct inode *inode, struct file *file)
> +{
> +	struct smb_drv_data *drvdata = container_of(file->private_data,
> +					struct smb_drv_data, miscdev);
> +
> +	mutex_lock(&drvdata->mutex);
> +	local_set(&drvdata->reading, 0);
> +	mutex_unlock(&drvdata->mutex);
> +
> +	return 0;
> +}
> +
> +static const struct file_operations smb_fops = {
> +	.owner		= THIS_MODULE,
> +	.open		= smb_open,
> +	.read		= smb_read,
> +	.release	= smb_release,
> +	.llseek		= no_llseek,
> +};
> +
> +static ssize_t buf_size_show(struct device *dev, struct device_attribute *attr,
> +			     char *buf)
> +{
> +	struct smb_drv_data *drvdata = dev_get_drvdata(dev->parent);
> +
> +	return sysfs_emit(buf, "0x%lx\n", drvdata->sdb.buf_size);

Need to update the data size before sysfs_emit().

> +}
> +static DEVICE_ATTR_RO(buf_size);
> +
> +static struct attribute *smb_sink_attrs[] = {
> +	coresight_simple_reg32(read_pos, SMB_LB_RD_ADDR_REG),
> +	coresight_simple_reg32(write_pos, SMB_LB_WR_ADDR_REG),
> +	coresight_simple_reg32(buf_status, SMB_LB_INT_STS_REG),
> +	&dev_attr_buf_size.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group smb_sink_group = {
> +	.attrs = smb_sink_attrs,
> +	.name = "mgmt",
> +};
> +
> +static const struct attribute_group *smb_sink_groups[] = {
> +	&smb_sink_group,
> +	NULL
> +};
> +
> +static void smb_enable_hw(struct smb_drv_data *drvdata)
> +{
> +	writel(SMB_GLB_EN_HW_ENABLE, drvdata->base + SMB_GLB_EN_REG);
> +}
> +
> +static void smb_disable_hw(struct smb_drv_data *drvdata)
> +{
> +	writel(0x0, drvdata->base + SMB_GLB_EN_REG);
> +}
> +
> +static void smb_enable_sysfs(struct coresight_device *csdev)
> +{
> +	struct smb_drv_data *drvdata = dev_get_drvdata(csdev->dev.parent);
> +
> +	if (drvdata->mode != CS_MODE_DISABLED)
> +		return;
> +
> +	smb_enable_hw(drvdata);
> +	drvdata->mode = CS_MODE_SYSFS;
> +}
> +
> +static int smb_enable_perf(struct coresight_device *csdev, void *data)
> +{
> +	struct smb_drv_data *drvdata = dev_get_drvdata(csdev->dev.parent);
> +	struct perf_output_handle *handle = data;
> +	struct cs_buffers *buf = etm_perf_sink_config(handle);
> +	pid_t pid;
> +
> +	if (!buf)
> +		return -EINVAL;
> +
> +	/* Get a handle on the pid of the target process */
> +	pid = buf->pid;
> +
> +	/* Device is already in used by other session */
> +	if (drvdata->pid != -1 && drvdata->pid != pid)
> +		return -EBUSY;
> +
> +	if (drvdata->pid == -1) {
> +		smb_enable_hw(drvdata);
> +		drvdata->pid = pid;
> +		drvdata->mode = CS_MODE_PERF;
> +	}
> +
> +	return 0;
> +}
> +
> +static int smb_enable(struct coresight_device *csdev, u32 mode, void *data)
> +{
> +	struct smb_drv_data *drvdata = dev_get_drvdata(csdev->dev.parent);
> +	int ret = 0;
> +
> +	mutex_lock(&drvdata->mutex);
> +
> +	/* Do nothing, the trace data is reading by other interface now */
> +	if (local_read(&drvdata->reading)) {
> +		ret = -EBUSY;
> +		goto out;
> +	}
> +
> +	/* Do nothing, the SMB is already enabled as other mode */
> +	if (drvdata->mode != CS_MODE_DISABLED && drvdata->mode != mode) {
> +		ret = -EBUSY;
> +		goto out;
> +	}
> +
> +	switch (mode) {
> +	case CS_MODE_SYSFS:
> +		smb_enable_sysfs(csdev);
> +		break;
> +	case CS_MODE_PERF:
> +		ret = smb_enable_perf(csdev, data);
> +		break;
> +	default:
> +		ret = -EINVAL;
> +	}
> +
> +	if (ret)
> +		goto out;
> +
> +	atomic_inc(csdev->refcnt);
> +
> +	dev_dbg(&csdev->dev, "Ultrasoc SMB enabled\n");
> +out:
> +	mutex_unlock(&drvdata->mutex);
> +
> +	return ret;
> +}
> +
> +static int smb_disable(struct coresight_device *csdev)
> +{
> +	struct smb_drv_data *drvdata = dev_get_drvdata(csdev->dev.parent);
> +	int ret = 0;
> +
> +	mutex_lock(&drvdata->mutex);
> +
> +	if (local_read(&drvdata->reading)) {
> +		ret = -EBUSY;
> +		goto out;
> +	}
> +
> +	if (atomic_dec_return(csdev->refcnt)) {
> +		ret = -EBUSY;
> +		goto out;
> +	}
> +
> +	/* Complain if we (somehow) got out of sync */
> +	WARN_ON_ONCE(drvdata->mode == CS_MODE_DISABLED);

Is there any conditions we can reach here? I see we always update
drvdata->mode within the mutex's protection.

> +
> +	smb_disable_hw(drvdata);
> +	smb_purge_data(drvdata);
> +
> +	/* Dissociate from the target process. */
> +	drvdata->pid = -1;
> +	drvdata->mode = CS_MODE_DISABLED;
> +
> +	dev_dbg(&csdev->dev, "Ultrasoc SMB disabled\n");
> +out:
> +	mutex_unlock(&drvdata->mutex);
> +
> +	return ret;
> +}
> +
> +static void *smb_alloc_buffer(struct coresight_device *csdev,
> +			      struct perf_event *event, void **pages,
> +			      int nr_pages, bool overwrite)
> +{
> +	struct cs_buffers *buf;
> +	int node;
> +
> +	node = (event->cpu == -1) ? NUMA_NO_NODE : cpu_to_node(event->cpu);
> +	buf = kzalloc_node(sizeof(struct cs_buffers), GFP_KERNEL, node);
> +	if (!buf)
> +		return NULL;
> +
> +	buf->snapshot = overwrite;
> +	buf->nr_pages = nr_pages;
> +	buf->data_pages = pages;
> +	buf->pid = task_pid_nr(event->owner);
> +
> +	return buf;
> +}
> +
> +static void smb_free_buffer(void *config)
> +{
> +	struct cs_buffers *buf = config;
> +
> +	kfree(buf);
> +}
> +
> +static void smb_sync_perf_buffer(struct smb_drv_data *drvdata,
> +				 struct cs_buffers *buf,
> +				 unsigned long head,
> +				 unsigned long data_size)
> +{
> +	struct smb_data_buffer *sdb = &drvdata->sdb;
> +	char **dst_pages = (char **)buf->data_pages;
> +	unsigned long to_copy;
> +	long pg_idx, pg_offset;
> +
> +	pg_idx = head >> PAGE_SHIFT;
> +	pg_offset = head & (PAGE_SIZE - 1);
> +
> +	while (data_size) {
> +		unsigned long pg_space = PAGE_SIZE - pg_offset;
> +
> +		/* Copy parts of trace data when read pointer wrap around */
> +		if (sdb->rd_offset + pg_space > sdb->buf_size)
> +			to_copy = sdb->buf_size - sdb->rd_offset;
> +		else
> +			to_copy = min(data_size, pg_space);
> +
> +		memcpy(dst_pages[pg_idx] + pg_offset,
> +			      sdb->buf_base + sdb->rd_offset, to_copy);
> +
> +		pg_offset += to_copy;
> +		if (pg_offset >= PAGE_SIZE) {
> +			pg_offset = 0;
> +			pg_idx++;
> +			pg_idx %= buf->nr_pages;
> +		}
> +		data_size -= to_copy;
> +		sdb->rd_offset += to_copy;
> +		sdb->rd_offset %= sdb->buf_size;
> +	}
> +
> +	sdb->data_size = 0;
> +	writel(sdb->start_addr + sdb->rd_offset, drvdata->base + SMB_LB_RD_ADDR_REG);
> +
> +	/*
> +	 * Data remained in link cannot be purged when SMB is full, so
> +	 * synchronize the read pointer to write pointer, to make sure
> +	 * these remained data won't influence next trace.
> +	 */
> +	if (sdb->full) {
> +		smb_purge_data(drvdata);
> +		writel(readl(drvdata->base + SMB_LB_WR_ADDR_REG),
> +		       drvdata->base + SMB_LB_RD_ADDR_REG);
> +	}
> +	smb_reset_buffer_status(drvdata);
> +}
> +
> +static unsigned long smb_update_buffer(struct coresight_device *csdev,
> +				       struct perf_output_handle *handle,
> +				       void *sink_config)
> +{
> +	struct smb_drv_data *drvdata = dev_get_drvdata(csdev->dev.parent);
> +	struct smb_data_buffer *sdb = &drvdata->sdb;
> +	struct cs_buffers *buf = sink_config;
> +	unsigned long data_size = 0;
> +	bool lost = false;
> +
> +	if (!buf)
> +		return 0;
> +
> +	mutex_lock(&drvdata->mutex);
> +
> +	/* Don't do anything if another tracer is using this sink. */
> +	if (atomic_read(csdev->refcnt) != 1)
> +		goto out;
> +
> +	smb_disable_hw(drvdata);
> +	smb_update_data_size(drvdata);
> +	data_size = sdb->data_size;
> +
> +	/*
> +	 * The SMB buffer may be bigger than the space available in the
> +	 * perf ring buffer (handle->size). If so advance the offset so
> +	 * that we get the latest trace data.
> +	 */
> +	if (data_size > handle->size) {
> +		sdb->rd_offset += data_size - handle->size;
> +		sdb->rd_offset %= sdb->buf_size;
> +		data_size = handle->size;
> +		lost = true;
> +	}
> +
> +	smb_sync_perf_buffer(drvdata, buf, handle->head, data_size);
> +	if (!buf->snapshot && lost)
> +		perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
> +
> +	smb_enable_hw(drvdata);
> +out:
> +	mutex_unlock(&drvdata->mutex);
> +
> +	return data_size;
> +}
> +
> +static const struct coresight_ops_sink smb_cs_ops = {
> +	.enable		= smb_enable,
> +	.disable	= smb_disable,
> +	.alloc_buffer	= smb_alloc_buffer,
> +	.free_buffer	= smb_free_buffer,
> +	.update_buffer	= smb_update_buffer,
> +};
> +
> +static const struct coresight_ops cs_ops = {
> +	.sink_ops	= &smb_cs_ops,
> +};
> +
> +static int smb_init_data_buffer(struct platform_device *pdev,
> +				struct smb_data_buffer *sdb)
> +{
> +	struct resource *res;
> +	void __iomem *base;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, SMB_BUF_ADDR_RES);
> +	if (IS_ERR(res)) {
> +		dev_err(&pdev->dev, "SMB device failed to get resource\n");
> +		return -EINVAL;
> +	}
> +
> +	sdb->start_addr = FIELD_PREP(SMB_BUF_ADDR_LO_MSK, res->start);

also FIELD_GET()?

> +	sdb->buf_size = resource_size(res);
> +	if (sdb->buf_size == 0)
> +		return -EINVAL;
> +
> +	/*
> +	 * This is a chunk of memory, use classic mapping with better
> +	 * performance.
> +	 */
> +	base = devm_memremap(&pdev->dev, sdb->start_addr, sdb->buf_size,
> +				MEMREMAP_WB);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	sdb->buf_base = base;
> +
> +	return 0;
> +}
> +
> +static void smb_init_hw(struct smb_drv_data *drvdata)
> +{
> +	/* First disable SMB and clear the status of SMB buffer */
> +	smb_reset_buffer_status(drvdata);
> +	smb_disable_hw(drvdata);
> +	smb_purge_data(drvdata);
> +
> +	writel(SMB_LB_CFG_LO_DEFAULT, drvdata->base + SMB_LB_CFG_LO_REG);
> +	writel(SMB_LB_CFG_HI_DEFAULT, drvdata->base + SMB_LB_CFG_HI_REG);
> +	writel(SMB_GLB_CFG_DEFAULT, drvdata->base + SMB_GLB_CFG_REG);
> +	writel(SMB_GLB_INT_CFG, drvdata->base + SMB_GLB_INT_REG);
> +	writel(SMB_LB_INT_CTRL_CFG, drvdata->base + SMB_LB_INT_CTRL_REG);
> +}
> +
> +static int smb_register_sink(struct platform_device *pdev,
> +			     struct smb_drv_data *drvdata)
> +{
> +	struct coresight_platform_data *pdata = NULL;
> +	struct coresight_desc desc = { 0 };
> +	int ret;
> +
> +	pdata = coresight_get_platform_data(&pdev->dev);
> +	if (IS_ERR(pdata))
> +		return PTR_ERR(pdata);
> +
> +	desc.type = CORESIGHT_DEV_TYPE_SINK;
> +	desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_BUFFER;
> +	desc.ops = &cs_ops;
> +	desc.pdata = pdata;
> +	desc.dev = &pdev->dev;
> +	desc.groups = smb_sink_groups;
> +	desc.name = coresight_alloc_device_name(&sink_devs, &pdev->dev);
> +	if (!desc.name) {
> +		dev_err(&pdev->dev, "Failed to alloc coresight device name");
> +		return -ENOMEM;
> +	}
> +	desc.access = CSDEV_ACCESS_IOMEM(drvdata->base);
> +
> +	drvdata->csdev = coresight_register(&desc);
> +	if (IS_ERR(drvdata->csdev))
> +		return PTR_ERR(drvdata->csdev);
> +
> +	drvdata->miscdev.name = desc.name;
> +	drvdata->miscdev.minor = MISC_DYNAMIC_MINOR;
> +	drvdata->miscdev.fops = &smb_fops;
> +	ret = misc_register(&drvdata->miscdev);
> +	if (ret) {
> +		coresight_unregister(drvdata->csdev);
> +		dev_err(&pdev->dev, "Failed to register misc, ret=%d\n", ret);
> +	}
> +
> +	return ret;
> +}
> +
> +static void smb_unregister_sink(struct smb_drv_data *drvdata)
> +{
> +	misc_deregister(&drvdata->miscdev);
> +	coresight_unregister(drvdata->csdev);
> +}
> +
> +static int smb_config_inport(struct device *dev, bool enable)
> +{
> +	u64 func = enable ? 1 : 0;
> +	union acpi_object *obj;
> +	guid_t guid;
> +	u64 rev = 0;
> +
> +	/*
> +	 * Using DSM calls to enable/disable ultrasoc hardwares on
> +	 * tracing path, to prevent ultrasoc packet format being exposed.
> +	 */
> +	if (guid_parse(ULTRASOC_SMB_DSM_UUID, &guid)) {
> +		dev_err(dev, "Get GUID failed\n");
> +		return -EINVAL;
> +	}
> +
> +	obj = acpi_evaluate_dsm(ACPI_HANDLE(dev), &guid, rev, func, NULL);
> +	if (!obj)
> +		dev_err(dev, "ACPI handle failed\n");
> +	else
> +		ACPI_FREE(obj);
> +
> +	return 0;
> +}
> +
> +static int smb_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct smb_drv_data *drvdata;
> +	int ret;
> +
> +	drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> +	if (!drvdata)
> +		return -ENOMEM;
> +
> +	drvdata->base = devm_platform_ioremap_resource(pdev, SMB_REG_ADDR_RES);
> +	if (IS_ERR(drvdata->base)) {
> +		dev_err(dev, "Failed to ioremap resource\n");
> +		return PTR_ERR(drvdata->base);
> +	}
> +
> +	ret = smb_init_data_buffer(pdev, &drvdata->sdb);
> +	if (ret) {
> +		dev_err(dev, "Failed to init buffer, ret = %d\n", ret);
> +		return ret;
> +	}
> +
> +	smb_init_hw(drvdata);
> +	mutex_init(&drvdata->mutex);
> +	drvdata->pid = -1;
> +
> +	ret = smb_register_sink(pdev, drvdata);
> +	if (ret) {
> +		dev_err(dev, "Failed to register SMB sink\n");
> +		return ret;
> +	}
> +
> +	ret = smb_config_inport(dev, true);
> +	if (ret) {
> +		smb_unregister_sink(drvdata);
> +		return ret;
> +	}
> +
> +	platform_set_drvdata(pdev, drvdata);
> +
> +	return 0;
> +}
> +
> +static int smb_remove(struct platform_device *pdev)
> +{
> +	struct smb_drv_data *drvdata = platform_get_drvdata(pdev);
> +	int ret;
> +
> +	ret = smb_config_inport(&pdev->dev, false);
> +	if (ret)
> +		return ret;
> +
> +	smb_unregister_sink(drvdata);
> +
> +	return 0;
> +}
> +
> +static const struct acpi_device_id ultrasoc_smb_acpi_match[] = {
> +	{"HISI03A1", 0},
> +	{}
> +};
> +MODULE_DEVICE_TABLE(acpi, ultrasoc_smb_acpi_match);
> +
> +static struct platform_driver smb_driver = {
> +	.driver = {
> +		.name = "ultrasoc-smb",
> +		.acpi_match_table = ACPI_PTR(ultrasoc_smb_acpi_match),
> +		.suppress_bind_attrs = true,
> +	},
> +	.probe = smb_probe,
> +	.remove = smb_remove,
> +};
> +module_platform_driver(smb_driver);
> +
> +MODULE_DESCRIPTION("UltraSoc SMB CoreSight driver");
> +MODULE_LICENSE("Dual MIT/GPL");
> +MODULE_AUTHOR("Jonathan Zhou <jonathan.zhouwen@huawei.com>");
> +MODULE_AUTHOR("Qi Liu <liuqi115@huawei.com>");
> diff --git a/drivers/hwtracing/coresight/ultrasoc-smb.h b/drivers/hwtracing/coresight/ultrasoc-smb.h
> new file mode 100644
> index 000000000000..2e2f9f8fe54b
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/ultrasoc-smb.h
> @@ -0,0 +1,116 @@
> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
> +/*
> + * Siemens System Memory Buffer driver.
> + * Copyright(c) 2022, HiSilicon Limited.
> + */
> +
> +#ifndef _ULTRASOC_SMB_H
> +#define _ULTRASOC_SMB_H
> +
> +#include <linux/miscdevice.h>
> +#include <linux/mutex.h>
> +
> +/* Offset of SMB global registers */
> +#define SMB_GLB_CFG_REG		0x00
> +#define SMB_GLB_EN_REG		0x04
> +#define SMB_GLB_INT_REG		0x08
> +
> +/* Offset of SMB logical buffer registers */
> +#define SMB_LB_CFG_LO_REG	0x40
> +#define SMB_LB_CFG_HI_REG	0x44
> +#define SMB_LB_INT_CTRL_REG	0x48
> +#define SMB_LB_INT_STS_REG	0x4c
> +#define SMB_LB_RD_ADDR_REG	0x5c
> +#define SMB_LB_WR_ADDR_REG	0x60
> +#define SMB_LB_PURGE_REG	0x64
> +
> +/* Set global config register */
> +#define SMB_CFG_BURST_LEN_MSK	GENMASK(11, 4)
> +#define SMB_CFG_IDLE_PRD_MSK	GENMASK(15, 12)
> +#define SMB_CFG_MEM_WR_MSK	GENMASK(21, 16)
> +#define SMB_CFG_MEM_RD_MSK	GENMASK(27, 22)
> +#define SMB_GLB_CFG_DEFAULT	(FIELD_PREP(SMB_CFG_BURST_LEN_MSK, 0xf) | \
> +				 FIELD_PREP(SMB_CFG_IDLE_PRD_MSK, 0xf) | \
> +				 FIELD_PREP(SMB_CFG_MEM_WR_MSK, 0x3) | \
> +				 FIELD_PREP(SMB_CFG_MEM_RD_MSK, 0x1b))
> +
> +/* Set global interrupt control register */
> +#define SMB_INT_EN		BIT(0)
> +#define SMB_INT_PULSE		BIT(1) /* Interrupt type: 1 - Pulse */
> +#define SMB_INT_ACT_H		BIT(2) /* Interrupt polarity: 1 - Active high */
> +#define SMB_GLB_INT_CFG		(SMB_INT_EN | SMB_INT_PULSE | SMB_INT_ACT_H)
> +
> +/* Set logical buffer config register lower 32 bits */
> +#define SMB_CFG_LO_EN		BIT(0)
> +#define SMB_CFG_LO_SINGLE_END	BIT(1)
> +#define SMB_CFG_LO_INIT		BIT(8)
> +#define SMB_CFG_LO_CONT		BIT(11)
> +#define SMB_CFG_LO_FLOW_MSK	GENMASK(19, 16)
> +#define SMB_LB_CFG_LO_DEFAULT	(SMB_CFG_LO_EN | SMB_CFG_LO_SINGLE_END | \
> +				 SMB_CFG_LO_INIT | SMB_CFG_LO_CONT | \
> +				 FIELD_PREP(SMB_CFG_LO_FLOW_MSK, 0xf))
> +
> +/* Set logical buffer config register upper 32 bits */
> +#define SMB_CFG_HI_RANGE_UP_MSK	GENMASK(15, 8)
> +#define SMB_LB_CFG_HI_DEFAULT	FIELD_PREP(SMB_CFG_HI_RANGE_UP_MSK, 0xff)
> +
> +/* Set logical buffer interrupt control register */
> +#define SMB_INT_CTRL_EN			BIT(0)
> +#define SMB_INT_CTRL_BUF_NOTE_MSK	GENMASK(11, 8)
> +#define SMB_LB_INT_CTRL_CFG	(SMB_INT_CTRL_EN | \
> +				 FIELD_PREP(SMB_INT_CTRL_BUF_NOTE_MSK, 0xf))
> +
> +#define SMB_LB_INT_STS_NOT_EMPTY_MSK	BIT(0)
> +#define SMB_LB_STS_RESET_MSK		GENMASK(3, 0)
> +#define SMB_LB_INT_BUF_STS_RESET	FIELD_PREP(SMB_LB_STS_RESET_MSK, 0xf)
> +#define SMB_LB_PURGE_PURGED	BIT(0)
> +#define SMB_GLB_EN_HW_ENABLE	BIT(0)
> +
> +#define SMB_REG_ADDR_RES	0
> +#define SMB_BUF_ADDR_RES	1
> +#define SMB_BUF_ADDR_LO_MSK	GENMASK(31, 0)
> +
> +/**
> + * struct smb_data_buffer - Details of the buffer used by SMB
> + * @buf_base:	Memory mapped base address of SMB.
> + * @start_addr:	SMB buffer start Physical address.
> + * @buf_size:	Size of the buffer.
> + * @data_size:	Size of Trace data copy to userspace.
> + * @rd_offset:	Offset of the read pointer in the buffer.
> + * @wr_offset:	Offset of the write pointer in the buffer.
> + * @full:	Trace data overflow.
> + */
> +struct smb_data_buffer {
> +	void __iomem *buf_base;

The backend of buffer is a chunk of memory, so I guess we don't need
this __iomem here.

> +	u32 start_addr;

Is u32 enough if we're running on a 64bit platform? You can use phys_addr_t
instead since you comment it to be "start Physical address".

> +	unsigned long buf_size;
> +	unsigned long data_size;
> +	unsigned long rd_offset;
> +	unsigned long wr_offset;
> +	bool full;
> +};
> +
> +/**
> + * struct smb_drv_data - specifics associated to an SMB component
> + * @base:	Memory mapped base address for SMB component.
> + * @csdev:	Component vitals needed by the framework.
> + * @sdb:	Data buffer for SMB.
> + * @miscdev:	Specifics to handle "/dev/xyz.smb" entry.
> + * @mutex:	Control data access to one at a time.
> + * @reading:	Synchronise user space access to SMB buffer.
> + * @pid:	Process ID of the process being monitored by the
> + *		session that is using this component.
> + * @mode:	how this SMB is being used, perf mode or sysfs mode.
> + */
> +struct smb_drv_data {
> +	void __iomem *base;
> +	struct coresight_device	*csdev;
> +	struct smb_data_buffer sdb;
> +	struct miscdevice miscdev;
> +	struct mutex mutex;
> +	local_t reading;

Why do we need to use local_t here? Considering the whole structure is
already protected by the mutex, maybe a boolean is enough?

Thanks.
  
kernel test robot Nov. 11, 2022, 1:30 p.m. UTC | #4
Hi Junhao,

I love your patch! Perhaps something to improve:

[auto build test WARNING on soc/for-next]
[also build test WARNING on linus/master v6.1-rc4 next-20221111]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Junhao-He/Add-support-for-UltraSoc-System-Memory-Buffer/20221109-215158
base:   https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
patch link:    https://lore.kernel.org/r/20221109135008.9485-2-hejunhao3%40huawei.com
patch subject: [PATCH v12 1/2] drivers/coresight: Add UltraSoc System Memory Buffer driver
config: arm64-randconfig-r023-20221111
compiler: aarch64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/49ede03fd874b65deca5382ea84c3377188136da
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Junhao-He/Add-support-for-UltraSoc-System-Memory-Buffer/20221109-215158
        git checkout 49ede03fd874b65deca5382ea84c3377188136da
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/hwtracing/coresight/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/hwtracing/coresight/ultrasoc-smb.c: In function 'smb_config_inport':
>> drivers/hwtracing/coresight/ultrasoc-smb.c:553:31: warning: suggest braces around empty body in an 'else' statement [-Wempty-body]
     553 |                 ACPI_FREE(obj);
         |                               ^
   drivers/hwtracing/coresight/ultrasoc-smb.c: At top level:
   drivers/hwtracing/coresight/ultrasoc-smb.c:615:36: warning: 'ultrasoc_smb_acpi_match' defined but not used [-Wunused-const-variable=]
     615 | static const struct acpi_device_id ultrasoc_smb_acpi_match[] = {
         |                                    ^~~~~~~~~~~~~~~~~~~~~~~


vim +/else +553 drivers/hwtracing/coresight/ultrasoc-smb.c

   532	
   533	static int smb_config_inport(struct device *dev, bool enable)
   534	{
   535		u64 func = enable ? 1 : 0;
   536		union acpi_object *obj;
   537		guid_t guid;
   538		u64 rev = 0;
   539	
   540		/*
   541		 * Using DSM calls to enable/disable ultrasoc hardwares on
   542		 * tracing path, to prevent ultrasoc packet format being exposed.
   543		 */
   544		if (guid_parse(ULTRASOC_SMB_DSM_UUID, &guid)) {
   545			dev_err(dev, "Get GUID failed\n");
   546			return -EINVAL;
   547		}
   548	
   549		obj = acpi_evaluate_dsm(ACPI_HANDLE(dev), &guid, rev, func, NULL);
   550		if (!obj)
   551			dev_err(dev, "ACPI handle failed\n");
   552		else
 > 553			ACPI_FREE(obj);
   554	
   555		return 0;
   556	}
   557
  
kernel test robot Nov. 11, 2022, 10:06 p.m. UTC | #5
Hi Junhao,

I love your patch! Perhaps something to improve:

[auto build test WARNING on soc/for-next]
[also build test WARNING on linus/master v6.1-rc4 next-20221111]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Junhao-He/Add-support-for-UltraSoc-System-Memory-Buffer/20221109-215158
base:   https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
patch link:    https://lore.kernel.org/r/20221109135008.9485-2-hejunhao3%40huawei.com
patch subject: [PATCH v12 1/2] drivers/coresight: Add UltraSoc System Memory Buffer driver
config: arm64-randconfig-s033-20221111
compiler: aarch64-linux-gcc (GCC) 12.1.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-39-gce1a6720-dirty
        # https://github.com/intel-lab-lkp/linux/commit/49ede03fd874b65deca5382ea84c3377188136da
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Junhao-He/Add-support-for-UltraSoc-System-Memory-Buffer/20221109-215158
        git checkout 49ede03fd874b65deca5382ea84c3377188136da
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/hwtracing/coresight/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

sparse warnings: (new ones prefixed by >>)
>> drivers/hwtracing/coresight/ultrasoc-smb.c:113:33: sparse: sparse: cast removes address space '__iomem' of expression
>> drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void const * @@     got void [noderef] __iomem * @@
   drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse:     expected void const *
   drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse:     got void [noderef] __iomem *
>> drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void const * @@     got void [noderef] __iomem * @@
   drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse:     expected void const *
   drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse:     got void [noderef] __iomem *
   drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse: sparse: incorrect type in argument 2 (different address spaces) @@     expected void const * @@     got void [noderef] __iomem * @@
   drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse:     expected void const *
   drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse:     got void [noderef] __iomem *
>> drivers/hwtracing/coresight/ultrasoc-smb.c:463:14: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected void [noderef] __iomem *base @@     got void * @@
   drivers/hwtracing/coresight/ultrasoc-smb.c:463:14: sparse:     expected void [noderef] __iomem *base
   drivers/hwtracing/coresight/ultrasoc-smb.c:463:14: sparse:     got void *

vim +/__iomem +113 drivers/hwtracing/coresight/ultrasoc-smb.c

    89	
    90	static ssize_t smb_read(struct file *file, char __user *data, size_t len,
    91				loff_t *ppos)
    92	{
    93		struct smb_drv_data *drvdata = container_of(file->private_data,
    94						struct smb_drv_data, miscdev);
    95		struct smb_data_buffer *sdb = &drvdata->sdb;
    96		struct device *dev = &drvdata->csdev->dev;
    97		ssize_t to_copy = 0;
    98	
    99		mutex_lock(&drvdata->mutex);
   100	
   101		if (!sdb->data_size) {
   102			smb_update_data_size(drvdata);
   103			if (!sdb->data_size)
   104				goto out;
   105		}
   106	
   107		to_copy = min(sdb->data_size, len);
   108	
   109		/* Copy parts of trace data when read pointer wrap around SMB buffer */
   110		if (sdb->rd_offset + to_copy > sdb->buf_size)
   111			to_copy = sdb->buf_size - sdb->rd_offset;
   112	
 > 113		if (copy_to_user(data, (void *)sdb->buf_base + sdb->rd_offset,
   114				 to_copy)) {
   115			dev_dbg(dev, "Failed to copy data to user\n");
   116			to_copy = -EFAULT;
   117			goto out;
   118		}
   119	
   120		*ppos += to_copy;
   121		sdb->data_size -= to_copy;
   122		sdb->rd_offset += to_copy;
   123		sdb->rd_offset %= sdb->buf_size;
   124		writel(sdb->start_addr + sdb->rd_offset,
   125		       drvdata->base + SMB_LB_RD_ADDR_REG);
   126		dev_dbg(dev, "%zu bytes copied\n", to_copy);
   127	out:
   128		if (!sdb->data_size)
   129			smb_reset_buffer_status(drvdata);
   130		mutex_unlock(&drvdata->mutex);
   131	
   132		return to_copy;
   133	}
   134
  
Suzuki K Poulose Nov. 14, 2022, 10:49 a.m. UTC | #6
On 11/11/2022 22:06, kernel test robot wrote:
> Hi Junhao,
> 
> I love your patch! Perhaps something to improve:
> 
> [auto build test WARNING on soc/for-next]
> [also build test WARNING on linus/master v6.1-rc4 next-20221111]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Junhao-He/Add-support-for-UltraSoc-System-Memory-Buffer/20221109-215158
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
> patch link:    https://lore.kernel.org/r/20221109135008.9485-2-hejunhao3%40huawei.com
> patch subject: [PATCH v12 1/2] drivers/coresight: Add UltraSoc System Memory Buffer driver
> config: arm64-randconfig-s033-20221111
> compiler: aarch64-linux-gcc (GCC) 12.1.0
> reproduce:
>          wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>          chmod +x ~/bin/make.cross
>          # apt-get install sparse
>          # sparse version: v0.6.4-39-gce1a6720-dirty
>          # https://github.com/intel-lab-lkp/linux/commit/49ede03fd874b65deca5382ea84c3377188136da
>          git remote add linux-review https://github.com/intel-lab-lkp/linux
>          git fetch --no-tags linux-review Junhao-He/Add-support-for-UltraSoc-System-Memory-Buffer/20221109-215158
>          git checkout 49ede03fd874b65deca5382ea84c3377188136da
>          # save the config file
>          mkdir build_dir && cp config build_dir/.config
>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/hwtracing/coresight/
> 
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@intel.com>
> 
> sparse warnings: (new ones prefixed by >>)
>>> drivers/hwtracing/coresight/ultrasoc-smb.c:113:33: sparse: sparse: cast removes address space '__iomem' of expression
>>> drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void const * @@     got void [noderef] __iomem * @@
>     drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse:     expected void const *
>     drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse:     got void [noderef] __iomem *
>>> drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void const * @@     got void [noderef] __iomem * @@
>     drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse:     expected void const *
>     drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse:     got void [noderef] __iomem *
>     drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse: sparse: incorrect type in argument 2 (different address spaces) @@     expected void const * @@     got void [noderef] __iomem * @@
>     drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse:     expected void const *
>     drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse:     got void [noderef] __iomem *
>>> drivers/hwtracing/coresight/ultrasoc-smb.c:463:14: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected void [noderef] __iomem *base @@     got void * @@
>     drivers/hwtracing/coresight/ultrasoc-smb.c:463:14: sparse:     expected void [noderef] __iomem *base
>     drivers/hwtracing/coresight/ultrasoc-smb.c:463:14: sparse:     got void *
> 
> vim +/__iomem +113 drivers/hwtracing/coresight/ultrasoc-smb.c
> 
>      89	
>      90	static ssize_t smb_read(struct file *file, char __user *data, size_t len,
>      91				loff_t *ppos)
>      92	{
>      93		struct smb_drv_data *drvdata = container_of(file->private_data,
>      94						struct smb_drv_data, miscdev);
>      95		struct smb_data_buffer *sdb = &drvdata->sdb;
>      96		struct device *dev = &drvdata->csdev->dev;
>      97		ssize_t to_copy = 0;
>      98	
>      99		mutex_lock(&drvdata->mutex);
>     100	
>     101		if (!sdb->data_size) {
>     102			smb_update_data_size(drvdata);
>     103			if (!sdb->data_size)
>     104				goto out;
>     105		}
>     106	
>     107		to_copy = min(sdb->data_size, len);
>     108	
>     109		/* Copy parts of trace data when read pointer wrap around SMB buffer */
>     110		if (sdb->rd_offset + to_copy > sdb->buf_size)
>     111			to_copy = sdb->buf_size - sdb->rd_offset;
>     112	
>   > 113		if (copy_to_user(data, (void *)sdb->buf_base + sdb->rd_offset,
>     114				 to_copy)) {
>     115			dev_dbg(dev, "Failed to copy data to user\n");
>     116			to_copy = -EFAULT;
>     117			goto out;
>     118		}
>     119	

Please could we replace this with copy_to_user_fromio() ?

Suzuki
  
hejunhao Nov. 14, 2022, 1:06 p.m. UTC | #7
On 2022/11/14 18:49, Suzuki K Poulose wrote:
> On 11/11/2022 22:06, kernel test robot wrote:
>> Hi Junhao,
>>
>> I love your patch! Perhaps something to improve:
>>
>> [auto build test WARNING on soc/for-next]
>> [also build test WARNING on linus/master v6.1-rc4 next-20221111]
>> [If your patch is applied to the wrong git tree, kindly drop us a note.
>> And when submitting patch, we suggest to use '--base' as documented in
>> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>>
>> url: 
>> https://github.com/intel-lab-lkp/linux/commits/Junhao-He/Add-support-for-UltraSoc-System-Memory-Buffer/20221109-215158
>> base: https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git 
>> for-next
>> patch link: 
>> https://lore.kernel.org/r/20221109135008.9485-2-hejunhao3%40huawei.com
>> patch subject: [PATCH v12 1/2] drivers/coresight: Add UltraSoc System 
>> Memory Buffer driver
>> config: arm64-randconfig-s033-20221111
>> compiler: aarch64-linux-gcc (GCC) 12.1.0
>> reproduce:
>>          wget 
>> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross 
>> -O ~/bin/make.cross
>>          chmod +x ~/bin/make.cross
>>          # apt-get install sparse
>>          # sparse version: v0.6.4-39-gce1a6720-dirty
>>          # 
>> https://github.com/intel-lab-lkp/linux/commit/49ede03fd874b65deca5382ea84c3377188136da
>>          git remote add linux-review 
>> https://github.com/intel-lab-lkp/linux
>>          git fetch --no-tags linux-review 
>> Junhao-He/Add-support-for-UltraSoc-System-Memory-Buffer/20221109-215158
>>          git checkout 49ede03fd874b65deca5382ea84c3377188136da
>>          # save the config file
>>          mkdir build_dir && cp config build_dir/.config
>>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 
>> make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' 
>> O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/hwtracing/coresight/
>>
>> If you fix the issue, kindly add following tag where applicable
>> | Reported-by: kernel test robot <lkp@intel.com>
>>
>> sparse warnings: (new ones prefixed by >>)
>>>> drivers/hwtracing/coresight/ultrasoc-smb.c:113:33: sparse: sparse: 
>>>> cast removes address space '__iomem' of expression
>>>> drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse: sparse: 
>>>> incorrect type in argument 1 (different address spaces) @@     
>>>> expected void const * @@     got void [noderef] __iomem * @@
>>     drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse:     
>> expected void const *
>>     drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse:     
>> got void [noderef] __iomem *
>>>> drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse: sparse: 
>>>> incorrect type in argument 1 (different address spaces) @@     
>>>> expected void const * @@     got void [noderef] __iomem * @@
>>     drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse:     
>> expected void const *
>>     drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse:     
>> got void [noderef] __iomem *
>>     drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse: 
>> sparse: incorrect type in argument 2 (different address spaces) 
>> @@     expected void const * @@     got void [noderef] __iomem * @@
>>     drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse:     
>> expected void const *
>>     drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse:     
>> got void [noderef] __iomem *
>>>> drivers/hwtracing/coresight/ultrasoc-smb.c:463:14: sparse: sparse: 
>>>> incorrect type in assignment (different address spaces) @@     
>>>> expected void [noderef] __iomem *base @@     got void * @@
>>     drivers/hwtracing/coresight/ultrasoc-smb.c:463:14: sparse:     
>> expected void [noderef] __iomem *base
>>     drivers/hwtracing/coresight/ultrasoc-smb.c:463:14: sparse:     
>> got void *
>>
>> vim +/__iomem +113 drivers/hwtracing/coresight/ultrasoc-smb.c
>>
>>      89
>>      90    static ssize_t smb_read(struct file *file, char __user 
>> *data, size_t len,
>>      91                loff_t *ppos)
>>      92    {
>>      93        struct smb_drv_data *drvdata = 
>> container_of(file->private_data,
>>      94                        struct smb_drv_data, miscdev);
>>      95        struct smb_data_buffer *sdb = &drvdata->sdb;
>>      96        struct device *dev = &drvdata->csdev->dev;
>>      97        ssize_t to_copy = 0;
>>      98
>>      99        mutex_lock(&drvdata->mutex);
>>     100
>>     101        if (!sdb->data_size) {
>>     102            smb_update_data_size(drvdata);
>>     103            if (!sdb->data_size)
>>     104                goto out;
>>     105        }
>>     106
>>     107        to_copy = min(sdb->data_size, len);
>>     108
>>     109        /* Copy parts of trace data when read pointer wrap 
>> around SMB buffer */
>>     110        if (sdb->rd_offset + to_copy > sdb->buf_size)
>>     111            to_copy = sdb->buf_size - sdb->rd_offset;
>>     112
>>   > 113        if (copy_to_user(data, (void *)sdb->buf_base + 
>> sdb->rd_offset,
>>     114                 to_copy)) {
>>     115            dev_dbg(dev, "Failed to copy data to user\n");
>>     116            to_copy = -EFAULT;
>>     117            goto out;
>>     118        }
>>     119
>
> Please could we replace this with copy_to_user_fromio() ?
>
> Suzuki
>
> _______________________________________________
> CoreSight mailing list -- coresight@lists.linaro.org
> To unsubscribe send an email to coresight-leave@lists.linaro.org
> .
>
Hi Suzuki,

This is a cache memory,  due to improper use of "__iomem" , the sparse 
generates warnings.
I has fix it in v13.
https://lore.kernel.org/lkml/20221114090316.63157-2-hejunhao3@huawei.com/

Best regards,
Junhao.
  
Suzuki K Poulose Nov. 14, 2022, 1:59 p.m. UTC | #8
On 14/11/2022 13:06, hejunhao wrote:
> 
> 
> On 2022/11/14 18:49, Suzuki K Poulose wrote:
>> On 11/11/2022 22:06, kernel test robot wrote:
>>> Hi Junhao,
>>>
>>> I love your patch! Perhaps something to improve:
>>>
>>> [auto build test WARNING on soc/for-next]
>>> [also build test WARNING on linus/master v6.1-rc4 next-20221111]
>>> [If your patch is applied to the wrong git tree, kindly drop us a note.
>>> And when submitting patch, we suggest to use '--base' as documented in
>>> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>>>
>>> url: 
>>> https://github.com/intel-lab-lkp/linux/commits/Junhao-He/Add-support-for-UltraSoc-System-Memory-Buffer/20221109-215158
>>> base: https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git 
>>> for-next
>>> patch link: 
>>> https://lore.kernel.org/r/20221109135008.9485-2-hejunhao3%40huawei.com
>>> patch subject: [PATCH v12 1/2] drivers/coresight: Add UltraSoc System 
>>> Memory Buffer driver
>>> config: arm64-randconfig-s033-20221111
>>> compiler: aarch64-linux-gcc (GCC) 12.1.0
>>> reproduce:
>>>          wget 
>>> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>>>          chmod +x ~/bin/make.cross
>>>          # apt-get install sparse
>>>          # sparse version: v0.6.4-39-gce1a6720-dirty
>>>          # 
>>> https://github.com/intel-lab-lkp/linux/commit/49ede03fd874b65deca5382ea84c3377188136da
>>>          git remote add linux-review 
>>> https://github.com/intel-lab-lkp/linux
>>>          git fetch --no-tags linux-review 
>>> Junhao-He/Add-support-for-UltraSoc-System-Memory-Buffer/20221109-215158
>>>          git checkout 49ede03fd874b65deca5382ea84c3377188136da
>>>          # save the config file
>>>          mkdir build_dir && cp config build_dir/.config
>>>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 
>>> make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' 
>>> O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/hwtracing/coresight/
>>>
>>> If you fix the issue, kindly add following tag where applicable
>>> | Reported-by: kernel test robot <lkp@intel.com>
>>>
>>> sparse warnings: (new ones prefixed by >>)
>>>>> drivers/hwtracing/coresight/ultrasoc-smb.c:113:33: sparse: sparse: 
>>>>> cast removes address space '__iomem' of expression
>>>>> drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse: sparse: 
>>>>> incorrect type in argument 1 (different address spaces) @@ expected 
>>>>> void const * @@     got void [noderef] __iomem * @@
>>>     drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse: 
>>> expected void const *
>>>     drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse: got 
>>> void [noderef] __iomem *
>>>>> drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse: sparse: 
>>>>> incorrect type in argument 1 (different address spaces) @@ expected 
>>>>> void const * @@     got void [noderef] __iomem * @@
>>>     drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse: 
>>> expected void const *
>>>     drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse: got 
>>> void [noderef] __iomem *
>>>     drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse: 
>>> sparse: incorrect type in argument 2 (different address spaces) 
>>> @@     expected void const * @@     got void [noderef] __iomem * @@
>>>     drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse: 
>>> expected void const *
>>>     drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse: got 
>>> void [noderef] __iomem *
>>>>> drivers/hwtracing/coresight/ultrasoc-smb.c:463:14: sparse: sparse: 
>>>>> incorrect type in assignment (different address spaces) @@ expected 
>>>>> void [noderef] __iomem *base @@     got void * @@
>>>     drivers/hwtracing/coresight/ultrasoc-smb.c:463:14: sparse: 
>>> expected void [noderef] __iomem *base
>>>     drivers/hwtracing/coresight/ultrasoc-smb.c:463:14: sparse: got 
>>> void *
>>>
>>> vim +/__iomem +113 drivers/hwtracing/coresight/ultrasoc-smb.c
>>>
>>>      89
>>>      90    static ssize_t smb_read(struct file *file, char __user 
>>> *data, size_t len,
>>>      91                loff_t *ppos)
>>>      92    {
>>>      93        struct smb_drv_data *drvdata = 
>>> container_of(file->private_data,
>>>      94                        struct smb_drv_data, miscdev);
>>>      95        struct smb_data_buffer *sdb = &drvdata->sdb;
>>>      96        struct device *dev = &drvdata->csdev->dev;
>>>      97        ssize_t to_copy = 0;
>>>      98
>>>      99        mutex_lock(&drvdata->mutex);
>>>     100
>>>     101        if (!sdb->data_size) {
>>>     102            smb_update_data_size(drvdata);
>>>     103            if (!sdb->data_size)
>>>     104                goto out;
>>>     105        }
>>>     106
>>>     107        to_copy = min(sdb->data_size, len);
>>>     108
>>>     109        /* Copy parts of trace data when read pointer wrap 
>>> around SMB buffer */
>>>     110        if (sdb->rd_offset + to_copy > sdb->buf_size)
>>>     111            to_copy = sdb->buf_size - sdb->rd_offset;
>>>     112
>>>   > 113        if (copy_to_user(data, (void *)sdb->buf_base + 
>>> sdb->rd_offset,
>>>     114                 to_copy)) {
>>>     115            dev_dbg(dev, "Failed to copy data to user\n");
>>>     116            to_copy = -EFAULT;
>>>     117            goto out;
>>>     118        }
>>>     119
>>
>> Please could we replace this with copy_to_user_fromio() ?
>>
>> Suzuki
>>
>> _______________________________________________
>> CoreSight mailing list -- coresight@lists.linaro.org
>> To unsubscribe send an email to coresight-leave@lists.linaro.org
>> .
>>
> Hi Suzuki,
> 
> This is a cache memory,  due to improper use of "__iomem" , the sparse 
> generates warnings.
> I has fix it in v13.
> https://lore.kernel.org/lkml/20221114090316.63157-2-hejunhao3@huawei.com/

I don't see a ChangeLog mentioning that this change. Please make sure
you accurately describe the changes (especially fixes like these).

Thanks
Suzuki

> 
> Best regards,
> Junhao.
> 
> 
>
  
hejunhao Nov. 15, 2022, 7:11 a.m. UTC | #9
On 2022/11/14 21:59, Suzuki K Poulose wrote:
> On 14/11/2022 13:06, hejunhao wrote:
>>
>>
>> On 2022/11/14 18:49, Suzuki K Poulose wrote:
>>> On 11/11/2022 22:06, kernel test robot wrote:
>>>> Hi Junhao,
>>>>
>>>> I love your patch! Perhaps something to improve:
>>>>
>>>> [auto build test WARNING on soc/for-next]
>>>> [also build test WARNING on linus/master v6.1-rc4 next-20221111]
>>>> [If your patch is applied to the wrong git tree, kindly drop us a 
>>>> note.
>>>> And when submitting patch, we suggest to use '--base' as documented in
>>>> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>>>>
>>>> url: 
>>>> https://github.com/intel-lab-lkp/linux/commits/Junhao-He/Add-support-for-UltraSoc-System-Memory-Buffer/20221109-215158
>>>> base: https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git 
>>>> for-next
>>>> patch link: 
>>>> https://lore.kernel.org/r/20221109135008.9485-2-hejunhao3%40huawei.com
>>>> patch subject: [PATCH v12 1/2] drivers/coresight: Add UltraSoc 
>>>> System Memory Buffer driver
>>>> config: arm64-randconfig-s033-20221111
>>>> compiler: aarch64-linux-gcc (GCC) 12.1.0
>>>> reproduce:
>>>>          wget 
>>>> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross 
>>>> -O ~/bin/make.cross
>>>>          chmod +x ~/bin/make.cross
>>>>          # apt-get install sparse
>>>>          # sparse version: v0.6.4-39-gce1a6720-dirty
>>>>          # 
>>>> https://github.com/intel-lab-lkp/linux/commit/49ede03fd874b65deca5382ea84c3377188136da
>>>>          git remote add linux-review 
>>>> https://github.com/intel-lab-lkp/linux
>>>>          git fetch --no-tags linux-review 
>>>> Junhao-He/Add-support-for-UltraSoc-System-Memory-Buffer/20221109-215158 
>>>>
>>>>          git checkout 49ede03fd874b65deca5382ea84c3377188136da
>>>>          # save the config file
>>>>          mkdir build_dir && cp config build_dir/.config
>>>>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 
>>>> make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' 
>>>> O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/hwtracing/coresight/
>>>>
>>>> If you fix the issue, kindly add following tag where applicable
>>>> | Reported-by: kernel test robot <lkp@intel.com>
>>>>
>>>> sparse warnings: (new ones prefixed by >>)
>>>>>> drivers/hwtracing/coresight/ultrasoc-smb.c:113:33: sparse: 
>>>>>> sparse: cast removes address space '__iomem' of expression
>>>>>> drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse: 
>>>>>> sparse: incorrect type in argument 1 (different address spaces) 
>>>>>> @@ expected void const * @@     got void [noderef] __iomem * @@
>>>>     drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse: 
>>>> expected void const *
>>>>     drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse: got 
>>>> void [noderef] __iomem *
>>>>>> drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse: 
>>>>>> sparse: incorrect type in argument 1 (different address spaces) 
>>>>>> @@ expected void const * @@     got void [noderef] __iomem * @@
>>>>     drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse: 
>>>> expected void const *
>>>>     drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse: got 
>>>> void [noderef] __iomem *
>>>>     drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse: 
>>>> sparse: incorrect type in argument 2 (different address spaces) 
>>>> @@     expected void const * @@     got void [noderef] __iomem * @@
>>>>     drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse: 
>>>> expected void const *
>>>>     drivers/hwtracing/coresight/ultrasoc-smb.c:354:17: sparse: got 
>>>> void [noderef] __iomem *
>>>>>> drivers/hwtracing/coresight/ultrasoc-smb.c:463:14: sparse: 
>>>>>> sparse: incorrect type in assignment (different address spaces) 
>>>>>> @@ expected void [noderef] __iomem *base @@     got void * @@
>>>>     drivers/hwtracing/coresight/ultrasoc-smb.c:463:14: sparse: 
>>>> expected void [noderef] __iomem *base
>>>>     drivers/hwtracing/coresight/ultrasoc-smb.c:463:14: sparse: got 
>>>> void *
>>>>
>>>> vim +/__iomem +113 drivers/hwtracing/coresight/ultrasoc-smb.c
>>>>
>>>>      89
>>>>      90    static ssize_t smb_read(struct file *file, char __user 
>>>> *data, size_t len,
>>>>      91                loff_t *ppos)
>>>>      92    {
>>>>      93        struct smb_drv_data *drvdata = 
>>>> container_of(file->private_data,
>>>>      94                        struct smb_drv_data, miscdev);
>>>>      95        struct smb_data_buffer *sdb = &drvdata->sdb;
>>>>      96        struct device *dev = &drvdata->csdev->dev;
>>>>      97        ssize_t to_copy = 0;
>>>>      98
>>>>      99        mutex_lock(&drvdata->mutex);
>>>>     100
>>>>     101        if (!sdb->data_size) {
>>>>     102            smb_update_data_size(drvdata);
>>>>     103            if (!sdb->data_size)
>>>>     104                goto out;
>>>>     105        }
>>>>     106
>>>>     107        to_copy = min(sdb->data_size, len);
>>>>     108
>>>>     109        /* Copy parts of trace data when read pointer wrap 
>>>> around SMB buffer */
>>>>     110        if (sdb->rd_offset + to_copy > sdb->buf_size)
>>>>     111            to_copy = sdb->buf_size - sdb->rd_offset;
>>>>     112
>>>>   > 113        if (copy_to_user(data, (void *)sdb->buf_base + 
>>>> sdb->rd_offset,
>>>>     114                 to_copy)) {
>>>>     115            dev_dbg(dev, "Failed to copy data to user\n");
>>>>     116            to_copy = -EFAULT;
>>>>     117            goto out;
>>>>     118        }
>>>>     119
>>>
>>> Please could we replace this with copy_to_user_fromio() ?
>>>
>>> Suzuki
>>>
>>> _______________________________________________
>>> CoreSight mailing list -- coresight@lists.linaro.org
>>> To unsubscribe send an email to coresight-leave@lists.linaro.org
>>> .
>>>
>> Hi Suzuki,
>>
>> This is a cache memory,  due to improper use of "__iomem" , the 
>> sparse generates warnings.
>> I has fix it in v13.
>> https://lore.kernel.org/lkml/20221114090316.63157-2-hejunhao3@huawei.com/ 
>>
>
> I don't see a ChangeLog mentioning that this change. Please make sure
> you accurately describe the changes (especially fixes like these).
>
> Thanks
> Suzuki
>
Hi Suzuki,

Sure, I will fill in the v13 change log.
Thanks for your comments. Any comment or suggestion is appreciated.

Best regards,
Junhao.
>>
>> Best regards,
>> Junhao.
>>
>>
>>
>
> _______________________________________________
> CoreSight mailing list -- coresight@lists.linaro.org
> To unsubscribe send an email to coresight-leave@lists.linaro.org
  

Patch

diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
index 45c1eb5dfcb7..cb17c207a728 100644
--- a/drivers/hwtracing/coresight/Kconfig
+++ b/drivers/hwtracing/coresight/Kconfig
@@ -201,4 +201,15 @@  config CORESIGHT_TRBE
 
 	  To compile this driver as a module, choose M here: the module will be
 	  called coresight-trbe.
+
+config ULTRASOC_SMB
+	tristate "Ultrasoc system memory buffer drivers"
+	depends on ARM64 && CORESIGHT_LINKS_AND_SINKS
+	help
+	  This driver provides support for the Ultrasoc system memory buffer (SMB).
+	  SMB is responsible for receiving the trace data from Coresight ETM devices
+	  and storing them to a system buffer.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called ultrasoc-smb.
 endif
diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
index b6c4a48140ec..344dba8d6ff8 100644
--- a/drivers/hwtracing/coresight/Makefile
+++ b/drivers/hwtracing/coresight/Makefile
@@ -27,3 +27,4 @@  obj-$(CONFIG_CORESIGHT_CTI) += coresight-cti.o
 obj-$(CONFIG_CORESIGHT_TRBE) += coresight-trbe.o
 coresight-cti-y := coresight-cti-core.o	coresight-cti-platform.o \
 		   coresight-cti-sysfs.o
+obj-$(CONFIG_ULTRASOC_SMB) += ultrasoc-smb.o
diff --git a/drivers/hwtracing/coresight/ultrasoc-smb.c b/drivers/hwtracing/coresight/ultrasoc-smb.c
new file mode 100644
index 000000000000..ea2552a98d28
--- /dev/null
+++ b/drivers/hwtracing/coresight/ultrasoc-smb.c
@@ -0,0 +1,635 @@ 
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+/*
+ * Siemens System Memory Buffer driver.
+ * Copyright(c) 2022, HiSilicon Limited.
+ */
+
+#include <linux/atomic.h>
+#include <linux/acpi.h>
+#include <linux/circ_buf.h>
+#include <linux/err.h>
+#include <linux/fs.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/platform_device.h>
+
+#include "coresight-etm-perf.h"
+#include "coresight-priv.h"
+#include "ultrasoc-smb.h"
+
+DEFINE_CORESIGHT_DEVLIST(sink_devs, "ultra_smb");
+
+#define ULTRASOC_SMB_DSM_UUID	"82ae1283-7f6a-4cbe-aa06-53e8fb24db18"
+
+static bool smb_buffer_is_empty(struct smb_drv_data *drvdata)
+{
+	u32 buf_status = readl(drvdata->base + SMB_LB_INT_STS_REG);
+
+	return !FIELD_PREP(SMB_LB_INT_STS_NOT_EMPTY_MSK, buf_status);
+}
+
+static void smb_buffer_sync_status(struct smb_drv_data *drvdata)
+{
+	struct smb_data_buffer *sdb = &drvdata->sdb;
+
+	sdb->wr_offset = readl(drvdata->base + SMB_LB_WR_ADDR_REG) - sdb->start_addr;
+	sdb->rd_offset = readl(drvdata->base + SMB_LB_RD_ADDR_REG) - sdb->start_addr;
+	sdb->full = sdb->wr_offset == sdb->rd_offset && !smb_buffer_is_empty(drvdata);
+}
+
+static void smb_reset_buffer_status(struct smb_drv_data *drvdata)
+{
+	writel(SMB_LB_INT_BUF_STS_RESET, drvdata->base + SMB_LB_INT_STS_REG);
+}
+
+/* Purge data remaining in hardware path in case them influence next trace */
+static void smb_purge_data(struct smb_drv_data *drvdata)
+{
+	writel(SMB_LB_PURGE_PURGED, drvdata->base + SMB_LB_PURGE_REG);
+}
+
+static void smb_update_data_size(struct smb_drv_data *drvdata)
+{
+	struct smb_data_buffer *sdb = &drvdata->sdb;
+
+	smb_purge_data(drvdata);
+	smb_buffer_sync_status(drvdata);
+	if (sdb->full) {
+		sdb->data_size = sdb->buf_size;
+		return;
+	}
+
+	sdb->data_size = CIRC_CNT(sdb->wr_offset, sdb->rd_offset, sdb->buf_size);
+}
+
+static int smb_open(struct inode *inode, struct file *file)
+{
+	struct smb_drv_data *drvdata = container_of(file->private_data,
+					struct smb_drv_data, miscdev);
+	int ret = 0;
+
+	mutex_lock(&drvdata->mutex);
+
+	if (local_read(&drvdata->reading)) {
+		ret = -EBUSY;
+		goto out;
+	}
+
+	if (atomic_read(drvdata->csdev->refcnt)) {
+		ret = -EBUSY;
+		goto out;
+	}
+
+	local_set(&drvdata->reading, 1);
+out:
+	mutex_unlock(&drvdata->mutex);
+
+	return ret;
+}
+
+static ssize_t smb_read(struct file *file, char __user *data, size_t len,
+			loff_t *ppos)
+{
+	struct smb_drv_data *drvdata = container_of(file->private_data,
+					struct smb_drv_data, miscdev);
+	struct smb_data_buffer *sdb = &drvdata->sdb;
+	struct device *dev = &drvdata->csdev->dev;
+	ssize_t to_copy = 0;
+
+	mutex_lock(&drvdata->mutex);
+
+	if (!sdb->data_size) {
+		smb_update_data_size(drvdata);
+		if (!sdb->data_size)
+			goto out;
+	}
+
+	to_copy = min(sdb->data_size, len);
+
+	/* Copy parts of trace data when read pointer wrap around SMB buffer */
+	if (sdb->rd_offset + to_copy > sdb->buf_size)
+		to_copy = sdb->buf_size - sdb->rd_offset;
+
+	if (copy_to_user(data, (void *)sdb->buf_base + sdb->rd_offset,
+			 to_copy)) {
+		dev_dbg(dev, "Failed to copy data to user\n");
+		to_copy = -EFAULT;
+		goto out;
+	}
+
+	*ppos += to_copy;
+	sdb->data_size -= to_copy;
+	sdb->rd_offset += to_copy;
+	sdb->rd_offset %= sdb->buf_size;
+	writel(sdb->start_addr + sdb->rd_offset,
+	       drvdata->base + SMB_LB_RD_ADDR_REG);
+	dev_dbg(dev, "%zu bytes copied\n", to_copy);
+out:
+	if (!sdb->data_size)
+		smb_reset_buffer_status(drvdata);
+	mutex_unlock(&drvdata->mutex);
+
+	return to_copy;
+}
+
+static int smb_release(struct inode *inode, struct file *file)
+{
+	struct smb_drv_data *drvdata = container_of(file->private_data,
+					struct smb_drv_data, miscdev);
+
+	mutex_lock(&drvdata->mutex);
+	local_set(&drvdata->reading, 0);
+	mutex_unlock(&drvdata->mutex);
+
+	return 0;
+}
+
+static const struct file_operations smb_fops = {
+	.owner		= THIS_MODULE,
+	.open		= smb_open,
+	.read		= smb_read,
+	.release	= smb_release,
+	.llseek		= no_llseek,
+};
+
+static ssize_t buf_size_show(struct device *dev, struct device_attribute *attr,
+			     char *buf)
+{
+	struct smb_drv_data *drvdata = dev_get_drvdata(dev->parent);
+
+	return sysfs_emit(buf, "0x%lx\n", drvdata->sdb.buf_size);
+}
+static DEVICE_ATTR_RO(buf_size);
+
+static struct attribute *smb_sink_attrs[] = {
+	coresight_simple_reg32(read_pos, SMB_LB_RD_ADDR_REG),
+	coresight_simple_reg32(write_pos, SMB_LB_WR_ADDR_REG),
+	coresight_simple_reg32(buf_status, SMB_LB_INT_STS_REG),
+	&dev_attr_buf_size.attr,
+	NULL
+};
+
+static const struct attribute_group smb_sink_group = {
+	.attrs = smb_sink_attrs,
+	.name = "mgmt",
+};
+
+static const struct attribute_group *smb_sink_groups[] = {
+	&smb_sink_group,
+	NULL
+};
+
+static void smb_enable_hw(struct smb_drv_data *drvdata)
+{
+	writel(SMB_GLB_EN_HW_ENABLE, drvdata->base + SMB_GLB_EN_REG);
+}
+
+static void smb_disable_hw(struct smb_drv_data *drvdata)
+{
+	writel(0x0, drvdata->base + SMB_GLB_EN_REG);
+}
+
+static void smb_enable_sysfs(struct coresight_device *csdev)
+{
+	struct smb_drv_data *drvdata = dev_get_drvdata(csdev->dev.parent);
+
+	if (drvdata->mode != CS_MODE_DISABLED)
+		return;
+
+	smb_enable_hw(drvdata);
+	drvdata->mode = CS_MODE_SYSFS;
+}
+
+static int smb_enable_perf(struct coresight_device *csdev, void *data)
+{
+	struct smb_drv_data *drvdata = dev_get_drvdata(csdev->dev.parent);
+	struct perf_output_handle *handle = data;
+	struct cs_buffers *buf = etm_perf_sink_config(handle);
+	pid_t pid;
+
+	if (!buf)
+		return -EINVAL;
+
+	/* Get a handle on the pid of the target process */
+	pid = buf->pid;
+
+	/* Device is already in used by other session */
+	if (drvdata->pid != -1 && drvdata->pid != pid)
+		return -EBUSY;
+
+	if (drvdata->pid == -1) {
+		smb_enable_hw(drvdata);
+		drvdata->pid = pid;
+		drvdata->mode = CS_MODE_PERF;
+	}
+
+	return 0;
+}
+
+static int smb_enable(struct coresight_device *csdev, u32 mode, void *data)
+{
+	struct smb_drv_data *drvdata = dev_get_drvdata(csdev->dev.parent);
+	int ret = 0;
+
+	mutex_lock(&drvdata->mutex);
+
+	/* Do nothing, the trace data is reading by other interface now */
+	if (local_read(&drvdata->reading)) {
+		ret = -EBUSY;
+		goto out;
+	}
+
+	/* Do nothing, the SMB is already enabled as other mode */
+	if (drvdata->mode != CS_MODE_DISABLED && drvdata->mode != mode) {
+		ret = -EBUSY;
+		goto out;
+	}
+
+	switch (mode) {
+	case CS_MODE_SYSFS:
+		smb_enable_sysfs(csdev);
+		break;
+	case CS_MODE_PERF:
+		ret = smb_enable_perf(csdev, data);
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
+	if (ret)
+		goto out;
+
+	atomic_inc(csdev->refcnt);
+
+	dev_dbg(&csdev->dev, "Ultrasoc SMB enabled\n");
+out:
+	mutex_unlock(&drvdata->mutex);
+
+	return ret;
+}
+
+static int smb_disable(struct coresight_device *csdev)
+{
+	struct smb_drv_data *drvdata = dev_get_drvdata(csdev->dev.parent);
+	int ret = 0;
+
+	mutex_lock(&drvdata->mutex);
+
+	if (local_read(&drvdata->reading)) {
+		ret = -EBUSY;
+		goto out;
+	}
+
+	if (atomic_dec_return(csdev->refcnt)) {
+		ret = -EBUSY;
+		goto out;
+	}
+
+	/* Complain if we (somehow) got out of sync */
+	WARN_ON_ONCE(drvdata->mode == CS_MODE_DISABLED);
+
+	smb_disable_hw(drvdata);
+	smb_purge_data(drvdata);
+
+	/* Dissociate from the target process. */
+	drvdata->pid = -1;
+	drvdata->mode = CS_MODE_DISABLED;
+
+	dev_dbg(&csdev->dev, "Ultrasoc SMB disabled\n");
+out:
+	mutex_unlock(&drvdata->mutex);
+
+	return ret;
+}
+
+static void *smb_alloc_buffer(struct coresight_device *csdev,
+			      struct perf_event *event, void **pages,
+			      int nr_pages, bool overwrite)
+{
+	struct cs_buffers *buf;
+	int node;
+
+	node = (event->cpu == -1) ? NUMA_NO_NODE : cpu_to_node(event->cpu);
+	buf = kzalloc_node(sizeof(struct cs_buffers), GFP_KERNEL, node);
+	if (!buf)
+		return NULL;
+
+	buf->snapshot = overwrite;
+	buf->nr_pages = nr_pages;
+	buf->data_pages = pages;
+	buf->pid = task_pid_nr(event->owner);
+
+	return buf;
+}
+
+static void smb_free_buffer(void *config)
+{
+	struct cs_buffers *buf = config;
+
+	kfree(buf);
+}
+
+static void smb_sync_perf_buffer(struct smb_drv_data *drvdata,
+				 struct cs_buffers *buf,
+				 unsigned long head,
+				 unsigned long data_size)
+{
+	struct smb_data_buffer *sdb = &drvdata->sdb;
+	char **dst_pages = (char **)buf->data_pages;
+	unsigned long to_copy;
+	long pg_idx, pg_offset;
+
+	pg_idx = head >> PAGE_SHIFT;
+	pg_offset = head & (PAGE_SIZE - 1);
+
+	while (data_size) {
+		unsigned long pg_space = PAGE_SIZE - pg_offset;
+
+		/* Copy parts of trace data when read pointer wrap around */
+		if (sdb->rd_offset + pg_space > sdb->buf_size)
+			to_copy = sdb->buf_size - sdb->rd_offset;
+		else
+			to_copy = min(data_size, pg_space);
+
+		memcpy(dst_pages[pg_idx] + pg_offset,
+			      sdb->buf_base + sdb->rd_offset, to_copy);
+
+		pg_offset += to_copy;
+		if (pg_offset >= PAGE_SIZE) {
+			pg_offset = 0;
+			pg_idx++;
+			pg_idx %= buf->nr_pages;
+		}
+		data_size -= to_copy;
+		sdb->rd_offset += to_copy;
+		sdb->rd_offset %= sdb->buf_size;
+	}
+
+	sdb->data_size = 0;
+	writel(sdb->start_addr + sdb->rd_offset, drvdata->base + SMB_LB_RD_ADDR_REG);
+
+	/*
+	 * Data remained in link cannot be purged when SMB is full, so
+	 * synchronize the read pointer to write pointer, to make sure
+	 * these remained data won't influence next trace.
+	 */
+	if (sdb->full) {
+		smb_purge_data(drvdata);
+		writel(readl(drvdata->base + SMB_LB_WR_ADDR_REG),
+		       drvdata->base + SMB_LB_RD_ADDR_REG);
+	}
+	smb_reset_buffer_status(drvdata);
+}
+
+static unsigned long smb_update_buffer(struct coresight_device *csdev,
+				       struct perf_output_handle *handle,
+				       void *sink_config)
+{
+	struct smb_drv_data *drvdata = dev_get_drvdata(csdev->dev.parent);
+	struct smb_data_buffer *sdb = &drvdata->sdb;
+	struct cs_buffers *buf = sink_config;
+	unsigned long data_size = 0;
+	bool lost = false;
+
+	if (!buf)
+		return 0;
+
+	mutex_lock(&drvdata->mutex);
+
+	/* Don't do anything if another tracer is using this sink. */
+	if (atomic_read(csdev->refcnt) != 1)
+		goto out;
+
+	smb_disable_hw(drvdata);
+	smb_update_data_size(drvdata);
+	data_size = sdb->data_size;
+
+	/*
+	 * The SMB buffer may be bigger than the space available in the
+	 * perf ring buffer (handle->size). If so advance the offset so
+	 * that we get the latest trace data.
+	 */
+	if (data_size > handle->size) {
+		sdb->rd_offset += data_size - handle->size;
+		sdb->rd_offset %= sdb->buf_size;
+		data_size = handle->size;
+		lost = true;
+	}
+
+	smb_sync_perf_buffer(drvdata, buf, handle->head, data_size);
+	if (!buf->snapshot && lost)
+		perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
+
+	smb_enable_hw(drvdata);
+out:
+	mutex_unlock(&drvdata->mutex);
+
+	return data_size;
+}
+
+static const struct coresight_ops_sink smb_cs_ops = {
+	.enable		= smb_enable,
+	.disable	= smb_disable,
+	.alloc_buffer	= smb_alloc_buffer,
+	.free_buffer	= smb_free_buffer,
+	.update_buffer	= smb_update_buffer,
+};
+
+static const struct coresight_ops cs_ops = {
+	.sink_ops	= &smb_cs_ops,
+};
+
+static int smb_init_data_buffer(struct platform_device *pdev,
+				struct smb_data_buffer *sdb)
+{
+	struct resource *res;
+	void __iomem *base;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, SMB_BUF_ADDR_RES);
+	if (IS_ERR(res)) {
+		dev_err(&pdev->dev, "SMB device failed to get resource\n");
+		return -EINVAL;
+	}
+
+	sdb->start_addr = FIELD_PREP(SMB_BUF_ADDR_LO_MSK, res->start);
+	sdb->buf_size = resource_size(res);
+	if (sdb->buf_size == 0)
+		return -EINVAL;
+
+	/*
+	 * This is a chunk of memory, use classic mapping with better
+	 * performance.
+	 */
+	base = devm_memremap(&pdev->dev, sdb->start_addr, sdb->buf_size,
+				MEMREMAP_WB);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	sdb->buf_base = base;
+
+	return 0;
+}
+
+static void smb_init_hw(struct smb_drv_data *drvdata)
+{
+	/* First disable SMB and clear the status of SMB buffer */
+	smb_reset_buffer_status(drvdata);
+	smb_disable_hw(drvdata);
+	smb_purge_data(drvdata);
+
+	writel(SMB_LB_CFG_LO_DEFAULT, drvdata->base + SMB_LB_CFG_LO_REG);
+	writel(SMB_LB_CFG_HI_DEFAULT, drvdata->base + SMB_LB_CFG_HI_REG);
+	writel(SMB_GLB_CFG_DEFAULT, drvdata->base + SMB_GLB_CFG_REG);
+	writel(SMB_GLB_INT_CFG, drvdata->base + SMB_GLB_INT_REG);
+	writel(SMB_LB_INT_CTRL_CFG, drvdata->base + SMB_LB_INT_CTRL_REG);
+}
+
+static int smb_register_sink(struct platform_device *pdev,
+			     struct smb_drv_data *drvdata)
+{
+	struct coresight_platform_data *pdata = NULL;
+	struct coresight_desc desc = { 0 };
+	int ret;
+
+	pdata = coresight_get_platform_data(&pdev->dev);
+	if (IS_ERR(pdata))
+		return PTR_ERR(pdata);
+
+	desc.type = CORESIGHT_DEV_TYPE_SINK;
+	desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_BUFFER;
+	desc.ops = &cs_ops;
+	desc.pdata = pdata;
+	desc.dev = &pdev->dev;
+	desc.groups = smb_sink_groups;
+	desc.name = coresight_alloc_device_name(&sink_devs, &pdev->dev);
+	if (!desc.name) {
+		dev_err(&pdev->dev, "Failed to alloc coresight device name");
+		return -ENOMEM;
+	}
+	desc.access = CSDEV_ACCESS_IOMEM(drvdata->base);
+
+	drvdata->csdev = coresight_register(&desc);
+	if (IS_ERR(drvdata->csdev))
+		return PTR_ERR(drvdata->csdev);
+
+	drvdata->miscdev.name = desc.name;
+	drvdata->miscdev.minor = MISC_DYNAMIC_MINOR;
+	drvdata->miscdev.fops = &smb_fops;
+	ret = misc_register(&drvdata->miscdev);
+	if (ret) {
+		coresight_unregister(drvdata->csdev);
+		dev_err(&pdev->dev, "Failed to register misc, ret=%d\n", ret);
+	}
+
+	return ret;
+}
+
+static void smb_unregister_sink(struct smb_drv_data *drvdata)
+{
+	misc_deregister(&drvdata->miscdev);
+	coresight_unregister(drvdata->csdev);
+}
+
+static int smb_config_inport(struct device *dev, bool enable)
+{
+	u64 func = enable ? 1 : 0;
+	union acpi_object *obj;
+	guid_t guid;
+	u64 rev = 0;
+
+	/*
+	 * Using DSM calls to enable/disable ultrasoc hardwares on
+	 * tracing path, to prevent ultrasoc packet format being exposed.
+	 */
+	if (guid_parse(ULTRASOC_SMB_DSM_UUID, &guid)) {
+		dev_err(dev, "Get GUID failed\n");
+		return -EINVAL;
+	}
+
+	obj = acpi_evaluate_dsm(ACPI_HANDLE(dev), &guid, rev, func, NULL);
+	if (!obj)
+		dev_err(dev, "ACPI handle failed\n");
+	else
+		ACPI_FREE(obj);
+
+	return 0;
+}
+
+static int smb_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct smb_drv_data *drvdata;
+	int ret;
+
+	drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
+	if (!drvdata)
+		return -ENOMEM;
+
+	drvdata->base = devm_platform_ioremap_resource(pdev, SMB_REG_ADDR_RES);
+	if (IS_ERR(drvdata->base)) {
+		dev_err(dev, "Failed to ioremap resource\n");
+		return PTR_ERR(drvdata->base);
+	}
+
+	ret = smb_init_data_buffer(pdev, &drvdata->sdb);
+	if (ret) {
+		dev_err(dev, "Failed to init buffer, ret = %d\n", ret);
+		return ret;
+	}
+
+	smb_init_hw(drvdata);
+	mutex_init(&drvdata->mutex);
+	drvdata->pid = -1;
+
+	ret = smb_register_sink(pdev, drvdata);
+	if (ret) {
+		dev_err(dev, "Failed to register SMB sink\n");
+		return ret;
+	}
+
+	ret = smb_config_inport(dev, true);
+	if (ret) {
+		smb_unregister_sink(drvdata);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, drvdata);
+
+	return 0;
+}
+
+static int smb_remove(struct platform_device *pdev)
+{
+	struct smb_drv_data *drvdata = platform_get_drvdata(pdev);
+	int ret;
+
+	ret = smb_config_inport(&pdev->dev, false);
+	if (ret)
+		return ret;
+
+	smb_unregister_sink(drvdata);
+
+	return 0;
+}
+
+static const struct acpi_device_id ultrasoc_smb_acpi_match[] = {
+	{"HISI03A1", 0},
+	{}
+};
+MODULE_DEVICE_TABLE(acpi, ultrasoc_smb_acpi_match);
+
+static struct platform_driver smb_driver = {
+	.driver = {
+		.name = "ultrasoc-smb",
+		.acpi_match_table = ACPI_PTR(ultrasoc_smb_acpi_match),
+		.suppress_bind_attrs = true,
+	},
+	.probe = smb_probe,
+	.remove = smb_remove,
+};
+module_platform_driver(smb_driver);
+
+MODULE_DESCRIPTION("UltraSoc SMB CoreSight driver");
+MODULE_LICENSE("Dual MIT/GPL");
+MODULE_AUTHOR("Jonathan Zhou <jonathan.zhouwen@huawei.com>");
+MODULE_AUTHOR("Qi Liu <liuqi115@huawei.com>");
diff --git a/drivers/hwtracing/coresight/ultrasoc-smb.h b/drivers/hwtracing/coresight/ultrasoc-smb.h
new file mode 100644
index 000000000000..2e2f9f8fe54b
--- /dev/null
+++ b/drivers/hwtracing/coresight/ultrasoc-smb.h
@@ -0,0 +1,116 @@ 
+/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
+/*
+ * Siemens System Memory Buffer driver.
+ * Copyright(c) 2022, HiSilicon Limited.
+ */
+
+#ifndef _ULTRASOC_SMB_H
+#define _ULTRASOC_SMB_H
+
+#include <linux/miscdevice.h>
+#include <linux/mutex.h>
+
+/* Offset of SMB global registers */
+#define SMB_GLB_CFG_REG		0x00
+#define SMB_GLB_EN_REG		0x04
+#define SMB_GLB_INT_REG		0x08
+
+/* Offset of SMB logical buffer registers */
+#define SMB_LB_CFG_LO_REG	0x40
+#define SMB_LB_CFG_HI_REG	0x44
+#define SMB_LB_INT_CTRL_REG	0x48
+#define SMB_LB_INT_STS_REG	0x4c
+#define SMB_LB_RD_ADDR_REG	0x5c
+#define SMB_LB_WR_ADDR_REG	0x60
+#define SMB_LB_PURGE_REG	0x64
+
+/* Set global config register */
+#define SMB_CFG_BURST_LEN_MSK	GENMASK(11, 4)
+#define SMB_CFG_IDLE_PRD_MSK	GENMASK(15, 12)
+#define SMB_CFG_MEM_WR_MSK	GENMASK(21, 16)
+#define SMB_CFG_MEM_RD_MSK	GENMASK(27, 22)
+#define SMB_GLB_CFG_DEFAULT	(FIELD_PREP(SMB_CFG_BURST_LEN_MSK, 0xf) | \
+				 FIELD_PREP(SMB_CFG_IDLE_PRD_MSK, 0xf) | \
+				 FIELD_PREP(SMB_CFG_MEM_WR_MSK, 0x3) | \
+				 FIELD_PREP(SMB_CFG_MEM_RD_MSK, 0x1b))
+
+/* Set global interrupt control register */
+#define SMB_INT_EN		BIT(0)
+#define SMB_INT_PULSE		BIT(1) /* Interrupt type: 1 - Pulse */
+#define SMB_INT_ACT_H		BIT(2) /* Interrupt polarity: 1 - Active high */
+#define SMB_GLB_INT_CFG		(SMB_INT_EN | SMB_INT_PULSE | SMB_INT_ACT_H)
+
+/* Set logical buffer config register lower 32 bits */
+#define SMB_CFG_LO_EN		BIT(0)
+#define SMB_CFG_LO_SINGLE_END	BIT(1)
+#define SMB_CFG_LO_INIT		BIT(8)
+#define SMB_CFG_LO_CONT		BIT(11)
+#define SMB_CFG_LO_FLOW_MSK	GENMASK(19, 16)
+#define SMB_LB_CFG_LO_DEFAULT	(SMB_CFG_LO_EN | SMB_CFG_LO_SINGLE_END | \
+				 SMB_CFG_LO_INIT | SMB_CFG_LO_CONT | \
+				 FIELD_PREP(SMB_CFG_LO_FLOW_MSK, 0xf))
+
+/* Set logical buffer config register upper 32 bits */
+#define SMB_CFG_HI_RANGE_UP_MSK	GENMASK(15, 8)
+#define SMB_LB_CFG_HI_DEFAULT	FIELD_PREP(SMB_CFG_HI_RANGE_UP_MSK, 0xff)
+
+/* Set logical buffer interrupt control register */
+#define SMB_INT_CTRL_EN			BIT(0)
+#define SMB_INT_CTRL_BUF_NOTE_MSK	GENMASK(11, 8)
+#define SMB_LB_INT_CTRL_CFG	(SMB_INT_CTRL_EN | \
+				 FIELD_PREP(SMB_INT_CTRL_BUF_NOTE_MSK, 0xf))
+
+#define SMB_LB_INT_STS_NOT_EMPTY_MSK	BIT(0)
+#define SMB_LB_STS_RESET_MSK		GENMASK(3, 0)
+#define SMB_LB_INT_BUF_STS_RESET	FIELD_PREP(SMB_LB_STS_RESET_MSK, 0xf)
+#define SMB_LB_PURGE_PURGED	BIT(0)
+#define SMB_GLB_EN_HW_ENABLE	BIT(0)
+
+#define SMB_REG_ADDR_RES	0
+#define SMB_BUF_ADDR_RES	1
+#define SMB_BUF_ADDR_LO_MSK	GENMASK(31, 0)
+
+/**
+ * struct smb_data_buffer - Details of the buffer used by SMB
+ * @buf_base:	Memory mapped base address of SMB.
+ * @start_addr:	SMB buffer start Physical address.
+ * @buf_size:	Size of the buffer.
+ * @data_size:	Size of Trace data copy to userspace.
+ * @rd_offset:	Offset of the read pointer in the buffer.
+ * @wr_offset:	Offset of the write pointer in the buffer.
+ * @full:	Trace data overflow.
+ */
+struct smb_data_buffer {
+	void __iomem *buf_base;
+	u32 start_addr;
+	unsigned long buf_size;
+	unsigned long data_size;
+	unsigned long rd_offset;
+	unsigned long wr_offset;
+	bool full;
+};
+
+/**
+ * struct smb_drv_data - specifics associated to an SMB component
+ * @base:	Memory mapped base address for SMB component.
+ * @csdev:	Component vitals needed by the framework.
+ * @sdb:	Data buffer for SMB.
+ * @miscdev:	Specifics to handle "/dev/xyz.smb" entry.
+ * @mutex:	Control data access to one at a time.
+ * @reading:	Synchronise user space access to SMB buffer.
+ * @pid:	Process ID of the process being monitored by the
+ *		session that is using this component.
+ * @mode:	how this SMB is being used, perf mode or sysfs mode.
+ */
+struct smb_drv_data {
+	void __iomem *base;
+	struct coresight_device	*csdev;
+	struct smb_data_buffer sdb;
+	struct miscdevice miscdev;
+	struct mutex mutex;
+	local_t reading;
+	pid_t pid;
+	u32 mode;
+};
+
+#endif