linux-kernel - Re: [PATCH v4 00/17] remoteproc: Add support for detaching a rproc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <64b559dc-9e89-c351-ddee-f9cebd155ed7@st.com>
Date:   Wed, 27 Jan 2021 10:21:24 +0100
From:   Arnaud POULIQUEN <arnaud.pouliquen@...com>
To:     Mathieu Poirier <mathieu.poirier@...aro.org>,
        "ohad@...ery.com" <ohad@...ery.com>,
        "bjorn.andersson@...aro.org" <bjorn.andersson@...aro.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>
CC:     "linux-remoteproc@...r.kernel.org" <linux-remoteproc@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 00/17] remoteproc: Add support for detaching a rproc

Hi Mathieu

On 12/18/20 6:32 PM, Mathieu Poirier wrote:
> Following the work done here [1], this set provides support for the
> remoteproc core to release resources associated with a remote processor
> without having to switch it off. That way a platform driver can be removed
> or the application processor power cycled while the remote processor is
> still operating.
> 
> Of special interest in this series are patches 5 and 6 where getting the
> address of the resource table installed by an eternal entity if moved to
> the core.  This is to support scenarios where a remote process has been
> booted by the core but is being detached.  To re-attach the remote
> processor, the address of the resource table needs to be known at a later
> time than the platform driver's probe() function.
> 
> Applies cleanly on v5.10
> 
> Thanks,
> Mathieu
> 
> [1]. https://lkml.org/lkml/2020/7/14/1600
> 
> ----
> New for v4:
> - Made binding description OS agnostic (Rob)
> - Added functionality to set the external resource table in the core
> - Fixed a crash when detaching (Arnaud)
> - Fixed error code propagation in rproc_cdev_relase() and rproc_del() (Arnaud)
> - Added RB tags


I tested you series, attach and  detach is working well.

Then I faced issue when tried to re-attach after a detach.

But I don't know if this feature has to be supported in this step.

The 2 issues I found are:

1) memory carveouts are released on detach so need to be reinitialized.
The use of prepare/unprepare for the attach and detach would solve the issue but
probably need to add parameter to differentiate a start/stop from a attach/detach.

2) The vrings in the loaded resource table (so no cached) has to be properly
reinitialized. In rproc_free_vring  the vring da is set to 0 that is then
considered as a fixed address.

Here is a fix which works on the stm32 platform

@@ -425,7 +425,7 @@ void rproc_free_vring(struct rproc_vring *rvring)
 	 */
 	if (rproc->table_ptr) {
 		rsc = (void *)rproc->table_ptr + rvring->rvdev->rsc_offset;
-		rsc->vring[idx].da = 0;
+		rsc->vring[idx].da = FW_RSC_ADDR_ANY;
 		rsc->vring[idx].notifyid = -1;
 	}
 }

Here, perhaps a better alternative would be to make a cached copy on attach
before updating it. On the next attach, the cached copy would be copied as it is
done in rproc_start.

Thanks,
Arnaud


> 
> Mathieu Poirier (17):
>   dt-bindings: remoteproc: Add bindind to support autonomous processors
>   remoteproc: Re-check state in rproc_shutdown()
>   remoteproc: Remove useless check in rproc_del()
>   remoteproc: Rename function rproc_actuate()
>   remoteproc: Add new get_loaded_rsc_table() remoteproc operation
>   remoteproc: stm32: Move resource table setup to rproc_ops
>   remoteproc: Add new RPROC_ATTACHED state
>   remoteproc: Properly represent the attached state
>   remoteproc: Properly deal with a kernel panic when attached
>   remoteproc: Add new detach() remoteproc operation
>   remoteproc: Introduce function __rproc_detach()
>   remoteproc: Introduce function rproc_detach()
>   remoteproc: Add return value to function rproc_shutdown()
>   remoteproc: Properly deal with a stop request when attached
>   remoteproc: Properly deal with a start request when attached
>   remoteproc: Properly deal with detach request
>   remoteproc: Refactor rproc delete and cdev release path
> 
>  .../bindings/remoteproc/remoteproc-core.yaml  |  27 +++
>  drivers/remoteproc/remoteproc_cdev.c          |  32 ++-
>  drivers/remoteproc/remoteproc_core.c          | 211 +++++++++++++++---
>  drivers/remoteproc/remoteproc_internal.h      |   8 +
>  drivers/remoteproc/remoteproc_sysfs.c         |  20 +-
>  drivers/remoteproc/stm32_rproc.c              | 147 ++++++------
>  include/linux/remoteproc.h                    |  24 +-
>  7 files changed, 344 insertions(+), 125 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/remoteproc/remoteproc-core.yaml
>