Re: [PATCH v2] mhi: host: Add standard elf image download functionality

[Qiang Yu]

On Tue, Jun 17, 2025 at 02:49:30PM +0530, Manivannan Sadhasivam wrote:
> 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@xxxxxxxxxxxxxxxx>
> > > > 
> > > > 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?

Yes

> 
> > 
> > > 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.

OK, will add it in commit message.

> 
> > > 
> > > > 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@xxxxxxxxxxxxxxxx>
> > > > Co-developed-by: Qiang Yu <qiang.yu@xxxxxxxxxxxxxxxx>
> > > > Signed-off-by: Qiang Yu <qiang.yu@xxxxxxxxxxxxxxxx>
> > > > ---
> > > > 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@xxxxxxxxxxx/
> > > > ---
> > > >  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.
>

It's possible that the WL firmware segment could contain the sequence 0x7f
'E' 'L' 'F', which might lead to a false detection of the second ELF
header.

- Qiang Yu

> - Mani
> 
> -- 
> மணிவண்ணன் சதாசிவம்
>