Re: [PATCH] bus: mhi: host: Flush the posted write after writing to MHI_SOC_RESET_REQ_OFFSET

On 8/3/2026 10:57 AM, Manivannan Sadhasivam wrote:
> On Mon, Aug 03, 2026 at 09:57:47AM -0600, Jeff Hugo wrote:
>> On 7/29/2026 11:19 PM, Manivannan Sadhasivam wrote:
>>> On Wed, Jul 29, 2026 at 03:07:00PM -0600, Jeff Hugo wrote:
>>>> On 6/23/2026 8:51 AM, Manivannan Sadhasivam wrote:
>>>>> mhi_soc_reset() tries to reset the device by writing to the
>>>>> MHI_SOC_RESET_REQ_OFFSET register. But it doesn't do a read-back to ensure
>>>>> that the write gets flushed to the device before returning to the caller.
>>>>>
>>>>> This may lead to the delay (if implemented) on the caller to be
>>>>> insufficient, if the posted write doesn't reach the device before the
>>>>> delay.
>>>>
>>>> Interesting.  Is the delay tight enough that a few ms will possibly blow it?
>>>> Seems like a poorly defined delay.  All the devices I'm familiar with take
>>>> multiple seconds to boot (with some variability due to ddr training and
>>>> thermal constraints), and if the reset triggers a crash dump, then its
>>>> easily tens of seconds.
>>>>
>>>
>>> Which delay you are referring to? RDDM delay which is just 2ms? Yeah, that seems
>>> questionable. But this function itself doesn't implement any delay. So your
>>> comment was somewhat confusing.
>>
>> The delay at the caller of the API referenced by the commit text - "This may
>> lead to the delay on the caller to be insufficient"
>>
>> I looked at the thread referenced by this patch via the closes tag, and
>> while I can follow the virtualization flow, I didn't get a good view on the
>> specifics.
>>
>>>> Regardless, since this reset will either kill the pcie link, or disconnect
>>>> the SoC from the link for a time,
>>>
>>> No, this will not reset the PCIe link AFAIK. PERST separation logic available in
>>> the Endpoint should make sure the PCIe link is active while the SoC is
>>> undergoing the reset. Otherwise, if the host tries to access the device config
>>> space before the device is ready, it will blow up.
>>
>> Reset separation (PERST separation is one part of) is not always enabled.
>> PBL won't enable it as it requires loading the reset sequences to the PMIC,
>> which is outside the scope of ROM.  So, if you have a non-flash boot device,
>> or a flash boot device that has a non-provisioned/otherwise corrupt flash,
>> the EP may be sitting in PBL from cold boot without reset separation
>> enabled.  If the EP is then passed to a VM per the related thread, a reset
>> will occur, which will take the link down.
>>
>> Also, if I recall correctly, automotive products do not enable reset
>> separation at all for ASIL reasons.
>>
> 
> Ok, I was not aware of these scenarios.
> 
>> If the link is down, the upstream component of the EP should complete the
>> read with an error, not cause the host to blow up.
> 
> If the PCIe link is down and if the host tries to read the Endpoint config
> space or BAR, most of the PCIe RC integrations in ARM64 platforms return AXI
> error instead of the typical 'all-one' response and that causes 'Synchronous
> Abort' on the host. And that's what I meant as 'blow up' as it will crash the
> host kernel.

This feels like a PCIe spec violation, or at-least undesired behavior 
from the user perspective.  Maybe a DoS attack vector?  I assume you are 
specifically talking about Qualcomm MSM Arm64 platforms, and not the 
entire Arm64 ecosystem?  I don't recall seeing this in the Arm64 server 
space.

Do we need to talk to Joe about this?

> Anyhow, the motivation of this patch is to ensure that the posted write gets
> flashed to the device before the delay. But I'm not sure on how one should
> address the concern you raised on waiting till the Endpoint reboots.

Sadly I think the user implementing the delay will need to be aware of 
the specific device they are using, and the likely required delay to 
account for the various scenarios that device implements.  I don't think 
we really have the infrastructure to make this universal, and if we did, 
I'm not sure a blanket "all MHI devices reboot in X time" is going to 
work in the long term.