| Message ID | 20240830095147.3538047-5-arnaud.pouliquen@foss.st.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Introduction of a remoteproc tee to load signed firmware | expand |
Hi Arnaud,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 5be63fc19fcaa4c236b307420483578a56986a37]
url: https://github.com/intel-lab-lkp/linux/commits/Arnaud-Pouliquen/remoteproc-core-Introduce-rproc_pa_to_va-helper/20240830-175609
base: 5be63fc19fcaa4c236b307420483578a56986a37
patch link: https://lore.kernel.org/r/20240830095147.3538047-5-arnaud.pouliquen%40foss.st.com
patch subject: [PATCH v9 4/7] remoteproc: core: Add TEE interface support for firmware release
config: loongarch-allmodconfig (https://download.01.org/0day-ci/archive/20240901/202409010034.Tln3soEY-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240901/202409010034.Tln3soEY-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409010034.Tln3soEY-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/remoteproc/remoteproc_core.c:32:
>> include/linux/remoteproc_tee.h:52:12: warning: 'tee_rproc_parse_fw' defined but not used [-Wunused-function]
52 | static int tee_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw)
| ^~~~~~~~~~~~~~~~~~
vim +/tee_rproc_parse_fw +52 include/linux/remoteproc_tee.h
498143a453d14e Arnaud Pouliquen 2024-08-30 51
498143a453d14e Arnaud Pouliquen 2024-08-30 @52 static int tee_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw)
498143a453d14e Arnaud Pouliquen 2024-08-30 53 {
498143a453d14e Arnaud Pouliquen 2024-08-30 54 /* This shouldn't be possible */
498143a453d14e Arnaud Pouliquen 2024-08-30 55 WARN_ON(1);
498143a453d14e Arnaud Pouliquen 2024-08-30 56
498143a453d14e Arnaud Pouliquen 2024-08-30 57 return 0;
498143a453d14e Arnaud Pouliquen 2024-08-30 58 }
498143a453d14e Arnaud Pouliquen 2024-08-30 59
On 8/31/24 18:43, kernel test robot wrote: > Hi Arnaud, > > kernel test robot noticed the following build warnings: > > [auto build test WARNING on 5be63fc19fcaa4c236b307420483578a56986a37] > > url: https://github.com/intel-lab-lkp/linux/commits/Arnaud-Pouliquen/remoteproc-core-Introduce-rproc_pa_to_va-helper/20240830-175609 > base: 5be63fc19fcaa4c236b307420483578a56986a37 > patch link: https://lore.kernel.org/r/20240830095147.3538047-5-arnaud.pouliquen%40foss.st.com > patch subject: [PATCH v9 4/7] remoteproc: core: Add TEE interface support for firmware release > config: loongarch-allmodconfig (https://download.01.org/0day-ci/archive/20240901/202409010034.Tln3soEY-lkp@intel.com/config) > compiler: loongarch64-linux-gcc (GCC) 14.1.0 > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240901/202409010034.Tln3soEY-lkp@intel.com/reproduce) > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot <lkp@intel.com> > | Closes: https://lore.kernel.org/oe-kbuild-all/202409010034.Tln3soEY-lkp@intel.com/ > > All warnings (new ones prefixed by >>): > > In file included from drivers/remoteproc/remoteproc_core.c:32: >>> include/linux/remoteproc_tee.h:52:12: warning: 'tee_rproc_parse_fw' defined but not used [-Wunused-function] > 52 | static int tee_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw) > | ^~~~~~~~~~~~~~~~~~ > > > vim +/tee_rproc_parse_fw +52 include/linux/remoteproc_tee.h > > 498143a453d14e Arnaud Pouliquen 2024-08-30 51 > 498143a453d14e Arnaud Pouliquen 2024-08-30 @52 static int tee_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw) > 498143a453d14e Arnaud Pouliquen 2024-08-30 53 { > 498143a453d14e Arnaud Pouliquen 2024-08-30 54 /* This shouldn't be possible */ > 498143a453d14e Arnaud Pouliquen 2024-08-30 55 WARN_ON(1); > 498143a453d14e Arnaud Pouliquen 2024-08-30 56 > 498143a453d14e Arnaud Pouliquen 2024-08-30 57 return 0; > 498143a453d14e Arnaud Pouliquen 2024-08-30 58 } > 498143a453d14e Arnaud Pouliquen 2024-08-30 59 > The inline attribute is missing. As it is a minor fix, I'm waiting for more reviews before fixing it in the next version. Regards, Arnaud
On Fri, Aug 30, 2024 at 11:51:44AM +0200, Arnaud Pouliquen wrote: > Add support for releasing remote processor firmware through > the Trusted Execution Environment (TEE) interface. > > The tee_rproc_release_fw() function is called in the following cases: > > - An error occurs in rproc_start() between the loading of the segments and > the start of the remote processor. > - When rproc_release_fw is called on error or after stopping the remote > processor. > > Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com> > --- > drivers/remoteproc/remoteproc_core.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c > index 7694817f25d4..32052dedc149 100644 > --- a/drivers/remoteproc/remoteproc_core.c > +++ b/drivers/remoteproc/remoteproc_core.c > @@ -29,6 +29,7 @@ > #include <linux/debugfs.h> > #include <linux/rculist.h> > #include <linux/remoteproc.h> > +#include <linux/remoteproc_tee.h> > #include <linux/iommu.h> > #include <linux/idr.h> > #include <linux/elf.h> > @@ -1258,6 +1259,9 @@ static int rproc_alloc_registered_carveouts(struct rproc *rproc) > > static void rproc_release_fw(struct rproc *rproc) > { > + if (rproc->state == RPROC_OFFLINE && rproc->tee_interface) > + tee_rproc_release_fw(rproc); > + If I understand correctly, the first condition is there because the attach/detach scenario does not yet support management by the TEE. I would simply move the check to tee_rproc_release_fw() with a comment to that effect. > /* Free the copy of the resource table */ > kfree(rproc->cached_table); > rproc->cached_table = NULL; > @@ -1348,7 +1352,7 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw) > if (ret) { > dev_err(dev, "failed to prepare subdevices for %s: %d\n", > rproc->name, ret); > - goto reset_table_ptr; > + goto release_fw; > } > > /* power up the remote processor */ > @@ -1376,7 +1380,9 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw) > rproc->ops->stop(rproc); > unprepare_subdevices: > rproc_unprepare_subdevices(rproc); > -reset_table_ptr: > +release_fw: I would have kept the old label. > + if (rproc->tee_interface) > + tee_rproc_release_fw(rproc); > rproc->table_ptr = rproc->cached_table; > > return ret; > -- > 2.25.1 >
On Fri, Aug 30, 2024 at 11:51:44AM +0200, Arnaud Pouliquen wrote: > Add support for releasing remote processor firmware through > the Trusted Execution Environment (TEE) interface. > > The tee_rproc_release_fw() function is called in the following cases: > > - An error occurs in rproc_start() between the loading of the segments and > the start of the remote processor. > - When rproc_release_fw is called on error or after stopping the remote > processor. > > Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com> > --- > drivers/remoteproc/remoteproc_core.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c > index 7694817f25d4..32052dedc149 100644 > --- a/drivers/remoteproc/remoteproc_core.c > +++ b/drivers/remoteproc/remoteproc_core.c > @@ -29,6 +29,7 @@ > #include <linux/debugfs.h> > #include <linux/rculist.h> > #include <linux/remoteproc.h> > +#include <linux/remoteproc_tee.h> > #include <linux/iommu.h> > #include <linux/idr.h> > #include <linux/elf.h> > @@ -1258,6 +1259,9 @@ static int rproc_alloc_registered_carveouts(struct rproc *rproc) > > static void rproc_release_fw(struct rproc *rproc) > { > + if (rproc->state == RPROC_OFFLINE && rproc->tee_interface) > + tee_rproc_release_fw(rproc); Function tee_rproc_release_fw() returns a value that is ignored. I don't know how it passes the Sparse checker but I already see patches coming in my Inbox to deal with that. In this case there is nothing else to do if there is an error releasing the firware. As such I would put a (void) in front and add a comment about the return value being ignore on purpose. > + > /* Free the copy of the resource table */ > kfree(rproc->cached_table); > rproc->cached_table = NULL; > @@ -1348,7 +1352,7 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw) > if (ret) { > dev_err(dev, "failed to prepare subdevices for %s: %d\n", > rproc->name, ret); > - goto reset_table_ptr; > + goto release_fw; > } > > /* power up the remote processor */ > @@ -1376,7 +1380,9 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw) > rproc->ops->stop(rproc); > unprepare_subdevices: > rproc_unprepare_subdevices(rproc); > -reset_table_ptr: > +release_fw: > + if (rproc->tee_interface) > + tee_rproc_release_fw(rproc); Same here. > rproc->table_ptr = rproc->cached_table; > > return ret; > -- > 2.25.1 >
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index 7694817f25d4..32052dedc149 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -29,6 +29,7 @@ #include <linux/debugfs.h> #include <linux/rculist.h> #include <linux/remoteproc.h> +#include <linux/remoteproc_tee.h> #include <linux/iommu.h> #include <linux/idr.h> #include <linux/elf.h> @@ -1258,6 +1259,9 @@ static int rproc_alloc_registered_carveouts(struct rproc *rproc) static void rproc_release_fw(struct rproc *rproc) { + if (rproc->state == RPROC_OFFLINE && rproc->tee_interface) + tee_rproc_release_fw(rproc); + /* Free the copy of the resource table */ kfree(rproc->cached_table); rproc->cached_table = NULL; @@ -1348,7 +1352,7 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw) if (ret) { dev_err(dev, "failed to prepare subdevices for %s: %d\n", rproc->name, ret); - goto reset_table_ptr; + goto release_fw; } /* power up the remote processor */ @@ -1376,7 +1380,9 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw) rproc->ops->stop(rproc); unprepare_subdevices: rproc_unprepare_subdevices(rproc); -reset_table_ptr: +release_fw: + if (rproc->tee_interface) + tee_rproc_release_fw(rproc); rproc->table_ptr = rproc->cached_table; return ret;
Add support for releasing remote processor firmware through the Trusted Execution Environment (TEE) interface. The tee_rproc_release_fw() function is called in the following cases: - An error occurs in rproc_start() between the loading of the segments and the start of the remote processor. - When rproc_release_fw is called on error or after stopping the remote processor. Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com> --- drivers/remoteproc/remoteproc_core.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)