Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753178Ab3EUNjK (ORCPT ); Tue, 21 May 2013 09:39:10 -0400 Received: from mail-pb0-f47.google.com ([209.85.160.47]:60201 "EHLO mail-pb0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752400Ab3EUNjI (ORCPT ); Tue, 21 May 2013 09:39:08 -0400 MIME-Version: 1.0 Reply-To: linasvepstas@gmail.com In-Reply-To: <1369122596.4432.22.camel@ymzhang.sh.intel.com> References: <1369122596.4432.22.camel@ymzhang.sh.intel.com> From: Linas Vepstas Date: Tue, 21 May 2013 08:38:47 -0500 Message-ID: Subject: Re: Subject : [ PATCH ] pci-reset-error_state-to-pci_channel_io_normal-at-report_slot_reset To: yanmin_zhang Cc: "Liu, Joseph" , Bjorn Helgaas , "Zhang, LongX" , "linux-pci@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "Rafael J. Wysocki" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2010 Lines: 43 Hi, On 21 May 2013 02:49, Yanmin Zhang 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/