linux-kernel - Re: [PATCH v3 1/4] PCI: Keep AER status in pci_restore

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: <adad9761-b671-66ec-8946-78d017071076@linux.intel.com>
Date:   Thu, 20 Apr 2023 19:30:47 -0700
From:   Sathyanarayanan Kuppuswamy 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>
To:     Kai-Heng Feng <kai.heng.feng@...onical.com>
Cc:     bhelgaas@...gle.com, mika.westerberg@...ux.intel.com,
        koba.ko@...onical.com, linux-pci@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/4] PCI: Keep AER status in pci_restore_state()



On 4/20/23 6:35 PM, Kai-Heng Feng wrote:
> Hi Sathyanarayanan,
> 
> On Thu, Apr 20, 2023 at 10:39 PM Sathyanarayanan Kuppuswamy
> <sathyanarayanan.kuppuswamy@...ux.intel.com> wrote:
>>
>> Hi Kai,
> 
> It's Kai-Heng :)
> 
>>
>> On 4/20/23 5:59 AM, Kai-Heng Feng wrote:
>>> When AER is using the same IRQ as PME, AER interrupt is treated as a
>>> wakeup event and it can disrupt system suspend process.
>>>
>>> If that happens, the system will report it's woken up by PME IRQ without
>>> indicating any AER error since AER status is cleared on resume.
>>>
>>> So keep the AER status so users can know the system is woken up by AER
>>> instead of PME.
>>>
>>> Reviewed-by: Mika Westerberg <mika.westerberg@...ux.intel.com>
>>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@...onical.com>
>>> ---
>>
>> Any history on why it is cleared before? Is it done to hide some resume
>> issues?
> 
> It was introduced by commit b07461a8e45b ("PCI/AER: Clear error status
> registers during enumeration and restore").
> The justification is quite reasonable so I think maybe we should keep it as is.

Yes. It looks like it is better to leave it as it is.


> 
> Kai-Heng
> 
>>
>>> v3:
>>>  - No change.
>>>
>>> v2:
>>>  - New patch.
>>>
>>>  drivers/pci/pci.c | 1 -
>>>  1 file changed, 1 deletion(-)
>>>
>>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>>> index 7a67611dc5f4..71aead00fc20 100644
>>> --- a/drivers/pci/pci.c
>>> +++ b/drivers/pci/pci.c
>>> @@ -1778,7 +1778,6 @@ void pci_restore_state(struct pci_dev *dev)
>>>       pci_restore_dpc_state(dev);
>>>       pci_restore_ptm_state(dev);
>>>
>>> -     pci_aer_clear_status(dev);
>>>       pci_restore_aer_state(dev);
>>>
>>>       pci_restore_config_space(dev);
>>
>> --
>> Sathyanarayanan Kuppuswamy
>> Linux Kernel Developer

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer