linux-kernel - 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] [day] [month] [year] [list]

Message-ID: <6510264c-d91a-45f3-b484-18d334c515e8@oss.qualcomm.com>
Date: Mon, 15 Sep 2025 12:56:33 +0530
From: Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>
To: Slark Xiao <slark_xiao@....com>, mani@...nel.org
Cc: mhi@...ts.linux.dev, linux-arm-msm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] bus: mhi: host: Update session id for each suspend and
 resume



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",
> +		session_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.
>   
>   	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),