Support for PCI Express reset type in EEH

By default, EEH does what's known as a "hot reset" during error recovery of a PCI Express device. We've found a case where the device needs a "fundamental reset" to recover properly. The current PCI error recovery and EEH frameworks do not support this distinction.

The attached patch (courtesy of Richard Lary) implements a reset type callback that can be used to determine what type of reset a device requires. It is backwards compatible with all other drivers that implement PCI error recovery callbacks. Only drivers that require a fundamental reset need to be changed. So far we're only aware of one driver that has the requirement (qla2xxx). The patch touches mostly EEH and pseries code, but does require a couple of minor additions to the overall PCI error recovery framework.

Signed-off-by Mike Mason <[email protected]>

--- a/arch/powerpc/include/asm/ppc-pci.h 2009-06-09
20:05:27.000000000 -0700
+++ b/arch/powerpc/include/asm/ppc-pci.h 2009-07-13
16:12:31.000000000 -0700

(quoted diff) +4 -1

@@ -90,7 +90,9 @@ int rtas_pci_enable(struct pci_dn *pdn,
 *
 * Returns a non-zero value if the reset failed.
 */
-int rtas_set_slot_reset (struct pci_dn *);
+#define HOT_RESET        1
+#define FUNDAMENTAL_RESET    3
+int rtas_set_slot_reset (struct pci_dn *, int reset_type);
 int eeh_wait_for_slot_status(struct pci_dn *pdn, int max_wait_msecs);
 
 /** --- a/arch/powerpc/platforms/pseries/eeh.c    2009-06-09 20:05:27.000000000 -0700
+++ b/arch/powerpc/platforms/pseries/eeh.c    2009-07-13 

16:27:27.000000000 -0700

(quoted diff) +5 -6

@@ -666,7 +666,7 @@ rtas_pci_enable(struct pci_dn *pdn, int
 /**
 * rtas_pci_slot_reset - raises/lowers the pci #RST line
 * @pdn pci device node
- * @state: 1/0 to raise/lower the #RST
+ * @state: 1/3/0 to raise hot-reset/fundamental-reset/lower the #RST
 *
 * Clear the EEH-frozen condition on a slot.  This routine
 * asserts the PCI #RST line if the 'state' argument is '1',

@@ -742,9 +742,9 @@ int pcibios_set_pcie_reset_state(struct
 *  Return 0 if success, else a non-zero value.
 */
 
-static void __rtas_set_slot_reset(struct pci_dn *pdn)
+static void __rtas_set_slot_reset(struct pci_dn *pdn, int reset_type)
 {
-    rtas_pci_slot_reset (pdn, 1);
+    rtas_pci_slot_reset (pdn, reset_type);
 
    /* The PCI bus requires that the reset be held high for at least
     * a 100 milliseconds. We wait a bit longer 'just in case'.  */

@@ -766,13 +766,13 @@ static void __rtas_set_slot_reset(struct
    msleep (PCI_BUS_SETTLE_TIME_MSEC);
 }
 
-int rtas_set_slot_reset(struct pci_dn *pdn)
+int rtas_set_slot_reset(struct pci_dn *pdn, int reset_type)
 {
    int i, rc;
 
    /* Take three shots at resetting the bus */
    for (i=0; i<3; i++) {
-        __rtas_set_slot_reset(pdn);
+        __rtas_set_slot_reset(pdn, reset_type);
 
        rc = eeh_wait_for_slot_status(pdn, PCI_BUS_RESET_WAIT_MSEC);
        if (rc == 0)
--- a/arch/powerpc/platforms/pseries/eeh_driver.c    2009-07-13 

14:25:24.000000000 -0700
+++ b/arch/powerpc/platforms/pseries/eeh_driver.c 2009-07-13
16:39:16.000000000 -0700

(quoted diff) +29 -0

@@ -115,6 +115,34 @@ static void eeh_enable_irq(struct pci_de
 
 /* ------------------------------------------------------- */
 /**
+ * eeh_query_reset_type - query each device driver for reset type
+ *
+ * Query each device driver for special reset type if required
+ * merge the device driver responses. Cumulative response
+ * passed back in "userdata".
+ */
+
+static int eeh_query_reset_type(struct pci_dev *dev, void *userdata)
+{
+    enum pci_ers_result rc, *res = userdata;
+    struct pci_driver *driver = dev->driver;
+
+    if (!driver)
+        return 0;
+
+    if (!driver->err_handler ||
+        !driver->err_handler->reset_type)
+        return 0;
+
+    rc = driver->err_handler->reset_type (dev);
+
+    /* A driver that needs a special reset trumps all others */
+    if (rc == PCI_ERS_RESULT_FUNDAMENTAL_RESET ) *res = rc;
+
+    return 0;
+}
+
+/**
 * eeh_report_error - report pci error to each device driver
 * * Report an EEH error to each device driver, collect up and @@ -282,9 
+310,12 @@ static int eeh_report_failure(struct pci

* @pe_dn: pointer to a "Partionable Endpoint" device node.
* This is the top-level structure on which pci
* bus resets can be performed.
+ *
+ * reset_type: some devices may require type other than default hot reset. */

(quoted diff) +2 -1

-static int eeh_reset_device (struct pci_dn *pe_dn, struct pci_bus *bus)
+static int eeh_reset_device (struct pci_dn *pe_dn, struct pci_bus *bus,
+                 int reset_type)

{
    struct device_node *dn;
    int cnt, rc;

(quoted diff) +14 -6

@@ -298,7 +329,7 @@ static int eeh_reset_device (struct pci_
    /* Reset the pci controller. (Asserts RST#; resets config space).
     * Reconfigure bridges and devices. Don't try to bring the system
     * up if the reset failed for some reason. */
-    rc = rtas_set_slot_reset(pe_dn);
+    rc = rtas_set_slot_reset(pe_dn, reset_type);
    if (rc)
        return rc;
 

@@ -343,6 +374,7 @@ struct pci_dn * handle_eeh_events (struc
    struct pci_dn *frozen_pdn;
    struct pci_bus *frozen_bus;
    int rc = 0;
+    int reset_type = HOT_RESET;
    enum pci_ers_result result = PCI_ERS_RESULT_NONE;
    const char *location, *pci_str, *drv_str;
 

@@ -400,10 +432,16 @@ struct pci_dn * handle_eeh_events (struc
 
    /* Walk the various device drivers attached to this slot through
     * a reset sequence, giving each an opportunity to do what it needs
-     * to accomplish the reset.  Each child gets a report of the
-     * status ... if any child can't handle the reset, then the entire
-     * slot is dlpar removed and added.
+     * to accomplish the reset.  Query device driver for special reset
+     * requiements. Report eeh error to each child with cumulative
+     * result status... if any child can't handle the reset,
+     * then the entire slot is dlpar removed and added.
     */
+    pci_walk_bus(frozen_bus, eeh_query_reset_type, &result);
+    if ( result == PCI_ERS_RESULT_FUNDAMENTAL_RESET )
+        reset_type = FUNDAMENTAL_RESET;
+
+    result = PCI_ERS_RESULT_NONE;
    pci_walk_bus(frozen_bus, eeh_report_error, &result);
 
    /* Get the current PCI slot state. This can take a long time,

@@ -425,7 +463,8 @@ struct pci_dn * handle_eeh_events (struc
     * go down willingly, without panicing the system.
     */
    if (result == PCI_ERS_RESULT_NONE) {
-        rc = eeh_reset_device(frozen_pdn, frozen_bus);
+        rc = eeh_reset_device(frozen_pdn, frozen_bus, reset_type);
+
        if (rc) {
            printk(KERN_WARNING "EEH: Unable to reset, rc=%d\n", rc);
            goto hard_fail;

@@ -466,7 +505,7 @@ struct pci_dn * handle_eeh_events (struc
 
    /* If any device called out for a reset, then reset the slot */
    if (result == PCI_ERS_RESULT_NEED_RESET) {
-        rc = eeh_reset_device(frozen_pdn, NULL);
+        rc = eeh_reset_device(frozen_pdn, NULL, reset_type);
        if (rc) {
            printk(KERN_WARNING "EEH: Cannot reset, rc=%d\n", rc);
            goto hard_fail;

include/linux/pci.h 2009-07-13 16:12:31.000000000 -0700 renamed from include/linux/pci.h 2009-07-13 14:25:37.000000000 -0700 +6 -1

@@ -446,6 +446,9 @@ enum pci_ers_result {
 
    /* Device driver is fully recovered and operational */
    PCI_ERS_RESULT_RECOVERED = (__force pci_ers_result_t) 5,
+
+    /* Device driver requires fundamental reset to recover */
+    PCI_ERS_RESULT_FUNDAMENTAL_RESET = (__force pci_ers_result_t) 6,
 };
 
 /* PCI bus error event callbacks */

@@ -465,6 +468,9 @@ struct pci_error_handlers {
 
    /* Device driver may resume normal operations */
    void (*resume)(struct pci_dev *dev);
+
+    /* PCI slot requires special reset type for recovery */
+    pci_ers_result_t (*reset_type)(struct pci_dev *dev);
 };
 
 /* ---------------------------------------------------------------- */
--- a/arch/powerpc/include/asm/ppc-pci.h    2009-06-09 

20:05:27.000000000 -0700
+++ b/arch/powerpc/include/asm/ppc-pci.h 2009-07-13
16:12:31.000000000 -0700

include/linux/pci.h 2009-07-13 16:12:31.000000000 -0700 renamed from include/linux/pci.h 2009-07-13 14:25:37.000000000 -0700 +4 -1

@@ -90,7 +90,9 @@ int rtas_pci_enable(struct pci_dn *pdn,
 *
 * Returns a non-zero value if the reset failed.
 */
-int rtas_set_slot_reset (struct pci_dn *);
+#define HOT_RESET        1
+#define FUNDAMENTAL_RESET    3
+int rtas_set_slot_reset (struct pci_dn *, int reset_type);
 int eeh_wait_for_slot_status(struct pci_dn *pdn, int max_wait_msecs);
 
 /** --- a/arch/powerpc/platforms/pseries/eeh.c    2009-06-09 20:05:27.000000000 -0700
+++ b/arch/powerpc/platforms/pseries/eeh.c    2009-07-13 

16:27:27.000000000 -0700

include/linux/pci.h 2009-07-13 16:12:31.000000000 -0700 renamed from include/linux/pci.h 2009-07-13 14:25:37.000000000 -0700 +5 -6

@@ -666,7 +666,7 @@ rtas_pci_enable(struct pci_dn *pdn, int
 /**
 * rtas_pci_slot_reset - raises/lowers the pci #RST line
 * @pdn pci device node
- * @state: 1/0 to raise/lower the #RST
+ * @state: 1/3/0 to raise hot-reset/fundamental-reset/lower the #RST
 *
 * Clear the EEH-frozen condition on a slot.  This routine
 * asserts the PCI #RST line if the 'state' argument is '1',

@@ -742,9 +742,9 @@ int pcibios_set_pcie_reset_state(struct
 *  Return 0 if success, else a non-zero value.
 */
 
-static void __rtas_set_slot_reset(struct pci_dn *pdn)
+static void __rtas_set_slot_reset(struct pci_dn *pdn, int reset_type)
 {
-    rtas_pci_slot_reset (pdn, 1);
+    rtas_pci_slot_reset (pdn, reset_type);
 
    /* The PCI bus requires that the reset be held high for at least
     * a 100 milliseconds. We wait a bit longer 'just in case'.  */

@@ -766,13 +766,13 @@ static void __rtas_set_slot_reset(struct
    msleep (PCI_BUS_SETTLE_TIME_MSEC);
 }
 
-int rtas_set_slot_reset(struct pci_dn *pdn)
+int rtas_set_slot_reset(struct pci_dn *pdn, int reset_type)
 {
    int i, rc;
 
    /* Take three shots at resetting the bus */
    for (i=0; i<3; i++) {
-        __rtas_set_slot_reset(pdn);
+        __rtas_set_slot_reset(pdn, reset_type);
 
        rc = eeh_wait_for_slot_status(pdn, PCI_BUS_RESET_WAIT_MSEC);
        if (rc == 0)
--- a/arch/powerpc/platforms/pseries/eeh_driver.c    2009-07-13 

14:25:24.000000000 -0700
+++ b/arch/powerpc/platforms/pseries/eeh_driver.c 2009-07-13
16:39:16.000000000 -0700

include/linux/pci.h 2009-07-13 16:12:31.000000000 -0700 renamed from include/linux/pci.h 2009-07-13 14:25:37.000000000 -0700 +29 -0

@@ -115,6 +115,34 @@ static void eeh_enable_irq(struct pci_de
 
 /* ------------------------------------------------------- */
 /**
+ * eeh_query_reset_type - query each device driver for reset type
+ *
+ * Query each device driver for special reset type if required
+ * merge the device driver responses. Cumulative response
+ * passed back in "userdata".
+ */
+
+static int eeh_query_reset_type(struct pci_dev *dev, void *userdata)
+{
+    enum pci_ers_result rc, *res = userdata;
+    struct pci_driver *driver = dev->driver;
+
+    if (!driver)
+        return 0;
+
+    if (!driver->err_handler ||
+        !driver->err_handler->reset_type)
+        return 0;
+
+    rc = driver->err_handler->reset_type (dev);
+
+    /* A driver that needs a special reset trumps all others */
+    if (rc == PCI_ERS_RESULT_FUNDAMENTAL_RESET ) *res = rc;
+
+    return 0;
+}
+
+/**
 * eeh_report_error - report pci error to each device driver
 * * Report an EEH error to each device driver, collect up and @@ -282,9 
+310,12 @@ static int eeh_report_failure(struct pci

* @pe_dn: pointer to a "Partionable Endpoint" device node.
* This is the top-level structure on which pci
* bus resets can be performed.
+ *
+ * reset_type: some devices may require type other than default hot reset. */

include/linux/pci.h 2009-07-13 16:12:31.000000000 -0700 renamed from include/linux/pci.h 2009-07-13 14:25:37.000000000 -0700 +2 -1

-static int eeh_reset_device (struct pci_dn *pe_dn, struct pci_bus *bus)
+static int eeh_reset_device (struct pci_dn *pe_dn, struct pci_bus *bus,
+                 int reset_type)

{
    struct device_node *dn;
    int cnt, rc;

include/linux/pci.h 2009-07-13 16:12:31.000000000 -0700 renamed from include/linux/pci.h 2009-07-13 14:25:37.000000000 -0700 +14 -6

@@ -298,7 +329,7 @@ static int eeh_reset_device (struct pci_
    /* Reset the pci controller. (Asserts RST#; resets config space).
     * Reconfigure bridges and devices. Don't try to bring the system
     * up if the reset failed for some reason. */
-    rc = rtas_set_slot_reset(pe_dn);
+    rc = rtas_set_slot_reset(pe_dn, reset_type);
    if (rc)
        return rc;
 

@@ -343,6 +374,7 @@ struct pci_dn * handle_eeh_events (struc
    struct pci_dn *frozen_pdn;
    struct pci_bus *frozen_bus;
    int rc = 0;
+    int reset_type = HOT_RESET;
    enum pci_ers_result result = PCI_ERS_RESULT_NONE;
    const char *location, *pci_str, *drv_str;
 

@@ -400,10 +432,16 @@ struct pci_dn * handle_eeh_events (struc
 
    /* Walk the various device drivers attached to this slot through
     * a reset sequence, giving each an opportunity to do what it needs
-     * to accomplish the reset.  Each child gets a report of the
-     * status ... if any child can't handle the reset, then the entire
-     * slot is dlpar removed and added.
+     * to accomplish the reset.  Query device driver for special reset
+     * requiements. Report eeh error to each child with cumulative
+     * result status... if any child can't handle the reset,
+     * then the entire slot is dlpar removed and added.
     */
+    pci_walk_bus(frozen_bus, eeh_query_reset_type, &result);
+    if ( result == PCI_ERS_RESULT_FUNDAMENTAL_RESET )
+        reset_type = FUNDAMENTAL_RESET;
+
+    result = PCI_ERS_RESULT_NONE;
    pci_walk_bus(frozen_bus, eeh_report_error, &result);
 
    /* Get the current PCI slot state. This can take a long time,

@@ -425,7 +463,8 @@ struct pci_dn * handle_eeh_events (struc
     * go down willingly, without panicing the system.
     */
    if (result == PCI_ERS_RESULT_NONE) {
-        rc = eeh_reset_device(frozen_pdn, frozen_bus);
+        rc = eeh_reset_device(frozen_pdn, frozen_bus, reset_type);
+
        if (rc) {
            printk(KERN_WARNING "EEH: Unable to reset, rc=%d\n", rc);
            goto hard_fail;

@@ -466,7 +505,7 @@ struct pci_dn * handle_eeh_events (struc
 
    /* If any device called out for a reset, then reset the slot */
    if (result == PCI_ERS_RESULT_NEED_RESET) {
-        rc = eeh_reset_device(frozen_pdn, NULL);
+        rc = eeh_reset_device(frozen_pdn, NULL, reset_type);
        if (rc) {
            printk(KERN_WARNING "EEH: Cannot reset, rc=%d\n", rc);
            goto hard_fail;

include/linux/pci.h 2009-07-13 16:12:31.000000000 -0700 renamed from include/linux/pci.h 2009-07-13 14:25:37.000000000 -0700 +6 -0

@@ -446,6 +446,9 @@ enum pci_ers_result {
 
    /* Device driver is fully recovered and operational */
    PCI_ERS_RESULT_RECOVERED = (__force pci_ers_result_t) 5,
+
+    /* Device driver requires fundamental reset to recover */
+    PCI_ERS_RESULT_FUNDAMENTAL_RESET = (__force pci_ers_result_t) 6,
 };
 
 /* PCI bus error event callbacks */

@@ -465,6 +468,9 @@ struct pci_error_handlers {
 
    /* Device driver may resume normal operations */
    void (*resume)(struct pci_dev *dev);
+
+    /* PCI slot requires special reset type for recovery */
+    pci_ers_result_t (*reset_type)(struct pci_dev *dev);
 };
 
 /* ---------------------------------------------------------------- */

By default, EEH does what's known as a "hot reset" during error recovery =

a PCI Express device. =C2=A0We've found a case where the device needs a "fundamental reset" to recover properly. =C2=A0The current PCI error reco=

EEH frameworks do not support this distinction.

The attached patch (courtesy of Richard Lary) adds a bit field to pci_dev that indicates whether the device requires a fundamental reset during err=

recovery. =C2=A0This bit can be checked by EEH to determine which reset t=

required.

This patch supersedes the previously submitted patch that implemented a reset type callback.

Please review and let me know of any concerns.

diff -uNrp a/include/linux/pci.h b/include/linux/pci.h
--- a/include/linux/pci.h =C2=A0 =C2=A0 =C2=A0 2009-07-13 14:25:37.000000=

+++ b/include/linux/pci.h =C2=A0 =C2=A0 =C2=A0 2009-07-15 10:25:37.000000=

(quoted diff) +0 -0

@@ -273,6 +273,7 @@ struct pci_dev {

=C2=A0 =C2=A0 =C2=A0 =C2=A0unsigned int =C2=A0 =C2=A0ari_enabled:1; =C2=

=C2=A0 =C2=A0 =C2=A0 =C2=A0unsigned int =C2=A0 =C2=A0is_managed:1; =C2=A0 =C2=A0 =C2=A0 =C2=A0unsigned int =C2=A0 =C2=A0is_pcie:1;
+ =C2=A0 =C2=A0 =C2=A0 unsigned int =C2=A0 =C2=A0fndmntl_rst_rqd:1; /* De=

*/
=C2=A0 =C2=A0 =C2=A0 =C2=A0unsigned int =C2=A0 =C2=A0state_saved:1; =C2=A0 =C2=A0 =C2=A0 =C2=A0unsigned int =C2=A0 =C2=A0is_physfn:1; =C2=A0 =C2=A0 =C2=A0 =C2=A0unsigned int =C2=A0 =C2=A0is_virtfn:1;

2009/7/15 Mike Mason <[email protected]>:

Quoted 1 line

By default, EEH does what's known as a "hot reset" during error

Quoted 2 lines

a PCI Express device.  We've found a case where the device needs a "fundamental reset" to recover properly.  The current PCI error

Quoted 3 lines

EEH frameworks do not support this distinction.

The attached patch (courtesy of Richard Lary) adds a bit field to

Quoted 1 line

that indicates whether the device requires a fundamental reset during

Quoted 1 line

recovery.  This bit can be checked by EEH to determine which reset type

Quoted 6 lines

required.

This patch supersedes the previously submitted patch that implemented a reset type callback.

Please review and let me know of any concerns.

I like this patch a *lot* better .. it is vastly simpler, more direct.

Quoted 8 lines

diff -uNrp a/include/linux/pci.h b/include/linux/pci.h

include/linux/pci.h       2009-07-15 10:25:37.000000000 -0700 renamed from include/linux/pci.h       2009-07-13 14:25:37.000000000 -0700 +0 -0

@@ -273,6 +273,7 @@ struct pci_dev {

       unsigned int    ari_enabled:1;  /* ARI forwarding */ unsigned int    is_managed:1;
       unsigned int    is_pcie:1;
+       unsigned int    fndmntl_rst_rqd:1; /* Dev requires fundamental

Quoted 4 lines

*/
       unsigned int    state_saved:1;
       unsigned int    is_physfn:1;
       unsigned int    is_virtfn:1;

As Ben points out, the name is awkward. How about needs_freset ?

Since this affects the entire pci subsystem, it should be documented properly. The "pci error recovery" subsystem was designed to be usable in other architectures, and so the error recovery docs should take at least a paragraph to describe what this flag means, and when its supposed to be used.

Providing the docs patch together with the pci.h patch *only* would probably simplify acceptance by the PCI community.

--linas

2009/7/15 Mike Mason <[email protected]>:

Quoted 1 line

By default, EEH does what's known as a "hot reset" during error

Quoted 2 lines

a PCI Express device.  We've found a case where the device needs a "fundamental reset" to recover properly.  The current PCI error

Quoted 3 lines

EEH frameworks do not support this distinction.

The attached patch (courtesy of Richard Lary) adds a bit field to

Quoted 1 line

that indicates whether the device requires a fundamental reset during

Quoted 1 line

recovery.  This bit can be checked by EEH to determine which reset type

Quoted 6 lines

required.

This patch supersedes the previously submitted patch that implemented a reset type callback.

Please review and let me know of any concerns.

I like this patch a *lot* better .. it is vastly simpler, more direct.

Quoted 8 lines

diff -uNrp a/include/linux/pci.h b/include/linux/pci.h

include/linux/pci.h       2009-07-15 10:25:37.000000000 -0700 renamed from include/linux/pci.h       2009-07-13 14:25:37.000000000 -0700 +0 -0

@@ -273,6 +273,7 @@ struct pci_dev {

       unsigned int    ari_enabled:1;  /* ARI forwarding */ unsigned int    is_managed:1;
       unsigned int    is_pcie:1;
+       unsigned int    fndmntl_rst_rqd:1; /* Dev requires fundamental

Quoted 4 lines

*/
       unsigned int    state_saved:1;
       unsigned int    is_physfn:1;
       unsigned int    is_virtfn:1;

As Ben points out, the name is awkward. How about needs_freset ?

Since this affects the entire pci subsystem, it should be documented properly. The "pci error recovery" subsystem was designed to be usable in other architectures, and so the error recovery docs should take at least a paragraph to describe what this flag means, and when its supposed to be used.

Providing the docs patch together with the pci.h patch *only* would probably simplify acceptance by the PCI community.

--linas

Linas Vepstas <[email protected]> wrote on 07/23/2009 07:44:33 AM:

Quoted 2 lines

2009/7/15 Mike Mason <[email protected]>:

Quoted 1 line

By default, EEH does what's known as a "hot reset" during error recove=

Quoted 2 lines

Quoted 2 lines

of
a PCI Express device. =C2=A0We've found a case where the device needs =

Quoted 1 line

Quoted 1 line

"fundamental reset" to recover properly. =C2=A0The current PCI error r=

Quoted 8 lines

Quoted 8 lines

and
EEH frameworks do not support this distinction.

The attached patch (courtesy of Richard Lary) adds a bit field to pci_dev
that indicates whether the device requires a fundamental reset during error
recovery. =C2=A0This bit can be checked by EEH to determine which rese=

Quoted 4 lines

Quoted 4 lines

is
required.

This patch supersedes the previously submitted patch that implemented =

Quoted 9 lines

Quoted 3 lines

reset type callback.

Please review and let me know of any concerns.

I like this patch a *lot* better .. it is vastly simpler, more direct.

Quoted 2 lines

diff -uNrp a/include/linux/pci.h b/include/linux/pci.h
--- a/include/linux/pci.h =C2=A0 =C2=A0 =C2=A0 2009-07-13 14:25:37.000=

Quoted 1 line

Quoted 1 line

+++ b/include/linux/pci.h =C2=A0 =C2=A0 =C2=A0 2009-07-15 10:25:37.000=

Quoted 2 lines

Quoted 2 lines

(quoted diff) +0 -0

@@ -273,6 +273,7 @@ struct pci_dev {

=C2=A0 =C2=A0 =C2=A0 =C2=A0unsigned int =C2=A0 =C2=A0ari_enabled:1; =

Quoted 3 lines

Quoted 3 lines

=C2=A0 =C2=A0 =C2=A0 =C2=A0unsigned int =C2=A0 =C2=A0is_managed:1; =C2=A0 =C2=A0 =C2=A0 =C2=A0unsigned int =C2=A0 =C2=A0is_pcie:1;
+ =C2=A0 =C2=A0 =C2=A0 unsigned int =C2=A0 =C2=A0fndmntl_rst_rqd:1; /*=

Quoted 7 lines

Quoted 5 lines

reset
*/
=C2=A0 =C2=A0 =C2=A0 =C2=A0unsigned int =C2=A0 =C2=A0state_saved:1; =C2=A0 =C2=A0 =C2=A0 =C2=A0unsigned int =C2=A0 =C2=A0is_physfn:1; =C2=A0 =C2=A0 =C2=A0 =C2=A0unsigned int =C2=A0 =C2=A0is_virtfn:1;

As Ben points out, the name is awkward. =C2=A0How about needs_freset ?

I am OK with name change.

Quoted 5 lines

Since this affects the entire pci subsystem, it should be documented properly. =C2=A0The "pci error recovery" subsystem was designed to be usable in other architectures, and so the error recovery docs should take at least a paragraph to describe what this flag means, and when its supposed to be used.

I will update the documentation, are you referring to Documentation/powerpc/eeh-pci-error-recovery.txt or some other documentation?

Quoted 5 lines

Providing the docs patch together with the pci.h patch *only* would probably simplify acceptance by the PCI community.

--linas

2009/7/24 Richard Lary <[email protected]>:

Quoted 4 lines

Linas Vepstas <[email protected]> wrote on 07/23/2009 07:44:33 AM:

Quoted 2 lines

2009/7/15 Mike Mason <[email protected]>:

Quoted 1 line

By default, EEH does what's known as a "hot reset" during error

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

of
a PCI Express device.  We've found a case where the device needs a "fundamental reset" to recover properly.  The current PCI error

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

and
EEH frameworks do not support this distinction.

The attached patch (courtesy of Richard Lary) adds a bit field to pci_dev
that indicates whether the device requires a fundamental reset

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

error
recovery.  This bit can be checked by EEH to determine which reset

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

is
required.

This patch supersedes the previously submitted patch that

Quoted 7 lines

Quoted 7 lines

Quoted 3 lines

reset type callback.

Please review and let me know of any concerns.

I like this patch a *lot* better .. it is vastly simpler, more direct.

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

diff -uNrp a/include/linux/pci.h b/include/linux/pci.h

include/linux/pci.h       2009-07-15 10:25:37.000000000 -0700 renamed from include/linux/pci.h       2009-07-13 14:25:37.000000000 -0700 +0 -0

@@ -273,6 +273,7 @@ struct pci_dev {

       unsigned int    ari_enabled:1;  /* ARI forwarding */ unsigned int    is_managed:1;
       unsigned int    is_pcie:1;
+       unsigned int    fndmntl_rst_rqd:1; /* Dev requires

Quoted 17 lines

Quoted 7 lines

Quoted 5 lines

reset
*/
       unsigned int    state_saved:1;
       unsigned int    is_physfn:1;
       unsigned int    is_virtfn:1;

As Ben points out, the name is awkward.  How about needs_freset ?

I am OK with name change.

Quoted 5 lines

Since this affects the entire pci subsystem, it should be documented properly.  The "pci error recovery" subsystem was designed to be usable in other architectures, and so the error recovery docs should take at least a paragraph to describe what this flag means, and when its supposed to be used.

Quoted 3 lines

I will update the documentation, are you referring to Documentation/powerpc/eeh-pci-error-recovery.txt or some other documentation?

No, I'm thinking
Documentation/PCI/pci-error-recovery.txt

because the flag is not powerpc-specific.

Quoted 6 lines

Quoted 5 lines

Providing the docs patch together with the pci.h patch *only* would probably simplify acceptance by the PCI community.

--linas