linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] remoteproc: Fixes for memoy carveout management
@ 2018-11-29 21:29 Loic Pallardy
  2018-11-29 21:29 ` [PATCH 1/7] remoteproc: correct rproc_mem_entry_init() comments Loic Pallardy
                   ` (6 more replies)
  0 siblings, 7 replies; 11+ messages in thread
From: Loic Pallardy @ 2018-11-29 21:29 UTC (permalink / raw)
  To: bjorn.andersson, ohad
  Cc: linux-remoteproc, linux-kernel, arnaud.pouliquen,
	benjamin.gaignard, s-anna, Loic Pallardy

These patches fix the comments sent on remoteproc mailing
list after acceptation of memory carveout patch series [1].

In few words, series corrects:
- memory carveout allocation for rproc without iommu
- rproc_da_to_va and trace buffer access to take into account
  late carveout allocation
- resource table cast by adding warning messages

Regards,
Loic

[1] https://lkml.org/lkml/2018/7/27/612

Loic Pallardy (7):
  remoteproc: correct rproc_mem_entry_init() comments
  remoteproc: fix rproc_da_to_va in case of unallocated carveout
  remoteproc: fix rproc_alloc_carveout() bad variable cast
  remoteproc: add warning on resource table cast
  remoteproc: fix rproc_alloc_carveout() for rproc with iommu domain
  remoteproc: fix trace buffer va initialization
  remoteproc: fix rproc_check_carveout_da() returned error and comments

 drivers/remoteproc/remoteproc_core.c     | 91 +++++++++++++++++++-------------
 drivers/remoteproc/remoteproc_debugfs.c  | 21 ++++++--
 drivers/remoteproc/remoteproc_internal.h |  9 +++-
 3 files changed, 78 insertions(+), 43 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/7] remoteproc: correct rproc_mem_entry_init() comments
  2018-11-29 21:29 [PATCH 0/7] remoteproc: Fixes for memoy carveout management Loic Pallardy
@ 2018-11-29 21:29 ` Loic Pallardy
  2018-11-29 21:29 ` [PATCH 2/7] remoteproc: fix rproc_da_to_va in case of unallocated carveout Loic Pallardy
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Loic Pallardy @ 2018-11-29 21:29 UTC (permalink / raw)
  To: bjorn.andersson, ohad
  Cc: linux-remoteproc, linux-kernel, arnaud.pouliquen,
	benjamin.gaignard, s-anna, Loic Pallardy

Add alloc parameter description and correct comment
about release one.

Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
---
 drivers/remoteproc/remoteproc_core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 54ec38fc5dca..c1a66e25b173 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -900,7 +900,8 @@ EXPORT_SYMBOL(rproc_add_carveout);
  * @dma: dma address
  * @len: memory carveout length
  * @da: device address
- * @release: memory carveout function
+ * @alloc: memory carveout allocation function
+ * @release: memory carveout release function
  * @name: carveout name
  *
  * This function allocates a rproc_mem_entry struct and fill it with parameters
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/7] remoteproc: fix rproc_da_to_va in case of unallocated carveout
  2018-11-29 21:29 [PATCH 0/7] remoteproc: Fixes for memoy carveout management Loic Pallardy
  2018-11-29 21:29 ` [PATCH 1/7] remoteproc: correct rproc_mem_entry_init() comments Loic Pallardy
@ 2018-11-29 21:29 ` Loic Pallardy
  2018-11-29 21:29 ` [PATCH 3/7] remoteproc: fix rproc_alloc_carveout() bad variable cast Loic Pallardy
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Loic Pallardy @ 2018-11-29 21:29 UTC (permalink / raw)
  To: bjorn.andersson, ohad
  Cc: linux-remoteproc, linux-kernel, arnaud.pouliquen,
	benjamin.gaignard, s-anna, Loic Pallardy

With introduction of rproc_alloc_registered_carveouts() which
delays carveout allocation just before the start of the remote
processor, rproc_da_to_va() could be called before all carveouts
are allocated.
This patch adds a check in rproc_da_to_va() to return NULL if
carveout is not allocated.

Fixes: d7c51706d095 ("remoteproc: add alloc ops in rproc_mem_entry struct")

Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
---
 drivers/remoteproc/remoteproc_core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index c1a66e25b173..28df71cb3fef 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -204,6 +204,10 @@ void *rproc_da_to_va(struct rproc *rproc, u64 da, int len)
 	list_for_each_entry(carveout, &rproc->carveouts, node) {
 		int offset = da - carveout->da;
 
+		/*  Verify that carveout is allocated */
+		if (!carveout->va)
+			continue;
+
 		/* try next carveout if da is too small */
 		if (offset < 0)
 			continue;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/7] remoteproc: fix rproc_alloc_carveout() bad variable cast
  2018-11-29 21:29 [PATCH 0/7] remoteproc: Fixes for memoy carveout management Loic Pallardy
  2018-11-29 21:29 ` [PATCH 1/7] remoteproc: correct rproc_mem_entry_init() comments Loic Pallardy
  2018-11-29 21:29 ` [PATCH 2/7] remoteproc: fix rproc_da_to_va in case of unallocated carveout Loic Pallardy
@ 2018-11-29 21:29 ` Loic Pallardy
  2018-11-29 21:29 ` [PATCH 4/7] remoteproc: add warning on resource table cast Loic Pallardy
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Loic Pallardy @ 2018-11-29 21:29 UTC (permalink / raw)
  To: bjorn.andersson, ohad
  Cc: linux-remoteproc, linux-kernel, arnaud.pouliquen,
	benjamin.gaignard, s-anna, Loic Pallardy

As dma member of struct rproc_mem_entry is dma_addr_t, no
need to cast in u32.

Fixes: d7c51706d095 ("remoteproc: add alloc ops in rproc_mem_entry struct")

Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
---
 drivers/remoteproc/remoteproc_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 28df71cb3fef..18a1bbf820c9 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -775,7 +775,7 @@ static int rproc_alloc_carveout(struct rproc *rproc,
 		mem->da = (u32)dma;
 	}
 
-	mem->dma = (u32)dma;
+	mem->dma = dma;
 	mem->va = va;
 
 	return 0;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/7] remoteproc: add warning on resource table cast
  2018-11-29 21:29 [PATCH 0/7] remoteproc: Fixes for memoy carveout management Loic Pallardy
                   ` (2 preceding siblings ...)
  2018-11-29 21:29 ` [PATCH 3/7] remoteproc: fix rproc_alloc_carveout() bad variable cast Loic Pallardy
@ 2018-11-29 21:29 ` Loic Pallardy
  2018-11-30 14:33   ` Arnd Bergmann
  2018-11-29 21:29 ` [PATCH 5/7] remoteproc: fix rproc_alloc_carveout() for rproc with iommu domain Loic Pallardy
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: Loic Pallardy @ 2018-11-29 21:29 UTC (permalink / raw)
  To: bjorn.andersson, ohad
  Cc: linux-remoteproc, linux-kernel, arnaud.pouliquen,
	benjamin.gaignard, s-anna, Loic Pallardy

Today resource table supports only 32bit address fields.
This is not compliant with 64bit platform for which addresses
are cast in 32bit.
This patch adds warn messages when address cast is done.

Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
---
 drivers/remoteproc/remoteproc_core.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 18a1bbf820c9..61c954bd695e 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -772,6 +772,10 @@ static int rproc_alloc_carveout(struct rproc *rproc,
 		dev_dbg(dev, "carveout mapped 0x%x to %pad\n",
 			mem->da, &dma);
 	} else {
+		/* Update device address as undefined by requester */
+		if (sizeof(dma_addr_t) > sizeof(u32))
+			dev_warn(dev, "DMA address cast in 32bit to fit resource table format\n");
+
 		mem->da = (u32)dma;
 	}
 
