From: Arnaud POULIQUEN <arnaud.pouliquen@foss.st.com>
To: <joakim.zhang@cixtech.com>, <andersson@kernel.org>,
<mathieu.poirier@linaro.org>
Cc: <linux-remoteproc@vger.kernel.org>,
<linux-kernel@vger.kernel.org>, <cix-kernel-upstream@cixtech.com>
Subject: Re: [PATCH V2] remoteproc: virtio: Fix wdg cannot recovery remote processor
Date: Fri, 15 Dec 2023 17:55:38 +0100 [thread overview]
Message-ID: <70376b4f-0fbe-4087-8c7c-eb7167191a37@foss.st.com> (raw)
In-Reply-To: <20231215145023.2248366-1-joakim.zhang@cixtech.com>
Hello Joakim,
On 12/15/23 15:50, joakim.zhang@cixtech.com wrote:
> From: Joakim Zhang <joakim.zhang@cixtech.com>
>
> Recovery remote processor failed when wdg irq received:
> [ 0.842574] remoteproc remoteproc0: crash detected in cix-dsp-rproc: type watchdog
> [ 0.842750] remoteproc remoteproc0: handling crash #1 in cix-dsp-rproc
> [ 0.842824] remoteproc remoteproc0: recovering cix-dsp-rproc
> [ 0.843342] remoteproc remoteproc0: stopped remote processor cix-dsp-rproc
> [ 0.847901] rproc-virtio rproc-virtio.0.auto: Failed to associate buffer
> [ 0.847979] remoteproc remoteproc0: failed to probe subdevices for cix-dsp-rproc: -16
>
> The reason is that dma coherent mem would not be released when
> recovering the remote processor, due to rproc_virtio_remove()
> would not be called, where the mem released. It will fail when
> it try to allocate and associate buffer again.
>
> We can see that dma coherent mem allocated from rproc_add_virtio_dev(),
> so should release it from rproc_remove_virtio_dev(). These functions should
> appear symmetrically:
> -rproc_vdev_do_start()->rproc_add_virtio_dev()->dma_declare_coherent_memory()
> -rproc_vdev_do_stop()->rproc_remove_virtio_dev()->dma_release_coherent_memory()
>
> The same for of_reserved_mem_device_init_by_idx() and of_reserved_mem_device_release().
>
> Fixes: 1d7b61c06dc3 ("remoteproc: virtio: Create platform device for the remoteproc_virtio")
> Signed-off-by: Joakim Zhang <joakim.zhang@cixtech.com>
> ---
> ChangeLogs:
> V1->V2:
> * the same for of_reserved_mem_device_release()
> ---
> drivers/remoteproc/remoteproc_virtio.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
> index 83d76915a6ad..e877ee78740d 100644
> --- a/drivers/remoteproc/remoteproc_virtio.c
> +++ b/drivers/remoteproc/remoteproc_virtio.c
> @@ -465,8 +465,12 @@ static int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
> static int rproc_remove_virtio_dev(struct device *dev, void *data)
> {
> struct virtio_device *vdev = dev_to_virtio(dev);
> + struct rproc_vdev *rvdev = vdev_to_rvdev(vdev);
>
> unregister_virtio_device(vdev);
> + of_reserved_mem_device_release(&rvdev->pdev->dev);
> + dma_release_coherent_memory(&rvdev->pdev->dev);
> +
At this step, the virtio device may not be released and may still be using the
memory.
Do you try to move this in rproc_virtio_dev_release?
Regards,
Arnaud
> return 0;
> }
>
> @@ -584,9 +588,6 @@ static void rproc_virtio_remove(struct platform_device *pdev)
> rproc_remove_subdev(rproc, &rvdev->subdev);
> rproc_remove_rvdev(rvdev);
>
> - of_reserved_mem_device_release(&pdev->dev);
> - dma_release_coherent_memory(&pdev->dev);
> -
> put_device(&rproc->dev);
> }
>
> --
> 2.25.1
>
>
>
> This email (including its attachments) is intended only for the person or entity to which it is addressed and may contain information that is privileged, confidential or otherwise protected from disclosure. Unauthorized use, dissemination, distribution or copying of this email or the information herein or taking any action in reliance on the contents of this email or the information herein, by anyone other than the intended recipient, or an employee or agent responsible for delivering the message to the intended recipient, is strictly prohibited. If you are not the intended recipient, please do not read, copy, use or disclose any part of this e-mail to others. Please notify the sender immediately and permanently delete this e-mail and any attachments if you received it in error. Internet communications cannot be guaranteed to be timely, secure, error-free or virus-free. The sender does not accept liability for any errors or omissions.
next prev parent reply other threads:[~2023-12-15 16:55 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-15 14:50 [PATCH V2] remoteproc: virtio: Fix wdg cannot recovery remote processor joakim.zhang
2023-12-15 16:55 ` Arnaud POULIQUEN [this message]
2023-12-17 5:26 ` Joakim Zhang
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=70376b4f-0fbe-4087-8c7c-eb7167191a37@foss.st.com \
--to=arnaud.pouliquen@foss.st.com \
--cc=andersson@kernel.org \
--cc=cix-kernel-upstream@cixtech.com \
--cc=joakim.zhang@cixtech.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-remoteproc@vger.kernel.org \
--cc=mathieu.poirier@linaro.org \
/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 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 read-only IMAP folder(s) and NNTP newsgroup(s).