All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nathan Chancellor via OP-TEE <op-tee@lists.trustedfirmware.org>
To: Julian Braha <julianbraha@gmail.com>
Cc: amirreza.zarrabi@oss.qualcomm.com, jenswi@kernel.org,
	sumit.garg@kernel.org, arnd@arndb.de, geert+renesas@glider.be,
	amirreza.zarrabi@oss.qualcomm.org,
	op-tee@lists.trustedfirmware.org, linux-kernel@vger.kernel.org,
	quic_eberman@quicinc.com, andersson@kernel.org, brgl@kernel.org,
	harshal.dev@oss.qualcomm.com, nsc@kernel.org,
	jani.nikula@linux.intel.com, kees@kernel.org
Subject: Re: [PATCH v3] tee: remove TZMEM_MODE_GENERIC
Date: Fri, 7 Aug 2026 12:05:13 -0700	[thread overview]
Message-ID: <20260807190513.GA2638974@ax162> (raw)
In-Reply-To: <20260807175041.3299349-1-julianbraha@gmail.com>

On Fri, Aug 07, 2026 at 06:50:41PM +0100, Julian Braha wrote:
> 'select' does not work on config options in a 'choice', so currently it is
> possible to enable QCOMTEE without QCOM_TZMEM_MODE_SHMBRIDGE, even though
> this is needed at runtime.
> 
> There are no users of the generic allocator option,
> QCOM_TZMEM_MODE_GENERIC, so let's remove it. Then, we can remove the
> containing choice..endchoice, which allows the 'select' to work as
> intended.
> 
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Julian Braha <julianbraha@gmail.com>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

One small nit below but I would only send v4 if there are other things
to be addressed.

A note to the maintainers: We would like to turn selecting a choice
symbol into a hard error in 7.4, so please consider picking this up for
7.3-rc1.

> ---
> Changes since v2:
> - add back stubs for when CONFIG_QCOM_TZMEM_MODE_GENERIC=n
> - updated help text accordingly
> 
> Link:
> https://lore.kernel.org/all/20260729203845.387239-1-julianbraha@gmail.com/
> 
> Changes since v1:
> - remove TZMEM_MODE_GENERIC instead of removing the dead select
> 
> Link:
> https://lore.kernel.org/all/20260715092539.18384-1-julianbraha@gmail.com/
> ---
>  drivers/firmware/qcom/Kconfig      | 26 +++++---------------------
>  drivers/firmware/qcom/qcom_tzmem.c |  4 ++--
>  2 files changed, 7 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/firmware/qcom/Kconfig b/drivers/firmware/qcom/Kconfig
> index c7f8413ab996..95b968d88dc3 100644
> --- a/drivers/firmware/qcom/Kconfig
> +++ b/drivers/firmware/qcom/Kconfig
> @@ -34,33 +34,17 @@ config QCOM_TZMEM
>  	tristate
>  	select GENERIC_ALLOCATOR
>  
> -choice
> -	prompt "TrustZone interface memory allocator mode"
> -	depends on QCOM_TZMEM
> -	default QCOM_TZMEM_MODE_GENERIC
> -	help
> -	  Selects the mode of the memory allocator providing memory buffers of
> -	  suitable format for sharing with the TrustZone. If in doubt, select
> -	  'Generic'.
> -
> -config QCOM_TZMEM_MODE_GENERIC
> -	bool "Generic"
> -	help
> -	  Use the generic allocator mode. The memory is page-aligned, non-cachable
> -	  and physically contiguous.
> -
>  config QCOM_TZMEM_MODE_SHMBRIDGE
> -	bool "SHM Bridge"
> +	bool "TrustZone interface memory allocator: SHM Bridge"
> +	depends on QCOM_TZMEM
>  	help
> -	  Use Qualcomm Shared Memory Bridge. The memory has the same alignment as
> -	  in the 'Generic' allocator but is also explicitly marked as an SHM Bridge
> -	  buffer.
> +	  Use Qualcomm Shared Memory Bridge as memory allocator. The memory has the
> +	  same alignment as in the 'Generic' allocator, which is used when this option
> +	  is disabled, but is also explicitly marked as an SHM Bridge buffer.
>  
>  	  With this selected, all buffers passed to the TrustZone must be allocated
>  	  using the TZMem allocator or else the TrustZone will refuse to use them.
>  
> -endchoice
> -
>  config QCOM_QSEECOM
>  	bool "Qualcomm QSEECOM interface driver"
>  	depends on QCOM_SCM=y
> diff --git a/drivers/firmware/qcom/qcom_tzmem.c b/drivers/firmware/qcom/qcom_tzmem.c
> index 0fd9581275f1..510474902c3a 100644
> --- a/drivers/firmware/qcom/qcom_tzmem.c
> +++ b/drivers/firmware/qcom/qcom_tzmem.c
> @@ -50,7 +50,7 @@ static struct device *qcom_tzmem_dev;
>  static RADIX_TREE(qcom_tzmem_chunks, GFP_ATOMIC);
>  static DEFINE_SPINLOCK(qcom_tzmem_chunks_lock);
>  
> -#if IS_ENABLED(CONFIG_QCOM_TZMEM_MODE_GENERIC)
> +#ifndef CONFIG_QCOM_TZMEM_MODE_SHMBRIDGE

