From patchwork Fri Dec 13 23:06:57 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bjorn Helgaas X-Patchwork-Id: 3343691 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: X-Original-To: patchwork-linux-pci@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id AC4B49F37A for ; Fri, 13 Dec 2013 23:07:19 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id B1C4020703 for ; Fri, 13 Dec 2013 23:07:18 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 9B5FE20705 for ; Fri, 13 Dec 2013 23:07:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753609Ab3LMXHF (ORCPT ); Fri, 13 Dec 2013 18:07:05 -0500 Received: from mail-ie0-f182.google.com ([209.85.223.182]:40779 "EHLO mail-ie0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752825Ab3LMXHC (ORCPT ); Fri, 13 Dec 2013 18:07:02 -0500 Received: by mail-ie0-f182.google.com with SMTP id as1so3817172iec.27 for ; Fri, 13 Dec 2013 15:07:01 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=RQYWImFfB+j/k5Cx+mY/2HOpe2Qh4FISwTk+bV5X2Gk=; b=Wd8Jg1jYzXBoCjuUUYM7uBEJkPkLi0NceQanj5nK4k9ONx0751BR4SqGDIZIEZkR+a HpkSh5oKj4iYUZcK4kDlI4zouPioUhBXVDjOHf91H1W1OQzhz4Oda3kPAQR1cM7FKPVo NW90Z9cKPMcaYjkpMk29bF5ZgJU/kMqz0/150o2lBuIAeeHrCeFuVg/Dl8IC+ZxtXMec 2XLgb2KuaTcLTVsPOJNfJmVRfYzpyXElwllSs3dTWdwFjD0oYqajFLoh7768AjPgjYsW 9CRX0KL7vUpHD376hMAJ2rxaS9LcvRIx+d0udaMXpJdqL8XCgM6IOCOsytLnXvpQFZEv Bcmg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-type:content-disposition:in-reply-to :user-agent; bh=RQYWImFfB+j/k5Cx+mY/2HOpe2Qh4FISwTk+bV5X2Gk=; b=WAUejEDyUk4XKLyXbKHcLwYYZfJEY0f1dOsbRJWcHj9XG9lZUOjJe+H4cqgG+PfeXh AF1Ge+o7LfmdQtNvvBoovlQA/d9d2nhO8UcEgaTlVGFLTu33sGyaOuanEpLkc2MSQM6y 1CwNJXjiE5B5bkqcyHlO2fwiKe2Z2TZ8NHPEMofORXjCMlC/YR4CKMEZ5k29bRKMMTHD ckTgC6bHRz16KvXu9smwXyPyJTWx2L6g5p4exFnXL2y3SuJs6Qziw2FE4gxRLtw4z+qR avOI5l5ru6nqj/nDt88txQnSgUSFZzV83xY2LqhOmScBjZj/xMhvJil85whBub9R2DaH PyYg== X-Gm-Message-State: ALoCoQmKY2CJOp6Iq64/MaAFJhWvhEjq+eACsbLmoXHHgrPuAUXYIJgvEB8sB/GUStqEr76TW3m7sdyWI7jqCv4LkuuPp5rdRtxiNlw/BMMBB+jsx6p/orFRHfr3bgrCHKqtugGm/7NgO3GCJx+jcB60OsgkfliBuAIJx3IMdImsFS/RmFSNVjwL+0a0LDucrY8Mz5JjaPLkwmxGzxsYJ/SA55riysgH3A== X-Received: by 10.43.52.129 with SMTP id vm1mr3957359icb.10.1386976021350; Fri, 13 Dec 2013 15:07:01 -0800 (PST) Received: from google.com ([172.16.48.173]) by mx.google.com with ESMTPSA id s4sm11288820ige.0.2013.12.13.15.06.59 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Fri, 13 Dec 2013 15:07:00 -0800 (PST) Date: Fri, 13 Dec 2013 16:06:57 -0700 From: Bjorn Helgaas To: Betty Dall Cc: lenb@kernel.org, rjw@rjwysocki.net, linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org Subject: Re: [PATCH] PCI AER: Handle non-AER HEST error sources in aer_hest_parse() Message-ID: <20131213230657.GD6746@google.com> References: <1386948196-9409-1-git-send-email-betty.dall@hp.com> <20131213221616.GC6746@google.com> <1386974853.10497.64.camel@ejdallLaptop> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1386974853.10497.64.camel@ejdallLaptop> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Spam-Status: No, score=-5.8 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FSL_HELO_FAKE, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Fri, Dec 13, 2013 at 03:47:33PM -0700, Betty Dall wrote: > Yes, that is more readable code. Thanks for revising it. I tested it on > my system that has non-AER error sources and it works fine. One nit is > that "Ignore" is misspelled in the subject line. Thanks, fixed. I should have thought longer before hitting send. I think it's worthwhile to use the same helper in aer_hest_parse_aff(), and once I did that, it became more obvious that aer_hest_parse() and aer_hest_parse_aff() are essentially similar, so I wonder if it's worth consolidating them, as below. It doesn't reduce the number of lines of code, but maybe it's simpler for the reader? Bjorn commit 8e7f8d0b30d4e3e30007b10eb2916d970b5f8c93 Author: Betty Dall Date: Fri Dec 13 08:23:16 2013 -0700 PCI/AER: Ignore non-PCIe AER error sources in aer_hest_parse() aer_set_firmware_first() searches the HEST for an error source descriptor matching the specified PCI device. It uses the apei_hest_parse() iterator to call aer_hest_parse() for every descriptor in the HEST. Previously, aer_hest_parse() incorrectly assumed every descriptor was for a PCIe error source. This patch adds a check to avoid that error. [bhelgaas: factor check into helper, use in aer_hest_parse_aff(), changelog] Signed-off-by: Betty Dall Signed-off-by: Bjorn Helgaas --- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/pci/pcie/aer/aerdrv_acpi.c b/drivers/pci/pcie/aer/aerdrv_acpi.c index cf611ab2193a..a23995749f1d 100644 --- a/drivers/pci/pcie/aer/aerdrv_acpi.c +++ b/drivers/pci/pcie/aer/aerdrv_acpi.c @@ -50,12 +50,24 @@ struct aer_hest_parse_info { int firmware_first; }; +static int hest_source_is_pcie_aer(struct acpi_hest_header *hest_hdr) +{ + if (hest_hdr->type == ACPI_HEST_TYPE_AER_ROOT_PORT || + hest_hdr->type == ACPI_HEST_TYPE_AER_ENDPOINT || + hest_hdr->type == ACPI_HEST_TYPE_AER_BRIDGE) + return 1; + return 0; +} + static int aer_hest_parse(struct acpi_hest_header *hest_hdr, void *data) { struct aer_hest_parse_info *info = data; struct acpi_hest_aer_common *p; int ff; + if (!hest_source_is_pcie_aer(hest_hdr)) + return 0; + p = (struct acpi_hest_aer_common *)(hest_hdr + 1); ff = !!(p->flags & ACPI_HEST_FIRMWARE_FIRST); if (p->flags & ACPI_HEST_GLOBAL) { @@ -104,15 +116,12 @@ static int aer_hest_parse_aff(struct acpi_hest_header *hest_hdr, void *data) if (aer_firmware_first) return 0; - switch (hest_hdr->type) { - case ACPI_HEST_TYPE_AER_ROOT_PORT: - case ACPI_HEST_TYPE_AER_ENDPOINT: - case ACPI_HEST_TYPE_AER_BRIDGE: - p = (struct acpi_hest_aer_common *)(hest_hdr + 1); - aer_firmware_first = !!(p->flags & ACPI_HEST_FIRMWARE_FIRST); - default: + if (!hest_source_is_pcie_aer(hest_hdr)) return 0; - } + + p = (struct acpi_hest_aer_common *)(hest_hdr + 1); + aer_firmware_first = !!(p->flags & ACPI_HEST_FIRMWARE_FIRST); + return 0; } /** commit 01600a1b034f93dab6f855c81626b8030b85aec0 Author: Bjorn Helgaas Date: Fri Dec 13 15:42:53 2013 -0700 PCI/AER: Consolidate HEST error source parsers aer_hest_parse() and aer_hest_parse_aff() are almost identical. We use aer_hest_parse() to check the ACPI_HEST_FIRMWARE_FIRST flag for a specific device, and we use aer_hest_parse_aff() to check to see if any device sets the flag. This drops aer_hest_parse_aff() and enhances aer_hest_parse() so it collects the union of the ACPI_HEST_FIRMWARE_FIRST flag setting when no specific device is supplied. No functional change. Signed-off-by: Bjorn Helgaas diff --git a/drivers/pci/pcie/aer/aerdrv_acpi.c b/drivers/pci/pcie/aer/aerdrv_acpi.c index a23995749f1d..4d6991794fa2 100644 --- a/drivers/pci/pcie/aer/aerdrv_acpi.c +++ b/drivers/pci/pcie/aer/aerdrv_acpi.c @@ -70,6 +70,17 @@ static int aer_hest_parse(struct acpi_hest_header *hest_hdr, void *data) p = (struct acpi_hest_aer_common *)(hest_hdr + 1); ff = !!(p->flags & ACPI_HEST_FIRMWARE_FIRST); + + /* + * If no specific device is supplied, determine whether + * FIRMWARE_FIRST is set for *any* PCIe device. + */ + if (!info->pci_dev) { + info->firmware_first |= ff; + return 0; + } + + /* Otherwise, check the specific device */ if (p->flags & ACPI_HEST_GLOBAL) { if (hest_match_type(hest_hdr, info->pci_dev)) info->firmware_first = ff; @@ -109,30 +120,20 @@ int pcie_aer_get_firmware_first(struct pci_dev *dev) static bool aer_firmware_first; -static int aer_hest_parse_aff(struct acpi_hest_header *hest_hdr, void *data) -{ - struct acpi_hest_aer_common *p; - - if (aer_firmware_first) - return 0; - - if (!hest_source_is_pcie_aer(hest_hdr)) - return 0; - - p = (struct acpi_hest_aer_common *)(hest_hdr + 1); - aer_firmware_first = !!(p->flags & ACPI_HEST_FIRMWARE_FIRST); - return 0; -} - /** * aer_acpi_firmware_first - Check if APEI should control AER. */ bool aer_acpi_firmware_first(void) { static bool parsed = false; + struct aer_hest_parse_info info = { + .pci_dev = NULL, /* Check all PCIe devices */ + .firmware_first = 0, + }; if (!parsed) { - apei_hest_parse(aer_hest_parse_aff, NULL); + apei_hest_parse(aer_hest_parse, &info); + aer_firmware_first = info.firmware_first; parsed = true; } return aer_firmware_first;