@@ -1150,6 +1154,10 @@ static int rproc_alloc_registered_carveouts(struct rproc *rproc)
 			 */
 
 			/* Use va if defined else dma to generate pa */
+			if (sizeof(dma_addr_t) > sizeof(u32) ||
+			    sizeof(phys_addr_t) > sizeof(u32))
+				dev_warn(dev, "Physical address cast in 32bit to fit resource table format\n");
+
 			if (entry->va)
 				rsc->pa = (u32)rproc_va_to_pa(entry->va);
 			else
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 5/7] remoteproc: fix rproc_alloc_carveout() for rproc with iommu domain
  2018-11-29 21:29 [PATCH 0/7] remoteproc: Fixes for memoy carveout management Loic Pallardy
                   ` (3 preceding siblings ...)
  2018-11-29 21:29 ` [PATCH 4/7] remoteproc: add warning on resource table cast Loic Pallardy
@ 2018-11-29 21:29 ` Loic Pallardy
  2018-11-29 21:29 ` [PATCH 6/7] remoteproc: fix trace buffer va initialization Loic Pallardy
  2018-11-29 21:29 ` [PATCH 7/7] remoteproc: fix rproc_check_carveout_da() returned error and comments Loic Pallardy
  6 siblings, 0 replies; 11+ messages in thread
From: Loic Pallardy @ 2018-11-29 21:29 UTC (permalink / raw)
  To: bjorn.andersson, ohad
  Cc: linux-remoteproc, linux-kernel, arnaud.pouliquen,
	benjamin.gaignard, s-anna, Loic Pallardy

Correct remoteproc core behavior when memory carveout device
address is fixed in resource table and rproc device doesn't have
associated IOMMU.
Current returned error is breaking legacy on TI platforms.
This patch restores previous behavior. It adds a warn message when
allocation doesn't fit carveout request, but doesn't stop rproc_start()
sequence anymore.

Fixes: 3bc8140b157c ("remoteproc: configure IOMMU only if device address requested")

Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
---
 drivers/remoteproc/remoteproc_core.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 61c954bd695e..f19bc6961e40 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -719,6 +719,18 @@ static int rproc_alloc_carveout(struct rproc *rproc,
 	dev_dbg(dev, "carveout va %pK, dma %pad, len 0x%x\n",
 		va, &dma, mem->len);
 
+	if (mem->da != FW_RSC_ADDR_ANY && !rproc->domain) {
+		/*
+		 * Check requested da is equal to dma address
+		 * and print a warn message in case of missalignment.
+		 * Don't stop rproc_start sequence as coprocessor may
+		 * build pa to da translation on its side.
+		 */
+		if (mem->da != (u32)dma)
+			dev_warn(dev->parent,
+				 "Allocated carveout doesn't fit device address request\n");
+	}
+
 	/*
 	 * Ok, this is non-standard.
 	 *
@@ -736,15 +748,7 @@ static int rproc_alloc_carveout(struct rproc *rproc,
 	 * to use the iommu-based DMA API: we expect 'dma' to contain the
 	 * physical address in this case.
 	 */
-
-	if (mem->da != FW_RSC_ADDR_ANY) {
-		if (!rproc->domain) {
-			dev_err(dev->parent,
-				"Bad carveout rsc configuration\n");
-			ret = -ENOMEM;
-			goto dma_free;
-		}
-
+	if (mem->da != FW_RSC_ADDR_ANY && rproc->domain) {
 		mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
 		if (!mapping) {
 			ret = -ENOMEM;
@@ -771,7 +775,9 @@ static int rproc_alloc_carveout(struct rproc *rproc,
 
 		dev_dbg(dev, "carveout mapped 0x%x to %pad\n",
 			mem->da, &dma);
-	} else {
+	}
+
+	if (mem->da == FW_RSC_ADDR_ANY) {
 		/* Update device address as undefined by requester */
 		if (sizeof(dma_addr_t) > sizeof(u32))
 			dev_warn(dev, "DMA address cast in 32bit to fit resource table format\n");
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 6/7] remoteproc: fix trace buffer va initialization
  2018-11-29 21:29 [PATCH 0/7] remoteproc: Fixes for memoy carveout management Loic Pallardy
                   ` (4 preceding siblings ...)
  2018-11-29 21:29 ` [PATCH 5/7] remoteproc: fix rproc_alloc_carveout() for rproc with iommu domain Loic Pallardy
@ 2018-11-29 21:29 ` Loic Pallardy
  2018-12-06 20:37   ` Loic PALLARDY
  2018-11-29 21:29 ` [PATCH 7/7] remoteproc: fix rproc_check_carveout_da() returned error and comments Loic Pallardy
  6 siblings, 1 reply; 11+ messages in thread
From: Loic Pallardy @ 2018-11-29 21:29 UTC (permalink / raw)
  To: bjorn.andersson, ohad
  Cc: linux-remoteproc, linux-kernel, arnaud.pouliquen,
	benjamin.gaignard, s-anna, Loic Pallardy

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@st.com>
---
 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;
 }
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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 7/7] remoteproc: fix rproc_check_carveout_da() returned error and comments
  2018-11-29 21:29 [PATCH 0/7] remoteproc: Fixes for memoy carveout management Loic Pallardy
                   ` (5 preceding siblings ...)
  2018-11-29 21:29 ` [PATCH 6/7] remoteproc: fix trace buffer va initialization Loic Pallardy
@ 2018-11-29 21:29 ` Loic Pallardy
  6 siblings, 0 replies; 11+ messages in thread
From: Loic Pallardy @ 2018-11-29 21:29 UTC (permalink / raw)
  To: bjorn.andersson, ohad
  Cc: linux-remoteproc, linux-kernel, arnaud.pouliquen,
	benjamin.gaignard, s-anna, Loic Pallardy

Fix typo in comments.
Change returned error from ENOMEM to EINVAL as
not dealing with memory allocation.
Remove carveout forced da update and return an error
when no configuration match

Fixes: c874bf59add0 ("remoteproc: add helper function to check carveout device address")

Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
---
 drivers/remoteproc/remoteproc_core.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 9dbcc16f8782..b69ce5bff732 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -276,25 +276,27 @@ rproc_find_carveout_by_name(struct rproc *rproc, const char *name, ...)
  * @len: associated area size
  *
  * This function is a helper function to verify requested device area (couple
- * da, len) is part of specified carevout.
+ * da, len) is part of specified carveout.
+ * If da is not set (defined as FW_RSC_ADDR_ANY), only requested length is
+ * checked.
  *
- * Return: 0 if carveout match request else -ENOMEM
+ * Return: 0 if carveout matches request else error
  */
-int rproc_check_carveout_da(struct rproc *rproc, struct rproc_mem_entry *mem,
-			    u32 da, u32 len)
+static int rproc_check_carveout_da(struct rproc *rproc,
+				   struct rproc_mem_entry *mem, u32 da, u32 len)
 {
 	struct device *dev = &rproc->dev;
-	int delta = 0;
+	int delta;
 
 	/* Check requested resource length */
 	if (len > mem->len) {
 		dev_err(dev, "Registered carveout doesn't fit len request\n");
-		return -ENOMEM;
+		return -EINVAL;
 	}
 
 	if (da != FW_RSC_ADDR_ANY && mem->da == FW_RSC_ADDR_ANY) {
-		/* Update existing carveout da */
-		mem->da = da;
+		/* Address doesn't match registered carveout configuration */
+		return -EINVAL;
 	} else if (da != FW_RSC_ADDR_ANY && mem->da != FW_RSC_ADDR_ANY) {
 		delta = da - mem->da;
 
@@ -302,13 +304,13 @@ int rproc_check_carveout_da(struct rproc *rproc, struct rproc_mem_entry *mem,
 		if (delta < 0) {
 			dev_err(dev,
 				"Registered carveout doesn't fit da request\n");
-			return -ENOMEM;
+			return -EINVAL;
 		}
 
 		if (delta + len > mem->len) {
 			dev_err(dev,
 				"Registered carveout doesn't fit len request\n");
-			return -ENOMEM;
+			return -EINVAL;
 		}
 	}
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/7] remoteproc: add warning on resource table cast
  2018-11-29 21:29 ` [PATCH 4/7] remoteproc: add warning on resource table cast Loic Pallardy
@ 2018-11-30 14:33   ` Arnd Bergmann
  2018-11-30 20:48     ` Loic PALLARDY
  0 siblings, 1 reply; 11+ messages in thread
From: Arnd Bergmann @ 2018-11-30 14:33 UTC (permalink / raw)
  To: Loic Pallardy
  Cc: Bjorn Andersson, Ohad Ben-Cohen, linux-remoteproc,
	Linux Kernel Mailing List, Arnaud Pouliquen, Benjamin Gaignard,
	Suman Anna

On Thu, Nov 29, 2018 at 10:31 PM Loic Pallardy <loic.pallardy@st.com> wrote:
>
> Today resource table supports only 32bit address fields.
> This is not compliant with 64bit platform for which addresses
> are cast in 32bit.
> This patch adds warn messages when address cast is done.
>
> Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
> ---
>  drivers/remoteproc/remoteproc_core.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 18a1bbf820c9..61c954bd695e 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -772,6 +772,10 @@ static int rproc_alloc_carveout(struct rproc *rproc,
>                 dev_dbg(dev, "carveout mapped 0x%x to %pad\n",
>                         mem->da, &dma);
>         } else {
> +               /* Update device address as undefined by requester */
> +               if (sizeof(dma_addr_t) > sizeof(u32))
> +                       dev_warn(dev, "DMA address cast in 32bit to fit resource table format\n");
> +
>                 mem->da = (u32)dma;
>         }

I think this patch is wrong: sizeof(dma_addr_t) is defined to be large enough
to support any machine that the kernel can run on. If you build a kernel that
happens to work on an ARM32 platform with LPAE enabled, then this will
warn every time, even if you are running on a machine that only has a 32-bit
address space.

     Arnd

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH 4/7] remoteproc: add warning on resource table cast
  2018-11-30 14:33   ` Arnd Bergmann
@ 2018-11-30 20:48     ` Loic PALLARDY
  0 siblings, 0 replies; 11+ messages in thread
From: Loic PALLARDY @ 2018-11-30 20:48 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Bjorn Andersson, Ohad Ben-Cohen, linux-remoteproc,
	Linux Kernel Mailing List, Arnaud POULIQUEN, Benjamin Gaignard,
	Suman Anna



> -----Original Message-----
> From: Arnd Bergmann <arnd@arndb.de>
> Sent: vendredi 30 novembre 2018 15:33
> To: Loic PALLARDY <loic.pallardy@st.com>
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>; Ohad Ben-Cohen
> <ohad@wizery.com>; linux-remoteproc@vger.kernel.org; Linux Kernel
> Mailing List <linux-kernel@vger.kernel.org>; Arnaud POULIQUEN
> <arnaud.pouliquen@st.com>; Benjamin Gaignard
> <benjamin.gaignard@linaro.org>; Suman Anna <s-anna@ti.com>
> Subject: Re: [PATCH 4/7] remoteproc: add warning on resource table cast
> 
> On Thu, Nov 29, 2018 at 10:31 PM Loic Pallardy <loic.pallardy@st.com> wrote:
> >
> > Today resource table supports only 32bit address fields.
> > This is not compliant with 64bit platform for which addresses
> > are cast in 32bit.
> > This patch adds warn messages when address cast is done.
> >
> > Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
> > ---
> >  drivers/remoteproc/remoteproc_core.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/remoteproc/remoteproc_core.c
> b/drivers/remoteproc/remoteproc_core.c
> > index 18a1bbf820c9..61c954bd695e 100644
> > --- a/drivers/remoteproc/remoteproc_core.c
> > +++ b/drivers/remoteproc/remoteproc_core.c
> > @@ -772,6 +772,10 @@ static int rproc_alloc_carveout(struct rproc *rproc,
> >                 dev_dbg(dev, "carveout mapped 0x%x to %pad\n",
> >                         mem->da, &dma);
> >         } else {
> > +               /* Update device address as undefined by requester */
> > +               if (sizeof(dma_addr_t) > sizeof(u32))
> > +                       dev_warn(dev, "DMA address cast in 32bit to fit resource table
> format\n");
> > +
> >                 mem->da = (u32)dma;
> >         }
> 
> I think this patch is wrong: sizeof(dma_addr_t) is defined to be large enough
> to support any machine that the kernel can run on. If you build a kernel that
> happens to work on an ARM32 platform with LPAE enabled, then this will
> warn every time, even if you are running on a machine that only has a 32-bit
> address space.

I suppose if LPAE is enabled, address space is larger than 32bit.
But I agree it is possible to display a warn only when some information will be lost
due to cast.
I'll propose a new version testing dma address itself and displaying warn only if above 4GB.

Regards,
Loic
> 
>      Arnd

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH 6/7] remoteproc: fix trace buffer va initialization
  2018-11-29 21:29 ` [PATCH 6/7] remoteproc: fix trace buffer va initialization Loic Pallardy
@ 2018-12-06 20:37   ` Loic PALLARDY
  0 siblings, 0 replies; 11+ messages in thread
From: Loic PALLARDY @ 2018-12-06 20:37 UTC (permalink / raw)
  To: bjorn.andersson, ohad
  Cc: linux-remoteproc, linux-kernel, Arnaud POULIQUEN,
	benjamin.gaignard, s-anna



> -----Original Message-----
> From: Loic PALLARDY
> Sent: jeudi 29 novembre 2018 22:29
> To: bjorn.andersson@linaro.org; ohad@wizery.com
> Cc: linux-remoteproc@vger.kernel.org; linux-kernel@vger.kernel.org;
> Arnaud POULIQUEN <arnaud.pouliquen@st.com>;
> benjamin.gaignard@linaro.org; s-anna@ti.com; Loic PALLARDY
> <loic.pallardy@st.com>
> 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@st.com>
> ---
>  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


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-12-06 20:37 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-29 21:29 [PATCH 0/7] remoteproc: Fixes for memoy carveout management Loic Pallardy
2018-11-29 21:29 ` [PATCH 1/7] remoteproc: correct rproc_mem_entry_init() comments Loic Pallardy
2018-11-29 21:29 ` [PATCH 2/7] remoteproc: fix rproc_da_to_va in case of unallocated carveout Loic Pallardy
2018-11-29 21:29 ` [PATCH 3/7] remoteproc: fix rproc_alloc_carveout() bad variable cast Loic Pallardy
2018-11-29 21:29 ` [PATCH 4/7] remoteproc: add warning on resource table cast Loic Pallardy
2018-11-30 14:33   ` Arnd Bergmann
2018-11-30 20:48     ` Loic PALLARDY
2018-11-29 21:29 ` [PATCH 5/7] remoteproc: fix rproc_alloc_carveout() for rproc with iommu domain Loic Pallardy
2018-11-29 21:29 ` [PATCH 6/7] remoteproc: fix trace buffer va initialization Loic Pallardy
2018-12-06 20:37   ` Loic PALLARDY
2018-11-29 21:29 ` [PATCH 7/7] remoteproc: fix rproc_check_carveout_da() returned error and comments Loic Pallardy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).