I realize you likely did this to keep the diff small but I think
negative conditional checks are harder to read than positive ones, so I
would consider making this an '#ifdef' and flipping the branches.

>  static int qcom_tzmem_init(void)
>  {
> @@ -67,7 +67,7 @@ static void qcom_tzmem_cleanup_area(struct qcom_tzmem_area *area)
>  
>  }
>  
> -#elif IS_ENABLED(CONFIG_QCOM_TZMEM_MODE_SHMBRIDGE)
> +#else
>  
>  #include <linux/firmware/qcom/qcom_scm.h>
>  #include <linux/of.h>
> -- 
> 2.55.0
> 

-- 
Cheers,
Nathan

WARNING: multiple messages have this Message-ID (diff)
From: Nathan Chancellor <nathan@kernel.org>
To: Julian Braha <julianbraha@gmail.com>
Cc: amirreza.zarrabi@oss.qualcomm.com, jenswi@kernel.org,
	sumit.garg@kernel.org, arnd@arndb.de, geert+renesas@glider.be,
	amirreza.zarrabi@oss.qualcomm.org,
	op-tee@lists.trustedfirmware.org, linux-kernel@vger.kernel.org,
	quic_eberman@quicinc.com, andersson@kernel.org, brgl@kernel.org,
	harshal.dev@oss.qualcomm.com, nsc@kernel.org,
	jani.nikula@linux.intel.com, kees@kernel.org
Subject: Re: [PATCH v3] tee: remove TZMEM_MODE_GENERIC
Date: Fri, 7 Aug 2026 12:05:13 -0700	[thread overview]
Message-ID: <20260807190513.GA2638974@ax162> (raw)
In-Reply-To: <20260807175041.3299349-1-julianbraha@gmail.com>

On Fri, Aug 07, 2026 at 06:50:41PM +0100, Julian Braha wrote:
> 'select' does not work on config options in a 'choice', so currently it is
> possible to enable QCOMTEE without QCOM_TZMEM_MODE_SHMBRIDGE, even though
> this is needed at runtime.
> 
> There are no users of the generic allocator option,
> QCOM_TZMEM_MODE_GENERIC, so let's remove it. Then, we can remove the
> containing choice..endchoice, which allows the 'select' to work as
> intended.
> 
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Julian Braha <julianbraha@gmail.com>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

One small nit below but I would only send v4 if there are other things
to be addressed.

A note to the maintainers: We would like to turn selecting a choice
symbol into a hard error in 7.4, so please consider picking this up for
7.3-rc1.

