linux-kernel - Re: Subject : [ PATCH ] pci-reset-error_state-to-pci_channel_io_normal-at-report_slot

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:	Tue, 21 May 2013 08:38:47 -0500
From:	Linas Vepstas <linasvepstas@...il.com>
To:	yanmin_zhang <yanmin_zhang@...ux.intel.com>
Cc:	"Liu, Joseph" <Joseph.Liu@...lex.com>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	"Zhang, LongX" <longx.zhang@...el.com>,
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"Rafael J. Wysocki" <rjw@...k.pl>
Subject: Re: Subject : [ PATCH ] pci-reset-error_state-to-pci_channel_io_normal-at-report_slot_reset

Hi,

On 21 May 2013 02:49, Yanmin Zhang <yanmin_zhang@...ux.intel.com> wrote:
> On Mon, 2013-05-20 at 10:37 -0500, Linas Vepstas wrote:

>>  My impression
>> is that maybe the AER driver had been doing not quite the right thing
>> for a long time.
> Pls. provide evidence/facts. The new patch is to facilitate device driver
> implementation. It doesn't mean current AER driver is incorrect. We need
> a tradeoff.
>
> Just like what Bjoin says, we shouldn't change error_state to pci_channel_io_normal
> before we really recover the hardware. The patch changes it just because
> drivers might call some functions to recover the devices, while such functions
> need (error_state==pci_channel_io_normal).

Perhaps we are talking past each other.  One needs to set error_state
== pci_channel_io_normal before calling slot_reset().  If the aer
driver wasn't doing this all along, then it seems like an old bug to
me.   The error_state flag indicates the status of the PCI channel,
and not the status of the attached device.  Once the channel has been
reset, the error state is "normal" even if the card itself hasn't yet
been brought up.

Whatever it is that the aer driver is doing, it should be doing
something similar to what the eeh driver is doing, in
arch/powerpc/platforms/pseries/eeh_driver.c  -- This is the "reference
implementation" -- Its known right, it was and continues to be heavily
tested, has found its way into sriov, etc.

The one thing that eeh does NOT do is to handle suspend/sleep states.
The basic design assumption back then was that no one would ever
suspend/sleep their server.  Since suspend/sleep messes with PCI
config space, then, yes, one would need to somehow save a second,
pristine copy of config space for device recovery.

-- Linas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/