Messages in this thread | | | Subject | Re: [PATCH v14 1/2] drivers/coresight: Add UltraSoc System Memory Buffer driver | From | hejunhao <> | Date | Fri, 25 Nov 2022 22:26:29 +0800 |
| |
On 2022/11/24 21:45, Suzuki K Poulose wrote: > On 24/11/2022 13:33, hejunhao wrote: >> >> On 2022/11/23 22:03, Suzuki K Poulose wrote: >>> On 23/11/2022 12:38, Junhao He wrote: >>>> From: Qi Liu <liuqi115@huawei.com> >>>> >>>> Add 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. >>>> The SMB device is identifier as ACPI HID "HISI03A1". Device >>>> system memory address resources are allocated using the _CRS >>>> method and buffer modes is the circular buffer mode. >>>> >>>> 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> >>>> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> >>>> --- >>>> drivers/hwtracing/coresight/Kconfig | 12 + >>>> drivers/hwtracing/coresight/Makefile | 1 + >>>> drivers/hwtracing/coresight/ultrasoc-smb.c | 658 >>>> +++++++++++++++++++++ >>>> drivers/hwtracing/coresight/ultrasoc-smb.h | 129 ++++ >>>> 4 files changed, 800 insertions(+) >>>> create mode 100644 drivers/hwtracing/coresight/ultrasoc-smb.c >>>> create mode 100644 drivers/hwtracing/coresight/ultrasoc-smb.h >>>> >>> >>>> +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; >>> >>> >>> --8>-- cut here --<8-- >>> >>>> + 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); >>>> + } >>> >>> --<8-- end here --8>-- >>> >>> As pointed out in the last review, we must do this step >>> everytime for perf mode irrespective of whether the buffer >>> was "FULL" or not. >>> >>> i.e, the above block should simply be: >>> >>> if (sdb->full) >>> smb_purge_data(drvdata); >>> >>> /* >>> * The uncollected Data must be discarded for perf, >>> * as it cannot be clubbed with next schedule. We >>> * any way TRUNCATE the buffer in this case. >>> */ >>> writel(readl(drvdata->base + SMB_LB_WR_ADDR_REG), >>> drvdata->base + SMB_LB_RD_ADDR_REG); >>> >>> Suzuki >> >> Hi Suzuki, >> >> We need to update SMB_LB_RD_ADDR_REG register first, then >> check the "full" flag, whether the register needs to be >> updated again. > > Why ? sdb->full is not updated after the write to RD_ADDR_REG. > Hi Suzuki,
Maybe using the code below is more appropriate. i.e,
writel(sdb->start_addr + sdb->rd_offset, drvdata->base + SMB_LB_RD_ADDR_REG);
/* * The uncollected Data must be discarded for perf, * as it cannot be clubbed with next schedule. We * any way TRUNCATE the buffer in this case. */ smb_update_data_size(drvdata); if (sdb->data_size) writel(readl(drvdata->base + SMB_LB_WR_ADDR_REG), drvdata->base + SMB_LB_RD_ADDR_REG);
Best regards, Junhao.
>> >> If we don`t update the value of SMB_LB_RD_ADDR_REG register >> or reset buffer state, the buffer state will still be "full". >> The buffer has not free area,so the data will still remain >> in link. > > My issue here is with potentially "leaving the trace from a previous > session for the next session". i.e., at the end of a run, we must always > make sure that the buffer is left empty (unlike the sysfs mode). > > e.g., > > perf_session_x: RUN0: RD=0x0, WR=0x5000, HANDLE_SIZE=0x3000, full=false. > At the end of the above routine we will have : > RD=0x3000, WR=0x5000, > > and if a different perf session comes in, say perf_session_y, it will > consume trace written by "x" at 0x3000-0x50000, right ? > > This is fine in the sysfs mode as we expect the entire sysfs mode > to be owned by a single session and is left to the user to split it. > But for perf mode we cannot do that and thus must make sure we don't > leak trace from one session to antoher. > > Suzuki >
In this cace: RUN0: RD=0x0, WR=0x5000, HANDLE_SIZE=0x3000, full=false. We will update the "rd_offset" in smb_update_buffer() function. like this:
... if (data_size > handle->size) { sdb->rd_offset += data_size - handle->size; sdb->rd_offset %= sdb->buf_size; ... } ...
So the rd_offset will advance to 0x2000 first, then we dump the latest trace data (0x2000 - 0x5000) from "buf_base + rd_offset".
Best regards, Junhao.
> >> >> Thanks. >> HeJunhao. >> >>> _______________________________________________ >>> CoreSight mailing list -- coresight@lists.linaro.org >>> To unsubscribe send an email to coresight-leave@lists.linaro.org >>> >>> . >>> >> > > _______________________________________________ > CoreSight mailing list -- coresight@lists.linaro.org > To unsubscribe send an email to coresight-leave@lists.linaro.org
| |