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

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

[Manivannan Sadhasivam]

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.

So add a read-back after writing to the MHI_SOC_RESET_REQ_OFFSET register.

Fixes: b5a8d233a588 ("bus: mhi: core: Add device hardware reset support")
Reported-by: Alex Williamson <alex@shazbot.org>
Closes: https://lore.kernel.org/linux-pci/20260622160822.09350246@shazbot.org
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/bus/mhi/host/main.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c
index 53c0ffe30070..4d458396233a 100644
--- a/drivers/bus/mhi/host/main.c
+++ b/drivers/bus/mhi/host/main.c
@@ -170,6 +170,9 @@ EXPORT_SYMBOL_GPL(mhi_get_mhi_state);
 
 void mhi_soc_reset(struct mhi_controller *mhi_cntrl)
 {
+	int __maybe_unused ret;
+	u32 tmp;
+
 	if (mhi_cntrl->reset) {
 		mhi_cntrl->reset(mhi_cntrl);
 		return;
@@ -178,6 +181,9 @@ void mhi_soc_reset(struct mhi_controller *mhi_cntrl)
 	/* Generic MHI SoC reset */
 	mhi_write_reg(mhi_cntrl, mhi_cntrl->regs, MHI_SOC_RESET_REQ_OFFSET,
 		      MHI_SOC_RESET_REQ);
+	/* Flush the posted write to the device (ignore return value) */
+	ret = mhi_read_reg(mhi_cntrl, mhi_cntrl->regs, MHI_SOC_RESET_REQ_OFFSET,
+			   &tmp);
 }
 EXPORT_SYMBOL_GPL(mhi_soc_reset);
 
-- 
2.43.0

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

[Jeff Hugo]

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.

Regardless, since this reset will either kill the pcie link, or 
disconnect the SoC from the link for a time, I've been trying to figure 
out how this change might break, but I haven't found a scenario, so I 
suspect this is good enough.

> So add a read-back after writing to the MHI_SOC_RESET_REQ_OFFSET register.
> 
> Fixes: b5a8d233a588 ("bus: mhi: core: Add device hardware reset support")
> Reported-by: Alex Williamson <alex@shazbot.org>
> Closes: https://lore.kernel.org/linux-pci/20260622160822.09350246@shazbot.org
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
>   drivers/bus/mhi/host/main.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c
> index 53c0ffe30070..4d458396233a 100644
> --- a/drivers/bus/mhi/host/main.c
> +++ b/drivers/bus/mhi/host/main.c
> @@ -170,6 +170,9 @@ EXPORT_SYMBOL_GPL(mhi_get_mhi_state);
>   
>   void mhi_soc_reset(struct mhi_controller *mhi_cntrl)
>   {
> +	int __maybe_unused ret;
> +	u32 tmp;
> +
>   	if (mhi_cntrl->reset) {
>   		mhi_cntrl->reset(mhi_cntrl);
>   		return;
> @@ -178,6 +181,9 @@ void mhi_soc_reset(struct mhi_controller *mhi_cntrl)
>   	/* Generic MHI SoC reset */
>   	mhi_write_reg(mhi_cntrl, mhi_cntrl->regs, MHI_SOC_RESET_REQ_OFFSET,
>   		      MHI_SOC_RESET_REQ);
> +	/* Flush the posted write to the device (ignore return value) */
> +	ret = mhi_read_reg(mhi_cntrl, mhi_cntrl->regs, MHI_SOC_RESET_REQ_OFFSET,
> +			   &tmp);

If you wanted, you could fit this all on one line.  word wrapping for a 
3 char parameter seems a bit silly to me, but I suspect this is highly 
subjective.

Reviewed-by: Jeff Hugo <jeff.hugo@oss.qualcomm.com>

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

[Manivannan Sadhasivam]

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.

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

>  I've been trying to figure out how this
> change might break, but I haven't found a scenario, so I suspect this is
> good enough.
> 

This change should not break any devices as it just ensures that the posted
write gets flushed to the device before the delay.

> > So add a read-back after writing to the MHI_SOC_RESET_REQ_OFFSET register.
> > 
> > Fixes: b5a8d233a588 ("bus: mhi: core: Add device hardware reset support")
> > Reported-by: Alex Williamson <alex@shazbot.org>
> > Closes: https://lore.kernel.org/linux-pci/20260622160822.09350246@shazbot.org
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > ---
> >   drivers/bus/mhi/host/main.c | 6 ++++++
> >   1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c
> > index 53c0ffe30070..4d458396233a 100644
> > --- a/drivers/bus/mhi/host/main.c
> > +++ b/drivers/bus/mhi/host/main.c
> > @@ -170,6 +170,9 @@ EXPORT_SYMBOL_GPL(mhi_get_mhi_state);
> >   void mhi_soc_reset(struct mhi_controller *mhi_cntrl)
> >   {
> > +	int __maybe_unused ret;
> > +	u32 tmp;
> > +
> >   	if (mhi_cntrl->reset) {
> >   		mhi_cntrl->reset(mhi_cntrl);
> >   		return;
> > @@ -178,6 +181,9 @@ void mhi_soc_reset(struct mhi_controller *mhi_cntrl)
> >   	/* Generic MHI SoC reset */
> >   	mhi_write_reg(mhi_cntrl, mhi_cntrl->regs, MHI_SOC_RESET_REQ_OFFSET,
> >   		      MHI_SOC_RESET_REQ);
> > +	/* Flush the posted write to the device (ignore return value) */
> > +	ret = mhi_read_reg(mhi_cntrl, mhi_cntrl->regs, MHI_SOC_RESET_REQ_OFFSET,
> > +			   &tmp);
> 
> If you wanted, you could fit this all on one line.  word wrapping for a 3
> char parameter seems a bit silly to me, but I suspect this is highly
> subjective.
> 

Entire driver is still 80 column width, so I was trying to be preserve the same
pattern.

> Reviewed-by: Jeff Hugo <jeff.hugo@oss.qualcomm.com>
> 

Thank you!

- Mani

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

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

[Jeff Hugo]

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.

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.  The only host 
issue I'm aware of is if flow control credits get exhausted, usually the 
host will watchdog, but I don't see how this patch or the situation it 
addresses would trigger that.

> 
>>   I've been trying to figure out how this
>> change might break, but I haven't found a scenario, so I suspect this is
>> good enough.
>>
> 
> This change should not break any devices as it just ensures that the posted
> write gets flushed to the device before the delay.
> 
>>> So add a read-back after writing to the MHI_SOC_RESET_REQ_OFFSET register.
>>>
>>> Fixes: b5a8d233a588 ("bus: mhi: core: Add device hardware reset support")
>>> Reported-by: Alex Williamson <alex@shazbot.org>
>>> Closes: https://lore.kernel.org/linux-pci/20260622160822.09350246@shazbot.org
>>> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
>>> ---
>>>    drivers/bus/mhi/host/main.c | 6 ++++++
>>>    1 file changed, 6 insertions(+)
>>>
>>> diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c
>>> index 53c0ffe30070..4d458396233a 100644
>>> --- a/drivers/bus/mhi/host/main.c
>>> +++ b/drivers/bus/mhi/host/main.c
>>> @@ -170,6 +170,9 @@ EXPORT_SYMBOL_GPL(mhi_get_mhi_state);
>>>    void mhi_soc_reset(struct mhi_controller *mhi_cntrl)
>>>    {
>>> +	int __maybe_unused ret;
>>> +	u32 tmp;
>>> +
>>>    	if (mhi_cntrl->reset) {
>>>    		mhi_cntrl->reset(mhi_cntrl);
>>>    		return;
>>> @@ -178,6 +181,9 @@ void mhi_soc_reset(struct mhi_controller *mhi_cntrl)
>>>    	/* Generic MHI SoC reset */
>>>    	mhi_write_reg(mhi_cntrl, mhi_cntrl->regs, MHI_SOC_RESET_REQ_OFFSET,
>>>    		      MHI_SOC_RESET_REQ);
>>> +	/* Flush the posted write to the device (ignore return value) */
>>> +	ret = mhi_read_reg(mhi_cntrl, mhi_cntrl->regs, MHI_SOC_RESET_REQ_OFFSET,
>>> +			   &tmp);
>>
>> If you wanted, you could fit this all on one line.  word wrapping for a 3
>> char parameter seems a bit silly to me, but I suspect this is highly
>> subjective.
>>
> 
> Entire driver is still 80 column width, so I was trying to be preserve the same
> pattern.
> 
>> Reviewed-by: Jeff Hugo <jeff.hugo@oss.qualcomm.com>
>>
> 
> Thank you!
> 
> - Mani
> 

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

[Manivannan Sadhasivam]

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.

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.

- Mani

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

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

[Jeff Hugo]

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.

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

[Manivannan Sadhasivam]

On Tue, Aug 04, 2026 at 10:51:35AM -0600, Jeff Hugo wrote:
> 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.
> 

This is commonly seen in ARM platforms, mostly in Embedded space, but it is not
an ARM problem though. On these platforms, they don't properly synthesize the
completion timeout and end up triggering Synchronous External Abort (SError),
which the kernel treats it as a panic condition.

There are drivers that try to mitigate this condition, but rather in a racy way
as there is no proper solution available in the kernel:

$ git grep -rl SError drivers/pci
drivers/pci/controller/dwc/pci-keystone.c
drivers/pci/controller/dwc/pcie-designware-host.c
drivers/pci/controller/dwc/pcie-rcar-gen4.c
drivers/pci/controller/pci-aardvark.c

> Do we need to talk to Joe about this?
> 

This is a known limitation and he should be knowing this by now.

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

Hmm, then let's just keep things as is and increase the timeout when someone
complains :/

- Mani

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