Linux-remoteproc Archive on lore.kernel.org
 help / color / Atom feed
From: Mathieu Poirier <mathieu.poirier@linaro.org>
To: Alexandre Bailon <abailon@baylibre.com>
Cc: ohad@wizery.com, bjorn.andersson@linaro.org, robh+dt@kernel.org,
	matthias.bgg@gmail.com, linux-remoteproc@vger.kernel.org,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/6] remoteproc: mtk_vpu_rproc: Don't try to load empty PT_LOAD segment
Date: Tue, 21 Jul 2020 14:21:27 -0600
Message-ID: <20200721202127.GB1227776@xps15> (raw)
In-Reply-To: <20200713132927.24925-5-abailon@baylibre.com>

On Mon, Jul 13, 2020 at 03:29:25PM +0200, Alexandre Bailon wrote:
> The firmware generated by our toolchain contains many empty PT_LOAD
> segments. The elf loader don't manage it and will raise an error:
> "bad phdr da 0x0 mem 0x0".
> To workaround it, implement the sanity_check callback to detect the
> empty PT_LOAD segment and change it to PT_NULL.
> In that way, the elf load won't try to load the segment.

This patch doesn't address the real problem, which are empty load segments.  In
my opinion that should be dealt with rather than having to patch things up.  On
the flip side I suspect that you don't control all the process and that systems
are out there with faulty fw images.  As such:

Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>

> 
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
> ---
>  drivers/remoteproc/mtk_apu_rproc.c | 35 +++++++++++++++++++++++++++---
>  1 file changed, 32 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/remoteproc/mtk_apu_rproc.c b/drivers/remoteproc/mtk_apu_rproc.c
> index f2342b747a35..565b3adca5de 100644
> --- a/drivers/remoteproc/mtk_apu_rproc.c
> +++ b/drivers/remoteproc/mtk_apu_rproc.c
> @@ -137,10 +137,39 @@ static void mtk_vpu_rproc_kick(struct rproc *rproc, int vqid)
>  	vpu_write32(vpu_rproc, CORE_CTL_XTENSA_INT, 1 << vqid);
>  }
>  
> +int mtk_vpu_elf_sanity_check(struct rproc *rproc, const struct firmware *fw)
> +{
> +	const u8 *elf_data = fw->data;
> +	struct elf32_hdr *ehdr;
> +	struct elf32_phdr *phdr;
> +	int ret;
> +	int i;
> +
> +	ret = rproc_elf_sanity_check(rproc, fw);
> +	if (ret)
> +		return ret;
> +
> +	ehdr = (struct elf32_hdr *)elf_data;
> +	phdr = (struct elf32_phdr *)(elf_data + ehdr->e_phoff);
> +
> +	for (i = 0; i < ehdr->e_phnum; i++, phdr++) {
> +		/* Remove empty PT_LOAD section */
> +		if (phdr->p_type == PT_LOAD && !phdr->p_paddr)
> +			phdr->p_type = PT_NULL;
> +	}
> +
> +	return 0;
> +}
> +
>  static const struct rproc_ops mtk_vpu_rproc_ops = {
> -	.start		= mtk_vpu_rproc_start,
> -	.stop		= mtk_vpu_rproc_stop,
> -	.kick		= mtk_vpu_rproc_kick,
> +	.start			= mtk_vpu_rproc_start,
> +	.stop			= mtk_vpu_rproc_stop,
> +	.kick			= mtk_vpu_rproc_kick,
> +	.load			= rproc_elf_load_segments,
> +	.parse_fw		= rproc_elf_load_rsc_table,
> +	.find_loaded_rsc_table	= rproc_elf_find_loaded_rsc_table,
> +	.sanity_check		= mtk_vpu_elf_sanity_check,
> +	.get_boot_addr		= rproc_elf_get_boot_addr,
>  };
>  
>  static irqreturn_t mtk_vpu_rproc_callback(int irq, void *data)
> -- 
> 2.26.2
> 

  reply index

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-13 13:29 [PATCH 0/6] Add support of mt8183 APU Alexandre Bailon
2020-07-13 13:29 ` [PATCH 1/6] dt bindings: remoteproc: Add bindings for MT8183 APU Alexandre Bailon
2020-07-14 17:19   ` Rob Herring
2020-07-14 17:22   ` Rob Herring
2020-07-13 13:29 ` [PATCH 2/6] remoteproc: Add a remoteproc driver for the MT8183's APU Alexandre Bailon
2020-07-20 22:17   ` Mathieu Poirier
2020-08-06 13:25     ` Alexandre Bailon
2020-07-20 22:19   ` Mathieu Poirier
2020-07-13 13:29 ` [PATCH 3/6] remoteproc: mtk_vpu_rproc: Add support of JTAG Alexandre Bailon
2020-07-21 19:52   ` Mathieu Poirier
2020-08-06 13:49     ` Alexandre Bailon
2020-07-13 13:29 ` [PATCH 4/6] remoteproc: mtk_vpu_rproc: Don't try to load empty PT_LOAD segment Alexandre Bailon
2020-07-21 20:21   ` Mathieu Poirier [this message]
2020-07-13 13:29 ` [PATCH 5/6] remoteproc: mtk_apu: Don't try to use the APU local RAM Alexandre Bailon
2020-07-21 20:43   ` Mathieu Poirier
2020-07-13 13:29 ` [PATCH 6/6] ARM64: mt8183: Add support of APU to mt8183 Alexandre Bailon
2020-07-17 21:02   ` kernel test robot

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=20200721202127.GB1227776@xps15 \
    --to=mathieu.poirier@linaro.org \
    --cc=abailon@baylibre.com \
    --cc=bjorn.andersson@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=linux-remoteproc@vger.kernel.org \
    --cc=matthias.bgg@gmail.com \
    --cc=ohad@wizery.com \
    --cc=robh+dt@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

Linux-remoteproc Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-remoteproc/0 linux-remoteproc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-remoteproc linux-remoteproc/ https://lore.kernel.org/linux-remoteproc \
		linux-remoteproc@vger.kernel.org
	public-inbox-index linux-remoteproc

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-remoteproc


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git