linux-kernel - Re: [PATCH v3 1/1] PCI/ERR: Fix reset logic in pcie_do

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]

Date:   Fri, 25 Sep 2020 13:47:41 -0400
From:   Sinan Kaya <okaya@...nel.org>
To:     "Kuppuswamy, Sathyanarayanan" 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>,
        Bjorn Helgaas <helgaas@...nel.org>
Cc:     bhelgaas@...gle.com, linux-pci@...r.kernel.org,
        linux-kernel@...r.kernel.org, ashok.raj@...el.com,
        Jay Vosburgh <jay.vosburgh@...onical.com>
Subject: Re: [PATCH v3 1/1] PCI/ERR: Fix reset logic in pcie_do_recovery()
 call

On 9/25/2020 1:11 PM, Kuppuswamy, Sathyanarayanan wrote:
>> Why? Isn't DPC slot reset enough?
> It will do the reset at hardware level. But driver state is not
> cleaned up. So doing bus reset will restore both driver and
> hardware states.

I really don't like this. If hotplug driver is restoring the state
and DPC driver is not; let's fix the DPC driver rather than causing
two resets and hope for the best.

One approach is to share the restore code between hotplug driver and
DPC driver.

If this is a too involved change, DPC driver should restore state
when hotplug is not supported.

DPC driver should be self-sufficient by itself.

> Also for non-fatal errors, if reset is requested then we still need
> some kind of bus reset call here.

DPC should handle both fatal and non-fatal cases and cause a bus reset
in hardware already before triggering an interrupt.

I disagree that you need an additional reset on top of DPC reset.
Isn't one reset enough?

What will the second reset provide that first reset won't provide?

I see that you are trying to do the second reset only because second
reset restores state.

That looks like a short-term fix only to explode on the next iteration.