Received-SPF: pass (google.com: domain of linux-kernel+bounces-85861-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <f446dfda-53e1-4e65-bbe5-0488028f55ad@linux.intel.com>
Date: Wed, 28 Feb 2024 16:00:06 -0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 0/3] PCI/DPC: Clean up DPC vs AER/EDR ownership and
 Kconfig
Content-Language: en-US
To: Ethan Zhao <haifeng.zhao@linux.intel.com>,
 Bjorn Helgaas <helgaas@kernel.org>, linux-pci@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, Matthew W Carlis <mattc@purestorage.com>,
 Keith Busch <kbusch@kernel.org>, Lukas Wunner <lukas@wunner.de>,
 Mika Westerberg <mika.westerberg@linux.intel.com>,
 Jesse Brandeburg <jesse.brandeburg@intel.com>,
 Bjorn Helgaas <bhelgaas@google.com>
References: <20240222221521.32159-1-helgaas@kernel.org>
 <89984f11-b84e-4da0-ab5b-f2048461aae0@linux.intel.com>
 <52245cb8-879e-4997-a1b5-cdfbd702dee7@linux.intel.com>
 <81f2dfa4-14c6-423c-96ee-b11b8a4670ed@linux.intel.com>
From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
In-Reply-To: <81f2dfa4-14c6-423c-96ee-b11b8a4670ed@linux.intel.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit


On 2/26/24 11:12 PM, Ethan Zhao wrote:
> On 2/27/2024 2:35 PM, Kuppuswamy Sathyanarayanan wrote:
>> Hi,
>>
>> On 2/26/24 10:18 PM, Ethan Zhao wrote:
>>> On 2/23/2024 6:15 AM, Bjorn Helgaas wrote:
>>>> From: Bjorn Helgaas <bhelgaas@google.com>
>>>>
>>>> Previously we could request control of DPC without AER, which is illegal
>>>> per spec.  Also, we could enable CONFIG_PCIE_DPC without CONFIG_PCIE_EDR,
>>>> which is also illegal.  This series addresses both.
>>> I have a question here, how to understand the relationship EDR & AER ?
>>> somewhere EDR touches AER status without checking _OSC granted bits,
>>> such as
>>>     pci_aer_raw_clear_status(edev);
>>
>> Which_OSC bits?
>>
>> EDR code will only get triggered if OS advertises the EDR support (which
>> also means OS supports AER and DPC), and both AER and DPC is owned by
>> the firmware. During the EDR notification, the OS is allowed to touch AER
>
> Means no need to check if host->native_aer ? why checked in
> pcie_do_recovery() ?

pcie_do_recovery() can be called form EDR (FF) or native path. That check is used
to skip device status clearing in FF mode. You can find details about it in

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/pci/pcie/err.c?id=068c29a248b6ddbfdf7bb150b547569759620d36


>
> Thanks,
> Ethan
>
>> and DPC registers. So there is no problem with EDR code using AER routines.
>>
>>
>>> sometimes EDR calling AER with host->native_aer checked, like
>>>
>>> pcie_do_recovery()
>>> {
>>>   ...
>>>   if (host->native_aer || pcie_ports_native) {
>>>          pcie_clear_device_status(dev);
>>>          pci_aer_clear_nonfatal_status(dev);
>>>      }
>>>   ...
>>> }
>>>
>>> That is really confusing. could we do some cleanup to eliminate it ?
>>> such as seperate AER code into common code and runtime part.
>>>
>>>
>>> Thanks,
>>> Ethan
>>>  
>>>> Bjorn Helgaas (3):
>>>>     PCI/DPC: Request DPC only if also requesting AER
>>>>     PCI/DPC: Remove CONFIG_PCIE_EDR
>>>>     PCI/DPC: Encapsulate pci_acpi_add_edr_notifier()
>>>>
>>>>    drivers/acpi/pci_root.c   | 22 ++++++++++++----------
>>>>    drivers/pci/pci.h         |  4 ++++
>>>>    drivers/pci/pcie/Kconfig  | 14 ++++----------
>>>>    drivers/pci/pcie/Makefile |  5 ++++-
>>>>    drivers/pci/pcie/dpc.c    | 10 ----------
>>>>    include/linux/pci-acpi.h  |  8 --------
>>>>    6 files changed, 24 insertions(+), 39 deletions(-)
>>>>
>
-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer