Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1423086AbbD2Nxl (ORCPT ); Wed, 29 Apr 2015 09:53:41 -0400 Received: from mga11.intel.com ([192.55.52.93]:8407 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1422821AbbD2Nxg (ORCPT ); Wed, 29 Apr 2015 09:53:36 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.11,671,1422950400"; d="scan'208";a="563673194" Message-ID: <5540E259.4040105@linux.intel.com> Date: Wed, 29 Apr 2015 21:53:29 +0800 From: Jiang Liu Organization: Intel User-Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: "Rafael J. Wysocki" , Bjorn Helgaas CC: Ingo Molnar , Thomas Gleixner , "H. Peter Anvin" , "x86@kernel.org" , Len Brown , Lv Zheng , LKML , "linux-pci@vger.kernel.org" , "linux-acpi@vger.kernel.org" Subject: Re: [Bugfix v5] x86/PCI/ACPI: Fix regression caused by commit 63f1789ec716 References: <1429499338-16967-1-git-send-email-jiang.liu@linux.intel.com> <5540DD9C.9030707@linux.intel.com> <2290160.KqiWSgJfFU@vostro.rjw.lan> In-Reply-To: <2290160.KqiWSgJfFU@vostro.rjw.lan> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5347 Lines: 114 On 2015/4/29 22:15, Rafael J. Wysocki wrote: > On Wednesday, April 29, 2015 09:33:16 PM Jiang Liu wrote: >> On 2015/4/29 21:20, Bjorn Helgaas wrote: >>> On Tue, Apr 28, 2015 at 7:40 PM, Rafael J. Wysocki wrote: >>>> On Monday, April 20, 2015 11:08:58 AM Jiang Liu wrote: >>>>> An IO port or MMIO resource assigned to a PCI host bridge may be >>>>> consumed by the host bridge itself or available to its child >>>>> bus/devices. On x86 and IA64 platforms, all IO port and MMIO >>>>> resources are assumed to be available to child bus/devices >>>>> except one special case: >>>>> IO port [0xCF8-0xCFF] is consumed by the host bridge itself >>>>> to access PCI configuration space. >>>>> >>>>> But the ACPI and PCI Firmware specifications haven't provided a method >>>>> to tell whether a resource is consumed by the host bridge itself. >>>>> So before commit 593669c2ac0f ("x86/PCI/ACPI: Use common ACPI resource >>>>> interfaces to simplify implementation"), arch/x86/pci/acpi.c ignored >>>>> all IO port resources defined by acpi_resource_io and >>>>> acpi_resource_fixed_io to filter out IO ports consumed by the host >>>>> bridge itself. >>>>> >>>>> Commit 593669c2ac0f ("x86/PCI/ACPI: Use common ACPI resource interfaces >>>>> to simplify implementation")started accepting all IO port and MMIO >>>>> resources, which caused a regression that IO port resources consumed >>>>> by the host bridge itself became available to its child devices. >>>>> >>>>> Then commit 63f1789ec716 ("x86/PCI/ACPI: Ignore resources consumed by >>>>> host bridge itself") ignored resources consumed by the host bridge >>>>> itself by checking the IORESOURCE_WINDOW flag, which accidently removed >>>>> MMIO resources defined by acpi_resource_memory24, acpi_resource_memory32 >>>>> and acpi_resource_fixed_memory32. >>>>> >>>>> So revert to the behavior before v3.19 to fix the regression. >>>>> >>>>> There is also a discussion about ignoring the Producer/Consumer flag on >>>>> IA64 platforms at: >>>>> http://patchwork.ozlabs.org/patch/461633/ >>>>> >>>>> Related ACPI table are archived at: >>>>> https://bugzilla.kernel.org/show_bug.cgi?id=94221 >>>>> >>>>> Fixes: 63f1789ec716("Ignore resources consumed by host bridge itself") >>>>> Reported-by: Bernhard Thaler >>>>> Signed-off-by: Jiang Liu >>>> >>>> Bjorn, Ingo, is anyone looking at this? We're still having a regression in >>>> this area ... >>> >>>>> --- >>>>> arch/x86/pci/acpi.c | 25 ++++++++++++++++++++++--- >>>>> drivers/acpi/resource.c | 6 +++++- >>>>> 2 files changed, 27 insertions(+), 4 deletions(-) >>>>> >>>>> diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c >>>>> index e4695985f9de..fc2da98985c3 100644 >>>>> --- a/arch/x86/pci/acpi.c >>>>> +++ b/arch/x86/pci/acpi.c >>>>> @@ -332,12 +332,32 @@ static void probe_pci_root_info(struct pci_root_info *info, >>>>> { >>>>> int ret; >>>>> struct resource_entry *entry, *tmp; >>>>> + unsigned long res_flags; >>>>> >>>>> sprintf(info->name, "PCI Bus %04x:%02x", domain, busnum); >>>>> info->bridge = device; >>>>> + >>>>> + /* >>>>> + * An IO or MMIO resource assigned to PCI host bridge may be consumed >>>>> + * by the host bridge itself or available to its child bus/devices. >>>>> + * On x86 and IA64 platforms, all IO and MMIO resources are assumed to >>>>> + * be available to child bus/devices except one special case: >>>>> + * IO port [0xCF8-0xCFF] is consumed by host bridge itself to >>>>> + * access PCI configuration space. >>>>> + * >>>>> + * Due to lack of specification to define resources consumed by host >>>>> + * bridge itself, all IO port resources defined by acpi_resource_io >>>>> + * and acpi_resource_fixed_io are ignored to filter out IO >>>>> + * port[0xCF8-0xCFF]. Seems this solution works with all BIOSes, though >>>>> + * it's not perfect. >>> >>> 1) I think it's misleading to say "the specs haven't provided a >>> method." As far as I can tell, the Producer/Consumer bit is intended >>> precisely to distinguish resources consumed by a bridge from those >>> forwarded to downstream devices. It would be more accurate to say >>> "the spec defines a bit, but firmware hasn't used that bit >>> consistently, so we can't rely on it." >> >> Hi Bjorn, >> Thanks for review, I will refine the words as suggested by you. >> >>> If you want to say "it's not perfect," it would be useful to mention >>> the ways in which it is not perfect. This code is still a candidate >>> for unification with ia64 and arm64, so we should avoid x86-specific >>> things here as much as possible. >> >> Yes, I have another pending patch set to consolidate IA64 and x86 code >> for ACPI PCI root. > > That's OK, but can we please fix the regression first before doing that > unification? Like to make life easier for the "stable" people and > whoever wants to backport the fix? Hi Rafael, Yes, I will fix the regression first before sending out the pending patch set. Thanks! Gerry -- 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/