From patchwork Fri Jul 26 17:39:56 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Bjorn Helgaas X-Patchwork-Id: 2834298 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 8F7EC9F4D4 for ; Fri, 26 Jul 2013 17:40:06 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 5B5BA20273 for ; Fri, 26 Jul 2013 17:40:05 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 17AD520257 for ; Fri, 26 Jul 2013 17:40:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759216Ab3GZRkC (ORCPT ); Fri, 26 Jul 2013 13:40:02 -0400 Received: from mail-ob0-f177.google.com ([209.85.214.177]:56720 "EHLO mail-ob0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758935Ab3GZRkA (ORCPT ); Fri, 26 Jul 2013 13:40:00 -0400 Received: by mail-ob0-f177.google.com with SMTP id ta17so4810199obb.22 for ; Fri, 26 Jul 2013 10:40:00 -0700 (PDT) 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:content-transfer-encoding :in-reply-to:user-agent; bh=UitXHi6VZGre3z3PppSPkNFTX2JUgNQfOWWuuGy+4So=; b=ka5OVwR2DclIzeriOR92OPQk54HOP6R5v9jtyIF2nHL+UwyCATWS9l7loS1BQFwL3I kQTuzHhd25+Nz6dvkYukc16D+wAdbxFXLsNpfxAmARSk1dL2699ZNx9riBXMiL5kTQFh q99zaWvq2J+nXJOfdPnuNHoJsQzDGxjm8PWeD6mIbTBuW9bdMB+a9EfyJDbFmbde3V6v 3GJnTpN4amE3p6eGBrfNd/6kHmHLWCZiHuutkQOuPTEy/NXnUYgEs7MJ0kUh4V4ZyzZD dH+ueRoMBg9RPC1IoUNbtsauq3gd8UmC4l66HbKDb8UknihbvCN/gk+JjegPm9xqeCMT iVTA== X-Google-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:content-transfer-encoding :in-reply-to:user-agent:x-gm-message-state; bh=UitXHi6VZGre3z3PppSPkNFTX2JUgNQfOWWuuGy+4So=; b=QkvjveLaSHxXudBqIGJ4MFfrU6U176Qpg8irxSAW83zhPexLHdFimyFCmQMiolWX7X EMS6DrpWqK39TZl6cprPV7+uFtv/pGqM1RYIs1K95o7bP2EOkiLQl0Ky+dRNomA8IPnM RCI+F+yef4Bfr9I/F61hOBkc+k1y/5+VKf3HPpXOgR5Mw8HA9NAOnlEAGdiGMqgXnAJa TNiqFPb9XYBgPdqIhjD/SgPvRO97oOFIn7aNYpTzBLwXmUN7v3lwRHwCr0KqLoXVCBOB Xtruoj+Z7cw5HyE5T3bWvV7EvhLog6liUj7y3n9RXZD2wxWlrvu2lkBNnkmwEzjhbgvJ SmyA== X-Received: by 10.182.27.102 with SMTP id s6mr41438549obg.42.1374860400204; Fri, 26 Jul 2013 10:40:00 -0700 (PDT) Received: from google.com ([172.16.55.224]) by mx.google.com with ESMTPSA id q8sm35723146obl.11.2013.07.26.10.39.58 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Fri, 26 Jul 2013 10:39:59 -0700 (PDT) Date: Fri, 26 Jul 2013 11:39:56 -0600 From: Bjorn Helgaas To: ethan zhao Cc: jbarnes@virtuousgeek.org, yinghai@kernel.org, linux-kernel@vger.kernel.org, ethan.kernel@gmail.com, Jiang Liu , linux-pci@vger.kernel.org Subject: Re: [PATCH] x86/PCI: MMCONFIG: cleanup and add address warning to pci_mmconfig_insert Message-ID: <20130726173956.GA14078@google.com> References: <51F23D0F.7090909@oracle.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <51F23D0F.7090909@oracle.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-Gm-Message-State: ALoCoQm69rwlCVlpgEcoBg+pCtMvXWYvH4+Rrd47geogJL9oNBuaFgBngtzkRneCHMZfmKHUJw+7u1ttlbS+/KAryAJ6KFMkA2a+sgUGwGvzGyswoGa5AnaZite3w43wOmz9o2CbRo50+Us2Yhg2X+p5LxfYADeWQdGRdRj3e4k/WnTF/NjiInuWKYyqHIaDukb74VGRe4y1tD/9EMq+FAYP1Q1jL3/mng== Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Spam-Status: No, score=-8.2 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, 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 [+cc Jiang, linux-pci, -cc bjorn.helgaas@hp.com (dead address)] On Fri, Jul 26, 2013 at 05:10:39PM +0800, ethan zhao wrote: > Cleanup the -EINVAL return value handling and add warning message > for invalid > start,end,addr parameters. > > Signed-off-by: ethan.zhao This patch was corrupted, so I couldn't apply it directly.  See Documentation/SubmittingPatches, sections 5-7. > --- > arch/x86/pci/mmconfig-shared.c | 12 ++++++------ > 1 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c > index 082e881..37f6c7f 100644 > --- a/arch/x86/pci/mmconfig-shared.c > +++ b/arch/x86/pci/mmconfig-shared.c > @@ -700,8 +700,13 @@ int pci_mmconfig_insert(struct device *dev, u16 > seg, u8 start, u8 end, > if (!(pci_probe & PCI_PROBE_MMCONF) || pci_mmcfg_arch_init_failed) > return -ENODEV; > > - if (start > end) > + if (start > end || !addr) { > + dev_warn(dev, FW_WARN > + "Invalid address to add MMCONFIG" > + "start %02x end %02x addr %pR\n", > + start, end, addr); > return -EINVAL; > + } I like the "!addr" cleanup. Did you actually see this problem on a machine? I expect this would be a BIOS bug, and one that should be found early in development, long before a machine is released. Therefore, I doubt that it's worth adding another printk for it. If an end-user sees this problem, I think we'll already get a generic message from check_segment(). I propose the patch below; what do you think? > > mutex_lock(&pci_mmcfg_lock); > cfg = pci_mmconfig_lookup(seg, start); > @@ -716,11 +721,6 @@ int pci_mmconfig_insert(struct device *dev, u16 > seg, u8 start, u8 end, > return -EEXIST; > } > > - if (!addr) { > - mutex_unlock(&pci_mmcfg_lock); > - return -EINVAL; > - } > - > rc = -EBUSY; > cfg = pci_mmconfig_alloc(seg, start, end, addr); > if (cfg == NULL) { Author: ethan.zhao Date: Fri Jul 26 11:21:24 2013 -0600 x86/PCI: MMCONFIG: Check earlier for MMCONFIG region at address zero We can check for addr being zero earlier and thus avoid the mutex_unlock() cleanup path. [bhelgaas: changelog, drop printk] Signed-off-by: ethan.zhao Signed-off-by: Bjorn Helgaas Acked-by: Yinghai Lu Acked-by: Yinghai Lu --- 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/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c index 082e881..5596c7b 100644 --- a/arch/x86/pci/mmconfig-shared.c +++ b/arch/x86/pci/mmconfig-shared.c @@ -700,7 +700,7 @@ int pci_mmconfig_insert(struct device *dev, u16 seg, u8 start, u8 end, if (!(pci_probe & PCI_PROBE_MMCONF) || pci_mmcfg_arch_init_failed) return -ENODEV; - if (start > end) + if (start > end || !addr) return -EINVAL; mutex_lock(&pci_mmcfg_lock); @@ -716,11 +716,6 @@ int pci_mmconfig_insert(struct device *dev, u16 seg, u8 start, u8 end, return -EEXIST; } - if (!addr) { - mutex_unlock(&pci_mmcfg_lock); - return -EINVAL; - } - rc = -EBUSY; cfg = pci_mmconfig_alloc(seg, start, end, addr); if (cfg == NULL) {