Re: race condition issue at remote proc startup

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello, thanks for the quick answer,

> Is the remote side waiting for the vdev status[1] update before accessing the vrings?
> [1] https://elixir.bootlin.com/linux/latest/source/include/linux/remoteproc.h#L307

We are using the openamp project that does it for us: https://github.com/OpenAMP/open-amp/blob/master/lib/remoteproc/remoteproc.c#L925

I also checked the defines and they seem aligned on both sides:
linux: include/uapi/linux/virtio_config.h:#define VIRTIO_CONFIG_S_DRIVER_OK    4
openamp: lib/include/openamp/virtio.h:#define VIRTIO_CONFIG_STATUS_DRIVER_OK 0x04

Thanks !

----- Original Message -----
From: "Arnaud POULIQUEN" <arnaud.pouliquen@xxxxxxxxxxx>
To: "Yann Sionneau" <ysionneau@xxxxxxxxx>, linux-remoteproc@xxxxxxxxxxxxxxx
Cc: "Pierre-Yves Kerbrat" <pkerbrat@xxxxxxxxxxxxx>, "Vincent Chardon" <vchardon@xxxxxxxxxxxxx>, "jhascoet" <jhascoet@xxxxxxxxxxxxx>
Sent: Tuesday, May 4, 2021 6:43:47 PM
Subject: Re: race condition issue at remote proc startup

Hello Yann

On 5/4/21 11:45 AM, Yann Sionneau wrote:
> Hello,
> 
> We (at Kalray) have some difficulties during initialization of a remoteproc
> device, and there seem to have no clean way (at least not one we know of) out of
> this problem.
> 
> We need vring defined in the resource table to be completely initialized before
> the remoteproc device is started. By completely initialized I mean that the
> vring device address defined in resource table shall be changed from 0xff..ff to
> a proper address. Currently the remote device is started before the
> initialization has completed, which creates a race condition between Linux and
> the remoteproc device. (We have a particular architecture in which the processor
> running Linux is the same as the embedded processor, this is why this problem
> happens in our case but probably not when the processor running Linux is much
> faster than the embedded processor).

Is the remote side waiting for the vdev status[1] update before accessing the
vrings?

[1] https://elixir.bootlin.com/linux/latest/source/include/linux/remoteproc.h#L307


> 
> Our best attempt up to now is to configure the virtio ring sooner i.e during
> subdevice preparation instead of subdevice start.
> i.e. in rproc_handle_vdev change code from
>     rvdev->subdev.start = rproc_vdev_do_start;
> to
>     /* da field in vring must be initialized before powering up
>      * the remoterproc, or else race condition may occur.
>      * Indeed the remoteproc may read it before it has been initialized.
>      */
>     rvdev->subdev.prepare = rproc_vdev_do_start;
> 
> This works but it has undesired side effects. In particular some notifications
> are sent (the remote proc kick function is being called), but since the remote
> CPU has not been started yet we are not able to handle them, thus we simply
> ignore them if the state of the remote proc is not RUNNING.
> At least this seems to solve our problem, but this is a particularly unpleasant
> way of solving the problem, in particular it might impact the existing
> remoteproc devices. Do you have any suggestion on some cleaner to way to solve
> this problem?
> 
> FYI, here is our arch specific remote proc implementation:
> https://github.com/kalray/linux_coolidge/blob/coolidge/drivers/remoteproc/kvx_remoteproc.c
> 
> 
> PS: there seem to be a similar problem when the remote device is being stopped.
> The vring buffer are destroyed and only after is the remote proc device stopped.
> There is once again a race condition as the remote proc device might try to
> access the vring after their destruction by the host. Proposed change is as follow:
> In rproc_handle_vdev change code from
>     rvdev->subdev.stop = rproc_vdev_do_stop;
> to
>     rvdev->subdev.unprepare = rproc_vdev_do_stop;

Should also be handled with the vdev status.

> 
> Note this change has much less impact on existing remote proc and is symmetric
> to the previous change thus it might make it sound more logical
> 
> PS2: I guess that this issue never showed up before because most other use cases
> are using fixed addresses in the resource tables and not dynamically allocated
> ones at runtime.

We use dynamic vring address allocation without any issue on STM323MP1 platform,
with the coprocessor started before the main processor running Linux.

Regards,
Arnaud

> 
> Regards,
>





[Index of Archives]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Photo Sharing]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux