On Sat, 15 Jul 2023 at 23:35, Bjorn Andersson <andersson@xxxxxxxxxx> wrote:
>
> On Mon, May 08, 2023 at 06:35:24PM +0300, Dmitry Baryshkov wrote:
> > On MSM8974 and APQ8074 the MSS requires loading raw MBA image instead of
> > the ELF file. Skip the ELF headers if mba.mbn was specified as the
> > firmware image.
> >
> > Fixes: 051fb70fd4ea ("remoteproc: qcom: Driver for the self-authenticating Hexagon v5")
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
>
> Rather than complicating the driver further, can't we just extract that
> first segment? Or specify the first split segment file?
Missed your response.
Of course, we can. But this makes this particular case really stand
off. In all other cases we use the .mbn file and only msm8974/apq8084
only mba requires .b00 file. I think from the point of uniformity we'd
better use mba.mbn here too.
If the q6v5_mba_get_offset() design seems too complicated, I can
replace that with the fixed offset of 0x1000. WDYT?
>
> Regards,
> Bjorn
>
> > ---
> >
> > Changes since v1:
> > - Replace fixed offset 0x1000 with the value obtained from ELF headers
> > - Implement ELF validity checks
> >
> > ---
> > drivers/remoteproc/qcom_q6v5_mss.c | 47 +++++++++++++++++++++++++++++-
> > 1 file changed, 46 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
> > index ab053084f7a2..b4ff900f0304 100644
> > --- a/drivers/remoteproc/qcom_q6v5_mss.c
> > +++ b/drivers/remoteproc/qcom_q6v5_mss.c
> > @@ -11,6 +11,7 @@
> > #include <linux/delay.h>
> > #include <linux/devcoredump.h>
> > #include <linux/dma-mapping.h>
> > +#include <linux/firmware.h>
> > #include <linux/interrupt.h>
> > #include <linux/kernel.h>
> > #include <linux/mfd/syscon.h>
> > @@ -29,6 +30,7 @@
> > #include <linux/iopoll.h>
> > #include <linux/slab.h>
> >
> > +#include "remoteproc_elf_helpers.h"
> > #include "remoteproc_internal.h"
> > #include "qcom_common.h"
> > #include "qcom_pil_info.h"
> > @@ -459,6 +461,35 @@ static void q6v5_debug_policy_load(struct q6v5 *qproc, void *mba_region)
> > release_firmware(dp_fw);
> > }
> >
> > +/* Get the offset of the segment 0 for mba.mbn */
> > +static int q6v5_mba_get_offset(struct rproc *rproc, const struct firmware *fw)
> > +{
> > + const struct elf32_hdr *ehdr;
> > + const void *phdr;
> > + char class;
> > + u64 phoffset, poffset;
> > + u16 phentsize;
> > + int ret;
> > +
> > + ret = rproc_elf_sanity_check(rproc, fw);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ehdr = (const struct elf32_hdr *)fw->data;
> > + class = ehdr->e_ident[EI_CLASS];
> > + phoffset = elf_hdr_get_e_phoff(class, ehdr);
> > + phentsize = elf_hdr_get_e_phentsize(class, ehdr);
> > + if (phoffset + phentsize > fw->size)
> > + return -EINVAL;
> > +
> > + phdr = fw->data + elf_hdr_get_e_phoff(class, ehdr);
> > + poffset = elf_phdr_get_p_offset(class, phdr);
> > + if (poffset > fw->size)
> > + return -EINVAL;
> > +
> > + return poffset;
> > +}
> > +
> > static int q6v5_load(struct rproc *rproc, const struct firmware *fw)
> > {
> > struct q6v5 *qproc = rproc->priv;
> > @@ -477,7 +508,21 @@ static int q6v5_load(struct rproc *rproc, const struct firmware *fw)
> > return -EBUSY;
> > }
> >
> > - memcpy(mba_region, fw->data, fw->size);
> > + if (qproc->version == MSS_MSM8974 &&
> > + !memcmp(fw->data, ELFMAG, SELFMAG)) {
> > + int poffset;
> > +
> > + poffset = q6v5_mba_get_offset(rproc, fw);
> > + if (poffset < 0) {
> > + memunmap(mba_region);
> > + return poffset;
> > + }
> > +
> > + memcpy(mba_region, fw->data + poffset, fw->size - poffset);
> > + } else {
> > + memcpy(mba_region, fw->data, fw->size);
> > + }
> > +
> > q6v5_debug_policy_load(qproc, mba_region);
> > memunmap(mba_region);
> >
> > --
> > 2.39.2
> >
--
With best wishes
Dmitry
