[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5g6j4guzrbhl4zqmt7amdgewdusycccsh5rdxlpjbkhjdhbdoa@h6tlwam4i3kq>
Date: Tue, 17 Jun 2025 14:49:30 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: Qiang Yu <qiang.yu@....qualcomm.com>
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
jeff.hugo@....qualcomm.com, mhi@...ts.linux.dev, linux-arm-msm@...r.kernel.org,
linux-kernel@...r.kernel.org, quic_bqiang@...cinc.com, can.guo@....qualcomm.com,
Mayank Rana <mayank.rana@....qualcomm.com>
Subject: Re: [PATCH v2] mhi: host: Add standard elf image download
functionality
On Fri, Jun 06, 2025 at 12:53:33AM -0700, Qiang Yu wrote:
> On Thu, Jun 05, 2025 at 10:34:50PM +0530, Manivannan Sadhasivam wrote:
> > On Tue, Jun 03, 2025 at 02:05:44AM -0700, Qiang Yu wrote:
> > > From: Mayank Rana <mayank.rana@....qualcomm.com>
> > >
> > > Currently, the FBC image is a non-standard ELF file that contains a single
> > > ELF header, followed by segments for SBL, RDDM, and AMSS. Some devices are
> > > unable to process this non-standard ELF format and therefore require
> > > special handling during image loading.
> > >
> >
> > What are those "some devices"? Why are they not able to process this format
>
> Eg. QCC2072
Is it a new kind of WLAN chipset using the ath12k driver?
>
> > which is used across the rest of the Qcom devices?
>
> These devices include TME-L (Trust Management Engine Lite).
> Currently, the FBC image is a non-standard ELF file containing an ELF
> header followed by segments for SBL and WLAN firmware. The ELF header and
> SBL segment within the first 512KB are loaded via BHI, while the full FBC
> image is loaded via BHIe.
>
> Due to TME-L limitations, the full FBC image loaded via BHIe cannot be
> processed, as it does not conform to the standard ELF format.
Okay. These information should be part of the patch description.
> >
> > > Add standard_elf_image flag to determine whether the device can process
> > > the non-standard ELF format. If this flag is set, a standard ELF image
> > > must be loaded, meaning the first 512 KB of the FBC image should be
> > > skipped when loading the AMSS image over the BHIe interface.
> >
> > Please explain what is present in the first 512KiB and why skipping that is
> > required.
>
> ELF header and SBL segment are in the first 512KiB.
>
> New FBC image format adds second ELF header in the start of WLAN FW
> segment on top of current format. After loading SBL, second ELF header
> and WLAN FW segment is loaded using BHIe.
> >
> > > Note that
> > > this flag does not affect the SBL image download process.
> > >
> > > Signed-off-by: Mayank Rana <mayank.rana@....qualcomm.com>
> > > Co-developed-by: Qiang Yu <qiang.yu@....qualcomm.com>
> > > Signed-off-by: Qiang Yu <qiang.yu@....qualcomm.com>
> > > ---
> > > Changes in v2:
> > > - V1 patch is paused because of no user. WLAN team plan to add support for
> > > new WLAN chip that requires this patch, so send v2.
> > > - Change author and SOB with new mail address.
> > > - Reword commit message.
> > > - Place standard_elf_image flag after wake_set in struct mhi_controller
> > > - Link to v1: https://lore.kernel.org/mhi/1689907189-21844-1-git-send-email-quic_qianyu@quicinc.com/
> > > ---
> > > drivers/bus/mhi/host/boot.c | 7 +++++++
> > > include/linux/mhi.h | 4 ++++
> > > 2 files changed, 11 insertions(+)
> > >
> > > diff --git a/drivers/bus/mhi/host/boot.c b/drivers/bus/mhi/host/boot.c
> > > index efa3b6dddf4d2f937535243bd8e8ed32109150a4..f1686a8e0681d49f778838820b44f4c845ddbd1f 100644
> > > --- a/drivers/bus/mhi/host/boot.c
> > > +++ b/drivers/bus/mhi/host/boot.c
> > > @@ -584,6 +584,13 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
> > > * device transitioning into MHI READY state
> > > */
> > > if (fw_load_type == MHI_FW_LOAD_FBC) {
> > > + dev_dbg(dev, "standard_elf_image:%s\n",
> > > + (mhi_cntrl->standard_elf_image ? "True" : "False"));
> >
> > This print is just a noise even for debug.
>
> Will drop it.
>
> >
> > > + if (mhi_cntrl->standard_elf_image) {
> > > + fw_data += mhi_cntrl->sbl_size;
> > > + fw_sz -= mhi_cntrl->sbl_size;
> >
> > Is it possible to detect the image type during runtime instead of using a flag?
> > Also, the flag is currently unused. So it should come along an user.
>
> Perhaps we can check the second ELF Magic Number, but I don't think it's
> safe to determine the format by doing such check. Using a flag is simple
> and safe.
Why do you think it is not safe? IMO, relying on a flag is the not so safe
option. What would happen if an user has used old FW? The driver would blindly
assume that the FW is always of the new format, but the user is not aware of it.
It may lead to weird FW crash that would be difficult to debug.
- Mani
--
மணிவண்ணன் சதாசிவம்
Powered by blists - more mailing lists