From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932160AbVHHR6n (ORCPT ); Mon, 8 Aug 2005 13:58:43 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S932167AbVHHR6n (ORCPT ); Mon, 8 Aug 2005 13:58:43 -0400 Received: from fmr18.intel.com ([134.134.136.17]:12758 "EHLO orsfmr003.jf.intel.com") by vger.kernel.org with ESMTP id S932160AbVHHR6m (ORCPT ); Mon, 8 Aug 2005 13:58:42 -0400 Subject: Re: [PATCH] 6700/6702PXH quirk From: Kristen Accardi To: Bjorn Helgaas Cc: linux-pci@atrey.karlin.mff.cuni.cz, Jeff Garzik , Greg KH , linux-kernel@vger.kernel.org, rajesh.shah@intel.com In-Reply-To: <200508081036.36903.bjorn.helgaas@hp.com> References: <1123259263.8917.9.camel@whizzy> <20050805225005.GA16155@havoc.gtf.org> <1123285907.4706.19.camel@whizzy> <200508081036.36903.bjorn.helgaas@hp.com> Content-Type: text/plain Content-Transfer-Encoding: 7bit Date: Mon, 08 Aug 2005 10:57:12 -0700 Message-Id: <1123523832.13928.15.camel@whizzy> Mime-Version: 1.0 X-Mailer: Evolution 2.0.4 (2.0.4-4) X-OriginalArrivalTime: 08 Aug 2005 17:57:14.0176 (UTC) FILETIME=[9B925400:01C59C42] Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 2005-08-08 at 10:36 -0600, Bjorn Helgaas wrote: > On Friday 05 August 2005 5:51 pm, Kristen Accardi wrote: > > On the 6700/6702 PXH part, a MSI may get corrupted if an ACPI hotplug > > driver and SHPC driver in MSI mode are used together. This patch will > > prevent MSI from being enabled for the SHPC as part of an early pci > > quirk, as well as on any pci device which sets the no_msi bit. > > For mailing list archaeology, I assume this is erratum 15 in the > 6700/6702 PXH spec update: > http://download.intel.com/design/chipsets/specupdt/30270609.pdf > > which says > > An MSI is generated by the standard hot-plug controller may > get corrupted in presence of another ACPI hot-plug driver. > The ACPI driver performs configuration reads of DWSEL/DWORD > register in order to determine the hot-plug capability of all > the ACPI devices. If the MSI is generated by the Standard > Hot-Plug Controller (SHPC) in this time period, there is a > possibility of the MSI getting corrupted. As a result the > MSI may not get issued upstream to the MCH. The above is a > result of interaction of separate events that are unpredict- > able. That's correct. > > So what still bugs me about this (and I'm probably just showing my > ignorance here), is that we seem to have two drivers (SHPC and ACPI) > poking at the same hardware. Why is this? > > And where exactly is the ACPI code that is involved? I see shpc_init() > doing config reads of DWORD_DATA, but I don't see how ACPI is involved. > Is there some AML that's doing the config accesses? Why would there > be AML if we're using SHPC? > > > @@ -699,6 +699,9 @@ int pci_enable_msi(struct pci_dev* dev) > > if (!pci_msi_enable || !dev) > > return status; > > > > + if (dev->no_msi) > > + return status; > > + > I am just learning this stuff as well, so hopefully someone will correct me if I'm wrong. This seems like a poorly worded erratum. The acpiphp driver does not actual do any config reads - it just asks the acpi core to read the acpi namespace to determine the hotplug capabilities. I will try to find out more about the test case that they used to discover this problem and get someone to explain it to me in english. > Is there any reason not to fold this into the test above it? > No, it seems that patches done at 4:45 on Friday don't turn out optimally :). > > +static void __devinit quirk_pcie_pxh(struct pci_dev *dev) > > +{ > > + disable_msi_mode(dev, pci_find_capability(dev, PCI_CAP_ID_MSI), > > + PCI_CAP_ID_MSI); > > Is this even needed? You're doing early fixups, which happen before > any drivers touch the device, so you should only need to disable MSI > if the BIOS can leave it enabled. But I would have thought MSI would > be disabled until a driver explicitly enables it. This was me being paranoid. I was concerned that some BIOS might decide to enable by default, so I was just trying to make really really sure that MSI would be turned off. Think that's overkill?