From patchwork Sat Sep 19 05:27:03 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yinghai Lu X-Patchwork-Id: 519626 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id A18FC140338 for ; Sat, 19 Sep 2015 15:27:08 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b=BmmnP+op; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752238AbbISF1H (ORCPT ); Sat, 19 Sep 2015 01:27:07 -0400 Received: from mail-vk0-f51.google.com ([209.85.213.51]:33393 "EHLO mail-vk0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751343AbbISF1G (ORCPT ); Sat, 19 Sep 2015 01:27:06 -0400 Received: by vkgd64 with SMTP id d64so41011949vkg.0 for ; Fri, 18 Sep 2015 22:27:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=NFB+zPFpX/RitNH/vmBv/hAnEqYSQRyemKhhYL82VsA=; b=BmmnP+opzWwSIA0uVPNHm7sUE1RVS+TXsjwfer1kLYJ1FkEeQLGZXVHqD005FBUgbc Yw8eMx9rTw0U3JM6WsHBGI8Mquh3DB4/W7VnD+G2wkaEx1WHf7TyNMILq/os0UcjzKnp ug3MqV2MXdSCXQx9si/nhjeqhhoDTpgiv/jAaMXf4pWTZUbyeEj4qWJ+y94v78RYEAat SJD2XGD4fM2hYFa1m5+H1dkn9Ye4p6s+SaiX/WZTD4KUT6SzgzHQqOxOZ5C5iElFXq3S TDdLvWRmSvhH14HrVqIlKttFpv4JRF8UQM6oioOJjDI4RlpsuEoHKGsId6BpPwxU44tZ tn8w== MIME-Version: 1.0 X-Received: by 10.31.0.215 with SMTP id 206mr868990vka.105.1442640423367; Fri, 18 Sep 2015 22:27:03 -0700 (PDT) Received: by 10.103.45.72 with HTTP; Fri, 18 Sep 2015 22:27:03 -0700 (PDT) In-Reply-To: <20150918223059.GO25767@google.com> References: <1441877628-28879-1-git-send-email-lorenzo.pieralisi@arm.com> <20150918223059.GO25767@google.com> Date: Fri, 18 Sep 2015 22:27:03 -0700 X-Google-Sender-Auth: yJ9CDNT_PX4IMGLCLjeA80dxFgs Message-ID: Subject: Re: [PATCH] PCI: Fix pci_claim_bridge_resource() resource claiming From: Yinghai Lu To: Bjorn Helgaas Cc: Lorenzo Pieralisi , "linux-pci@vger.kernel.org" Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org On Fri, Sep 18, 2015 at 3:30 PM, Bjorn Helgaas wrote: > On Thu, Sep 10, 2015 at 10:33:48AM +0100, Lorenzo Pieralisi wrote: > > I wasn't sure about the provenance of your patch, Lorenzo. You had a > Signed-off-by from Yinghai, but I didn't see the original posting. If > it originally came from Yinghai, I should change the > "Based-on-patch-by" below. Lorenzo posted v1 that removed second claim. -- >8 -- Subject: [PATCH] PCI: remove dead code in pci_claim_bridge_resource() Commit 8505e729a2f6eb ("PCI: Add pci_claim_bridge_resource() to clip window if necessary") introduced a new API to claim bridge resources. pci_claim_bridge_resource() tries to claim a bridge resource, and if the claiming fails the function tries to clip the resource to make it fit within the parent resource window. If the clipping succeeds the bridge apertures are set-up accordingly and the pci_claim_bridge_resource() tries to claim the resource again. Commit c770cb4cb505 ("PCI: Mark invalid BARs as unassigned") added code that sets the IORESOURCE_UNSET flag on claiming failure. This means that the second resource claiming after window clipping will always fail, since the resource flags contain IORESOURCE_UNSET, previously set on failure by pci_claim_resource(), so the subsequent pci_claim_resource() call ends up spitting a log message and return failure with no chance whatsoever to succeed. This patch removes the second pci_claim_resource() call in pci_claim_bridge_resource() since it basically has no chance to succeed, leaving the current behaviour unchanged. Signed-off-by: Lorenzo Pieralisi Cc: Bjorn Helgaas Cc: Yinghai Lu --- drivers/pci/setup-bus.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) > > commit a4ad03352739c96842af5d06387595665cdd875e > Author: Bjorn Helgaas > Date: Fri Sep 18 17:15:01 2015 -0500 > > PCI: Clear IORESOURCE_UNSET when clipping a bridge window > > c770cb4cb505 ("PCI: Mark invalid BARs as unassigned") sets IORESOURCE_UNSET > if we fail to claim a resource. If we tried to claim a bridge window, > failed, clipped the window, and tried to claim the clipped window, we > failed again because of IORESOURCE_UNSET. > > When pci_bus_clip_resource() clips a bridge window to fit inside an > upstream window, we're reassigning the window, so clear the > IORESOURCE_UNSET flag. Also clear IORESOURCE_UNSET in our copy of the > unclipped window so we can see exactly what the original window was and how > it now fits inside the upstream window. > > Fixes: c770cb4cb505 ("PCI: Mark invalid BARs as unassigned") > Based-on-patch-by: Lorenzo Pieralisi > Signed-off-by: Bjorn Helgaas > CC: stable@vger.kernel.org # 4.1+ > > diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c > index 6fbd3f2..d3346d2 100644 > --- a/drivers/pci/bus.c > +++ b/drivers/pci/bus.c > @@ -256,6 +256,8 @@ bool pci_bus_clip_resource(struct pci_dev *dev, int idx) > > res->start = start; > res->end = end; > + res->flags &= ~IORESOURCE_UNSET; > + orig_res.flags &= ~IORESOURCE_UNSET; > dev_printk(KERN_DEBUG, &dev->dev, "%pR clipped to %pR\n", > &orig_res, res); > Yes that should be ok too. 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/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c index 508cc56..2bf4ac1 100644 --- a/drivers/pci/setup-bus.c +++ b/drivers/pci/setup-bus.c @@ -728,14 +728,10 @@ int pci_claim_bridge_resource(struct pci_dev *bridge, int i) break; case 2: pci_setup_bridge_mmio_pref(bridge); - break; default: - return -EINVAL; + break; } - if (pci_claim_resource(bridge, i) == 0) - return 0; /* claimed a smaller window */ - return -EINVAL; } And I suggested to clear UNSET instead, and posted the patch but I had the From : Lorenzo to keep him have to authorship as he noticed the problem. From: Lorenzo Pieralisi Subject: [PATCH] PCI: Fix clipped bridge resource reserve Commit 8505e729a2f6eb ("PCI: Add pci_claim_bridge_resource() to clip window if necessary") introduced a new API to claim bridge resources. pci_claim_bridge_resource() tries to claim a bridge resource, and if the claiming fails the function tries to clip the resource to make it fit within the parent resource window. If the clipping succeeds the bridge apertures are set-up accordingly and the pci_claim_bridge_resource() tries to claim the resource again. Commit c770cb4cb505 ("PCI: Mark invalid BARs as unassigned") added code that sets the IORESOURCE_UNSET flag on claiming failure. This means that the second resource claiming after window clipping will always fail, since the resource flags contain IORESOURCE_UNSET, previously set on failure by pci_claim_resource(), so the subsequent pci_claim_resource() call ends up spitting a log message and return failure with no chance whatsoever to succeed. This patch clear the UNSET in resource flags, and it makes second pci_claim_resource() work again. Fixes: c770cb4cb505 ("PCI: Mark invalid BARs as unassigned") Cc: stable@vger.kernel.org [v4.1+] [change to clear UNSET instead, Yinghai] Signed-off-by: Yinghai Lu diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c index 508cc56..76b3349 100644 --- a/drivers/pci/setup-bus.c +++ b/drivers/pci/setup-bus.c @@ -733,6 +733,7 @@ int pci_claim_bridge_resource(struct pci_dev *bridge, int i) return -EINVAL; } + bridge->resource[i].flags &= ~IORESOURCE_UNSET; if (pci_claim_resource(bridge, i) == 0) return 0; /* claimed a smaller window */