lkml.org 
[lkml]   [2023]   [Jan]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] PCI: Exit restore process when device is still powerdown
From
在 2023/1/13 6:13, Bjorn Helgaas 写道:
> On Thu, Dec 22, 2022 at 12:41:04PM +0000, Jiantao Zhang wrote:
>> We get this stack when the rp doesn't power up in resume noirq:
>
> s/rp/Root Port/
>
> "resume noirq" seems to refer to a function, so please mention the
> exact function name.
>
>> dump_backtrace.cfi_jt+0x0/0x4
>> dump_stack_lvl+0xb4/0x10c
>> show_regs_before_dump_stack+0x1c/0x30
>> arm64_serror_panic+0x110/0x1a8
>> do_serror+0x16c/0x1cc
>> el1_error+0x8c/0x10c
>> do_raw_spin_unlock+0x74/0xdc
>> pci_bus_read_config_word+0xdc/0x1dc
>> pci_restore_msi_state+0x2f4/0x36c
>> pci_restore_state+0x13f0/0x1444
>> pci_pm_resume_noirq+0x158/0x318
>> dpm_run_callback+0x178/0x5e8
>> device_resume_noirq+0x250/0x264
>> async_resume_noirq+0x20/0xf8
>> async_run_entry_fn+0xfc/0x364
>> process_one_work+0x37c/0x7f4
>> worker_thread+0x3e8/0x754
>> kthread+0x168/0x204
>> ret_from_fork+0x10/0x18
>> The ep device uses msix, the restore process will write bar space
>> in __pci_msix_desc_mask_irq, which will result in accessing the
>> powerdown area when the rp doesn't power on.
>
> s/ep/endpoint/
> s/msix/MSI-X/ to match spec usage
> s/bar/BAR/
> Add "()" after function names, e.g., __pci_msix_desc_mask_irq()
> s/rp/Root Port/
>
>> It makes sense we should do nothing when the device is still powerdown.
>>
>> Signed-off-by: Jianrong Zhang <zhangjianrong5@huawei.com>
>> Signed-off-by: Jiantao Zhang <water.zhangjiantao@huawei.com>
>> ---
>> drivers/pci/pci.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index fba95486caaf..279f6e8c5a00 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -1764,7 +1764,7 @@ static void pci_restore_rebar_state(struct pci_dev *pdev)
>> */
>> void pci_restore_state(struct pci_dev *dev)
>> {
>> - if (!dev->state_saved)
>> + if (!dev->state_saved || dev->current_state == PCI_UNKNOWN)
>> return;
>
> This doesn't seem right to me because it seems like we're covering up
> a problem elsewhere.
>
> If we need access to the endpoint to restore state, shouldn't we
> ensure that the endpoint is powered up before we try to access it?
>
> We depend on the state being restored, so if we skip the restore here,
> where *will* it happen?
As the call stack shows the serror happens in pci_pm_resume_noirq(),
which belongs to pci pm framework. The resume process related to pci
devices goes like this:

stage noirq:
Root Port's call stack: device_resume_noirq() --> pci_pm_resume_noirq()
--> resume_noirq callback
endpoint's call stack: device_resume_noirq() --> pci_pm_resume_noirq()
--> resume_noirq callback

stage early:
Root Port's call stack: device_resume_early() --> pci_pm_resume_early()
--> device resume_early callback
endpoint's call stack: device_resume_early() --> pci_pm_resume_early()
--> device resume_early callback

stage normal:
Root Port's call stack: device_resume() --> pci_pm_resume() --> device
resume callback
endpoint's call stack: device_resume() --> pci_pm_resume() --> device
resume callback

The problem is we don't power up the controller in Root Port's
resume_noirq callback
(actually we don't even register resume_noirq callback for some reason),
so the serror happens because of accessing powerdown area when
endpoint's pci_pm_resume_noirq()
calls pci_restore_state() which will call pci_restore_msi_state() to
restore MSI-X state.
So we wonder if there is strong restriction that we must poweron in Root
Port's resume_noirq callback.
The pci_restore_state() can't restore anything when the device is still
at PCI_UNKNOWN state,
and if the device is accessible it can't be at PCI_UNKNOWN state, so the
patch doesn't make any difference
for original process.
>
> Bjorn
> .

\
 
 \ /
  Last update: 2023-03-26 23:48    [W:0.025 / U:1.004 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site