linux-kernel - Re:Re:Re: [PATCH] bus: mhi: host: Update session id for each suspend and resume

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2d5c6560.6e9e.199e1b29e6f.Coremail.slark_xiao@163.com>
Date: Tue, 14 Oct 2025 15:50:03 +0800 (CST)
From: "Slark Xiao" <slark_xiao@....com>
To: "Krishna Chaitanya Chundru" <krishna.chundru@....qualcomm.com>
Cc: mani@...nel.org, mhi@...ts.linux.dev, linux-arm-msm@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re:Re:Re: [PATCH] bus: mhi: host: Update session id for each
 suspend and resume

At 2025-09-16 11:26:16, "Slark Xiao" <slark_xiao@....com> wrote:
>
>
>At 2025-09-15 15:26:33, "Krishna Chaitanya Chundru" <krishna.chundru@....qualcomm.com> wrote:
>>
>>
>>On 9/12/2025 3:38 PM, Slark Xiao wrote:
>>> On Qualcomm module side, there is a UART print as below:
>>> session id: 0x355fe689 state:2
>>> session id: 0x1f478e42 state:5
>>> The session id reads from register BHI_IMGTXDB(0x218), and the
>>> state indicates the D3 or D0 state.
>>> 
>>> In Windows side, MHI driver would update this session id for each
>>> suspend/resume progress. We benefit from this mechanism since it
>>> could help sync each suspend and resume progress between host and
>>> device, especially for some suspend issue which needs to take
>>> hundreds or thousands cycle. We can easy to figure out which
>>> suspend cycle get a problem and what's happened at that time for
>>> both host and device because they have same id.
>>> 
>>> But in Linux side, this session id value would always be 0x0.
>>> So we add it for potential debug usage.
>>> 
>>> Signed-off-by: Slark Xiao <slark_xiao@....com>
>>> ---
>>>   drivers/bus/mhi/host/pm.c | 15 +++++++++++++++
>>>   1 file changed, 15 insertions(+)
>>> 
>>> diff --git a/drivers/bus/mhi/host/pm.c b/drivers/bus/mhi/host/pm.c
>>> index 33d92bf2fc3e..a6573f687363 100644
>>> --- a/drivers/bus/mhi/host/pm.c
>>> +++ b/drivers/bus/mhi/host/pm.c
>>> @@ -864,6 +864,13 @@ int mhi_pm_suspend(struct mhi_controller *mhi_cntrl)
>>>   	struct device *dev = &mhi_cntrl->mhi_dev->dev;
>>>   	enum mhi_pm_state new_state;
>>>   	int ret;
>>> +	void __iomem *base = mhi_cntrl->bhi;
>>> +	u32 session_id;
>>> +
>>> +	session_id = MHI_RANDOM_U32_NONZERO(BHI_TXDB_SEQNUM_BMSK);
>>> +	dev_dbg(dev, "Starting enter suspend, session id: 0x%x\n",
>>> +		sessin_id);
>>> +	mhi_write_reg(mhi_cntrl, base, BHI_IMGTXDB, session_id);
>>I agree this will help in debugging, but unless it is documented
>>in the MHI spec we can't have this. Since in future if there is
>>some other purpose for this register we end up facing issues.
>>
>>if it already part of spec point it in the commit text.
>>
>>- Krishna Chaitanya.
>Hi Krishna,
>I am not a member of Qualcomm so I don't have a detailed MHI
>spec to describe this register usage.
>But I get some information from their Windows MHI driver:
>
>1. There is release note which describe it:
>UDE/QMI/win/qcude/installer/ReadMe.txt 
>UDE/QCUDE.Standalone.Source.1.00.44.Windows-AnyCPU_ReadMe_1.txt
>......
>MHI driver 1.1.0.2
>  b. Add write to BHI_IMGTXDB for debug purposes.
>......
>
>2.From the code side,  there is a same operation for MHI driver
>enter into M0:
>HostDriver/win/NTAD/MhiHost/Mhi/src/Mhi.c
>......
>void MhiRequestM0(PMHI_DEV_CTXT MhiCtxt)
>{
>.....
>   /* Set BHI_IMGTXDB */
>   KeQuerySystemTime(&randSeed);
>   MhiCtxt->SessionID = RtlRandomEx(&randSeed.LowPart);
>   MhiTrace(TRACE_LEVEL_ERROR, TRACE_FLAG_MHICONFIG, "NEW SessionID: 0x%x\n", MhiCtxt->SessionID);
>   MHI_WRITE_REG(deviceContext->BHIContext.BhiBase, BHI_IMGTXDB, MhiCtxt->SessionID);
>
>   MhiTrace(TRACE_LEVEL_ERROR, TRACE_FLAG_MHICONFIG, "Req -> M0\n");
>......
>
>Not sure if above information is enough for this commit.
>
>Thanks

Any updates about this commit?


>>>   
>>>   	if (mhi_cntrl->pm_state == MHI_PM_DISABLE)
>>>   		return -EINVAL;
>>> @@ -952,6 +959,14 @@ static int __mhi_pm_resume(struct mhi_controller *mhi_cntrl, bool force)
>>>   	struct device *dev = &mhi_cntrl->mhi_dev->dev;
>>>   	enum mhi_pm_state cur_state;
>>>   	int ret;
>>> +	void __iomem *base = mhi_cntrl->bhi;
>>> +	u32 session_id;
>>> +
>>> +	session_id = MHI_RANDOM_U32_NONZERO(BHI_TXDB_SEQNUM_BMSK);
>>> +	dev_dbg(dev, "Starting enter resume, session id: 0x%x\n",
>>> +		session_id);
>>> +
>>> +	mhi_write_reg(mhi_cntrl, base, BHI_IMGTXDB, session_id);
>>>   
>>>   	dev_dbg(dev, "Entered with PM state: %s, MHI state: %s\n",
>>>   		to_mhi_pm_state_str(mhi_cntrl->pm_state),