mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Qiang Yu <qiang.yu@oss.qualcomm.com>
To: Manivannan Sadhasivam <mani@kernel.org>
Cc: Baochen Qiang <baochen.qiang@oss.qualcomm.com>,
	Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>,
	mhi@lists.linux.dev, linux-arm-msm@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Mayank Rana <mayank.rana@oss.qualcomm.com>
Subject: Re: [PATCH v3] mhi: host: Add standard elf image download functionality
Date: Mon, 22 Dec 2025 02:24:10 -0800	[thread overview]
Message-ID: <aUkcSt2egvtqlkVl@hu-qianyu-lv.qualcomm.com> (raw)
In-Reply-To: <u5scdkfwygizo4rsuuciwsygkgl5danzsobbpytz7recuekfz4@yclft32plmoc>

On Thu, Dec 18, 2025 at 06:12:37PM +0530, Manivannan Sadhasivam wrote:
> On Thu, Dec 18, 2025 at 04:36:28AM -0800, Qiang Yu wrote:
> > On Thu, Dec 18, 2025 at 04:51:18PM +0530, Manivannan Sadhasivam wrote:
> > > On Thu, Dec 18, 2025 at 05:21:54PM +0800, Baochen Qiang wrote:
> > > > 
> > > > 
> > > > On 12/18/2025 5:13 PM, Baochen Qiang wrote:
> > > > > 
> > > > > 
> > > > > On 12/18/2025 4:04 PM, Qiang Yu wrote:
> > > > >> On Thu, Dec 18, 2025 at 10:25:08AM +0530, Manivannan Sadhasivam wrote:
> > > > >>> On Tue, Dec 16, 2025 at 12:26:41AM -0800, Qiang Yu wrote:
> > > > >>>> On Mon, Dec 15, 2025 at 08:41:32PM +0200, Dmitry Baryshkov wrote:
> > > > >>>>> On Sun, Dec 14, 2025 at 11:09:58PM -0800, Qiang Yu wrote:
> > > > >>>>>> On Sat, Dec 13, 2025 at 11:21:11AM +0900, Manivannan Sadhasivam wrote:
> > > > >>>>>>> On Fri, Dec 12, 2025 at 09:24:06PM +0200, Dmitry Baryshkov wrote:
> > > > >>>>>>>> On Fri, Dec 12, 2025 at 10:07:01AM +0900, Manivannan Sadhasivam wrote:
> > > > >>>>>>>>> On Thu, Dec 11, 2025 at 03:57:54PM +0200, Dmitry Baryshkov wrote:
> > > > >>>>>>>>>> On Thu, Dec 11, 2025 at 01:37:12AM -0800, Qiang Yu wrote:
> > > > >>>>>>>>>>> On Wed, Dec 10, 2025 at 12:57:11AM +0200, Dmitry Baryshkov wrote:
> > > > >>>>>>>>>>>> On Sun, Dec 07, 2025 at 10:35:26PM -0800, Qiang Yu wrote:
> > > > >>>>>>>>>>>>> On Sat, Dec 06, 2025 at 01:25:34PM +0200, Dmitry Baryshkov wrote:
> > > > >>>>>>>>>>>>>> On Mon, Dec 01, 2025 at 06:33:15PM -0800, Qiang Yu wrote:
> > > > >>>>>>>>>>>>>>> From: Mayank Rana <mayank.rana@oss.qualcomm.com>
> > > > >>>>>>>>>>>>>>>
> > > > >>>>>>>>>>>>>>> Currently, the FBC image is a non-standard ELF file that contains a single
> > > > >>>>>>>>>>>>>>> ELF header, followed by segments for SBL, and WLAN FW. However, TME-L
> > > > >>>>>>>>>>>>>>> (Trust Management Engine Lite) supported devices (eg. QCC2072) requires
> > > > >>>>>>>>>>>>>>> separate ELF headers for SBL and WLAN FW segments due to TME-L image
> > > > >>>>>>>>>>>>>>> authentication requirement.
> > > > >>>>>>>>>>>>>>>
> > > > >>>>>>>>>>>>>>> Current image format contains two sections in a single binary:
> > > > >>>>>>>>>>>>>>> - First 512KB: ELF header + SBL segments
> > > > >>>>>>>>>>>>>>> - Remaining: WLAN FW segments
> > > > >>>>>>>>>>>>>>>
> > > > >>>>>>>>>>>>>>> The TME-L supported image format contains two sections with two elf
> > > > >>>>>>>>>>>>>>> headers in a single binary:
> > > > >>>>>>>>>>>>>>> - First 512KB: First ELF header + SBL segments
> > > > >>>>>>>>>>>>>>> - Remaining: Second ELF header + WLAN FW segments
> > > > >>>>>>>>>>>>>>>
> > > > >>>>>>>>>>>>>>> Download behavior:
> > > > >>>>>>>>>>>>>>> - Legacy: 1. First 512KB via BHI (ELF header + SBL)
> > > > >>>>>>>>>>>>>>>           2. Full image via BHIe
> > > > >>>>>>>>>>>>>>>
> > > > >>>>>>>>>>>>>>> - TME-L: 1. First 512KB via BHI (First ELF header + SBL)
> > > > >>>>>>>>>>>>>>>          2. Remaining via BHIe (Second ELF header + WLAN FW segments)
> > > > >>>>>>>>>>>>>>>
> > > > >>>>>>>>>>>>>>> Add standard_elf_image flag to mhi_controller_config to indicate TME-L
> > > > >>>>>>>>>>>>>>> supported image format. When set, MHI skips the first 512KB during WLAN FW
> > > > >>>>>>>>>>>>>>> download over BHIe as it is loaded in BHI phase.
> > > > >>>>>>>>>>>>>>
> > > > >>>>>>>>>>>>>> What is standard about it?
> > > > >>>>>>>>>>>>>
> > > > >>>>>>>>>>>>> The TME-L requires standard elf image format which includes single EFL
> > > > >>>>>>>>>>>>> header and WLAN FW segment.
> > > > >>>>>>>>>>>>>
> > > > >>>>>>>>>>>>> The "standard_elf_image" seems misleading. Since the new image format is
> > > > >>>>>>>>>>>>> required for TME-L image authentication, how about using 
> > > > >>>>>>>>>>>>> tme_supported_image?
> > > > >>>>>>>>>>>>
> > > > >>>>>>>>>>>> Just elf_image?
> > > > >>>>>>>>>>>
> > > > >>>>>>>>>>> Is it too generic for this specific use case. Current image format also
> > > > >>>>>>>>>>> contains elf header.
> > > > >>>>>>>>>>
> > > > >>>>>>>>>> upload_elf_image?
> > > > >>>>>>>>>>
> > > > >>>>>>>>>
> > > > >>>>>>>>> Nope. What does 'upload' even mean here? The 'TIS and ELF' spec v1.2 clearly
> > > > >>>>>>>>> defines that an ELF executable can have only one ELF header. So I'd prefer
> > > > >>>>>>>>> 'standard_elf_image' to differentiate it from the non-spec-conformant ELF image
> > > > >>>>>>>>> used previously.
> > > > >>>>>>>>
> > > > >>>>>>>> What kind of ELF image was used previously? Could you please explain
> > > > >>>>>>>> what do 'First ELF header' vs 'Second ELF header' mean here?
> > > > >>>>>>>>
> > > > >>>>>>>
> > > > >>>>>>> I don't have the details of it, but Qiang should be able to explain. But AFAIC,
> > > > >>>>>>> that was a non-standard ELF image and the new one is going to be spec
> > > > >>>>>>> conformant.
> > > > >>>>>>>
> > > > >>>>>> Previous image format:
> > > > >>>>>> ELF header + SBL segments + WLAN FW segments
> > > > >>>>>>
> > > > >>>>>> The TME-L supported image format:
> > > > >>>>>> First ELF header + SBL segments + Second ELF header + WLAN FW segments
> > > > >>>>>
> > > > >>>>> What is the Second ELF header in this context? ELF files usually have
> > > > >>>>> only one header. Are we repeating the same ELF header or is some kind of
> > > > >>>>> an embedded ELF-in-ELF.
> > > > >>>>
> > > > >>>> The "Second ELF header" refers to a separate, complete ELF file embedded
> > > > >>>> within the FBC image, not a duplicate header. The TME-L supported format
> > > > >>>> contains:
> > > > >>>>
> > > > >>>> FBC Image Structure:
> > > > >>>> ┌─────────────────────────────────────┐
> > > > >>>> │  Complete ELF File #1 (SBL)        │
> > > > >>>> │  ┌─────────────────────────────┐   │
> > > > >>>> │  │ ELF Header                  │   │ ← First ELF header
> > > > >>>> │  │ Program Headers             │   │
> > > > >>>> │  │ SBL Segments                │   │
> > > > >>>> │  └─────────────────────────────┘   │
> > > > >>>> ├─────────────────────────────────────┤
> > > > >>>> │  Complete ELF File #2 (WLAN FW)    │
> > > > >>>> │  ┌─────────────────────────────┐   │
> > > > >>>> │  │ ELF Header                  │   │ ← Second ELF header
> > > > >>>> │  │ Program Headers             │   │
> > > > >>>> │  │ WLAN FW Segments            │   │
> > > > >>>> │  └─────────────────────────────┘   │
> > > > >>>> └─────────────────────────────────────┘
> > > > >>>>>
> > > > >>>>>>
> > > > >>>>>> As per 'TIS and ELF' spec v1.2 Mani mentioned, the previous image format
> > > > >>>>>
> > > > >>>>> pointer?
> > > > >>>>
> > > > >>>> The entire 'TIS and ELF' spec v1.2 document descibes the structure of the
> > > > >>>> ELF excutable file, I can not point out a specfic sentence or phase that
> > > > >>>> tell us the previous image format is standard. But at least there is an
> > > > >>>> example we can refer to: Figure A-4. Executable File Example. And I can
> > > > >>>> also use readelf cmd to parse the image.
> > > > >>>>
> > > > >>>>>
> > > > >>>>>> is also standard elf image. But it doesn't meet the requirement of TME-L
> > > > >>>>>> because we need separate elf header for SBL and WL FW for TME-L
> > > > >>>>>> authentication.
> > > > >>>>>>
> > > > >>>>>> So the commit message stating "Currently, the FBC image is a non-standard
> > > > >>>>>> ELF file that contains a single ELF header, followed by segments for SBL,
> > > > >>>>>> and WLAN FW" is not correct and standard_elf_image is not accurate.
> > > > >>>>>>
> > > > >>>>>> Can we avoid saying anything about standard in commit message? Flags eg.
> > > > >>>>>> separate_elf_header and tme_supported_image are more accurate.
> > > > >>>>>
> > > > >>>>> Please define, what is the supported image.
> > > > >>>>
> > > > >>>> The supported image refers to an image format that TME-L can authenticate.
> > > > >>>> Both SBL and WLAN FW should be in ELF format. After powering on, SBL (ELF
> > > > >>>> format, ELF header + SBL segment, first 512 KB) is loaded over BHI and
> > > > >>>> authenticated by TME-L. After entering SBL, WLAN FW (ELF format, skip
> > > > >>>> first 512KB of fbc image) is loaded over BHIe and also authenticated by
> > > > >>>> TME-L.
> > > > >>>>
> > > > >>>
> > > > >>> So what makes it different here is that you are now sending the two FWs
> > > > >>> separately as standalone ELF image to the device for authentication by TME-L,
> > > > >>> but those are combined in a single image file in the host. But what makes you to
> > > > >>> combine two images in the first place? Why can't they be separate ELF files?
> > > > >>>
> > > > >>> I think you can avoid the hassle if you could just have separate ELF images for
> > > > >>> SBL and WLAN FW and say that the TME-L just expects individual ELF image.
> > > > >>>
> > > > >> Yes, they are two separate images combined into a single file. I'm not
> > > > >> sure of the specific reasons for this design choice, so I can't comment
> > > > >> on it. The WLAN team provides a single file for both SBL and WLAN FW, and
> > > > >> I don't know whether they're willing to change.
> > > > >>
> > > > >> Baochen, do you have any comment on this?
> > > > > 
> > > > > Hmm, sorry, no idea :(
> > > > 
> > > > I mean I don't know the reason behind the design choice.
> > > > 
> > > 
> > > Ok, then I guess we should try to get rid of the flag and just check for the
> > > WLAN FW ELF header during runtime:
> > > 
> > > 	/*
> > > 	 * Some FW combine two separate ELF images (SBL + WLAN FW) in a single
> > > 	 * file. Hence, check for the existence of the second ELF header after
> > > 	 * SBL. If present, load the second image separately.
> > > 	 */
> > > 	if (!memcmp(fw_data + mhi_cntrl->sbl_size, ELFMAG, SELFMAG)) {
> > > 		fw_data += mhi_cntrl->sbl_size
> > > 		fw_sz -= mhi_cntrl->sbl_size;
> > > 	}
> > >
> > Hmmm, for the old format image, since the data at `fw_data + mhi_cntrl->sbl_size`
> > is raw WLAN FW data, there's a possibility that the raw binary data could
> > accidentally contain the ELF magic number at that offset, even though it's
> > not actually an ELF file. This could lead to false positive detection and
> > incorrect parsing.
> > 
> 
> Really? How can the WLAN FW segment have the ELF magic at the start of the
> segment? Then it becomes a separate ELF file.

I confirmed with WLAN FW team. It is extremely rare scenario that the old
format image contains ELF magic number at offset 512KB. It is practically
not possible.

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

  parent reply	other threads:[~2025-12-22 10:24 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-02  2:33 Qiang Yu
2025-12-06 11:25 ` Dmitry Baryshkov
2025-12-08  6:35   ` Qiang Yu
2025-12-09 22:57     ` Dmitry Baryshkov
2025-12-11  9:37       ` Qiang Yu
2025-12-11 13:57         ` Dmitry Baryshkov
2025-12-12  1:07           ` Manivannan Sadhasivam
2025-12-12 19:24             ` Dmitry Baryshkov
2025-12-13  2:21               ` Manivannan Sadhasivam
2025-12-15  7:09                 ` Qiang Yu
2025-12-15 18:41                   ` Dmitry Baryshkov
2025-12-16  8:26                     ` Qiang Yu
2025-12-18  1:12                       ` Dmitry Baryshkov
2025-12-18  7:41                         ` Qiang Yu
2025-12-18  4:55                       ` Manivannan Sadhasivam
2025-12-18  8:04                         ` Qiang Yu
2025-12-18  9:13                           ` Baochen Qiang
2025-12-18  9:21                             ` Baochen Qiang
2025-12-18 11:21                               ` Manivannan Sadhasivam
2025-12-18 12:36                                 ` Qiang Yu
2025-12-18 12:42                                   ` Manivannan Sadhasivam
2025-12-18 13:18                                     ` Bjorn Andersson
2025-12-19  4:07                                       ` Qiang Yu
2025-12-22 10:24                                     ` Qiang Yu [this message]
2025-12-15 18:21     ` Jeff Johnson
2025-12-16  6:11       ` Qiang Yu
2025-12-18 18:31 ` Jeff Johnson

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=aUkcSt2egvtqlkVl@hu-qianyu-lv.qualcomm.com \
    --to=qiang.yu@oss.qualcomm.com \
    --cc=baochen.qiang@oss.qualcomm.com \
    --cc=dmitry.baryshkov@oss.qualcomm.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mani@kernel.org \
    --cc=mayank.rana@oss.qualcomm.com \
    --cc=mhi@lists.linux.dev \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®