All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mathieu Poirier <mathieu.poirier@linaro.org>
To: Beleswar Padhi <b-padhi@ti.com>
Cc: andersson@kernel.org, afd@ti.com, hnagalla@ti.com, jm@ti.com,
	jan.kiszka@siemens.com, christophe.jaillet@wanadoo.fr,
	linux-remoteproc@vger.kernel.org, linux-kernel@vger.kernel.org,
	daniel.baluta@nxp.com, iuliana.prodan@nxp.com,
	arnaud.pouliquen@foss.st.com, tanmay.shah@amd.com
Subject: Re: [RFC PATCH] remoteproc: core: Do not process carveout and devmem rsc in attach mode
Date: Tue, 29 Jul 2025 09:34:30 -0600	[thread overview]
Message-ID: <aIjqBi3X4hWGsJLP@p14s> (raw)
In-Reply-To: <20250724133144.3776839-1-b-padhi@ti.com>

Hi Beleswar,

On Thu, Jul 24, 2025 at 07:01:44PM +0530, Beleswar Padhi wrote:
> When attaching to a remote processor, it is implied that the rproc was
> booted by an external entity. Thus, it's carveout and devmem resources
> would already have been processed by the external entity during boot.
> 
> Re-allocating the carveouts in Linux (without proper flags) would zero
> out the memory regions used by the firmware and can lead to undefined
> behaviour. And there is no need to re-map the memory regions for devmem
> resources as well.
> 
> Therefore, do not process the carveout and devmem resources in attach
> mode by not appending them to the rproc->carveouts and rproc->mappings
> lists respectively.
> 

I think what you are proposing is logical.  Arnaud, Daniel, Iuliana and Tanmay -
please test this on your platforms.  I will also need another TB from someone at
TI.

Regards,
Mathieu

> Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
> ---
> Testing:
> 1. Tested IPC with remoteprocs in attach mode in TI platforms.
> [However, TI K3 platforms do not use resource table for carveouts,
> all the memory regions are reserved statically in Device Tree.]
> 
>  drivers/remoteproc/remoteproc_core.c | 30 ++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 825672100528..ef709a5fa73c 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -640,6 +640,20 @@ static int rproc_handle_devmem(struct rproc *rproc, void *ptr,
>  		return -EINVAL;
>  	}
>  
> +	/*
> +	 * When attaching to a remote processor, it is implied that the rproc
> +	 * was booted by an external entity. Thus, it's devmem resources would
> +	 * already have been mapped by the external entity during boot. There is
> +	 * no need to re-map the memory regions here.
> +	 *
> +	 * Skip adding the devmem rsc to the mapping list and return without
> +	 * complaining.
> +	 */
> +	if (rproc->state == RPROC_DETACHED) {
> +		dev_info(dev, "skipping devmem rsc in attach mode\n");
> +		return 0;
> +	}
> +
>  	mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
>  	if (!mapping)
>  		return -ENOMEM;
> @@ -839,6 +853,22 @@ static int rproc_handle_carveout(struct rproc *rproc,
>  		return -EINVAL;
>  	}
>  
> +	/*
> +	 * When attaching to a remote processor, it is implied that the rproc
> +	 * was booted by an external entity. Thus, it's carveout resources would
> +	 * already have been allocated by the external entity during boot.
> +	 * Re-allocating the carveouts here (without proper flags) would zero
> +	 * out the memory regions used by the firmware and can lead to undefined
> +	 * behaviour.
> +	 *
> +	 * Skip adding the carveouts to the alloc list and return without
> +	 * complaining.
> +	 */
> +	if (rproc->state == RPROC_DETACHED) {
> +		dev_info(dev, "skipping carveout allocation in attach mode\n");
> +		return 0;
> +	}
> +
>  	dev_dbg(dev, "carveout rsc: name: %s, da 0x%x, pa 0x%x, len 0x%x, flags 0x%x\n",
>  		rsc->name, rsc->da, rsc->pa, rsc->len, rsc->flags);
>  
> -- 
> 2.34.1
> 

  reply	other threads:[~2025-07-29 15:34 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-24 13:31 [RFC PATCH] remoteproc: core: Do not process carveout and devmem rsc in attach mode Beleswar Padhi
2025-07-29 15:34 ` Mathieu Poirier [this message]
2025-08-01 16:31   ` Tanmay Shah
2025-07-30 18:44 ` Andrew Davis

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=aIjqBi3X4hWGsJLP@p14s \
    --to=mathieu.poirier@linaro.org \
    --cc=afd@ti.com \
    --cc=andersson@kernel.org \
    --cc=arnaud.pouliquen@foss.st.com \
    --cc=b-padhi@ti.com \
    --cc=christophe.jaillet@wanadoo.fr \
    --cc=daniel.baluta@nxp.com \
    --cc=hnagalla@ti.com \
    --cc=iuliana.prodan@nxp.com \
    --cc=jan.kiszka@siemens.com \
    --cc=jm@ti.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-remoteproc@vger.kernel.org \
    --cc=tanmay.shah@amd.com \
    /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.