RE: [PATCH 6/7] remoteproc: fix trace buffer va initialization
From: Loic PALLARDY
Date: Thu Dec 06 2018 - 15:37:12 EST
> -----Original Message-----
> From: Loic PALLARDY
> Sent: jeudi 29 novembre 2018 22:29
> To: bjorn.andersson@xxxxxxxxxx; ohad@xxxxxxxxxx
> Cc: linux-remoteproc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> Arnaud POULIQUEN <arnaud.pouliquen@xxxxxx>;
> benjamin.gaignard@xxxxxxxxxx; s-anna@xxxxxx; Loic PALLARDY
> <loic.pallardy@xxxxxx>
> Subject: [PATCH 6/7] remoteproc: fix trace buffer va initialization
>
> With rproc_alloc_registered_carveouts() introduction, carveouts are
> allocated after resource table parsing.
> rproc_da_to_va() may return NULL at trace resource registering.
> This patch modifies trace debufs registering to provide device address
> (da) instead of va.
> da to va translation is done at each trace buffer access
> through debugfs interface.
>
> Fixes: d7c51706d095 ("remoteproc: add alloc ops in rproc_mem_entry
> struct")
>
> Signed-off-by: Loic Pallardy <loic.pallardy@xxxxxx>
> ---
> drivers/remoteproc/remoteproc_core.c | 26 ++++++++++----------------
> drivers/remoteproc/remoteproc_debugfs.c | 21 +++++++++++++++++----
> drivers/remoteproc/remoteproc_internal.h | 9 ++++++++-
> 3 files changed, 35 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/remoteproc/remoteproc_core.c
> b/drivers/remoteproc/remoteproc_core.c
> index f19bc6961e40..9dbcc16f8782 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -562,9 +562,8 @@ void rproc_vdev_release(struct kref *ref)
> static int rproc_handle_trace(struct rproc *rproc, struct fw_rsc_trace *rsc,
> int offset, int avail)
> {
> - struct rproc_mem_entry *trace;
> + struct rproc_debug_trace *trace;
> struct device *dev = &rproc->dev;
> - void *ptr;
> char name[15];
>
> if (sizeof(*rsc) > avail) {
> @@ -578,28 +577,23 @@ static int rproc_handle_trace(struct rproc *rproc,
> struct fw_rsc_trace *rsc,
> return -EINVAL;
> }
>
> - /* what's the kernel address of this resource ? */
> - ptr = rproc_da_to_va(rproc, rsc->da, rsc->len);
> - if (!ptr) {
> - dev_err(dev, "erroneous trace resource entry\n");
> - return -EINVAL;
> - }
> -
> trace = kzalloc(sizeof(*trace), GFP_KERNEL);
> if (!trace)
> return -ENOMEM;
>
> /* set the trace buffer dma properties */
> - trace->len = rsc->len;
> - trace->va = ptr;
> + trace->trace_mem.len = rsc->len;
> + trace->trace_mem.da = rsc->da;
> +
> + /* set pointer on rproc device */
> + trace->rproc = rproc;
>
> /* make sure snprintf always null terminates, even if truncating */
> snprintf(name, sizeof(name), "trace%d", rproc->num_traces);
>
> /* create the debugfs entry */
> - trace->priv = rproc_create_trace_file(name, rproc, trace);
> - if (!trace->priv) {
> - trace->va = NULL;
> + trace->tfile = rproc_create_trace_file(name, rproc, trace);
> + if (!trace->tfile) {
> kfree(trace);
> return -EINVAL;
> }
> @@ -608,8 +602,8 @@ static int rproc_handle_trace(struct rproc *rproc,
> struct fw_rsc_trace *rsc,
>
> rproc->num_traces++;
>
> - dev_dbg(dev, "%s added: va %pK, da 0x%x, len 0x%x\n",
> - name, ptr, rsc->da, rsc->len);
> + dev_dbg(dev, "%s added: da 0x%x, len 0x%x\n",
> + name, rsc->da, rsc->len);
>
> return 0;
> }
rproc_resource_cleanup() modification not included in this patch. (lost during rebase)
I'll post a v2.
Regards,
Loic
> diff --git a/drivers/remoteproc/remoteproc_debugfs.c
> b/drivers/remoteproc/remoteproc_debugfs.c
> index e90135c64af0..11240b4d0a91 100644
> --- a/drivers/remoteproc/remoteproc_debugfs.c
> +++ b/drivers/remoteproc/remoteproc_debugfs.c
> @@ -47,10 +47,23 @@ static struct dentry *rproc_dbg;
> static ssize_t rproc_trace_read(struct file *filp, char __user *userbuf,
> size_t count, loff_t *ppos)
> {
> - struct rproc_mem_entry *trace = filp->private_data;
> - int len = strnlen(trace->va, trace->len);
> + struct rproc_debug_trace *data = filp->private_data;
> + struct rproc_mem_entry *trace = &data->trace_mem;
> + void *va;
> + char buf[100];
> + int len;
> +
> + va = rproc_da_to_va(data->rproc, trace->da, trace->len);
> +
> + if (!va) {
> + len = scnprintf(buf, sizeof(buf), "Trace %s not available\n",
> + trace->name);
> + va = buf;
> + } else {
> + len = strnlen(va, trace->len);
> + }
>
> - return simple_read_from_buffer(userbuf, count, ppos, trace->va,
> len);
> + return simple_read_from_buffer(userbuf, count, ppos, va, len);
> }
>
> static const struct file_operations trace_rproc_ops = {
> @@ -288,7 +301,7 @@ void rproc_remove_trace_file(struct dentry *tfile)
> }
>
> struct dentry *rproc_create_trace_file(const char *name, struct rproc
> *rproc,
> - struct rproc_mem_entry *trace)
> + struct rproc_debug_trace *trace)
> {
> struct dentry *tfile;
>
> diff --git a/drivers/remoteproc/remoteproc_internal.h
> b/drivers/remoteproc/remoteproc_internal.h
> index f6cad243d7ca..7d8936688164 100644
> --- a/drivers/remoteproc/remoteproc_internal.h
> +++ b/drivers/remoteproc/remoteproc_internal.h
> @@ -25,6 +25,13 @@
>
> struct rproc;
>
> +struct rproc_debug_trace {
> + struct rproc *rproc;
> + struct dentry *tfile;
> + struct list_head node;
> + struct rproc_mem_entry trace_mem;
> +};
> +
> /* from remoteproc_core.c */
> void rproc_release(struct kref *kref);
> irqreturn_t rproc_vq_interrupt(struct rproc *rproc, int vq_id);
> @@ -37,7 +44,7 @@ void rproc_remove_virtio_dev(struct rproc_vdev
> *rvdev);
> /* from remoteproc_debugfs.c */
> void rproc_remove_trace_file(struct dentry *tfile);
> struct dentry *rproc_create_trace_file(const char *name, struct rproc
> *rproc,
> - struct rproc_mem_entry *trace);
> + struct rproc_debug_trace *trace);
> void rproc_delete_debug_dir(struct rproc *rproc);
> void rproc_create_debug_dir(struct rproc *rproc);
> void rproc_init_debugfs(void);
> --
> 2.7.4