linux-kernel - Re: [bug report] scsi: SATA devices missing after FLR is triggered during HBA suspended

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: <93bd907d-c298-497d-ada2-88706233232e@kernel.org>
Date: Tue, 2 Jul 2024 11:38:23 +0900
From: Damien Le Moal <dlemoal@...nel.org>
To: Bjorn Helgaas <helgaas@...nel.org>
Cc: Yihang Li <liyihang9@...wei.com>, cassel@...nel.org,
 James.Bottomley@...senpartnership.com, martin.petersen@...cle.com,
 john.g.garry@...cle.com, yanaijie@...wei.com, linux-kernel@...r.kernel.org,
 linux-scsi@...r.kernel.org, linuxarm@...wei.com, chenxiang66@...ilicon.com,
 prime.zeng@...wei.com, "linux-pci@...r.kernel.org"
 <linux-pci@...r.kernel.org>, Bjorn Helgaas <bhelgaas@...gle.com>,
 Alex Williamson <alex.williamson@...hat.com>
Subject: Re: [bug report] scsi: SATA devices missing after FLR is triggered
 during HBA suspended

On 7/2/24 05:39, Bjorn Helgaas wrote:
> [+cc Alex]
> 
> On Thu, Jun 27, 2024 at 09:56:02AM +0900, Damien Le Moal wrote:
>> On 6/27/24 00:15, Bjorn Helgaas wrote:
>>>>> Yes, I am talking about the PCI "Function Level Reset"
>>>>>
>>>>>> FLR and disk/controller suspend execution timing are unrelated.
>>>>>> FLR can be triggered at any time through sysfs. So please give
>>>>>> details here. Why is FLR done when the system is being
>>>>>> suspended ?
>>>>>
>>>>> Yes, it is because FLR can be triggered at any time that we are
>>>>> testing the reliability of executing FLR commands after
>>>>> disk/controller suspended.
>>>>
>>>> "can be triggered" ? FLR is not a random asynchronous event. It
>>>> is an action that is *issued* by a user with sys admin rights.
>>>> And such users can do a lot of things that can break a machine...
>>>>
>>>> I fail to see the point of doing a function reset while the
>>>> device is suspended. But granted, I guess the device should
>>>> comeback up in such case, though I would like to hear what the
>>>> PCI guys have to say about this.
>>>>
>>>> Bjorn,
>>>>
>>>> Is reseting a suspended PCI device something that should be/is
>>>> supported ?
>>>
>>> I doubt it.  The PCI core should be preserving all the generic PCI
>>> state across suspend/resume.  The driver should only need to
>>> save/restore device-specific things the PCI core doesn't know about.
>>>
>>> A reset will clear out most state, and the driver doesn't know the
>>> reset happened, so it will expect most device state to have been
>>> preserved.
>>
>> That is what I suspected. However, checking the code, reset_store() in
>> pci-sysfs.c does:
>>
>> 	pm_runtime_get_sync(dev);
>> 	result = pci_reset_function(pdev);
>> 	pm_runtime_put(dev);
>>
>> and pm_runtime_get_sync() calls __pm_runtime_resume() which will
>> resume a suspended device.
>>
>> So while I still think it is not a good idea to reset a suspended
>> device, things should still work as execpected and not cause any
>> problem with the device state, right ?
> 
> The reset will clear almost all state, including both the generic PCI
> part that pci_reset_function() saves/restores *and* any
> device-specific state the PCI core doesn't know about.
> 
> That device-specific state isn't saved and restored anywhere in the
> sysfs reset path, and the driver doesn't know this reset happened, so
> I think all bets are off and we shouldn't expect the driver to work
> afterwards.
> 
> A user-space reset might make sense if there's no driver bound to the
> device, but I don't think it does if there is a driver (except maybe a
> trivial stub driver that doesn't actually operate the device).

OK, makes sense.

I amstill looking into this though because I did find a nasty issue: if the HBA
is reset while all the drives connected to it are suspended (spun down), the
drives are never woken up and the drive re-scan trigerred by the PCI reset fails
with command timeouts. And even worse, I hit a deadlock when unloading the
driver after that happens.

All of that should not be happening: the HBA reset should simply result in
either all drives coming back or the drives (scsi devices) being dropped and
re-scan creating new ones. But that I think is not a PCI issue but rather a HBA
driver issue, or a problem with libsas/scsi/libata power management.

Thanks for the comments.

-- 
Damien Le Moal
Western Digital Research