Hi Bjorn
On 3/11/20 9:01 PM, Bjorn Andersson wrote:
On Tue 10 Mar 06:19 PDT 2020, Arnaud POULIQUEN wrote:
-----Original Message----- From: linux-remoteproc-owner@vger.kernel.org <linux-remoteproc- owner@vger.kernel.org> On Behalf Of Nikita Shubin Sent: mardi 10 mars 2020 13:09 To: Arnaud POULIQUEN arnaud.pouliquen@st.com Cc: stable@vger.kernel.org; Ohad Ben-Cohen ohad@wizery.com; Bjorn Andersson bjorn.andersson@linaro.org; linux- remoteproc@vger.kernel.org; linux-kernel@vger.kernel.org; Mathieu Poirier mathieu.poirier@linaro.org Subject: Re: [PATCH v5] remoteproc: Fix NULL pointer dereference in rproc_virtio_notify
On Mon, Mar 09, 2020 at 03:22:24PM +0100, Arnaud POULIQUEN wrote:
Hi,
sorry for the late answer...
On 3/6/20 8:24 AM, Nikita Shubin wrote:
Undefined rproc_ops .kick method in remoteproc driver will result in "Unable to handle kernel NULL pointer dereference" in rproc_virtio_notify, after firmware loading if:
- .kick method wasn't defined in driver
- resource_table exists in firmware and has "Virtio device entry"
defined
Let's refuse to register an rproc-induced virtio device if no kick method was defined for rproc.
[...]
Signed-off-by: Nikita Shubin NShubin@topcon.com Fixes: 7a186941626d ("remoteproc: remove the single rpmsg vdev limitation") Cc: stable@vger.kernel.org
drivers/remoteproc/remoteproc_virtio.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c index 8c07cb2ca8ba..31a62a0b470e 100644 --- a/drivers/remoteproc/remoteproc_virtio.c +++ b/drivers/remoteproc/remoteproc_virtio.c @@ -334,6 +334,13 @@ int rproc_add_virtio_dev(struct rproc_vdev
*rvdev, int id)
struct rproc_mem_entry *mem; int ret;
- if (rproc->ops->kick == NULL) {
ret = -EINVAL;dev_err(dev, ".kick method not defined for %s",rproc->name);goto out;- }
Should the kick ops be mandatory for all the platforms? How about making
it optional instead?
Hi, Arnaud.
It is not mandatory, currently it must be provided if specified vdev entry is in resourse table. Otherwise it looks like there is no point in creating vdev.
Yes, my question was about having it optional for vdev also. A platform could implement the vdev without kick mechanism but by polling depending due to hardware capability... This could be an alternative avoiding to implement a dummy function in platform driver.
Is this a real thing or a theoretical suggestion?
Only a theoretical suggestion, trigged by the IMX platform patchset which implement a "temporary" dummy kick. and based on OpenAMP lib implementation which does not request a doorbell. Anyway no issue to keep it mandatory here.
Regards, Arnaud
Regards, Bjorn
Anyway it just a proposal that makes sense from MPOV. If Bjorn is ok with your patch, nothing blocking on my side.
Regards Arnaud
Regards, Arnaud
/* Try to find dedicated vdev buffer carveout */ mem = rproc_find_carveout_by_name(rproc, "vdev%dbuffer",
rvdev->index);
if (mem) {