From: Suman Anna <s-anna@ti.com>
To: Bjorn Andersson <bjorn.andersson@linaro.org>,
Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Grzegorz Jaszczyk <grzegorz.jaszczyk@linaro.org>,
Jan Kiszka <jan.kiszka@siemens.com>,
Vignesh Raghavendra <vigneshr@ti.com>,
Lokesh Vutla <lokeshvutla@ti.com>,
<linux-remoteproc@vger.kernel.org>, <linux-omap@vger.kernel.org>,
<linux-arm-kernel@lists.infradead.org>,
<linux-kernel@vger.kernel.org>
Subject: [PATCH v2 3/3] remoteproc: pru: Fix and cleanup firmware interrupt mapping logic
Date: Wed, 7 Apr 2021 10:56:41 -0500 [thread overview]
Message-ID: <20210407155641.5501-4-s-anna@ti.com> (raw)
In-Reply-To: <20210407155641.5501-1-s-anna@ti.com>
The PRU firmware interrupt mappings are configured and unconfigured in
.start() and .stop() callbacks respectively using the variables 'evt_count'
and a 'mapped_irq' pointer. These variables are modified only during these
callbacks but are not re-initialized/reset properly during unwind or
failure paths. These stale values caused a kernel crash while stopping a
PRU remoteproc running a different firmware with no events on a subsequent
run after a previous run that was running a firmware with events.
Fix this crash by ensuring that the evt_count is 0 and the mapped_irq
pointer is set to NULL in pru_dispose_irq_mapping(). Also, reset these
variables properly during any failures in the .start() callback. While
at this, the pru_dispose_irq_mapping() callsites are all made to look
the same, moving any conditional logic to inside the function.
Fixes: c75c9fdac66e ("remoteproc: pru: Add support for PRU specific interrupt configuration")
Reported-by: Vignesh Raghavendra <vigneshr@ti.com>
Signed-off-by: Suman Anna <s-anna@ti.com>
---
v2:
- Fixed two additional cleanup paths in pru_handle_intrmap()
addressing Mathieu's review comment
v1: https://patchwork.kernel.org/project/linux-remoteproc/patch/20210323223839.17464-4-s-anna@ti.com/
drivers/remoteproc/pru_rproc.c | 22 +++++++++++++++++-----
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
index 87b43976c51b..04863bf23db8 100644
--- a/drivers/remoteproc/pru_rproc.c
+++ b/drivers/remoteproc/pru_rproc.c
@@ -266,12 +266,17 @@ static void pru_rproc_create_debug_entries(struct rproc *rproc)
static void pru_dispose_irq_mapping(struct pru_rproc *pru)
{
- while (pru->evt_count--) {
+ if (!pru->mapped_irq)
+ return;
+
+ while (pru->evt_count) {
+ pru->evt_count--;
if (pru->mapped_irq[pru->evt_count] > 0)
irq_dispose_mapping(pru->mapped_irq[pru->evt_count]);
}
kfree(pru->mapped_irq);
+ pru->mapped_irq = NULL;
}
/*
@@ -307,8 +312,10 @@ static int pru_handle_intrmap(struct rproc *rproc)
pru->evt_count = rsc->num_evts;
pru->mapped_irq = kcalloc(pru->evt_count, sizeof(unsigned int),
GFP_KERNEL);
- if (!pru->mapped_irq)
+ if (!pru->mapped_irq) {
+ pru->evt_count = 0;
return -ENOMEM;
+ }
/*
* parse and fill in system event to interrupt channel and
@@ -317,13 +324,19 @@ static int pru_handle_intrmap(struct rproc *rproc)
* corresponding sibling PRUSS INTC node.
*/
parent = of_get_parent(dev_of_node(pru->dev));
- if (!parent)
+ if (!parent) {
+ kfree(pru->mapped_irq);
+ pru->mapped_irq = NULL;
+ pru->evt_count = 0;
return -ENODEV;
+ }
irq_parent = of_get_child_by_name(parent, "interrupt-controller");
of_node_put(parent);
if (!irq_parent) {
kfree(pru->mapped_irq);
+ pru->mapped_irq = NULL;
+ pru->evt_count = 0;
return -ENODEV;
}
@@ -398,8 +411,7 @@ static int pru_rproc_stop(struct rproc *rproc)
pru_control_write_reg(pru, PRU_CTRL_CTRL, val);
/* dispose irq mapping - new firmware can provide new mapping */
- if (pru->mapped_irq)
- pru_dispose_irq_mapping(pru);
+ pru_dispose_irq_mapping(pru);
return 0;
}
--
2.30.1
WARNING: multiple messages have this Message-ID (diff)
From: Suman Anna <s-anna@ti.com>
To: Bjorn Andersson <bjorn.andersson@linaro.org>,
Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Grzegorz Jaszczyk <grzegorz.jaszczyk@linaro.org>,
Jan Kiszka <jan.kiszka@siemens.com>,
Vignesh Raghavendra <vigneshr@ti.com>,
Lokesh Vutla <lokeshvutla@ti.com>,
<linux-remoteproc@vger.kernel.org>, <linux-omap@vger.kernel.org>,
<linux-arm-kernel@lists.infradead.org>,
<linux-kernel@vger.kernel.org>
Subject: [PATCH v2 3/3] remoteproc: pru: Fix and cleanup firmware interrupt mapping logic
Date: Wed, 7 Apr 2021 10:56:41 -0500 [thread overview]
Message-ID: <20210407155641.5501-4-s-anna@ti.com> (raw)
In-Reply-To: <20210407155641.5501-1-s-anna@ti.com>
The PRU firmware interrupt mappings are configured and unconfigured in
.start() and .stop() callbacks respectively using the variables 'evt_count'
and a 'mapped_irq' pointer. These variables are modified only during these
callbacks but are not re-initialized/reset properly during unwind or
failure paths. These stale values caused a kernel crash while stopping a
PRU remoteproc running a different firmware with no events on a subsequent
run after a previous run that was running a firmware with events.
Fix this crash by ensuring that the evt_count is 0 and the mapped_irq
pointer is set to NULL in pru_dispose_irq_mapping(). Also, reset these
variables properly during any failures in the .start() callback. While
at this, the pru_dispose_irq_mapping() callsites are all made to look
the same, moving any conditional logic to inside the function.
Fixes: c75c9fdac66e ("remoteproc: pru: Add support for PRU specific interrupt configuration")
Reported-by: Vignesh Raghavendra <vigneshr@ti.com>
Signed-off-by: Suman Anna <s-anna@ti.com>
---
v2:
- Fixed two additional cleanup paths in pru_handle_intrmap()
addressing Mathieu's review comment
v1: https://patchwork.kernel.org/project/linux-remoteproc/patch/20210323223839.17464-4-s-anna@ti.com/
drivers/remoteproc/pru_rproc.c | 22 +++++++++++++++++-----
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
index 87b43976c51b..04863bf23db8 100644
--- a/drivers/remoteproc/pru_rproc.c
+++ b/drivers/remoteproc/pru_rproc.c
@@ -266,12 +266,17 @@ static void pru_rproc_create_debug_entries(struct rproc *rproc)
static void pru_dispose_irq_mapping(struct pru_rproc *pru)
{
- while (pru->evt_count--) {
+ if (!pru->mapped_irq)
+ return;
+
+ while (pru->evt_count) {
+ pru->evt_count--;
if (pru->mapped_irq[pru->evt_count] > 0)
irq_dispose_mapping(pru->mapped_irq[pru->evt_count]);
}
kfree(pru->mapped_irq);
+ pru->mapped_irq = NULL;
}
/*
@@ -307,8 +312,10 @@ static int pru_handle_intrmap(struct rproc *rproc)
pru->evt_count = rsc->num_evts;
pru->mapped_irq = kcalloc(pru->evt_count, sizeof(unsigned int),
GFP_KERNEL);
- if (!pru->mapped_irq)
+ if (!pru->mapped_irq) {
+ pru->evt_count = 0;
return -ENOMEM;
+ }
/*
* parse and fill in system event to interrupt channel and
@@ -317,13 +324,19 @@ static int pru_handle_intrmap(struct rproc *rproc)
* corresponding sibling PRUSS INTC node.
*/
parent = of_get_parent(dev_of_node(pru->dev));
- if (!parent)
+ if (!parent) {
+ kfree(pru->mapped_irq);
+ pru->mapped_irq = NULL;
+ pru->evt_count = 0;
return -ENODEV;
+ }
irq_parent = of_get_child_by_name(parent, "interrupt-controller");
of_node_put(parent);
if (!irq_parent) {
kfree(pru->mapped_irq);
+ pru->mapped_irq = NULL;
+ pru->evt_count = 0;
return -ENODEV;
}
@@ -398,8 +411,7 @@ static int pru_rproc_stop(struct rproc *rproc)
pru_control_write_reg(pru, PRU_CTRL_CTRL, val);
/* dispose irq mapping - new firmware can provide new mapping */
- if (pru->mapped_irq)
- pru_dispose_irq_mapping(pru);
+ pru_dispose_irq_mapping(pru);
return 0;
}
--
2.30.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-04-07 15:57 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-07 15:56 [PATCH v2 0/3] PRU firmware event/interrupt mapping fixes Suman Anna
2021-04-07 15:56 ` Suman Anna
2021-04-07 15:56 ` [PATCH v2 1/3] remoteproc: pru: Fixup interrupt-parent logic for fw events Suman Anna
2021-04-07 15:56 ` Suman Anna
2021-04-07 15:56 ` [PATCH v2 2/3] remoteproc: pru: Fix wrong success return value " Suman Anna
2021-04-07 15:56 ` Suman Anna
2021-04-07 15:56 ` Suman Anna [this message]
2021-04-07 15:56 ` [PATCH v2 3/3] remoteproc: pru: Fix and cleanup firmware interrupt mapping logic Suman Anna
2021-04-07 16:10 ` Mathieu Poirier
2021-04-07 16:10 ` Mathieu Poirier
2021-04-13 18:10 ` [PATCH v2 0/3] PRU firmware event/interrupt mapping fixes Suman Anna
2021-04-13 18:10 ` Suman Anna
2021-04-14 2:20 ` patchwork-bot+linux-remoteproc
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210407155641.5501-4-s-anna@ti.com \
--to=s-anna@ti.com \
--cc=bjorn.andersson@linaro.org \
--cc=grzegorz.jaszczyk@linaro.org \
--cc=jan.kiszka@siemens.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-omap@vger.kernel.org \
--cc=linux-remoteproc@vger.kernel.org \
--cc=lokeshvutla@ti.com \
--cc=mathieu.poirier@linaro.org \
--cc=vigneshr@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.