Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Cc:     <helgaas@kernel.org>, <oohall@gmail.com>,
        <gregkh@linuxfoundation.org>, <mr.nuke.me@gmail.com>,
        <linux-pci@vger.kernel.org>, <Austin.Bolen@dell.com>,
        <Shyam.Iyer@dell.com>, <linux-kernel@vger.kernel.org>,
        <jonathan.derrick@intel.com>, <lukas@wunner.de>,
        <ruscur@russell.cc>, <sbobroff@linux.ibm.com>,
        <linuxppc-dev@lists.ozlabs.org>
From:   <Alex_Gagniuc@Dellteam.com>
To:     <keith.busch@intel.com>
Subject: Re: [PATCH v2] PCI/MSI: Don't touch MSI bits when the PCI device is
 disconnected
Thread-Topic: [PATCH v2] PCI/MSI: Don't touch MSI bits when the PCI device is
 disconnected
Thread-Index: AQHUT50UQ290XPGvJEGnaSTTjbCF+g==
Date:   Wed, 14 Nov 2018 20:52:10 +0000
Message-ID: <644fd16cf02c4fe5b7e250c226c80f2e@ausx13mps321.AMER.DELL.COM>
References: <20181108224255.GA20619@kroah.com>
 <20d68e586fff4dcca5616d5056f6fc21@ausx13mps321.AMER.DELL.COM>
 <20181108225109.GA3023@kroah.com>
 <16bf9d14bc5f4a90b2b88dd2eb165186@ausx13mps321.AMER.DELL.COM>
 <5da8d8aa9f3818af649b1ac547bc4e6062626ddf.camel@gmail.com>
 <df85813c9860463d85f6c302dfe07b12@ausx13mps321.AMER.DELL.COM>
 <20181113050240.GA182139@google.com>
 <19136f44cd5c45e79bbef7e78a6bf332@ausx13mps321.AMER.DELL.COM>
 <20181114055956.GA144931@google.com>
 <1eb0fa27924f426992715684f5e63346@ausx13mps321.AMER.DELL.COM>
 <20181114202333.GE11416@localhost.localdomain>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 11/14/2018 02:27 PM, Keith Busch wrote:=0A=
> On Wed, Nov 14, 2018 at 07:22:04PM +0000, Alex_Gagniuc@Dellteam.com wrote=
:=0A=
>> On 11/14/2018 12:00 AM, Bjorn Helgaas wrote:=0A=
>>> Just to make sure we're on the same page, can you point me to this=0A=
>>> rule?  I do see that OSPM must request control of AER using _OSC=0A=
>>> before it touches the AER registers.  What I don't see is the=0A=
>>> connection between firmware-first and the AER registers.=0A=
>>=0A=
>> ACPI 6.2 - 6.2.11.3, Table 6-197:=0A=
>>[...]=0A=
>> Maybe Keith knows better why we're doing it this way. From ACPI text, it=
=0A=
>> doesn't seem that control of AER would be tied to HEST entries, although=
=0A=
>> in practice, it is.=0A=
> =0A=
> I'm not sure, that predates me.  HEST does have a FIRMWARE_FIRST flag, bu=
t=0A=
> spec does not say anymore on relation to _OSC control or AER capability.=
=0A=
> Nothing in PCIe spec either.=0A=
=0A=
Speaking to one of the PCIe (and _HPX type 3) spec authors, ownership of =
=0A=
AER should be determined by _OSC. period. The result of _OSC applies to =0A=
every device under the root port. This crap we do with checking HEST is =0A=
crap.=0A=
=0A=
If I'm not stepping on anyone toes, and there's no known unintended =0A=
consequences, I can look at patching this up. I'm not promising a patch, =
=0A=
though, but it's exactly the sort of thing I like to fix.=0A=
=0A=
> I also don't know why Linux disables the AER driver if only one=0A=
> device has a FIRMWARE_FIRST HEST. Shouldn't that just be a per-device=0A=
> decision?=0A=
=0A=
I think the logic is if one HEST entry has both FFS and GLOBAL flags =0A=
set, then then disable AER services for all devices. It works in =0A=
practice better than it works in theory. I think _OSC should be the =0A=
determining factor here, not HEST.=0A=
=0A=
>>> The closest I can find is the "Enabled" field in the HEST PCIe=0A=
>>> AER structures (ACPI v6.2, sec 18.3.2.4, .5, .6), where it says:=0A=
>>> [...]=0A=
>>> AFAICT, Linux completely ignores the Enabled field in these=0A=
>>> structures.=0A=
>>=0A=
>> I don't think ignoring the field is a problem:=0A=
>>    * With FFS, OS should ignore it.=0A=
>>    * Without FFS, we have control, and we get to make the decisions anyw=
ay.=0A=
>> In the latter case we decide whether to use AER, independent of the crap=
=0A=
>> in ACPI. I'm not even sure why "Enabled" matters in native AER handling.=
=0A=
>> Probably one of the check-boxes in "Binary table designer's handbook"?=
=0A=
> =0A=
> And why doesn't Linux do anything with _OSC response other than logging=
=0A=
> it? If OS control wasn't granted, shouldn't that take priority over HEST?=
=0A=
=0A=
But it does in portdrv_core.c:=0A=
=0A=
	if (dev->aer_cap && pci_aer_available() &&=0A=
	    (pcie_ports_native || host->native_aer)) {=0A=
		services |=3D PCIE_PORT_SERVICE_AER;=0A=
=0A=
That flag later creates a pcie device that allows aerdrv to attach to.=0A=
=0A=
Alex=0A=