* Re: [PATCH 12/15] accel/qda: Add FastRPC invocation support
From: Ekansh Gupta @ 2026-06-04 5:09 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Oded Gabbay, Jonathan Corbet, Shuah Khan, Joerg Roedel,
Will Deacon, Robin Murphy, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Sumit Semwal,
Christian König, Bharath Kumar, Chenna Kesava Raju, srini,
andersson, konradybcio, robin.clark, linux-kernel, dri-devel,
linux-doc, linux-arm-msm, iommu, linux-media, linaro-mm-sig
In-Reply-To: <43a7laqb7mnrvleunnmbxwhvzr6w3au4ofjri4r4ap7clsx6mc@jxqlr4a2lw56>
On 20-05-2026 19:26, Dmitry Baryshkov wrote:
> On Tue, May 19, 2026 at 11:46:02AM +0530, Ekansh Gupta via B4 Relay wrote:
>> From: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
>>
>> Implement the FastRPC remote procedure call path, allowing user-space
>> to invoke methods on the DSP via DRM_IOCTL_QDA_REMOTE_INVOKE.
>>
>> qda_fastrpc.c / qda_fastrpc.h
>> Implements the FastRPC protocol layer: argument marshalling
>> (qda_fastrpc_invoke_pack), response unmarshalling
>> (qda_fastrpc_invoke_unpack), and invocation context lifecycle
>> management. Each invocation allocates a fastrpc_invoke_context
>> which tracks buffer descriptors, GEM objects, and the completion
>> used to synchronise with the DSP response.
>>
>> Buffer arguments are handled in three ways:
>> - DMA-BUF fd: imported via PRIME, IOMMU-mapped dma_addr used
>> - Direct (inline): copied into the GEM-backed message buffer
>> - DMA handle: fd forwarded to DSP, physical page descriptor computed
>
> No. This needs to go away. The QDA should support only one way to pass
> data - via the GEM buffers. Everything else should be handled by the
> shim layer, etc.
each FD passed here is a GEM buffer. The reason to pass fd is that there
are some APIs on DSP side which takes fd as an argument and the user
might use the same on their skel implementation. So in this case the
remote call will take fd to DSP and the skel implementation will use the
FD.>
>>
>> qda_rpmsg.c
>> Implements qda_rpmsg_send_msg() which sends the wire-format
>> fastrpc_msg (embedded as the first member of qda_msg) directly
>> via rpmsg_send(), and qda_rpmsg_wait_for_rsp() which blocks on
>> the context completion. The RPMsg callback dispatches responses
>> to waiting contexts via the ctx_xa XArray.
>>
>> qda_ioctl.c
>> qda_ioctl_invoke() drives the full invocation lifecycle:
>> allocate context → assign XArray ID → prepare args → allocate
>> GEM message buffer → pack → send → wait → unpack → free.
>>
>> qda_drv.h / qda_drv.c
>> qda_dev gains ctx_xa (XArray for in-flight context lookup) and
>> remote_session_id_counter (atomic counter for session IDs).
>> qda_file_priv gains remote_session_id for per-session tracking.
>>
>> include/uapi/drm/qda_accel.h
>> Adds DRM_IOCTL_QDA_REMOTE_INVOKE (command 0x07; command numbers
>> 0x03–0x06 are reserved) and the associated drm_qda_invoke_args
>> and drm_qda_fastrpc_invoke_args structures.
>>
>> Assisted-by: Claude:claude-4-6-sonnet
>> Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
>> ---
>> drivers/accel/qda/Makefile | 1 +
>> drivers/accel/qda/qda_drv.c | 17 ++
>> drivers/accel/qda/qda_drv.h | 8 +
>> drivers/accel/qda/qda_fastrpc.c | 597 ++++++++++++++++++++++++++++++++++++++++
>> drivers/accel/qda/qda_fastrpc.h | 271 ++++++++++++++++++
>> drivers/accel/qda/qda_ioctl.c | 104 +++++++
>> drivers/accel/qda/qda_ioctl.h | 1 +
>> drivers/accel/qda/qda_rpmsg.c | 136 ++++++++-
>> drivers/accel/qda/qda_rpmsg.h | 17 ++
>> include/uapi/drm/qda_accel.h | 39 +++
>> 10 files changed, 1189 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/accel/qda/Makefile b/drivers/accel/qda/Makefile
>> index fb092e56d7f3..2d10420cd1ec 100644
>> --- a/drivers/accel/qda/Makefile
>> +++ b/drivers/accel/qda/Makefile
>> @@ -8,6 +8,7 @@ obj-$(CONFIG_DRM_ACCEL_QDA) := qda.o
>> qda-y := \
>> qda_cb.o \
>> qda_drv.o \
>> + qda_fastrpc.o \
>> qda_gem.o \
>> qda_ioctl.o \
>> qda_memory_dma.o \
>> diff --git a/drivers/accel/qda/qda_drv.c b/drivers/accel/qda/qda_drv.c
>> index ef8bd573b836..704c7d3127d2 100644
>> --- a/drivers/accel/qda/qda_drv.c
>> +++ b/drivers/accel/qda/qda_drv.c
>> @@ -26,6 +26,8 @@ static int qda_open(struct drm_device *dev, struct drm_file *file)
>>
>> qda_file_priv->pid = current->pid;
>> qda_file_priv->qda_dev = qda_dev_from_drm(dev);
>> + qda_file_priv->remote_session_id =
>> + atomic_inc_return(&qda_file_priv->qda_dev->remote_session_id_counter);
>> file->driver_priv = qda_file_priv;
>>
>> return 0;
>> @@ -57,6 +59,7 @@ static const struct drm_ioctl_desc qda_ioctls[] = {
>> DRM_IOCTL_DEF_DRV(QDA_QUERY, qda_ioctl_query, 0),
>> DRM_IOCTL_DEF_DRV(QDA_GEM_CREATE, qda_ioctl_gem_create, 0),
>> DRM_IOCTL_DEF_DRV(QDA_GEM_MMAP_OFFSET, qda_ioctl_gem_mmap_offset, 0),
>> + DRM_IOCTL_DEF_DRV(QDA_REMOTE_INVOKE, qda_ioctl_invoke, 0),
>> };
>>
>> static const struct drm_driver qda_drm_driver = {
>> @@ -93,6 +96,17 @@ static void cleanup_memory_manager(struct qda_dev *qdev)
>> }
>> }
>>
>> +static void cleanup_device_resources(struct qda_dev *qdev)
>> +{
>> + xa_destroy(&qdev->ctx_xa);
>> +}
>> +
>> +static void init_device_resources(struct qda_dev *qdev)
>> +{
>> + atomic_set(&qdev->remote_session_id_counter, 0);
>> + xa_init_flags(&qdev->ctx_xa, XA_FLAGS_ALLOC1);
>> +}
>> +
>> static int init_memory_manager(struct qda_dev *qdev)
>> {
>> qdev->iommu_mgr = kzalloc_obj(*qdev->iommu_mgr);
>> @@ -106,6 +120,7 @@ void qda_deinit_device(struct qda_dev *qdev)
>> {
>> mutex_destroy(&qdev->import_lock);
>> cleanup_memory_manager(qdev);
>> + cleanup_device_resources(qdev);
>> }
>>
>> int qda_init_device(struct qda_dev *qdev)
>> @@ -114,10 +129,12 @@ int qda_init_device(struct qda_dev *qdev)
>>
>> mutex_init(&qdev->import_lock);
>> qdev->current_import_file_priv = NULL;
>> + init_device_resources(qdev);
>>
>> ret = init_memory_manager(qdev);
>> if (ret) {
>> drm_err(&qdev->drm_dev, "Failed to initialize memory manager: %d\n", ret);
>> + cleanup_device_resources(qdev);
>> mutex_destroy(&qdev->import_lock);
>> }
>>
>> diff --git a/drivers/accel/qda/qda_drv.h b/drivers/accel/qda/qda_drv.h
>> index 96ce4135e2d9..420cccff42bf 100644
>> --- a/drivers/accel/qda/qda_drv.h
>> +++ b/drivers/accel/qda/qda_drv.h
>> @@ -6,10 +6,12 @@
>> #ifndef __QDA_DRV_H__
>> #define __QDA_DRV_H__
>>
>> +#include <linux/atomic.h>
>> #include <linux/device.h>
>> #include <linux/list.h>
>> #include <linux/rpmsg.h>
>> #include <linux/types.h>
>> +#include <linux/xarray.h>
>> #include <drm/drm_device.h>
>> #include <drm/drm_drv.h>
>> #include <drm/drm_file.h>
>> @@ -28,6 +30,8 @@ struct qda_file_priv {
>> struct qda_iommu_device *assigned_iommu_dev;
>> /** @pid: Process ID for tracking */
>> pid_t pid;
>> + /** @remote_session_id: Unique session identifier */
>> + u32 remote_session_id;
>> };
>>
>> /**
>> @@ -51,8 +55,12 @@ struct qda_dev {
>> struct mutex import_lock;
>> /** @current_import_file_priv: Current file_priv during prime import */
>> struct drm_file *current_import_file_priv;
>> + /** @ctx_xa: XArray for FastRPC context management */
>> + struct xarray ctx_xa;
>> /** @dsp_name: Name of the DSP domain (e.g. "cdsp", "adsp") */
>> const char *dsp_name;
>> + /** @remote_session_id_counter: Atomic counter for unique session IDs */
>> + atomic_t remote_session_id_counter;
>> };
>>
>> /**
>> diff --git a/drivers/accel/qda/qda_fastrpc.c b/drivers/accel/qda/qda_fastrpc.c
>> new file mode 100644
>> index 000000000000..0ec37175a098
>> --- /dev/null
>> +++ b/drivers/accel/qda/qda_fastrpc.c
>> @@ -0,0 +1,597 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +// Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>> +#include <linux/slab.h>
>> +#include <linux/uaccess.h>
>> +#include <linux/sort.h>
>> +#include <linux/completion.h>
>> +#include <linux/dma-buf.h>
>> +#include <drm/drm_gem.h>
>> +#include "qda_fastrpc.h"
>> +#include "qda_drv.h"
>> +#include "qda_gem.h"
>> +#include "qda_memory_manager.h"
>> +#include "qda_prime.h"
>> +
>> +/**
>> + * get_gem_obj_from_dmabuf_fd() - Import a DMA-BUF fd and return the GEM object
>> + * @ctx: FastRPC invocation context
>> + * @dmabuf_fd: DMA-BUF file descriptor supplied by user space
>> + * @gem_obj: Output GEM object (caller must call drm_gem_object_put() when done)
>> + *
>> + * Imports the DMA-BUF fd into the QDA device via qda_prime_fd_to_handle()
>> + * (which performs IOMMU device assignment for newly imported buffers) and
>> + * then looks up the resulting GEM object. The caller is responsible for
>> + * calling drm_gem_object_put() on the returned object.
>> + *
>> + * Return: 0 on success, negative error code on failure
>> + */
>> +static int get_gem_obj_from_dmabuf_fd(struct fastrpc_invoke_context *ctx,
>> + int dmabuf_fd,
>> + struct drm_gem_object **gem_obj)
>> +{
>> + struct drm_device *dev = ctx->file_priv->minor->dev;
>> + u32 handle;
>> + int ret;
>> +
>> + ret = qda_prime_fd_to_handle(dev, ctx->file_priv, dmabuf_fd, &handle);
>> + if (ret)
>> + return ret;
>> +
>> + *gem_obj = drm_gem_object_lookup(ctx->file_priv, handle);
>> + if (!*gem_obj)
>> + return -ENOENT;
>> +
>> + return 0;
>> +}
>> +
>> +static void setup_pages_from_gem_obj(struct qda_gem_obj *qda_gem_obj,
>> + struct fastrpc_phy_page *pages)
>> +{
>> + pages->addr = qda_gem_obj->dma_addr;
>> + pages->size = qda_gem_obj->size;
>> +}
>> +
>> +static u64 calculate_vma_offset(u64 user_ptr)
>> +{
>> + struct vm_area_struct *vma;
>> + u64 user_ptr_page_mask = user_ptr & PAGE_MASK;
>> + u64 vma_offset = 0;
>> +
>> + mmap_read_lock(current->mm);
>> + vma = find_vma(current->mm, user_ptr);
>> + if (vma)
>> + vma_offset = user_ptr_page_mask - vma->vm_start;
>> + mmap_read_unlock(current->mm);
>> +
>> + return vma_offset;
>> +}
>> +
>> +static u64 calculate_page_aligned_size(u64 ptr, u64 len)
>> +{
>> + u64 pg_start = (ptr & PAGE_MASK) >> PAGE_SHIFT;
>> + u64 pg_end = ((ptr + len - 1) & PAGE_MASK) >> PAGE_SHIFT;
>> + u64 aligned_size = (pg_end - pg_start + 1) * PAGE_SIZE;
>> +
>> + return aligned_size;
>> +}
>> +
>> +static struct fastrpc_invoke_buf *fastrpc_invoke_buf_start(union fastrpc_remote_arg *pra, int len)
>> +{
>> + return (struct fastrpc_invoke_buf *)(&pra[len]);
>> +}
>> +
>> +static struct fastrpc_phy_page *fastrpc_phy_page_start(struct fastrpc_invoke_buf *buf, int len)
>> +{
>> + return (struct fastrpc_phy_page *)(&buf[len]);
>> +}
>> +
>> +static int fastrpc_get_meta_size(struct fastrpc_invoke_context *ctx)
>> +{
>> + int size = 0;
>> +
>> + size = (sizeof(struct fastrpc_remote_buf) +
>> + sizeof(struct fastrpc_invoke_buf) +
>> + sizeof(struct fastrpc_phy_page)) * ctx->nscalars +
>> + sizeof(u64) * FASTRPC_MAX_FDLIST +
>> + sizeof(u32) * FASTRPC_MAX_CRCLIST;
>> +
>> + return size;
>> +}
>> +
>> +static u64 fastrpc_get_payload_size(struct fastrpc_invoke_context *ctx, int metalen)
>> +{
>> + u64 size = 0;
>> + int oix;
>> +
>> + size = ALIGN(metalen, FASTRPC_ALIGN);
>> +
>> + for (oix = 0; oix < ctx->nbufs; oix++) {
>> + int i = ctx->olaps[oix].raix;
>> +
>> + if (ctx->args[i].fd == 0 || ctx->args[i].fd == -1) {
>> + if (ctx->olaps[oix].offset == 0)
>> + size = ALIGN(size, FASTRPC_ALIGN);
>> +
>> + size += (ctx->olaps[oix].mend - ctx->olaps[oix].mstart);
>> + }
>> + }
>> +
>> + return size;
>> +}
>> +
>> +/**
>> + * qda_fastrpc_context_free() - Free an invocation context
>> + * @ref: Reference counter embedded in the context
>> + *
>> + * Called when the reference count reaches zero; releases all resources
>> + * associated with the invocation context.
>> + */
>> +void qda_fastrpc_context_free(struct kref *ref)
>> +{
>> + struct fastrpc_invoke_context *ctx;
>> + int i;
>> +
>> + ctx = container_of(ref, struct fastrpc_invoke_context, refcount);
>> + if (ctx->gem_objs) {
>> + for (i = 0; i < ctx->nscalars; ++i) {
>> + if (ctx->gem_objs[i])
>> + drm_gem_object_put(ctx->gem_objs[i]);
>> + }
>> + kfree(ctx->gem_objs);
>> + }
>> +
>> + if (ctx->msg_gem_obj)
>> + drm_gem_object_put(&ctx->msg_gem_obj->base);
>> +
>> + kfree(ctx->olaps);
>> +
>> + kfree(ctx->args);
>> + kfree(ctx->req);
>> + kfree(ctx->rsp);
>> + kfree(ctx->input_pages);
>> + kfree(ctx->inbuf);
>> +
>> + kfree(ctx);
>> +}
>> +
>> +#define CMP(aa, bb) ((aa) == (bb) ? 0 : (aa) < (bb) ? -1 : 1)
>> +
>> +static int olaps_cmp(const void *a, const void *b)
>> +{
>> + struct fastrpc_buf_overlap *pa = (struct fastrpc_buf_overlap *)a;
>> + struct fastrpc_buf_overlap *pb = (struct fastrpc_buf_overlap *)b;
>> + /* sort with lowest starting buffer first */
>> + int st = CMP(pa->start, pb->start);
>> + /* sort with highest ending buffer first */
>> + int ed = CMP(pb->end, pa->end);
>> +
>> + return st == 0 ? ed : st;
>> +}
>> +
>> +static void fastrpc_get_buff_overlaps(struct fastrpc_invoke_context *ctx)
>> +{
>> + u64 max_end = 0;
>> + int i;
>> +
>> + for (i = 0; i < ctx->nbufs; ++i) {
>> + ctx->olaps[i].start = ctx->args[i].ptr;
>> + ctx->olaps[i].end = ctx->olaps[i].start + ctx->args[i].length;
>> + ctx->olaps[i].raix = i;
>> + }
>> +
>> + sort(ctx->olaps, ctx->nbufs, sizeof(*ctx->olaps), olaps_cmp, NULL);
>> +
>> + for (i = 0; i < ctx->nbufs; ++i) {
>> + if (ctx->olaps[i].start < max_end) {
>> + ctx->olaps[i].mstart = max_end;
>> + ctx->olaps[i].mend = ctx->olaps[i].end;
>> + ctx->olaps[i].offset = max_end - ctx->olaps[i].start;
>> +
>> + if (ctx->olaps[i].end > max_end) {
>> + max_end = ctx->olaps[i].end;
>> + } else {
>> + ctx->olaps[i].mend = 0;
>> + ctx->olaps[i].mstart = 0;
>> + }
>> + } else {
>> + ctx->olaps[i].mend = ctx->olaps[i].end;
>> + ctx->olaps[i].mstart = ctx->olaps[i].start;
>> + ctx->olaps[i].offset = 0;
>> + max_end = ctx->olaps[i].end;
>> + }
>> + }
>> +}
>> +
>> +/**
>> + * qda_fastrpc_context_alloc() - Allocate a new FastRPC invocation context
>> + *
>> + * Return: Pointer to allocated context, or ERR_PTR on failure
>> + */
>> +struct fastrpc_invoke_context *qda_fastrpc_context_alloc(void)
>> +{
>> + struct fastrpc_invoke_context *ctx = NULL;
>> +
>> + ctx = kzalloc_obj(*ctx);
>> + if (!ctx)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + INIT_LIST_HEAD(&ctx->node);
>> +
>> + ctx->retval = -1;
>> + ctx->pid = current->pid;
>> + init_completion(&ctx->work);
>> + ctx->msg_gem_obj = NULL;
>> + kref_init(&ctx->refcount);
>> +
>> + return ctx;
>> +}
>> +
>> +/*
>> + * process_fd_buffer() - Handle an in/out buffer argument backed by a DMA-BUF fd
>> + *
>> + * args[i].fd is a DMA-BUF fd. We import it to obtain the GEM object and its
>> + * IOMMU-mapped dma_addr for the physical page descriptor. The DSP uses the
>> + * physical address directly for this buffer type; the fd is not forwarded.
>> + */
>> +static int process_fd_buffer(struct fastrpc_invoke_context *ctx, int i,
>> + union fastrpc_remote_arg *rpra, struct fastrpc_phy_page *pages)
>> +{
>> + struct drm_gem_object *gem_obj;
>> + struct qda_gem_obj *qda_gem_obj;
>> + int err;
>> + u64 len = ctx->args[i].length;
>> + u64 vma_offset;
>> +
>> + err = get_gem_obj_from_dmabuf_fd(ctx, ctx->args[i].fd, &gem_obj);
>> + if (err)
>> + return err;
>> +
>> + ctx->gem_objs[i] = gem_obj;
>> + qda_gem_obj = to_qda_gem_obj(gem_obj);
>> +
>> + rpra[i].buf.pv = (u64)ctx->args[i].ptr;
>> +
>> + pages[i].addr = qda_gem_obj->dma_addr;
>> +
>> + vma_offset = calculate_vma_offset(ctx->args[i].ptr);
>> + pages[i].addr += vma_offset;
>> + pages[i].size = calculate_page_aligned_size(ctx->args[i].ptr, len);
>> +
>> + return 0;
>> +}
>> +
>> +static int process_direct_buffer(struct fastrpc_invoke_context *ctx, int i, int oix,
>> + union fastrpc_remote_arg *rpra, struct fastrpc_phy_page *pages,
>> + uintptr_t *args, u64 *rlen, u64 pkt_size)
>> +{
>> + int mlen;
>> + u64 len = ctx->args[i].length;
>> + int inbufs = ctx->inbufs;
>> +
>> + if (ctx->olaps[oix].offset == 0) {
>> + *rlen -= ALIGN(*args, FASTRPC_ALIGN) - *args;
>> + *args = ALIGN(*args, FASTRPC_ALIGN);
>> + }
>> +
>> + mlen = ctx->olaps[oix].mend - ctx->olaps[oix].mstart;
>> +
>> + if (*rlen < mlen)
>> + return -ENOSPC;
>> +
>> + rpra[i].buf.pv = *args - ctx->olaps[oix].offset;
>> +
>> + pages[i].addr = ctx->msg->phys - ctx->olaps[oix].offset + (pkt_size - *rlen);
>> + pages[i].addr = pages[i].addr & PAGE_MASK;
>> + pages[i].size = calculate_page_aligned_size(rpra[i].buf.pv, len);
>> +
>> + *args = *args + mlen;
>> + *rlen -= mlen;
>> +
>> + if (i < inbufs) {
>> + void *dst = (void *)(uintptr_t)rpra[i].buf.pv;
>> + void *src = (void *)(uintptr_t)ctx->args[i].ptr;
>> +
>> + /*
>> + * For user-space invocations (INVOKE_DYNAMIC), ptr is a user
>> + * virtual address and must be copied safely. For all other
>> + * (kernel-internal) invocations, ptr is a kernel address set
>> + * by the driver itself and can be copied directly.
>> + */
>> + if (ctx->type == FASTRPC_RMID_INVOKE_DYNAMIC) {
>> + if (copy_from_user(dst, (void __user *)src, len))
>> + return -EFAULT;
>> + } else {
>> + memcpy(dst, src, len);
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +/*
>> + * process_dma_handle() - Handle a DMA-handle scalar argument
>> + *
>> + * args[i].fd is a DMA-BUF fd. We import it to get the physical page
>> + * descriptor for the kernel, but forward the original DMA-BUF fd to the
>> + * DSP in rpra[i].dma.fd so the DSP can identify the buffer by its fd.
>> + */
>> +static int process_dma_handle(struct fastrpc_invoke_context *ctx, int i,
>> + union fastrpc_remote_arg *rpra, struct fastrpc_phy_page *pages)
>> +{
>> + if (ctx->args[i].fd > 0) {
>> + struct drm_gem_object *gem_obj;
>> + struct qda_gem_obj *qda_gem_obj;
>> + int err;
>> +
>> + err = get_gem_obj_from_dmabuf_fd(ctx, ctx->args[i].fd, &gem_obj);
>> + if (err)
>> + return err;
>> +
>> + ctx->gem_objs[i] = gem_obj;
>> + qda_gem_obj = to_qda_gem_obj(gem_obj);
>> +
>> + setup_pages_from_gem_obj(qda_gem_obj, &pages[i]);
>> +
>> + /* Forward the original DMA-BUF fd to the DSP */
>> + rpra[i].dma.fd = ctx->args[i].fd;
>> + rpra[i].dma.len = ctx->args[i].length;
>> + rpra[i].dma.offset = (u64)ctx->args[i].ptr;
>> + } else {
>> + rpra[i].buf.pv = ctx->args[i].ptr;
>> + rpra[i].buf.len = ctx->args[i].length;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * qda_fastrpc_get_header_size() - Compute the FastRPC message header size
>> + * @ctx: FastRPC invocation context
>> + * @out_size: Pointer to store the aligned packet size in bytes
>> + *
>> + * Return: 0 on success, negative error code on failure
>> + */
>> +int qda_fastrpc_get_header_size(struct fastrpc_invoke_context *ctx, size_t *out_size)
>> +{
>> + ctx->inbufs = REMOTE_SCALARS_INBUFS(ctx->sc);
>> + ctx->metalen = fastrpc_get_meta_size(ctx);
>> + ctx->pkt_size = fastrpc_get_payload_size(ctx, ctx->metalen);
>> +
>> + ctx->aligned_pkt_size = PAGE_ALIGN(ctx->pkt_size);
>> + if (ctx->aligned_pkt_size == 0)
>> + return -EINVAL;
>> +
>> + *out_size = ctx->aligned_pkt_size;
>> + return 0;
>> +}
>> +
>> +static int fastrpc_get_args(struct fastrpc_invoke_context *ctx)
>> +{
>> + union fastrpc_remote_arg *rpra;
>> + struct fastrpc_invoke_buf *list;
>> + struct fastrpc_phy_page *pages;
>> + int i, oix, err = 0;
>> + u64 rlen;
>> + uintptr_t args;
>> + size_t hdr_size;
>> +
>> + ctx->inbufs = REMOTE_SCALARS_INBUFS(ctx->sc);
>> + err = qda_fastrpc_get_header_size(ctx, &hdr_size);
>> + if (err)
>> + return err;
>> +
>> + ctx->msg->buf = ctx->msg_gem_obj->virt;
>> + ctx->msg->phys = ctx->msg_gem_obj->dma_addr;
>> +
>> + memset(ctx->msg->buf, 0, ctx->aligned_pkt_size);
>> +
>> + rpra = (union fastrpc_remote_arg *)ctx->msg->buf;
>> + ctx->list = fastrpc_invoke_buf_start(rpra, ctx->nscalars);
>> + ctx->pages = fastrpc_phy_page_start(ctx->list, ctx->nscalars);
>> + list = ctx->list;
>> + pages = ctx->pages;
>> + args = (uintptr_t)ctx->msg->buf + ctx->metalen;
>> + rlen = ctx->pkt_size - ctx->metalen;
>> + ctx->rpra = rpra;
>> +
>> + for (oix = 0; oix < ctx->nbufs; ++oix) {
>> + i = ctx->olaps[oix].raix;
>> +
>> + rpra[i].buf.pv = 0;
>> + rpra[i].buf.len = ctx->args[i].length;
>> + list[i].num = ctx->args[i].length ? 1 : 0;
>> + list[i].pgidx = i;
>> +
>> + if (!ctx->args[i].length)
>> + continue;
>> +
>> + if (ctx->args[i].fd > 0)
>> + err = process_fd_buffer(ctx, i, rpra, pages);
>> + else
>> + err = process_direct_buffer(ctx, i, oix, rpra, pages, &args, &rlen,
>> + ctx->pkt_size);
>> +
>> + if (err)
>> + goto bail_gem;
>> + }
>> +
>> + for (i = ctx->nbufs; i < ctx->nscalars; ++i) {
>> + list[i].num = ctx->args[i].length ? 1 : 0;
>> + list[i].pgidx = i;
>> +
>> + err = process_dma_handle(ctx, i, rpra, pages);
>> + if (err)
>> + goto bail_gem;
>> + }
>> +
>> + return 0;
>> +
>> +bail_gem:
>> + if (ctx->msg_gem_obj) {
>> + drm_gem_object_put(&ctx->msg_gem_obj->base);
>> + ctx->msg_gem_obj = NULL;
>> + }
>> +
>> + return err;
>> +}
>> +
>> +static int fastrpc_put_args(struct fastrpc_invoke_context *ctx, struct qda_msg *msg)
>> +{
>> + union fastrpc_remote_arg *rpra;
>> + int i, err = 0;
>> +
>> + if (!ctx)
>> + return -EINVAL;
>> +
>> + rpra = ctx->rpra;
>> + if (!rpra)
>> + return -EINVAL;
>> +
>> + for (i = ctx->inbufs; i < ctx->nbufs; ++i) {
>> + if (ctx->args[i].fd <= 0) {
>> + void *src = (void *)(uintptr_t)rpra[i].buf.pv;
>> + void *dst = (void *)(uintptr_t)ctx->args[i].ptr;
>> + u64 len = rpra[i].buf.len;
>> +
>> + if (ctx->type == FASTRPC_RMID_INVOKE_DYNAMIC)
>> + err = copy_to_user((void __user *)dst, src, len) ? -EFAULT : 0;
>> + else
>> + memcpy(dst, src, len);
>> + if (err)
>> + break;
>> + }
>> + }
>> +
>> + return err;
>> +}
>> +
>> +/**
>> + * qda_fastrpc_invoke_pack() - Pack an invocation context into a QDA message
>> + * @ctx: FastRPC invocation context
>> + * @msg: QDA message structure to pack into
>> + *
>> + * Return: 0 on success, negative error code on failure
>> + */
>> +int qda_fastrpc_invoke_pack(struct fastrpc_invoke_context *ctx,
>> + struct qda_msg *msg)
>> +{
>> + int err = 0;
>> +
>> + if (ctx->handle == FASTRPC_INIT_HANDLE)
>> + msg->fastrpc.remote_session_id = 0;
>> + else
>> + msg->fastrpc.remote_session_id = ctx->remote_session_id;
>> +
>> + ctx->msg = msg;
>> +
>> + err = fastrpc_get_args(ctx);
>> + if (err)
>> + return err;
>> +
>> + dma_wmb();
>> +
>> + msg->fastrpc.tid = ctx->pid;
>> + msg->fastrpc.ctx = ctx->ctxid | ctx->pd;
>> + msg->fastrpc.handle = ctx->handle;
>> + msg->fastrpc.sc = ctx->sc;
>> + msg->fastrpc.addr = ctx->msg->phys;
>> + msg->fastrpc.size = roundup(ctx->pkt_size, PAGE_SIZE);
>> + msg->fastrpc_ctx = ctx;
>> + msg->file_priv = ctx->file_priv;
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * qda_fastrpc_invoke_unpack() - Unpack a response message into an invocation context
>> + * @ctx: FastRPC invocation context
>> + * @msg: QDA message structure to unpack from
>> + *
>> + * Return: 0 on success, negative error code on failure
>> + */
>> +int qda_fastrpc_invoke_unpack(struct fastrpc_invoke_context *ctx,
>> + struct qda_msg *msg)
>> +{
>> + int err;
>> +
>> + dma_rmb();
>> +
>> + err = fastrpc_put_args(ctx, msg);
>> + if (err)
>> + return err;
>> +
>> + err = ctx->retval;
>> + return err;
>> +}
>> +
>> +static int fastrpc_prepare_args_invoke(struct fastrpc_invoke_context *ctx, char __user *argp)
>> +{
>> + struct drm_qda_invoke_args invoke_args;
>> + struct drm_qda_fastrpc_invoke_args *args = NULL;
>> + u32 nscalars;
>> +
>> + /* argp is DRM ioctl data (kernel pointer); args pointer within it is user-space */
>> + memcpy(&invoke_args, argp, sizeof(invoke_args));
>> +
>> + ctx->handle = invoke_args.handle;
>> + ctx->sc = invoke_args.sc;
>> +
>> + nscalars = REMOTE_SCALARS_LENGTH(ctx->sc);
>> + if (!nscalars) {
>> + ctx->args = NULL;
>> + return 0;
>> + }
>> +
>> + args = kcalloc(nscalars, sizeof(*args), GFP_KERNEL);
>> + if (!args)
>> + return -ENOMEM;
>> +
>> + if (copy_from_user(args, u64_to_user_ptr(invoke_args.args),
>> + nscalars * sizeof(*args))) {
>> + kfree(args);
>> + return -EFAULT;
>> + }
>> +
>> + ctx->args = args;
>> + return 0;
>> +}
>> +
>> +/**
>> + * qda_fastrpc_prepare_args() - Prepare arguments for a FastRPC invocation
>> + * @ctx: FastRPC invocation context
>> + * @argp: User-space pointer to invocation arguments
>> + *
>> + * Return: 0 on success, negative error code on failure
>> + */
>> +int qda_fastrpc_prepare_args(struct fastrpc_invoke_context *ctx, char __user *argp)
>> +{
>> + int err;
>> +
>> + switch (ctx->type) {
>> + case FASTRPC_RMID_INVOKE_DYNAMIC:
>> + err = fastrpc_prepare_args_invoke(ctx, argp);
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> + if (err)
>> + return err;
>> +
>> + ctx->nscalars = REMOTE_SCALARS_LENGTH(ctx->sc);
>> + ctx->nbufs = REMOTE_SCALARS_INBUFS(ctx->sc) + REMOTE_SCALARS_OUTBUFS(ctx->sc);
>> +
>> + if (ctx->nscalars) {
>> + ctx->gem_objs = kcalloc(ctx->nscalars, sizeof(*ctx->gem_objs), GFP_KERNEL);
>> + if (!ctx->gem_objs)
>> + return -ENOMEM;
>> + ctx->olaps = kcalloc(ctx->nscalars, sizeof(*ctx->olaps), GFP_KERNEL);
>> + if (!ctx->olaps) {
>> + kfree(ctx->gem_objs);
>> + ctx->gem_objs = NULL;
>> + return -ENOMEM;
>> + }
>> + fastrpc_get_buff_overlaps(ctx);
>> + }
>> +
>> + return err;
>> +}
>> diff --git a/drivers/accel/qda/qda_fastrpc.h b/drivers/accel/qda/qda_fastrpc.h
>> new file mode 100644
>> index 000000000000..ce77baeccfba
>> --- /dev/null
>> +++ b/drivers/accel/qda/qda_fastrpc.h
>> @@ -0,0 +1,271 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>> + */
>> +
>> +#ifndef __QDA_FASTRPC_H__
>> +#define __QDA_FASTRPC_H__
>> +
>> +#include <linux/completion.h>
>> +#include <linux/kref.h>
>> +#include <linux/list.h>
>> +#include <linux/types.h>
>> +#include <drm/drm_drv.h>
>> +#include <drm/drm_file.h>
>> +#include <drm/qda_accel.h>
>> +
>> +/* Forward declarations */
>> +struct qda_gem_obj;
>> +
>> +/*
>> + * FastRPC scalar extraction macros
>> + *
>> + * These macros extract different fields from the scalar value that describes
>> + * the arguments passed in a FastRPC invocation.
>> + */
>> +#define REMOTE_SCALARS_INBUFS(sc) (((sc) >> 16) & 0x0ff)
>> +#define REMOTE_SCALARS_OUTBUFS(sc) (((sc) >> 8) & 0x0ff)
>> +#define REMOTE_SCALARS_INHANDLES(sc) (((sc) >> 4) & 0x0f)
>> +#define REMOTE_SCALARS_OUTHANDLES(sc) ((sc) & 0x0f)
>> +#define REMOTE_SCALARS_LENGTH(sc) (REMOTE_SCALARS_INBUFS(sc) + \
>> + REMOTE_SCALARS_OUTBUFS(sc) + \
>> + REMOTE_SCALARS_INHANDLES(sc) + \
>> + REMOTE_SCALARS_OUTHANDLES(sc))
>> +
>> +/* FastRPC configuration constants */
>> +#define FASTRPC_ALIGN 128 /* Alignment requirement */
>> +#define FASTRPC_MAX_FDLIST 16 /* Maximum file descriptors */
>> +#define FASTRPC_MAX_CRCLIST 64 /* Maximum CRC list entries */
>> +
>> +/*
>> + * FastRPC scalar construction macros
>> + *
>> + * These macros build the scalar value that describes the arguments
>> + * for a FastRPC invocation.
>> + */
>> +#define FASTRPC_BUILD_SCALARS(attr, method, in, out, oin, oout) \
>> + (((attr & 0x07) << 29) | \
>> + ((method & 0x1f) << 24) | \
>> + ((in & 0xff) << 16) | \
>> + ((out & 0xff) << 8) | \
>> + ((oin & 0x0f) << 4) | \
>> + (oout & 0x0f))
>> +
>> +#define FASTRPC_SCALARS(method, in, out) \
>> + FASTRPC_BUILD_SCALARS(0, method, in, out, 0, 0)
>> +
>> +/**
>> + * struct fastrpc_buf_overlap - Buffer overlap tracking structure
>> + *
>> + * Tracks overlapping buffer regions to optimise memory mapping and avoid
>> + * redundant mappings of the same physical memory.
>
> WHat for? Even if this is a valid optimization, implement it as a
> subsequent patch. The first goal should be very simple - get GEM buffers
> from the app, pass them to the DSP, read the results.
yes, this implementation is mimicking the existing fastrpc design where
non-FD buffers are also supported. I am currently evaluating the
maintainance of such buffers from userspace side and trying to
understand the impacts of the same. I am planning to bring it as a
future enhancement if there is no regression.>
>> + */
>> +struct fastrpc_buf_overlap {
>
> Stop clashing the names with the existing fastrpc driver.
ack.>
>> + /** @start: Start address of the buffer in user virtual address space */
>> + u64 start;
>> + /** @end: End address of the buffer in user virtual address space */
>> + u64 end;
>> + /** @raix: Remote argument index associated with this overlap */
>> + int raix;
>> + /** @mstart: Start address of the mapped region */
>> + u64 mstart;
>> + /** @mend: End address of the mapped region */
>> + u64 mend;
>> + /** @offset: Offset within the mapped region */
>> + u64 offset;
>> +};
>> +
>> +/**
>> + * struct fastrpc_remote_dmahandle - Remote DMA handle descriptor
>> + */
>> +struct fastrpc_remote_dmahandle {
>> + /** @fd: DMA-BUF file descriptor */
>> + s32 fd;
>> + /** @offset: Byte offset within the DMA-BUF */
>> + u32 offset;
>> + /** @len: Length of the region in bytes */
>> + u32 len;
>> +};
>> +
>> +/**
>> + * struct fastrpc_remote_buf - Remote buffer descriptor
>> + */
>> +struct fastrpc_remote_buf {
>> + /** @pv: Buffer pointer (user virtual address) */
>> + u64 pv;
>> + /** @len: Length of the buffer in bytes */
>> + u64 len;
>> +};
>> +
>> +/**
>> + * union fastrpc_remote_arg - Remote argument (buffer or DMA handle)
>> + */
>> +union fastrpc_remote_arg {
>> + /** @buf: Inline buffer descriptor */
>> + struct fastrpc_remote_buf buf;
>> + /** @dma: DMA-BUF handle descriptor */
>> + struct fastrpc_remote_dmahandle dma;
>> +};
>> +
>> +/**
>> + * struct fastrpc_phy_page - Physical page descriptor
>> + */
>> +struct fastrpc_phy_page {
>> + /** @addr: Physical (IOMMU) address of the page */
>> + u64 addr;
>> + /** @size: Size of the contiguous region in bytes */
>> + u64 size;
>> +};
>> +
>> +/**
>> + * struct fastrpc_invoke_buf - Invoke buffer descriptor
>> + */
>> +struct fastrpc_invoke_buf {
>> + /** @num: Number of contiguous physical regions */
>> + u32 num;
>> + /** @pgidx: Index into the physical page array */
>> + u32 pgidx;
>> +};
>> +
>> +/**
>> + * struct fastrpc_msg - FastRPC wire message for remote invocations
>> + *
>> + * Sent to the remote processor via RPMsg. This is the exact layout
>> + * the DSP expects; do not reorder or add fields without DSP firmware
>> + * coordination.
>> + */
>> +struct fastrpc_msg {
>> + /** @remote_session_id: Session identifier on the remote processor */
>> + int remote_session_id;
>> + /** @tid: Thread ID of the invoking thread */
>> + int tid;
>> + /** @ctx: Context identifier for matching request/response */
>> + u64 ctx;
>> + /** @handle: Handle of the remote method to invoke */
>> + u32 handle;
>> + /** @sc: Scalars value encoding in/out buffer counts */
>> + u32 sc;
>> + /** @addr: Physical address of the message payload buffer */
>> + u64 addr;
>> + /** @size: Size of the message payload in bytes */
>> + u64 size;
>> +};
>> +
>> +/**
>> + * struct qda_msg - FastRPC message with kernel-internal bookkeeping
>> + *
>> + * The wire-format portion is kept in the embedded @fastrpc member (must
>> + * be first) so that &qda_msg->fastrpc can be passed directly to
>> + * rpmsg_send() without a copy.
>> + */
>> +struct qda_msg {
>> + /**
>> + * @fastrpc: Wire-format message sent to the DSP via RPMsg.
>> + * Must be the first member.
>> + */
>> + struct fastrpc_msg fastrpc;
>> + /** @buf: Kernel virtual address of the payload buffer */
>> + void *buf;
>> + /** @phys: Physical/DMA address of the payload buffer */
>> + u64 phys;
>> + /** @ret: Return value from the remote processor */
>> + int ret;
>> + /** @fastrpc_ctx: Back-pointer to the owning invocation context */
>> + struct fastrpc_invoke_context *fastrpc_ctx;
>> + /** @file_priv: DRM file private data for GEM object lookup */
>> + struct drm_file *file_priv;
>> +};
>> +
>> +/**
>> + * struct fastrpc_invoke_context - Remote procedure call invocation context
>> + *
>> + * Maintains all state for a single remote procedure call, including buffer
>> + * management, synchronisation, and result handling.
>> + */
>> +struct fastrpc_invoke_context {
>> + /** @node: List node for linking contexts in a queue */
>> + struct list_head node;
>> + /** @ctxid: Unique context identifier (XArray key shifted left by 4) */
>> + u64 ctxid;
>> + /** @inbufs: Number of input buffers */
>> + int inbufs;
>> + /** @outbufs: Number of output buffers */
>> + int outbufs;
>> + /** @handles: Number of DMA-BUF handle arguments */
>> + int handles;
>> + /** @nscalars: Total number of scalar arguments */
>> + int nscalars;
>> + /** @nbufs: Total number of buffer arguments (inbufs + outbufs) */
>> + int nbufs;
>
> If it is inbufs + outbufs, why do you need it here?
>
>> + /** @pid: Process ID of the calling process */
>> + int pid;
>> + /** @retval: Return value from the remote invocation */
>> + int retval;
>> + /** @metalen: Length of the FastRPC metadata header in bytes */
>> + int metalen;
>
> size_t, also why do you need it?
>
>> + /** @remote_session_id: Session identifier on the remote processor */
>> + int remote_session_id;
>> + /** @pd: Protection domain identifier encoded into the context ID */
>> + int pd;
>> + /** @type: Invocation type (e.g. FASTRPC_RMID_INVOKE_DYNAMIC) */
>> + int type;
>> + /** @sc: Scalars value encoding in/out buffer counts */
>> + u32 sc;
>
> How is this different from the counts above?
sc carries the method id and handle counts. The reason to maintain count
separately is to avoid calculating it again and again.>
>> + /** @handle: Handle of the remote method being invoked */
>> + u32 handle;
>> + /** @crc: Pointer to CRC values for data integrity checking */
>> + u32 *crc;
>
> Add it later. It's unused. Drop all unused fields.
ack.>
>> + /** @fdlist: Pointer to array of DMA-BUF file descriptors */
>> + u64 *fdlist;
>
> Why do you need DMA-BUFs in the invocation context? They all should be
> GEM buffers.
the reason is that the users are dependent on FDs as they can import
buffers allocated from anywhere and there are DSP APIs which takes fd as
an argument, so they might end up using the same in there skel
implementation.>
>> + /** @pkt_size: Total payload size in bytes */
>> + u64 pkt_size;
>> + /** @aligned_pkt_size: Page-aligned payload size for GEM allocation */
>> + u64 aligned_pkt_size;
>> + /** @list: Array of invoke buffer descriptors */
>> + struct fastrpc_invoke_buf *list;
>> + /** @pages: Array of physical page descriptors for all arguments */
>> + struct fastrpc_phy_page *pages;
>> + /** @input_pages: Array of physical page descriptors for input buffers */
>> + struct fastrpc_phy_page *input_pages;
>
> I think you are trying to bring all the complexity from the old driver
> with no added benefit. Please don't. Use the existing memory manager.
> Let it handle all the gory details. If someting is not there, we should
> consider extending GEM instead.
I'm not changing the metadata format as the DSP might not understand the
messages if we modify it. Also, the fd is still being used because of
the client dependency on it. I'll check if there is any other logic that
needs alteration here.>
>> + /** @work: Completion used to synchronise with the DSP response */
>> + struct completion work;
>> + /** @msg: Pointer to the QDA message structure for this invocation */
>> + struct qda_msg *msg;
>> + /** @rpra: Array of remote procedure arguments */
>> + union fastrpc_remote_arg *rpra;
>> + /** @gem_objs: Array of GEM objects imported for argument buffers */
>> + struct drm_gem_object **gem_objs;
>> + /** @args: User-space invoke argument descriptors */
>> + struct drm_qda_fastrpc_invoke_args *args;
>> + /** @olaps: Array of buffer overlap descriptors for deduplication */
>> + struct fastrpc_buf_overlap *olaps;
>> + /** @refcount: Reference counter for context lifetime management */
>> + struct kref refcount;
>> + /** @msg_gem_obj: GEM object backing the message payload buffer */
>> + struct qda_gem_obj *msg_gem_obj;
>> + /** @file_priv: DRM file private data */
>> + struct drm_file *file_priv;
>> + /** @init_mem_gem_obj: GEM object for protection domain init memory */
>> + struct qda_gem_obj *init_mem_gem_obj;
>> + /** @req: Pointer to kernel-internal request buffer */
>> + void *req;
>> + /** @rsp: Pointer to kernel-internal response buffer */
>> + void *rsp;
>> + /** @inbuf: Pointer to kernel-internal input buffer */
>> + void *inbuf;
>> +};
>> +
>> +/* Remote Method ID table - identifies initialization and control operations */
>> +#define FASTRPC_RMID_INVOKE_DYNAMIC 0xFFFFFFFF /* Dynamic method invocation */
>> +
>> +/* Common handle for initialization operations */
>> +#define FASTRPC_INIT_HANDLE 0x1
>> +
>> +void qda_fastrpc_context_free(struct kref *ref);
>> +struct fastrpc_invoke_context *qda_fastrpc_context_alloc(void);
>> +int qda_fastrpc_prepare_args(struct fastrpc_invoke_context *ctx, char __user *argp);
>> +int qda_fastrpc_get_header_size(struct fastrpc_invoke_context *ctx, size_t *out_size);
>> +int qda_fastrpc_invoke_pack(struct fastrpc_invoke_context *ctx, struct qda_msg *msg);
>> +int qda_fastrpc_invoke_unpack(struct fastrpc_invoke_context *ctx, struct qda_msg *msg);
>> +
>> +#endif /* __QDA_FASTRPC_H__ */
>> diff --git a/drivers/accel/qda/qda_ioctl.c b/drivers/accel/qda/qda_ioctl.c
>> index 1769c85a3e98..c81268c20b04 100644
>> --- a/drivers/accel/qda/qda_ioctl.c
>> +++ b/drivers/accel/qda/qda_ioctl.c
>> @@ -3,8 +3,10 @@
>> #include <drm/drm_ioctl.h>
>> #include <drm/qda_accel.h>
>> #include "qda_drv.h"
>> +#include "qda_fastrpc.h"
>> #include "qda_gem.h"
>> #include "qda_ioctl.h"
>> +#include "qda_rpmsg.h"
>>
>> /**
>> * qda_ioctl_query() - Query DSP device information
>> @@ -74,3 +76,105 @@ int qda_ioctl_gem_mmap_offset(struct drm_device *dev, void *data, struct drm_fil
>>
>> return drm_gem_dumb_map_offset(file_priv, dev, args->handle, &args->offset);
>> }
>> +
>> +static int fastrpc_context_get_id(struct fastrpc_invoke_context *ctx, struct qda_dev *qdev)
>> +{
>> + int ret;
>> + u32 id;
>> +
>> + if (!qdev)
>> + return -EINVAL;
>> +
>> + ret = xa_alloc(&qdev->ctx_xa, &id, ctx, xa_limit_32b, GFP_KERNEL);
>> + if (ret)
>> + return ret;
>> +
>> + ctx->ctxid = id << 4;
>
> Why is it being shifted?
this is to accomodate PD type>
>> + return 0;
>> +}
>> +
>
^ permalink raw reply
* Re: [PATCH 13/15] accel/qda: Add DSP process creation and release
From: Ekansh Gupta @ 2026-06-04 5:17 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Oded Gabbay, Jonathan Corbet, Shuah Khan, Joerg Roedel,
Will Deacon, Robin Murphy, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Sumit Semwal,
Christian König, Bharath Kumar, Chenna Kesava Raju, srini,
andersson, konradybcio, robin.clark, linux-kernel, dri-devel,
linux-doc, linux-arm-msm, iommu, linux-media, linaro-mm-sig
In-Reply-To: <w44qzw2ryg7bpbte3hegopmtkfjd2gby532rdjarm4i3tylpv2@2rmruincfdgc>
On 20-05-2026 19:30, Dmitry Baryshkov wrote:
> On Tue, May 19, 2026 at 11:46:03AM +0530, Ekansh Gupta via B4 Relay wrote:
>> From: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
>>
>> Implement the REMOTE_SESSION_CREATE and INIT_RELEASE FastRPC
>> operations, which establish and tear down a user process on the
>> DSP.
>>
>> DRM_IOCTL_QDA_REMOTE_SESSION_CREATE (drm_qda_init_create)
>> Creates a new process on the DSP by sending an INIT_CREATE message
>> via the FastRPC INIT_HANDLE. The caller provides an ELF file (via
>> DMA-BUF fd or direct pointer) and optional process attributes. A
>> 4 MB GEM buffer is allocated per session to hold the DSP process
>> image; this buffer is stored in qda_file_priv and reused for the
>> lifetime of the session.
>>
>> If attrs is non-zero, INIT_CREATE_ATTR is used instead of
>> INIT_CREATE to pass the extended attribute and signature fields.
>
> What is the difference?
This attaches attributes with the PD that is being created, I'll add
more details>
>>
>> INIT_RELEASE
>> Sends a release message to the DSP when the DRM file is closed
>> (qda_postclose via qda_release_dsp_process), freeing the remote
>> process and its resources. The release is skipped if the device
>> has already been unplugged.
>>
>> qda_fastrpc.c
>> fastrpc_prepare_args_init_create() marshals the six-argument
>> create-process payload: the inbuf descriptor, process name,
>> ELF file, physical pages, attrs, and siglen.
>> fastrpc_prepare_args_release_process() marshals the single-
>> argument release payload (remote_session_id).
>>
>> qda_drv.c
>> qda_postclose() is extended to call qda_release_dsp_process()
>> under drm_dev_enter() so the release message is only sent while
>> the device is still accessible.
>>
>> Assisted-by: Claude:claude-4-6-sonnet
>> Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
>> ---
>> drivers/accel/qda/qda_drv.c | 8 +++
>> drivers/accel/qda/qda_drv.h | 5 ++
>> drivers/accel/qda/qda_fastrpc.c | 140 ++++++++++++++++++++++++++++++++++++++++
>> drivers/accel/qda/qda_fastrpc.h | 39 +++++++++--
>> drivers/accel/qda/qda_ioctl.c | 52 +++++++++++++++
>> drivers/accel/qda/qda_ioctl.h | 1 +
>> include/uapi/drm/qda_accel.h | 32 ++++++++-
>> 7 files changed, 270 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/accel/qda/qda_drv.c b/drivers/accel/qda/qda_drv.c
>> index 704c7d3127d2..4eaba9b050c0 100644
>> --- a/drivers/accel/qda/qda_drv.c
>> +++ b/drivers/accel/qda/qda_drv.c
>> @@ -36,6 +36,13 @@ static int qda_open(struct drm_device *dev, struct drm_file *file)
>> static void qda_postclose(struct drm_device *dev, struct drm_file *file)
>> {
>> struct qda_file_priv *qda_file_priv = file->driver_priv;
>> + int idx;
>> +
>> + /* Only send the DSP release message while the device is accessible */
>> + if (drm_dev_enter(dev, &idx)) {
>> + qda_release_dsp_process(qda_file_priv->qda_dev, file);
>> + drm_dev_exit(idx);
>> + }
>>
>> if (qda_file_priv->assigned_iommu_dev) {
>> struct qda_iommu_device *iommu_dev = qda_file_priv->assigned_iommu_dev;
>> @@ -59,6 +66,7 @@ static const struct drm_ioctl_desc qda_ioctls[] = {
>> DRM_IOCTL_DEF_DRV(QDA_QUERY, qda_ioctl_query, 0),
>> DRM_IOCTL_DEF_DRV(QDA_GEM_CREATE, qda_ioctl_gem_create, 0),
>> DRM_IOCTL_DEF_DRV(QDA_GEM_MMAP_OFFSET, qda_ioctl_gem_mmap_offset, 0),
>> + DRM_IOCTL_DEF_DRV(QDA_REMOTE_SESSION_CREATE, qda_ioctl_init_create, 0),
>
> Why is it being added in the middle?
no specific reason, I'll correct this.>
>> DRM_IOCTL_DEF_DRV(QDA_REMOTE_INVOKE, qda_ioctl_invoke, 0),
>> };
>>
>> diff --git a/drivers/accel/qda/qda_drv.h b/drivers/accel/qda/qda_drv.h
>> index 420cccff42bf..4b4639961d95 100644
>> --- a/drivers/accel/qda/qda_drv.h
>> +++ b/drivers/accel/qda/qda_drv.h
>> @@ -28,6 +28,8 @@ struct qda_file_priv {
>> struct qda_dev *qda_dev;
>> /** @assigned_iommu_dev: IOMMU device assigned to this process */
>> struct qda_iommu_device *assigned_iommu_dev;
>> + /** @init_mem_gem_obj: GEM object for PD initialization memory */
>> + struct qda_gem_obj *init_mem_gem_obj;
>> /** @pid: Process ID for tracking */
>> pid_t pid;
>> /** @remote_session_id: Unique session identifier */
>> @@ -83,4 +85,7 @@ void qda_deinit_device(struct qda_dev *qdev);
>> int qda_register_device(struct qda_dev *qdev);
>> void qda_unregister_device(struct qda_dev *qdev);
>>
>> +/* DSP process / protection domain management */
>> +int qda_release_dsp_process(struct qda_dev *qdev, struct drm_file *file_priv);
>> +
>> #endif /* __QDA_DRV_H__ */
>> diff --git a/drivers/accel/qda/qda_fastrpc.c b/drivers/accel/qda/qda_fastrpc.c
>> index 0ec37175a098..305915022b91 100644
>> --- a/drivers/accel/qda/qda_fastrpc.c
>> +++ b/drivers/accel/qda/qda_fastrpc.c
>> @@ -524,6 +524,138 @@ int qda_fastrpc_invoke_unpack(struct fastrpc_invoke_context *ctx,
>> return err;
>> }
>>
>> +static void setup_create_process_args(struct drm_qda_fastrpc_invoke_args *args,
>> + struct fastrpc_create_process_inbuf *inbuf,
>> + struct drm_qda_init_create *init,
>> + struct fastrpc_phy_page *pages)
>> +{
>> + args[0].ptr = (u64)(uintptr_t)inbuf;
>> + args[0].length = sizeof(*inbuf);
>> + args[0].fd = -1;
>> +
>> + args[1].ptr = (u64)(uintptr_t)current->comm;
>> + args[1].length = inbuf->namelen;
>> + args[1].fd = -1;
>> +
>> + args[2].ptr = (u64)init->file;
>> + args[2].length = inbuf->filelen;
>> + args[2].fd = init->filefd; /* DMA-BUF fd forwarded to DSP */
>> +
>> + args[3].ptr = (u64)(uintptr_t)pages;
>> + args[3].length = 1 * sizeof(*pages);
>> + args[3].fd = -1;
>> +
>> + args[4].ptr = (u64)(uintptr_t)&inbuf->attrs;
>> + args[4].length = sizeof(inbuf->attrs);
>> + args[4].fd = -1;
>> +
>> + args[5].ptr = (u64)(uintptr_t)&inbuf->siglen;
>> + args[5].length = sizeof(inbuf->siglen);
>> + args[5].fd = -1;
>> +}
>> +
>> +static void setup_single_arg(struct drm_qda_fastrpc_invoke_args *args, const void *ptr, size_t size)
>> +{
>> + args[0].ptr = (u64)(uintptr_t)ptr;
>> + args[0].length = size;
>> + args[0].fd = -1;
>> +}
>> +
>> +static int fastrpc_prepare_args_release_process(struct fastrpc_invoke_context *ctx)
>> +{
>> + struct drm_qda_fastrpc_invoke_args *args;
>> +
>> + args = kzalloc_obj(*args);
>> + if (!args)
>> + return -ENOMEM;
>> +
>> + setup_single_arg(args, &ctx->remote_session_id, sizeof(ctx->remote_session_id));
>> + ctx->sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_RELEASE, 1, 0);
>> + ctx->args = args;
>> + ctx->handle = FASTRPC_INIT_HANDLE;
>> +
>> + return 0;
>> +}
>> +
>> +static int fastrpc_prepare_args_init_create(struct fastrpc_invoke_context *ctx,
>> + char __user *argp)
>> +{
>> + struct drm_qda_init_create init;
>> + struct drm_qda_fastrpc_invoke_args *args;
>> + struct fastrpc_create_process_inbuf *inbuf;
>> + int err;
>> + u32 sc;
>> +
>> + args = kcalloc(FASTRPC_CREATE_PROCESS_NARGS, sizeof(*args), GFP_KERNEL);
>> + if (!args)
>> + return -ENOMEM;
>> +
>> + ctx->input_pages = kcalloc(1, sizeof(*ctx->input_pages), GFP_KERNEL);
>> + if (!ctx->input_pages) {
>> + err = -ENOMEM;
>> + goto err_free_args;
>> + }
>> +
>> + ctx->inbuf = kcalloc(1, sizeof(*inbuf), GFP_KERNEL);
>> + if (!ctx->inbuf) {
>> + err = -ENOMEM;
>> + goto err_free_input_pages;
>> + }
>> + inbuf = ctx->inbuf;
>> +
>> + memcpy(&init, argp, sizeof(init));
>> +
>> + if (init.filelen > FASTRPC_INIT_FILELEN_MAX) {
>> + err = -EINVAL;
>> + goto err_free_inbuf;
>> + }
>> +
>> + /*
>> + * Validate that the DMA-BUF fd is importable. The fd itself is kept
>> + * in init.filefd and forwarded to the DSP via setup_create_process_args().
>> + */
>> + if (init.filelen && init.filefd > 0) {
>> + struct drm_gem_object *file_gem_obj;
>> +
>> + err = get_gem_obj_from_dmabuf_fd(ctx, init.filefd, &file_gem_obj);
>> + if (err) {
>> + err = -EINVAL;
>> + goto err_free_inbuf;
>> + }
>> + drm_gem_object_put(file_gem_obj);
>> + }
>> +
>> + inbuf->remote_session_id = ctx->remote_session_id;
>> + inbuf->namelen = strlen(current->comm) + 1;
>> + inbuf->filelen = init.filelen;
>> + inbuf->pageslen = 1;
>> + inbuf->attrs = init.attrs;
>> + inbuf->siglen = init.siglen;
>> +
>> + setup_pages_from_gem_obj(ctx->init_mem_gem_obj, &ctx->input_pages[0]);
>> +
>> + setup_create_process_args(args, inbuf, &init, ctx->input_pages);
>> +
>> + sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_CREATE, 4, 0);
>> + if (init.attrs)
>> + sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_CREATE_ATTR, 4, 0);
>> + ctx->sc = sc;
>> + ctx->args = args;
>> + ctx->handle = FASTRPC_INIT_HANDLE;
>> +
>> + return 0;
>> +
>> +err_free_inbuf:
>> + kfree(ctx->inbuf);
>> + ctx->inbuf = NULL;
>> +err_free_input_pages:
>> + kfree(ctx->input_pages);
>> + ctx->input_pages = NULL;
>> +err_free_args:
>> + kfree(args);
>> + return err;
>> +}
>> +
>> static int fastrpc_prepare_args_invoke(struct fastrpc_invoke_context *ctx, char __user *argp)
>> {
>> struct drm_qda_invoke_args invoke_args;
>> @@ -568,6 +700,14 @@ int qda_fastrpc_prepare_args(struct fastrpc_invoke_context *ctx, char __user *ar
>> int err;
>>
>> switch (ctx->type) {
>> + case FASTRPC_RMID_INIT_RELEASE:
>> + err = fastrpc_prepare_args_release_process(ctx);
>> + break;
>> + case FASTRPC_RMID_INIT_CREATE:
>> + case FASTRPC_RMID_INIT_CREATE_ATTR:
>> + ctx->pd = QDA_USER_PD;
>> + err = fastrpc_prepare_args_init_create(ctx, argp);
>> + break;
>> case FASTRPC_RMID_INVOKE_DYNAMIC:
>> err = fastrpc_prepare_args_invoke(ctx, argp);
>> break;
>> diff --git a/drivers/accel/qda/qda_fastrpc.h b/drivers/accel/qda/qda_fastrpc.h
>> index ce77baeccfba..1c1236f9525e 100644
>> --- a/drivers/accel/qda/qda_fastrpc.h
>> +++ b/drivers/accel/qda/qda_fastrpc.h
>> @@ -127,6 +127,27 @@ struct fastrpc_invoke_buf {
>> u32 pgidx;
>> };
>>
>> +/**
>> + * struct fastrpc_create_process_inbuf - Input buffer for process creation
>> + *
>> + * This structure defines the input buffer format for creating a new
>> + * process on the remote DSP.
>> + */
>> +struct fastrpc_create_process_inbuf {
>> + /** @remote_session_id: Client identifier for the session */
>> + int remote_session_id;
>> + /** @namelen: Length of the process name string including NUL terminator */
>> + u32 namelen;
>> + /** @filelen: Length of the ELF shell file in bytes */
>> + u32 filelen;
>> + /** @pageslen: Number of physical page descriptors */
>> + u32 pageslen;
>> + /** @attrs: Process attribute flags */
>> + u32 attrs;
>> + /** @siglen: Length of the signature data in bytes */
>> + u32 siglen;
>> +};
>> +
>> /**
>> * struct fastrpc_msg - FastRPC wire message for remote invocations
>> *
>> @@ -153,10 +174,6 @@ struct fastrpc_msg {
>>
>> /**
>> * struct qda_msg - FastRPC message with kernel-internal bookkeeping
>> - *
>> - * The wire-format portion is kept in the embedded @fastrpc member (must
>> - * be first) so that &qda_msg->fastrpc can be passed directly to
>> - * rpmsg_send() without a copy.
>> */
>> struct qda_msg {
>> /**
>> @@ -245,7 +262,7 @@ struct fastrpc_invoke_context {
>> struct qda_gem_obj *msg_gem_obj;
>> /** @file_priv: DRM file private data */
>> struct drm_file *file_priv;
>> - /** @init_mem_gem_obj: GEM object for protection domain init memory */
>> + /** @init_mem_gem_obj: GEM object for PD initialization memory */
>> struct qda_gem_obj *init_mem_gem_obj;
>> /** @req: Pointer to kernel-internal request buffer */
>> void *req;
>> @@ -256,11 +273,23 @@ struct fastrpc_invoke_context {
>> };
>>
>> /* Remote Method ID table - identifies initialization and control operations */
>> +#define FASTRPC_RMID_INIT_RELEASE 1 /* Release DSP process */
>> +#define FASTRPC_RMID_INIT_CREATE 6 /* Create DSP process */
>> +#define FASTRPC_RMID_INIT_CREATE_ATTR 7 /* Create DSP process with attributes */
>> #define FASTRPC_RMID_INVOKE_DYNAMIC 0xFFFFFFFF /* Dynamic method invocation */
>>
>> /* Common handle for initialization operations */
>> #define FASTRPC_INIT_HANDLE 0x1
>>
>> +/* Protection Domain (PD) identifiers */
>> +#define QDA_ROOT_PD (0)
>> +#define QDA_USER_PD (1)
>> +
>> +/* Number of arguments for process creation */
>> +#define FASTRPC_CREATE_PROCESS_NARGS 6
>> +/* Maximum initialization file size (4 MB) */
>> +#define FASTRPC_INIT_FILELEN_MAX (4 * 1024 * 1024)
>> +
>> void qda_fastrpc_context_free(struct kref *ref);
>> struct fastrpc_invoke_context *qda_fastrpc_context_alloc(void);
>> int qda_fastrpc_prepare_args(struct fastrpc_invoke_context *ctx, char __user *argp);
>> diff --git a/drivers/accel/qda/qda_ioctl.c b/drivers/accel/qda/qda_ioctl.c
>> index c81268c20b04..33f0a798ad13 100644
>> --- a/drivers/accel/qda/qda_ioctl.c
>> +++ b/drivers/accel/qda/qda_ioctl.c
>> @@ -109,6 +109,7 @@ static int fastrpc_invoke(int type, struct drm_device *dev, void *data,
>> struct drm_gem_object *gem_obj;
>> int err;
>> size_t hdr_size;
>> + size_t initmem_size = FASTRPC_INIT_FILELEN_MAX;
>>
>> ctx = qda_fastrpc_context_alloc();
>> if (IS_ERR(ctx))
>> @@ -124,6 +125,27 @@ static int fastrpc_invoke(int type, struct drm_device *dev, void *data,
>> ctx->file_priv = file_priv;
>> ctx->remote_session_id = qda_file_priv->remote_session_id;
>>
>> + if (type == FASTRPC_RMID_INIT_CREATE) {
>> + struct drm_gem_object *initmem_gem_obj;
>> +
>> + if (qda_file_priv->init_mem_gem_obj) {
>
> Why is it non-NULL here?
I had added this check in case the init failed earlier and there is a
retry for the same, but now I see it's not correct and should be handled
in the error path. I'll fix this.>
>> + drm_gem_object_put(&qda_file_priv->init_mem_gem_obj->base);
>> + qda_file_priv->init_mem_gem_obj = NULL;
>> + }
>> +
>> + initmem_gem_obj = qda_gem_create_object(dev, qdev->iommu_mgr,
>> + initmem_size, file_priv);
>> + if (IS_ERR(initmem_gem_obj)) {
>> + err = PTR_ERR(initmem_gem_obj);
>> + goto err_context_free;
>> + }
>> +
>> + ctx->init_mem_gem_obj = to_qda_gem_obj(initmem_gem_obj);
>> + qda_file_priv->init_mem_gem_obj = ctx->init_mem_gem_obj;
>> + } else if (type == FASTRPC_RMID_INIT_RELEASE) {
>> + ctx->init_mem_gem_obj = qda_file_priv->init_mem_gem_obj;
>> + }
>> +
>> err = qda_fastrpc_prepare_args(ctx, (char __user *)data);
>> if (err)
>> goto err_context_free;
>> @@ -161,11 +183,41 @@ static int fastrpc_invoke(int type, struct drm_device *dev, void *data,
>> return 0;
>>
>> err_context_free:
>> + if (type == FASTRPC_RMID_INIT_RELEASE && !err && qda_file_priv->init_mem_gem_obj) {
>> + drm_gem_object_put(&qda_file_priv->init_mem_gem_obj->base);
>> + qda_file_priv->init_mem_gem_obj = NULL;
>> + }
>> +
>> fastrpc_context_put_id(ctx, qdev);
>> kref_put(&ctx->refcount, qda_fastrpc_context_free);
>> return err;
>> }
>>
>> +/**
>> + * qda_ioctl_init_create() - Create a DSP process
>> + * @dev: DRM device structure
>> + * @data: User-space data (struct drm_qda_init_create)
>> + * @file_priv: DRM file private data
>> + *
>> + * Return: 0 on success, negative error code on failure
>> + */
>> +int qda_ioctl_init_create(struct drm_device *dev, void *data, struct drm_file *file_priv)
>> +{
>> + return fastrpc_invoke(FASTRPC_RMID_INIT_CREATE, dev, data, file_priv);
>
> Where is INIT_CREATE_ATTR, which you described earlier?
INIT_CREATE_ATTR is used while `sc` creation so the DSP considers the
request is coming with some attributes, the ioctl functions are going to
be the same in both the cases, so keeping it unchanged and the decision
is taken while `sc` is getting created.>
>> +}
>> +
>> +/**
>> + * qda_release_dsp_process() - Release DSP process resources for a file
>> + * @qdev: QDA device structure
>> + * @file_priv: DRM file private data
>> + *
>> + * Return: 0 on success, negative error code on failure
>> + */
>> +int qda_release_dsp_process(struct qda_dev *qdev, struct drm_file *file_priv)
>> +{
>> + return fastrpc_invoke(FASTRPC_RMID_INIT_RELEASE, &qdev->drm_dev, NULL, file_priv);
>> +}
>> +
>> /**
>> * qda_ioctl_invoke() - Perform a dynamic FastRPC method invocation
>> * @dev: DRM device structure
>> diff --git a/drivers/accel/qda/qda_ioctl.h b/drivers/accel/qda/qda_ioctl.h
>> index 3bb9cfd98370..192565434363 100644
>> --- a/drivers/accel/qda/qda_ioctl.h
>> +++ b/drivers/accel/qda/qda_ioctl.h
>> @@ -9,6 +9,7 @@
>> #include "qda_drv.h"
>>
>> int qda_ioctl_query(struct drm_device *dev, void *data, struct drm_file *file_priv);
>> +int qda_ioctl_init_create(struct drm_device *dev, void *data, struct drm_file *file_priv);
>> int qda_ioctl_gem_create(struct drm_device *dev, void *data, struct drm_file *file_priv);
>> int qda_ioctl_gem_mmap_offset(struct drm_device *dev, void *data, struct drm_file *file_priv);
>> int qda_ioctl_invoke(struct drm_device *dev, void *data, struct drm_file *file_priv);
>> diff --git a/include/uapi/drm/qda_accel.h b/include/uapi/drm/qda_accel.h
>> index 72512213741f..711e2523a570 100644
>> --- a/include/uapi/drm/qda_accel.h
>> +++ b/include/uapi/drm/qda_accel.h
>> @@ -21,8 +21,9 @@ extern "C" {
>> #define DRM_QDA_QUERY 0x00
>> #define DRM_QDA_GEM_CREATE 0x01
>> #define DRM_QDA_GEM_MMAP_OFFSET 0x02
>> -/* Command numbers 0x03-0x06 reserved for INIT_ATTACH, INIT_CREATE, MAP, MUNMAP */
>> -#define DRM_QDA_REMOTE_INVOKE 0x07
>> +/* Command number 0x03 reserved for INIT_ATTACH; 0x05-0x06 reserved for MAP, MUNMAP */
>> +#define DRM_QDA_REMOTE_SESSION_CREATE 0x04
>> +#define DRM_QDA_REMOTE_INVOKE 0x07
>>
>> /*
>> * QDA IOCTL definitions
>> @@ -37,6 +38,9 @@ extern "C" {
>> struct drm_qda_gem_create)
>> #define DRM_IOCTL_QDA_GEM_MMAP_OFFSET DRM_IOWR(DRM_COMMAND_BASE + DRM_QDA_GEM_MMAP_OFFSET, \
>> struct drm_qda_gem_mmap_offset)
>> +#define DRM_IOCTL_QDA_REMOTE_SESSION_CREATE \
>> + DRM_IOWR(DRM_COMMAND_BASE + DRM_QDA_REMOTE_SESSION_CREATE, \
>> + struct drm_qda_init_create)
>> #define DRM_IOCTL_QDA_REMOTE_INVOKE DRM_IOWR(DRM_COMMAND_BASE + DRM_QDA_REMOTE_INVOKE, \
>> struct drm_qda_invoke_args)
>>
>> @@ -99,6 +103,30 @@ struct drm_qda_fastrpc_invoke_args {
>> __u32 attr;
>> };
>>
>> +/**
>> + * struct drm_qda_init_create - Accelerator process initialization parameters
>> + * @filelen: Length of the ELF file in bytes
>> + * @filefd: DMA-BUF file descriptor containing the ELF file
>> + * @attrs: Process attributes flags
>> + * @siglen: Length of signature data in bytes
>> + * @file: Pointer to ELF file data if not using filefd
>> + *
>> + * This structure is used with DRM_IOCTL_QDA_INIT_CREATE to initialize
>> + * a new process on the accelerator. The process code is provided either
>> + * via a file descriptor (filefd, typically a GEM object) or a direct
>> + * pointer (file). Set file to 0 if using filefd.
>> + *
>> + * The attrs field contains bit flags for debug mode, privileged execution,
>> + * and other process attributes.
>> + */
>> +struct drm_qda_init_create {
>> + __u32 filelen;
>> + __s32 filefd;
>> + __u32 attrs;
>> + __u32 siglen;
>> + __u64 file;
>> +};
>> +
>> /**
>> * struct drm_qda_invoke_args - Dynamic FastRPC invocation parameters
>> * @handle: Remote handle to invoke on the DSP
>>
>> --
>> 2.34.1
>>
>>
>
^ permalink raw reply
* Re: [PATCH v3 1/4] mm/zswap: Make shrink_worker writeback cursor per-memcg
From: Yosry Ahmed @ 2026-06-04 5:34 UTC (permalink / raw)
To: Hao Jia
Cc: akpm, tj, hannes, shakeel.butt, mhocko, mkoutny, nphamcs,
chengming.zhou, muchun.song, roman.gushchin, cgroups, linux-mm,
linux-kernel, linux-doc, Hao Jia
In-Reply-To: <9898f83d-fae9-e284-6b85-c7f4089840a0@gmail.com>
> >> For instance, suppose a parent memcg has two children, memcg1 and memcg2,
> >> each with 200MB of zswap (100MB inactive). Triggering proactive writeback on
> >> the parent memcg will exhaust memcg1's inactive zswap pages. After that,
> >> even though memcg2 still has plenty of inactive zswap pages, it will
> >> continue to write back memcg1's active zswap pages. Writing back active
> >> zswap pages causes the user-space agent to prematurely abort the writeback
> >> because it detects that certain memcg metrics have exceeded predefined
> >> thresholds.
> >
> > This will only happen if the reclaim size is smaller than the batch
> > size, right? Otherwise the kernel should reclaim more or less equally
> > from both memcgs?
> >
>
> I gave it some thought. Not using a cursor could lead to unfairness
> issues with certain writeback sizes:
>
> - If the writeback size is an odd multiple of WB_BATCH (e.g.,
> triggering a writeback of 3 * WB_BATCH), with 2 child cgroups, the
> writeback ratio might end up being 2:1.
> - If a memcg has 5 child cgroups and a writeback of 2 * WB_BATCH is
> triggered, it might repeatedly write back from only the first 2 child
> cgroups.
>
> Although setting a smaller WB_BATCH might mitigate this unfairness, it
> could hurt writeback efficiency. Let's just use per-memcg cursors to
> completely fix these corner cases.
Exactly, the batch size should be small enough that any unfairness is
not a problem. I would honestly just do batching without a per-memcg
cursor, unless we have numbers to prove that the efficiency is
affected when we use a small batch size. Let's only introduce
complexity when needed please.
^ permalink raw reply
* Re: [PATCH v3 2/4] mm/zswap: Implement proactive writeback
From: Yosry Ahmed @ 2026-06-04 5:36 UTC (permalink / raw)
To: Hao Jia, Johannes Weiner, shakeel.butt, mhocko, tj, mkoutny,
roman.gushchin
Cc: Nhat Pham, akpm, chengming.zhou, muchun.song, cgroups, linux-mm,
linux-kernel, linux-doc, Hao Jia
In-Reply-To: <6db27a22-cc7a-9a94-db3f-c912fd39aa32@gmail.com>
> >> But doesn't it make more sense to specify the compressed size, which is
> >> ultimately the amount of memory you actually want to reclaim.
> >>
> >
> > I personally prefer compressed size to pre-compressed size. That's
> > kinda what user cares about, no?
> >
> > One thing we can do is let users prescribe a compressed size, but
> > internally, we can multiply that by the average compression ratio.
> > That gives us a guesstimate of how many pages we need to reclaim, and
> > you can follow the rest of your implementation as is (perhaps with
> > short-circuit when we reach the goal with fewer pages reclaimed).
>
> Got it. I will change it to use the compressed size in the next version.
>
> Yosry, Nhat, should we continue using the zswap_writeback_only key to
> trigger proactive writeback?
I *really* want the memcg maintainers to chime in here, it's
ultimately their call.
Michal? Johannes? Shakeel? Roman? Anyone? :D
^ permalink raw reply
* Re: [PATCH net-next] docs: exclude driver and netdevsim bugs
From: kernel test robot @ 2026-06-04 6:03 UTC (permalink / raw)
To: Jakub Kicinski, davem
Cc: oe-kbuild-all, netdev, edumazet, pabeni, andrew+netdev, horms,
johannes, Jakub Kicinski, corbet, skhan, workflows, linux-doc
In-Reply-To: <20260603162943.2406080-1-kuba@kernel.org>
Hi Jakub,
kernel test robot noticed the following build warnings:
[auto build test WARNING on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Jakub-Kicinski/docs-exclude-driver-and-netdevsim-bugs/20260604-003949
base: net-next/main
patch link: https://lore.kernel.org/r/20260603162943.2406080-1-kuba%40kernel.org
patch subject: [PATCH net-next] docs: exclude driver and netdevsim bugs
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project f43d6834093b19baf79beda8c0337ab020ac5f17)
docutils: docutils (Docutils 0.21.2, Python 3.13.5, on linux)
reproduce: (https://download.01.org/0day-ci/archive/20260604/202606040736.mHgLfIxh-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202606040736.mHgLfIxh-lkp@intel.com/
All warnings (new ones prefixed by >>):
Checksumming on output with GSO
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [docutils]
Documentation/process/maintainer-netdev.rst:295: ERROR: Unexpected indentation. [docutils]
>> Documentation/process/maintainer-netdev.rst:299: WARNING: Block quote ends without a blank line; unexpected unindent. [docutils]
MAINTAINERS:40: WARNING: Inline strong start-string without end-string. [docutils]
Documentation/userspace-api/landlock:504: ./security/landlock/errata/abi-4.h:5: ERROR: Unexpected section title.
vim +299 Documentation/process/maintainer-netdev.rst
292
293 Additionally, netdev does not consider bugs to be ``net``-worthy
294 if they fulfill **all** of the following criteria:
295 - bug is in a hardware device driver;
296 - bug is either a missing error handling or is part of the error handling flow;
297 - bug was discovered by a static analysis / AI tool;
298 - bug was triggered/observed only with kernel changes or fault injection.
> 299 Fixes for such bugs should default to ``net-next`` and should **not** contain
300 a Fixes tag. Networking or driver maintainers may redirect such fixes to ``net``
301 at their discretion if they consider the condition to be relevant enough.
302
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* RE: [PATCH 01/11] net: wwan: t9xx: Add PCIe core
From: Wu. JackBB (GSM) @ 2026-06-04 6:42 UTC (permalink / raw)
To: Jagielski, Jedrzej, Loic Poulain, Sergey Ryazanov, Johannes Berg,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Wen-Zhi Huang, Shi-Wei Yeh, Minano Tseng,
Matthias Brugger, AngeloGioacchino Del Regno, Simon Horman,
Jonathan Corbet, Shuah Khan
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-mediatek@lists.infradead.org, linux-doc@vger.kernel.org
In-Reply-To: <PH0PR11MB5902127C590230B9FE50F78AF0152@PH0PR11MB5902.namprd11.prod.outlook.com>
> Sent: Friday, May 29, 2026 12:32 PM
>
> > +
>
> please also take a look on sashiko notes, there is some number of them
Hi Jagielski,
Thank you for your review. We have fixed some issues and are still discussing others with MediaTek. All of them will be addressed in V2.
Regarding sashiko notes, how should I handle them if discussion is needed? I cannot find sashiko's email address, and its website does not have a reply option.
For example:
https://sashiko.dev/#/patchset/20260529-t9xx_driver_v1-v1-0-bdbfe2c01e57%40compal.com?part=2
Q1:
The commit message mentions implementing TX and RX services, but the patch primarily adds empty structures and boilerplate code. Is the patch missing the actual TX/RX implementation described here?
Reply:
We plan to update the commit message. Would the following be acceptable?
Add the control plane transaction layer framework for the t9xx
WWAN driver, including configuration options, device structure
definitions, and initialization/cleanup functions.
The actual TX/RX service implementations that use this framework
are introduced in subsequent patches.
Thanks.
================================================================================================================================================================
This message may contain information which is private, privileged or confidential of Compal Electronics, Inc. If you are not the intended recipient of this message, please notify the sender and destroy/delete the message. Any review, retransmission, dissemination or other use of, or taking of any action in reliance upon this information, by persons or entities other than the intended recipient is prohibited.
================================================================================================================================================================
^ permalink raw reply
* [PATCH 00/10] perf hisi-ptt: Enhance TLP packet decoder with field-level parsing and versioning
From: Sizhe Liu @ 2026-06-04 7:49 UTC (permalink / raw)
To: rostedt, mhiramat, mathieu.desnoyers, corbet, skhan, bhelgaas,
yangyccccc, jic23, john.g.garry, will, james.clark, mike.leach,
leo.yan, peterz, mingo, acme, namhyung, mark.rutland,
alexander.shishkin, jolsa, irogers, adrian.hunter, wangyushan12,
shenyang39, gaozhihao6, yuzhichengcheng, liyihang9
Cc: linux-kernel, linux-pci, linux-perf-users, linux-arm-kernel,
linux-doc, linuxarm, prime.zeng, fanghao11, wuyifan50, liusizhe5
This series enhances the HiSilicon PTT (PCIe Trace and Tuning) packet
decoder in perf to provide detailed field-level parsing of TLP headers
based on message type, and adds a versioning mechanism for backward
compatibility.
The original decoder only prints raw hex values with generic field
names for each DW, without classifying TLP types or decoding
DW2/DW3 fields according to the hardware-defined packet format.
Patch overview:
Patches 1-2: Bug fixes
- Fix spelling/abbreviation errors (FIELD_LENTH, kpt_desc)
- Fix DW0 bit field ordering and DW hex value printing
Patches 3-7: Refactoring for field-level parsing
- Rename union and add named struct member for multi-DW reuse
- Abstract trace buffer/offset into struct hisi_ptt_pkt_buf
- Complete missing field name entries in 4DW/8DW tables
- Extract raw data printing into function
- Unify 4DW/8DW HEAD0 printing into hisi_ptt_print_head0()
Patch 8: TLP message type classification
- Parse Format/Type from DW0 to classify packets as
MWr/Msg/Atomic/IO/Cfg/Cpl
Patch 9: Field-level DW2/DW3 parsing
- Decode DW2 fields for MWr/Msg/Atomic/IO TLPs
- Decode DW3 fields for Completion and Configuration TLPs
Patch 10: Version compatibility
- Add V1/V2 version in auxtrace info to distinguish old
(generic field names) from new (detailed field parsing)
trace data, with backward compatibility for V1 files
Sizhe Liu (10):
perf hisi-ptt: Fix spelling and abbreviation errors
perf hisi-ptt: Fix PTT trace TLP Header parsing
perf hisi-ptt: Rename hisi_ptt_4dw union for reuse
perf hisi-ptt: Abstract trace data buf and offset
perf hisi-ptt: Complete the field names for 4DW and 8DW packets
perf hisi-ptt: Extract the raw data printing part
perf hisi-ptt: Merge 4DW and 8DW HEAD0 printing
perf hisi-ptt: Add parsing of supported message types
perf hisi-ptt: Add field-level parsing for header DW2/DW3
perf hisi-ptt: Add decoder version compatibility
Documentation/trace/hisi-ptt.rst | 28 +-
tools/perf/arch/arm64/util/hisi-ptt.c | 2 +
.../hisi-ptt-decoder/hisi-ptt-pkt-decoder.c | 352 +++++++++++++++---
.../hisi-ptt-decoder/hisi-ptt-pkt-decoder.h | 27 +-
tools/perf/util/hisi-ptt.c | 46 ++-
tools/perf/util/hisi-ptt.h | 4 +-
6 files changed, 365 insertions(+), 94 deletions(-)
--
2.33.0
^ permalink raw reply
* [PATCH 01/10] perf hisi-ptt: Fix spelling and abbreviation errors
From: Sizhe Liu @ 2026-06-04 7:49 UTC (permalink / raw)
To: rostedt, mhiramat, mathieu.desnoyers, corbet, skhan, bhelgaas,
yangyccccc, jic23, john.g.garry, will, james.clark, mike.leach,
leo.yan, peterz, mingo, acme, namhyung, mark.rutland,
alexander.shishkin, jolsa, irogers, adrian.hunter, wangyushan12,
shenyang39, gaozhihao6, yuzhichengcheng, liyihang9
Cc: linux-kernel, linux-pci, linux-perf-users, linux-arm-kernel,
linux-doc, linuxarm, prime.zeng, fanghao11, wuyifan50, liusizhe5
In-Reply-To: <20260604075005.2219785-1-liusizhe5@huawei.com>
Fix spelling and abbreviation errors in the PTT packet decoder:
- HISI_PTT_FIELD_LENTH -> HISI_PTT_FIELD_LENGTH
- hisi_ptt_8dw_kpt_desc -> hisi_ptt_8dw_pkt_desc
- hisi_ptt_4dw_kpt_desc -> hisi_ptt_4dw_pkt_desc
Cc: stable@vger.kernel.org
Fixes: 5e91e57e6809 ("perf auxtrace arm64: Add support for parsing HiSilicon PCIe Trace packet")
Signed-off-by: Sizhe Liu <liusizhe5@huawei.com>
---
.../hisi-ptt-decoder/hisi-ptt-pkt-decoder.c | 20 +++++++++----------
.../hisi-ptt-decoder/hisi-ptt-pkt-decoder.h | 2 +-
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
index a17c423a526d..c48b2ce7c4a3 100644
--- a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
+++ b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
@@ -94,26 +94,26 @@ static void hisi_ptt_print_pkt(const unsigned char *buf, int pos, const char *de
printf(".");
color_fprintf(stdout, color, " %08x: ", pos);
- for (i = 0; i < HISI_PTT_FIELD_LENTH; i++)
+ for (i = 0; i < HISI_PTT_FIELD_LENGTH; i++)
color_fprintf(stdout, color, "%02x ", buf[pos + i]);
for (i = 0; i < HISI_PTT_MAX_SPACE_LEN; i++)
color_fprintf(stdout, color, " ");
color_fprintf(stdout, color, " %s\n", desc);
}
-static int hisi_ptt_8dw_kpt_desc(const unsigned char *buf, int pos)
+static int hisi_ptt_8dw_pkt_desc(const unsigned char *buf, int pos)
{
int i;
for (i = 0; i < HISI_PTT_8DW_TYPE_MAX; i++) {
/* Do not show 8DW check field and reserved fields */
if (i == HISI_PTT_8DW_CHK_AND_RSV0 || i == HISI_PTT_8DW_RSV1) {
- pos += HISI_PTT_FIELD_LENTH;
+ pos += HISI_PTT_FIELD_LENGTH;
continue;
}
hisi_ptt_print_pkt(buf, pos, hisi_ptt_8dw_pkt_field_name[i]);
- pos += HISI_PTT_FIELD_LENTH;
+ pos += HISI_PTT_FIELD_LENGTH;
}
return hisi_ptt_pkt_size[HISI_PTT_8DW_PKT];
@@ -128,7 +128,7 @@ static void hisi_ptt_4dw_print_dw0(const unsigned char *buf, int pos)
dw0.value = *(uint32_t *)(buf + pos);
printf(".");
color_fprintf(stdout, color, " %08x: ", pos);
- for (i = 0; i < HISI_PTT_FIELD_LENTH; i++)
+ for (i = 0; i < HISI_PTT_FIELD_LENGTH; i++)
color_fprintf(stdout, color, "%02x ", buf[pos + i]);
for (i = 0; i < HISI_PTT_MAX_SPACE_LEN; i++)
color_fprintf(stdout, color, " ");
@@ -140,16 +140,16 @@ static void hisi_ptt_4dw_print_dw0(const unsigned char *buf, int pos)
dw0.len, "Time", dw0.time);
}
-static int hisi_ptt_4dw_kpt_desc(const unsigned char *buf, int pos)
+static int hisi_ptt_4dw_pkt_desc(const unsigned char *buf, int pos)
{
int i;
hisi_ptt_4dw_print_dw0(buf, pos);
- pos += HISI_PTT_FIELD_LENTH;
+ pos += HISI_PTT_FIELD_LENGTH;
for (i = 0; i < HISI_PTT_4DW_TYPE_MAX; i++) {
hisi_ptt_print_pkt(buf, pos, hisi_ptt_4dw_pkt_field_name[i]);
- pos += HISI_PTT_FIELD_LENTH;
+ pos += HISI_PTT_FIELD_LENGTH;
}
return hisi_ptt_pkt_size[HISI_PTT_4DW_PKT];
@@ -158,7 +158,7 @@ static int hisi_ptt_4dw_kpt_desc(const unsigned char *buf, int pos)
int hisi_ptt_pkt_desc(const unsigned char *buf, int pos, enum hisi_ptt_pkt_type type)
{
if (type == HISI_PTT_8DW_PKT)
- return hisi_ptt_8dw_kpt_desc(buf, pos);
+ return hisi_ptt_8dw_pkt_desc(buf, pos);
- return hisi_ptt_4dw_kpt_desc(buf, pos);
+ return hisi_ptt_4dw_pkt_desc(buf, pos);
}
diff --git a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h
index e78f1b5bc836..6772b16b817b 100644
--- a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h
+++ b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h
@@ -13,7 +13,7 @@
#define HISI_PTT_8DW_CHECK_MASK GENMASK(31, 11)
#define HISI_PTT_IS_8DW_PKT GENMASK(31, 11)
#define HISI_PTT_MAX_SPACE_LEN 10
-#define HISI_PTT_FIELD_LENTH 4
+#define HISI_PTT_FIELD_LENGTH 4
enum hisi_ptt_pkt_type {
HISI_PTT_4DW_PKT,
--
2.33.0
^ permalink raw reply related
* [PATCH 03/10] perf hisi-ptt: Rename hisi_ptt_4dw union for reuse
From: Sizhe Liu @ 2026-06-04 7:49 UTC (permalink / raw)
To: rostedt, mhiramat, mathieu.desnoyers, corbet, skhan, bhelgaas,
yangyccccc, jic23, john.g.garry, will, james.clark, mike.leach,
leo.yan, peterz, mingo, acme, namhyung, mark.rutland,
alexander.shishkin, jolsa, irogers, adrian.hunter, wangyushan12,
shenyang39, gaozhihao6, yuzhichengcheng, liyihang9
Cc: linux-kernel, linux-pci, linux-perf-users, linux-arm-kernel,
linux-doc, linuxarm, prime.zeng, fanghao11, wuyifan50, liusizhe5
In-Reply-To: <20260604075005.2219785-1-liusizhe5@huawei.com>
Rename union hisi_ptt_4dw to hisi_ptt_field_data so that it can hold
bit field layouts for different DW headers. Add the struct member name
dw0_4dw to indicate that the inner struct is for 4DW format DW0.
Signed-off-by: Sizhe Liu <liusizhe5@huawei.com>
---
.../hisi-ptt-decoder/hisi-ptt-pkt-decoder.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
index 67024f18ebbb..5daae2eaf435 100644
--- a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
+++ b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
@@ -74,7 +74,8 @@ static const char * const hisi_ptt_4dw_pkt_field_name[] = {
[HISI_PTT_4DW_HEAD3] = "Header DW3",
};
-union hisi_ptt_4dw {
+union hisi_ptt_field_data {
+ /* Header DW0 for 4DW format */
struct {
uint32_t time : 11;
uint32_t len : 10;
@@ -84,7 +85,7 @@ union hisi_ptt_4dw {
uint32_t t9 : 1;
uint32_t type : 5;
uint32_t format : 2;
- };
+ } dw0_4dw;
uint32_t value;
};
@@ -128,15 +129,15 @@ static int hisi_ptt_8dw_pkt_desc(const unsigned char *buf, int pos)
static void hisi_ptt_4dw_print_dw0(const unsigned char *buf, int pos)
{
const char *color = PERF_COLOR_BLUE;
- union hisi_ptt_4dw dw0;
+ union hisi_ptt_field_data dw;
uint8_t byte;
int i;
- dw0.value = le32_to_cpu(*(__le32 *)(buf + pos));
+ dw.value = le32_to_cpu(*(__le32 *)(buf + pos));
printf(".");
color_fprintf(stdout, color, " %08x: ", pos);
for (i = 0; i < HISI_PTT_FIELD_LENGTH; i++) {
- byte = (dw0.value >> (24 - i * 8)) & 0xFF;
+ byte = (dw.value >> (24 - i * 8)) & 0xFF;
color_fprintf(stdout, color, "%02x ", byte);
}
for (i = 0; i < HISI_PTT_MAX_SPACE_LEN; i++)
@@ -144,9 +145,10 @@ static void hisi_ptt_4dw_print_dw0(const unsigned char *buf, int pos)
color_fprintf(stdout, color,
" %s %x %s %x %s %x %s %x %s %x %s %x %s %x %s %x\n",
- "Format", dw0.format, "Type", dw0.type, "T9", dw0.t9,
- "T8", dw0.t8, "TH", dw0.th, "SO", dw0.so, "Length",
- dw0.len, "Time", dw0.time);
+ "Format", dw.dw0_4dw.format, "Type", dw.dw0_4dw.type,
+ "T9", dw.dw0_4dw.t9, "T8", dw.dw0_4dw.t8,
+ "TH", dw.dw0_4dw.th, "SO", dw.dw0_4dw.so,
+ "Length", dw.dw0_4dw.len, "Time", dw.dw0_4dw.time);
}
static int hisi_ptt_4dw_pkt_desc(const unsigned char *buf, int pos)
--
2.33.0
^ permalink raw reply related
* [PATCH 05/10] perf hisi-ptt: Complete the field names for 4DW and 8DW packets
From: Sizhe Liu @ 2026-06-04 7:50 UTC (permalink / raw)
To: rostedt, mhiramat, mathieu.desnoyers, corbet, skhan, bhelgaas,
yangyccccc, jic23, john.g.garry, will, james.clark, mike.leach,
leo.yan, peterz, mingo, acme, namhyung, mark.rutland,
alexander.shishkin, jolsa, irogers, adrian.hunter, wangyushan12,
shenyang39, gaozhihao6, yuzhichengcheng, liyihang9
Cc: linux-kernel, linux-pci, linux-perf-users, linux-arm-kernel,
linux-doc, linuxarm, prime.zeng, fanghao11, wuyifan50, liusizhe5
In-Reply-To: <20260604075005.2219785-1-liusizhe5@huawei.com>
Add the missing HISI_PTT_4DW_HEAD0 entry to the 4DW field name table
and add the HISI_PTT_8DW_CHK_AND_RSV0/HISI_PTT_8DW_RSV1 entries to
the 8DW field name table so that all DW indices have corresponding
names.
Replace the variable in the printing loop with enum members.
8DW format is like:
bits [ 31:11 ][ 10:0 ]
|---------------------------------------|-------------------|
DW0 [ 0x1fffff ][ Reserved (0x7ff) ]
DW1 [ Prefix ]
DW2 [ Header DW0 ]
DW3 [ Header DW1 ]
DW4 [ Header DW2 ]
DW5 [ Header DW3 ]
DW6 [ Reserved (0x0) ]
DW7 [ Time ]
Signed-off-by: Sizhe Liu <liusizhe5@huawei.com>
---
.../hisi-ptt-decoder/hisi-ptt-pkt-decoder.c | 21 +++++++++++--------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
index 201ca948c4fb..33804bcd0642 100644
--- a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
+++ b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
@@ -53,6 +53,7 @@ enum hisi_ptt_8dw_pkt_field_type {
};
enum hisi_ptt_4dw_pkt_field_type {
+ HISI_PTT_4DW_HEAD0,
HISI_PTT_4DW_HEAD1,
HISI_PTT_4DW_HEAD2,
HISI_PTT_4DW_HEAD3,
@@ -60,15 +61,18 @@ enum hisi_ptt_4dw_pkt_field_type {
};
static const char * const hisi_ptt_8dw_pkt_field_name[] = {
- [HISI_PTT_8DW_PREFIX] = "Prefix",
- [HISI_PTT_8DW_HEAD0] = "Header DW0",
- [HISI_PTT_8DW_HEAD1] = "Header DW1",
- [HISI_PTT_8DW_HEAD2] = "Header DW2",
- [HISI_PTT_8DW_HEAD3] = "Header DW3",
- [HISI_PTT_8DW_TIME] = "Time"
+ [HISI_PTT_8DW_CHK_AND_RSV0] = "CHK & RSV0",
+ [HISI_PTT_8DW_PREFIX] = "Prefix",
+ [HISI_PTT_8DW_HEAD0] = "Header DW0",
+ [HISI_PTT_8DW_HEAD1] = "Header DW1",
+ [HISI_PTT_8DW_HEAD2] = "Header DW2",
+ [HISI_PTT_8DW_HEAD3] = "Header DW3",
+ [HISI_PTT_8DW_RSV1] = "RSV1",
+ [HISI_PTT_8DW_TIME] = "Time"
};
static const char * const hisi_ptt_4dw_pkt_field_name[] = {
+ [HISI_PTT_4DW_HEAD0] = "Header DW0",
[HISI_PTT_4DW_HEAD1] = "Header DW1",
[HISI_PTT_4DW_HEAD2] = "Header DW2",
[HISI_PTT_4DW_HEAD3] = "Header DW3",
@@ -114,7 +118,7 @@ static int hisi_ptt_8dw_pkt_desc(struct hisi_ptt_pkt_buf *pkt_buf)
{
int i;
- for (i = 0; i < HISI_PTT_8DW_TYPE_MAX; i++) {
+ for (i = HISI_PTT_8DW_CHK_AND_RSV0; i < HISI_PTT_8DW_TYPE_MAX; i++) {
/* Do not show 8DW check field and reserved fields */
if (i == HISI_PTT_8DW_CHK_AND_RSV0 || i == HISI_PTT_8DW_RSV1) {
pkt_buf->pos += HISI_PTT_FIELD_LENGTH;
@@ -160,9 +164,8 @@ static int hisi_ptt_4dw_pkt_desc(struct hisi_ptt_pkt_buf *pkt_buf)
hisi_ptt_4dw_print_dw0(pkt_buf);
- for (i = 0; i < HISI_PTT_4DW_TYPE_MAX; i++) {
+ for (i = HISI_PTT_4DW_HEAD1; i < HISI_PTT_4DW_TYPE_MAX; i++)
hisi_ptt_print_pkt(pkt_buf, hisi_ptt_4dw_pkt_field_name[i]);
- }
return hisi_ptt_pkt_size[HISI_PTT_4DW_PKT];
}
--
2.33.0
^ permalink raw reply related
* [PATCH 04/10] perf hisi-ptt: Abstract trace data buf and offset
From: Sizhe Liu @ 2026-06-04 7:49 UTC (permalink / raw)
To: rostedt, mhiramat, mathieu.desnoyers, corbet, skhan, bhelgaas,
yangyccccc, jic23, john.g.garry, will, james.clark, mike.leach,
leo.yan, peterz, mingo, acme, namhyung, mark.rutland,
alexander.shishkin, jolsa, irogers, adrian.hunter, wangyushan12,
shenyang39, gaozhihao6, yuzhichengcheng, liyihang9
Cc: linux-kernel, linux-pci, linux-perf-users, linux-arm-kernel,
linux-doc, linuxarm, prime.zeng, fanghao11, wuyifan50, liusizhe5
In-Reply-To: <20260604075005.2219785-1-liusizhe5@huawei.com>
Abstract the base address, current offset, length and packet type of
analysing trace data into structure hisi_ptt_pkt_buf and move the step
of current offset into the corresponding function.
Signed-off-by: Sizhe Liu <liusizhe5@huawei.com>
---
.../hisi-ptt-decoder/hisi-ptt-pkt-decoder.c | 39 ++++++++++---------
.../hisi-ptt-decoder/hisi-ptt-pkt-decoder.h | 9 ++++-
tools/perf/util/hisi-ptt.c | 20 ++++------
3 files changed, 36 insertions(+), 32 deletions(-)
diff --git a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
index 5daae2eaf435..201ca948c4fb 100644
--- a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
+++ b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
@@ -89,16 +89,17 @@ union hisi_ptt_field_data {
uint32_t value;
};
-static void hisi_ptt_print_pkt(const unsigned char *buf, int pos, const char *desc)
+static void hisi_ptt_print_pkt(struct hisi_ptt_pkt_buf *pkt_buf,
+ const char *desc)
{
const char *color = PERF_COLOR_BLUE;
uint32_t value;
uint8_t byte;
int i;
- value = le32_to_cpu(*(__le32 *)(buf + pos));
+ value = le32_to_cpu(*(__le32 *)(pkt_buf->buf + pkt_buf->pos));
printf(".");
- color_fprintf(stdout, color, " %08x: ", pos);
+ color_fprintf(stdout, color, " %08zx: ", pkt_buf->pos);
for (i = 0; i < HISI_PTT_FIELD_LENGTH; i++) {
byte = (value >> (24 - i * 8)) & 0xFF;
color_fprintf(stdout, color, "%02x ", byte);
@@ -106,36 +107,36 @@ static void hisi_ptt_print_pkt(const unsigned char *buf, int pos, const char *de
for (i = 0; i < HISI_PTT_MAX_SPACE_LEN; i++)
color_fprintf(stdout, color, " ");
color_fprintf(stdout, color, " %s\n", desc);
+ pkt_buf->pos += HISI_PTT_FIELD_LENGTH;
}
-static int hisi_ptt_8dw_pkt_desc(const unsigned char *buf, int pos)
+static int hisi_ptt_8dw_pkt_desc(struct hisi_ptt_pkt_buf *pkt_buf)
{
int i;
for (i = 0; i < HISI_PTT_8DW_TYPE_MAX; i++) {
/* Do not show 8DW check field and reserved fields */
if (i == HISI_PTT_8DW_CHK_AND_RSV0 || i == HISI_PTT_8DW_RSV1) {
- pos += HISI_PTT_FIELD_LENGTH;
+ pkt_buf->pos += HISI_PTT_FIELD_LENGTH;
continue;
}
- hisi_ptt_print_pkt(buf, pos, hisi_ptt_8dw_pkt_field_name[i]);
- pos += HISI_PTT_FIELD_LENGTH;
+ hisi_ptt_print_pkt(pkt_buf, hisi_ptt_8dw_pkt_field_name[i]);
}
return hisi_ptt_pkt_size[HISI_PTT_8DW_PKT];
}
-static void hisi_ptt_4dw_print_dw0(const unsigned char *buf, int pos)
+static void hisi_ptt_4dw_print_dw0(struct hisi_ptt_pkt_buf *pkt_buf)
{
const char *color = PERF_COLOR_BLUE;
union hisi_ptt_field_data dw;
uint8_t byte;
int i;
- dw.value = le32_to_cpu(*(__le32 *)(buf + pos));
+ dw.value = le32_to_cpu(*(__le32 *)(pkt_buf->buf + pkt_buf->pos));
printf(".");
- color_fprintf(stdout, color, " %08x: ", pos);
+ color_fprintf(stdout, color, " %08zx: ", pkt_buf->pos);
for (i = 0; i < HISI_PTT_FIELD_LENGTH; i++) {
byte = (dw.value >> (24 - i * 8)) & 0xFF;
color_fprintf(stdout, color, "%02x ", byte);
@@ -149,27 +150,27 @@ static void hisi_ptt_4dw_print_dw0(const unsigned char *buf, int pos)
"T9", dw.dw0_4dw.t9, "T8", dw.dw0_4dw.t8,
"TH", dw.dw0_4dw.th, "SO", dw.dw0_4dw.so,
"Length", dw.dw0_4dw.len, "Time", dw.dw0_4dw.time);
+
+ pkt_buf->pos += HISI_PTT_FIELD_LENGTH;
}
-static int hisi_ptt_4dw_pkt_desc(const unsigned char *buf, int pos)
+static int hisi_ptt_4dw_pkt_desc(struct hisi_ptt_pkt_buf *pkt_buf)
{
int i;
- hisi_ptt_4dw_print_dw0(buf, pos);
- pos += HISI_PTT_FIELD_LENGTH;
+ hisi_ptt_4dw_print_dw0(pkt_buf);
for (i = 0; i < HISI_PTT_4DW_TYPE_MAX; i++) {
- hisi_ptt_print_pkt(buf, pos, hisi_ptt_4dw_pkt_field_name[i]);
- pos += HISI_PTT_FIELD_LENGTH;
+ hisi_ptt_print_pkt(pkt_buf, hisi_ptt_4dw_pkt_field_name[i]);
}
return hisi_ptt_pkt_size[HISI_PTT_4DW_PKT];
}
-int hisi_ptt_pkt_desc(const unsigned char *buf, int pos, enum hisi_ptt_pkt_type type)
+int hisi_ptt_pkt_desc(struct hisi_ptt_pkt_buf *pkt_buf)
{
- if (type == HISI_PTT_8DW_PKT)
- return hisi_ptt_8dw_pkt_desc(buf, pos);
+ if (pkt_buf->pkt_type == HISI_PTT_8DW_PKT)
+ return hisi_ptt_8dw_pkt_desc(pkt_buf);
- return hisi_ptt_4dw_pkt_desc(buf, pos);
+ return hisi_ptt_4dw_pkt_desc(pkt_buf);
}
diff --git a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h
index 6772b16b817b..316f24f01068 100644
--- a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h
+++ b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h
@@ -26,6 +26,13 @@ static int hisi_ptt_pkt_size[] = {
[HISI_PTT_8DW_PKT] = 32,
};
-int hisi_ptt_pkt_desc(const unsigned char *buf, int pos, enum hisi_ptt_pkt_type type);
+struct hisi_ptt_pkt_buf {
+ const unsigned char *buf;
+ size_t pos;
+ size_t len;
+ enum hisi_ptt_pkt_type pkt_type;
+};
+
+int hisi_ptt_pkt_desc(struct hisi_ptt_pkt_buf *pkt_buf);
#endif
diff --git a/tools/perf/util/hisi-ptt.c b/tools/perf/util/hisi-ptt.c
index e4cc4785f744..4efda3f3e5f9 100644
--- a/tools/perf/util/hisi-ptt.c
+++ b/tools/perf/util/hisi-ptt.c
@@ -49,22 +49,18 @@ static void hisi_ptt_dump(struct hisi_ptt *ptt __maybe_unused,
unsigned char *buf, size_t len)
{
const char *color = PERF_COLOR_BLUE;
- enum hisi_ptt_pkt_type type;
- size_t pos = 0;
- int pkt_len;
+ struct hisi_ptt_pkt_buf pkt_buf;
- type = hisi_ptt_check_packet_type(buf);
- len = round_down(len, hisi_ptt_pkt_size[type]);
+ pkt_buf.buf = buf;
+ pkt_buf.pos = 0;
+ pkt_buf.pkt_type = hisi_ptt_check_packet_type(buf);
+ pkt_buf.len = round_down(len, hisi_ptt_pkt_size[pkt_buf.pkt_type]);
color_fprintf(stdout, color, ". ... HISI PTT data: size %zu bytes\n",
- len);
+ pkt_buf.len);
- while (len > 0) {
- pkt_len = hisi_ptt_pkt_desc(buf, pos, type);
- if (!pkt_len)
+ while (pkt_buf.pos < pkt_buf.len) {
+ if (!hisi_ptt_pkt_desc(&pkt_buf))
color_fprintf(stdout, color, " Bad packet!\n");
-
- pos += pkt_len;
- len -= pkt_len;
}
}
--
2.33.0
^ permalink raw reply related
* [PATCH 07/10] perf hisi-ptt: Merge 4DW and 8DW HEAD0 printing
From: Sizhe Liu @ 2026-06-04 7:50 UTC (permalink / raw)
To: rostedt, mhiramat, mathieu.desnoyers, corbet, skhan, bhelgaas,
yangyccccc, jic23, john.g.garry, will, james.clark, mike.leach,
leo.yan, peterz, mingo, acme, namhyung, mark.rutland,
alexander.shishkin, jolsa, irogers, adrian.hunter, wangyushan12,
shenyang39, gaozhihao6, yuzhichengcheng, liyihang9
Cc: linux-kernel, linux-pci, linux-perf-users, linux-arm-kernel,
linux-doc, linuxarm, prime.zeng, fanghao11, wuyifan50, liusizhe5
In-Reply-To: <20260604075005.2219785-1-liusizhe5@huawei.com>
Merge the printing of HEAD0 for both 4DW and 8DW TLP headers into
hisi_ptt_print_head0(). This unifies the entry point and makes it
easier to add HEAD1/HEAD2/HEAD3 field parsing in subsequent patches.
Signed-off-by: Sizhe Liu <liusizhe5@huawei.com>
---
.../hisi-ptt-decoder/hisi-ptt-pkt-decoder.c | 49 ++++++++++++-------
1 file changed, 30 insertions(+), 19 deletions(-)
diff --git a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
index a5b66e0f7827..9ec84d398cc1 100644
--- a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
+++ b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
@@ -122,6 +122,30 @@ static void hisi_ptt_print_pkt(struct hisi_ptt_pkt_buf *pkt_buf,
pkt_buf->pos += HISI_PTT_FIELD_LENGTH;
}
+static void hisi_ptt_print_head0(struct hisi_ptt_pkt_buf *pkt_buf)
+{
+ const char *color = PERF_COLOR_BLUE;
+ union hisi_ptt_field_data dw;
+
+ dw.value = le32_to_cpu(*(__le32 *)(pkt_buf->buf + pkt_buf->pos));
+ hisi_ptt_print_raw_record(pkt_buf->pos, dw.value);
+
+ if (pkt_buf->pkt_type == HISI_PTT_4DW_PKT)
+ color_fprintf(stdout, color,
+ " %s %x %s %x %s %x %s %x %s %x %s %x %s %x %s %x\n",
+ "Format", dw.dw0_4dw.format,
+ "Type", dw.dw0_4dw.type,
+ "T9", dw.dw0_4dw.t9, "T8", dw.dw0_4dw.t8,
+ "TH", dw.dw0_4dw.th, "SO", dw.dw0_4dw.so,
+ "Length", dw.dw0_4dw.len,
+ "Time", dw.dw0_4dw.time);
+ else
+ color_fprintf(stdout, color, " %s\n",
+ hisi_ptt_8dw_pkt_field_name[HISI_PTT_8DW_HEAD0]);
+
+ pkt_buf->pos += HISI_PTT_FIELD_LENGTH;
+}
+
static int hisi_ptt_8dw_pkt_desc(struct hisi_ptt_pkt_buf *pkt_buf)
{
int i;
@@ -133,35 +157,22 @@ static int hisi_ptt_8dw_pkt_desc(struct hisi_ptt_pkt_buf *pkt_buf)
continue;
}
+ if (i == HISI_PTT_8DW_HEAD0) {
+ hisi_ptt_print_head0(pkt_buf);
+ continue;
+ }
+
hisi_ptt_print_pkt(pkt_buf, hisi_ptt_8dw_pkt_field_name[i]);
}
return hisi_ptt_pkt_size[HISI_PTT_8DW_PKT];
}
-static void hisi_ptt_4dw_print_dw0(struct hisi_ptt_pkt_buf *pkt_buf)
-{
- const char *color = PERF_COLOR_BLUE;
- union hisi_ptt_field_data dw;
-
- dw.value = le32_to_cpu(*(__le32 *)(pkt_buf->buf + pkt_buf->pos));
- hisi_ptt_print_raw_record(pkt_buf->pos, dw.value);
-
- color_fprintf(stdout, color,
- " %s %x %s %x %s %x %s %x %s %x %s %x %s %x %s %x\n",
- "Format", dw.dw0_4dw.format, "Type", dw.dw0_4dw.type,
- "T9", dw.dw0_4dw.t9, "T8", dw.dw0_4dw.t8,
- "TH", dw.dw0_4dw.th, "SO", dw.dw0_4dw.so,
- "Length", dw.dw0_4dw.len, "Time", dw.dw0_4dw.time);
-
- pkt_buf->pos += HISI_PTT_FIELD_LENGTH;
-}
-
static int hisi_ptt_4dw_pkt_desc(struct hisi_ptt_pkt_buf *pkt_buf)
{
int i;
- hisi_ptt_4dw_print_dw0(pkt_buf);
+ hisi_ptt_print_head0(pkt_buf);
for (i = HISI_PTT_4DW_HEAD1; i < HISI_PTT_4DW_TYPE_MAX; i++)
hisi_ptt_print_pkt(pkt_buf, hisi_ptt_4dw_pkt_field_name[i]);
--
2.33.0
^ permalink raw reply related
* [PATCH 02/10] perf hisi-ptt: Fix PTT trace TLP Header parsing
From: Sizhe Liu @ 2026-06-04 7:49 UTC (permalink / raw)
To: rostedt, mhiramat, mathieu.desnoyers, corbet, skhan, bhelgaas,
yangyccccc, jic23, john.g.garry, will, james.clark, mike.leach,
leo.yan, peterz, mingo, acme, namhyung, mark.rutland,
alexander.shishkin, jolsa, irogers, adrian.hunter, wangyushan12,
shenyang39, gaozhihao6, yuzhichengcheng, liyihang9
Cc: linux-kernel, linux-pci, linux-perf-users, linux-arm-kernel,
linux-doc, linuxarm, prime.zeng, fanghao11, wuyifan50, liusizhe5
In-Reply-To: <20260604075005.2219785-1-liusizhe5@huawei.com>
The DW0 bit field layout of the hisi_ptt_4dw union does not match the
actual bit ordering in little-endian memory, causing incorrect field
extraction. Reorder the struct members from LSB to MSB to match the
le32_to_cpu() conversion.
Also print all DW hex values in big-endian byte order for readability,
matching the bit field layout shown in the format diagram.
4DW format is like:
bits [31:30] [ 29:25 ][24][23][22][21][ 20:11 ][ 10:0 ]
|-----|---------|---|---|---|---|-------------|-------------|
DW0 [ Fmt ][ Type ][T9][T8][TH][SO][ Length ][ Time ]
DW1 [ Header DW1 ]
DW2 [ Header DW2 ]
DW3 [ Header DW3 ]
Cc: stable@vger.kernel.org
Fixes: 5e91e57e6809 ("perf auxtrace arm64: Add support for parsing HiSilicon PCIe Trace packet")
Signed-off-by: Sizhe Liu <liusizhe5@huawei.com>
---
Documentation/trace/hisi-ptt.rst | 28 ++++++++--------
.../hisi-ptt-decoder/hisi-ptt-pkt-decoder.c | 33 ++++++++++++-------
2 files changed, 35 insertions(+), 26 deletions(-)
diff --git a/Documentation/trace/hisi-ptt.rst b/Documentation/trace/hisi-ptt.rst
index 6eef28ebb0c7..f6a2655f99e5 100644
--- a/Documentation/trace/hisi-ptt.rst
+++ b/Documentation/trace/hisi-ptt.rst
@@ -285,20 +285,20 @@ according to the format described previously (take 8DW as an example):
[...perf headers and other information]
. ... HISI PTT data: size 4194304 bytes
. 00000000: 00 00 00 00 Prefix
- . 00000004: 01 00 00 60 Header DW0
- . 00000008: 0f 1e 00 01 Header DW1
- . 0000000c: 04 00 00 00 Header DW2
- . 00000010: 40 00 81 02 Header DW3
- . 00000014: 33 c0 04 00 Time
+ . 00000004: 60 00 00 01 Header DW0
+ . 00000008: 01 00 1e 0f Header DW1
+ . 0000000c: 00 00 00 04 Header DW2
+ . 00000010: 02 81 00 40 Header DW3
+ . 00000014: 00 04 c0 33 Time
. 00000020: 00 00 00 00 Prefix
- . 00000024: 01 00 00 60 Header DW0
- . 00000028: 0f 1e 00 01 Header DW1
- . 0000002c: 04 00 00 00 Header DW2
- . 00000030: 40 00 81 02 Header DW3
- . 00000034: 02 00 00 00 Time
+ . 00000024: 60 00 00 01 Header DW0
+ . 00000028: 01 00 1e 0f Header DW1
+ . 0000002c: 00 00 00 04 Header DW2
+ . 00000030: 02 81 00 40 Header DW3
+ . 00000034: 00 00 00 02 Time
. 00000040: 00 00 00 00 Prefix
- . 00000044: 01 00 00 60 Header DW0
- . 00000048: 0f 1e 00 01 Header DW1
- . 0000004c: 04 00 00 00 Header DW2
- . 00000050: 40 00 81 02 Header DW3
+ . 00000044: 60 00 00 01 Header DW0
+ . 00000048: 01 00 1e 0f Header DW1
+ . 0000004c: 00 00 00 04 Header DW2
+ . 00000050: 02 81 00 40 Header DW3
[...]
diff --git a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
index c48b2ce7c4a3..67024f18ebbb 100644
--- a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
+++ b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
@@ -11,6 +11,7 @@
#include <byteswap.h>
#include <linux/bitops.h>
#include <stdarg.h>
+#include <linux/kernel.h>
#include "../color.h"
#include "hisi-ptt-pkt-decoder.h"
@@ -75,14 +76,14 @@ static const char * const hisi_ptt_4dw_pkt_field_name[] = {
union hisi_ptt_4dw {
struct {
- uint32_t format : 2;
- uint32_t type : 5;
- uint32_t t9 : 1;
- uint32_t t8 : 1;
- uint32_t th : 1;
- uint32_t so : 1;
- uint32_t len : 10;
uint32_t time : 11;
+ uint32_t len : 10;
+ uint32_t so : 1;
+ uint32_t th : 1;
+ uint32_t t8 : 1;
+ uint32_t t9 : 1;
+ uint32_t type : 5;
+ uint32_t format : 2;
};
uint32_t value;
};
@@ -90,12 +91,17 @@ union hisi_ptt_4dw {
static void hisi_ptt_print_pkt(const unsigned char *buf, int pos, const char *desc)
{
const char *color = PERF_COLOR_BLUE;
+ uint32_t value;
+ uint8_t byte;
int i;
+ value = le32_to_cpu(*(__le32 *)(buf + pos));
printf(".");
color_fprintf(stdout, color, " %08x: ", pos);
- for (i = 0; i < HISI_PTT_FIELD_LENGTH; i++)
- color_fprintf(stdout, color, "%02x ", buf[pos + i]);
+ for (i = 0; i < HISI_PTT_FIELD_LENGTH; i++) {
+ byte = (value >> (24 - i * 8)) & 0xFF;
+ color_fprintf(stdout, color, "%02x ", byte);
+ }
for (i = 0; i < HISI_PTT_MAX_SPACE_LEN; i++)
color_fprintf(stdout, color, " ");
color_fprintf(stdout, color, " %s\n", desc);
@@ -123,13 +129,16 @@ static void hisi_ptt_4dw_print_dw0(const unsigned char *buf, int pos)
{
const char *color = PERF_COLOR_BLUE;
union hisi_ptt_4dw dw0;
+ uint8_t byte;
int i;
- dw0.value = *(uint32_t *)(buf + pos);
+ dw0.value = le32_to_cpu(*(__le32 *)(buf + pos));
printf(".");
color_fprintf(stdout, color, " %08x: ", pos);
- for (i = 0; i < HISI_PTT_FIELD_LENGTH; i++)
- color_fprintf(stdout, color, "%02x ", buf[pos + i]);
+ for (i = 0; i < HISI_PTT_FIELD_LENGTH; i++) {
+ byte = (dw0.value >> (24 - i * 8)) & 0xFF;
+ color_fprintf(stdout, color, "%02x ", byte);
+ }
for (i = 0; i < HISI_PTT_MAX_SPACE_LEN; i++)
color_fprintf(stdout, color, " ");
--
2.33.0
^ permalink raw reply related
* [PATCH 09/10] perf hisi-ptt: Add field-level parsing for header DW2/DW3
From: Sizhe Liu @ 2026-06-04 7:50 UTC (permalink / raw)
To: rostedt, mhiramat, mathieu.desnoyers, corbet, skhan, bhelgaas,
yangyccccc, jic23, john.g.garry, will, james.clark, mike.leach,
leo.yan, peterz, mingo, acme, namhyung, mark.rutland,
alexander.shishkin, jolsa, irogers, adrian.hunter, wangyushan12,
shenyang39, gaozhihao6, yuzhichengcheng, liyihang9
Cc: linux-kernel, linux-pci, linux-perf-users, linux-arm-kernel,
linux-doc, linuxarm, prime.zeng, fanghao11, wuyifan50, liusizhe5
In-Reply-To: <20260604075005.2219785-1-liusizhe5@huawei.com>
Add detailed field parsing for TLP header DW2 and DW3 based on the
message type parsed from header DW0:
- HEADER DW0: fields printed in 4DW format, printed as generic label
in 8DW format for compatibility.
- HEADER DW1: printed with the field name for both 4DW and 8DW formats.
- HEADER DW2: fields printed for MWr/Msg/Atomic/IO TLPs, generic label
for others.
- HEADER DW3: fields printed for Completion and Configuration TLPs,
generic label for others.
This gives users more structured information when analysing PTT
trace data.
Signed-off-by: Sizhe Liu <liusizhe5@huawei.com>
---
.../hisi-ptt-decoder/hisi-ptt-pkt-decoder.c | 161 +++++++++++++++++-
1 file changed, 152 insertions(+), 9 deletions(-)
diff --git a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
index 59ab8ec3a03d..46f11d5719ac 100644
--- a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
+++ b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
@@ -38,6 +38,21 @@
* DW1 [ Header DW1 ]
* DW2 [ Header DW2 ]
* DW3 [ Header DW3 ]
+ *
+ * Header DW2 for MWr/Msg/MsgD/FetchAdd/Swap/CAS/IORd/IOWr is like:
+ * bits [ 31 ][ 30:23 ][22][21][20][ 19:16 ][ 15:0 ]
+ * |---------|----------------|----|---|--|-----------|----------|
+ * fields [Reserved][Request Segment][RSV][TV][T][Tag<13:10>][Header DW2]
+ *
+ * Header DW3 for CfgRd0/CfgWr0/CfgRd1/CfgWr1 is like:
+ * bits [ 31 ][ 30:23 ][22][21][20][ 19:16 ][ 15:0 ]
+ * |---------|--------------------|----|---|--|-----------|----------|
+ * fields [Reserved][Destination Segment][DSV][TV][T][Tag<13:10>][Header DW3]
+ *
+ * Header DW3 for Cpl/CplD/CplLk/CplDlk is like:
+ * bits [ 31:24 ][ 23:16 ][15][ 14:6 ][5][4][ 3:0 ]
+ * |--------------------|------------------|----|---------|--|---|----------|
+ * fields [Destination Segment][Completer Segment][DSV][Reserved][TV][T][Tag<13:10>]
*/
enum hisi_ptt_8dw_pkt_field_type {
@@ -127,6 +142,45 @@ union hisi_ptt_field_data {
uint32_t type : 5;
uint32_t format : 3;
} dw0_8dw;
+ /*
+ * Header DW2 for MWr/Msg/MsgD/FetchAdd/Swap/CAS/IORd/IOWr TLPs.
+ * Affects both 4DW and 8DW format.
+ */
+ struct {
+ uint32_t header_dw2 : 16;
+ uint32_t tag : 4;
+ uint32_t t : 1;
+ uint32_t tv : 1;
+ uint32_t rsv : 1;
+ uint32_t request_segment : 8;
+ uint32_t reserved : 1;
+ } dw2_mixed;
+ /*
+ * Header DW3 for CfgRd0/CfgWr0/CfgRd1/CfgWr1 TLPs.
+ * Affects both 4DW and 8DW format.
+ */
+ struct {
+ uint32_t header_dw3 : 16;
+ uint32_t tag : 4;
+ uint32_t t : 1;
+ uint32_t tv : 1;
+ uint32_t dsv : 1;
+ uint32_t destination_segment : 8;
+ uint32_t reserved : 1;
+ } dw3_cfg;
+ /*
+ * Header DW3 for Cpl/CplD/CplLk/CplDlk TLPs.
+ * Affects both 4DW and 8DW format.
+ */
+ struct {
+ uint32_t tag : 4;
+ uint32_t t : 1;
+ uint32_t tv : 1;
+ uint32_t reserved : 9;
+ uint32_t dsv : 1;
+ uint32_t completer_segment : 8;
+ uint32_t destination_segment : 8;
+ } dw3_cpl;
uint32_t value;
};
@@ -211,6 +265,85 @@ static void hisi_ptt_print_head0(struct hisi_ptt_pkt_buf *pkt_buf)
pkt_buf->pos += HISI_PTT_FIELD_LENGTH;
}
+static void hisi_ptt_print_head1(struct hisi_ptt_pkt_buf *pkt_buf)
+{
+ const char *color = PERF_COLOR_BLUE;
+ union hisi_ptt_field_data dw;
+
+ dw.value = le32_to_cpu(*(__le32 *)(pkt_buf->buf + pkt_buf->pos));
+ hisi_ptt_print_raw_record(pkt_buf->pos, dw.value);
+ color_fprintf(stdout, color, " %s\n",
+ pkt_buf->pkt_type == HISI_PTT_4DW_PKT ?
+ hisi_ptt_4dw_pkt_field_name[HISI_PTT_4DW_HEAD1] :
+ hisi_ptt_8dw_pkt_field_name[HISI_PTT_8DW_HEAD1]);
+
+ pkt_buf->pos += HISI_PTT_FIELD_LENGTH;
+}
+
+static void hisi_ptt_print_head2(struct hisi_ptt_pkt_buf *pkt_buf)
+{
+ const char *color = PERF_COLOR_BLUE;
+ union hisi_ptt_field_data dw;
+
+ dw.value = le32_to_cpu(*(__le32 *)(pkt_buf->buf + pkt_buf->pos));
+ hisi_ptt_print_raw_record(pkt_buf->pos, dw.value);
+
+ if (pkt_buf->pkt_msg_type == HISI_PTT_PKT_TYPE_MWR ||
+ pkt_buf->pkt_msg_type == HISI_PTT_PKT_TYPE_MSG ||
+ pkt_buf->pkt_msg_type == HISI_PTT_PKT_TYPE_ATOM ||
+ pkt_buf->pkt_msg_type == HISI_PTT_PKT_TYPE_IO)
+ color_fprintf(stdout, color,
+ " %s %x %s %x %s %x %s %x %s %x %s %x %s %x\n",
+ "Reserved", dw.dw2_mixed.reserved,
+ "Request Segment", dw.dw2_mixed.request_segment,
+ "RSV", dw.dw2_mixed.rsv, "TV", dw.dw2_mixed.tv,
+ "T", dw.dw2_mixed.t, "Tag", dw.dw2_mixed.tag,
+ "Header DW2", dw.dw2_mixed.header_dw2);
+ else
+ color_fprintf(stdout, color, " %s\n",
+ pkt_buf->pkt_type == HISI_PTT_4DW_PKT ?
+ hisi_ptt_4dw_pkt_field_name[HISI_PTT_4DW_HEAD2] :
+ hisi_ptt_8dw_pkt_field_name[HISI_PTT_8DW_HEAD2]);
+
+ pkt_buf->pos += HISI_PTT_FIELD_LENGTH;
+}
+
+static void hisi_ptt_print_head3(struct hisi_ptt_pkt_buf *pkt_buf)
+{
+ const char *color = PERF_COLOR_BLUE;
+ union hisi_ptt_field_data dw;
+
+ dw.value = le32_to_cpu(*(__le32 *)(pkt_buf->buf + pkt_buf->pos));
+ hisi_ptt_print_raw_record(pkt_buf->pos, dw.value);
+
+ if (pkt_buf->pkt_msg_type == HISI_PTT_PKT_TYPE_CPL)
+ color_fprintf(stdout, color,
+ " %s %x %s %x %s %x %s %x %s %x %s %x %s %x\n",
+ "Destination Segment",
+ dw.dw3_cpl.destination_segment,
+ "Completer Segment", dw.dw3_cpl.completer_segment,
+ "DSV", dw.dw3_cpl.dsv,
+ "Reserved", dw.dw3_cpl.reserved,
+ "TV", dw.dw3_cpl.tv, "T", dw.dw3_cpl.t,
+ "Tag", dw.dw3_cpl.tag);
+ else if (pkt_buf->pkt_msg_type == HISI_PTT_PKT_TYPE_CFG)
+ color_fprintf(stdout, color,
+ " %s %x %s %x %s %x %s %x %s %x %s %x %s %x\n",
+ "Reserved", dw.dw3_cfg.reserved,
+ "Destination Segment",
+ dw.dw3_cfg.destination_segment,
+ "DSV", dw.dw3_cfg.dsv, "TV", dw.dw3_cfg.tv,
+ "T", dw.dw3_cfg.t, "Tag", dw.dw3_cfg.tag,
+ "Header DW3", dw.dw3_cfg.header_dw3);
+ else
+ color_fprintf(stdout, color, " %s\n",
+ pkt_buf->pkt_type == HISI_PTT_4DW_PKT ?
+ hisi_ptt_4dw_pkt_field_name[HISI_PTT_4DW_HEAD3] :
+ hisi_ptt_8dw_pkt_field_name[HISI_PTT_8DW_HEAD3]);
+
+ pkt_buf->pos += HISI_PTT_FIELD_LENGTH;
+}
+
static int hisi_ptt_8dw_pkt_desc(struct hisi_ptt_pkt_buf *pkt_buf)
{
int i;
@@ -222,12 +355,24 @@ static int hisi_ptt_8dw_pkt_desc(struct hisi_ptt_pkt_buf *pkt_buf)
continue;
}
- if (i == HISI_PTT_8DW_HEAD0) {
+ switch (i) {
+ case HISI_PTT_8DW_HEAD0:
hisi_ptt_print_head0(pkt_buf);
- continue;
+ break;
+ case HISI_PTT_8DW_HEAD1:
+ hisi_ptt_print_head1(pkt_buf);
+ break;
+ case HISI_PTT_8DW_HEAD2:
+ hisi_ptt_print_head2(pkt_buf);
+ break;
+ case HISI_PTT_8DW_HEAD3:
+ hisi_ptt_print_head3(pkt_buf);
+ break;
+ default:
+ hisi_ptt_print_pkt(pkt_buf,
+ hisi_ptt_8dw_pkt_field_name[i]);
+ break;
}
-
- hisi_ptt_print_pkt(pkt_buf, hisi_ptt_8dw_pkt_field_name[i]);
}
return hisi_ptt_pkt_size[HISI_PTT_8DW_PKT];
@@ -235,12 +380,10 @@ static int hisi_ptt_8dw_pkt_desc(struct hisi_ptt_pkt_buf *pkt_buf)
static int hisi_ptt_4dw_pkt_desc(struct hisi_ptt_pkt_buf *pkt_buf)
{
- int i;
-
hisi_ptt_print_head0(pkt_buf);
-
- for (i = HISI_PTT_4DW_HEAD1; i < HISI_PTT_4DW_TYPE_MAX; i++)
- hisi_ptt_print_pkt(pkt_buf, hisi_ptt_4dw_pkt_field_name[i]);
+ hisi_ptt_print_head1(pkt_buf);
+ hisi_ptt_print_head2(pkt_buf);
+ hisi_ptt_print_head3(pkt_buf);
return hisi_ptt_pkt_size[HISI_PTT_4DW_PKT];
}
--
2.33.0
^ permalink raw reply related
* [PATCH 08/10] perf hisi-ptt: Add parsing of supported message types
From: Sizhe Liu @ 2026-06-04 7:50 UTC (permalink / raw)
To: rostedt, mhiramat, mathieu.desnoyers, corbet, skhan, bhelgaas,
yangyccccc, jic23, john.g.garry, will, james.clark, mike.leach,
leo.yan, peterz, mingo, acme, namhyung, mark.rutland,
alexander.shishkin, jolsa, irogers, adrian.hunter, wangyushan12,
shenyang39, gaozhihao6, yuzhichengcheng, liyihang9
Cc: linux-kernel, linux-pci, linux-perf-users, linux-arm-kernel,
linux-doc, linuxarm, prime.zeng, fanghao11, wuyifan50, liusizhe5
In-Reply-To: <20260604075005.2219785-1-liusizhe5@huawei.com>
Parse TLP message types from Header DW0 Format and Type fields for
both 4DW and 8DW formats, classifying packets into:
- MWr (Posted Memory Write)
- Msg (Posted Message)
- Atom (Non-Posted Atomic)
- IO (Non-Posted IO)
- CFG (Non-Posted Configuration)
- CPL (Completion)
Support for those message types depends on the hisi_ptt hardware.
The parsed message type is stored in pkt_buf->pkt_msg_type and will
be used by subsequent patches to select the correct field layout for
DW2 and DW3 printing.
Signed-off-by: Sizhe Liu <liusizhe5@huawei.com>
---
.../hisi-ptt-decoder/hisi-ptt-pkt-decoder.c | 65 +++++++++++++++++++
.../hisi-ptt-decoder/hisi-ptt-pkt-decoder.h | 12 ++++
tools/perf/util/hisi-ptt.c | 1 +
3 files changed, 78 insertions(+)
diff --git a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
index 9ec84d398cc1..59ab8ec3a03d 100644
--- a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
+++ b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
@@ -78,6 +78,37 @@ static const char * const hisi_ptt_4dw_pkt_field_name[] = {
[HISI_PTT_4DW_HEAD3] = "Header DW3",
};
+static bool hisi_ptt_is_mwr_tlp(uint32_t format, uint32_t type)
+{
+ return (format == 0x2 || format == 0x3) && (type == 0);
+}
+
+static bool hisi_ptt_is_msg_tlp(uint32_t format, uint32_t type)
+{
+ return (format == 0x1 || format == 0x3) && ((type & 0x10) != 0);
+}
+
+static bool hisi_ptt_is_io_tlp(uint32_t format, uint32_t type)
+{
+ return (format == 0 || format == 0x2) && (type == 0x2);
+}
+
+static bool hisi_ptt_is_atomic_tlp(uint32_t format, uint32_t type)
+{
+ return (format == 0x2 || format == 0x3) &&
+ (type == 0xc || type == 0xd || type == 0xe);
+}
+
+static bool hisi_ptt_is_cfg_tlp(uint32_t format, uint32_t type)
+{
+ return (format == 0 || format == 0x2) && (type == 0x4 || type == 0x5);
+}
+
+static bool hisi_ptt_is_cpl_tlp(uint32_t format, uint32_t type)
+{
+ return (format == 0 || format == 0x2) && (type == 0xa || type == 0xb);
+}
+
union hisi_ptt_field_data {
/* Header DW0 for 4DW format */
struct {
@@ -90,9 +121,41 @@ union hisi_ptt_field_data {
uint32_t type : 5;
uint32_t format : 2;
} dw0_4dw;
+ /* Header DW0 for 8DW format */
+ struct {
+ uint32_t others : 24;
+ uint32_t type : 5;
+ uint32_t format : 3;
+ } dw0_8dw;
uint32_t value;
};
+static int hisi_ptt_parse_pkt_msg_type(union hisi_ptt_field_data dw,
+ enum hisi_ptt_pkt_type pkt_type)
+{
+ uint32_t format, type;
+
+ format = (pkt_type == HISI_PTT_4DW_PKT) ? dw.dw0_4dw.format :
+ dw.dw0_8dw.format;
+ type = (pkt_type == HISI_PTT_4DW_PKT) ? dw.dw0_4dw.type :
+ dw.dw0_8dw.type;
+
+ if (hisi_ptt_is_mwr_tlp(format, type))
+ return HISI_PTT_PKT_TYPE_MWR;
+ else if (hisi_ptt_is_msg_tlp(format, type))
+ return HISI_PTT_PKT_TYPE_MSG;
+ else if (hisi_ptt_is_atomic_tlp(format, type))
+ return HISI_PTT_PKT_TYPE_ATOM;
+ else if (hisi_ptt_is_io_tlp(format, type))
+ return HISI_PTT_PKT_TYPE_IO;
+ else if (hisi_ptt_is_cfg_tlp(format, type))
+ return HISI_PTT_PKT_TYPE_CFG;
+ else if (hisi_ptt_is_cpl_tlp(format, type))
+ return HISI_PTT_PKT_TYPE_CPL;
+
+ return HISI_PTT_PKT_TYPE_UNKNOWN;
+}
+
static void hisi_ptt_print_raw_record(size_t offset, uint32_t value)
{
const char *color = PERF_COLOR_BLUE;
@@ -128,6 +191,8 @@ static void hisi_ptt_print_head0(struct hisi_ptt_pkt_buf *pkt_buf)
union hisi_ptt_field_data dw;
dw.value = le32_to_cpu(*(__le32 *)(pkt_buf->buf + pkt_buf->pos));
+ pkt_buf->pkt_msg_type = hisi_ptt_parse_pkt_msg_type(dw,
+ pkt_buf->pkt_type);
hisi_ptt_print_raw_record(pkt_buf->pos, dw.value);
if (pkt_buf->pkt_type == HISI_PTT_4DW_PKT)
diff --git a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h
index 316f24f01068..3fdad34fe400 100644
--- a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h
+++ b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h
@@ -26,11 +26,23 @@ static int hisi_ptt_pkt_size[] = {
[HISI_PTT_8DW_PKT] = 32,
};
+enum hisi_ptt_pkt_msg_type {
+ HISI_PTT_PKT_TYPE_UNKNOWN, /* Types do not support analysis */
+ HISI_PTT_PKT_TYPE_MWR, /* P-(MemWr) */
+ HISI_PTT_PKT_TYPE_MSG, /* P-(Message) */
+ HISI_PTT_PKT_TYPE_ATOM, /* NP-(Atomic) */
+ HISI_PTT_PKT_TYPE_IO, /* NP-(IO) */
+ HISI_PTT_PKT_TYPE_CFG, /* NP-(CFG) */
+ HISI_PTT_PKT_TYPE_CPL, /* CPL-(CPL) */
+ HISI_PTT_PKT_TYPE_MAX
+};
+
struct hisi_ptt_pkt_buf {
const unsigned char *buf;
size_t pos;
size_t len;
enum hisi_ptt_pkt_type pkt_type;
+ enum hisi_ptt_pkt_msg_type pkt_msg_type;
};
int hisi_ptt_pkt_desc(struct hisi_ptt_pkt_buf *pkt_buf);
diff --git a/tools/perf/util/hisi-ptt.c b/tools/perf/util/hisi-ptt.c
index 4efda3f3e5f9..e321f393601b 100644
--- a/tools/perf/util/hisi-ptt.c
+++ b/tools/perf/util/hisi-ptt.c
@@ -55,6 +55,7 @@ static void hisi_ptt_dump(struct hisi_ptt *ptt __maybe_unused,
pkt_buf.pos = 0;
pkt_buf.pkt_type = hisi_ptt_check_packet_type(buf);
pkt_buf.len = round_down(len, hisi_ptt_pkt_size[pkt_buf.pkt_type]);
+ pkt_buf.pkt_msg_type = HISI_PTT_PKT_TYPE_UNKNOWN;
color_fprintf(stdout, color, ". ... HISI PTT data: size %zu bytes\n",
pkt_buf.len);
--
2.33.0
^ permalink raw reply related
* [PATCH 10/10] perf hisi-ptt: Add decoder version compatibility
From: Sizhe Liu @ 2026-06-04 7:50 UTC (permalink / raw)
To: rostedt, mhiramat, mathieu.desnoyers, corbet, skhan, bhelgaas,
yangyccccc, jic23, john.g.garry, will, james.clark, mike.leach,
leo.yan, peterz, mingo, acme, namhyung, mark.rutland,
alexander.shishkin, jolsa, irogers, adrian.hunter, wangyushan12,
shenyang39, gaozhihao6, yuzhichengcheng, liyihang9
Cc: linux-kernel, linux-pci, linux-perf-users, linux-arm-kernel,
linux-doc, linuxarm, prime.zeng, fanghao11, wuyifan50, liusizhe5
In-Reply-To: <20260604075005.2219785-1-liusizhe5@huawei.com>
The hisi_ptt packet decoder now performs detailed field-level parsing
of TLP header DW2 and DW3 based on the message type classified from
DW0 (added in the previous patches). However, trace data recorded
with older versions of the tool does not contain the information
needed for this detailed parsing, and should continue to use the
generic field-name-only output.
Introduce a version field (V1/V2) in the auxtrace info record to
distinguish between the two data formats:
- V1 (legacy): auxtrace priv contains only PMU type. DW2 and DW3
are printed with generic field names only, no message-type-based
field decoding.
- V2 (current): auxtrace priv contains PMU type and version. DW2
and DW3 are decoded according to the TLP message type (MWr, Msg,
Atomic, IO, CPL, Cfg) with detailed field names.
At recording time, set the version to HISI_PTT_DECODER_V2 in the
auxtrace info. At decoding time, determine the version from the
priv data size and the version field:
- If priv_size >= V2, read the version from priv[1].
- If priv_size < V2, assume V1 for backward compatibility.
- If version is unknown (future), warn and decode auxtrace as v2.
Also add the version to the dump output alongside the PMU type.
Adjust hisi_ptt_pkt_size position to adapt to compilation.
Signed-off-by: Sizhe Liu <liusizhe5@huawei.com>
---
tools/perf/arch/arm64/util/hisi-ptt.c | 2 ++
.../hisi-ptt-decoder/hisi-ptt-pkt-decoder.c | 35 ++++++++++++-------
.../hisi-ptt-decoder/hisi-ptt-pkt-decoder.h | 8 ++---
tools/perf/util/hisi-ptt.c | 23 ++++++++----
tools/perf/util/hisi-ptt.h | 4 ++-
5 files changed, 47 insertions(+), 25 deletions(-)
diff --git a/tools/perf/arch/arm64/util/hisi-ptt.c b/tools/perf/arch/arm64/util/hisi-ptt.c
index fe457fd58c9e..9a28f96b8e57 100644
--- a/tools/perf/arch/arm64/util/hisi-ptt.c
+++ b/tools/perf/arch/arm64/util/hisi-ptt.c
@@ -24,6 +24,7 @@
#include "../../../util/record.h"
#include "../../../util/session.h"
#include "../../../util/tsc.h"
+#include "../../../util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h"
#define KiB(x) ((x) * 1024)
#define MiB(x) ((x) * 1024 * 1024)
@@ -58,6 +59,7 @@ static int hisi_ptt_info_fill(struct auxtrace_record *itr,
auxtrace_info->type = PERF_AUXTRACE_HISI_PTT;
auxtrace_info->priv[0] = hisi_ptt_pmu->type;
+ auxtrace_info->priv[1] = HISI_PTT_DECODER_V2;
return 0;
}
diff --git a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
index 46f11d5719ac..0611f121431c 100644
--- a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
+++ b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
@@ -75,6 +75,11 @@ enum hisi_ptt_4dw_pkt_field_type {
HISI_PTT_4DW_TYPE_MAX
};
+static int hisi_ptt_pkt_size[] = {
+ [HISI_PTT_4DW_PKT] = 16,
+ [HISI_PTT_8DW_PKT] = 32,
+};
+
static const char * const hisi_ptt_8dw_pkt_field_name[] = {
[HISI_PTT_8DW_CHK_AND_RSV0] = "CHK & RSV0",
[HISI_PTT_8DW_PREFIX] = "Prefix",
@@ -284,14 +289,19 @@ static void hisi_ptt_print_head2(struct hisi_ptt_pkt_buf *pkt_buf)
{
const char *color = PERF_COLOR_BLUE;
union hisi_ptt_field_data dw;
+ const char *desc = pkt_buf->pkt_type == HISI_PTT_4DW_PKT ?
+ hisi_ptt_4dw_pkt_field_name[HISI_PTT_4DW_HEAD2] :
+ hisi_ptt_8dw_pkt_field_name[HISI_PTT_8DW_HEAD2];
dw.value = le32_to_cpu(*(__le32 *)(pkt_buf->buf + pkt_buf->pos));
hisi_ptt_print_raw_record(pkt_buf->pos, dw.value);
- if (pkt_buf->pkt_msg_type == HISI_PTT_PKT_TYPE_MWR ||
- pkt_buf->pkt_msg_type == HISI_PTT_PKT_TYPE_MSG ||
- pkt_buf->pkt_msg_type == HISI_PTT_PKT_TYPE_ATOM ||
- pkt_buf->pkt_msg_type == HISI_PTT_PKT_TYPE_IO)
+ if (pkt_buf->version < HISI_PTT_DECODER_V2)
+ color_fprintf(stdout, color, " %s\n", desc);
+ else if (pkt_buf->pkt_msg_type == HISI_PTT_PKT_TYPE_MWR ||
+ pkt_buf->pkt_msg_type == HISI_PTT_PKT_TYPE_MSG ||
+ pkt_buf->pkt_msg_type == HISI_PTT_PKT_TYPE_ATOM ||
+ pkt_buf->pkt_msg_type == HISI_PTT_PKT_TYPE_IO)
color_fprintf(stdout, color,
" %s %x %s %x %s %x %s %x %s %x %s %x %s %x\n",
"Reserved", dw.dw2_mixed.reserved,
@@ -300,10 +310,7 @@ static void hisi_ptt_print_head2(struct hisi_ptt_pkt_buf *pkt_buf)
"T", dw.dw2_mixed.t, "Tag", dw.dw2_mixed.tag,
"Header DW2", dw.dw2_mixed.header_dw2);
else
- color_fprintf(stdout, color, " %s\n",
- pkt_buf->pkt_type == HISI_PTT_4DW_PKT ?
- hisi_ptt_4dw_pkt_field_name[HISI_PTT_4DW_HEAD2] :
- hisi_ptt_8dw_pkt_field_name[HISI_PTT_8DW_HEAD2]);
+ color_fprintf(stdout, color, " %s\n", desc);
pkt_buf->pos += HISI_PTT_FIELD_LENGTH;
}
@@ -312,11 +319,16 @@ static void hisi_ptt_print_head3(struct hisi_ptt_pkt_buf *pkt_buf)
{
const char *color = PERF_COLOR_BLUE;
union hisi_ptt_field_data dw;
+ const char *desc = pkt_buf->pkt_type == HISI_PTT_4DW_PKT ?
+ hisi_ptt_4dw_pkt_field_name[HISI_PTT_4DW_HEAD3] :
+ hisi_ptt_8dw_pkt_field_name[HISI_PTT_8DW_HEAD3];
dw.value = le32_to_cpu(*(__le32 *)(pkt_buf->buf + pkt_buf->pos));
hisi_ptt_print_raw_record(pkt_buf->pos, dw.value);
- if (pkt_buf->pkt_msg_type == HISI_PTT_PKT_TYPE_CPL)
+ if (pkt_buf->version < HISI_PTT_DECODER_V2)
+ color_fprintf(stdout, color, " %s\n", desc);
+ else if (pkt_buf->pkt_msg_type == HISI_PTT_PKT_TYPE_CPL)
color_fprintf(stdout, color,
" %s %x %s %x %s %x %s %x %s %x %s %x %s %x\n",
"Destination Segment",
@@ -336,10 +348,7 @@ static void hisi_ptt_print_head3(struct hisi_ptt_pkt_buf *pkt_buf)
"T", dw.dw3_cfg.t, "Tag", dw.dw3_cfg.tag,
"Header DW3", dw.dw3_cfg.header_dw3);
else
- color_fprintf(stdout, color, " %s\n",
- pkt_buf->pkt_type == HISI_PTT_4DW_PKT ?
- hisi_ptt_4dw_pkt_field_name[HISI_PTT_4DW_HEAD3] :
- hisi_ptt_8dw_pkt_field_name[HISI_PTT_8DW_HEAD3]);
+ color_fprintf(stdout, color, " %s\n", desc);
pkt_buf->pos += HISI_PTT_FIELD_LENGTH;
}
diff --git a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h
index 3fdad34fe400..d1bffab449d4 100644
--- a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h
+++ b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h
@@ -14,6 +14,8 @@
#define HISI_PTT_IS_8DW_PKT GENMASK(31, 11)
#define HISI_PTT_MAX_SPACE_LEN 10
#define HISI_PTT_FIELD_LENGTH 4
+#define HISI_PTT_DECODER_V1 1
+#define HISI_PTT_DECODER_V2 2
enum hisi_ptt_pkt_type {
HISI_PTT_4DW_PKT,
@@ -21,11 +23,6 @@ enum hisi_ptt_pkt_type {
HISI_PTT_PKT_MAX
};
-static int hisi_ptt_pkt_size[] = {
- [HISI_PTT_4DW_PKT] = 16,
- [HISI_PTT_8DW_PKT] = 32,
-};
-
enum hisi_ptt_pkt_msg_type {
HISI_PTT_PKT_TYPE_UNKNOWN, /* Types do not support analysis */
HISI_PTT_PKT_TYPE_MWR, /* P-(MemWr) */
@@ -43,6 +40,7 @@ struct hisi_ptt_pkt_buf {
size_t len;
enum hisi_ptt_pkt_type pkt_type;
enum hisi_ptt_pkt_msg_type pkt_msg_type;
+ uint32_t version;
};
int hisi_ptt_pkt_desc(struct hisi_ptt_pkt_buf *pkt_buf);
diff --git a/tools/perf/util/hisi-ptt.c b/tools/perf/util/hisi-ptt.c
index e321f393601b..4f52cfb4e190 100644
--- a/tools/perf/util/hisi-ptt.c
+++ b/tools/perf/util/hisi-ptt.c
@@ -33,6 +33,12 @@ struct hisi_ptt {
struct perf_session *session;
struct machine *machine;
u32 pmu_type;
+ u32 version;
+};
+
+static int hisi_ptt_pkt_size[] = {
+ [HISI_PTT_4DW_PKT] = 16,
+ [HISI_PTT_8DW_PKT] = 32,
};
static enum hisi_ptt_pkt_type hisi_ptt_check_packet_type(unsigned char *buf)
@@ -45,8 +51,7 @@ static enum hisi_ptt_pkt_type hisi_ptt_check_packet_type(unsigned char *buf)
return HISI_PTT_4DW_PKT;
}
-static void hisi_ptt_dump(struct hisi_ptt *ptt __maybe_unused,
- unsigned char *buf, size_t len)
+static void hisi_ptt_dump(struct hisi_ptt *ptt, unsigned char *buf, size_t len)
{
const char *color = PERF_COLOR_BLUE;
struct hisi_ptt_pkt_buf pkt_buf;
@@ -56,6 +61,7 @@ static void hisi_ptt_dump(struct hisi_ptt *ptt __maybe_unused,
pkt_buf.pkt_type = hisi_ptt_check_packet_type(buf);
pkt_buf.len = round_down(len, hisi_ptt_pkt_size[pkt_buf.pkt_type]);
pkt_buf.pkt_msg_type = HISI_PTT_PKT_TYPE_UNKNOWN;
+ pkt_buf.version = ptt->version;
color_fprintf(stdout, color, ". ... HISI PTT data: size %zu bytes\n",
pkt_buf.len);
@@ -146,12 +152,13 @@ static bool hisi_ptt_evsel_is_auxtrace(struct perf_session *session,
return evsel->core.attr.type == ptt->pmu_type;
}
-static void hisi_ptt_print_info(__u64 type)
+static void hisi_ptt_print_info(__u64 type, u32 version)
{
if (!dump_trace)
return;
fprintf(stdout, " PMU Type %" PRId64 "\n", (s64) type);
+ fprintf(stdout, " Tracer Version %" PRIu32 "\n", version);
}
int hisi_ptt_process_auxtrace_info(union perf_event *event,
@@ -159,10 +166,13 @@ int hisi_ptt_process_auxtrace_info(union perf_event *event,
{
struct perf_record_auxtrace_info *auxtrace_info = &event->auxtrace_info;
struct hisi_ptt *ptt;
+ size_t priv_size;
- if (auxtrace_info->header.size < HISI_PTT_AUXTRACE_PRIV_SIZE +
+ if (auxtrace_info->header.size < HISI_PTT_AUXTRACE_PRIV_SIZE_V1 +
sizeof(struct perf_record_auxtrace_info))
return -EINVAL;
+ priv_size = auxtrace_info->header.size -
+ sizeof(struct perf_record_auxtrace_info);
ptt = zalloc(sizeof(*ptt));
if (!ptt)
@@ -172,7 +182,8 @@ int hisi_ptt_process_auxtrace_info(union perf_event *event,
ptt->machine = &session->machines.host; /* No kvm support */
ptt->auxtrace_type = auxtrace_info->type;
ptt->pmu_type = auxtrace_info->priv[0];
-
+ ptt->version = priv_size >= HISI_PTT_AUXTRACE_PRIV_SIZE_V2 ?
+ (u32)auxtrace_info->priv[1] : HISI_PTT_DECODER_V1;
ptt->auxtrace.process_event = hisi_ptt_process_event;
ptt->auxtrace.process_auxtrace_event = hisi_ptt_process_auxtrace_event;
ptt->auxtrace.flush_events = hisi_ptt_flush;
@@ -181,7 +192,7 @@ int hisi_ptt_process_auxtrace_info(union perf_event *event,
ptt->auxtrace.evsel_is_auxtrace = hisi_ptt_evsel_is_auxtrace;
session->auxtrace = &ptt->auxtrace;
- hisi_ptt_print_info(auxtrace_info->priv[0]);
+ hisi_ptt_print_info(auxtrace_info->priv[0], ptt->version);
return 0;
}
diff --git a/tools/perf/util/hisi-ptt.h b/tools/perf/util/hisi-ptt.h
index 2db9b4056214..67b051980113 100644
--- a/tools/perf/util/hisi-ptt.h
+++ b/tools/perf/util/hisi-ptt.h
@@ -8,7 +8,9 @@
#define INCLUDE__PERF_HISI_PTT_H__
#define HISI_PTT_PMU_NAME "hisi_ptt"
-#define HISI_PTT_AUXTRACE_PRIV_SIZE sizeof(u64)
+#define HISI_PTT_AUXTRACE_PRIV_SIZE_V1 sizeof(u64)
+#define HISI_PTT_AUXTRACE_PRIV_SIZE_V2 (2 * sizeof(u64))
+#define HISI_PTT_AUXTRACE_PRIV_SIZE HISI_PTT_AUXTRACE_PRIV_SIZE_V2
struct auxtrace_record *hisi_ptt_recording_init(int *err,
struct perf_pmu *hisi_ptt_pmu);
--
2.33.0
^ permalink raw reply related
* [PATCH 06/10] perf hisi-ptt: Extract the raw data printing part
From: Sizhe Liu @ 2026-06-04 7:50 UTC (permalink / raw)
To: rostedt, mhiramat, mathieu.desnoyers, corbet, skhan, bhelgaas,
yangyccccc, jic23, john.g.garry, will, james.clark, mike.leach,
leo.yan, peterz, mingo, acme, namhyung, mark.rutland,
alexander.shishkin, jolsa, irogers, adrian.hunter, wangyushan12,
shenyang39, gaozhihao6, yuzhichengcheng, liyihang9
Cc: linux-kernel, linux-pci, linux-perf-users, linux-arm-kernel,
linux-doc, linuxarm, prime.zeng, fanghao11, wuyifan50, liusizhe5
In-Reply-To: <20260604075005.2219785-1-liusizhe5@huawei.com>
Extract the raw data printing part of the TLP header into
hisi_ptt_print_raw_record() for reuse by subsequent patches that
add field-level parsing of individual DW headers.
Signed-off-by: Sizhe Liu <liusizhe5@huawei.com>
---
.../hisi-ptt-decoder/hisi-ptt-pkt-decoder.c | 29 +++++++++----------
1 file changed, 14 insertions(+), 15 deletions(-)
diff --git a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
index 33804bcd0642..a5b66e0f7827 100644
--- a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
+++ b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
@@ -93,23 +93,31 @@ union hisi_ptt_field_data {
uint32_t value;
};
-static void hisi_ptt_print_pkt(struct hisi_ptt_pkt_buf *pkt_buf,
- const char *desc)
+static void hisi_ptt_print_raw_record(size_t offset, uint32_t value)
{
const char *color = PERF_COLOR_BLUE;
- uint32_t value;
uint8_t byte;
int i;
- value = le32_to_cpu(*(__le32 *)(pkt_buf->buf + pkt_buf->pos));
printf(".");
- color_fprintf(stdout, color, " %08zx: ", pkt_buf->pos);
+ color_fprintf(stdout, color, " %08zx: ", offset);
for (i = 0; i < HISI_PTT_FIELD_LENGTH; i++) {
byte = (value >> (24 - i * 8)) & 0xFF;
color_fprintf(stdout, color, "%02x ", byte);
}
for (i = 0; i < HISI_PTT_MAX_SPACE_LEN; i++)
color_fprintf(stdout, color, " ");
+}
+
+static void hisi_ptt_print_pkt(struct hisi_ptt_pkt_buf *pkt_buf,
+ const char *desc)
+{
+ const char *color = PERF_COLOR_BLUE;
+ uint32_t value;
+
+ value = le32_to_cpu(*(__le32 *)(pkt_buf->buf + pkt_buf->pos));
+ hisi_ptt_print_raw_record(pkt_buf->pos, value);
+
color_fprintf(stdout, color, " %s\n", desc);
pkt_buf->pos += HISI_PTT_FIELD_LENGTH;
}
@@ -135,18 +143,9 @@ static void hisi_ptt_4dw_print_dw0(struct hisi_ptt_pkt_buf *pkt_buf)
{
const char *color = PERF_COLOR_BLUE;
union hisi_ptt_field_data dw;
- uint8_t byte;
- int i;
dw.value = le32_to_cpu(*(__le32 *)(pkt_buf->buf + pkt_buf->pos));
- printf(".");
- color_fprintf(stdout, color, " %08zx: ", pkt_buf->pos);
- for (i = 0; i < HISI_PTT_FIELD_LENGTH; i++) {
- byte = (dw.value >> (24 - i * 8)) & 0xFF;
- color_fprintf(stdout, color, "%02x ", byte);
- }
- for (i = 0; i < HISI_PTT_MAX_SPACE_LEN; i++)
- color_fprintf(stdout, color, " ");
+ hisi_ptt_print_raw_record(pkt_buf->pos, dw.value);
color_fprintf(stdout, color,
" %s %x %s %x %s %x %s %x %s %x %s %x %s %x %s %x\n",
--
2.33.0
^ permalink raw reply related
* Re: [PATCH] Documentation: KVM: Synchronize x86 VM types
From: Binbin Wu @ 2026-06-04 8:12 UTC (permalink / raw)
To: Carlos López
Cc: kvm, linux-doc, linux-kernel, Paolo Bonzini, Jonathan Corbet,
Shuah Khan, Ashish Kalra, Michael Roth, Brijesh Singh,
Isaku Yamahata
In-Reply-To: <20260603114504.814647-2-clopez@suse.de>
On 6/3/2026 7:45 PM, Carlos López wrote:
> KVM has reflected KVM_X86_SNP_VM to userspace since 1dfe571c12cf
> ("KVM: SEV: Add initial SEV-SNP support"), and KVM_X86_TDX_VM since
> 161d34609f9b ("KVM: TDX: Make TDX VM type supported"). Update the
> documentation to reflect this fact.
>
> Fixes: 1dfe571c12cf ("KVM: SEV: Add initial SEV-SNP support")
> Fixes: 161d34609f9b ("KVM: TDX: Make TDX VM type supported")
> Signed-off-by: Carlos López <clopez@suse.de>
Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
> ---
> Documentation/virt/kvm/api.rst | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index 52bbbb553ce1..3ec574a41f60 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -9363,6 +9363,8 @@ means the VM type with value @n is supported. Possible values of @n are::
> #define KVM_X86_SW_PROTECTED_VM 1
> #define KVM_X86_SEV_VM 2
> #define KVM_X86_SEV_ES_VM 3
> + #define KVM_X86_SNP_VM 4
> + #define KVM_X86_TDX_VM 5
>
> Note, KVM_X86_SW_PROTECTED_VM is currently only for development and testing.
> Do not use KVM_X86_SW_PROTECTED_VM for "real" VMs, and especially not in
^ permalink raw reply
* RE: [External Mail] Re: [PATCH 00/11] net: wwan: t9xx: Add MediaTek T9XX WWAN driver
From: Wu. JackBB (GSM) @ 2026-06-04 8:22 UTC (permalink / raw)
To: Sergey Ryazanov, Jakub Kicinski, Jack Wu via B4 Relay
Cc: Loic Poulain, Johannes Berg, Andrew Lunn, David S. Miller,
Eric Dumazet, Paolo Abeni, Wen-Zhi Huang, Shi-Wei Yeh,
Minano Tseng, Matthias Brugger, AngeloGioacchino Del Regno,
Simon Horman, Jonathan Corbet, Shuah Khan,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-mediatek@lists.infradead.org, linux-doc@vger.kernel.org
In-Reply-To: <e7df5082-ef48-43d0-ad07-10e1e64e1d26@gmail.com>
Hi
> let me join the discussion and put my 2c.
>
> On 6/2/26 13:58, Wu. JackBB (GSM) wrote:
> > Hi Jakub,
> >
> > > On Fri, 29 May 2026 18:31:39 +0800 Jack Wu via B4 Relay wrote:
> > > > 43 files changed, 14761 insertions(+)
> > >
> > > Please try to cut this down to ~5kLoC for the initial submission.
> > > Whatever the absolute minimum sensible chunk of code is.
> > >
> > > Each patch must build cleanly with W=1
> >
> > We've already reduced this significantly from the original 41k LoC
> > down to ~14.7k by stripping out non-essential features such as
> > exception handling, memory logging, devlink, statistics, debug
> > tracing, and others.
> >
> > We even removed some arguably necessary features (PM, mdlog,
> > throughput optimizations) that we plan to submit as follow-up
> > series.
>
> Great work. Highly appreciate!
>
> > Note that the line count may slightly increase in v2, as we plan
> > to add missing kdoc comments based on review feedback.
> >
> > For reference, the t7xx driver (two generations older, simpler HW)
> > had an initial submission of ~11.3k LoC [1]. The t9xx hardware is
> > more complex, so we believe being in a similar range is reasonable.
>
> Let me elaborate a bit here. The size problem is not due to a git or a
> mailbox limitation. It arise due to the human limitation. The T7xx
> submission review took something about 4 months and 8 iterations. And it
> was 'only' 11.3k lines. Let's do some extrapolation assuming that
> function is linear. 14.7k is 30% bigger, thus, estimated reviewing time
> should be 5 months and 2 weeks. And this looks optimistic.
>
> Recommendation, shared by Jakub, is practical. 5k lines might be
> reviewed in a reasonable time and merged with the full confidence of the
> quality.
>
> > We'd like to keep the driver functional and reviewable in its
> > current scope. Do you have any suggestions on how we could further
> > reduce the size while maintaining a working initial submission?
>
> Off the top of my head, I would suggest joining T7xx and T9xx code
> bases. It could be done through factoring out a core functionality of
> T7xx into a library, or through making the driver layered.
>
> I am not pretending being an expert in any of these drivers, but
> generally divide-n-conqueror together with code reuse work reliable. As
> an alternative, I could spend a couple of weeks reviewing the new
> submission and will come with more specific ideas on what can be thrown
> away or reused.
>
Thank you for the detailed explanation and the practical suggestions.
We discussed this with MediaTek. The T7XX and T9XX hardware architectures
have diverged significantly, so developing a shared driver would require
substantial effort and risk introducing regressions in the existing T7XX
driver, requiring extensive testing to ensure reliability.
MediaTek also confirmed that the current driver cannot be reduced further
without removing functionality. One option would be to remove the data
plane and power management patches, which would bring the submission down
to approximately 9,000 lines:
[0/7] net: wwan: t9xx: Add MediaTek T9XX WWAN driver
[1/7] net: wwan: t9xx: Add PCIe core
[2/7] net: wwan: t9xx: Add control plane transaction layer
[3/7] net: wwan: t9xx: Add control DMA interface
[4/7] net: wwan: t9xx: Add control port
[5/7] net: wwan: t9xx: Add FSM thread
[6/7] net: wwan: t9xx: Add AT & MBIM WWAN ports
[7/7] net: wwan: t9xx: Add maintainers and documentation
We have verified that the MBIM and AT ports work independently in this
configuration — the control plane is fully functional, just without the
network interface.
That said, we would prefer to continue the review with the current 14.7k
line submission if the community is open to it, as it represents a more
complete and testable driver. We are happy to go either way based on your
preference.
Thanks.
> > [1]
> > https://patchwork.kernel.org/project/netdevbpf/cover/20220506181310.2183829-1-ricardo.martinez@linux.intel.com/
> >
> > Thanks.
> >
> >
> > ================================================================================================================================================================
> > This message may contain information which is private, privileged or
> > confidential of Compal Electronics, Inc. If you are not the intended
> > recipient of this message, please notify the sender and destroy/delete the
> > message. Any review, retransmission, dissemination or other use of, or
> > taking of any action in reliance upon this information, by persons or
> > entities other than the intended recipient is prohibited.
> > ================================================================================================================================================================
>
> And this disclaimer does not facilitate the review. Am I 'intended'
> recipient or should I destroy the message ASAP?
Please ignore this message.
Our company's mail server automatically adds this announcement to all external emails.
We apologize for the inconvenience.
Thanks.
================================================================================================================================================================
This message may contain information which is private, privileged or confidential of Compal Electronics, Inc. If you are not the intended recipient of this message, please notify the sender and destroy/delete the message. Any review, retransmission, dissemination or other use of, or taking of any action in reliance upon this information, by persons or entities other than the intended recipient is prohibited.
================================================================================================================================================================
^ permalink raw reply
* Re: [RFC PATCH v2 0/6] kcov: per-task dataflow extraction at kernel function boundaries
From: Peter Zijlstra @ 2026-06-04 8:40 UTC (permalink / raw)
To: Yunseong Kim
Cc: Ingo Molnar, Juri Lelli, Vincent Guittot, Dietmar Eggemann,
Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider,
K Prateek Nayak, Dmitry Vyukov, Andrey Konovalov, Andrew Morton,
Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
Nicolas Schier, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Jonathan Corbet, Shuah Khan,
linux-kernel, kasan-dev, llvm, linux-kbuild, rust-for-linux,
workflows, linux-doc, Yunseong Kim
In-Reply-To: <20260603-kcov-dataflow-next-20260603-v2-0-fee0939de2c4@est.tech>
On Wed, Jun 03, 2026 at 07:43:27PM +0200, Yunseong Kim wrote:
> CONFIG_KCOV_DATAFLOW_INSTRUMENT_ALL instruments every function in the
> kernel.
Well, I would hope it would very much not instrument noinstr functions.
^ permalink raw reply
* Re: [RFC PATCH v2 1/6] kcov: add per-task dataflow tracking for function arguments/return values
From: Peter Zijlstra @ 2026-06-04 8:41 UTC (permalink / raw)
To: Yunseong Kim
Cc: Ingo Molnar, Juri Lelli, Vincent Guittot, Dietmar Eggemann,
Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider,
K Prateek Nayak, Dmitry Vyukov, Andrey Konovalov, Andrew Morton,
Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
Nicolas Schier, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Jonathan Corbet, Shuah Khan,
linux-kernel, kasan-dev, llvm, linux-kbuild, rust-for-linux,
workflows, linux-doc, Yunseong Kim
In-Reply-To: <20260603-kcov-dataflow-next-20260603-v2-1-fee0939de2c4@est.tech>
On Wed, Jun 03, 2026 at 07:43:28PM +0200, Yunseong Kim wrote:
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index e2f976c3301b..abd1a94589aa 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2261,6 +2261,28 @@ config KCOV_SELFTEST
> On test failure, causes the kernel to panic. Recommended to be
> enabled, ensuring critical functionality works as intended.
>
> +
^ This line...
> +config KCOV_DATAFLOW_ARGS
> + bool "Enable KCOV dataflow: function argument capture"
> + depends on KCOV
> + depends on $(cc-option,-fsanitize-coverage=dataflow-args)
> + help
> + Captures function arguments at entry via /sys/kernel/debug/kcov_dataflow.
> + Struct pointer arguments are auto-expanded using compiler DebugInfo
> + metadata, recording individual field values at runtime.
> + Enable per-module with: KCOV_DATAFLOW_file.o := y in the Makefile.
> + Requires clang with -fsanitize-coverage=dataflow-args support.
> +
> +config KCOV_DATAFLOW_RET
> + bool "Enable KCOV dataflow: return value capture"
> + depends on KCOV
> + depends on $(cc-option,-fsanitize-coverage=dataflow-ret)
> + help
> + Captures function return values via /sys/kernel/debug/kcov_dataflow.
> + Struct pointer returns are auto-expanded using compiler DebugInfo
> + metadata, recording individual field values at runtime.
> + Enable per-module with: KCOV_DATAFLOW_file.o := y in the Makefile.
> + Requires clang with -fsanitize-coverage=dataflow-ret support.
Probably wants to be here...
> config DEBUG_AID_FOR_SYZBOT
> bool "Additional debug code for syzbot"
> default n
>
> --
> 2.43.0
>
^ permalink raw reply
* Re: [RFC PATCH v2 2/6] kcov: add build system support for dataflow instrumentation
From: Peter Zijlstra @ 2026-06-04 8:45 UTC (permalink / raw)
To: Yunseong Kim
Cc: Ingo Molnar, Juri Lelli, Vincent Guittot, Dietmar Eggemann,
Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider,
K Prateek Nayak, Dmitry Vyukov, Andrey Konovalov, Andrew Morton,
Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
Nicolas Schier, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Jonathan Corbet, Shuah Khan,
linux-kernel, kasan-dev, llvm, linux-kbuild, rust-for-linux,
workflows, linux-doc, Yunseong Kim
In-Reply-To: <20260603-kcov-dataflow-next-20260603-v2-2-fee0939de2c4@est.tech>
On Wed, Jun 03, 2026 at 07:43:29PM +0200, Yunseong Kim wrote:
> Add CFLAGS_KCOV_DATAFLOW and RUSTFLAGS_KCOV_DATAFLOW exports to
> scripts/Makefile.kcov, containing:
> -fsanitize-coverage=dataflow-args,dataflow-ret -g
> (with optional -fno-inline via CONFIG_KCOV_DATAFLOW_NO_INLINE)
>
> scripts/Makefile.lib applies these flags when a module's Makefile sets:
> KCOV_DATAFLOW_file.o := y (per-file)
> KCOV_DATAFLOW := y (per-directory)
>
> Also supports CONFIG_KCOV_DATAFLOW_INSTRUMENT_ALL for global enablement.
> The flags are only applied to kernel objects (same guard as basic KCOV).
>
> Signed-off-by: Yunseong Kim <yunseong.kim@est.tech>
> ---
> scripts/Makefile.kcov | 6 ++++++
> scripts/Makefile.lib | 7 +++++++
> 2 files changed, 13 insertions(+)
>
> diff --git a/scripts/Makefile.kcov b/scripts/Makefile.kcov
> index 78305a84ba9d..101173fe194b 100644
> --- a/scripts/Makefile.kcov
> +++ b/scripts/Makefile.kcov
> @@ -2,10 +2,16 @@
> kcov-flags-y += -fsanitize-coverage=trace-pc
> kcov-flags-$(CONFIG_KCOV_ENABLE_COMPARISONS) += -fsanitize-coverage=trace-cmp
>
> +# KCOV dataflow: trace function args and return values
> +kcov-dataflow-flags-y := -fsanitize-coverage=dataflow-args,dataflow-ret -g
> +kcov-dataflow-flags-$(CONFIG_KCOV_DATAFLOW_NO_INLINE) += -fno-inline
https://clang.llvm.org/docs/ClangCommandLineReference.html
Has no mention of -fno-inline, furthermore, what are the exact
semantics? Does it inhibit __always_inline?
^ permalink raw reply
* Re: [RFC PATCH v2 3/6] kcov: add CONFIG_KCOV_DATAFLOW_INSTRUMENT_ALL and NO_INLINE
From: Peter Zijlstra @ 2026-06-04 8:46 UTC (permalink / raw)
To: Yunseong Kim
Cc: Ingo Molnar, Juri Lelli, Vincent Guittot, Dietmar Eggemann,
Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider,
K Prateek Nayak, Dmitry Vyukov, Andrey Konovalov, Andrew Morton,
Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
Nicolas Schier, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Jonathan Corbet, Shuah Khan,
linux-kernel, kasan-dev, llvm, linux-kbuild, rust-for-linux,
workflows, linux-doc, Yunseong Kim
In-Reply-To: <20260603-kcov-dataflow-next-20260603-v2-3-fee0939de2c4@est.tech>
On Wed, Jun 03, 2026 at 07:43:30PM +0200, Yunseong Kim wrote:
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index abd1a94589aa..3b952b6361a8 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2261,7 +2261,6 @@ config KCOV_SELFTEST
> On test failure, causes the kernel to panic. Recommended to be
> enabled, ensuring critical functionality works as intended.
>
> -
> config KCOV_DATAFLOW_ARGS
> bool "Enable KCOV dataflow: function argument capture"
> depends on KCOV
> @@ -2283,6 +2282,28 @@ config KCOV_DATAFLOW_RET
> metadata, recording individual field values at runtime.
> Enable per-module with: KCOV_DATAFLOW_file.o := y in the Makefile.
> Requires clang with -fsanitize-coverage=dataflow-ret support.
> +
This goes into patch 1
^ permalink raw reply
* Re: [RFC PATCH v2 5/6] kcov: add interrupt context guard to kcov_df_write()
From: Peter Zijlstra @ 2026-06-04 8:48 UTC (permalink / raw)
To: Yunseong Kim
Cc: Ingo Molnar, Juri Lelli, Vincent Guittot, Dietmar Eggemann,
Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider,
K Prateek Nayak, Dmitry Vyukov, Andrey Konovalov, Andrew Morton,
Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
Nicolas Schier, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Jonathan Corbet, Shuah Khan,
linux-kernel, kasan-dev, llvm, linux-kbuild, rust-for-linux,
workflows, linux-doc, Yunseong Kim
In-Reply-To: <20260603-kcov-dataflow-next-20260603-v2-5-fee0939de2c4@est.tech>
On Wed, Jun 03, 2026 at 07:43:32PM +0200, Yunseong Kim wrote:
> The KCOV-Dataflow write path (kcov_df_write) only checks
> t->kcov_df_enabled before writing to the shared ring buffer. Unlike
> the standard KCOV check_kcov_mode() which rejects interrupt context,
> kcov_df_write() has no such protection. This means instrumented code
> running in hardirq, softirq, or NMI context that interrupts a task
> mid-write can re-enter kcov_df_write(), causing:
>
> - Data corruption in the ring buffer (interleaved records)
> - Out-of-order sequence counter increments
> - Potential faults from nested pointer dereferences
>
> Add an in_task() check to reject calls from non-task context, matching
> the safety model of the standard KCOV tracing path.
>
> Also suppress -Wmissing-prototypes in the eight_args_c test module
> Makefile, as the exported test functions intentionally lack a shared
> header.
>
> Signed-off-by: Yunseong Kim <yunseong.kim@est.tech>
> ---
> kernel/kcov.c | 4 ++++
> tools/kcov-dataflow/eight_args_c/Makefile | 1 +
> 2 files changed, 5 insertions(+)
>
> diff --git a/kernel/kcov.c b/kernel/kcov.c
> index d3c9c0efe961..373b8034ca5c 100644
> --- a/kernel/kcov.c
> +++ b/kernel/kcov.c
> @@ -409,6 +409,10 @@ kcov_df_write(u64 type_marker, u64 pc, u64 meta, void *ptr,
> if (!t->kcov_df_enabled)
> return;
>
> + /* Reject calls from hardirq/softirq/NMI to prevent reentrant corruption. */
> + if (!in_task())
> + return;
> +
> area = (u64 *)t->kcov_df_area;
> if (!area)
> return;
> diff --git a/tools/kcov-dataflow/eight_args_c/Makefile b/tools/kcov-dataflow/eight_args_c/Makefile
> index de35bb541f07..038775b49435 100644
> --- a/tools/kcov-dataflow/eight_args_c/Makefile
> +++ b/tools/kcov-dataflow/eight_args_c/Makefile
> @@ -1,2 +1,3 @@
> obj-m := eight_args_mod.o
> KCOV_DATAFLOW_eight_args_mod.o := y
> +ccflags-y += -Wno-missing-prototypes
This is a weird commit and probably should not exist. You introduce
kcov_df_write() a few patches ago, why doesn't it add these few lines
there?
Similarly, you introduce this tools thing a few patches ago, fix the
Makefile there?
^ permalink raw reply
* Re: [RFC PATCH v2 6/6] kcov: add recursion guard and documentation for kcov-dataflow
From: Peter Zijlstra @ 2026-06-04 8:52 UTC (permalink / raw)
To: Yunseong Kim
Cc: Ingo Molnar, Juri Lelli, Vincent Guittot, Dietmar Eggemann,
Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider,
K Prateek Nayak, Dmitry Vyukov, Andrey Konovalov, Andrew Morton,
Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
Nicolas Schier, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Jonathan Corbet, Shuah Khan,
linux-kernel, kasan-dev, llvm, linux-kbuild, rust-for-linux,
workflows, linux-doc, Yunseong Kim
In-Reply-To: <20260603-kcov-dataflow-next-20260603-v2-6-fee0939de2c4@est.tech>
On Wed, Jun 03, 2026 at 07:43:33PM +0200, Yunseong Kim wrote:
> Add a per-task recursion guard to kcov_df_write() using the high bit of
> kcov_dataflow_seq. This prevents infinite recursion when
> CONFIG_KCOV_DATAFLOW_INSTRUMENT_ALL is enabled: functions called by the
> callback itself (copy_from_kernel_nofault, xadd helpers) are also
> instrumented and would re-enter kcov_df_write() without this guard.
>
> The guard uses the sequence counter's bit 31 as a re-entrancy flag.
> The low 24 bits (used for TLV record sequence numbers) are unaffected.
>
> Also:
> - Exclude kcov.o, extable.o, softirq.o from dataflow instrumentation
> (same pattern as KCOV_INSTRUMENT exclusions)
> - Add Documentation/dev-tools/kcov-dataflow.rst with:
> - Prerequisites and Kconfig options
> - Per-module instrumentation instructions
> - Complete C example for data collection
> - Ring buffer format specification
> - Ioctl interface reference
> - Fork interception example for child process tracing
> - Rust module support via post-compilation pipeline
>
> Signed-off-by: Yunseong Kim <yunseong.kim@est.tech>
Another really weird patch. Adding Documentation is okay I suppose,
although I still utterly detest this rst crap. I still fail to see
what's wrong with plain text :-(
But then you do these two other random things in the same patch. Which
make no sense.
> diff --git a/kernel/Makefile b/kernel/Makefile
> index 1e1a31673577..9c56421c5390 100644
> --- a/kernel/Makefile
> +++ b/kernel/Makefile
> @@ -37,6 +37,7 @@ KCOV_INSTRUMENT_extable.o := n
> KCOV_INSTRUMENT_stacktrace.o := n
> # Don't self-instrument.
> KCOV_INSTRUMENT_kcov.o := n
> +KCOV_DATAFLOW_kcov.o := n
> # If sanitizers detect any issues in kcov, it may lead to recursion
> # via printk, etc.
> KASAN_SANITIZE_kcov.o := n
> @@ -207,3 +208,5 @@ $(obj)/kheaders.md5: $(obj)/kheaders-srclist FORCE
> $(call filechk,kheaders_md5sum)
>
> clean-files := kheaders.md5 kheaders-srclist kheaders-objlist
> +KCOV_DATAFLOW_extable.o := n
> +KCOV_DATAFLOW_softirq.o := n
Why?!?! You Changelog also does not elucidate.
> diff --git a/kernel/kcov.c b/kernel/kcov.c
> index 373b8034ca5c..8d9d5e33549f 100644
> --- a/kernel/kcov.c
> +++ b/kernel/kcov.c
> @@ -413,6 +413,16 @@ kcov_df_write(u64 type_marker, u64 pc, u64 meta, void *ptr,
> if (!in_task())
> return;
>
> + /*
> + * Prevent recursion: functions called by this callback
> + * (copy_from_kernel_nofault, xadd helpers) may be instrumented
> + * with INSTRUMENT_ALL. Use a per-task guard via the sequence
> + * counter's high bit.
> + */
> + if (t->kcov_dataflow_seq & (1U << 31))
> + return;
> + t->kcov_dataflow_seq |= (1U << 31);
> +
> area = (u64 *)t->kcov_df_area;
> if (!area)
> return;
> @@ -449,7 +459,7 @@ kcov_df_write(u64 type_marker, u64 pc, u64 meta, void *ptr,
> if (KCOV_DF_IS_ERR(ptr)) {
> for (i = 0; i < num_fields; i++)
> area[pos + 3 + i] = KCOV_DF_MAGIC_BAD;
> - return;
> + goto out;
> }
> for (i = 0; i < num_fields; i++) {
> u64 off, sz, val = KCOV_DF_MAGIC_BAD;
> @@ -469,6 +479,8 @@ kcov_df_write(u64 type_marker, u64 pc, u64 meta, void *ptr,
> area[pos + 3 + i] = val;
> }
> }
> +out:
> + t->kcov_dataflow_seq &= ~(1U << 31);
> }
>
> #ifdef CONFIG_KCOV_DATAFLOW_ARGS
This needs barrier()s to be functional. And should be in a separate
patch.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox