Use the macro IS_REACHABLE instead of IS_ENABLED in <linux/rpmb.h> when
deciding if prototypes or stubbed static inline functions should be
provided. This fixes link errors when the calling code is builtin while
the RPMB subsystem is a module.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202409021448.RSvcBPzt-lkp@intel.com/
Fixes: 1e9046e3a154 ("rpmb: add Replay Protected Memory Block (RPMB) subsystem")
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
---
 include/linux/rpmb.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/rpmb.h b/include/linux/rpmb.h
index cccda73eea4d..37b5273c4027 100644
--- a/include/linux/rpmb.h
+++ b/include/linux/rpmb.h
@@ -61,7 +61,7 @@ struct rpmb_dev {
 
 #define to_rpmb_dev(x)		container_of((x), struct rpmb_dev, dev)
 
-#if IS_ENABLED(CONFIG_RPMB)
+#if IS_REACHABLE(CONFIG_RPMB)
 struct rpmb_dev *rpmb_dev_get(struct rpmb_dev *rdev);
 void rpmb_dev_put(struct rpmb_dev *rdev);
 struct rpmb_dev *rpmb_dev_find_device(const void *data,
-- 
2.34.1

On Mon, Sep 2, 2024, at 08:07, Jens Wiklander wrote:
> Use the macro IS_REACHABLE instead of IS_ENABLED in <linux/rpmb.h> when
> deciding if prototypes or stubbed static inline functions should be
> provided. This fixes link errors when the calling code is builtin while
> the RPMB subsystem is a module.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: 
> https://lore.kernel.org/oe-kbuild-all/202409021448.RSvcBPzt-lkp@intel.com/
> Fixes: 1e9046e3a154 ("rpmb: add Replay Protected Memory Block (RPMB) 
> subsystem")
> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>

Please don't work around a bug like this, fix it properly instead.

> diff --git a/include/linux/rpmb.h b/include/linux/rpmb.h
> index cccda73eea4d..37b5273c4027 100644
> --- a/include/linux/rpmb.h
> +++ b/include/linux/rpmb.h
> @@ -61,7 +61,7 @@ struct rpmb_dev {
> 
>  #define to_rpmb_dev(x)		container_of((x), struct rpmb_dev, dev)
> 
> -#if IS_ENABLED(CONFIG_RPMB)
> +#if IS_REACHABLE(CONFIG_RPMB)
>  struct rpmb_dev *rpmb_dev_get(struct rpmb_dev *rdev);
>  void rpmb_dev_put(struct rpmb_dev *rdev);

This gives very unexpected runtime behavior where both RPMB and
its user are enabled, but it doesn't work.

I think what you want here is a dependency like

      depends on RPMB || !RPMB

for every caller. This enforces at build time that the MMC core can
be built either when RPMB is disabled, of when it is reachable.

    Arnd

Hi Arnd,

On Mon, Sep 2, 2024 at 10:31 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Mon, Sep 2, 2024, at 08:07, Jens Wiklander wrote:
> > Use the macro IS_REACHABLE instead of IS_ENABLED in <linux/rpmb.h> when
> > deciding if prototypes or stubbed static inline functions should be
> > provided. This fixes link errors when the calling code is builtin while
> > the RPMB subsystem is a module.
> >
> > Reported-by: kernel test robot <lkp@intel.com>
> > Closes:
> > https://lore.kernel.org/oe-kbuild-all/202409021448.RSvcBPzt-lkp@intel.com/
> > Fixes: 1e9046e3a154 ("rpmb: add Replay Protected Memory Block (RPMB)
> > subsystem")
> > Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
>
> Please don't work around a bug like this, fix it properly instead.
>
> > diff --git a/include/linux/rpmb.h b/include/linux/rpmb.h
> > index cccda73eea4d..37b5273c4027 100644
> > --- a/include/linux/rpmb.h
> > +++ b/include/linux/rpmb.h
> > @@ -61,7 +61,7 @@ struct rpmb_dev {
> >
> >  #define to_rpmb_dev(x)               container_of((x), struct rpmb_dev, dev)
> >
> > -#if IS_ENABLED(CONFIG_RPMB)
> > +#if IS_REACHABLE(CONFIG_RPMB)
> >  struct rpmb_dev *rpmb_dev_get(struct rpmb_dev *rdev);
> >  void rpmb_dev_put(struct rpmb_dev *rdev);
>
> This gives very unexpected runtime behavior where both RPMB and
> its user are enabled, but it doesn't work.
>
> I think what you want here is a dependency like
>
>       depends on RPMB || !RPMB
>
> for every caller. This enforces at build time that the MMC core can
> be built either when RPMB is disabled, of when it is reachable.

Thanks for the suggestion.

I tried adding the dependency above for MMC_BLOCK and OPTEE.

Setting CONFIG_RPMB to "y" works as expected CONFIG_MMC_BLOCK and
CONFIG_OPTEE can be configured as both "y" or "m".

Setting CONFIG_RPMB to "m" will make CONFIG_MMC_BLOCK and CONFIG_OPTEE
"m" too, even if they are set as "y" in the defconfig. So the module
status seems to spread to the options with the dependency. For
tristate options, it guarantees that the expected features are
available even if it changes from built-in to a loadable module.

It will do nothing for bool options, but that's a hypothetical case
for now.  Wouldn't spreading the built-in status to CONFIG_RPMB be
better? So CONFIG_RPMB turns into "y" if an option configured as
built-in depends on it. I don't know how to do that though, so that
could be saved for when it's needed.

I'll send out patches for MMC_BLOCK and OPTEE options shortly if we're
happy with the "depends on RPMB || !RPMB" approach.

Cheers,
Jens