> ---
> Changes since v2:
> - add back stubs for when CONFIG_QCOM_TZMEM_MODE_GENERIC=n
> - updated help text accordingly
> 
> Link:
> https://lore.kernel.org/all/20260729203845.387239-1-julianbraha@gmail.com/
> 
> Changes since v1:
> - remove TZMEM_MODE_GENERIC instead of removing the dead select
> 
> Link:
> https://lore.kernel.org/all/20260715092539.18384-1-julianbraha@gmail.com/
> ---
>  drivers/firmware/qcom/Kconfig      | 26 +++++---------------------
>  drivers/firmware/qcom/qcom_tzmem.c |  4 ++--
>  2 files changed, 7 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/firmware/qcom/Kconfig b/drivers/firmware/qcom/Kconfig
> index c7f8413ab996..95b968d88dc3 100644
> --- a/drivers/firmware/qcom/Kconfig
> +++ b/drivers/firmware/qcom/Kconfig
> @@ -34,33 +34,17 @@ config QCOM_TZMEM
>  	tristate
>  	select GENERIC_ALLOCATOR
>  
> -choice
> -	prompt "TrustZone interface memory allocator mode"
> -	depends on QCOM_TZMEM
> -	default QCOM_TZMEM_MODE_GENERIC
> -	help
> -	  Selects the mode of the memory allocator providing memory buffers of
> -	  suitable format for sharing with the TrustZone. If in doubt, select
> -	  'Generic'.
> -
> -config QCOM_TZMEM_MODE_GENERIC
> -	bool "Generic"
> -	help
> -	  Use the generic allocator mode. The memory is page-aligned, non-cachable
> -	  and physically contiguous.
> -
>  config QCOM_TZMEM_MODE_SHMBRIDGE
> -	bool "SHM Bridge"
> +	bool "TrustZone interface memory allocator: SHM Bridge"
> +	depends on QCOM_TZMEM
>  	help
> -	  Use Qualcomm Shared Memory Bridge. The memory has the same alignment as
> -	  in the 'Generic' allocator but is also explicitly marked as an SHM Bridge
> -	  buffer.
> +	  Use Qualcomm Shared Memory Bridge as memory allocator. The memory has the
> +	  same alignment as in the 'Generic' allocator, which is used when this option
> +	  is disabled, but is also explicitly marked as an SHM Bridge buffer.
>  
>  	  With this selected, all buffers passed to the TrustZone must be allocated
>  	  using the TZMem allocator or else the TrustZone will refuse to use them.
>  
> -endchoice
> -
>  config QCOM_QSEECOM
>  	bool "Qualcomm QSEECOM interface driver"
>  	depends on QCOM_SCM=y
> diff --git a/drivers/firmware/qcom/qcom_tzmem.c b/drivers/firmware/qcom/qcom_tzmem.c
> index 0fd9581275f1..510474902c3a 100644
> --- a/drivers/firmware/qcom/qcom_tzmem.c
> +++ b/drivers/firmware/qcom/qcom_tzmem.c
> @@ -50,7 +50,7 @@ static struct device *qcom_tzmem_dev;
>  static RADIX_TREE(qcom_tzmem_chunks, GFP_ATOMIC);
>  static DEFINE_SPINLOCK(qcom_tzmem_chunks_lock);
>  
> -#if IS_ENABLED(CONFIG_QCOM_TZMEM_MODE_GENERIC)
> +#ifndef CONFIG_QCOM_TZMEM_MODE_SHMBRIDGE

I realize you likely did this to keep the diff small but I think
negative conditional checks are harder to read than positive ones, so I
would consider making this an '#ifdef' and flipping the branches.

>  static int qcom_tzmem_init(void)
>  {
> @@ -67,7 +67,7 @@ static void qcom_tzmem_cleanup_area(struct qcom_tzmem_area *area)
>  
>  }
>  
> -#elif IS_ENABLED(CONFIG_QCOM_TZMEM_MODE_SHMBRIDGE)
> +#else
>  
>  #include <linux/firmware/qcom/qcom_scm.h>
>  #include <linux/of.h>
> -- 
> 2.55.0
> 

-- 
Cheers,
Nathan

  parent reply	other threads:[~2026-08-07 19:05 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-07 17:50 [PATCH v3] tee: remove TZMEM_MODE_GENERIC Julian Braha
2026-08-07 18:33 ` Arnd Bergmann
2026-08-07 19:05 ` Nathan Chancellor via OP-TEE [this message]
2026-08-07 19:05   ` Nathan Chancellor

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=20260807190513.GA2638974@ax162 \
    --to=op-tee@lists.trustedfirmware.org \
    --cc=amirreza.zarrabi@oss.qualcomm.com \
    --cc=amirreza.zarrabi@oss.qualcomm.org \
    --cc=andersson@kernel.org \
    --cc=arnd@arndb.de \
    --cc=brgl@kernel.org \
    --cc=geert+renesas@glider.be \
    --cc=harshal.dev@oss.qualcomm.com \
    --cc=jani.nikula@linux.intel.com \
    --cc=jenswi@kernel.org \
    --cc=julianbraha@gmail.com \
    --cc=kees@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nathan@kernel.org \
    --cc=nsc@kernel.org \
    --cc=quic_eberman@quicinc.com \
    --cc=sumit.garg@kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.