Hi Suzuki, On 2022/6/7 16:33, Suzuki K Poulose wrote:
Hi Qi
On 06/06/2022 14:02, Qi Liu wrote:
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 in system memory.
nit: "CPU instruction 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 Tested-by: JunHao He hejunhao2@hisilicon.com
drivers/hwtracing/coresight/Kconfig | 10 + drivers/hwtracing/coresight/Makefile | 1 + drivers/hwtracing/coresight/ultrasoc-smb.c | 663 +++++++++++++++++++++ drivers/hwtracing/coresight/ultrasoc-smb.h | 110 ++++ 4 files changed, 784 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 514a9b8086e3..4380eb1a0a73 100644 --- a/drivers/hwtracing/coresight/Kconfig +++ b/drivers/hwtracing/coresight/Kconfig
...
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..44b4bceb6f28 --- /dev/null +++ b/drivers/hwtracing/coresight/ultrasoc-smb.c
...
+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); + unsigned long flags; + int ret = 0;
+ spin_lock_irqsave(&drvdata->spinlock, flags);
+ if (local_read(&drvdata->reading)) { + ret = -EBUSY; + goto out; + }
+ if (drvdata->mode == CS_MODE_PERF) { + ret = -EINVAL; + goto out; + }
+out: + spin_unlock_irqrestore(&drvdata->spinlock, flags); + 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; + unsigned long flags; + int to_copy = 0;
+ spin_lock_irqsave(&drvdata->spinlock, flags);
+ if (atomic_read(drvdata->csdev->refcnt)) { + spin_unlock_irqrestore(&drvdata->spinlock, flags); + return -EBUSY; + }
+ if (local_cmpxchg(&drvdata->reading, 0, 1)) { + spin_unlock_irqrestore(&drvdata->spinlock, flags); + return -EBUSY; + }
Shouldn't this be done in smb_open() ? Otherwise, a subsequent read will fail with EBUSY ? i.e, fd = open(/dev/ultra_smb0, ..);
# size of the SMB buffer = 1M, size = read(fd, buf, SZ_4K); /* Do something the buf */ if (size >= 0) size = read(fd, buf, SZ_4K);
^^ This would fail, isn't it ?
Yes, I'll change it to: 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); unsigned long flags; int ret = 0;
spin_lock_irqsave(&drvdata->spinlock, flags);
if (local_read(&drvdata->reading)) { ret = -EBUSY; goto out; }
if (drvdata->mode != CS_MODE_DISABLED) { ret = -EBUSY; goto out; }
local_set(&drvdata->reading, 1); out: spin_unlock_irqrestore(&drvdata->spinlock, flags); return ret; }
So SMB data can only be read when SMB is disabled, and could be read subsequently.
+ 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); + dev_dbg(dev, "%d bytes copied.\n", to_copy); +out: + if (!sdb->data_size) + smb_reset_buffer_status(drvdata); + spin_unlock_irqrestore(&drvdata->spinlock, flags); + return to_copy; +}
[...]
+static int smb_set_perf_buffer(struct perf_output_handle *handle) +{ + struct cs_buffers *buf = etm_perf_sink_config(handle); + u32 head;
+ if (!buf) + return -EINVAL;
+ /* Wrap head around to the amount of space we have */ + head = handle->head & ((buf->nr_pages << PAGE_SHIFT) - 1);
+ /* Find the page to write to and offset within that page */ + buf->cur = head / PAGE_SIZE; + buf->offset = head % PAGE_SIZE;
I believe these fields should be dropped and instead use the handle->head, to determine the page number and offset. See the TMC-ETR driver for e.g.
got it, I'll do this next time.
+ local_set(&buf->data_size, 0);
+ 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;
+ /* Do nothing if trace data is reading by other interface now */ + if (local_read(&drvdata->reading)) + return -EBUSY;
Is there global switch for all the inports ? Or is there a control for individual inports to the SMB ? Do we need additional information here to enable the corresponding port ?
There is a global switch to enable and disable all the inports in smb_config_inport() when probe the SMB driver.
+ switch (mode) { + case CS_MODE_SYSFS: + ret = smb_enable_sysfs(csdev); + break; + case CS_MODE_PERF: + ret = smb_enable_perf(csdev, data); + break; + default: + ret = -EINVAL; + break; + }
+ if (ret) + return ret; + dev_dbg(&csdev->dev, "Ultrasoc SMB enabled.\n");
+ return 0; +}
+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 data_size)
You may need to pass the aux_handle here, to determine the handle->head and the size. See my comments above.
ok, I'll do this next time, thanks.
+{ + struct smb_data_buffer *sdb = &drvdata->sdb; + char **dst_pages = (char **)buf->data_pages; + unsigned long page_offset = buf->offset; + unsigned int cur = buf->cur; + unsigned long to_copy;
+ while (data_size) { + unsigned long page_space = PAGE_SIZE - page_offset;
+ /* Copy parts of trace data when read pointer wrap around */ + if (sdb->rd_offset + page_space > sdb->buf_size) + to_copy = sdb->buf_size - sdb->rd_offset; + else + to_copy = min(data_size, page_space);
+ memcpy_fromio(dst_pages[cur] + page_offset, + sdb->buf_base + sdb->rd_offset, to_copy);
+ page_offset += to_copy; + if (page_offset >= PAGE_SIZE) { + page_offset = 0; + cur++; + cur %= 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);
+ /* + * 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), + drvdata->base + SMB_LB_RD_ADDR); + } + smb_reset_buffer_status(drvdata); +}
+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);
Please fill in the csdev_access method for the device.
will do this next time, thanks.
Thanks, Qi
desc.access = CSDEV_ACCESS_IOMEM(drvdata->base);
+ if (!desc.name) { + dev_err(&pdev->dev, "Failed to alloc coresight device name."); + return -ENOMEM; + }
+ 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 smb_drv_data *drvdata; + int ret;
+ drvdata = devm_kzalloc(&pdev->dev, sizeof(*drvdata), GFP_KERNEL); + if (!drvdata) + return -ENOMEM;
+ drvdata->base = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(drvdata->base)) { + dev_err(&pdev->dev, "Failed to ioremap resource.\n"); + return PTR_ERR(drvdata->base); + }
+ ret = smb_init_data_buffer(pdev, &drvdata->sdb); + if (ret) { + dev_err(&pdev->dev, "Failed to init buffer, ret = %d.\n", ret); + return ret; + }
+ smb_init_hw(drvdata); + spin_lock_init(&drvdata->spinlock); + drvdata->pid = -1;
+ ret = smb_register_sink(pdev, drvdata); + if (ret) { + dev_err(&pdev->dev, "Failed to register smb sink.\n"); + return ret; + }
+ ret = smb_config_inport(&pdev->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 driver");
"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..a01f869fe122 --- /dev/null +++ b/drivers/hwtracing/coresight/ultrasoc-smb.h @@ -0,0 +1,110 @@ +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */ +/*
- Siemens System Memory Buffer driver.
- Copyright(c) 2021, HiSilicon Limited.
- */
+#ifndef _ULTRASOC_SMB_H +#define _ULTRASOC_SMB_H
+#include <linux/coresight.h> +#include <linux/list.h> +#include <linux/miscdevice.h>
+#include "coresight-etm-perf.h" +#include "coresight-priv.h"
+/* Offset of SMB logical buffer registers */ +#define SMB_CFG_REG 0x00 +#define SMB_GLOBAL_EN 0x04 +#define SMB_GLOBAL_INT 0x08 +#define SMB_LB_CFG_LO 0x40 +#define SMB_LB_CFG_HI 0x44 +#define SMB_LB_INT_CTRL 0x48 +#define SMB_LB_INT_STS 0x4c +#define SMB_LB_LIMIT 0x58 +#define SMB_LB_RD_ADDR 0x5c +#define SMB_LB_WR_ADDR 0x60 +#define SMB_LB_PURGE 0x64
+/* Set SMB_CFG_REG register */ +#define SMB_BURST_LEN GENMASK(7, 4) +#define SMB_IDLE_PRD GENMASK(15, 12) +#define SMB_MEM_WR GENMASK(17, 16) +#define SMB_MEM_RD (GENMASK(26, 25) | GENMASK(23, 22)) +#define SMB_GLOBAL_CFG (SMB_IDLE_PRD | SMB_MEM_WR | SMB_MEM_RD | \ + SMB_BURST_LEN)
+/* Set SMB_GLOBAL_INT register */ +#define SMB_INT_EN BIT(0) +#define SMB_INT_TYPE_PULSE BIT(1) +#define SMB_INT_POLARITY_HIGH BIT(2) +#define SMB_GLB_INT_CFG (SMB_INT_EN | SMB_INT_TYPE_PULSE | \ + SMB_INT_POLARITY_HIGH)
+/* Set SMB_LB_CFG_LO register */ +#define SMB_BUF_EN BIT(0) +#define SMB_BUF_SINGLE_END BIT(1) +#define SMB_BUF_INIT BIT(8) +#define SMB_BUF_CONTINUOUS BIT(11) +#define SMB_FLOW_MASK GENMASK(19, 16)
What does the value GENMASK(19, 16) indicate for SMB_FLOW ? That is the value being passed down. Please add a descriptive name. .e.g,
#define SMB_FLOW_MODE_X GENMASK(19, 16)
Similarly for SMB_BUF_NOTE_MASK, unless there is an explanation.
+#define SMB_BUF_CFG_STREAMING (SMB_BUF_INIT | SMB_BUF_CONTINUOUS | \ + SMB_FLOW_MASK | SMB_BUF_SINGLE_END | \ + SMB_BUF_EN)
+#define SMB_BASE_LOW_MASK GENMASK(31, 0)
+/* Set SMB_LB_CFG_HI register */ +#define SMB_MSG_FILTER GENMASK(15, 8)
+/* Set SMB_LB_INT_CTRL */ +#define SMB_BUF_INT_EN BIT(0) +#define SMB_BUF_NOTE_MASK GENMASK(11, 8) +#define SMB_BUF_INT_CFG (SMB_BUF_INT_EN | SMB_BUF_NOTE_MASK)
+/**
- 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.
Please align the field definitions.
- @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.
- @status : Status of SMB buffer.
- */
+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.
- @spinlock: Only one at a time pls.
- @reading: Synchronise user space access to SMB buffer.
- @pid: Process ID of the process being monitored by the session
- * that is using this component.
Same as above.
- @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; + spinlock_t spinlock; + local_t reading; + pid_t pid; + u32 mode; +};
+#define smb_reg(name, offset) coresight_simple_reg32(struct smb_drv_data, name, offset)
+#endif
Suzuki .