From: "Joakim Zhang" <joakim.zhang@cixtech.com>
To: Arnaud POULIQUEN <arnaud.pouliquen@foss.st.com>,
"andersson@kernel.org" <andersson@kernel.org>,
"mathieu.poirier@linaro.org" <mathieu.poirier@linaro.org>
Cc: "linux-remoteproc@vger.kernel.org"
<linux-remoteproc@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
cix-kernel-upstream <cix-kernel-upstream@cixtech.com>
Subject: RE: [PATCH V2] remoteproc: virtio: Fix wdg cannot recovery remote processor
Date: Sun, 17 Dec 2023 05:26:09 +0000 [thread overview]
Message-ID: <SEYPR06MB627880579FD241453E287F378291A@SEYPR06MB6278.apcprd06.prod.outlook.com> (raw)
In-Reply-To: <70376b4f-0fbe-4087-8c7c-eb7167191a37@foss.st.com>
Hello Arnaud,
> -----Original Message-----
> From: Arnaud POULIQUEN <arnaud.pouliquen@foss.st.com>
> Sent: Saturday, December 16, 2023 12:56 AM
> To: Joakim Zhang <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 <cix-kernel-upstream@cixtech.com>
> Subject: Re: [PATCH V2] remoteproc: virtio: Fix wdg cannot recovery remote
> processor
>
> 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_m
> > emory()
> > -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?
Oh, yes, thanks for the hint, I tested, and it can fix the issue, I will send v3 soon.
Joakim
> 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
prev parent reply other threads:[~2023-12-17 5:26 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
2023-12-17 5:26 ` Joakim Zhang [this message]
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=SEYPR06MB627880579FD241453E287F378291A@SEYPR06MB6278.apcprd06.prod.outlook.com \
--to=joakim.zhang@cixtech.com \
--cc=andersson@kernel.org \
--cc=arnaud.pouliquen@foss.st.com \
--cc=cix-kernel-upstream@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).