MIME-Version: 1.0
References: <20230125110100.14647-1-iuliana.prodan@oss.nxp.com> <20230126224905.GA4020499@p14s>
In-Reply-To: <20230126224905.GA4020499@p14s>
From:   Daniel Baluta <daniel.baluta@gmail.com>
Date:   Fri, 27 Jan 2023 16:12:48 +0200
Message-ID: <CAEnQRZDOvcnr0NtJJp40DvTsUggmmPd3UsVramWxwj35OYfR6g@mail.gmail.com>
Subject: Re: [PATCH] remoteproc: imx_dsp_rproc: add custom memory copy
 implementation for i.MX DSP Cores
To:     Mathieu Poirier <mathieu.poirier@linaro.org>
Cc:     "Iuliana Prodan (OSS)" <iuliana.prodan@oss.nxp.com>,
        Bjorn Andersson <andersson@kernel.org>,
        Shawn Guo <shawnguo@kernel.org>,
        Sascha Hauer <s.hauer@pengutronix.de>,
        "S.J. Wang" <shengjiu.wang@nxp.com>,
        Fabio Estevam <festevam@gmail.com>,
        Daniel Baluta <daniel.baluta@nxp.com>,
        Paul Olaru <paul.olaru@nxp.com>,
        Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>,
        Iuliana Prodan <iuliana.prodan@nxp.com>,
        linux-imx <linux-imx@nxp.com>, linux-remoteproc@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        Pengutronix Kernel Team <kernel@pengutronix.de>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

<snip>

>
> This patchset from last year[1] goes to great length to avoid using a driver
> specific function and now you are trying to bring that back...  So how was it
> working before and why are things broken now?  Moreover, function
> rproc_elf_load_segments() deals with situations where the memory slot is bigger
> than the file size[2], which is omitted here.

As I mentioned in [1] the problem was and it is still there if an elf
section has a size which
is not a multiple of 4.

And we do notice this when compiling Sound Open Firmware and loading
it via rproc interface.

This is a hardware limitation! And we need to address it. The
limitation happens for IRAM address range
of HIFI4 DSP integration with I.MX.

Some call it a limitation, others call it an optimization. Either way
we need to avoid crashing the kernel.

We have two options:

1) Fully address it in rproc driver (like the present patch).
2) Partially address it in rproc driver (by returning an error for
section with wrong sizes) and then
making all linker scripts to force elf sections to be a multiple of 4.

I would go for option 1) even if we go back to use our custom `load` function.