Re: [PATCH v2 1/1] PCI: pciehp: Skip DLLSC handling if DPC is triggered
Hi, On Wed, Mar 17, 2021 at 10:45 AM Dan Williams wrote: > > On Wed, Mar 17, 2021 at 10:20 AM Sathyanarayanan Kuppuswamy Natarajan > wrote: > > > > Hi, > > > > On Wed, Mar 17, 2021 at 9:31 AM Dan Williams > > wrote: > > > > > > On Tue, Mar 16, 2021 at 10:31 PM Lukas Wunner wrote: > > > > > > > > On Tue, Mar 16, 2021 at 10:08:31PM -0700, Dan Williams wrote: > > > > > On Tue, Mar 16, 2021 at 9:14 PM Lukas Wunner wrote: > > > > > > > > > > > > On Fri, Mar 12, 2021 at 07:32:08PM -0800, > > > > > > sathyanarayanan.kuppusw...@linux.intel.com wrote: > > > > > > > + if ((events == PCI_EXP_SLTSTA_DLLSC) && > > > > > > > is_dpc_reset_active(pdev)) { > > > > > > > + ctrl_info(ctrl, "Slot(%s): DLLSC event(DPC), > > > > > > > skipped\n", > > > > > > > + slot_name(ctrl)); > > > > > > > + ret = IRQ_HANDLED; > > > > > > > + goto out; > > > > > > > + } > > > > > > > > > > > > Two problems here: > > > > > > > > > > > > (1) If recovery fails, the link will *remain* down, so there'll be > > > > > > no Link Up event. You've filtered the Link Down event, thus the > > > > > > slot will remain in ON_STATE even though the device in the slot > > > > > > is > > > > > > no longer accessible. That's not good, the slot should be > > > > > > brought > > > > > > down in this case. > > > > > > > > > > Can you elaborate on why that is "not good" from the end user > > > > > perspective? From a driver perspective the device driver context is > > > > > lost and the card needs servicing. The service event starts a new > > > > > cycle of slot-attention being triggered and that syncs the slot-down > > > > > state at that time. > > > > > > > > All of pciehp's code assumes that if the link is down, the slot must be > > > > off. A slot which is in ON_STATE for a prolonged period of time even > > > > though the link is down is an oddity the code doesn't account for. > > > > > > > > If the link goes down, the slot should be brought into OFF_STATE. > > > > (It's okay though to delay bringdown until DPC recovery has completed > > > > unsuccessfully, which is what the patch I'm proposing does.) > > > > > > > > I don't understand what you mean by "service event". Someone unplugging > > > > and replugging the NVMe drive? > > > > > > Yes, service meaning a technician physically removes the card. > > > > > > > > > > > > > > > > > (2) If recovery succeeds, there's a race where pciehp may call > > > > > > is_dpc_reset_active() *after* dpc_reset_link() has finished. > > > > > > So both the DPC Trigger Status bit as well as > > > > > > pdev->dpc_reset_active > > > > > > will be cleared. Thus, the Link Up event is not filtered by > > > > > > pciehp > > > > > > and the slot is brought down and back up even though DPC > > > > > > recovery > > > > > > was succesful, which seems undesirable. > > > > > > > > > > The hotplug driver never saw the Link Down, so what does it do when > > > > > the slot transitions from Link Up to Link Up? Do you mean the Link > > > > > Down might fire after the dpc recovery has completed if the hotplug > > > > > notification was delayed? > > > > > > > > If the Link Down is filtered and the Link Up is not, pciehp will > > > > bring down the slot and then bring it back up. That's because pciehp > > > > can't really tell whether a DLLSC event is Link Up or Link Down. > > > > > > > > It just knows that the link was previously up, is now up again, > > > > but must have been down intermittently, so transactions to the > > > > device in the slot may have been lost and the slot is therefore > > > > brought down for safety. Because the link is up, it is then > > > > brought back up. > > > > > > I wonder why we're not seeing that effect in testing? > > > > In our test case, there is a good chance that the LINK UP event is also > > filtered. We change the dpc_reset_active status only after we verify > > the link is up. So if hotplug handler handles the LINK UP event before > > we change the status of dpc_reset_active, then it will not lead to the > > issue mentioned by Lukas. > > > > Ah, ok, we're missing a flush of the hotplug event handler after the > link is up to make sure the hotplug handler does not see the Link Up. Flush of hotplug event after successful recovery, and a simulated hotplug link down event after link recovery fails should solve the problems raised by Lukas. I assume Lukas' proposal adds this support. I will check his patch shortly. > I'm not immediately seeing how the new proposal ensures that there is > no Link Up event still in flight after DPC completes its work. > Wouldn't it be required to throw away Link Up to Link Up transitions? -- Sathyanarayanan Kuppuswamy Linux Kernel Developer
Re: [PATCH v2 1/1] PCI: pciehp: Skip DLLSC handling if DPC is triggered
Hi, On Wed, Mar 17, 2021 at 9:31 AM Dan Williams wrote: > > On Tue, Mar 16, 2021 at 10:31 PM Lukas Wunner wrote: > > > > On Tue, Mar 16, 2021 at 10:08:31PM -0700, Dan Williams wrote: > > > On Tue, Mar 16, 2021 at 9:14 PM Lukas Wunner wrote: > > > > > > > > On Fri, Mar 12, 2021 at 07:32:08PM -0800, > > > > sathyanarayanan.kuppusw...@linux.intel.com wrote: > > > > > + if ((events == PCI_EXP_SLTSTA_DLLSC) && > > > > > is_dpc_reset_active(pdev)) { > > > > > + ctrl_info(ctrl, "Slot(%s): DLLSC event(DPC), skipped\n", > > > > > + slot_name(ctrl)); > > > > > + ret = IRQ_HANDLED; > > > > > + goto out; > > > > > + } > > > > > > > > Two problems here: > > > > > > > > (1) If recovery fails, the link will *remain* down, so there'll be > > > > no Link Up event. You've filtered the Link Down event, thus the > > > > slot will remain in ON_STATE even though the device in the slot is > > > > no longer accessible. That's not good, the slot should be brought > > > > down in this case. > > > > > > Can you elaborate on why that is "not good" from the end user > > > perspective? From a driver perspective the device driver context is > > > lost and the card needs servicing. The service event starts a new > > > cycle of slot-attention being triggered and that syncs the slot-down > > > state at that time. > > > > All of pciehp's code assumes that if the link is down, the slot must be > > off. A slot which is in ON_STATE for a prolonged period of time even > > though the link is down is an oddity the code doesn't account for. > > > > If the link goes down, the slot should be brought into OFF_STATE. > > (It's okay though to delay bringdown until DPC recovery has completed > > unsuccessfully, which is what the patch I'm proposing does.) > > > > I don't understand what you mean by "service event". Someone unplugging > > and replugging the NVMe drive? > > Yes, service meaning a technician physically removes the card. > > > > > > > > > (2) If recovery succeeds, there's a race where pciehp may call > > > > is_dpc_reset_active() *after* dpc_reset_link() has finished. > > > > So both the DPC Trigger Status bit as well as pdev->dpc_reset_active > > > > will be cleared. Thus, the Link Up event is not filtered by pciehp > > > > and the slot is brought down and back up even though DPC recovery > > > > was succesful, which seems undesirable. > > > > > > The hotplug driver never saw the Link Down, so what does it do when > > > the slot transitions from Link Up to Link Up? Do you mean the Link > > > Down might fire after the dpc recovery has completed if the hotplug > > > notification was delayed? > > > > If the Link Down is filtered and the Link Up is not, pciehp will > > bring down the slot and then bring it back up. That's because pciehp > > can't really tell whether a DLLSC event is Link Up or Link Down. > > > > It just knows that the link was previously up, is now up again, > > but must have been down intermittently, so transactions to the > > device in the slot may have been lost and the slot is therefore > > brought down for safety. Because the link is up, it is then > > brought back up. > > I wonder why we're not seeing that effect in testing? In our test case, there is a good chance that the LINK UP event is also filtered. We change the dpc_reset_active status only after we verify the link is up. So if hotplug handler handles the LINK UP event before we change the status of dpc_reset_active, then it will not lead to the issue mentioned by Lukas. if (!pcie_wait_for_link(pdev, true)) { pci_info(pdev, "Data Link Layer Link Active not set in 1000 msec\n"); - return PCI_ERS_RESULT_DISCONNECT; + status = PCI_ERS_RESULT_DISCONNECT; } - return PCI_ERS_RESULT_RECOVERED; + atomic_dec_return_release(>dpc_reset_active); -- Sathyanarayanan Kuppuswamy Linux Kernel Developer
[PATCH v2 1/1] PCI: pciehp: Skip DLLSC handling if DPC is triggered
From: Kuppuswamy Sathyanarayanan When hotplug and DPC are both enabled on a Root port or Downstream Port, during DPC events that cause a DLLSC link down/up events, such events (DLLSC) must be suppressed to let the DPC driver own the recovery path. When DPC is present and enabled, hardware will put the port in containment state to allow SW to recover from the error condition in the seamless manner. But, during the DPC error recovery process, since the link is in disabled state, it will also raise the DLLSC event. In Linux kernel architecture, DPC events are handled by DPC driver and DLLSC events are handled by hotplug driver. If a hotplug driver is allowed to handle such DLLSC event (triggered by DPC containment), then we will have a race condition between error recovery handler (in DPC driver) and hotplug handler in recovering the contained port. Allowing such a race leads to a lot of stability issues while recovering the device. So skip DLLSC handling in the hotplug driver when the PCIe port associated with the hotplug event is in DPC triggered state and let the DPC driver be responsible for the port recovery. Following is the sample dmesg log which shows the contention between hotplug handler and error recovery handler. In this case, hotplug handler won the race and error recovery handler reported failure. pcieport :97:02.0: pciehp: Slot(4): Link Down pcieport :97:02.0: DPC: containment event, status:0x1f01 source:0x pcieport :97:02.0: DPC: unmasked uncorrectable error detected pcieport :97:02.0: PCIe Bus Error: severity=Uncorrected (Non-Fatal), type=Transaction Layer, (Requester ID) pcieport :97:02.0: device [8086:347a] error status/mask=4000/00100020 pcieport :97:02.0:[14] CmpltTO(First) pci :98:00.0: AER: can't recover (no error_detected callback) pcieport :97:02.0: pciehp: Slot(4): Card present pcieport :97:02.0: DPC: Data Link Layer Link Active not set in 1000 msec pcieport :97:02.0: AER: subordinate device reset failed pcieport :97:02.0: AER: device recovery failed pci :98:00.0: [8086:0953] type 00 class 0x010802 nvme nvme1: pci function :98:00.0 nvme :98:00.0: enabling device (0140 -> 0142) nvme nvme1: 31/0/0 default/read/poll queues nvme1n2: p1 Signed-off-by: Kuppuswamy Sathyanarayanan Reviewed-by: Dan Williams Reviewed-by: Raj Ashok --- drivers/pci/hotplug/pciehp_hpc.c | 19 + drivers/pci/pci.h| 2 ++ drivers/pci/pcie/dpc.c | 36 ++-- include/linux/pci.h | 1 + 4 files changed, 56 insertions(+), 2 deletions(-) diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c index fb3840e222ad..55da5208c7e5 100644 --- a/drivers/pci/hotplug/pciehp_hpc.c +++ b/drivers/pci/hotplug/pciehp_hpc.c @@ -691,6 +691,25 @@ static irqreturn_t pciehp_ist(int irq, void *dev_id) goto out; } + /* +* If the DLLSC link up/down event is generated due to DPC containment +* in the PCIe port, skip the DLLSC event handling and let the DPC +* driver own the port recovery. Allowing both hotplug DLLSC event +* handler and DPC event trigger handler to attempt recovery on the +* same port leads to stability issues. If DPC recovery is successful, +* is_dpc_reset_active() will return false and the hotplug handler will +* not suppress the DLLSC event. If DPC recovery fails and the link is +* left in disabled state, once the user changes the faulty card, the +* hotplug handler can still handle the PRESENCE change event and bring +* the device back up. +*/ + if ((events == PCI_EXP_SLTSTA_DLLSC) && is_dpc_reset_active(pdev)) { + ctrl_info(ctrl, "Slot(%s): DLLSC event(DPC), skipped\n", + slot_name(ctrl)); + ret = IRQ_HANDLED; + goto out; + } + /* Check Attention Button Pressed */ if (events & PCI_EXP_SLTSTA_ABP) { ctrl_info(ctrl, "Slot(%s): Attention button pressed\n", diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index ef7c4661314f..cee7095483bd 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -446,10 +446,12 @@ void pci_restore_dpc_state(struct pci_dev *dev); void pci_dpc_init(struct pci_dev *pdev); void dpc_process_error(struct pci_dev *pdev); pci_ers_result_t dpc_reset_link(struct pci_dev *pdev); +bool is_dpc_reset_active(struct pci_dev *pdev); #else static inline void pci_save_dpc_state(struct pci_dev *dev) {} static inline void pci_restore_dpc_state(struct pci_dev *dev) {} static inline void pci_dpc_init(struct pci_dev *pdev) {} +static inline bool is_dpc_reset_active(struct pci_dev *pdev) { return false; } #endif #ifdef CONFIG_PCIEPORTBUS diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index e05aba86a317..9157d70ebe21 100644 --- a/drivers/pci/pcie/dpc.c +++
[PATCH v1 1/1] PCI: pciehp: Skip DLLSC handling if DPC is triggered
From: Kuppuswamy Sathyanarayanan When hotplug and DPC are both enabled on a Root port or Downstream Port, during DPC events that cause a DLLSC link down/up events, such events must be suppressed to let the DPC driver own the recovery path. When DPC is present and enabled, hardware will put the port in containment state to allow SW to recover from the error condition in the seamless manner. But, during the DPC error recovery process, since the link is in disabled state, it will also raise the DLLSC event. In Linux kernel architecture, DPC events are handled by DPC driver and DLLSC events are handled by hotplug driver. If a hotplug driver is allowed to handle such DLLSC event (triggered by DPC containment), then we will have a race condition between error recovery handler (in DPC driver) and hotplug handler in recovering the contained port. Allowing such a race leads to a lot of stability issues while recovering the device. So skip DLLSC handling in the hotplug driver when the PCIe port associated with the hotplug event is in DPC triggered state and let the DPC driver be responsible for the port recovery. Following is the sample dmesg log which shows the contention between hotplug handler and error recovery handler. In this case, hotplug handler won the race and error recovery handler reported failure. [ 724.974237] pcieport :97:02.0: pciehp: Slot(4): Link Down [ 724.974266] pcieport :97:02.0: DPC: containment event, status:0x1f01 source:0x [ 724.974269] pcieport :97:02.0: DPC: unmasked uncorrectable error detected [ 724.974275] pcieport :97:02.0: PCIe Bus Error: severity=Uncorrected (Non-Fatal), type=Transaction Layer, (Requester ID) [ 724.974283] pcieport :97:02.0: device [8086:347a] error status/mask=4000/00100020 [ 724.974288] pcieport :97:02.0:[14] CmpltTO(First) [ 724.999181] pci :98:00.0: AER: can't recover (no error_detected callback) [ 724.999227] pci :98:00.0: Removing from iommu group 181 [ 726.063125] pcieport :97:02.0: pciehp: Slot(4): Card present [ 726.221117] pcieport :97:02.0: DPC: Data Link Layer Link Active not set in 1000 msec [ 726.221122] pcieport :97:02.0: AER: subordinate device reset failed [ 726.221162] pcieport :97:02.0: AER: device recovery failed [ 727.227176] pci :98:00.0: [8086:0953] type 00 class 0x010802 [ 727.227202] pci :98:00.0: reg 0x10: [mem 0x-0x3fff 64bit] [ 727.227234] pci :98:00.0: reg 0x30: [mem 0x-0x pref] [ 727.227246] pci :98:00.0: Max Payload Size set to 256 (was 128, max 256) [ 727.227251] pci :98:00.0: enabling Extended Tags [ 727.227736] pci :98:00.0: Adding to iommu group 181 [ 727.231150] pci :98:00.0: BAR 6: assigned [mem 0xd100-0xd100 pref] [ 727.231156] pci :98:00.0: BAR 0: assigned [mem 0xd101-0xd1013fff 64bit] [ 727.231170] pcieport :97:02.0: PCI bridge to [bus 98] [ 727.231174] pcieport :97:02.0: bridge window [io 0xc000-0xcfff] [ 727.231181] pcieport :97:02.0: bridge window [mem 0xd100-0xd10f] [ 727.231186] pcieport :97:02.0: bridge window [mem 0x2060-0x2060001f 64bit pref] [ 727.231555] nvme nvme1: pci function :98:00.0 [ 727.231581] nvme :98:00.0: enabling device (0140 -> 0142) [ 737.141132] nvme nvme1: 31/0/0 default/read/poll queues [ 737.146211] nvme1n2: p1 Signed-off-by: Kuppuswamy Sathyanarayanan Reviewed-by: Dan Williams Reviewed-by: Raj Ashok --- drivers/pci/hotplug/pciehp_hpc.c | 18 + drivers/pci/pci.h| 2 ++ drivers/pci/pcie/dpc.c | 33 ++-- include/linux/pci.h | 1 + 4 files changed, 52 insertions(+), 2 deletions(-) diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c index fb3840e222ad..8e7916abc60e 100644 --- a/drivers/pci/hotplug/pciehp_hpc.c +++ b/drivers/pci/hotplug/pciehp_hpc.c @@ -691,6 +691,24 @@ static irqreturn_t pciehp_ist(int irq, void *dev_id) goto out; } + /* +* If the DLLSC link up/down event is generated due to DPC containment +* in the PCIe port, skip the DLLSC event handling and let the DPC driver +* own the port recovery. Allowing both hotplug DLLSC event handler and DPC +* event trigger handler attempt recovery on the same port leads to stability +* issues. if DPC recovery is successful, is_dpc_reset_active() will return +* false and the hotplug handler will not suppress the DLLSC event. If DPC +* recovery fails and the link is left in disabled state, once the user +* changes the faulty card, the hotplug handler can still handle the PRESENCE +* change event and bring the device back up. +*/ + if ((events == PCI_EXP_SLTSTA_DLLSC) && is_dpc_reset_active(pdev)) { + ctrl_info(ctrl, "Slot(%s): DLLSC event(DPC), skipped\n", +
Test Email
From: Kuppuswamy Sathyanarayanan Hi All, Sending a test email to verify my mail server. please ignore it. -- 2.25.1
Re: [PATCH v4 2/2] PCI/ERR: Split the fatal and non-fatal error recovery handling
On Wed, Oct 14, 2020 at 11:43 PM Christoph Hellwig wrote: > > On Tue, Oct 13, 2020 at 08:17:39AM -0700, Kuppuswamy, Sathyanarayanan wrote: > > > > > > On 10/13/20 4:56 AM, Christoph Hellwig wrote: > > > You might want to split out pcie_do_fatal_recovery and get rid of the > > > state argument: > > This is how it was before Keith merged fatal and non-fatal error recovery > > paths. When the comparison is between additional-parameter vs new-interface > > , I choose the former. But I can merge your change in next version. > > But now you split the implementation. Keith merged made complete sense > when the code was mostly identical. But now that the code is separate > again it doesn't make sense to hide it under a common interface that > uses a flags value to call different functions. Agreed. Already included this change in v6.
[PATCH v9 3/5] ACPI/PCI: Ignore _OSC DPC negotiation result if pcie_ports_dpc_native is set.
From: Kuppuswamy Sathyanarayanan pcie_ports_dpc_native is set only if user requests native handling of PCIe DPC capability via pcie_port_setup command line option. User input takes precedence over _OSC based control negotiation result. So consider the _OSC negotiated result for DPC ownership only if pcie_ports_dpc_native is unset. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/acpi/pci_root.c | 8 ++-- drivers/pci/pcie/dpc.c | 3 ++- drivers/pci/pcie/portdrv.h | 2 -- drivers/pci/pcie/portdrv_core.c | 2 +- include/linux/pci.h | 2 ++ 5 files changed, 11 insertions(+), 6 deletions(-) diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index 9749b7abdd7e..979d03494476 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -935,8 +935,12 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, host_bridge->native_ltr); OSC_OWNER(ctrl, OSC_PCI_EXPRESS_DPC_CONTROL, host_bridge->native_dpc); - } - + if (pcie_ports_dpc_native) + dev_warn(>dev, "OS forcibly taking over DPC\n"); + else + OSC_OWNER(ctrl, OSC_PCI_EXPRESS_DPC_CONTROL, + host_bridge->native_dpc); + } if (!(root->osc_control_set & OSC_PCI_SHPC_NATIVE_HP_CONTROL)) host_bridge->native_shpc_hotplug = 0; diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index daa9a4153776..5b1025a2994d 100644 --- a/drivers/pci/pcie/dpc.c +++ b/drivers/pci/pcie/dpc.c @@ -280,11 +280,12 @@ void pci_dpc_init(struct pci_dev *pdev) static int dpc_probe(struct pcie_device *dev) { struct pci_dev *pdev = dev->port; + struct pci_host_bridge *host = pci_find_host_bridge(pdev->bus); struct device *device = >device; int status; u16 ctl, cap; - if (!pcie_aer_is_native(pdev) && !pcie_ports_dpc_native) + if (!pcie_aer_is_native(pdev) && !host->native_dpc) return -ENOTSUPP; status = devm_request_threaded_irq(device, dev->irq, dpc_irq, diff --git a/drivers/pci/pcie/portdrv.h b/drivers/pci/pcie/portdrv.h index af7cf237432a..0ac20feef24e 100644 --- a/drivers/pci/pcie/portdrv.h +++ b/drivers/pci/pcie/portdrv.h @@ -25,8 +25,6 @@ #define PCIE_PORT_DEVICE_MAXSERVICES 5 -extern bool pcie_ports_dpc_native; - #ifdef CONFIG_PCIEAER int pcie_aer_init(void); int pcie_aer_is_native(struct pci_dev *dev); diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index ccd5e0ce5605..2c0278f0fdcc 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -253,7 +253,7 @@ static int get_port_device_capability(struct pci_dev *dev) */ if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) && pci_aer_available() && - (pcie_ports_dpc_native || (services & PCIE_PORT_SERVICE_AER))) + (host->native_dpc || (services & PCIE_PORT_SERVICE_AER))) services |= PCIE_PORT_SERVICE_DPC; if (pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM || diff --git a/include/linux/pci.h b/include/linux/pci.h index 835530605c0d..dc03b6c65742 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1556,9 +1556,11 @@ static inline int pci_irqd_intx_xlate(struct irq_domain *d, #ifdef CONFIG_PCIEPORTBUS extern bool pcie_ports_disabled; extern bool pcie_ports_native; +extern bool pcie_ports_dpc_native; #else #define pcie_ports_disabledtrue #define pcie_ports_native false +#define pcie_ports_dpc_native false #endif #define PCIE_LINK_STATE_L0SBIT(0) -- 2.17.1
[PATCH v9 4/5] PCI/portdrv: Remove redundant pci_aer_available() check in DPC enable logic
From: Kuppuswamy Sathyanarayanan In DPC service enable logic, check for services & PCIE_PORT_SERVICE_AER implies pci_aer_available() is true. So there is no need to explicitly check it again. Also, passing pcie_ports=dpc-native in kernel command line implies DPC needs to be enabled in native mode irrespective of AER ownership status. So checking for pci_aer_available() without checking for pcie_ports status is incorrect. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/pcie/portdrv_core.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index 2c0278f0fdcc..e257a2ca3595 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -252,7 +252,6 @@ static int get_port_device_capability(struct pci_dev *dev) * permission to use AER. */ if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) && - pci_aer_available() && (host->native_dpc || (services & PCIE_PORT_SERVICE_AER))) services |= PCIE_PORT_SERVICE_DPC; -- 2.17.1
[PATCH v9 5/5] PCI/DPC: Move AER/DPC dependency checks out of DPC driver
From: Kuppuswamy Sathyanarayanan Currently, AER and DPC Capabilities dependency checks is distributed between DPC and portdrv service drivers. So move them out of DPC driver. Also, since services & PCIE_PORT_SERVICE_AER check already ensures AER native ownership, no need to add additional pcie_aer_is_native() check. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/pcie/dpc.c | 4 drivers/pci/pcie/portdrv_core.c | 1 + 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index 5b1025a2994d..6261b0382f65 100644 --- a/drivers/pci/pcie/dpc.c +++ b/drivers/pci/pcie/dpc.c @@ -280,14 +280,10 @@ void pci_dpc_init(struct pci_dev *pdev) static int dpc_probe(struct pcie_device *dev) { struct pci_dev *pdev = dev->port; - struct pci_host_bridge *host = pci_find_host_bridge(pdev->bus); struct device *device = >device; int status; u16 ctl, cap; - if (!pcie_aer_is_native(pdev) && !host->native_dpc) - return -ENOTSUPP; - status = devm_request_threaded_irq(device, dev->irq, dpc_irq, dpc_handler, IRQF_SHARED, "pcie-dpc", pdev); diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index e257a2ca3595..ffa1d9fc458e 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -252,6 +252,7 @@ static int get_port_device_capability(struct pci_dev *dev) * permission to use AER. */ if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) && + host->native_dpc && (host->native_dpc || (services & PCIE_PORT_SERVICE_AER))) services |= PCIE_PORT_SERVICE_DPC; -- 2.17.1
[PATCH v9 1/5] PCI: Conditionally initialize host bridge native_* members
From: Kuppuswamy Sathyanarayanan If CONFIG_PCIEPORTBUS is not enabled in kernel then initialing struct pci_host_bridge PCIe specific native_* members to "1" is incorrect. So protect the PCIe specific member initialization with CONFIG_PCIEPORTBUS. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/probe.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 03d37128a24f..0da0fc034413 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -588,12 +588,14 @@ static void pci_init_host_bridge(struct pci_host_bridge *bridge) * may implement its own AER handling and use _OSC to prevent the * OS from interfering. */ +#ifdef CONFIG_PCIEPORTBUS bridge->native_aer = 1; bridge->native_pcie_hotplug = 1; - bridge->native_shpc_hotplug = 1; bridge->native_pme = 1; - bridge->native_ltr = 1; bridge->native_dpc = 1; +#endif + bridge->native_ltr = 1; + bridge->native_shpc_hotplug = 1; device_initialize(>dev); } -- 2.17.1
[PATCH v9 0/5] Simplify PCIe native ownership detection logic
From: Kuppuswamy Sathyanarayanan Currently, PCIe capabilities ownership status is detected by verifying the status of pcie_ports_native, pcie_ports_dpc_native and _OSC negotiated results (cached in struct pci_host_bridge ->native_* members). But this logic can be simplified, and we can use only struct pci_host_bridge ->native_* members to detect it. This patchset removes the distributed checks for pcie_ports_native, pcie_ports_dpc_native parameters. Changes since v8: * Simplified setting _OSC ownwership logic * Moved bridge->native_ltr out of #ifdef CONFIG_PCIEPORTBUS. Changes since v7: * Fixed "fix array_size.cocci warnings". Changes since v6: * Created new patch for CONFIG_PCIEPORTBUS check in pci_init_host_bridge(). * Added warning message for a case when pcie_ports_native overrides _OSC negotiation result. Changes since v5: * Rebased on top of v5.8-rc1 Changes since v4: * Changed the patch set title (Original link: https://lkml.org/lkml/2020/5/26/1710) * Added AER/DPC dependency logic cleanup fixes. Kuppuswamy Sathyanarayanan (5): PCI: Conditionally initialize host bridge native_* members ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set. ACPI/PCI: Ignore _OSC DPC negotiation result if pcie_ports_dpc_native is set. PCI/portdrv: Remove redundant pci_aer_available() check in DPC enable logic PCI/DPC: Move AER/DPC dependency checks out of DPC driver drivers/acpi/pci_root.c | 37 ++- drivers/pci/hotplug/pciehp_core.c | 2 +- drivers/pci/pci-acpi.c| 3 --- drivers/pci/pcie/aer.c| 2 +- drivers/pci/pcie/dpc.c| 3 --- drivers/pci/pcie/portdrv.h| 2 -- drivers/pci/pcie/portdrv_core.c | 13 +-- drivers/pci/probe.c | 6 +++-- include/linux/acpi.h | 2 ++ include/linux/pci.h | 2 ++ 10 files changed, 42 insertions(+), 30 deletions(-) -- 2.17.1
[PATCH v9 5/5] PCI/DPC: Move AER/DPC dependency checks out of DPC driver
From: Kuppuswamy Sathyanarayanan Currently, AER and DPC Capabilities dependency checks is distributed between DPC and portdrv service drivers. So move them out of DPC driver. Also, since services & PCIE_PORT_SERVICE_AER check already ensures AER native ownership, no need to add additional pcie_aer_is_native() check. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/pcie/dpc.c | 4 drivers/pci/pcie/portdrv_core.c | 1 + 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index 5b1025a2994d..6261b0382f65 100644 --- a/drivers/pci/pcie/dpc.c +++ b/drivers/pci/pcie/dpc.c @@ -280,14 +280,10 @@ void pci_dpc_init(struct pci_dev *pdev) static int dpc_probe(struct pcie_device *dev) { struct pci_dev *pdev = dev->port; - struct pci_host_bridge *host = pci_find_host_bridge(pdev->bus); struct device *device = >device; int status; u16 ctl, cap; - if (!pcie_aer_is_native(pdev) && !host->native_dpc) - return -ENOTSUPP; - status = devm_request_threaded_irq(device, dev->irq, dpc_irq, dpc_handler, IRQF_SHARED, "pcie-dpc", pdev); diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index e257a2ca3595..ffa1d9fc458e 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -252,6 +252,7 @@ static int get_port_device_capability(struct pci_dev *dev) * permission to use AER. */ if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) && + host->native_dpc && (host->native_dpc || (services & PCIE_PORT_SERVICE_AER))) services |= PCIE_PORT_SERVICE_DPC; -- 2.17.1
[PATCH v9 1/5] PCI: Conditionally initialize host bridge native_* members
From: Kuppuswamy Sathyanarayanan If CONFIG_PCIEPORTBUS is not enabled in kernel then initialing struct pci_host_bridge PCIe specific native_* members to "1" is incorrect. So protect the PCIe specific member initialization with CONFIG_PCIEPORTBUS. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/probe.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 03d37128a24f..0da0fc034413 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -588,12 +588,14 @@ static void pci_init_host_bridge(struct pci_host_bridge *bridge) * may implement its own AER handling and use _OSC to prevent the * OS from interfering. */ +#ifdef CONFIG_PCIEPORTBUS bridge->native_aer = 1; bridge->native_pcie_hotplug = 1; - bridge->native_shpc_hotplug = 1; bridge->native_pme = 1; - bridge->native_ltr = 1; bridge->native_dpc = 1; +#endif + bridge->native_ltr = 1; + bridge->native_shpc_hotplug = 1; device_initialize(>dev); } -- 2.17.1
[PATCH v9 3/5] ACPI/PCI: Ignore _OSC DPC negotiation result if pcie_ports_dpc_native is set.
From: Kuppuswamy Sathyanarayanan pcie_ports_dpc_native is set only if user requests native handling of PCIe DPC capability via pcie_port_setup command line option. User input takes precedence over _OSC based control negotiation result. So consider the _OSC negotiated result for DPC ownership only if pcie_ports_dpc_native is unset. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/acpi/pci_root.c | 8 ++-- drivers/pci/pcie/dpc.c | 3 ++- drivers/pci/pcie/portdrv.h | 2 -- drivers/pci/pcie/portdrv_core.c | 2 +- include/linux/pci.h | 2 ++ 5 files changed, 11 insertions(+), 6 deletions(-) diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index 9749b7abdd7e..979d03494476 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -935,8 +935,12 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, host_bridge->native_ltr); OSC_OWNER(ctrl, OSC_PCI_EXPRESS_DPC_CONTROL, host_bridge->native_dpc); - } - + if (pcie_ports_dpc_native) + dev_warn(>dev, "OS forcibly taking over DPC\n"); + else + OSC_OWNER(ctrl, OSC_PCI_EXPRESS_DPC_CONTROL, + host_bridge->native_dpc); + } if (!(root->osc_control_set & OSC_PCI_SHPC_NATIVE_HP_CONTROL)) host_bridge->native_shpc_hotplug = 0; diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index daa9a4153776..5b1025a2994d 100644 --- a/drivers/pci/pcie/dpc.c +++ b/drivers/pci/pcie/dpc.c @@ -280,11 +280,12 @@ void pci_dpc_init(struct pci_dev *pdev) static int dpc_probe(struct pcie_device *dev) { struct pci_dev *pdev = dev->port; + struct pci_host_bridge *host = pci_find_host_bridge(pdev->bus); struct device *device = >device; int status; u16 ctl, cap; - if (!pcie_aer_is_native(pdev) && !pcie_ports_dpc_native) + if (!pcie_aer_is_native(pdev) && !host->native_dpc) return -ENOTSUPP; status = devm_request_threaded_irq(device, dev->irq, dpc_irq, diff --git a/drivers/pci/pcie/portdrv.h b/drivers/pci/pcie/portdrv.h index af7cf237432a..0ac20feef24e 100644 --- a/drivers/pci/pcie/portdrv.h +++ b/drivers/pci/pcie/portdrv.h @@ -25,8 +25,6 @@ #define PCIE_PORT_DEVICE_MAXSERVICES 5 -extern bool pcie_ports_dpc_native; - #ifdef CONFIG_PCIEAER int pcie_aer_init(void); int pcie_aer_is_native(struct pci_dev *dev); diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index ccd5e0ce5605..2c0278f0fdcc 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -253,7 +253,7 @@ static int get_port_device_capability(struct pci_dev *dev) */ if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) && pci_aer_available() && - (pcie_ports_dpc_native || (services & PCIE_PORT_SERVICE_AER))) + (host->native_dpc || (services & PCIE_PORT_SERVICE_AER))) services |= PCIE_PORT_SERVICE_DPC; if (pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM || diff --git a/include/linux/pci.h b/include/linux/pci.h index 835530605c0d..dc03b6c65742 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1556,9 +1556,11 @@ static inline int pci_irqd_intx_xlate(struct irq_domain *d, #ifdef CONFIG_PCIEPORTBUS extern bool pcie_ports_disabled; extern bool pcie_ports_native; +extern bool pcie_ports_dpc_native; #else #define pcie_ports_disabledtrue #define pcie_ports_native false +#define pcie_ports_dpc_native false #endif #define PCIE_LINK_STATE_L0SBIT(0) -- 2.17.1
[PATCH v8 1/5] PCI: Conditionally initialize host bridge native_* members
From: Kuppuswamy Sathyanarayanan If CONFIG_PCIEPORTBUS is not enabled in kernel then initialing struct pci_host_bridge PCIe specific native_* members to "1" is incorrect. So protect the PCIe specific member initialization with CONFIG_PCIEPORTBUS. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/probe.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 2f66988cea25..a94b97564ceb 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -588,12 +588,14 @@ static void pci_init_host_bridge(struct pci_host_bridge *bridge) * may implement its own AER handling and use _OSC to prevent the * OS from interfering. */ +#ifdef CONFIG_PCIEPORTBUS bridge->native_aer = 1; bridge->native_pcie_hotplug = 1; - bridge->native_shpc_hotplug = 1; bridge->native_pme = 1; bridge->native_ltr = 1; bridge->native_dpc = 1; +#endif + bridge->native_shpc_hotplug = 1; device_initialize(>dev); } -- 2.17.1
[PATCH v8 4/5] PCI/portdrv: Remove redundant pci_aer_available() check in DPC enable logic
From: Kuppuswamy Sathyanarayanan In DPC service enable logic, check for services & PCIE_PORT_SERVICE_AER implies pci_aer_available() is true. So there is no need to explicitly check it again. Also, passing pcie_ports=dpc-native in kernel command line implies DPC needs to be enabled in native mode irrespective of AER ownership status. So checking for pci_aer_available() without checking for pcie_ports status is incorrect. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/pcie/portdrv_core.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index 2c0278f0fdcc..e257a2ca3595 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -252,7 +252,6 @@ static int get_port_device_capability(struct pci_dev *dev) * permission to use AER. */ if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) && - pci_aer_available() && (host->native_dpc || (services & PCIE_PORT_SERVICE_AER))) services |= PCIE_PORT_SERVICE_DPC; -- 2.17.1
[PATCH v8 2/5] ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set.
From: Kuppuswamy Sathyanarayanan pcie_ports_native is set only if user requests native handling of PCIe capabilities via pcie_port_setup command line option. User input takes precedence over _OSC based control negotiation result. So consider the _OSC negotiated result only if pcie_ports_native is unset. Also, since struct pci_host_bridge ->native_* members caches the ownership status of various PCIe capabilities, use them instead of distributed checks for pcie_ports_native. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/acpi/pci_root.c | 61 ++- drivers/pci/hotplug/pciehp_core.c | 2 +- drivers/pci/pci-acpi.c| 3 -- drivers/pci/pcie/aer.c| 2 +- drivers/pci/pcie/portdrv_core.c | 9 ++--- 5 files changed, 56 insertions(+), 21 deletions(-) diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index f90e841c59f5..f8981d4e044d 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -145,6 +145,17 @@ static struct pci_osc_bit_struct pci_osc_control_bit[] = { { OSC_PCI_EXPRESS_DPC_CONTROL, "DPC" }, }; +static char *get_osc_desc(u32 bit) +{ + int i = 0; + + for (i = 0; i < ARRAY_SIZE(pci_osc_control_bit); i++) + if (bit == pci_osc_control_bit[i].bit) + return pci_osc_control_bit[i].desc; + + return NULL; +} + static void decode_osc_bits(struct acpi_pci_root *root, char *msg, u32 word, struct pci_osc_bit_struct *table, int size) { @@ -914,18 +925,48 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, goto out_release_info; host_bridge = to_pci_host_bridge(bus->bridge); - if (!(root->osc_control_set & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL)) - host_bridge->native_pcie_hotplug = 0; + if (!(root->osc_control_set & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL)) { + if (!pcie_ports_native) + host_bridge->native_pcie_hotplug = 0; + else + dev_warn(>dev, "OS overrides %s firmware control", + get_osc_desc(OSC_PCI_EXPRESS_NATIVE_HP_CONTROL)); + } + if (!(root->osc_control_set & OSC_PCI_SHPC_NATIVE_HP_CONTROL)) host_bridge->native_shpc_hotplug = 0; - if (!(root->osc_control_set & OSC_PCI_EXPRESS_AER_CONTROL)) - host_bridge->native_aer = 0; - if (!(root->osc_control_set & OSC_PCI_EXPRESS_PME_CONTROL)) - host_bridge->native_pme = 0; - if (!(root->osc_control_set & OSC_PCI_EXPRESS_LTR_CONTROL)) - host_bridge->native_ltr = 0; - if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL)) - host_bridge->native_dpc = 0; + + if (!(root->osc_control_set & OSC_PCI_EXPRESS_AER_CONTROL)) { + if (!pcie_ports_native) + host_bridge->native_aer = 0; + else + dev_warn(>dev, "OS overrides %s firmware control", + get_osc_desc(OSC_PCI_EXPRESS_AER_CONTROL)); + } + + if (!(root->osc_control_set & OSC_PCI_EXPRESS_PME_CONTROL)) { + if (!pcie_ports_native) + host_bridge->native_pme = 0; + else + dev_warn(>dev, "OS overrides %s firmware control", + get_osc_desc(OSC_PCI_EXPRESS_PME_CONTROL)); + } + + if (!(root->osc_control_set & OSC_PCI_EXPRESS_LTR_CONTROL)) { + if (!pcie_ports_native) + host_bridge->native_ltr = 0; + else + dev_warn(>dev, "OS overrides %s firmware control", + get_osc_desc(OSC_PCI_EXPRESS_LTR_CONTROL)); + } + + if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL)) { + if (!pcie_ports_native) + host_bridge->native_dpc = 0; + else + dev_warn(>dev, "OS overrides %s firmware control", + get_osc_desc(OSC_PCI_EXPRESS_DPC_CONTROL)); + } /* * Evaluate the "PCI Boot Configuration" _DSM Function. If it diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c index bf779f291f15..5fc999bf6f1b 100644 --- a/drivers/pci/hotplug/pciehp_core.c +++ b/drivers/pci/hotplug/pciehp_core.c @@ -255,7 +255,7 @@ static bool pme_is_native(struct pcie_device *dev) const struct pci_host_bridge *host; host = pci_find_host_bridge(dev->port->bus); - return pcie_ports_native || host->native_pme; + return host->native_pme; } static void pciehp_disable_interrupt(struct pcie_device *dev) diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c index 7224b1e5f2a8..e09589571a9d 100644 --- a/drivers/pci/pci-acpi.c +++ b/drivers/pci/pci-acpi.c @@ -800,9 +800,6 @@ bool pciehp_is_native(struct pci_dev *bridge)
[PATCH v8 5/5] PCI/DPC: Move AER/DPC dependency checks out of DPC driver
From: Kuppuswamy Sathyanarayanan Currently, AER and DPC Capabilities dependency checks is distributed between DPC and portdrv service drivers. So move them out of DPC driver. Also, since services & PCIE_PORT_SERVICE_AER check already ensures AER native ownership, no need to add additional pcie_aer_is_native() check. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/pcie/dpc.c | 3 --- drivers/pci/pcie/portdrv_core.c | 1 + 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index 5b1025a2994d..3efbe43764f3 100644 --- a/drivers/pci/pcie/dpc.c +++ b/drivers/pci/pcie/dpc.c @@ -285,9 +285,6 @@ static int dpc_probe(struct pcie_device *dev) int status; u16 ctl, cap; - if (!pcie_aer_is_native(pdev) && !host->native_dpc) - return -ENOTSUPP; - status = devm_request_threaded_irq(device, dev->irq, dpc_irq, dpc_handler, IRQF_SHARED, "pcie-dpc", pdev); diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index e257a2ca3595..ffa1d9fc458e 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -252,6 +252,7 @@ static int get_port_device_capability(struct pci_dev *dev) * permission to use AER. */ if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) && + host->native_dpc && (host->native_dpc || (services & PCIE_PORT_SERVICE_AER))) services |= PCIE_PORT_SERVICE_DPC; -- 2.17.1
[PATCH v8 3/5] ACPI/PCI: Ignore _OSC DPC negotiation result if pcie_ports_dpc_native is set.
From: Kuppuswamy Sathyanarayanan pcie_ports_dpc_native is set only if user requests native handling of PCIe DPC capability via pcie_port_setup command line option. User input takes precedence over _OSC based control negotiation result. So consider the _OSC negotiated result for DPC ownership only if pcie_ports_dpc_native is unset. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/acpi/pci_root.c | 2 +- drivers/pci/pcie/dpc.c | 3 ++- drivers/pci/pcie/portdrv.h | 2 -- drivers/pci/pcie/portdrv_core.c | 2 +- include/linux/pci.h | 2 ++ 5 files changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index f8981d4e044d..3942bb42cb93 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -961,7 +961,7 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, } if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL)) { - if (!pcie_ports_native) + if (!pcie_ports_native && !pcie_ports_dpc_native) host_bridge->native_dpc = 0; else dev_warn(>dev, "OS overrides %s firmware control", diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index daa9a4153776..5b1025a2994d 100644 --- a/drivers/pci/pcie/dpc.c +++ b/drivers/pci/pcie/dpc.c @@ -280,11 +280,12 @@ void pci_dpc_init(struct pci_dev *pdev) static int dpc_probe(struct pcie_device *dev) { struct pci_dev *pdev = dev->port; + struct pci_host_bridge *host = pci_find_host_bridge(pdev->bus); struct device *device = >device; int status; u16 ctl, cap; - if (!pcie_aer_is_native(pdev) && !pcie_ports_dpc_native) + if (!pcie_aer_is_native(pdev) && !host->native_dpc) return -ENOTSUPP; status = devm_request_threaded_irq(device, dev->irq, dpc_irq, diff --git a/drivers/pci/pcie/portdrv.h b/drivers/pci/pcie/portdrv.h index af7cf237432a..0ac20feef24e 100644 --- a/drivers/pci/pcie/portdrv.h +++ b/drivers/pci/pcie/portdrv.h @@ -25,8 +25,6 @@ #define PCIE_PORT_DEVICE_MAXSERVICES 5 -extern bool pcie_ports_dpc_native; - #ifdef CONFIG_PCIEAER int pcie_aer_init(void); int pcie_aer_is_native(struct pci_dev *dev); diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index ccd5e0ce5605..2c0278f0fdcc 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -253,7 +253,7 @@ static int get_port_device_capability(struct pci_dev *dev) */ if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) && pci_aer_available() && - (pcie_ports_dpc_native || (services & PCIE_PORT_SERVICE_AER))) + (host->native_dpc || (services & PCIE_PORT_SERVICE_AER))) services |= PCIE_PORT_SERVICE_DPC; if (pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM || diff --git a/include/linux/pci.h b/include/linux/pci.h index 34c1c4f45288..fe7ce06a4f40 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1556,9 +1556,11 @@ static inline int pci_irqd_intx_xlate(struct irq_domain *d, #ifdef CONFIG_PCIEPORTBUS extern bool pcie_ports_disabled; extern bool pcie_ports_native; +extern bool pcie_ports_dpc_native; #else #define pcie_ports_disabledtrue #define pcie_ports_native false +#define pcie_ports_dpc_native false #endif #define PCIE_LINK_STATE_L0SBIT(0) -- 2.17.1
[PATCH v8 0/5] Simplify PCIe native ownership detection logic
From: Kuppuswamy Sathyanarayanan Currently, PCIe capabilities ownership status is detected by verifying the status of pcie_ports_native, pcie_ports_dpc_native and _OSC negotiated results (cached in struct pci_host_bridge ->native_* members). But this logic can be simplified, and we can use only struct pci_host_bridge ->native_* members to detect it. This patchset removes the distributed checks for pcie_ports_native, pcie_ports_dpc_native parameters. Changes since v7: * Fixed "fix array_size.cocci warnings". Changes since v6: * Created new patch for CONFIG_PCIEPORTBUS check in pci_init_host_bridge(). * Added warning message for a case when pcie_ports_native overrides _OSC negotiation result. Changes since v5: * Rebased on top of v5.8-rc1 Changes since v4: * Changed the patch set title (Original link: https://lkml.org/lkml/2020/5/26/1710) * Added AER/DPC dependency logic cleanup fixes. Kuppuswamy Sathyanarayanan (5): PCI: Conditionally initialize host bridge native_* members ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set. ACPI/PCI: Ignore _OSC DPC negotiation result if pcie_ports_dpc_native is set. PCI/portdrv: Remove redundant pci_aer_available() check in DPC enable logic PCI/DPC: Move AER/DPC dependency checks out of DPC driver drivers/acpi/pci_root.c | 61 ++- drivers/pci/hotplug/pciehp_core.c | 2 +- drivers/pci/pci-acpi.c| 3 -- drivers/pci/pcie/aer.c| 2 +- drivers/pci/pcie/dpc.c| 4 +- drivers/pci/pcie/portdrv.h| 2 - drivers/pci/pcie/portdrv_core.c | 13 +++ drivers/pci/probe.c | 4 +- include/linux/pci.h | 2 + 9 files changed, 64 insertions(+), 29 deletions(-) -- 2.17.1
[PATCH v3 1/1] PCI/ERR: Fix reset logic in pcie_do_recovery() call
From: Kuppuswamy Sathyanarayanan Current pcie_do_recovery() implementation has following two issues: 1. Fatal (DPC) error recovery is currently broken for non-hotplug capable devices. Current fatal error recovery implementation relies on PCIe hotplug (pciehp) handler for detaching and re-enumerating the affected devices/drivers. pciehp handler listens for DLLSC state changes and handles device/driver detachment on DLLSC_LINK_DOWN event and re-enumeration on DLLSC_LINK_UP event. So when dealing with non-hotplug capable devices, recovery code does not restore the state of the affected devices correctly. Correct implementation should restore the device state and call report_slot_reset() function after resetting the link to restore the state of the device/driver. You can find fatal non-hotplug related issues reported in following links: https://lore.kernel.org/linux-pci/20200527083130.4137-1-zhiqiang@nxp.com/ https://lore.kernel.org/linux-pci/12115.1588207324@famine/ https://lore.kernel.org/linux-pci/0e6f89cd6b9e4a72293cc90fafe93487d7c2d295.158584.git.sathyanarayanan.kuppusw...@linux.intel.com/ 2. For non-fatal errors if report_error_detected() or report_mmio_enabled() functions requests PCI_ERS_RESULT_NEED_RESET then current pcie_do_recovery() implementation does not do the requested explicit device reset, instead just calls the report_slot_reset() on all affected devices. Notifying about the reset via report_slot_reset() without doing the actual device reset is incorrect. To fix above issues, use PCI_ERS_RESULT_NEED_RESET as error state after successful reset_link() operation. This will ensure ->slot_reset() be called after reset_link() operation for fatal errors. Also call pci_bus_reset() to do slot/bus reset() before triggering device specific ->slot_reset() callback. Also, using pci_bus_reset() will restore the state of the devices after performing the reset operation. Even though using pci_bus_reset() will do redundant reset operation after ->reset_link() for fatal errors, it should should affect the functional behavior. [original patch is from jay.vosbu...@canonical.com] [original patch link https://lore.kernel.org/linux-pci/12115.1588207324@famine/] Fixes: 6d2c89441571 ("PCI/ERR: Update error status after reset_link()") Signed-off-by: Jay Vosburgh Signed-off-by: Kuppuswamy Sathyanarayanan --- Changes since v2: * Changed the subject of patch to "PCI/ERR: Fix reset logic in pcie_do_recovery() call". v2 patch link is, https://lore.kernel.org/linux-pci/ce417fbf81a8a46a89535f44b9224ee9fbb55a29.1591307288.git.sathyanarayanan.kuppusw...@linux.intel.com/ * Squashed "PCI/ERR: Add reset support for non fatal errors" patch. drivers/pci/pcie/err.c | 41 + 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c index 14bb8f54723e..b5eb6ba65be1 100644 --- a/drivers/pci/pcie/err.c +++ b/drivers/pci/pcie/err.c @@ -165,8 +165,29 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev, pci_dbg(dev, "broadcast error_detected message\n"); if (state == pci_channel_io_frozen) { pci_walk_bus(bus, report_frozen_detected, ); + /* +* After resetting the link using reset_link() call, the +* possible value of error status is either +* PCI_ERS_RESULT_DISCONNECT (failure case) or +* PCI_ERS_RESULT_NEED_RESET (success case). +* So ignore the return value of report_error_detected() +* call for fatal errors. +* +* In EDR mode, since AER and DPC Capabilities are owned by +* firmware, reported_error_detected() will return error +* status PCI_ERS_RESULT_NO_AER_DRIVER. Continuing +* pcie_do_recovery() with error status as +* PCI_ERS_RESULT_NO_AER_DRIVER will report recovery failure +* irrespective of recovery status. But successful reset_link() +* call usually recovers all fatal errors. So ignoring the +* status result of report_error_detected() also helps EDR based +* error recovery. +*/ status = reset_link(dev); - if (status != PCI_ERS_RESULT_RECOVERED) { + if (status == PCI_ERS_RESULT_RECOVERED) { + status = PCI_ERS_RESULT_NEED_RESET; + } else { + status = PCI_ERS_RESULT_DISCONNECT; pci_warn(dev, "link reset failed\n"); goto failed; } @@ -182,10 +203,22 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev, if (status == PCI_ERS_RESULT_NEED_RESET) { /* -* TODO: Should call platform-specific -* functions to reset slot before calling -* drivers' slot_reset
[PATCH v7 3/5] ACPI/PCI: Ignore _OSC DPC negotiation result if pcie_ports_dpc_native is set.
From: Kuppuswamy Sathyanarayanan pcie_ports_dpc_native is set only if user requests native handling of PCIe DPC capability via pcie_port_setup command line option. User input takes precedence over _OSC based control negotiation result. So consider the _OSC negotiated result for DPC ownership only if pcie_ports_dpc_native is unset. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/acpi/pci_root.c | 2 +- drivers/pci/pcie/dpc.c | 3 ++- drivers/pci/pcie/portdrv.h | 2 -- drivers/pci/pcie/portdrv_core.c | 2 +- include/linux/pci.h | 2 ++ 5 files changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index a82069d064fe..a7bc1a864b14 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -962,7 +962,7 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, } if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL)) { - if (!pcie_ports_native) + if (!pcie_ports_native && !pcie_ports_dpc_native) host_bridge->native_dpc = 0; else dev_warn(>dev, "OS overrides %s firmware control", diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index daa9a4153776..5b1025a2994d 100644 --- a/drivers/pci/pcie/dpc.c +++ b/drivers/pci/pcie/dpc.c @@ -280,11 +280,12 @@ void pci_dpc_init(struct pci_dev *pdev) static int dpc_probe(struct pcie_device *dev) { struct pci_dev *pdev = dev->port; + struct pci_host_bridge *host = pci_find_host_bridge(pdev->bus); struct device *device = >device; int status; u16 ctl, cap; - if (!pcie_aer_is_native(pdev) && !pcie_ports_dpc_native) + if (!pcie_aer_is_native(pdev) && !host->native_dpc) return -ENOTSUPP; status = devm_request_threaded_irq(device, dev->irq, dpc_irq, diff --git a/drivers/pci/pcie/portdrv.h b/drivers/pci/pcie/portdrv.h index af7cf237432a..0ac20feef24e 100644 --- a/drivers/pci/pcie/portdrv.h +++ b/drivers/pci/pcie/portdrv.h @@ -25,8 +25,6 @@ #define PCIE_PORT_DEVICE_MAXSERVICES 5 -extern bool pcie_ports_dpc_native; - #ifdef CONFIG_PCIEAER int pcie_aer_init(void); int pcie_aer_is_native(struct pci_dev *dev); diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index ccd5e0ce5605..2c0278f0fdcc 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -253,7 +253,7 @@ static int get_port_device_capability(struct pci_dev *dev) */ if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) && pci_aer_available() && - (pcie_ports_dpc_native || (services & PCIE_PORT_SERVICE_AER))) + (host->native_dpc || (services & PCIE_PORT_SERVICE_AER))) services |= PCIE_PORT_SERVICE_DPC; if (pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM || diff --git a/include/linux/pci.h b/include/linux/pci.h index 34c1c4f45288..fe7ce06a4f40 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1556,9 +1556,11 @@ static inline int pci_irqd_intx_xlate(struct irq_domain *d, #ifdef CONFIG_PCIEPORTBUS extern bool pcie_ports_disabled; extern bool pcie_ports_native; +extern bool pcie_ports_dpc_native; #else #define pcie_ports_disabledtrue #define pcie_ports_native false +#define pcie_ports_dpc_native false #endif #define PCIE_LINK_STATE_L0SBIT(0) -- 2.17.1
[PATCH v7 1/5] PCI: Conditionally initialize host bridge native_* members
From: Kuppuswamy Sathyanarayanan If CONFIG_PCIEPORTBUS is not enabled in kernel then initialing struct pci_host_bridge PCIe specific native_* members to "1" is incorrect. So protect the PCIe specific member initialization with CONFIG_PCIEPORTBUS. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/probe.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 2f66988cea25..a94b97564ceb 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -588,12 +588,14 @@ static void pci_init_host_bridge(struct pci_host_bridge *bridge) * may implement its own AER handling and use _OSC to prevent the * OS from interfering. */ +#ifdef CONFIG_PCIEPORTBUS bridge->native_aer = 1; bridge->native_pcie_hotplug = 1; - bridge->native_shpc_hotplug = 1; bridge->native_pme = 1; bridge->native_ltr = 1; bridge->native_dpc = 1; +#endif + bridge->native_shpc_hotplug = 1; device_initialize(>dev); } -- 2.17.1
[PATCH v7 5/5] PCI/DPC: Move AER/DPC dependency checks out of DPC driver
From: Kuppuswamy Sathyanarayanan Currently, AER and DPC Capabilities dependency checks is distributed between DPC and portdrv service drivers. So move them out of DPC driver. Also, since services & PCIE_PORT_SERVICE_AER check already ensures AER native ownership, no need to add additional pcie_aer_is_native() check. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/pcie/dpc.c | 3 --- drivers/pci/pcie/portdrv_core.c | 1 + 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index 5b1025a2994d..3efbe43764f3 100644 --- a/drivers/pci/pcie/dpc.c +++ b/drivers/pci/pcie/dpc.c @@ -285,9 +285,6 @@ static int dpc_probe(struct pcie_device *dev) int status; u16 ctl, cap; - if (!pcie_aer_is_native(pdev) && !host->native_dpc) - return -ENOTSUPP; - status = devm_request_threaded_irq(device, dev->irq, dpc_irq, dpc_handler, IRQF_SHARED, "pcie-dpc", pdev); diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index e257a2ca3595..ffa1d9fc458e 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -252,6 +252,7 @@ static int get_port_device_capability(struct pci_dev *dev) * permission to use AER. */ if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) && + host->native_dpc && (host->native_dpc || (services & PCIE_PORT_SERVICE_AER))) services |= PCIE_PORT_SERVICE_DPC; -- 2.17.1
[PATCH v7 2/5] ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set.
From: Kuppuswamy Sathyanarayanan pcie_ports_native is set only if user requests native handling of PCIe capabilities via pcie_port_setup command line option. User input takes precedence over _OSC based control negotiation result. So consider the _OSC negotiated result only if pcie_ports_native is unset. Also, since struct pci_host_bridge ->native_* members caches the ownership status of various PCIe capabilities, use them instead of distributed checks for pcie_ports_native. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/acpi/pci_root.c | 62 ++- drivers/pci/hotplug/pciehp_core.c | 2 +- drivers/pci/pci-acpi.c| 3 -- drivers/pci/pcie/aer.c| 2 +- drivers/pci/pcie/portdrv_core.c | 9 ++--- 5 files changed, 57 insertions(+), 21 deletions(-) diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index f90e841c59f5..a82069d064fe 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -145,6 +145,18 @@ static struct pci_osc_bit_struct pci_osc_control_bit[] = { { OSC_PCI_EXPRESS_DPC_CONTROL, "DPC" }, }; +static char *get_osc_desc(u32 bit) +{ + int len = sizeof(pci_osc_control_bit) / sizeof(pci_osc_control_bit[0]); + int i = 0; + + for (i = 0; i bridge); - if (!(root->osc_control_set & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL)) - host_bridge->native_pcie_hotplug = 0; + if (!(root->osc_control_set & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL)) { + if (!pcie_ports_native) + host_bridge->native_pcie_hotplug = 0; + else + dev_warn(>dev, "OS overrides %s firmware control", + get_osc_desc(OSC_PCI_EXPRESS_NATIVE_HP_CONTROL)); + } + if (!(root->osc_control_set & OSC_PCI_SHPC_NATIVE_HP_CONTROL)) host_bridge->native_shpc_hotplug = 0; - if (!(root->osc_control_set & OSC_PCI_EXPRESS_AER_CONTROL)) - host_bridge->native_aer = 0; - if (!(root->osc_control_set & OSC_PCI_EXPRESS_PME_CONTROL)) - host_bridge->native_pme = 0; - if (!(root->osc_control_set & OSC_PCI_EXPRESS_LTR_CONTROL)) - host_bridge->native_ltr = 0; - if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL)) - host_bridge->native_dpc = 0; + + if (!(root->osc_control_set & OSC_PCI_EXPRESS_AER_CONTROL)) { + if (!pcie_ports_native) + host_bridge->native_aer = 0; + else + dev_warn(>dev, "OS overrides %s firmware control", + get_osc_desc(OSC_PCI_EXPRESS_AER_CONTROL)); + } + + if (!(root->osc_control_set & OSC_PCI_EXPRESS_PME_CONTROL)) { + if (!pcie_ports_native) + host_bridge->native_pme = 0; + else + dev_warn(>dev, "OS overrides %s firmware control", + get_osc_desc(OSC_PCI_EXPRESS_PME_CONTROL)); + } + + if (!(root->osc_control_set & OSC_PCI_EXPRESS_LTR_CONTROL)) { + if (!pcie_ports_native) + host_bridge->native_ltr = 0; + else + dev_warn(>dev, "OS overrides %s firmware control", + get_osc_desc(OSC_PCI_EXPRESS_LTR_CONTROL)); + } + + if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL)) { + if (!pcie_ports_native) + host_bridge->native_dpc = 0; + else + dev_warn(>dev, "OS overrides %s firmware control", + get_osc_desc(OSC_PCI_EXPRESS_DPC_CONTROL)); + } /* * Evaluate the "PCI Boot Configuration" _DSM Function. If it diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c index bf779f291f15..5fc999bf6f1b 100644 --- a/drivers/pci/hotplug/pciehp_core.c +++ b/drivers/pci/hotplug/pciehp_core.c @@ -255,7 +255,7 @@ static bool pme_is_native(struct pcie_device *dev) const struct pci_host_bridge *host; host = pci_find_host_bridge(dev->port->bus); - return pcie_ports_native || host->native_pme; + return host->native_pme; } static void pciehp_disable_interrupt(struct pcie_device *dev) diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c index 7224b1e5f2a8..e09589571a9d 100644 --- a/drivers/pci/pci-acpi.c +++ b/drivers/pci/pci-acpi.c @@ -800,9 +800,6 @@ bool pciehp_is_native(struct pci_dev *bridge) if (!(slot_cap & PCI_EXP_SLTCAP_HPC)) return false; - if (pcie_ports_native) - return true; - host = pci_find_host_bridge(bridge->bus); return host->native_pcie_hotplug; } diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c index 3acf56683915..d663bd9c7257 100644 --- a/drivers/pci/pcie/aer.c +++ b/drivers/pci/pcie/aer.c @@ -219,7 +219,7 @@ int
[PATCH v7 4/5] PCI/portdrv: Remove redundant pci_aer_available() check in DPC enable logic
From: Kuppuswamy Sathyanarayanan In DPC service enable logic, check for services & PCIE_PORT_SERVICE_AER implies pci_aer_available() is true. So there is no need to explicitly check it again. Also, passing pcie_ports=dpc-native in kernel command line implies DPC needs to be enabled in native mode irrespective of AER ownership status. So checking for pci_aer_available() without checking for pcie_ports status is incorrect. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/pcie/portdrv_core.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index 2c0278f0fdcc..e257a2ca3595 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -252,7 +252,6 @@ static int get_port_device_capability(struct pci_dev *dev) * permission to use AER. */ if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) && - pci_aer_available() && (host->native_dpc || (services & PCIE_PORT_SERVICE_AER))) services |= PCIE_PORT_SERVICE_DPC; -- 2.17.1
[PATCH v7 0/5] Simplify PCIe native ownership detection logic
From: Kuppuswamy Sathyanarayanan Currently, PCIe capabilities ownership status is detected by verifying the status of pcie_ports_native, pcie_ports_dpc_native and _OSC negotiated results (cached in struct pci_host_bridge ->native_* members). But this logic can be simplified, and we can use only struct pci_host_bridge ->native_* members to detect it. This patchset removes the distributed checks for pcie_ports_native, pcie_ports_dpc_native parameters. Changes since v6: * Created new patch for CONFIG_PCIEPORTBUS check in pci_init_host_bridge(). * Added warning message for a case when pcie_ports_native overrides _OSC negotiation result. Changes since v5: * Rebased on top of v5.8-rc1 Changes since v4: * Changed the patch set title (Original link: https://lkml.org/lkml/2020/5/26/1710) * Added AER/DPC dependency logic cleanup fixes. Kuppuswamy Sathyanarayanan (5): PCI: Conditionally initialize host bridge native_* members ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set. ACPI/PCI: Ignore _OSC DPC negotiation result if pcie_ports_dpc_native is set. PCI/portdrv: Remove redundant pci_aer_available() check in DPC enable logic PCI/DPC: Move AER/DPC dependency checks out of DPC driver drivers/acpi/pci_root.c | 62 ++- drivers/pci/hotplug/pciehp_core.c | 2 +- drivers/pci/pci-acpi.c| 3 -- drivers/pci/pcie/aer.c| 2 +- drivers/pci/pcie/dpc.c| 4 +- drivers/pci/pcie/portdrv.h| 2 - drivers/pci/pcie/portdrv_core.c | 13 +++ drivers/pci/probe.c | 4 +- include/linux/pci.h | 2 + 9 files changed, 65 insertions(+), 29 deletions(-) -- 2.17.1
[PATCH v6 3/4] PCI/portdrv: Remove redundant pci_aer_available() check in DPC enable logic
From: Kuppuswamy Sathyanarayanan In DPC service enable logic, check for services & PCIE_PORT_SERVICE_AER implies pci_aer_available() is true. So there is no need to explicitly check it again. Also, passing pcie_ports=dpc-native in kernel command line implies DPC needs to be enabled in native mode irrespective of AER ownership status. So checking for pci_aer_available() without checking for pcie_ports status is incorrect. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/pcie/portdrv_core.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index 2c0278f0fdcc..e257a2ca3595 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -252,7 +252,6 @@ static int get_port_device_capability(struct pci_dev *dev) * permission to use AER. */ if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) && - pci_aer_available() && (host->native_dpc || (services & PCIE_PORT_SERVICE_AER))) services |= PCIE_PORT_SERVICE_DPC; -- 2.17.1
[PATCH v6 4/4] PCI/DPC: Move AER/DPC dependency checks out of DPC driver
From: Kuppuswamy Sathyanarayanan Currently, AER and DPC Capabilities dependency checks is distributed between DPC and portdrv service drivers. So move them out of DPC driver. Also, since services & PCIE_PORT_SERVICE_AER check already ensures AER native ownership, no need to add additional pcie_aer_is_native() check. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/pcie/dpc.c | 3 --- drivers/pci/pcie/portdrv_core.c | 1 + 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index 5b1025a2994d..3efbe43764f3 100644 --- a/drivers/pci/pcie/dpc.c +++ b/drivers/pci/pcie/dpc.c @@ -285,9 +285,6 @@ static int dpc_probe(struct pcie_device *dev) int status; u16 ctl, cap; - if (!pcie_aer_is_native(pdev) && !host->native_dpc) - return -ENOTSUPP; - status = devm_request_threaded_irq(device, dev->irq, dpc_irq, dpc_handler, IRQF_SHARED, "pcie-dpc", pdev); diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index e257a2ca3595..ffa1d9fc458e 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -252,6 +252,7 @@ static int get_port_device_capability(struct pci_dev *dev) * permission to use AER. */ if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) && + host->native_dpc && (host->native_dpc || (services & PCIE_PORT_SERVICE_AER))) services |= PCIE_PORT_SERVICE_DPC; -- 2.17.1
[PATCH v6 2/4] ACPI/PCI: Ignore _OSC DPC negotiation result if pcie_ports_dpc_native is set.
From: Kuppuswamy Sathyanarayanan pcie_ports_dpc_native is set only if user requests native handling of PCIe DPC capability via pcie_port_setup command line option. User input takes precedence over _OSC based control negotiation result. So consider the _OSC negotiated result for DPC ownership only if pcie_ports_dpc_native is unset. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/acpi/pci_root.c | 2 ++ drivers/pci/pcie/dpc.c | 3 ++- drivers/pci/pcie/portdrv.h | 2 -- drivers/pci/pcie/portdrv_core.c | 2 +- include/linux/pci.h | 2 ++ 5 files changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index 02fab8b0118e..93a26a35aad1 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -925,6 +925,8 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, host_bridge->native_pme = 0; if (!(root->osc_control_set & OSC_PCI_EXPRESS_LTR_CONTROL)) host_bridge->native_ltr = 0; + } + if (!pcie_ports_native && !pcie_ports_dpc_native) { if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL)) host_bridge->native_dpc = 0; } diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index daa9a4153776..5b1025a2994d 100644 --- a/drivers/pci/pcie/dpc.c +++ b/drivers/pci/pcie/dpc.c @@ -280,11 +280,12 @@ void pci_dpc_init(struct pci_dev *pdev) static int dpc_probe(struct pcie_device *dev) { struct pci_dev *pdev = dev->port; + struct pci_host_bridge *host = pci_find_host_bridge(pdev->bus); struct device *device = >device; int status; u16 ctl, cap; - if (!pcie_aer_is_native(pdev) && !pcie_ports_dpc_native) + if (!pcie_aer_is_native(pdev) && !host->native_dpc) return -ENOTSUPP; status = devm_request_threaded_irq(device, dev->irq, dpc_irq, diff --git a/drivers/pci/pcie/portdrv.h b/drivers/pci/pcie/portdrv.h index af7cf237432a..0ac20feef24e 100644 --- a/drivers/pci/pcie/portdrv.h +++ b/drivers/pci/pcie/portdrv.h @@ -25,8 +25,6 @@ #define PCIE_PORT_DEVICE_MAXSERVICES 5 -extern bool pcie_ports_dpc_native; - #ifdef CONFIG_PCIEAER int pcie_aer_init(void); int pcie_aer_is_native(struct pci_dev *dev); diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index ccd5e0ce5605..2c0278f0fdcc 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -253,7 +253,7 @@ static int get_port_device_capability(struct pci_dev *dev) */ if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) && pci_aer_available() && - (pcie_ports_dpc_native || (services & PCIE_PORT_SERVICE_AER))) + (host->native_dpc || (services & PCIE_PORT_SERVICE_AER))) services |= PCIE_PORT_SERVICE_DPC; if (pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM || diff --git a/include/linux/pci.h b/include/linux/pci.h index c79d83304e52..7513b1078cb1 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1556,9 +1556,11 @@ static inline int pci_irqd_intx_xlate(struct irq_domain *d, #ifdef CONFIG_PCIEPORTBUS extern bool pcie_ports_disabled; extern bool pcie_ports_native; +extern bool pcie_ports_dpc_native; #else #define pcie_ports_disabledtrue #define pcie_ports_native false +#define pcie_ports_dpc_native false #endif #define PCIE_LINK_STATE_L0SBIT(0) -- 2.17.1
[PATCH v6 0/4] Simplify PCIe native ownership detection logic
From: Kuppuswamy Sathyanarayanan Currently, PCIe capabilities ownership status is detected by verifying the status of pcie_ports_native, pcie_ports_dpc_native and _OSC negotiated results (cached in struct pci_host_bridge ->native_* members). But this logic can be simplified, and we can use only struct pci_host_bridge ->native_* members to detect it. This patchset removes the distributed checks for pcie_ports_native, pcie_ports_dpc_native parameters. Changes since v5: * Rebased on top of v5.8-rc1 Changes since v4: * Changed the patch set title (Original link: https://lkml.org/lkml/2020/5/26/1710) * Added AER/DPC dependency logic cleanup fixes. Kuppuswamy Sathyanarayanan (4): ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set. ACPI/PCI: Ignore _OSC DPC negotiation result if pcie_ports_dpc_native is set. PCI/portdrv: Remove redundant pci_aer_available() check in DPC enable logic PCI/DPC: Move AER/DPC dependency checks out of DPC driver drivers/acpi/pci_root.c | 28 drivers/pci/hotplug/pciehp_core.c | 2 +- drivers/pci/pci-acpi.c| 3 --- drivers/pci/pcie/aer.c| 2 +- drivers/pci/pcie/dpc.c| 4 +--- drivers/pci/pcie/portdrv.h| 2 -- drivers/pci/pcie/portdrv_core.c | 13 + drivers/pci/probe.c | 5 +++-- include/linux/pci.h | 2 ++ 9 files changed, 29 insertions(+), 32 deletions(-) -- 2.17.1
[PATCH v6 1/4] ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set.
From: Kuppuswamy Sathyanarayanan pcie_ports_native is set only if user requests native handling of PCIe capabilities via pcie_port_setup command line option. User input takes precedence over _OSC based control negotiation result. So consider the _OSC negotiated result only if pcie_ports_native is unset. Also, since struct pci_host_bridge ->native_* members caches the ownership status of various PCIe capabilities, use them instead of distributed checks for pcie_ports_native. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/acpi/pci_root.c | 26 ++ drivers/pci/hotplug/pciehp_core.c | 2 +- drivers/pci/pci-acpi.c| 3 --- drivers/pci/pcie/aer.c| 2 +- drivers/pci/pcie/portdrv_core.c | 9 +++-- drivers/pci/probe.c | 5 +++-- 6 files changed, 22 insertions(+), 25 deletions(-) diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index f90e841c59f5..02fab8b0118e 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -914,18 +914,20 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, goto out_release_info; host_bridge = to_pci_host_bridge(bus->bridge); - if (!(root->osc_control_set & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL)) - host_bridge->native_pcie_hotplug = 0; - if (!(root->osc_control_set & OSC_PCI_SHPC_NATIVE_HP_CONTROL)) - host_bridge->native_shpc_hotplug = 0; - if (!(root->osc_control_set & OSC_PCI_EXPRESS_AER_CONTROL)) - host_bridge->native_aer = 0; - if (!(root->osc_control_set & OSC_PCI_EXPRESS_PME_CONTROL)) - host_bridge->native_pme = 0; - if (!(root->osc_control_set & OSC_PCI_EXPRESS_LTR_CONTROL)) - host_bridge->native_ltr = 0; - if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL)) - host_bridge->native_dpc = 0; + if (!pcie_ports_native) { + if (!(root->osc_control_set & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL)) + host_bridge->native_pcie_hotplug = 0; + if (!(root->osc_control_set & OSC_PCI_SHPC_NATIVE_HP_CONTROL)) + host_bridge->native_shpc_hotplug = 0; + if (!(root->osc_control_set & OSC_PCI_EXPRESS_AER_CONTROL)) + host_bridge->native_aer = 0; + if (!(root->osc_control_set & OSC_PCI_EXPRESS_PME_CONTROL)) + host_bridge->native_pme = 0; + if (!(root->osc_control_set & OSC_PCI_EXPRESS_LTR_CONTROL)) + host_bridge->native_ltr = 0; + if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL)) + host_bridge->native_dpc = 0; + } /* * Evaluate the "PCI Boot Configuration" _DSM Function. If it diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c index bf779f291f15..5fc999bf6f1b 100644 --- a/drivers/pci/hotplug/pciehp_core.c +++ b/drivers/pci/hotplug/pciehp_core.c @@ -255,7 +255,7 @@ static bool pme_is_native(struct pcie_device *dev) const struct pci_host_bridge *host; host = pci_find_host_bridge(dev->port->bus); - return pcie_ports_native || host->native_pme; + return host->native_pme; } static void pciehp_disable_interrupt(struct pcie_device *dev) diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c index 7224b1e5f2a8..e09589571a9d 100644 --- a/drivers/pci/pci-acpi.c +++ b/drivers/pci/pci-acpi.c @@ -800,9 +800,6 @@ bool pciehp_is_native(struct pci_dev *bridge) if (!(slot_cap & PCI_EXP_SLTCAP_HPC)) return false; - if (pcie_ports_native) - return true; - host = pci_find_host_bridge(bridge->bus); return host->native_pcie_hotplug; } diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c index 3acf56683915..d663bd9c7257 100644 --- a/drivers/pci/pcie/aer.c +++ b/drivers/pci/pcie/aer.c @@ -219,7 +219,7 @@ int pcie_aer_is_native(struct pci_dev *dev) if (!dev->aer_cap) return 0; - return pcie_ports_native || host->native_aer; + return host->native_aer; } int pci_enable_pcie_error_reporting(struct pci_dev *dev) diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index 50a9522ab07d..ccd5e0ce5605 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -208,8 +208,7 @@ static int get_port_device_capability(struct pci_dev *dev) struct pci_host_bridge *host = pci_find_host_bridge(dev->bus); int services = 0; - if (dev->is_hotplug_bridge && - (pcie_ports_native || host->native_pcie_hotplug)) { + if (dev->is_hotplug_bridge && host->native_pcie_hotplug) { services |= PCIE_PORT_SERVICE_HP; /* @@ -221,8 +220,7 @@ static int get_port_device_capability(struct pci_dev *dev) } #ifdef
[PATCH v5 1/4] ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set.
From: Kuppuswamy Sathyanarayanan pcie_ports_native is set only if user requests native handling of PCIe capabilities via pcie_port_setup command line option. User input takes precedence over _OSC based control negotiation result. So consider the _OSC negotiated result only if pcie_ports_native is unset. Also, since struct pci_host_bridge ->native_* members caches the ownership status of various PCIe capabilities, use them instead of distributed checks for pcie_ports_native. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/acpi/pci_root.c | 26 ++ drivers/pci/hotplug/pciehp_core.c | 2 +- drivers/pci/pci-acpi.c| 3 --- drivers/pci/pcie/aer.c| 2 +- drivers/pci/pcie/portdrv_core.c | 9 +++-- drivers/pci/probe.c | 4 +++- 6 files changed, 22 insertions(+), 24 deletions(-) diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index 9e235c1a75ff..e0039ad3480a 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -914,18 +914,20 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, goto out_release_info; host_bridge = to_pci_host_bridge(bus->bridge); - if (!(root->osc_control_set & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL)) - host_bridge->native_pcie_hotplug = 0; - if (!(root->osc_control_set & OSC_PCI_SHPC_NATIVE_HP_CONTROL)) - host_bridge->native_shpc_hotplug = 0; - if (!(root->osc_control_set & OSC_PCI_EXPRESS_AER_CONTROL)) - host_bridge->native_aer = 0; - if (!(root->osc_control_set & OSC_PCI_EXPRESS_PME_CONTROL)) - host_bridge->native_pme = 0; - if (!(root->osc_control_set & OSC_PCI_EXPRESS_LTR_CONTROL)) - host_bridge->native_ltr = 0; - if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL)) - host_bridge->native_dpc = 0; + if (!pcie_ports_native) { + if (!(root->osc_control_set & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL)) + host_bridge->native_pcie_hotplug = 0; + if (!(root->osc_control_set & OSC_PCI_SHPC_NATIVE_HP_CONTROL)) + host_bridge->native_shpc_hotplug = 0; + if (!(root->osc_control_set & OSC_PCI_EXPRESS_AER_CONTROL)) + host_bridge->native_aer = 0; + if (!(root->osc_control_set & OSC_PCI_EXPRESS_PME_CONTROL)) + host_bridge->native_pme = 0; + if (!(root->osc_control_set & OSC_PCI_EXPRESS_LTR_CONTROL)) + host_bridge->native_ltr = 0; + if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL)) + host_bridge->native_dpc = 0; + } /* * Evaluate the "PCI Boot Configuration" _DSM Function. If it diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c index 312cc45c44c7..71bc6a6310bb 100644 --- a/drivers/pci/hotplug/pciehp_core.c +++ b/drivers/pci/hotplug/pciehp_core.c @@ -255,7 +255,7 @@ static bool pme_is_native(struct pcie_device *dev) const struct pci_host_bridge *host; host = pci_find_host_bridge(dev->port->bus); - return pcie_ports_native || host->native_pme; + return host->native_pme; } static void pciehp_disable_interrupt(struct pcie_device *dev) diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c index d21969fba6ab..5551877a8a60 100644 --- a/drivers/pci/pci-acpi.c +++ b/drivers/pci/pci-acpi.c @@ -800,9 +800,6 @@ bool pciehp_is_native(struct pci_dev *bridge) if (!(slot_cap & PCI_EXP_SLTCAP_HPC)) return false; - if (pcie_ports_native) - return true; - host = pci_find_host_bridge(bridge->bus); return host->native_pcie_hotplug; } diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c index 3acf56683915..d663bd9c7257 100644 --- a/drivers/pci/pcie/aer.c +++ b/drivers/pci/pcie/aer.c @@ -219,7 +219,7 @@ int pcie_aer_is_native(struct pci_dev *dev) if (!dev->aer_cap) return 0; - return pcie_ports_native || host->native_aer; + return host->native_aer; } int pci_enable_pcie_error_reporting(struct pci_dev *dev) diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index 50a9522ab07d..ccd5e0ce5605 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -208,8 +208,7 @@ static int get_port_device_capability(struct pci_dev *dev) struct pci_host_bridge *host = pci_find_host_bridge(dev->bus); int services = 0; - if (dev->is_hotplug_bridge && - (pcie_ports_native || host->native_pcie_hotplug)) { + if (dev->is_hotplug_bridge && host->native_pcie_hotplug) { services |= PCIE_PORT_SERVICE_HP; /* @@ -221,8 +220,7 @@ static int get_port_device_capability(struct pci_dev *dev) } #ifdef
[PATCH v5 3/4] PCI/portdrv: Remove redundant pci_aer_available() check in DPC enable logic
From: Kuppuswamy Sathyanarayanan In DPC service enable logic, check for services & PCIE_PORT_SERVICE_AER implies pci_aer_available() is true. So there is no need to explicitly check it again. Also, passing pcie_ports=dpc-native in kernel command line implies DPC needs to be enabled in native mode irrespective of AER ownership status. So checking for pci_aer_available() without checking for pcie_ports status is incorrect. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/pcie/portdrv_core.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index 2c0278f0fdcc..e257a2ca3595 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -252,7 +252,6 @@ static int get_port_device_capability(struct pci_dev *dev) * permission to use AER. */ if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) && - pci_aer_available() && (host->native_dpc || (services & PCIE_PORT_SERVICE_AER))) services |= PCIE_PORT_SERVICE_DPC; -- 2.17.1
[PATCH v5 0/4] Simplify PCIe native ownership detection logic
From: Kuppuswamy Sathyanarayanan Currently, PCIe capabilities ownership status is detected by verifying the status of pcie_ports_native, pcie_ports_dpc_native and _OSC negotiated results (cached in struct pci_host_bridge ->native_* members). But this logic can be simplified, and we can use only struct pci_host_bridge ->native_* members to detect it. This patchset removes the distributed checks for pcie_ports_native, pcie_ports_dpc_native parameters. Changes since v4: * Changed the patch set title (Original link: https://lkml.org/lkml/2020/5/26/1710) * Added AER/DPC dependency logic cleanup fixes. Kuppuswamy Sathyanarayanan (4): ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set. ACPI/PCI: Ignore _OSC DPC negotiation result if pcie_ports_dpc_native is set. PCI/portdrv: Remove redundant pci_aer_available() check in DPC enable logic PCI/DPC: Move AER/DPC dependency checks out of DPC driver drivers/acpi/pci_root.c | 28 drivers/pci/hotplug/pciehp_core.c | 2 +- drivers/pci/pci-acpi.c| 3 --- drivers/pci/pcie/aer.c| 2 +- drivers/pci/pcie/dpc.c| 4 +--- drivers/pci/pcie/portdrv.h| 2 -- drivers/pci/pcie/portdrv_core.c | 13 + drivers/pci/probe.c | 4 +++- include/linux/pci.h | 2 ++ 9 files changed, 29 insertions(+), 31 deletions(-) -- 2.17.1
[PATCH v5 2/4] ACPI/PCI: Ignore _OSC DPC negotiation result if pcie_ports_dpc_native is set.
From: Kuppuswamy Sathyanarayanan pcie_ports_dpc_native is set only if user requests native handling of PCIe DPC capability via pcie_port_setup command line option. User input takes precedence over _OSC based control negotiation result. So consider the _OSC negotiated result for DPC ownership only if pcie_ports_dpc_native is unset. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/acpi/pci_root.c | 2 ++ drivers/pci/pcie/dpc.c | 3 ++- drivers/pci/pcie/portdrv.h | 2 -- drivers/pci/pcie/portdrv_core.c | 2 +- include/linux/pci.h | 2 ++ 5 files changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index e0039ad3480a..f90dba464ec2 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -925,6 +925,8 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, host_bridge->native_pme = 0; if (!(root->osc_control_set & OSC_PCI_EXPRESS_LTR_CONTROL)) host_bridge->native_ltr = 0; + } + if (!pcie_ports_native && !pcie_ports_dpc_native) { if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL)) host_bridge->native_dpc = 0; } diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index daa9a4153776..5b1025a2994d 100644 --- a/drivers/pci/pcie/dpc.c +++ b/drivers/pci/pcie/dpc.c @@ -280,11 +280,12 @@ void pci_dpc_init(struct pci_dev *pdev) static int dpc_probe(struct pcie_device *dev) { struct pci_dev *pdev = dev->port; + struct pci_host_bridge *host = pci_find_host_bridge(pdev->bus); struct device *device = >device; int status; u16 ctl, cap; - if (!pcie_aer_is_native(pdev) && !pcie_ports_dpc_native) + if (!pcie_aer_is_native(pdev) && !host->native_dpc) return -ENOTSUPP; status = devm_request_threaded_irq(device, dev->irq, dpc_irq, diff --git a/drivers/pci/pcie/portdrv.h b/drivers/pci/pcie/portdrv.h index af7cf237432a..0ac20feef24e 100644 --- a/drivers/pci/pcie/portdrv.h +++ b/drivers/pci/pcie/portdrv.h @@ -25,8 +25,6 @@ #define PCIE_PORT_DEVICE_MAXSERVICES 5 -extern bool pcie_ports_dpc_native; - #ifdef CONFIG_PCIEAER int pcie_aer_init(void); int pcie_aer_is_native(struct pci_dev *dev); diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index ccd5e0ce5605..2c0278f0fdcc 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -253,7 +253,7 @@ static int get_port_device_capability(struct pci_dev *dev) */ if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) && pci_aer_available() && - (pcie_ports_dpc_native || (services & PCIE_PORT_SERVICE_AER))) + (host->native_dpc || (services & PCIE_PORT_SERVICE_AER))) services |= PCIE_PORT_SERVICE_DPC; if (pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM || diff --git a/include/linux/pci.h b/include/linux/pci.h index 83ce1cdf5676..07d4db97f5f4 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1547,9 +1547,11 @@ static inline int pci_irqd_intx_xlate(struct irq_domain *d, #ifdef CONFIG_PCIEPORTBUS extern bool pcie_ports_disabled; extern bool pcie_ports_native; +extern bool pcie_ports_dpc_native; #else #define pcie_ports_disabledtrue #define pcie_ports_native false +#define pcie_ports_dpc_native false #endif #define PCIE_LINK_STATE_L0SBIT(0) -- 2.17.1
[PATCH v5 4/4] PCI/DPC: Move AER/DPC dependency checks out of DPC driver
From: Kuppuswamy Sathyanarayanan Currently, AER and DPC Capabilities dependency checks is distributed between DPC and portdrv service drivers. So move them out of DPC driver. Also, since services & PCIE_PORT_SERVICE_AER check already ensures AER native ownership, no need to add additional pcie_aer_is_native() check. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/pcie/dpc.c | 3 --- drivers/pci/pcie/portdrv_core.c | 1 + 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index 5b1025a2994d..3efbe43764f3 100644 --- a/drivers/pci/pcie/dpc.c +++ b/drivers/pci/pcie/dpc.c @@ -285,9 +285,6 @@ static int dpc_probe(struct pcie_device *dev) int status; u16 ctl, cap; - if (!pcie_aer_is_native(pdev) && !host->native_dpc) - return -ENOTSUPP; - status = devm_request_threaded_irq(device, dev->irq, dpc_irq, dpc_handler, IRQF_SHARED, "pcie-dpc", pdev); diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index e257a2ca3595..ffa1d9fc458e 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -252,6 +252,7 @@ static int get_port_device_capability(struct pci_dev *dev) * permission to use AER. */ if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) && + host->native_dpc && (host->native_dpc || (services & PCIE_PORT_SERVICE_AER))) services |= PCIE_PORT_SERVICE_DPC; -- 2.17.1
[PATCH v2 2/2] PCI/ERR: Add reset support for non fatal errors
From: Kuppuswamy Sathyanarayanan PCI_ERS_RESULT_NEED_RESET error status implies the device is requesting a slot reset and a notification about slot reset completion via ->slot_reset() callback. But in non-fatal errors case, if report_error_detected() or report_mmio_enabled() functions requests PCI_ERS_RESULT_NEED_RESET then current pcie_do_recovery() implementation does not do the requested explicit slot reset, instead just calls the ->slot_reset() callback on all affected devices. Notifying about the slot reset completion without resetting it incorrect. So add this support. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/pcie/err.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c index 5fe8561c7185..94d1c2ff7b40 100644 --- a/drivers/pci/pcie/err.c +++ b/drivers/pci/pcie/err.c @@ -206,6 +206,9 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev, * functions to reset slot before calling * drivers' slot_reset callbacks? */ + if (state != pci_channel_io_frozen) + pci_reset_bus(dev); + status = PCI_ERS_RESULT_RECOVERED; pci_dbg(dev, "broadcast slot_reset message\n"); pci_walk_bus(bus, report_slot_reset, ); -- 2.17.1
[PATCH v2 1/2] PCI/ERR: Fix fatal error recovery for non-hotplug capable devices
From: Kuppuswamy Sathyanarayanan Fatal (DPC) error recovery is currently broken for non-hotplug capable devices. With current implementation, after successful fatal error recovery, non-hotplug capable device state won't be restored properly. You can find related issues in following links. https://lkml.org/lkml/2020/5/27/290 https://lore.kernel.org/linux-pci/12115.1588207324@famine/ https://lkml.org/lkml/2020/3/28/328 Current fatal error recovery implementation relies on hotplug handler for detaching/re-enumerating the affected devices/drivers on DLLSC state changes. So when dealing with non-hotplug capable devices, recovery code does not restore the state of the affected devices correctly. Correct implementation should call report_slot_reset() function after resetting the link to restore the state of the device/driver. So use PCI_ERS_RESULT_NEED_RESET as error status for successful reset_link() operation and use PCI_ERS_RESULT_DISCONNECT for failure case. PCI_ERS_RESULT_NEED_RESET error state will ensure slot_reset() is called after reset link operation which will also fix the above mentioned issue. [original patch is from jay.vosbu...@canonical.com] [original patch link https://lore.kernel.org/linux-pci/12115.1588207324@famine/] Fixes: 6d2c89441571 ("PCI/ERR: Update error status after reset_link()") Signed-off-by: Jay Vosburgh Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/pcie/err.c | 24 ++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c index 14bb8f54723e..5fe8561c7185 100644 --- a/drivers/pci/pcie/err.c +++ b/drivers/pci/pcie/err.c @@ -165,8 +165,28 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev, pci_dbg(dev, "broadcast error_detected message\n"); if (state == pci_channel_io_frozen) { pci_walk_bus(bus, report_frozen_detected, ); - status = reset_link(dev); - if (status != PCI_ERS_RESULT_RECOVERED) { + /* +* After resetting the link using reset_link() call, the +* possible value of error status is either +* PCI_ERS_RESULT_DISCONNECT (failure case) or +* PCI_ERS_RESULT_NEED_RESET (success case). +* So ignore the return value of report_error_detected() +* call for fatal errors. Instead use +* PCI_ERS_RESULT_NEED_RESET as initial status value. +* +* Ignoring the status return value of report_error_detected() +* call will also help in case of EDR mode based error +* recovery. In EDR mode AER and DPC Capabilities are owned by +* firmware and hence report_error_detected() call will possibly +* return PCI_ERS_RESULT_NO_AER_DRIVER. So if we don't ignore +* the return value of report_error_detected() then +* pcie_do_recovery() would report incorrect status after +* successful recovery. Ignoring PCI_ERS_RESULT_NO_AER_DRIVER +* in non EDR case should not have any functional impact. +*/ + status = PCI_ERS_RESULT_NEED_RESET; + if (reset_link(dev) != PCI_ERS_RESULT_RECOVERED) { + status = PCI_ERS_RESULT_DISCONNECT; pci_warn(dev, "link reset failed\n"); goto failed; } -- 2.17.1
[PATCH v4 0/5] Remove AER HEST table parser
From: Kuppuswamy Sathyanarayanan Commit c100beb9ccfb ("PCI/AER: Use only _OSC to determine AER ownership") removed HEST dependency in determining the AER ownership status. The following patch set cleansup rest of the HEST table related code from AER driver. This patchset also includes some minor AER driver fixes. Changes since v3: * Fixed compilation issues reported by kbuild test robot. Changes since v2: * Fixed commit sha id for patch "PCI/AER: Use only _OSC to determine AER ownership". Kuppuswamy Sathyanarayanan (5): PCI/AER: Remove redundant pci_is_pcie() checks. PCI/AER: Remove redundant dev->aer_cap checks. ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set. ACPI/PCI: Ignore _OSC DPC negotiation result if pcie_ports_dpc_native is set. PCI/AER: Replace pcie_aer_get_firmware_first() with pcie_aer_is_native() drivers/acpi/pci_root.c| 28 drivers/pci/pcie/aer.c | 139 - drivers/pci/pcie/dpc.c | 2 +- drivers/pci/pcie/portdrv.h | 15 +--- include/linux/pci.h| 2 + 5 files changed, 34 insertions(+), 152 deletions(-) -- 2.17.1
[PATCH v4 1/5] PCI/AER: Remove redundant pci_is_pcie() checks.
From: Kuppuswamy Sathyanarayanan AER is a PCIe Extended Capability. So checking for dev->aer_cap will implicitly include check for PCIe device. So remove redundant pci_is_pcie() checks. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/pcie/aer.c | 11 +-- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c index efc26773cc6d..7c4294454df0 100644 --- a/drivers/pci/pcie/aer.c +++ b/drivers/pci/pcie/aer.c @@ -139,9 +139,6 @@ static int enable_ecrc_checking(struct pci_dev *dev) int pos; u32 reg32; - if (!pci_is_pcie(dev)) - return -ENODEV; - pos = dev->aer_cap; if (!pos) return -ENODEV; @@ -167,9 +164,6 @@ static int disable_ecrc_checking(struct pci_dev *dev) int pos; u32 reg32; - if (!pci_is_pcie(dev)) - return -ENODEV; - pos = dev->aer_cap; if (!pos) return -ENODEV; @@ -308,7 +302,7 @@ static void aer_set_firmware_first(struct pci_dev *pci_dev) int pcie_aer_get_firmware_first(struct pci_dev *dev) { - if (!pci_is_pcie(dev)) + if (!dev->aer_cap) return 0; if (pcie_ports_native) @@ -411,9 +405,6 @@ int pci_aer_raw_clear_status(struct pci_dev *dev) u32 status; int port_type; - if (!pci_is_pcie(dev)) - return -ENODEV; - pos = dev->aer_cap; if (!pos) return -EIO; -- 2.17.1
[PATCH v4 2/5] PCI/AER: Remove redundant dev->aer_cap checks.
From: Kuppuswamy Sathyanarayanan pcie_aer_get_firmware_first() includes check for dev->aer_cap. So remove redundant dev->aer_cap checks. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/pcie/aer.c | 15 ++- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c index 7c4294454df0..5f5ffe2f0986 100644 --- a/drivers/pci/pcie/aer.c +++ b/drivers/pci/pcie/aer.c @@ -322,9 +322,6 @@ int pci_enable_pcie_error_reporting(struct pci_dev *dev) if (pcie_aer_get_firmware_first(dev)) return -EIO; - if (!dev->aer_cap) - return -EIO; - return pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_AER_FLAGS); } EXPORT_SYMBOL_GPL(pci_enable_pcie_error_reporting); @@ -349,13 +346,9 @@ void pci_aer_clear_device_status(struct pci_dev *dev) int pci_aer_clear_nonfatal_status(struct pci_dev *dev) { - int pos; + int pos = dev->aer_cap; u32 status, sev; - pos = dev->aer_cap; - if (!pos) - return -EIO; - if (pcie_aer_get_firmware_first(dev)) return -EIO; @@ -372,13 +365,9 @@ EXPORT_SYMBOL_GPL(pci_aer_clear_nonfatal_status); void pci_aer_clear_fatal_status(struct pci_dev *dev) { - int pos; + int pos = dev->aer_cap; u32 status, sev; - pos = dev->aer_cap; - if (!pos) - return; - if (pcie_aer_get_firmware_first(dev)) return; -- 2.17.1
[PATCH v4 4/5] ACPI/PCI: Ignore _OSC DPC negotiation result if pcie_ports_dpc_native is set.
From: Kuppuswamy Sathyanarayanan pcie_ports_dpc_native is set only if user requests native handling of PCIe DPC capability via pcie_port_setup command line option. User input takes precedence over _OSC based control negotiation result. So consider the _OSC negotiated result for DPC ownership only if pcie_ports_dpc_native is unset. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/acpi/pci_root.c| 2 ++ drivers/pci/pcie/portdrv.h | 2 -- include/linux/pci.h| 2 ++ 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index e0039ad3480a..f90dba464ec2 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -925,6 +925,8 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, host_bridge->native_pme = 0; if (!(root->osc_control_set & OSC_PCI_EXPRESS_LTR_CONTROL)) host_bridge->native_ltr = 0; + } + if (!pcie_ports_native && !pcie_ports_dpc_native) { if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL)) host_bridge->native_dpc = 0; } diff --git a/drivers/pci/pcie/portdrv.h b/drivers/pci/pcie/portdrv.h index 64b5e081cdb2..e4999f24ad92 100644 --- a/drivers/pci/pcie/portdrv.h +++ b/drivers/pci/pcie/portdrv.h @@ -25,8 +25,6 @@ #define PCIE_PORT_DEVICE_MAXSERVICES 5 -extern bool pcie_ports_dpc_native; - #ifdef CONFIG_PCIEAER int pcie_aer_init(void); #else diff --git a/include/linux/pci.h b/include/linux/pci.h index 83ce1cdf5676..07d4db97f5f4 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1547,9 +1547,11 @@ static inline int pci_irqd_intx_xlate(struct irq_domain *d, #ifdef CONFIG_PCIEPORTBUS extern bool pcie_ports_disabled; extern bool pcie_ports_native; +extern bool pcie_ports_dpc_native; #else #define pcie_ports_disabledtrue #define pcie_ports_native false +#define pcie_ports_dpc_native false #endif #define PCIE_LINK_STATE_L0SBIT(0) -- 2.17.1
[PATCH v4 3/5] ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set.
From: Kuppuswamy Sathyanarayanan pcie_ports_native is set only if user requests native handling of PCIe capabilities via pcie_port_setup command line option. User input takes precedence over _OSC based control negotiation result. So consider the _OSC negotiated result only if pcie_ports_native is unset. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/acpi/pci_root.c | 26 ++ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index 9e235c1a75ff..e0039ad3480a 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -914,18 +914,20 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, goto out_release_info; host_bridge = to_pci_host_bridge(bus->bridge); - if (!(root->osc_control_set & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL)) - host_bridge->native_pcie_hotplug = 0; - if (!(root->osc_control_set & OSC_PCI_SHPC_NATIVE_HP_CONTROL)) - host_bridge->native_shpc_hotplug = 0; - if (!(root->osc_control_set & OSC_PCI_EXPRESS_AER_CONTROL)) - host_bridge->native_aer = 0; - if (!(root->osc_control_set & OSC_PCI_EXPRESS_PME_CONTROL)) - host_bridge->native_pme = 0; - if (!(root->osc_control_set & OSC_PCI_EXPRESS_LTR_CONTROL)) - host_bridge->native_ltr = 0; - if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL)) - host_bridge->native_dpc = 0; + if (!pcie_ports_native) { + if (!(root->osc_control_set & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL)) + host_bridge->native_pcie_hotplug = 0; + if (!(root->osc_control_set & OSC_PCI_SHPC_NATIVE_HP_CONTROL)) + host_bridge->native_shpc_hotplug = 0; + if (!(root->osc_control_set & OSC_PCI_EXPRESS_AER_CONTROL)) + host_bridge->native_aer = 0; + if (!(root->osc_control_set & OSC_PCI_EXPRESS_PME_CONTROL)) + host_bridge->native_pme = 0; + if (!(root->osc_control_set & OSC_PCI_EXPRESS_LTR_CONTROL)) + host_bridge->native_ltr = 0; + if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL)) + host_bridge->native_dpc = 0; + } /* * Evaluate the "PCI Boot Configuration" _DSM Function. If it -- 2.17.1
[PATCH v4 5/5] PCI/AER: Replace pcie_aer_get_firmware_first() with pcie_aer_is_native()
From: Kuppuswamy Sathyanarayanan Commit c100beb9ccfb ("PCI/AER: Use only _OSC to determine AER ownership") removed the dependency of HEST table in determining the status of AER ownership. But AER driver still uses HEST table parsed result in verifying the AER ownership status in some of its API's. So remove HEST table dependency, and instead use pcie_aer_is_native() to verify the AER native ownership status. Also remove unused HEST table parsing helper functions from AER driver. We can reintroduce HEST table parser once the usage of FIRMWARE_FIRST bit is clarified in PCI/AER specification. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/pcie/aer.c | 113 - drivers/pci/pcie/dpc.c | 2 +- drivers/pci/pcie/portdrv.h | 13 + 3 files changed, 13 insertions(+), 115 deletions(-) diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c index 5f5ffe2f0986..12fa67c9ed9c 100644 --- a/drivers/pci/pcie/aer.c +++ b/drivers/pci/pcie/aer.c @@ -211,115 +211,22 @@ void pcie_ecrc_get_policy(char *str) } #endif /* CONFIG_PCIE_ECRC */ -#ifdef CONFIG_ACPI_APEI -static inline int hest_match_pci(struct acpi_hest_aer_common *p, -struct pci_dev *pci) -{ - return ACPI_HEST_SEGMENT(p->bus) == pci_domain_nr(pci->bus) && -ACPI_HEST_BUS(p->bus) == pci->bus->number && -p->device == PCI_SLOT(pci->devfn) && -p->function == PCI_FUNC(pci->devfn); -} - -static inline bool hest_match_type(struct acpi_hest_header *hest_hdr, - struct pci_dev *dev) -{ - u16 hest_type = hest_hdr->type; - u8 pcie_type = pci_pcie_type(dev); - - if ((hest_type == ACPI_HEST_TYPE_AER_ROOT_PORT && - pcie_type == PCI_EXP_TYPE_ROOT_PORT) || - (hest_type == ACPI_HEST_TYPE_AER_ENDPOINT && - pcie_type == PCI_EXP_TYPE_ENDPOINT) || - (hest_type == ACPI_HEST_TYPE_AER_BRIDGE && - (dev->class >> 16) == PCI_BASE_CLASS_BRIDGE)) - return true; - return false; -} - -struct aer_hest_parse_info { - struct pci_dev *pci_dev; - int firmware_first; -}; - -static int hest_source_is_pcie_aer(struct acpi_hest_header *hest_hdr) -{ - if (hest_hdr->type == ACPI_HEST_TYPE_AER_ROOT_PORT || - hest_hdr->type == ACPI_HEST_TYPE_AER_ENDPOINT || - hest_hdr->type == ACPI_HEST_TYPE_AER_BRIDGE) - return 1; - return 0; -} - -static int aer_hest_parse(struct acpi_hest_header *hest_hdr, void *data) -{ - struct aer_hest_parse_info *info = data; - struct acpi_hest_aer_common *p; - int ff; - - if (!hest_source_is_pcie_aer(hest_hdr)) - return 0; - - p = (struct acpi_hest_aer_common *)(hest_hdr + 1); - ff = !!(p->flags & ACPI_HEST_FIRMWARE_FIRST); - - /* -* If no specific device is supplied, determine whether -* FIRMWARE_FIRST is set for *any* PCIe device. -*/ - if (!info->pci_dev) { - info->firmware_first |= ff; - return 0; - } - - /* Otherwise, check the specific device */ - if (p->flags & ACPI_HEST_GLOBAL) { - if (hest_match_type(hest_hdr, info->pci_dev)) - info->firmware_first = ff; - } else - if (hest_match_pci(p, info->pci_dev)) - info->firmware_first = ff; - - return 0; -} +#definePCI_EXP_AER_FLAGS (PCI_EXP_DEVCTL_CERE | PCI_EXP_DEVCTL_NFERE | \ +PCI_EXP_DEVCTL_FERE | PCI_EXP_DEVCTL_URRE) -static void aer_set_firmware_first(struct pci_dev *pci_dev) +int pcie_aer_is_native(struct pci_dev *dev) { - int rc; - struct aer_hest_parse_info info = { - .pci_dev= pci_dev, - .firmware_first = 0, - }; - - rc = apei_hest_parse(aer_hest_parse, ); + struct pci_host_bridge *host = pci_find_host_bridge(dev->bus); - if (rc) - pci_dev->__aer_firmware_first = 0; - else - pci_dev->__aer_firmware_first = info.firmware_first; - pci_dev->__aer_firmware_first_valid = 1; -} - -int pcie_aer_get_firmware_first(struct pci_dev *dev) -{ if (!dev->aer_cap) return 0; - if (pcie_ports_native) - return 0; - - if (!dev->__aer_firmware_first_valid) - aer_set_firmware_first(dev); - return dev->__aer_firmware_first; + return host->native_aer; } -#endif - -#definePCI_EXP_AER_FLAGS (PCI_EXP_DEVCTL_CERE | PCI_EXP_DEVCTL_NFERE | \ -PCI_EXP_DEVCTL_FERE | PCI_EXP_DEVCTL_URRE) int pci_enable_pcie_error_reporting(struct pci_dev *dev) { - if (pcie_aer_get_firmware_first(dev)) + if (!pcie_aer_is_native(dev)) return -EIO; return
[PATCH v3 1/5] PCI/AER: Remove redundant pci_is_pcie() checks.
From: Kuppuswamy Sathyanarayanan AER is a PCIe Extended Capability. So checking for dev->aer_cap will implicitly include check for PCIe device. So remove redundant pci_is_pcie() checks. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/pcie/aer.c | 11 +-- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c index efc26773cc6d..7c4294454df0 100644 --- a/drivers/pci/pcie/aer.c +++ b/drivers/pci/pcie/aer.c @@ -139,9 +139,6 @@ static int enable_ecrc_checking(struct pci_dev *dev) int pos; u32 reg32; - if (!pci_is_pcie(dev)) - return -ENODEV; - pos = dev->aer_cap; if (!pos) return -ENODEV; @@ -167,9 +164,6 @@ static int disable_ecrc_checking(struct pci_dev *dev) int pos; u32 reg32; - if (!pci_is_pcie(dev)) - return -ENODEV; - pos = dev->aer_cap; if (!pos) return -ENODEV; @@ -308,7 +302,7 @@ static void aer_set_firmware_first(struct pci_dev *pci_dev) int pcie_aer_get_firmware_first(struct pci_dev *dev) { - if (!pci_is_pcie(dev)) + if (!dev->aer_cap) return 0; if (pcie_ports_native) @@ -411,9 +405,6 @@ int pci_aer_raw_clear_status(struct pci_dev *dev) u32 status; int port_type; - if (!pci_is_pcie(dev)) - return -ENODEV; - pos = dev->aer_cap; if (!pos) return -EIO; -- 2.17.1
[PATCH v3 0/5] Remove AER HEST table parser
From: Kuppuswamy Sathyanarayanan Commit c100beb9ccfb ("PCI/AER: Use only _OSC to determine AER ownership") removed HEST dependency in determining the AER ownership status. The following patch set cleansup rest of the HEST table related code from AER driver. This patchset also includes some minor AER driver fixes. Changes since v2: * Fixed commit sha id for patch "PCI/AER: Use only _OSC to determine AER ownership". Kuppuswamy Sathyanarayanan (5): PCI/AER: Remove redundant pci_is_pcie() checks. PCI/AER: Remove redundant dev->aer_cap checks. ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set. ACPI/PCI: Ignore _OSC DPC negotiation result if pcie_ports_dpc_native is set. PCI/AER: Replace pcie_aer_get_firmware_first() with pcie_aer_is_native() drivers/acpi/pci_root.c| 28 drivers/pci/pcie/aer.c | 139 - drivers/pci/pcie/dpc.c | 2 +- drivers/pci/pcie/portdrv.h | 13 +--- include/linux/pci.h| 2 + 5 files changed, 34 insertions(+), 150 deletions(-) -- 2.17.1
[PATCH v3 4/5] ACPI/PCI: Ignore _OSC DPC negotiation result if pcie_ports_dpc_native is set.
From: Kuppuswamy Sathyanarayanan pcie_ports_dpc_native is set only if user requests native handling of PCIe DPC capability via pcie_port_setup command line option. User input takes precedence over _OSC based control negotiation result. So consider the _OSC negotiated result for DPC ownership only if pcie_ports_dpc_native is unset. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/acpi/pci_root.c | 2 ++ include/linux/pci.h | 2 ++ 2 files changed, 4 insertions(+) diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index e0039ad3480a..f90dba464ec2 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -925,6 +925,8 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, host_bridge->native_pme = 0; if (!(root->osc_control_set & OSC_PCI_EXPRESS_LTR_CONTROL)) host_bridge->native_ltr = 0; + } + if (!pcie_ports_native && !pcie_ports_dpc_native) { if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL)) host_bridge->native_dpc = 0; } diff --git a/include/linux/pci.h b/include/linux/pci.h index 83ce1cdf5676..07d4db97f5f4 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1547,9 +1547,11 @@ static inline int pci_irqd_intx_xlate(struct irq_domain *d, #ifdef CONFIG_PCIEPORTBUS extern bool pcie_ports_disabled; extern bool pcie_ports_native; +extern bool pcie_ports_dpc_native; #else #define pcie_ports_disabledtrue #define pcie_ports_native false +#define pcie_ports_dpc_native false #endif #define PCIE_LINK_STATE_L0SBIT(0) -- 2.17.1
[PATCH v3 5/5] PCI/AER: Replace pcie_aer_get_firmware_first() with pcie_aer_is_native()
From: Kuppuswamy Sathyanarayanan Commit c100beb9ccfb ("PCI/AER: Use only _OSC to determine AER ownership") removed the dependency of HEST table in determining the status of AER ownership. But AER driver still uses HEST table parsed result in verifying the AER ownership status in some of its API's. So remove HEST table dependency, and instead use pcie_aer_is_native() to verify the AER native ownership status. Also remove unused HEST table parsing helper functions from AER driver. We can reintroduce HEST table parser once the usage of FIRMWARE_FIRST bit is clarified in PCI/AER specification. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/pcie/aer.c | 113 - drivers/pci/pcie/dpc.c | 2 +- drivers/pci/pcie/portdrv.h | 13 + 3 files changed, 13 insertions(+), 115 deletions(-) diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c index 5f5ffe2f0986..12fa67c9ed9c 100644 --- a/drivers/pci/pcie/aer.c +++ b/drivers/pci/pcie/aer.c @@ -211,115 +211,22 @@ void pcie_ecrc_get_policy(char *str) } #endif /* CONFIG_PCIE_ECRC */ -#ifdef CONFIG_ACPI_APEI -static inline int hest_match_pci(struct acpi_hest_aer_common *p, -struct pci_dev *pci) -{ - return ACPI_HEST_SEGMENT(p->bus) == pci_domain_nr(pci->bus) && -ACPI_HEST_BUS(p->bus) == pci->bus->number && -p->device == PCI_SLOT(pci->devfn) && -p->function == PCI_FUNC(pci->devfn); -} - -static inline bool hest_match_type(struct acpi_hest_header *hest_hdr, - struct pci_dev *dev) -{ - u16 hest_type = hest_hdr->type; - u8 pcie_type = pci_pcie_type(dev); - - if ((hest_type == ACPI_HEST_TYPE_AER_ROOT_PORT && - pcie_type == PCI_EXP_TYPE_ROOT_PORT) || - (hest_type == ACPI_HEST_TYPE_AER_ENDPOINT && - pcie_type == PCI_EXP_TYPE_ENDPOINT) || - (hest_type == ACPI_HEST_TYPE_AER_BRIDGE && - (dev->class >> 16) == PCI_BASE_CLASS_BRIDGE)) - return true; - return false; -} - -struct aer_hest_parse_info { - struct pci_dev *pci_dev; - int firmware_first; -}; - -static int hest_source_is_pcie_aer(struct acpi_hest_header *hest_hdr) -{ - if (hest_hdr->type == ACPI_HEST_TYPE_AER_ROOT_PORT || - hest_hdr->type == ACPI_HEST_TYPE_AER_ENDPOINT || - hest_hdr->type == ACPI_HEST_TYPE_AER_BRIDGE) - return 1; - return 0; -} - -static int aer_hest_parse(struct acpi_hest_header *hest_hdr, void *data) -{ - struct aer_hest_parse_info *info = data; - struct acpi_hest_aer_common *p; - int ff; - - if (!hest_source_is_pcie_aer(hest_hdr)) - return 0; - - p = (struct acpi_hest_aer_common *)(hest_hdr + 1); - ff = !!(p->flags & ACPI_HEST_FIRMWARE_FIRST); - - /* -* If no specific device is supplied, determine whether -* FIRMWARE_FIRST is set for *any* PCIe device. -*/ - if (!info->pci_dev) { - info->firmware_first |= ff; - return 0; - } - - /* Otherwise, check the specific device */ - if (p->flags & ACPI_HEST_GLOBAL) { - if (hest_match_type(hest_hdr, info->pci_dev)) - info->firmware_first = ff; - } else - if (hest_match_pci(p, info->pci_dev)) - info->firmware_first = ff; - - return 0; -} +#definePCI_EXP_AER_FLAGS (PCI_EXP_DEVCTL_CERE | PCI_EXP_DEVCTL_NFERE | \ +PCI_EXP_DEVCTL_FERE | PCI_EXP_DEVCTL_URRE) -static void aer_set_firmware_first(struct pci_dev *pci_dev) +int pcie_aer_is_native(struct pci_dev *dev) { - int rc; - struct aer_hest_parse_info info = { - .pci_dev= pci_dev, - .firmware_first = 0, - }; - - rc = apei_hest_parse(aer_hest_parse, ); + struct pci_host_bridge *host = pci_find_host_bridge(dev->bus); - if (rc) - pci_dev->__aer_firmware_first = 0; - else - pci_dev->__aer_firmware_first = info.firmware_first; - pci_dev->__aer_firmware_first_valid = 1; -} - -int pcie_aer_get_firmware_first(struct pci_dev *dev) -{ if (!dev->aer_cap) return 0; - if (pcie_ports_native) - return 0; - - if (!dev->__aer_firmware_first_valid) - aer_set_firmware_first(dev); - return dev->__aer_firmware_first; + return host->native_aer; } -#endif - -#definePCI_EXP_AER_FLAGS (PCI_EXP_DEVCTL_CERE | PCI_EXP_DEVCTL_NFERE | \ -PCI_EXP_DEVCTL_FERE | PCI_EXP_DEVCTL_URRE) int pci_enable_pcie_error_reporting(struct pci_dev *dev) { - if (pcie_aer_get_firmware_first(dev)) + if (!pcie_aer_is_native(dev)) return -EIO; return
[PATCH v3 3/5] ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set.
From: Kuppuswamy Sathyanarayanan pcie_ports_native is set only if user requests native handling of PCIe capabilities via pcie_port_setup command line option. User input takes precedence over _OSC based control negotiation result. So consider the _OSC negotiated result only if pcie_ports_native is unset. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/acpi/pci_root.c | 26 ++ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index 9e235c1a75ff..e0039ad3480a 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -914,18 +914,20 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, goto out_release_info; host_bridge = to_pci_host_bridge(bus->bridge); - if (!(root->osc_control_set & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL)) - host_bridge->native_pcie_hotplug = 0; - if (!(root->osc_control_set & OSC_PCI_SHPC_NATIVE_HP_CONTROL)) - host_bridge->native_shpc_hotplug = 0; - if (!(root->osc_control_set & OSC_PCI_EXPRESS_AER_CONTROL)) - host_bridge->native_aer = 0; - if (!(root->osc_control_set & OSC_PCI_EXPRESS_PME_CONTROL)) - host_bridge->native_pme = 0; - if (!(root->osc_control_set & OSC_PCI_EXPRESS_LTR_CONTROL)) - host_bridge->native_ltr = 0; - if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL)) - host_bridge->native_dpc = 0; + if (!pcie_ports_native) { + if (!(root->osc_control_set & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL)) + host_bridge->native_pcie_hotplug = 0; + if (!(root->osc_control_set & OSC_PCI_SHPC_NATIVE_HP_CONTROL)) + host_bridge->native_shpc_hotplug = 0; + if (!(root->osc_control_set & OSC_PCI_EXPRESS_AER_CONTROL)) + host_bridge->native_aer = 0; + if (!(root->osc_control_set & OSC_PCI_EXPRESS_PME_CONTROL)) + host_bridge->native_pme = 0; + if (!(root->osc_control_set & OSC_PCI_EXPRESS_LTR_CONTROL)) + host_bridge->native_ltr = 0; + if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL)) + host_bridge->native_dpc = 0; + } /* * Evaluate the "PCI Boot Configuration" _DSM Function. If it -- 2.17.1
[PATCH v3 2/5] PCI/AER: Remove redundant dev->aer_cap checks.
From: Kuppuswamy Sathyanarayanan pcie_aer_get_firmware_first() includes check for dev->aer_cap. So remove redundant dev->aer_cap checks. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/pcie/aer.c | 15 ++- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c index 7c4294454df0..5f5ffe2f0986 100644 --- a/drivers/pci/pcie/aer.c +++ b/drivers/pci/pcie/aer.c @@ -322,9 +322,6 @@ int pci_enable_pcie_error_reporting(struct pci_dev *dev) if (pcie_aer_get_firmware_first(dev)) return -EIO; - if (!dev->aer_cap) - return -EIO; - return pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_AER_FLAGS); } EXPORT_SYMBOL_GPL(pci_enable_pcie_error_reporting); @@ -349,13 +346,9 @@ void pci_aer_clear_device_status(struct pci_dev *dev) int pci_aer_clear_nonfatal_status(struct pci_dev *dev) { - int pos; + int pos = dev->aer_cap; u32 status, sev; - pos = dev->aer_cap; - if (!pos) - return -EIO; - if (pcie_aer_get_firmware_first(dev)) return -EIO; @@ -372,13 +365,9 @@ EXPORT_SYMBOL_GPL(pci_aer_clear_nonfatal_status); void pci_aer_clear_fatal_status(struct pci_dev *dev) { - int pos; + int pos = dev->aer_cap; u32 status, sev; - pos = dev->aer_cap; - if (!pos) - return; - if (pcie_aer_get_firmware_first(dev)) return; -- 2.17.1
[PATCH v2 4/5] ACPI/PCI: Ignore _OSC DPC negotiation result if pcie_ports_dpc_native is set.
From: Kuppuswamy Sathyanarayanan pcie_ports_dpc_native is set only if user requests native handling of PCIe DPC capability via pcie_port_setup command line option. User input takes precedence over _OSC based control negotiation result. So consider the _OSC negotiated result for DPC ownership only if pcie_ports_dpc_native is unset. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/acpi/pci_root.c | 2 ++ include/linux/pci.h | 2 ++ 2 files changed, 4 insertions(+) diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index e0039ad3480a..f90dba464ec2 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -925,6 +925,8 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, host_bridge->native_pme = 0; if (!(root->osc_control_set & OSC_PCI_EXPRESS_LTR_CONTROL)) host_bridge->native_ltr = 0; + } + if (!pcie_ports_native && !pcie_ports_dpc_native) { if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL)) host_bridge->native_dpc = 0; } diff --git a/include/linux/pci.h b/include/linux/pci.h index 83ce1cdf5676..07d4db97f5f4 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1547,9 +1547,11 @@ static inline int pci_irqd_intx_xlate(struct irq_domain *d, #ifdef CONFIG_PCIEPORTBUS extern bool pcie_ports_disabled; extern bool pcie_ports_native; +extern bool pcie_ports_dpc_native; #else #define pcie_ports_disabledtrue #define pcie_ports_native false +#define pcie_ports_dpc_native false #endif #define PCIE_LINK_STATE_L0SBIT(0) -- 2.17.1
[PATCH v2 3/5] ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set.
From: Kuppuswamy Sathyanarayanan pcie_ports_native is set only if user requests native handling of PCIe capabilities via pcie_port_setup command line option. User input takes precedence over _OSC based control negotiation result. So consider the _OSC negotiated result only if pcie_ports_native is unset. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/acpi/pci_root.c | 26 ++ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index 9e235c1a75ff..e0039ad3480a 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -914,18 +914,20 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, goto out_release_info; host_bridge = to_pci_host_bridge(bus->bridge); - if (!(root->osc_control_set & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL)) - host_bridge->native_pcie_hotplug = 0; - if (!(root->osc_control_set & OSC_PCI_SHPC_NATIVE_HP_CONTROL)) - host_bridge->native_shpc_hotplug = 0; - if (!(root->osc_control_set & OSC_PCI_EXPRESS_AER_CONTROL)) - host_bridge->native_aer = 0; - if (!(root->osc_control_set & OSC_PCI_EXPRESS_PME_CONTROL)) - host_bridge->native_pme = 0; - if (!(root->osc_control_set & OSC_PCI_EXPRESS_LTR_CONTROL)) - host_bridge->native_ltr = 0; - if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL)) - host_bridge->native_dpc = 0; + if (!pcie_ports_native) { + if (!(root->osc_control_set & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL)) + host_bridge->native_pcie_hotplug = 0; + if (!(root->osc_control_set & OSC_PCI_SHPC_NATIVE_HP_CONTROL)) + host_bridge->native_shpc_hotplug = 0; + if (!(root->osc_control_set & OSC_PCI_EXPRESS_AER_CONTROL)) + host_bridge->native_aer = 0; + if (!(root->osc_control_set & OSC_PCI_EXPRESS_PME_CONTROL)) + host_bridge->native_pme = 0; + if (!(root->osc_control_set & OSC_PCI_EXPRESS_LTR_CONTROL)) + host_bridge->native_ltr = 0; + if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL)) + host_bridge->native_dpc = 0; + } /* * Evaluate the "PCI Boot Configuration" _DSM Function. If it -- 2.17.1
[PATCH v2 1/5] PCI/AER: Remove redundant pci_is_pcie() checks.
From: Kuppuswamy Sathyanarayanan AER is a PCIe Extended Capability. So checking for dev->aer_cap will implicitly include check for PCIe device. So remove redundant pci_is_pcie() checks. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/pcie/aer.c | 11 +-- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c index efc26773cc6d..7c4294454df0 100644 --- a/drivers/pci/pcie/aer.c +++ b/drivers/pci/pcie/aer.c @@ -139,9 +139,6 @@ static int enable_ecrc_checking(struct pci_dev *dev) int pos; u32 reg32; - if (!pci_is_pcie(dev)) - return -ENODEV; - pos = dev->aer_cap; if (!pos) return -ENODEV; @@ -167,9 +164,6 @@ static int disable_ecrc_checking(struct pci_dev *dev) int pos; u32 reg32; - if (!pci_is_pcie(dev)) - return -ENODEV; - pos = dev->aer_cap; if (!pos) return -ENODEV; @@ -308,7 +302,7 @@ static void aer_set_firmware_first(struct pci_dev *pci_dev) int pcie_aer_get_firmware_first(struct pci_dev *dev) { - if (!pci_is_pcie(dev)) + if (!dev->aer_cap) return 0; if (pcie_ports_native) @@ -411,9 +405,6 @@ int pci_aer_raw_clear_status(struct pci_dev *dev) u32 status; int port_type; - if (!pci_is_pcie(dev)) - return -ENODEV; - pos = dev->aer_cap; if (!pos) return -EIO; -- 2.17.1
[PATCH v2 5/5] PCI/AER: Replace pcie_aer_get_firmware_first() with pcie_aer_is_native()
From: Kuppuswamy Sathyanarayanan Commit e7909188dd4d ("PCI/AER: Use only _OSC to determine AER ownership") removed the dependency of HEST table in determining the status of AER ownership. But AER driver still uses HEST table parsed result in verifying the AER ownership status in some of its API's. So remove HEST table dependency, and instead use pcie_aer_is_native() to verify the AER native ownership status. Also remove unused HEST table parsing helper functions from AER driver. We can reintroduce HEST table parser once the usage of FIRMWARE_FIRST bit is clarified in PCI/AER specification. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/pcie/aer.c | 113 - drivers/pci/pcie/dpc.c | 2 +- drivers/pci/pcie/portdrv.h | 13 + 3 files changed, 13 insertions(+), 115 deletions(-) diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c index 5f5ffe2f0986..12fa67c9ed9c 100644 --- a/drivers/pci/pcie/aer.c +++ b/drivers/pci/pcie/aer.c @@ -211,115 +211,22 @@ void pcie_ecrc_get_policy(char *str) } #endif /* CONFIG_PCIE_ECRC */ -#ifdef CONFIG_ACPI_APEI -static inline int hest_match_pci(struct acpi_hest_aer_common *p, -struct pci_dev *pci) -{ - return ACPI_HEST_SEGMENT(p->bus) == pci_domain_nr(pci->bus) && -ACPI_HEST_BUS(p->bus) == pci->bus->number && -p->device == PCI_SLOT(pci->devfn) && -p->function == PCI_FUNC(pci->devfn); -} - -static inline bool hest_match_type(struct acpi_hest_header *hest_hdr, - struct pci_dev *dev) -{ - u16 hest_type = hest_hdr->type; - u8 pcie_type = pci_pcie_type(dev); - - if ((hest_type == ACPI_HEST_TYPE_AER_ROOT_PORT && - pcie_type == PCI_EXP_TYPE_ROOT_PORT) || - (hest_type == ACPI_HEST_TYPE_AER_ENDPOINT && - pcie_type == PCI_EXP_TYPE_ENDPOINT) || - (hest_type == ACPI_HEST_TYPE_AER_BRIDGE && - (dev->class >> 16) == PCI_BASE_CLASS_BRIDGE)) - return true; - return false; -} - -struct aer_hest_parse_info { - struct pci_dev *pci_dev; - int firmware_first; -}; - -static int hest_source_is_pcie_aer(struct acpi_hest_header *hest_hdr) -{ - if (hest_hdr->type == ACPI_HEST_TYPE_AER_ROOT_PORT || - hest_hdr->type == ACPI_HEST_TYPE_AER_ENDPOINT || - hest_hdr->type == ACPI_HEST_TYPE_AER_BRIDGE) - return 1; - return 0; -} - -static int aer_hest_parse(struct acpi_hest_header *hest_hdr, void *data) -{ - struct aer_hest_parse_info *info = data; - struct acpi_hest_aer_common *p; - int ff; - - if (!hest_source_is_pcie_aer(hest_hdr)) - return 0; - - p = (struct acpi_hest_aer_common *)(hest_hdr + 1); - ff = !!(p->flags & ACPI_HEST_FIRMWARE_FIRST); - - /* -* If no specific device is supplied, determine whether -* FIRMWARE_FIRST is set for *any* PCIe device. -*/ - if (!info->pci_dev) { - info->firmware_first |= ff; - return 0; - } - - /* Otherwise, check the specific device */ - if (p->flags & ACPI_HEST_GLOBAL) { - if (hest_match_type(hest_hdr, info->pci_dev)) - info->firmware_first = ff; - } else - if (hest_match_pci(p, info->pci_dev)) - info->firmware_first = ff; - - return 0; -} +#definePCI_EXP_AER_FLAGS (PCI_EXP_DEVCTL_CERE | PCI_EXP_DEVCTL_NFERE | \ +PCI_EXP_DEVCTL_FERE | PCI_EXP_DEVCTL_URRE) -static void aer_set_firmware_first(struct pci_dev *pci_dev) +int pcie_aer_is_native(struct pci_dev *dev) { - int rc; - struct aer_hest_parse_info info = { - .pci_dev= pci_dev, - .firmware_first = 0, - }; - - rc = apei_hest_parse(aer_hest_parse, ); + struct pci_host_bridge *host = pci_find_host_bridge(dev->bus); - if (rc) - pci_dev->__aer_firmware_first = 0; - else - pci_dev->__aer_firmware_first = info.firmware_first; - pci_dev->__aer_firmware_first_valid = 1; -} - -int pcie_aer_get_firmware_first(struct pci_dev *dev) -{ if (!dev->aer_cap) return 0; - if (pcie_ports_native) - return 0; - - if (!dev->__aer_firmware_first_valid) - aer_set_firmware_first(dev); - return dev->__aer_firmware_first; + return host->native_aer; } -#endif - -#definePCI_EXP_AER_FLAGS (PCI_EXP_DEVCTL_CERE | PCI_EXP_DEVCTL_NFERE | \ -PCI_EXP_DEVCTL_FERE | PCI_EXP_DEVCTL_URRE) int pci_enable_pcie_error_reporting(struct pci_dev *dev) { - if (pcie_aer_get_firmware_first(dev)) + if (!pcie_aer_is_native(dev)) return -EIO; return
[PATCH v2 2/5] PCI/AER: Remove redundant dev->aer_cap checks.
From: Kuppuswamy Sathyanarayanan pcie_aer_get_firmware_first() includes check for dev->aer_cap. So remove redundant dev->aer_cap checks. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/pcie/aer.c | 15 ++- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c index 7c4294454df0..5f5ffe2f0986 100644 --- a/drivers/pci/pcie/aer.c +++ b/drivers/pci/pcie/aer.c @@ -322,9 +322,6 @@ int pci_enable_pcie_error_reporting(struct pci_dev *dev) if (pcie_aer_get_firmware_first(dev)) return -EIO; - if (!dev->aer_cap) - return -EIO; - return pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_AER_FLAGS); } EXPORT_SYMBOL_GPL(pci_enable_pcie_error_reporting); @@ -349,13 +346,9 @@ void pci_aer_clear_device_status(struct pci_dev *dev) int pci_aer_clear_nonfatal_status(struct pci_dev *dev) { - int pos; + int pos = dev->aer_cap; u32 status, sev; - pos = dev->aer_cap; - if (!pos) - return -EIO; - if (pcie_aer_get_firmware_first(dev)) return -EIO; @@ -372,13 +365,9 @@ EXPORT_SYMBOL_GPL(pci_aer_clear_nonfatal_status); void pci_aer_clear_fatal_status(struct pci_dev *dev) { - int pos; + int pos = dev->aer_cap; u32 status, sev; - pos = dev->aer_cap; - if (!pos) - return; - if (pcie_aer_get_firmware_first(dev)) return; -- 2.17.1
[PATCH v2 0/5] Remove AER HEST table parser
From: Kuppuswamy Sathyanarayanan Commit e7909188dd4d ("PCI/AER: Use only _OSC to determine AER ownership") removed HEST dependency in determining the AER ownership status. The following patch set cleansup rest of the HEST table related code from AER driver. This patchset also includes some minor AER driver fixes. Kuppuswamy Sathyanarayanan (5): PCI/AER: Remove redundant pci_is_pcie() checks. PCI/AER: Remove redundant dev->aer_cap checks. ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set. ACPI/PCI: Ignore _OSC DPC negotiation result if pcie_ports_dpc_native is set. PCI/AER: Replace pcie_aer_get_firmware_first() with pcie_aer_is_native() drivers/acpi/pci_root.c| 28 drivers/pci/pcie/aer.c | 139 - drivers/pci/pcie/dpc.c | 2 +- drivers/pci/pcie/portdrv.h | 13 +--- include/linux/pci.h| 2 + 5 files changed, 34 insertions(+), 150 deletions(-) -- 2.17.1
[PATCH v1 1/1] PCI/ERR: Handle fatal error recovery for non-hotplug capable devices
From: Kuppuswamy Sathyanarayanan If there are non-hotplug capable devices connected to a given port, then during the fatal error recovery(triggered by DPC or AER), after calling reset_link() function, we cannot rely on hotplug handler to detach and re-enumerate the device drivers in the affected bus. Instead, we will have to let the error recovery handler call report_slot_reset() for all devices in the bus to notify about the reset operation. Although this is only required for non hot-plug capable devices, doing it for hotplug capable devices should not affect the functionality. Along with above issue, this fix also applicable to following issue. Commit 6d2c89441571 ("PCI/ERR: Update error status after reset_link()") added support to store status of reset_link() call. Although this fixed the error recovery issue observed if the initial value of error status is PCI_ERS_RESULT_DISCONNECT or PCI_ERS_RESULT_NO_AER_DRIVER, it also discarded the status result from report_frozen_detected. This can cause a failure to recover if _NEED_RESET is returned by report_frozen_detected and report_slot_reset is not invoked. Such an event can be induced for testing purposes by reducing the Max_Payload_Size of a PCIe bridge to less than that of a device downstream from the bridge, and then initiating I/O through the device, resulting in oversize transactions. In the presence of DPC, this results in a containment event and attempted reset and recovery via pcie_do_recovery. After 6d2c89441571 report_slot_reset is not invoked, and the device does not recover. [original patch is from jay.vosbu...@canonical.com] [original patch link https://lore.kernel.org/linux-pci/18609.1588812972@famine/] Fixes: 6d2c89441571 ("PCI/ERR: Update error status after reset_link()") Signed-off-by: Jay Vosburgh Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/pcie/err.c | 19 +++ 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c index 14bb8f54723e..db80e1ecb2dc 100644 --- a/drivers/pci/pcie/err.c +++ b/drivers/pci/pcie/err.c @@ -165,13 +165,24 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev, pci_dbg(dev, "broadcast error_detected message\n"); if (state == pci_channel_io_frozen) { pci_walk_bus(bus, report_frozen_detected, ); - status = reset_link(dev); - if (status != PCI_ERS_RESULT_RECOVERED) { + status = PCI_ERS_RESULT_NEED_RESET; + } else { + pci_walk_bus(bus, report_normal_detected, ); + } + + if (status == PCI_ERS_RESULT_NEED_RESET) { + if (reset_link) { + if (reset_link(dev) != PCI_ERS_RESULT_RECOVERED) + status = PCI_ERS_RESULT_DISCONNECT; + } else { + if (pci_bus_error_reset(dev)) + status = PCI_ERS_RESULT_DISCONNECT; + } + + if (status == PCI_ERS_RESULT_DISCONNECT) { pci_warn(dev, "link reset failed\n"); goto failed; } - } else { - pci_walk_bus(bus, report_normal_detected, ); } if (status == PCI_ERS_RESULT_CAN_RECOVER) { -- 2.17.1
[PATCH v1 1/1] PCI/ATS: Optimize pci_prg_resp_pasid_required() function
From: Kuppuswamy Sathyanarayanan Currently, pci_prg_resp_pasid_required() function reads the PASID Required bit status from register every time we call the function. Since PASID Required bit is a read-only value, instead of reading it from register every time, read it once and cache it in struct pci_dev. Also, since we are caching PASID Required bit in pci_pri_init() function, move the caching of PRI Capability check result to the same function. This will group all PRI related caching at one place. Since "pasid_required" structure member is protected by CONFIG_PRI, its users should also be protected by same #ifdef. So correct the #ifdef dependency of pci_prg_resp_pasid_required() function. Signed-off-by: Kuppuswamy Sathyanarayanan Cc: Ashok Raj Cc: Keith Busch --- drivers/pci/ats.c | 50 - include/linux/pci.h | 1 + 2 files changed, 28 insertions(+), 23 deletions(-) diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c index cb4f62da7b8a..2b5df5ea208f 100644 --- a/drivers/pci/ats.c +++ b/drivers/pci/ats.c @@ -16,6 +16,24 @@ #include "pci.h" +static void pci_pri_init(struct pci_dev *pdev) +{ +#ifdef CONFIG_PCI_PRI + int pos; + u16 status; + + pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI); + if (!pos) + return; + + pdev->pri_cap = pos; + + pci_read_config_word(pdev, pos + PCI_PRI_STATUS, ); + if (status & PCI_PRI_STATUS_PASID) + pdev->pasid_required = 1; +#endif +} + void pci_ats_init(struct pci_dev *dev) { int pos; @@ -28,6 +46,8 @@ void pci_ats_init(struct pci_dev *dev) return; dev->ats_cap = pos; + + pci_pri_init(dev); } /** @@ -185,12 +205,8 @@ int pci_enable_pri(struct pci_dev *pdev, u32 reqs) if (WARN_ON(pdev->pri_enabled)) return -EBUSY; - if (!pri) { - pri = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI); - if (!pri) - return -EINVAL; - pdev->pri_cap = pri; - } + if (!pri) + return -EINVAL; pci_read_config_word(pdev, pri + PCI_PRI_STATUS, ); if (!(status & PCI_PRI_STATUS_STOPPED)) @@ -425,6 +441,7 @@ int pci_pasid_features(struct pci_dev *pdev) } EXPORT_SYMBOL_GPL(pci_pasid_features); +#ifdef CONFIG_PCI_PRI /** * pci_prg_resp_pasid_required - Return PRG Response PASID Required bit * status. @@ -432,31 +449,18 @@ EXPORT_SYMBOL_GPL(pci_pasid_features); * * Returns 1 if PASID is required in PRG Response Message, 0 otherwise. * - * Even though the PRG response PASID status is read from PRI Status - * Register, since this API will mainly be used by PASID users, this - * function is defined within #ifdef CONFIG_PCI_PASID instead of - * CONFIG_PCI_PRI. + * Since this API has dependency on both PRI and PASID, protect it + * with both CONFIG_PCI_PRI and CONFIG_PCI_PASID. */ int pci_prg_resp_pasid_required(struct pci_dev *pdev) { - u16 status; - int pri; - if (pdev->is_virtfn) pdev = pci_physfn(pdev); - pri = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI); - if (!pri) - return 0; - - pci_read_config_word(pdev, pri + PCI_PRI_STATUS, ); - - if (status & PCI_PRI_STATUS_PASID) - return 1; - - return 0; + return pdev->pasid_required; } EXPORT_SYMBOL_GPL(pci_prg_resp_pasid_required); +#endif /* CONFIG_PCI_PRI */ #define PASID_NUMBER_SHIFT 8 #define PASID_NUMBER_MASK (0x1f << PASID_NUMBER_SHIFT) diff --git a/include/linux/pci.h b/include/linux/pci.h index 6542100bd2dd..f1131fee7fcd 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -456,6 +456,7 @@ struct pci_dev { #ifdef CONFIG_PCI_PRI u16 pri_cap;/* PRI Capability offset */ u32 pri_reqs_alloc; /* Number of PRI requests allocated */ + unsigned intpasid_required:1; /* PRG Response PASID Required bit status */ #endif #ifdef CONFIG_PCI_PASID u16 pasid_cap; /* PASID Capability offset */ -- 2.21.0
[PATCH v9 8/8] PCI/ACPI: Enable EDR support
From: Kuppuswamy Sathyanarayanan As per PCI firmware specification r3.2 Downstream Port Containment Related Enhancements ECN, sec 4.5.1, OS must implement following steps to enable/use EDR feature. 1. OS can use bit 7 of _OSC Control Field to negotiate control over Downstream Port Containment (DPC) configuration of PCIe port. After _OSC negotiation, firmware will Set this bit to grant OS control over PCIe DPC configuration and Clear it if this feature was requested and denied, or was not requested. 2. Also, if OS supports EDR, it should expose its support to BIOS by setting bit 7 of _OSC Support Field. And if OS sets bit 7 of _OSC Control Field it must also expose support for EDR by setting bit 7 of _OSC Support Field. Cc: Bjorn Helgaas Cc: "Rafael J. Wysocki" Cc: Len Brown Signed-off-by: Kuppuswamy Sathyanarayanan Acked-by: Keith Busch Tested-by: Huong Nguyen Tested-by: Austin Bolen --- drivers/acpi/pci_root.c | 9 + drivers/pci/pcie/portdrv_core.c | 8 +++- drivers/pci/probe.c | 1 + include/linux/acpi.h| 6 -- include/linux/pci.h | 3 ++- 5 files changed, 23 insertions(+), 4 deletions(-) diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index d1e666ef3fcc..134e20474dfd 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -131,6 +131,7 @@ static struct pci_osc_bit_struct pci_osc_support_bit[] = { { OSC_PCI_CLOCK_PM_SUPPORT, "ClockPM" }, { OSC_PCI_SEGMENT_GROUPS_SUPPORT, "Segments" }, { OSC_PCI_MSI_SUPPORT, "MSI" }, + { OSC_PCI_EDR_SUPPORT, "EDR" }, { OSC_PCI_HPX_TYPE_3_SUPPORT, "HPX-Type3" }, }; @@ -141,6 +142,7 @@ static struct pci_osc_bit_struct pci_osc_control_bit[] = { { OSC_PCI_EXPRESS_AER_CONTROL, "AER" }, { OSC_PCI_EXPRESS_CAPABILITY_CONTROL, "PCIeCapability" }, { OSC_PCI_EXPRESS_LTR_CONTROL, "LTR" }, + { OSC_PCI_EXPRESS_DPC_CONTROL, "DPC" }, }; static void decode_osc_bits(struct acpi_pci_root *root, char *msg, u32 word, @@ -440,6 +442,8 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm, support |= OSC_PCI_ASPM_SUPPORT | OSC_PCI_CLOCK_PM_SUPPORT; if (pci_msi_enabled()) support |= OSC_PCI_MSI_SUPPORT; + if (IS_ENABLED(CONFIG_PCIE_EDR)) + support |= OSC_PCI_EDR_SUPPORT; decode_osc_support(root, "OS supports", support); status = acpi_pci_osc_support(root, support); @@ -487,6 +491,9 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm, control |= OSC_PCI_EXPRESS_AER_CONTROL; } + if (IS_ENABLED(CONFIG_PCIE_DPC)) + control |= OSC_PCI_EXPRESS_DPC_CONTROL; + requested = control; status = acpi_pci_osc_control_set(handle, , OSC_PCI_EXPRESS_CAPABILITY_CONTROL); @@ -916,6 +923,8 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, host_bridge->native_pme = 0; if (!(root->osc_control_set & OSC_PCI_EXPRESS_LTR_CONTROL)) host_bridge->native_ltr = 0; + if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL)) + host_bridge->native_dpc = 0; /* * Evaluate the "PCI Boot Configuration" _DSM Function. If it diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index 1b330129089f..1b54a39df795 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -250,8 +250,14 @@ static int get_port_device_capability(struct pci_dev *dev) pcie_pme_interrupt_enable(dev, false); } + /* +* If EDR support is enabled in OS, then even if AER is not handled in +* OS, DPC service can be enabled. +*/ if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) && - pci_aer_available() && services & PCIE_PORT_SERVICE_AER) + ((IS_ENABLED(CONFIG_PCIE_EDR) && !host->native_dpc) || + (pci_aer_available() && services & PCIE_PORT_SERVICE_AER && + (pcie_ports_native || host->native_dpc services |= PCIE_PORT_SERVICE_DPC; if (pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM || diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 3d5271a7a849..54be2f93eba3 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -596,6 +596,7 @@ static void pci_init_host_bridge(struct pci_host_bridge *bridge) bridge->native_shpc_hotplug = 1; bridge->native_pme = 1; bridge->native_ltr = 1; + bridge->native_dpc = 1; } struct pci_host_bridge *pci_alloc_host_bridge(size_t priv) diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 8b4e516bac00..71452d4959ec 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -515,8 +515,9 @@ extern bool osc_pc_lpi_support_confirmed; #define OSC_PCI_CLOCK_PM_SUPPORT
[PATCH v9 2/8] PCI/DPC: Allow dpc_probe() even if firmware first mode is enabled
From: Kuppuswamy Sathyanarayanan As per ACPI specification v6.3, sec 5.6.6, Error Disconnect Recover (EDR) notification used by firmware to let OS know about the DPC event and permit OS to perform error recovery when processing the EDR notification. Also, as per PCI firmware specification r3.2 Downstream Port Containment Related Enhancements ECN, sec 4.5.1, table 4-6, if DPC is controlled by firmware (firmware first mode), it's responsible for initializing Downstream Port Containment Extended Capability Structures per firmware policy. And, OS is permitted to read or write DPC Control and Status registers of a port while processing an Error Disconnect Recover (EDR) notification from firmware on that port. Currently, if firmware controls DPC (firmware first mode), OS will not create/enumerate DPC PCIe port services. But, if OS supports EDR feature, then as mentioned in above spec references, it should permit enumeration of DPC driver and also support handling ACPI EDR notification. So as first step, allow dpc_probe() to continue even if firmware first mode is enabled. Also add appropriate checks to ensure device registers are not modified outside EDR notification window in firmware first mode. This is a preparatory patch for adding EDR support. Signed-off-by: Kuppuswamy Sathyanarayanan Acked-by: Keith Busch --- drivers/pci/pcie/dpc.c | 49 +++--- 1 file changed, 36 insertions(+), 13 deletions(-) diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index a32ec3487a8d..9717fda012f8 100644 --- a/drivers/pci/pcie/dpc.c +++ b/drivers/pci/pcie/dpc.c @@ -22,6 +22,8 @@ struct dpc_dev { u16 cap_pos; boolrp_extensions; u8 rp_log_size; + /* Set True if DPC is controlled by firmware */ + boolfirmware_dpc; }; static const char * const rp_pio_error_string[] = { @@ -69,6 +71,9 @@ void pci_save_dpc_state(struct pci_dev *dev) if (!dpc) return; + if (dpc->firmware_dpc) + return; + save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_DPC); if (!save_state) return; @@ -90,6 +95,9 @@ void pci_restore_dpc_state(struct pci_dev *dev) if (!dpc) return; + if (dpc->firmware_dpc) + return; + save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_DPC); if (!save_state) return; @@ -291,9 +299,6 @@ static int dpc_probe(struct pcie_device *dev) int status; u16 ctl, cap; - if (pcie_aer_get_firmware_first(pdev)) - return -ENOTSUPP; - dpc = devm_kzalloc(device, sizeof(*dpc), GFP_KERNEL); if (!dpc) return -ENOMEM; @@ -302,13 +307,25 @@ static int dpc_probe(struct pcie_device *dev) dpc->dev = dev; set_service_data(dev, dpc); - status = devm_request_threaded_irq(device, dev->irq, dpc_irq, - dpc_handler, IRQF_SHARED, - "pcie-dpc", dpc); - if (status) { - pci_warn(pdev, "request IRQ%d failed: %d\n", dev->irq, -status); - return status; + if (pcie_aer_get_firmware_first(pdev)) + dpc->firmware_dpc = 1; + + /* +* If DPC is handled in firmware and ACPI support is not enabled +* in OS, skip probe and return error. +*/ + if (dpc->firmware_dpc && !IS_ENABLED(CONFIG_ACPI)) + return -ENODEV; + + if (!dpc->firmware_dpc) { + status = devm_request_threaded_irq(device, dev->irq, dpc_irq, + dpc_handler, IRQF_SHARED, + "pcie-dpc", dpc); + if (status) { + pci_warn(pdev, "request IRQ%d failed: %d\n", dev->irq, +status); + return status; + } } pci_read_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_CAP, ); @@ -323,9 +340,12 @@ static int dpc_probe(struct pcie_device *dev) dpc->rp_log_size = 0; } } - - ctl = (ctl & 0xfff4) | PCI_EXP_DPC_CTL_EN_FATAL | PCI_EXP_DPC_CTL_INT_EN; - pci_write_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_CTL, ctl); + if (!dpc->firmware_dpc) { + ctl = (ctl & 0xfff4) | + (PCI_EXP_DPC_CTL_EN_FATAL | PCI_EXP_DPC_CTL_INT_EN); + pci_write_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_CTL, + ctl); + } pci_info(pdev, "error containment capabilities: Int Msg #%d, RPExt%c PoisonedTLP%c SwTrigger%c RP PIO Log %d, DL_ActiveErr%c\n", cap & PCI_EXP_DPC_IRQ, FLAG(cap, PCI_EXP_DPC_CAP_RP_EXT), @@ -343,6 +363,9 @@
[PATCH v9 7/8] PCI/DPC: Clear AER registers in EDR mode
From: Kuppuswamy Sathyanarayanan As per PCI firmware specification r3.2 Downstream Port Containment Related Enhancements ECN, OS is responsible for clearing the AER registers in EDR mode. So clear AER registers in dpc_process_error() function. Signed-off-by: Kuppuswamy Sathyanarayanan Acked-by: Keith Busch --- drivers/pci/pcie/dpc.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index fafc55c00fe0..de2d892bc7c4 100644 --- a/drivers/pci/pcie/dpc.c +++ b/drivers/pci/pcie/dpc.c @@ -275,6 +275,10 @@ static void dpc_process_error(struct dpc_dev *dpc) pci_aer_clear_fatal_status(pdev); } + /* In EDR mode, OS is responsible for clearing AER registers */ + if (dpc->firmware_dpc) + pci_cleanup_aer_error_status_regs(pdev); + /* * Irrespective of whether the DPC event is triggered by * ERR_FATAL or ERR_NONFATAL, since the link is already down, -- 2.21.0
[PATCH v9 1/8] PCI/ERR: Update error status after reset_link()
From: Kuppuswamy Sathyanarayanan Commit bdb5ac85777d ("PCI/ERR: Handle fatal error recovery") uses reset_link() to recover from fatal errors. But, if the reset is successful there is no need to continue the rest of the error recovery checks. Also, during fatal error recovery, if the initial value of error status is PCI_ERS_RESULT_DISCONNECT or PCI_ERS_RESULT_NO_AER_DRIVER then even after successful recovery (using reset_link()) pcie_do_recovery() will report the recovery result as failure. So update the status of error after reset_link(). Fixes: bdb5ac85777d ("PCI/ERR: Handle fatal error recovery") Cc: Ashok Raj Cc: Keith Busch Signed-off-by: Kuppuswamy Sathyanarayanan Acked-by: Keith Busch --- drivers/pci/pcie/err.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c index b0e6048a9208..53cd9200ec2c 100644 --- a/drivers/pci/pcie/err.c +++ b/drivers/pci/pcie/err.c @@ -204,9 +204,12 @@ void pcie_do_recovery(struct pci_dev *dev, enum pci_channel_state state, else pci_walk_bus(bus, report_normal_detected, ); - if (state == pci_channel_io_frozen && - reset_link(dev, service) != PCI_ERS_RESULT_RECOVERED) - goto failed; + if (state == pci_channel_io_frozen) { + status = reset_link(dev, service); + if (status != PCI_ERS_RESULT_RECOVERED) + goto failed; + goto done; + } if (status == PCI_ERS_RESULT_CAN_RECOVER) { status = PCI_ERS_RESULT_RECOVERED; @@ -228,6 +231,7 @@ void pcie_do_recovery(struct pci_dev *dev, enum pci_channel_state state, if (status != PCI_ERS_RESULT_RECOVERED) goto failed; +done: pci_dbg(dev, "broadcast resume message\n"); pci_walk_bus(bus, report_resume, ); -- 2.21.0
[PATCH v9 6/8] PCI/DPC: Update comments related to DPC recovery on NON_FATAL errors
From: Kuppuswamy Sathyanarayanan Currently, in native mode, DPC driver is configured to trigger DPC only for FATAL errors and hence it only supports port recovery for FATAL errors. But with Error Disconnect Recover (EDR) support, DPC configuration is done by firmware, and hence we should expect DPC triggered for both FATAL/NON_FATAL errors. So update comments and add details about how NON_FATAL dpc recovery is handled. Signed-off-by: Kuppuswamy Sathyanarayanan Acked-by: Keith Busch --- drivers/pci/pcie/dpc.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index a8b634e05694..fafc55c00fe0 100644 --- a/drivers/pci/pcie/dpc.c +++ b/drivers/pci/pcie/dpc.c @@ -275,7 +275,11 @@ static void dpc_process_error(struct dpc_dev *dpc) pci_aer_clear_fatal_status(pdev); } - /* We configure DPC so it only triggers on ERR_FATAL */ + /* +* Irrespective of whether the DPC event is triggered by +* ERR_FATAL or ERR_NONFATAL, since the link is already down, +* use the FATAL error recovery path for both cases. +*/ pcie_do_recovery(pdev, pci_channel_io_frozen, PCIE_PORT_SERVICE_DPC); } -- 2.21.0
[PATCH v9 3/8] PCI/DPC: Add dpc_process_error() wrapper function
From: Kuppuswamy Sathyanarayanan With Error Disconnect Recover (EDR) support, we need to support processing DPC event either from DPC IRQ or ACPI EDR event. So create a wrapper function dpc_process_error() and move common error handling code in to it. It will be used to process the DPC event in both DPC IRQ and EDR ACPI event contexts. Signed-off-by: Kuppuswamy Sathyanarayanan Acked-by: Keith Busch --- drivers/pci/pcie/dpc.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index 9717fda012f8..4137ec7b48cc 100644 --- a/drivers/pci/pcie/dpc.c +++ b/drivers/pci/pcie/dpc.c @@ -232,10 +232,9 @@ static int dpc_get_aer_uncorrect_severity(struct pci_dev *dev, return 1; } -static irqreturn_t dpc_handler(int irq, void *context) +static void dpc_process_error(struct dpc_dev *dpc) { struct aer_err_info info; - struct dpc_dev *dpc = context; struct pci_dev *pdev = dpc->dev->port; u16 cap = dpc->cap_pos, status, source, reason, ext_reason; @@ -268,6 +267,13 @@ static irqreturn_t dpc_handler(int irq, void *context) /* We configure DPC so it only triggers on ERR_FATAL */ pcie_do_recovery(pdev, pci_channel_io_frozen, PCIE_PORT_SERVICE_DPC); +} + +static irqreturn_t dpc_handler(int irq, void *context) +{ + struct dpc_dev *dpc = context; + + dpc_process_error(dpc); return IRQ_HANDLED; } -- 2.21.0
[PATCH v9 5/8] PCI/AER: Allow clearing Error Status Register in FF mode
From: Kuppuswamy Sathyanarayanan As per PCI firmware specification r3.2 Downstream Port Containment Related Enhancements ECN, sec 4.5.1, table 4-6, Error Disconnect Recover (EDR) support allows OS to handle error recovery and clearing Error Registers even in FF mode. So remove FF mode checks in pci_cleanup_aer_uncorrect_error_status(), pci_aer_clear_fatal_status() and pci_cleanup_aer_error_status_regs() functions. Signed-off-by: Kuppuswamy Sathyanarayanan Acked-by: Keith Busch --- drivers/pci/pcie/aer.c | 12 ++-- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c index b45bc47d04fe..e1509bb900c5 100644 --- a/drivers/pci/pcie/aer.c +++ b/drivers/pci/pcie/aer.c @@ -383,9 +383,6 @@ int pci_cleanup_aer_uncorrect_error_status(struct pci_dev *dev) if (!pos) return -EIO; - if (pcie_aer_get_firmware_first(dev)) - return -EIO; - /* Clear status bits for ERR_NONFATAL errors only */ pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_STATUS, ); pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_SEVER, ); @@ -406,9 +403,6 @@ void pci_aer_clear_fatal_status(struct pci_dev *dev) if (!pos) return; - if (pcie_aer_get_firmware_first(dev)) - return; - /* Clear status bits for ERR_FATAL errors only */ pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_STATUS, ); pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_SEVER, ); @@ -430,9 +424,6 @@ int pci_cleanup_aer_error_status_regs(struct pci_dev *dev) if (!pos) return -EIO; - if (pcie_aer_get_firmware_first(dev)) - return -EIO; - port_type = pci_pcie_type(dev); if (port_type == PCI_EXP_TYPE_ROOT_PORT) { pci_read_config_dword(dev, pos + PCI_ERR_ROOT_STATUS, ); @@ -455,7 +446,8 @@ void pci_aer_init(struct pci_dev *dev) if (dev->aer_cap) dev->aer_stats = kzalloc(sizeof(struct aer_stats), GFP_KERNEL); - pci_cleanup_aer_error_status_regs(dev); + if (!pcie_aer_get_firmware_first(dev)) + pci_cleanup_aer_error_status_regs(dev); } void pci_aer_exit(struct pci_dev *dev) -- 2.21.0
[PATCH v9 4/8] PCI/DPC: Add Error Disconnect Recover (EDR) support
From: Kuppuswamy Sathyanarayanan As per ACPI specification r6.3, sec 5.6.6, when firmware owns Downstream Port Containment (DPC), its expected to use the "Error Disconnect Recover" (EDR) notification to alert OSPM of a DPC event and if OS supports EDR, its expected to handle the software state invalidation and port recovery in OS, and also let firmware know the recovery status via _OST ACPI call. Related _OST status codes can be found in ACPI specification r6.3, sec 6.3.5.2. Also, as per PCI firmware specification r3.2 Downstream Port Containment Related Enhancements ECN, sec 4.5.1, table 4-6, If DPC is controlled by firmware (firmware first mode), firmware is responsible for configuring the DPC and OS is responsible for error recovery. Also, OS is allowed to modify DPC registers only during the EDR notification window. So with EDR support, OS should provide DPC port services even in firmware first mode. Cc: Keith Busch Signed-off-by: Kuppuswamy Sathyanarayanan Acked-by: Keith Busch Tested-by: Huong Nguyen Tested-by: Austin Bolen --- drivers/pci/pci-acpi.c | 91 drivers/pci/pcie/Kconfig | 10 drivers/pci/pcie/dpc.c | 125 ++- include/linux/pci-acpi.h | 11 4 files changed, 236 insertions(+), 1 deletion(-) diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c index 0c02d500158f..6d9ddef0caf8 100644 --- a/drivers/pci/pci-acpi.c +++ b/drivers/pci/pci-acpi.c @@ -103,6 +103,97 @@ int acpi_get_rc_resources(struct device *dev, const char *hid, u16 segment, } #endif +#if defined(CONFIG_PCIE_DPC) && defined(CONFIG_ACPI) + +/* + * _DSM wrapper function to enable/disable DPC port. + * @dpc : DPC device structure + * @enable: status of DPC port (0 or 1). + * + * returns 0 on success or errno on failure. + */ +int acpi_enable_dpc_port(struct pci_dev *pdev, acpi_handle handle, bool enable) +{ + union acpi_object *obj; + int status = 0; + union acpi_object argv4; + + argv4.type = ACPI_TYPE_INTEGER; + argv4.integer.value = enable; + + obj = acpi_evaluate_dsm(handle, _acpi_dsm_guid, 5, + EDR_PORT_ENABLE_DSM, ); + if (!obj) + return -EIO; + + if (obj->type != ACPI_TYPE_INTEGER || obj->integer.value != enable) + status = -EIO; + + ACPI_FREE(obj); + + return status; +} + +/* + * _DSM wrapper function to locate DPC port. + * @dpc : DPC device structure + * + * returns pci_dev or NULL. + */ +struct pci_dev *acpi_locate_dpc_port(struct pci_dev *pdev, acpi_handle handle) +{ + union acpi_object *obj; + u16 port; + + obj = acpi_evaluate_dsm(handle, _acpi_dsm_guid, 5, + EDR_PORT_LOCATE_DSM, NULL); + if (!obj) + return pci_dev_get(pdev); + + if (obj->type != ACPI_TYPE_INTEGER) { + ACPI_FREE(obj); + return NULL; + } + + /* +* Firmware returns DPC port BDF details in following format: +* 15:8 = bus +* 7:3 = device +* 2:0 = function +*/ + port = obj->integer.value; + + ACPI_FREE(obj); + + return pci_get_domain_bus_and_slot(pci_domain_nr(pdev->bus), + PCI_BUS_NUM(port), port & 0xff); +} + +/* + * _OST wrapper function to let firmware know the status of EDR event. + * @dpc : DPC device structure. + * @status: Status of EDR event. + */ +int acpi_send_edr_status(struct pci_dev *pdev, acpi_handle handle, u16 status) +{ + u32 ost_status; + + pci_dbg(pdev, "Sending EDR status :%x\n", status); + + ost_status = PCI_DEVID(pdev->bus->number, pdev->devfn); + ost_status = (ost_status << 16) | status; + + status = acpi_evaluate_ost(handle, + ACPI_NOTIFY_DISCONNECT_RECOVER, + ost_status, NULL); + if (ACPI_FAILURE(status)) + return -EINVAL; + + return 0; +} + +#endif /* CONFIG_PCIE_DPC && CONFIG_ACPI */ + phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle) { acpi_status status = AE_NOT_EXIST; diff --git a/drivers/pci/pcie/Kconfig b/drivers/pci/pcie/Kconfig index 362eb8cfa53b..9a05015af7cd 100644 --- a/drivers/pci/pcie/Kconfig +++ b/drivers/pci/pcie/Kconfig @@ -150,3 +150,13 @@ config PCIE_BW This enables PCI Express Bandwidth Change Notification. If you know link width or rate changes occur only to correct unreliable links, you may answer Y. + +config PCIE_EDR + bool "PCI Express Error Disconnect Recover support" + default n + depends on PCIE_DPC && ACPI + help + This options adds Error Disconnect Recover support as specified + in PCI firmware specification v3.2 Downstream Port Containment + Related Enhancements ECN. Enable this if you want to support hybrid + DPC model which
[PATCH v9 0/8] Add Error Disconnect Recover (EDR) support
From: Kuppuswamy Sathyanarayanan This patchset adds support for following features: 1. Error Disconnect Recover (EDR) support. 2. _OSC based negotiation support for DPC. You can find EDR spec in the following link. https://members.pcisig.com/wg/PCI-SIG/document/12614 Changes since v8: * Rebased on top of v5.4-rc1 Changes since v7: * Updated DSM version number to match the spec. Changes since v6: * Modified the order of patches to enable EDR only after all necessary support is added in kernel. * Addressed Bjorn comments. Changes since v5: * Addressed Keith's comments. * Added additional check for FF mode in pci_aer_init(). * Updated commit history of "PCI/DPC: Add support for DPC recovery on NON_FATAL errors" patch. Changes since v4: * Rebased on top of v5.3-rc1 * Fixed lock/unlock issue in edr_handle_event(). * Merged "Update error status after reset_link()" patch into this patchset. Changes since v3: * Moved EDR related ACPI functions/definitions to pci-acpi.c * Modified commit history in few patches to include spec reference. * Added support to handle DPC triggered by NON_FATAL errors. * Added edr_lock to protect PCI device receiving duplicate EDR notifications. * Addressed Bjorn comments. Changes since v2: * Split EDR support patch into multiple patches. * Addressed Bjorn comments. Changes since v1: * Rebased on top of v5.1-rc1 Kuppuswamy Sathyanarayanan (8): PCI/ERR: Update error status after reset_link() PCI/DPC: Allow dpc_probe() even if firmware first mode is enabled PCI/DPC: Add dpc_process_error() wrapper function PCI/DPC: Add Error Disconnect Recover (EDR) support PCI/AER: Allow clearing Error Status Register in FF mode PCI/DPC: Update comments related to DPC recovery on NON_FATAL errors PCI/DPC: Clear AER registers in EDR mode PCI/ACPI: Enable EDR support drivers/acpi/pci_root.c | 9 ++ drivers/pci/pci-acpi.c | 91 +++ drivers/pci/pcie/Kconfig| 10 ++ drivers/pci/pcie/aer.c | 12 +- drivers/pci/pcie/dpc.c | 194 +--- drivers/pci/pcie/err.c | 10 +- drivers/pci/pcie/portdrv_core.c | 8 +- drivers/pci/probe.c | 1 + include/linux/acpi.h| 6 +- include/linux/pci-acpi.h| 11 ++ include/linux/pci.h | 3 +- 11 files changed, 321 insertions(+), 34 deletions(-) -- 2.21.0
[PATCH v7 0/7] Fix PF/VF dependency issue
From: Kuppuswamy Sathyanarayanan Current implementation of ATS, PASID, PRI does not handle VF dependencies correctly. Following patches addresses this issue. Changes since v6: * Removed locking interfaces used in v6. * Made necessary changes to PRI/PASID enable/disable calls to allow register changes only for PF. Changes since v5: * Created new patches for PRI/PASID capability caching. * Removed individual locks (pri_lock, pasid_lock) and added common resource lock to synchronize PRI/PASID updates between PF/VF. * Addressed comments from Bjorn Helgaas. Changes since v4: * Defined empty functions for pci_pri_init() and pci_pasid_init() for cases where CONFIG_PCI_PRI and CONFIG_PCI_PASID are not enabled. Changes since v3: * Fixed critical path (lock context) in pci_restore_*_state functions. Changes since v2: * Added locking mechanism to synchronize accessing PF registers in VF. * Removed spec compliance checks in patches. * Addressed comments from Bjorn Helgaas. Changes since v1: * Added more details about the patches in commit log. * Removed bulk spec check patch. * Addressed comments from Bjorn Helgaas. Kuppuswamy Sathyanarayanan (7): PCI/ATS: Fix pci_prg_resp_pasid_required() dependency issues PCI/ATS: Cache PRI capability check result PCI/ATS: Cache PASID capability check result PCI/ATS: Add PRI support for PCIe VF devices PCI/ATS: Add PASID support for PCIe VF devices PCI/ATS: Disable PF/VF ATS service independently PCI: Skip Enhanced Allocation (EA) initialization for VF device drivers/pci/ats.c | 202 drivers/pci/pci.c | 7 ++ include/linux/pci-ats.h | 12 ++- include/linux/pci.h | 3 +- 4 files changed, 159 insertions(+), 65 deletions(-) -- 2.21.0
[PATCH v7 1/7] PCI/ATS: Fix pci_prg_resp_pasid_required() dependency issues
From: Kuppuswamy Sathyanarayanan Since pci_prg_resp_pasid_required() function has dependency on both PASID and PRI, define it only if both CONFIG_PCI_PRI and CONFIG_PCI_PASID config options are enabled. Fixes: e5567f5f6762 ("PCI/ATS: Add pci_prg_resp_pasid_required() interface.") Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/ats.c | 10 ++ include/linux/pci-ats.h | 12 +--- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c index e18499243f84..cdd936d10f68 100644 --- a/drivers/pci/ats.c +++ b/drivers/pci/ats.c @@ -395,6 +395,8 @@ int pci_pasid_features(struct pci_dev *pdev) } EXPORT_SYMBOL_GPL(pci_pasid_features); +#ifdef CONFIG_PCI_PRI + /** * pci_prg_resp_pasid_required - Return PRG Response PASID Required bit * status. @@ -402,10 +404,8 @@ EXPORT_SYMBOL_GPL(pci_pasid_features); * * Returns 1 if PASID is required in PRG Response Message, 0 otherwise. * - * Even though the PRG response PASID status is read from PRI Status - * Register, since this API will mainly be used by PASID users, this - * function is defined within #ifdef CONFIG_PCI_PASID instead of - * CONFIG_PCI_PRI. + * Since this API has dependency on both PRI and PASID, protect it + * with both CONFIG_PCI_PRI and CONFIG_PCI_PASID. */ int pci_prg_resp_pasid_required(struct pci_dev *pdev) { @@ -425,6 +425,8 @@ int pci_prg_resp_pasid_required(struct pci_dev *pdev) } EXPORT_SYMBOL_GPL(pci_prg_resp_pasid_required); +#endif + #define PASID_NUMBER_SHIFT 8 #define PASID_NUMBER_MASK (0x1f << PASID_NUMBER_SHIFT) /** diff --git a/include/linux/pci-ats.h b/include/linux/pci-ats.h index 1ebb88e7c184..1a0bdaee2f32 100644 --- a/include/linux/pci-ats.h +++ b/include/linux/pci-ats.h @@ -40,7 +40,6 @@ void pci_disable_pasid(struct pci_dev *pdev); void pci_restore_pasid_state(struct pci_dev *pdev); int pci_pasid_features(struct pci_dev *pdev); int pci_max_pasids(struct pci_dev *pdev); -int pci_prg_resp_pasid_required(struct pci_dev *pdev); #else /* CONFIG_PCI_PASID */ @@ -67,11 +66,18 @@ static inline int pci_max_pasids(struct pci_dev *pdev) return -EINVAL; } +#endif /* CONFIG_PCI_PASID */ + +#if defined(CONFIG_PCI_PRI) && defined(CONFIG_PCI_PASID) + +int pci_prg_resp_pasid_required(struct pci_dev *pdev); + +#else /* CONFIG_PCI_PASID && CONFIG_PCI_PRI */ + static inline int pci_prg_resp_pasid_required(struct pci_dev *pdev) { return 0; } -#endif /* CONFIG_PCI_PASID */ - +#endif #endif /* LINUX_PCI_ATS_H*/ -- 2.21.0
[PATCH v7 4/7] PCI/ATS: Add PRI support for PCIe VF devices
From: Kuppuswamy Sathyanarayanan When IOMMU tries to enable Page Request Interface (PRI) for VF device in iommu_enable_dev_iotlb(), it always fails because PRI support for PCIe VF device is currently broken. Current implementation expects the given PCIe device (PF & VF) to implement PRI capability before enabling the PRI support. But this assumption is incorrect. As per PCIe spec r4.0, sec 9.3.7.11, all VFs associated with PF can only use the PRI of the PF and not implement it. Hence we need to create exception for handling the PRI support for PCIe VF device. Cc: Ashok Raj Cc: Keith Busch Suggested-by: Ashok Raj Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/ats.c | 42 -- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c index 022698a85c98..9af1e518a9ab 100644 --- a/drivers/pci/ats.c +++ b/drivers/pci/ats.c @@ -21,6 +21,15 @@ static void pci_pri_init(struct pci_dev *pdev) #ifdef CONFIG_PCI_PRI int pos; + /* +* As per PCIe r4.0, sec 9.3.7.11, only PF is permitted to +* implement PRI and all associated VFs can only use it. +* Since PF already initialized the PRI parameters there is +* no need to proceed further. +*/ + if (pdev->is_virtfn) + return; + pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI); if (!pos) return; @@ -210,6 +219,20 @@ int pci_enable_pri(struct pci_dev *pdev, u32 reqs) { u16 control, status; u32 max_requests; + struct pci_dev *pf = pci_physfn(pdev); + + /* +* IOMMU is the only user of this function and as per +* current usage, PF PRI enable always happens before +* VF and hence we don't need to do anything special +* for VF. So just return success if PRI is enabled in PF. +*/ + if (pdev->is_virtfn) { + if (pf->pri_enabled) + return 0; + else + return -EINVAL; + } if (WARN_ON(pdev->pri_enabled)) return -EBUSY; @@ -246,6 +269,14 @@ void pci_disable_pri(struct pci_dev *pdev) { u16 control; + /* +* As per PCIe r4.0, sec 9.3.7.11, only PF is permitted to +* implement PRI and all associated VFs can only use it. +* So don't do anything for VF and just return. +*/ + if (pdev->is_virtfn) + return; + if (WARN_ON(!pdev->pri_enabled)) return; @@ -269,6 +300,9 @@ void pci_restore_pri_state(struct pci_dev *pdev) u16 control = PCI_PRI_CTRL_ENABLE; u32 reqs = pdev->pri_reqs_alloc; + if (pdev->is_virtfn) + return; + if (!pdev->pri_enabled) return; @@ -291,6 +325,9 @@ int pci_reset_pri(struct pci_dev *pdev) { u16 control; + if (pdev->is_virtfn) + return 0; + if (WARN_ON(pdev->pri_enabled)) return -EBUSY; @@ -427,11 +464,12 @@ EXPORT_SYMBOL_GPL(pci_pasid_features); int pci_prg_resp_pasid_required(struct pci_dev *pdev) { u16 status; + struct pci_dev *pf = pci_physfn(pdev); - if (!pdev->pri_cap) + if (!pf->pri_cap) return 0; - pci_read_config_word(pdev, pdev->pri_cap + PCI_PRI_STATUS, ); + pci_read_config_word(pf, pf->pri_cap + PCI_PRI_STATUS, ); if (status & PCI_PRI_STATUS_PASID) return 1; -- 2.21.0
[PATCH v7 5/7] PCI/ATS: Add PASID support for PCIe VF devices
From: Kuppuswamy Sathyanarayanan When IOMMU tries to enable PASID for VF device in iommu_enable_dev_iotlb(), it always fails because PASID support for PCIe VF device is currently broken in PCIE driver. Current implementation expects the given PCIe device (PF & VF) to implement PASID capability before enabling the PASID support. But this assumption is incorrect. As per PCIe spec r4.0, sec 9.3.7.14, all VFs associated with PF can only use the PASID of the PF and not implement it. Hence we need to create exception for handling the PASID support for PCIe VF device. Cc: Ashok Raj Cc: Keith Busch Suggested-by: Ashok Raj Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/ats.c | 49 +-- 1 file changed, 43 insertions(+), 6 deletions(-) diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c index 9af1e518a9ab..893674520bbf 100644 --- a/drivers/pci/ats.c +++ b/drivers/pci/ats.c @@ -43,6 +43,15 @@ static void pci_pasid_init(struct pci_dev *pdev) #ifdef CONFIG_PCI_PASID int pos; + /* +* As per PCIe r4.0, sec 9.3.7.14, only PF is permitted to +* implement PASID Capability and all associated VFs can +* only use it. Since PF already initialized the PASID +* parameters there is no need to proceed further. +*/ + if (pdev->is_virtfn) + return; + pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PASID); if (!pos) return; @@ -355,7 +364,20 @@ EXPORT_SYMBOL_GPL(pci_reset_pri); int pci_enable_pasid(struct pci_dev *pdev, int features) { u16 control, supported; + struct pci_dev *pf = pci_physfn(pdev); + /* +* IOMMU is the only user of this function and as per +* current usage, PF PASID enable always happens before +* VF and hence we don't need to do anything special +* for VF. So just return success if PASID is enabled in PF. +*/ + if (pdev->is_virtfn) { + if (pf->pasid_enabled) + return 0; + else + return -EINVAL; + } if (WARN_ON(pdev->pasid_enabled)) return -EBUSY; @@ -392,6 +414,14 @@ void pci_disable_pasid(struct pci_dev *pdev) { u16 control = 0; + /* +* As per PCIe r4.0, sec 9.3.7.14, only PF is permitted to +* implement PASID Capability and all associated VFs can +* only use it. So don't do anything for VF and just return. +*/ + if (pdev->is_virtfn) + return; + if (WARN_ON(!pdev->pasid_enabled)) return; @@ -412,6 +442,13 @@ void pci_restore_pasid_state(struct pci_dev *pdev) { u16 control; + /* +* PF should have already restored the PASID state. So for +* VF, just return. +*/ + if (pdev->is_virtfn) + return; + if (!pdev->pasid_enabled) return; @@ -436,12 +473,12 @@ EXPORT_SYMBOL_GPL(pci_restore_pasid_state); int pci_pasid_features(struct pci_dev *pdev) { u16 supported; + struct pci_dev *pf = pci_physfn(pdev); - if (!pdev->pasid_cap) + if (!pf->pasid_cap) return -EINVAL; - pci_read_config_word(pdev, pdev->pasid_cap + PCI_PASID_CAP, -); + pci_read_config_word(pf, pf->pasid_cap + PCI_PASID_CAP, ); supported &= PCI_PASID_CAP_EXEC | PCI_PASID_CAP_PRIV; @@ -492,12 +529,12 @@ EXPORT_SYMBOL_GPL(pci_prg_resp_pasid_required); int pci_max_pasids(struct pci_dev *pdev) { u16 supported; + struct pci_dev *pf = pci_physfn(pdev); - if (!pdev->pasid_cap) + if (!pf->pasid_cap) return -EINVAL; - pci_read_config_word(pdev, pdev->pasid_cap + PCI_PASID_CAP, -); + pci_read_config_word(pf, pf->pasid_cap + PCI_PASID_CAP, ); supported = (supported & PASID_NUMBER_MASK) >> PASID_NUMBER_SHIFT; -- 2.21.0
[PATCH v7 3/7] PCI/ATS: Cache PASID capability check result
From: Kuppuswamy Sathyanarayanan Currently, PASID capability checks are repeated across all PASID API's. Instead, cache the capability check result in pci_pasid_init() and use it in other PASID API's. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/ats.c | 50 ++--- include/linux/pci.h | 1 + 2 files changed, 30 insertions(+), 21 deletions(-) diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c index b863562b6702..022698a85c98 100644 --- a/drivers/pci/ats.c +++ b/drivers/pci/ats.c @@ -29,6 +29,19 @@ static void pci_pri_init(struct pci_dev *pdev) #endif } +static void pci_pasid_init(struct pci_dev *pdev) +{ +#ifdef CONFIG_PCI_PASID + int pos; + + pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PASID); + if (!pos) + return; + + pdev->pasid_cap = pos; +#endif +} + void pci_ats_init(struct pci_dev *dev) { int pos; @@ -43,6 +56,8 @@ void pci_ats_init(struct pci_dev *dev) dev->ats_cap = pos; pci_pri_init(dev); + + pci_pasid_init(dev); } /** @@ -303,7 +318,6 @@ EXPORT_SYMBOL_GPL(pci_reset_pri); int pci_enable_pasid(struct pci_dev *pdev, int features) { u16 control, supported; - int pos; if (WARN_ON(pdev->pasid_enabled)) return -EBUSY; @@ -311,11 +325,11 @@ int pci_enable_pasid(struct pci_dev *pdev, int features) if (!pdev->eetlp_prefix_path) return -EINVAL; - pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PASID); - if (!pos) + if (!pdev->pasid_cap) return -EINVAL; - pci_read_config_word(pdev, pos + PCI_PASID_CAP, ); + pci_read_config_word(pdev, pdev->pasid_cap + PCI_PASID_CAP, +); supported &= PCI_PASID_CAP_EXEC | PCI_PASID_CAP_PRIV; /* User wants to enable anything unsupported? */ @@ -325,7 +339,7 @@ int pci_enable_pasid(struct pci_dev *pdev, int features) control = PCI_PASID_CTRL_ENABLE | features; pdev->pasid_features = features; - pci_write_config_word(pdev, pos + PCI_PASID_CTRL, control); + pci_write_config_word(pdev, pdev->pasid_cap + PCI_PASID_CTRL, control); pdev->pasid_enabled = 1; @@ -340,16 +354,14 @@ EXPORT_SYMBOL_GPL(pci_enable_pasid); void pci_disable_pasid(struct pci_dev *pdev) { u16 control = 0; - int pos; if (WARN_ON(!pdev->pasid_enabled)) return; - pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PASID); - if (!pos) + if (!pdev->pasid_cap) return; - pci_write_config_word(pdev, pos + PCI_PASID_CTRL, control); + pci_write_config_word(pdev, pdev->pasid_cap + PCI_PASID_CTRL, control); pdev->pasid_enabled = 0; } @@ -362,17 +374,15 @@ EXPORT_SYMBOL_GPL(pci_disable_pasid); void pci_restore_pasid_state(struct pci_dev *pdev) { u16 control; - int pos; if (!pdev->pasid_enabled) return; - pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PASID); - if (!pos) + if (!pdev->pasid_cap) return; control = PCI_PASID_CTRL_ENABLE | pdev->pasid_features; - pci_write_config_word(pdev, pos + PCI_PASID_CTRL, control); + pci_write_config_word(pdev, pdev->pasid_cap + PCI_PASID_CTRL, control); } EXPORT_SYMBOL_GPL(pci_restore_pasid_state); @@ -389,13 +399,12 @@ EXPORT_SYMBOL_GPL(pci_restore_pasid_state); int pci_pasid_features(struct pci_dev *pdev) { u16 supported; - int pos; - pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PASID); - if (!pos) + if (!pdev->pasid_cap) return -EINVAL; - pci_read_config_word(pdev, pos + PCI_PASID_CAP, ); + pci_read_config_word(pdev, pdev->pasid_cap + PCI_PASID_CAP, +); supported &= PCI_PASID_CAP_EXEC | PCI_PASID_CAP_PRIV; @@ -445,13 +454,12 @@ EXPORT_SYMBOL_GPL(pci_prg_resp_pasid_required); int pci_max_pasids(struct pci_dev *pdev) { u16 supported; - int pos; - pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PASID); - if (!pos) + if (!pdev->pasid_cap) return -EINVAL; - pci_read_config_word(pdev, pos + PCI_PASID_CAP, ); + pci_read_config_word(pdev, pdev->pasid_cap + PCI_PASID_CAP, +); supported = (supported & PASID_NUMBER_MASK) >> PASID_NUMBER_SHIFT; diff --git a/include/linux/pci.h b/include/linux/pci.h index 79991984c4cd..782bca667010 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -459,6 +459,7 @@ struct pci_dev { u32 pri_reqs_alloc; /* Number of PRI requests allocated */ #endif #ifdef CONFIG_PCI_PASID + u16 pasid_cap; /* PASID Capability offset */ u16 pasid_features; #endif #ifdef CONFIG_PCI_P2PDMA -- 2.21.0
[PATCH v7 7/7] PCI: Skip Enhanced Allocation (EA) initialization for VF device
From: Kuppuswamy Sathyanarayanan As per PCIe r4.0, sec 9.3.6, VF must not implement Enhanced Allocation Capability. So skip pci_ea_init() for virtual devices. Cc: Ashok Raj Cc: Keith Busch Suggested-by: Ashok Raj Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/pci.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 1b27b5af3d55..266600a11769 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -3025,6 +3025,13 @@ void pci_ea_init(struct pci_dev *dev) int offset; int i; + /* +* Per PCIe r4.0, sec 9.3.6, VF must not implement Enhanced +* Allocation Capability. +*/ + if (dev->is_virtfn) + return; + /* find PCI EA capability in list */ ea = pci_find_capability(dev, PCI_CAP_ID_EA); if (!ea) -- 2.21.0
[PATCH v7 6/7] PCI/ATS: Disable PF/VF ATS service independently
From: Kuppuswamy Sathyanarayanan Currently all VF's needs to be disable their ATS service before disabling the ATS service in corresponding PF device. But this logic is incorrect and does not align with the spec. Also it might lead to some power and performance impact in the system. As per PCIe spec r4.0, sec 9.3.7.8, ATS Capabilities in VFs and their associated PFs may be enabled/disabled independently. So remove this dependency logic in enable/disable code. Cc: Ashok Raj Cc: Keith Busch Suggested-by: Ashok Raj Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/ats.c | 11 --- include/linux/pci.h | 1 - 2 files changed, 12 deletions(-) diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c index 893674520bbf..e7db42128176 100644 --- a/drivers/pci/ats.c +++ b/drivers/pci/ats.c @@ -108,8 +108,6 @@ int pci_enable_ats(struct pci_dev *dev, int ps) pdev = pci_physfn(dev); if (pdev->ats_stu != ps) return -EINVAL; - - atomic_inc(>ats_ref_cnt); /* count enabled VFs */ } else { dev->ats_stu = ps; ctrl |= PCI_ATS_CTRL_STU(dev->ats_stu - PCI_ATS_MIN_STU); @@ -127,20 +125,11 @@ EXPORT_SYMBOL_GPL(pci_enable_ats); */ void pci_disable_ats(struct pci_dev *dev) { - struct pci_dev *pdev; u16 ctrl; if (WARN_ON(!dev->ats_enabled)) return; - if (atomic_read(>ats_ref_cnt)) - return; /* VFs still enabled */ - - if (dev->is_virtfn) { - pdev = pci_physfn(dev); - atomic_dec(>ats_ref_cnt); - } - pci_read_config_word(dev, dev->ats_cap + PCI_ATS_CTRL, ); ctrl &= ~PCI_ATS_CTRL_ENABLE; pci_write_config_word(dev, dev->ats_cap + PCI_ATS_CTRL, ctrl); diff --git a/include/linux/pci.h b/include/linux/pci.h index 782bca667010..2428414b2b9c 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -452,7 +452,6 @@ struct pci_dev { }; u16 ats_cap;/* ATS Capability offset */ u8 ats_stu;/* ATS Smallest Translation Unit */ - atomic_tats_ref_cnt;/* Number of VFs with ATS enabled */ #endif #ifdef CONFIG_PCI_PRI u16 pri_cap;/* PRI Capability offset */ -- 2.21.0
[PATCH v7 2/7] PCI/ATS: Cache PRI capability check result
From: Kuppuswamy Sathyanarayanan Currently, PRI capability checks are repeated across all PRI API's. Instead, cache the capability check result in pci_pri_init() and use it in other PRI API's. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/ats.c | 56 + include/linux/pci.h | 1 + 2 files changed, 32 insertions(+), 25 deletions(-) diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c index cdd936d10f68..b863562b6702 100644 --- a/drivers/pci/ats.c +++ b/drivers/pci/ats.c @@ -16,6 +16,19 @@ #include "pci.h" +static void pci_pri_init(struct pci_dev *pdev) +{ +#ifdef CONFIG_PCI_PRI + int pos; + + pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI); + if (!pos) + return; + + pdev->pri_cap = pos; +#endif +} + void pci_ats_init(struct pci_dev *dev) { int pos; @@ -28,6 +41,8 @@ void pci_ats_init(struct pci_dev *dev) return; dev->ats_cap = pos; + + pci_pri_init(dev); } /** @@ -180,26 +195,25 @@ int pci_enable_pri(struct pci_dev *pdev, u32 reqs) { u16 control, status; u32 max_requests; - int pos; if (WARN_ON(pdev->pri_enabled)) return -EBUSY; - pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI); - if (!pos) + if (!pdev->pri_cap) return -EINVAL; - pci_read_config_word(pdev, pos + PCI_PRI_STATUS, ); + pci_read_config_word(pdev, pdev->pri_cap + PCI_PRI_STATUS, ); if (!(status & PCI_PRI_STATUS_STOPPED)) return -EBUSY; - pci_read_config_dword(pdev, pos + PCI_PRI_MAX_REQ, _requests); + pci_read_config_dword(pdev, pdev->pri_cap + PCI_PRI_MAX_REQ, + _requests); reqs = min(max_requests, reqs); pdev->pri_reqs_alloc = reqs; - pci_write_config_dword(pdev, pos + PCI_PRI_ALLOC_REQ, reqs); + pci_write_config_dword(pdev, pdev->pri_cap + PCI_PRI_ALLOC_REQ, reqs); control = PCI_PRI_CTRL_ENABLE; - pci_write_config_word(pdev, pos + PCI_PRI_CTRL, control); + pci_write_config_word(pdev, pdev->pri_cap + PCI_PRI_CTRL, control); pdev->pri_enabled = 1; @@ -216,18 +230,16 @@ EXPORT_SYMBOL_GPL(pci_enable_pri); void pci_disable_pri(struct pci_dev *pdev) { u16 control; - int pos; if (WARN_ON(!pdev->pri_enabled)) return; - pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI); - if (!pos) + if (!pdev->pri_cap) return; - pci_read_config_word(pdev, pos + PCI_PRI_CTRL, ); + pci_read_config_word(pdev, pdev->pri_cap + PCI_PRI_CTRL, ); control &= ~PCI_PRI_CTRL_ENABLE; - pci_write_config_word(pdev, pos + PCI_PRI_CTRL, control); + pci_write_config_word(pdev, pdev->pri_cap + PCI_PRI_CTRL, control); pdev->pri_enabled = 0; } @@ -241,17 +253,15 @@ void pci_restore_pri_state(struct pci_dev *pdev) { u16 control = PCI_PRI_CTRL_ENABLE; u32 reqs = pdev->pri_reqs_alloc; - int pos; if (!pdev->pri_enabled) return; - pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI); - if (!pos) + if (!pdev->pri_cap) return; - pci_write_config_dword(pdev, pos + PCI_PRI_ALLOC_REQ, reqs); - pci_write_config_word(pdev, pos + PCI_PRI_CTRL, control); + pci_write_config_dword(pdev, pdev->pri_cap + PCI_PRI_ALLOC_REQ, reqs); + pci_write_config_word(pdev, pdev->pri_cap + PCI_PRI_CTRL, control); } EXPORT_SYMBOL_GPL(pci_restore_pri_state); @@ -265,17 +275,15 @@ EXPORT_SYMBOL_GPL(pci_restore_pri_state); int pci_reset_pri(struct pci_dev *pdev) { u16 control; - int pos; if (WARN_ON(pdev->pri_enabled)) return -EBUSY; - pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI); - if (!pos) + if (!pdev->pri_cap) return -EINVAL; control = PCI_PRI_CTRL_RESET; - pci_write_config_word(pdev, pos + PCI_PRI_CTRL, control); + pci_write_config_word(pdev, pdev->pri_cap + PCI_PRI_CTRL, control); return 0; } @@ -410,13 +418,11 @@ EXPORT_SYMBOL_GPL(pci_pasid_features); int pci_prg_resp_pasid_required(struct pci_dev *pdev) { u16 status; - int pos; - pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI); - if (!pos) + if (!pdev->pri_cap) return 0; - pci_read_config_word(pdev, pos + PCI_PRI_STATUS, ); + pci_read_config_word(pdev, pdev->pri_cap + PCI_PRI_STATUS, ); if (status & PCI_PRI_STATUS_PASID) return 1; diff --git a/include/linux/pci.h b/include/linux/pci.h index 82e4cd1b7ac3..79991984c4cd 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -455,6 +455,7 @@ struct pci_dev { atomic_tats_ref_cnt;/* Number of VFs with ATS enabled */ #endif #ifdef CONFIG_PCI_PRI +
[PATCH v8 0/8] Add Error Disconnect Recover (EDR) support
From: Kuppuswamy Sathyanarayanan This patchset adds support for following features: 1. Error Disconnect Recover (EDR) support. 2. _OSC based negotiation support for DPC. You can find EDR spec in the following link. https://members.pcisig.com/wg/PCI-SIG/document/12614 Changes since v7: * Updated DSM version number to match the spec. Changes since v6: * Modified the order of patches to enable EDR only after all necessary support is added in kernel. * Addressed Bjorn comments. Changes since v5: * Addressed Keith's comments. * Added additional check for FF mode in pci_aer_init(). * Updated commit history of "PCI/DPC: Add support for DPC recovery on NON_FATAL errors" patch. Changes since v4: * Rebased on top of v5.3-rc1 * Fixed lock/unlock issue in edr_handle_event(). * Merged "Update error status after reset_link()" patch into this patchset. Changes since v3: * Moved EDR related ACPI functions/definitions to pci-acpi.c * Modified commit history in few patches to include spec reference. * Added support to handle DPC triggered by NON_FATAL errors. * Added edr_lock to protect PCI device receiving duplicate EDR notifications. * Addressed Bjorn comments. Changes since v2: * Split EDR support patch into multiple patches. * Addressed Bjorn comments. Changes since v1: * Rebased on top of v5.1-rc1 Kuppuswamy Sathyanarayanan (8): PCI/ERR: Update error status after reset_link() PCI/DPC: Allow dpc_probe() even if firmware first mode is enabled PCI/DPC: Add dpc_process_error() wrapper function PCI/DPC: Add Error Disconnect Recover (EDR) support PCI/AER: Allow clearing Error Status Register in FF mode PCI/DPC: Update comments related to DPC recovery on NON_FATAL errors PCI/DPC: Clear AER registers in EDR mode PCI/ACPI: Enable EDR support drivers/acpi/pci_root.c | 9 ++ drivers/pci/pci-acpi.c | 91 +++ drivers/pci/pcie/Kconfig| 10 ++ drivers/pci/pcie/aer.c | 12 +- drivers/pci/pcie/dpc.c | 194 +--- drivers/pci/pcie/err.c | 10 +- drivers/pci/pcie/portdrv_core.c | 8 +- drivers/pci/probe.c | 1 + include/linux/acpi.h| 6 +- include/linux/pci-acpi.h| 11 ++ include/linux/pci.h | 3 +- 11 files changed, 321 insertions(+), 34 deletions(-) -- 2.21.0
[PATCH v8 7/8] PCI/DPC: Clear AER registers in EDR mode
From: Kuppuswamy Sathyanarayanan As per PCI firmware specification r3.2 Downstream Port Containment Related Enhancements ECN, OS is responsible for clearing the AER registers in EDR mode. So clear AER registers in dpc_process_error() function. Signed-off-by: Kuppuswamy Sathyanarayanan Acked-by: Keith Busch --- drivers/pci/pcie/dpc.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index fafc55c00fe0..de2d892bc7c4 100644 --- a/drivers/pci/pcie/dpc.c +++ b/drivers/pci/pcie/dpc.c @@ -275,6 +275,10 @@ static void dpc_process_error(struct dpc_dev *dpc) pci_aer_clear_fatal_status(pdev); } + /* In EDR mode, OS is responsible for clearing AER registers */ + if (dpc->firmware_dpc) + pci_cleanup_aer_error_status_regs(pdev); + /* * Irrespective of whether the DPC event is triggered by * ERR_FATAL or ERR_NONFATAL, since the link is already down, -- 2.21.0
[PATCH v8 3/8] PCI/DPC: Add dpc_process_error() wrapper function
From: Kuppuswamy Sathyanarayanan With Error Disconnect Recover (EDR) support, we need to support processing DPC event either from DPC IRQ or ACPI EDR event. So create a wrapper function dpc_process_error() and move common error handling code in to it. It will be used to process the DPC event in both DPC IRQ and EDR ACPI event contexts. Signed-off-by: Kuppuswamy Sathyanarayanan Acked-by: Keith Busch --- drivers/pci/pcie/dpc.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index 9717fda012f8..4137ec7b48cc 100644 --- a/drivers/pci/pcie/dpc.c +++ b/drivers/pci/pcie/dpc.c @@ -232,10 +232,9 @@ static int dpc_get_aer_uncorrect_severity(struct pci_dev *dev, return 1; } -static irqreturn_t dpc_handler(int irq, void *context) +static void dpc_process_error(struct dpc_dev *dpc) { struct aer_err_info info; - struct dpc_dev *dpc = context; struct pci_dev *pdev = dpc->dev->port; u16 cap = dpc->cap_pos, status, source, reason, ext_reason; @@ -268,6 +267,13 @@ static irqreturn_t dpc_handler(int irq, void *context) /* We configure DPC so it only triggers on ERR_FATAL */ pcie_do_recovery(pdev, pci_channel_io_frozen, PCIE_PORT_SERVICE_DPC); +} + +static irqreturn_t dpc_handler(int irq, void *context) +{ + struct dpc_dev *dpc = context; + + dpc_process_error(dpc); return IRQ_HANDLED; } -- 2.21.0
[PATCH v8 8/8] PCI/ACPI: Enable EDR support
From: Kuppuswamy Sathyanarayanan As per PCI firmware specification r3.2 Downstream Port Containment Related Enhancements ECN, sec 4.5.1, OS must implement following steps to enable/use EDR feature. 1. OS can use bit 7 of _OSC Control Field to negotiate control over Downstream Port Containment (DPC) configuration of PCIe port. After _OSC negotiation, firmware will Set this bit to grant OS control over PCIe DPC configuration and Clear it if this feature was requested and denied, or was not requested. 2. Also, if OS supports EDR, it should expose its support to BIOS by setting bit 7 of _OSC Support Field. And if OS sets bit 7 of _OSC Control Field it must also expose support for EDR by setting bit 7 of _OSC Support Field. Cc: Bjorn Helgaas Cc: "Rafael J. Wysocki" Cc: Len Brown Signed-off-by: Kuppuswamy Sathyanarayanan Acked-by: Keith Busch Tested-by: Huong Nguyen Tested-by: Austin Bolen --- drivers/acpi/pci_root.c | 9 + drivers/pci/pcie/portdrv_core.c | 8 +++- drivers/pci/probe.c | 1 + include/linux/acpi.h| 6 -- include/linux/pci.h | 3 ++- 5 files changed, 23 insertions(+), 4 deletions(-) diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index 314a187ed572..988d09d788b6 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -132,6 +132,7 @@ static struct pci_osc_bit_struct pci_osc_support_bit[] = { { OSC_PCI_CLOCK_PM_SUPPORT, "ClockPM" }, { OSC_PCI_SEGMENT_GROUPS_SUPPORT, "Segments" }, { OSC_PCI_MSI_SUPPORT, "MSI" }, + { OSC_PCI_EDR_SUPPORT, "EDR" }, { OSC_PCI_HPX_TYPE_3_SUPPORT, "HPX-Type3" }, }; @@ -142,6 +143,7 @@ static struct pci_osc_bit_struct pci_osc_control_bit[] = { { OSC_PCI_EXPRESS_AER_CONTROL, "AER" }, { OSC_PCI_EXPRESS_CAPABILITY_CONTROL, "PCIeCapability" }, { OSC_PCI_EXPRESS_LTR_CONTROL, "LTR" }, + { OSC_PCI_EXPRESS_DPC_CONTROL, "DPC" }, }; static void decode_osc_bits(struct acpi_pci_root *root, char *msg, u32 word, @@ -441,6 +443,8 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm, support |= OSC_PCI_ASPM_SUPPORT | OSC_PCI_CLOCK_PM_SUPPORT; if (pci_msi_enabled()) support |= OSC_PCI_MSI_SUPPORT; + if (IS_ENABLED(CONFIG_PCIE_EDR)) + support |= OSC_PCI_EDR_SUPPORT; decode_osc_support(root, "OS supports", support); status = acpi_pci_osc_support(root, support); @@ -488,6 +492,9 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm, control |= OSC_PCI_EXPRESS_AER_CONTROL; } + if (IS_ENABLED(CONFIG_PCIE_DPC)) + control |= OSC_PCI_EXPRESS_DPC_CONTROL; + requested = control; status = acpi_pci_osc_control_set(handle, , OSC_PCI_EXPRESS_CAPABILITY_CONTROL); @@ -917,6 +924,8 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, host_bridge->native_pme = 0; if (!(root->osc_control_set & OSC_PCI_EXPRESS_LTR_CONTROL)) host_bridge->native_ltr = 0; + if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL)) + host_bridge->native_dpc = 0; /* * Evaluate the "PCI Boot Configuration" _DSM Function. If it diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index 1b330129089f..1b54a39df795 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -250,8 +250,14 @@ static int get_port_device_capability(struct pci_dev *dev) pcie_pme_interrupt_enable(dev, false); } + /* +* If EDR support is enabled in OS, then even if AER is not handled in +* OS, DPC service can be enabled. +*/ if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) && - pci_aer_available() && services & PCIE_PORT_SERVICE_AER) + ((IS_ENABLED(CONFIG_PCIE_EDR) && !host->native_dpc) || + (pci_aer_available() && services & PCIE_PORT_SERVICE_AER && + (pcie_ports_native || host->native_dpc services |= PCIE_PORT_SERVICE_DPC; if (pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM || diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index a3c7338fad86..cf8acdd62089 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -601,6 +601,7 @@ static void pci_init_host_bridge(struct pci_host_bridge *bridge) bridge->native_shpc_hotplug = 1; bridge->native_pme = 1; bridge->native_ltr = 1; + bridge->native_dpc = 1; } struct pci_host_bridge *pci_alloc_host_bridge(size_t priv) diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 9426b9aaed86..7b29c640a9db 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -515,8 +515,9 @@ extern bool osc_pc_lpi_support_confirmed; #define OSC_PCI_CLOCK_PM_SUPPORT
[PATCH v8 6/8] PCI/DPC: Update comments related to DPC recovery on NON_FATAL errors
From: Kuppuswamy Sathyanarayanan Currently, in native mode, DPC driver is configured to trigger DPC only for FATAL errors and hence it only supports port recovery for FATAL errors. But with Error Disconnect Recover (EDR) support, DPC configuration is done by firmware, and hence we should expect DPC triggered for both FATAL/NON_FATAL errors. So update comments and add details about how NON_FATAL dpc recovery is handled. Signed-off-by: Kuppuswamy Sathyanarayanan Acked-by: Keith Busch --- drivers/pci/pcie/dpc.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index a8b634e05694..fafc55c00fe0 100644 --- a/drivers/pci/pcie/dpc.c +++ b/drivers/pci/pcie/dpc.c @@ -275,7 +275,11 @@ static void dpc_process_error(struct dpc_dev *dpc) pci_aer_clear_fatal_status(pdev); } - /* We configure DPC so it only triggers on ERR_FATAL */ + /* +* Irrespective of whether the DPC event is triggered by +* ERR_FATAL or ERR_NONFATAL, since the link is already down, +* use the FATAL error recovery path for both cases. +*/ pcie_do_recovery(pdev, pci_channel_io_frozen, PCIE_PORT_SERVICE_DPC); } -- 2.21.0
[PATCH v8 4/8] PCI/DPC: Add Error Disconnect Recover (EDR) support
From: Kuppuswamy Sathyanarayanan As per ACPI specification r6.3, sec 5.6.6, when firmware owns Downstream Port Containment (DPC), its expected to use the "Error Disconnect Recover" (EDR) notification to alert OSPM of a DPC event and if OS supports EDR, its expected to handle the software state invalidation and port recovery in OS, and also let firmware know the recovery status via _OST ACPI call. Related _OST status codes can be found in ACPI specification r6.3, sec 6.3.5.2. Also, as per PCI firmware specification r3.2 Downstream Port Containment Related Enhancements ECN, sec 4.5.1, table 4-6, If DPC is controlled by firmware (firmware first mode), firmware is responsible for configuring the DPC and OS is responsible for error recovery. Also, OS is allowed to modify DPC registers only during the EDR notification window. So with EDR support, OS should provide DPC port services even in firmware first mode. Cc: Keith Busch Signed-off-by: Kuppuswamy Sathyanarayanan Acked-by: Keith Busch Tested-by: Huong Nguyen Tested-by: Austin Bolen --- drivers/pci/pci-acpi.c | 91 drivers/pci/pcie/Kconfig | 10 drivers/pci/pcie/dpc.c | 125 ++- include/linux/pci-acpi.h | 11 4 files changed, 236 insertions(+), 1 deletion(-) diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c index 45049f558860..98f88d9c24ba 100644 --- a/drivers/pci/pci-acpi.c +++ b/drivers/pci/pci-acpi.c @@ -104,6 +104,97 @@ int acpi_get_rc_resources(struct device *dev, const char *hid, u16 segment, } #endif +#if defined(CONFIG_PCIE_DPC) && defined(CONFIG_ACPI) + +/* + * _DSM wrapper function to enable/disable DPC port. + * @dpc : DPC device structure + * @enable: status of DPC port (0 or 1). + * + * returns 0 on success or errno on failure. + */ +int acpi_enable_dpc_port(struct pci_dev *pdev, acpi_handle handle, bool enable) +{ + union acpi_object *obj; + int status = 0; + union acpi_object argv4; + + argv4.type = ACPI_TYPE_INTEGER; + argv4.integer.value = enable; + + obj = acpi_evaluate_dsm(handle, _acpi_dsm_guid, 5, + EDR_PORT_ENABLE_DSM, ); + if (!obj) + return -EIO; + + if (obj->type != ACPI_TYPE_INTEGER || obj->integer.value != enable) + status = -EIO; + + ACPI_FREE(obj); + + return status; +} + +/* + * _DSM wrapper function to locate DPC port. + * @dpc : DPC device structure + * + * returns pci_dev or NULL. + */ +struct pci_dev *acpi_locate_dpc_port(struct pci_dev *pdev, acpi_handle handle) +{ + union acpi_object *obj; + u16 port; + + obj = acpi_evaluate_dsm(handle, _acpi_dsm_guid, 5, + EDR_PORT_LOCATE_DSM, NULL); + if (!obj) + return pci_dev_get(pdev); + + if (obj->type != ACPI_TYPE_INTEGER) { + ACPI_FREE(obj); + return NULL; + } + + /* +* Firmware returns DPC port BDF details in following format: +* 15:8 = bus +* 7:3 = device +* 2:0 = function +*/ + port = obj->integer.value; + + ACPI_FREE(obj); + + return pci_get_domain_bus_and_slot(pci_domain_nr(pdev->bus), + PCI_BUS_NUM(port), port & 0xff); +} + +/* + * _OST wrapper function to let firmware know the status of EDR event. + * @dpc : DPC device structure. + * @status: Status of EDR event. + */ +int acpi_send_edr_status(struct pci_dev *pdev, acpi_handle handle, u16 status) +{ + u32 ost_status; + + pci_dbg(pdev, "Sending EDR status :%x\n", status); + + ost_status = PCI_DEVID(pdev->bus->number, pdev->devfn); + ost_status = (ost_status << 16) | status; + + status = acpi_evaluate_ost(handle, + ACPI_NOTIFY_DISCONNECT_RECOVER, + ost_status, NULL); + if (ACPI_FAILURE(status)) + return -EINVAL; + + return 0; +} + +#endif /* CONFIG_PCIE_DPC && CONFIG_ACPI */ + phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle) { acpi_status status = AE_NOT_EXIST; diff --git a/drivers/pci/pcie/Kconfig b/drivers/pci/pcie/Kconfig index 362eb8cfa53b..9a05015af7cd 100644 --- a/drivers/pci/pcie/Kconfig +++ b/drivers/pci/pcie/Kconfig @@ -150,3 +150,13 @@ config PCIE_BW This enables PCI Express Bandwidth Change Notification. If you know link width or rate changes occur only to correct unreliable links, you may answer Y. + +config PCIE_EDR + bool "PCI Express Error Disconnect Recover support" + default n + depends on PCIE_DPC && ACPI + help + This options adds Error Disconnect Recover support as specified + in PCI firmware specification v3.2 Downstream Port Containment + Related Enhancements ECN. Enable this if you want to support hybrid + DPC model which
[PATCH v8 1/8] PCI/ERR: Update error status after reset_link()
From: Kuppuswamy Sathyanarayanan Commit bdb5ac85777d ("PCI/ERR: Handle fatal error recovery") uses reset_link() to recover from fatal errors. But, if the reset is successful there is no need to continue the rest of the error recovery checks. Also, during fatal error recovery, if the initial value of error status is PCI_ERS_RESULT_DISCONNECT or PCI_ERS_RESULT_NO_AER_DRIVER then even after successful recovery (using reset_link()) pcie_do_recovery() will report the recovery result as failure. So update the status of error after reset_link(). Fixes: bdb5ac85777d ("PCI/ERR: Handle fatal error recovery") Cc: Ashok Raj Cc: Keith Busch Signed-off-by: Kuppuswamy Sathyanarayanan Acked-by: Keith Busch --- drivers/pci/pcie/err.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c index 773197a12568..c3e883d47675 100644 --- a/drivers/pci/pcie/err.c +++ b/drivers/pci/pcie/err.c @@ -204,9 +204,12 @@ void pcie_do_recovery(struct pci_dev *dev, enum pci_channel_state state, else pci_walk_bus(bus, report_normal_detected, ); - if (state == pci_channel_io_frozen && - reset_link(dev, service) != PCI_ERS_RESULT_RECOVERED) - goto failed; + if (state == pci_channel_io_frozen) { + status = reset_link(dev, service); + if (status != PCI_ERS_RESULT_RECOVERED) + goto failed; + goto done; + } if (status == PCI_ERS_RESULT_CAN_RECOVER) { status = PCI_ERS_RESULT_RECOVERED; @@ -228,6 +231,7 @@ void pcie_do_recovery(struct pci_dev *dev, enum pci_channel_state state, if (status != PCI_ERS_RESULT_RECOVERED) goto failed; +done: pci_dbg(dev, "broadcast resume message\n"); pci_walk_bus(bus, report_resume, ); -- 2.21.0
[PATCH v8 5/8] PCI/AER: Allow clearing Error Status Register in FF mode
From: Kuppuswamy Sathyanarayanan As per PCI firmware specification r3.2 Downstream Port Containment Related Enhancements ECN, sec 4.5.1, table 4-6, Error Disconnect Recover (EDR) support allows OS to handle error recovery and clearing Error Registers even in FF mode. So remove FF mode checks in pci_cleanup_aer_uncorrect_error_status(), pci_aer_clear_fatal_status() and pci_cleanup_aer_error_status_regs() functions. Signed-off-by: Kuppuswamy Sathyanarayanan Acked-by: Keith Busch --- drivers/pci/pcie/aer.c | 12 ++-- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c index b45bc47d04fe..e1509bb900c5 100644 --- a/drivers/pci/pcie/aer.c +++ b/drivers/pci/pcie/aer.c @@ -383,9 +383,6 @@ int pci_cleanup_aer_uncorrect_error_status(struct pci_dev *dev) if (!pos) return -EIO; - if (pcie_aer_get_firmware_first(dev)) - return -EIO; - /* Clear status bits for ERR_NONFATAL errors only */ pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_STATUS, ); pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_SEVER, ); @@ -406,9 +403,6 @@ void pci_aer_clear_fatal_status(struct pci_dev *dev) if (!pos) return; - if (pcie_aer_get_firmware_first(dev)) - return; - /* Clear status bits for ERR_FATAL errors only */ pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_STATUS, ); pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_SEVER, ); @@ -430,9 +424,6 @@ int pci_cleanup_aer_error_status_regs(struct pci_dev *dev) if (!pos) return -EIO; - if (pcie_aer_get_firmware_first(dev)) - return -EIO; - port_type = pci_pcie_type(dev); if (port_type == PCI_EXP_TYPE_ROOT_PORT) { pci_read_config_dword(dev, pos + PCI_ERR_ROOT_STATUS, ); @@ -455,7 +446,8 @@ void pci_aer_init(struct pci_dev *dev) if (dev->aer_cap) dev->aer_stats = kzalloc(sizeof(struct aer_stats), GFP_KERNEL); - pci_cleanup_aer_error_status_regs(dev); + if (!pcie_aer_get_firmware_first(dev)) + pci_cleanup_aer_error_status_regs(dev); } void pci_aer_exit(struct pci_dev *dev) -- 2.21.0
[PATCH v8 2/8] PCI/DPC: Allow dpc_probe() even if firmware first mode is enabled
From: Kuppuswamy Sathyanarayanan As per ACPI specification v6.3, sec 5.6.6, Error Disconnect Recover (EDR) notification used by firmware to let OS know about the DPC event and permit OS to perform error recovery when processing the EDR notification. Also, as per PCI firmware specification r3.2 Downstream Port Containment Related Enhancements ECN, sec 4.5.1, table 4-6, if DPC is controlled by firmware (firmware first mode), it's responsible for initializing Downstream Port Containment Extended Capability Structures per firmware policy. And, OS is permitted to read or write DPC Control and Status registers of a port while processing an Error Disconnect Recover (EDR) notification from firmware on that port. Currently, if firmware controls DPC (firmware first mode), OS will not create/enumerate DPC PCIe port services. But, if OS supports EDR feature, then as mentioned in above spec references, it should permit enumeration of DPC driver and also support handling ACPI EDR notification. So as first step, allow dpc_probe() to continue even if firmware first mode is enabled. Also add appropriate checks to ensure device registers are not modified outside EDR notification window in firmware first mode. This is a preparatory patch for adding EDR support. Signed-off-by: Kuppuswamy Sathyanarayanan Acked-by: Keith Busch --- drivers/pci/pcie/dpc.c | 49 +++--- 1 file changed, 36 insertions(+), 13 deletions(-) diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index a32ec3487a8d..9717fda012f8 100644 --- a/drivers/pci/pcie/dpc.c +++ b/drivers/pci/pcie/dpc.c @@ -22,6 +22,8 @@ struct dpc_dev { u16 cap_pos; boolrp_extensions; u8 rp_log_size; + /* Set True if DPC is controlled by firmware */ + boolfirmware_dpc; }; static const char * const rp_pio_error_string[] = { @@ -69,6 +71,9 @@ void pci_save_dpc_state(struct pci_dev *dev) if (!dpc) return; + if (dpc->firmware_dpc) + return; + save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_DPC); if (!save_state) return; @@ -90,6 +95,9 @@ void pci_restore_dpc_state(struct pci_dev *dev) if (!dpc) return; + if (dpc->firmware_dpc) + return; + save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_DPC); if (!save_state) return; @@ -291,9 +299,6 @@ static int dpc_probe(struct pcie_device *dev) int status; u16 ctl, cap; - if (pcie_aer_get_firmware_first(pdev)) - return -ENOTSUPP; - dpc = devm_kzalloc(device, sizeof(*dpc), GFP_KERNEL); if (!dpc) return -ENOMEM; @@ -302,13 +307,25 @@ static int dpc_probe(struct pcie_device *dev) dpc->dev = dev; set_service_data(dev, dpc); - status = devm_request_threaded_irq(device, dev->irq, dpc_irq, - dpc_handler, IRQF_SHARED, - "pcie-dpc", dpc); - if (status) { - pci_warn(pdev, "request IRQ%d failed: %d\n", dev->irq, -status); - return status; + if (pcie_aer_get_firmware_first(pdev)) + dpc->firmware_dpc = 1; + + /* +* If DPC is handled in firmware and ACPI support is not enabled +* in OS, skip probe and return error. +*/ + if (dpc->firmware_dpc && !IS_ENABLED(CONFIG_ACPI)) + return -ENODEV; + + if (!dpc->firmware_dpc) { + status = devm_request_threaded_irq(device, dev->irq, dpc_irq, + dpc_handler, IRQF_SHARED, + "pcie-dpc", dpc); + if (status) { + pci_warn(pdev, "request IRQ%d failed: %d\n", dev->irq, +status); + return status; + } } pci_read_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_CAP, ); @@ -323,9 +340,12 @@ static int dpc_probe(struct pcie_device *dev) dpc->rp_log_size = 0; } } - - ctl = (ctl & 0xfff4) | PCI_EXP_DPC_CTL_EN_FATAL | PCI_EXP_DPC_CTL_INT_EN; - pci_write_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_CTL, ctl); + if (!dpc->firmware_dpc) { + ctl = (ctl & 0xfff4) | + (PCI_EXP_DPC_CTL_EN_FATAL | PCI_EXP_DPC_CTL_INT_EN); + pci_write_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_CTL, + ctl); + } pci_info(pdev, "error containment capabilities: Int Msg #%d, RPExt%c PoisonedTLP%c SwTrigger%c RP PIO Log %d, DL_ActiveErr%c\n", cap & PCI_EXP_DPC_IRQ, FLAG(cap, PCI_EXP_DPC_CAP_RP_EXT), @@ -343,6 +363,9 @@
[PATCH v7 4/8] PCI/DPC: Add Error Disconnect Recover (EDR) support
From: Kuppuswamy Sathyanarayanan As per ACPI specification r6.3, sec 5.6.6, when firmware owns Downstream Port Containment (DPC), its expected to use the "Error Disconnect Recover" (EDR) notification to alert OSPM of a DPC event and if OS supports EDR, its expected to handle the software state invalidation and port recovery in OS, and also let firmware know the recovery status via _OST ACPI call. Related _OST status codes can be found in ACPI specification r6.3, sec 6.3.5.2. Also, as per PCI firmware specification r3.2 Downstream Port Containment Related Enhancements ECN, sec 4.5.1, table 4-6, If DPC is controlled by firmware (firmware first mode), firmware is responsible for configuring the DPC and OS is responsible for error recovery. Also, OS is allowed to modify DPC registers only during the EDR notification window. So with EDR support, OS should provide DPC port services even in firmware first mode. Cc: Keith Busch Signed-off-by: Kuppuswamy Sathyanarayanan Acked-by: Keith Busch Tested-by: Huong Nguyen Tested-by: Austin Bolen --- drivers/pci/pci-acpi.c | 91 drivers/pci/pcie/Kconfig | 10 drivers/pci/pcie/dpc.c | 125 ++- include/linux/pci-acpi.h | 11 4 files changed, 236 insertions(+), 1 deletion(-) diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c index 45049f558860..82abab25daf2 100644 --- a/drivers/pci/pci-acpi.c +++ b/drivers/pci/pci-acpi.c @@ -104,6 +104,97 @@ int acpi_get_rc_resources(struct device *dev, const char *hid, u16 segment, } #endif +#if defined(CONFIG_PCIE_DPC) && defined(CONFIG_ACPI) + +/* + * _DSM wrapper function to enable/disable DPC port. + * @dpc : DPC device structure + * @enable: status of DPC port (0 or 1). + * + * returns 0 on success or errno on failure. + */ +int acpi_enable_dpc_port(struct pci_dev *pdev, acpi_handle handle, bool enable) +{ + union acpi_object *obj; + int status = 0; + union acpi_object argv4; + + argv4.type = ACPI_TYPE_INTEGER; + argv4.integer.value = enable; + + obj = acpi_evaluate_dsm(handle, _acpi_dsm_guid, 1, + EDR_PORT_ENABLE_DSM, ); + if (!obj) + return -EIO; + + if (obj->type != ACPI_TYPE_INTEGER || obj->integer.value != enable) + status = -EIO; + + ACPI_FREE(obj); + + return status; +} + +/* + * _DSM wrapper function to locate DPC port. + * @dpc : DPC device structure + * + * returns pci_dev or NULL. + */ +struct pci_dev *acpi_locate_dpc_port(struct pci_dev *pdev, acpi_handle handle) +{ + union acpi_object *obj; + u16 port; + + obj = acpi_evaluate_dsm(handle, _acpi_dsm_guid, 1, + EDR_PORT_LOCATE_DSM, NULL); + if (!obj) + return pci_dev_get(pdev); + + if (obj->type != ACPI_TYPE_INTEGER) { + ACPI_FREE(obj); + return NULL; + } + + /* +* Firmware returns DPC port BDF details in following format: +* 15:8 = bus +* 7:3 = device +* 2:0 = function +*/ + port = obj->integer.value; + + ACPI_FREE(obj); + + return pci_get_domain_bus_and_slot(pci_domain_nr(pdev->bus), + PCI_BUS_NUM(port), port & 0xff); +} + +/* + * _OST wrapper function to let firmware know the status of EDR event. + * @dpc : DPC device structure. + * @status: Status of EDR event. + */ +int acpi_send_edr_status(struct pci_dev *pdev, acpi_handle handle, u16 status) +{ + u32 ost_status; + + pci_dbg(pdev, "Sending EDR status :%x\n", status); + + ost_status = PCI_DEVID(pdev->bus->number, pdev->devfn); + ost_status = (ost_status << 16) | status; + + status = acpi_evaluate_ost(handle, + ACPI_NOTIFY_DISCONNECT_RECOVER, + ost_status, NULL); + if (ACPI_FAILURE(status)) + return -EINVAL; + + return 0; +} + +#endif /* CONFIG_PCIE_DPC && CONFIG_ACPI */ + phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle) { acpi_status status = AE_NOT_EXIST; diff --git a/drivers/pci/pcie/Kconfig b/drivers/pci/pcie/Kconfig index 362eb8cfa53b..9a05015af7cd 100644 --- a/drivers/pci/pcie/Kconfig +++ b/drivers/pci/pcie/Kconfig @@ -150,3 +150,13 @@ config PCIE_BW This enables PCI Express Bandwidth Change Notification. If you know link width or rate changes occur only to correct unreliable links, you may answer Y. + +config PCIE_EDR + bool "PCI Express Error Disconnect Recover support" + default n + depends on PCIE_DPC && ACPI + help + This options adds Error Disconnect Recover support as specified + in PCI firmware specification v3.2 Downstream Port Containment + Related Enhancements ECN. Enable this if you want to support hybrid + DPC model which
[PATCH v7 2/8] PCI/DPC: Allow dpc_probe() even if firmware first mode is enabled
From: Kuppuswamy Sathyanarayanan As per ACPI specification v6.3, sec 5.6.6, Error Disconnect Recover (EDR) notification used by firmware to let OS know about the DPC event and permit OS to perform error recovery when processing the EDR notification. Also, as per PCI firmware specification r3.2 Downstream Port Containment Related Enhancements ECN, sec 4.5.1, table 4-6, if DPC is controlled by firmware (firmware first mode), it's responsible for initializing Downstream Port Containment Extended Capability Structures per firmware policy. And, OS is permitted to read or write DPC Control and Status registers of a port while processing an Error Disconnect Recover (EDR) notification from firmware on that port. Currently, if firmware controls DPC (firmware first mode), OS will not create/enumerate DPC PCIe port services. But, if OS supports EDR feature, then as mentioned in above spec references, it should permit enumeration of DPC driver and also support handling ACPI EDR notification. So as first step, allow dpc_probe() to continue even if firmware first mode is enabled. Also add appropriate checks to ensure device registers are not modified outside EDR notification window in firmware first mode. This is a preparatory patch for adding EDR support. Signed-off-by: Kuppuswamy Sathyanarayanan Acked-by: Keith Busch --- drivers/pci/pcie/dpc.c | 49 +++--- 1 file changed, 36 insertions(+), 13 deletions(-) diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index a32ec3487a8d..9717fda012f8 100644 --- a/drivers/pci/pcie/dpc.c +++ b/drivers/pci/pcie/dpc.c @@ -22,6 +22,8 @@ struct dpc_dev { u16 cap_pos; boolrp_extensions; u8 rp_log_size; + /* Set True if DPC is controlled by firmware */ + boolfirmware_dpc; }; static const char * const rp_pio_error_string[] = { @@ -69,6 +71,9 @@ void pci_save_dpc_state(struct pci_dev *dev) if (!dpc) return; + if (dpc->firmware_dpc) + return; + save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_DPC); if (!save_state) return; @@ -90,6 +95,9 @@ void pci_restore_dpc_state(struct pci_dev *dev) if (!dpc) return; + if (dpc->firmware_dpc) + return; + save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_DPC); if (!save_state) return; @@ -291,9 +299,6 @@ static int dpc_probe(struct pcie_device *dev) int status; u16 ctl, cap; - if (pcie_aer_get_firmware_first(pdev)) - return -ENOTSUPP; - dpc = devm_kzalloc(device, sizeof(*dpc), GFP_KERNEL); if (!dpc) return -ENOMEM; @@ -302,13 +307,25 @@ static int dpc_probe(struct pcie_device *dev) dpc->dev = dev; set_service_data(dev, dpc); - status = devm_request_threaded_irq(device, dev->irq, dpc_irq, - dpc_handler, IRQF_SHARED, - "pcie-dpc", dpc); - if (status) { - pci_warn(pdev, "request IRQ%d failed: %d\n", dev->irq, -status); - return status; + if (pcie_aer_get_firmware_first(pdev)) + dpc->firmware_dpc = 1; + + /* +* If DPC is handled in firmware and ACPI support is not enabled +* in OS, skip probe and return error. +*/ + if (dpc->firmware_dpc && !IS_ENABLED(CONFIG_ACPI)) + return -ENODEV; + + if (!dpc->firmware_dpc) { + status = devm_request_threaded_irq(device, dev->irq, dpc_irq, + dpc_handler, IRQF_SHARED, + "pcie-dpc", dpc); + if (status) { + pci_warn(pdev, "request IRQ%d failed: %d\n", dev->irq, +status); + return status; + } } pci_read_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_CAP, ); @@ -323,9 +340,12 @@ static int dpc_probe(struct pcie_device *dev) dpc->rp_log_size = 0; } } - - ctl = (ctl & 0xfff4) | PCI_EXP_DPC_CTL_EN_FATAL | PCI_EXP_DPC_CTL_INT_EN; - pci_write_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_CTL, ctl); + if (!dpc->firmware_dpc) { + ctl = (ctl & 0xfff4) | + (PCI_EXP_DPC_CTL_EN_FATAL | PCI_EXP_DPC_CTL_INT_EN); + pci_write_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_CTL, + ctl); + } pci_info(pdev, "error containment capabilities: Int Msg #%d, RPExt%c PoisonedTLP%c SwTrigger%c RP PIO Log %d, DL_ActiveErr%c\n", cap & PCI_EXP_DPC_IRQ, FLAG(cap, PCI_EXP_DPC_CAP_RP_EXT), @@ -343,6 +363,9 @@
[PATCH v7 3/8] PCI/DPC: Add dpc_process_error() wrapper function
From: Kuppuswamy Sathyanarayanan With Error Disconnect Recover (EDR) support, we need to support processing DPC event either from DPC IRQ or ACPI EDR event. So create a wrapper function dpc_process_error() and move common error handling code in to it. It will be used to process the DPC event in both DPC IRQ and EDR ACPI event contexts. Signed-off-by: Kuppuswamy Sathyanarayanan Acked-by: Keith Busch --- drivers/pci/pcie/dpc.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index 9717fda012f8..4137ec7b48cc 100644 --- a/drivers/pci/pcie/dpc.c +++ b/drivers/pci/pcie/dpc.c @@ -232,10 +232,9 @@ static int dpc_get_aer_uncorrect_severity(struct pci_dev *dev, return 1; } -static irqreturn_t dpc_handler(int irq, void *context) +static void dpc_process_error(struct dpc_dev *dpc) { struct aer_err_info info; - struct dpc_dev *dpc = context; struct pci_dev *pdev = dpc->dev->port; u16 cap = dpc->cap_pos, status, source, reason, ext_reason; @@ -268,6 +267,13 @@ static irqreturn_t dpc_handler(int irq, void *context) /* We configure DPC so it only triggers on ERR_FATAL */ pcie_do_recovery(pdev, pci_channel_io_frozen, PCIE_PORT_SERVICE_DPC); +} + +static irqreturn_t dpc_handler(int irq, void *context) +{ + struct dpc_dev *dpc = context; + + dpc_process_error(dpc); return IRQ_HANDLED; } -- 2.21.0
[PATCH v7 5/8] PCI/AER: Allow clearing Error Status Register in FF mode
From: Kuppuswamy Sathyanarayanan As per PCI firmware specification r3.2 Downstream Port Containment Related Enhancements ECN, sec 4.5.1, table 4-6, Error Disconnect Recover (EDR) support allows OS to handle error recovery and clearing Error Registers even in FF mode. So remove FF mode checks in pci_cleanup_aer_uncorrect_error_status(), pci_aer_clear_fatal_status() and pci_cleanup_aer_error_status_regs() functions. Signed-off-by: Kuppuswamy Sathyanarayanan Acked-by: Keith Busch --- drivers/pci/pcie/aer.c | 12 ++-- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c index b45bc47d04fe..e1509bb900c5 100644 --- a/drivers/pci/pcie/aer.c +++ b/drivers/pci/pcie/aer.c @@ -383,9 +383,6 @@ int pci_cleanup_aer_uncorrect_error_status(struct pci_dev *dev) if (!pos) return -EIO; - if (pcie_aer_get_firmware_first(dev)) - return -EIO; - /* Clear status bits for ERR_NONFATAL errors only */ pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_STATUS, ); pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_SEVER, ); @@ -406,9 +403,6 @@ void pci_aer_clear_fatal_status(struct pci_dev *dev) if (!pos) return; - if (pcie_aer_get_firmware_first(dev)) - return; - /* Clear status bits for ERR_FATAL errors only */ pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_STATUS, ); pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_SEVER, ); @@ -430,9 +424,6 @@ int pci_cleanup_aer_error_status_regs(struct pci_dev *dev) if (!pos) return -EIO; - if (pcie_aer_get_firmware_first(dev)) - return -EIO; - port_type = pci_pcie_type(dev); if (port_type == PCI_EXP_TYPE_ROOT_PORT) { pci_read_config_dword(dev, pos + PCI_ERR_ROOT_STATUS, ); @@ -455,7 +446,8 @@ void pci_aer_init(struct pci_dev *dev) if (dev->aer_cap) dev->aer_stats = kzalloc(sizeof(struct aer_stats), GFP_KERNEL); - pci_cleanup_aer_error_status_regs(dev); + if (!pcie_aer_get_firmware_first(dev)) + pci_cleanup_aer_error_status_regs(dev); } void pci_aer_exit(struct pci_dev *dev) -- 2.21.0
[PATCH v7 7/8] PCI/DPC: Clear AER registers in EDR mode
From: Kuppuswamy Sathyanarayanan As per PCI firmware specification r3.2 Downstream Port Containment Related Enhancements ECN, OS is responsible for clearing the AER registers in EDR mode. So clear AER registers in dpc_process_error() function. Signed-off-by: Kuppuswamy Sathyanarayanan Acked-by: Keith Busch --- drivers/pci/pcie/dpc.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index fafc55c00fe0..de2d892bc7c4 100644 --- a/drivers/pci/pcie/dpc.c +++ b/drivers/pci/pcie/dpc.c @@ -275,6 +275,10 @@ static void dpc_process_error(struct dpc_dev *dpc) pci_aer_clear_fatal_status(pdev); } + /* In EDR mode, OS is responsible for clearing AER registers */ + if (dpc->firmware_dpc) + pci_cleanup_aer_error_status_regs(pdev); + /* * Irrespective of whether the DPC event is triggered by * ERR_FATAL or ERR_NONFATAL, since the link is already down, -- 2.21.0
[PATCH v7 6/8] PCI/DPC: Update comments related to DPC recovery on NON_FATAL errors
From: Kuppuswamy Sathyanarayanan Currently, in native mode, DPC driver is configured to trigger DPC only for FATAL errors and hence it only supports port recovery for FATAL errors. But with Error Disconnect Recover (EDR) support, DPC configuration is done by firmware, and hence we should expect DPC triggered for both FATAL/NON_FATAL errors. So update comments and add details about how NON_FATAL dpc recovery is handled. Signed-off-by: Kuppuswamy Sathyanarayanan Acked-by: Keith Busch --- drivers/pci/pcie/dpc.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index a8b634e05694..fafc55c00fe0 100644 --- a/drivers/pci/pcie/dpc.c +++ b/drivers/pci/pcie/dpc.c @@ -275,7 +275,11 @@ static void dpc_process_error(struct dpc_dev *dpc) pci_aer_clear_fatal_status(pdev); } - /* We configure DPC so it only triggers on ERR_FATAL */ + /* +* Irrespective of whether the DPC event is triggered by +* ERR_FATAL or ERR_NONFATAL, since the link is already down, +* use the FATAL error recovery path for both cases. +*/ pcie_do_recovery(pdev, pci_channel_io_frozen, PCIE_PORT_SERVICE_DPC); } -- 2.21.0
[PATCH v7 8/8] PCI/ACPI: Enable EDR support
From: Kuppuswamy Sathyanarayanan As per PCI firmware specification r3.2 Downstream Port Containment Related Enhancements ECN, sec 4.5.1, OS must implement following steps to enable/use EDR feature. 1. OS can use bit 7 of _OSC Control Field to negotiate control over Downstream Port Containment (DPC) configuration of PCIe port. After _OSC negotiation, firmware will Set this bit to grant OS control over PCIe DPC configuration and Clear it if this feature was requested and denied, or was not requested. 2. Also, if OS supports EDR, it should expose its support to BIOS by setting bit 7 of _OSC Support Field. And if OS sets bit 7 of _OSC Control Field it must also expose support for EDR by setting bit 7 of _OSC Support Field. Cc: Bjorn Helgaas Cc: "Rafael J. Wysocki" Cc: Len Brown Signed-off-by: Kuppuswamy Sathyanarayanan Acked-by: Keith Busch Tested-by: Huong Nguyen Tested-by: Austin Bolen --- drivers/acpi/pci_root.c | 9 + drivers/pci/pcie/portdrv_core.c | 8 +++- drivers/pci/probe.c | 1 + include/linux/acpi.h| 6 -- include/linux/pci.h | 3 ++- 5 files changed, 23 insertions(+), 4 deletions(-) diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index 314a187ed572..988d09d788b6 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -132,6 +132,7 @@ static struct pci_osc_bit_struct pci_osc_support_bit[] = { { OSC_PCI_CLOCK_PM_SUPPORT, "ClockPM" }, { OSC_PCI_SEGMENT_GROUPS_SUPPORT, "Segments" }, { OSC_PCI_MSI_SUPPORT, "MSI" }, + { OSC_PCI_EDR_SUPPORT, "EDR" }, { OSC_PCI_HPX_TYPE_3_SUPPORT, "HPX-Type3" }, }; @@ -142,6 +143,7 @@ static struct pci_osc_bit_struct pci_osc_control_bit[] = { { OSC_PCI_EXPRESS_AER_CONTROL, "AER" }, { OSC_PCI_EXPRESS_CAPABILITY_CONTROL, "PCIeCapability" }, { OSC_PCI_EXPRESS_LTR_CONTROL, "LTR" }, + { OSC_PCI_EXPRESS_DPC_CONTROL, "DPC" }, }; static void decode_osc_bits(struct acpi_pci_root *root, char *msg, u32 word, @@ -441,6 +443,8 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm, support |= OSC_PCI_ASPM_SUPPORT | OSC_PCI_CLOCK_PM_SUPPORT; if (pci_msi_enabled()) support |= OSC_PCI_MSI_SUPPORT; + if (IS_ENABLED(CONFIG_PCIE_EDR)) + support |= OSC_PCI_EDR_SUPPORT; decode_osc_support(root, "OS supports", support); status = acpi_pci_osc_support(root, support); @@ -488,6 +492,9 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm, control |= OSC_PCI_EXPRESS_AER_CONTROL; } + if (IS_ENABLED(CONFIG_PCIE_DPC)) + control |= OSC_PCI_EXPRESS_DPC_CONTROL; + requested = control; status = acpi_pci_osc_control_set(handle, , OSC_PCI_EXPRESS_CAPABILITY_CONTROL); @@ -917,6 +924,8 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, host_bridge->native_pme = 0; if (!(root->osc_control_set & OSC_PCI_EXPRESS_LTR_CONTROL)) host_bridge->native_ltr = 0; + if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL)) + host_bridge->native_dpc = 0; /* * Evaluate the "PCI Boot Configuration" _DSM Function. If it diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index 1b330129089f..1b54a39df795 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -250,8 +250,14 @@ static int get_port_device_capability(struct pci_dev *dev) pcie_pme_interrupt_enable(dev, false); } + /* +* If EDR support is enabled in OS, then even if AER is not handled in +* OS, DPC service can be enabled. +*/ if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) && - pci_aer_available() && services & PCIE_PORT_SERVICE_AER) + ((IS_ENABLED(CONFIG_PCIE_EDR) && !host->native_dpc) || + (pci_aer_available() && services & PCIE_PORT_SERVICE_AER && + (pcie_ports_native || host->native_dpc services |= PCIE_PORT_SERVICE_DPC; if (pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM || diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index a3c7338fad86..cf8acdd62089 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -601,6 +601,7 @@ static void pci_init_host_bridge(struct pci_host_bridge *bridge) bridge->native_shpc_hotplug = 1; bridge->native_pme = 1; bridge->native_ltr = 1; + bridge->native_dpc = 1; } struct pci_host_bridge *pci_alloc_host_bridge(size_t priv) diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 9426b9aaed86..7b29c640a9db 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -515,8 +515,9 @@ extern bool osc_pc_lpi_support_confirmed; #define OSC_PCI_CLOCK_PM_SUPPORT
[PATCH v7 0/8] Add Error Disconnect Recover (EDR) support
From: Kuppuswamy Sathyanarayanan This patchset adds support for following features: 1. Error Disconnect Recover (EDR) support. 2. _OSC based negotiation support for DPC. You can find EDR spec in the following link. https://members.pcisig.com/wg/PCI-SIG/document/12614 Changes since v6: * Modified the order of patches to enable EDR only after all necessary support is added in kernel. * Addressed Bjorn comments. Changes since v5: * Addressed Keith's comments. * Added additional check for FF mode in pci_aer_init(). * Updated commit history of "PCI/DPC: Add support for DPC recovery on NON_FATAL errors" patch. Changes since v4: * Rebased on top of v5.3-rc1 * Fixed lock/unlock issue in edr_handle_event(). * Merged "Update error status after reset_link()" patch into this patchset. Changes since v3: * Moved EDR related ACPI functions/definitions to pci-acpi.c * Modified commit history in few patches to include spec reference. * Added support to handle DPC triggered by NON_FATAL errors. * Added edr_lock to protect PCI device receiving duplicate EDR notifications. * Addressed Bjorn comments. Changes since v2: * Split EDR support patch into multiple patches. * Addressed Bjorn comments. Changes since v1: * Rebased on top of v5.1-rc1 Kuppuswamy Sathyanarayanan (8): PCI/ERR: Update error status after reset_link() PCI/DPC: Allow dpc_probe() even if firmware first mode is enabled PCI/DPC: Add dpc_process_error() wrapper function PCI/DPC: Add Error Disconnect Recover (EDR) support PCI/AER: Allow clearing Error Status Register in FF mode PCI/DPC: Update comments related to DPC recovery on NON_FATAL errors PCI/DPC: Clear AER registers in EDR mode PCI/ACPI: Enable EDR support drivers/acpi/pci_root.c | 9 ++ drivers/pci/pci-acpi.c | 91 +++ drivers/pci/pcie/Kconfig| 10 ++ drivers/pci/pcie/aer.c | 12 +- drivers/pci/pcie/dpc.c | 194 +--- drivers/pci/pcie/err.c | 10 +- drivers/pci/pcie/portdrv_core.c | 8 +- drivers/pci/probe.c | 1 + include/linux/acpi.h| 6 +- include/linux/pci-acpi.h| 11 ++ include/linux/pci.h | 3 +- 11 files changed, 321 insertions(+), 34 deletions(-) -- 2.21.0
[PATCH v7 1/8] PCI/ERR: Update error status after reset_link()
From: Kuppuswamy Sathyanarayanan Commit bdb5ac85777d ("PCI/ERR: Handle fatal error recovery") uses reset_link() to recover from fatal errors. But, if the reset is successful there is no need to continue the rest of the error recovery checks. Also, during fatal error recovery, if the initial value of error status is PCI_ERS_RESULT_DISCONNECT or PCI_ERS_RESULT_NO_AER_DRIVER then even after successful recovery (using reset_link()) pcie_do_recovery() will report the recovery result as failure. So update the status of error after reset_link(). Fixes: bdb5ac85777d ("PCI/ERR: Handle fatal error recovery") Cc: Ashok Raj Cc: Keith Busch Signed-off-by: Kuppuswamy Sathyanarayanan Acked-by: Keith Busch --- drivers/pci/pcie/err.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c index 773197a12568..c3e883d47675 100644 --- a/drivers/pci/pcie/err.c +++ b/drivers/pci/pcie/err.c @@ -204,9 +204,12 @@ void pcie_do_recovery(struct pci_dev *dev, enum pci_channel_state state, else pci_walk_bus(bus, report_normal_detected, ); - if (state == pci_channel_io_frozen && - reset_link(dev, service) != PCI_ERS_RESULT_RECOVERED) - goto failed; + if (state == pci_channel_io_frozen) { + status = reset_link(dev, service); + if (status != PCI_ERS_RESULT_RECOVERED) + goto failed; + goto done; + } if (status == PCI_ERS_RESULT_CAN_RECOVER) { status = PCI_ERS_RESULT_RECOVERED; @@ -228,6 +231,7 @@ void pcie_do_recovery(struct pci_dev *dev, enum pci_channel_state state, if (status != PCI_ERS_RESULT_RECOVERED) goto failed; +done: pci_dbg(dev, "broadcast resume message\n"); pci_walk_bus(bus, report_resume, ); -- 2.21.0
[PATCH v6 5/8] PCI/ATS: Add PRI support for PCIe VF devices
From: Kuppuswamy Sathyanarayanan When IOMMU tries to enable Page Request Interface (PRI) for VF device in iommu_enable_dev_iotlb(), it always fails because PRI support for PCIe VF device is currently broken. Current implementation expects the given PCIe device (PF & VF) to implement PRI capability before enabling the PRI support. But this assumption is incorrect. As per PCIe spec r4.0, sec 9.3.7.11, all VFs associated with PF can only use the PRI of the PF and not implement it. Hence we need to create exception for handling the PRI support for PCIe VF device. Also, since PRI is a shared resource between PF/VF, following rules should apply. 1. Use proper locking before accessing/modifying PF resources in VF PRI enable/disable call. 2. Use reference count logic to track the usage of PRI resource. 3. Disable PRI only if the PRI reference count (pri_ref_cnt) is zero. Cc: Ashok Raj Cc: Keith Busch Suggested-by: Ashok Raj Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/ats.c | 121 ++-- include/linux/pci.h | 1 + 2 files changed, 95 insertions(+), 27 deletions(-) diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c index 022698a85c98..e71187d83401 100644 --- a/drivers/pci/ats.c +++ b/drivers/pci/ats.c @@ -21,6 +21,15 @@ static void pci_pri_init(struct pci_dev *pdev) #ifdef CONFIG_PCI_PRI int pos; + /* +* As per PCIe r4.0, sec 9.3.7.11, only PF is permitted to +* implement PRI and all associated VFs can only use it. +* Since PF already initialized the PRI parameters there is +* no need to proceed further. +*/ + if (pdev->is_virtfn) + return; + pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI); if (!pos) return; @@ -208,31 +217,55 @@ EXPORT_SYMBOL_GPL(pci_ats_page_aligned); */ int pci_enable_pri(struct pci_dev *pdev, u32 reqs) { - u16 control, status; + u16 status; u32 max_requests; + int ret = 0; + struct pci_dev *pf = pci_physfn(pdev); if (WARN_ON(pdev->pri_enabled)) return -EBUSY; - if (!pdev->pri_cap) + if (!pf->pri_cap) return -EINVAL; - pci_read_config_word(pdev, pdev->pri_cap + PCI_PRI_STATUS, ); - if (!(status & PCI_PRI_STATUS_STOPPED)) - return -EBUSY; + pci_physfn_reslock(pdev); + + if (pdev->is_virtfn && pf->pri_enabled) + goto update_status; - pci_read_config_dword(pdev, pdev->pri_cap + PCI_PRI_MAX_REQ, - _requests); + /* +* Before updating PRI registers, make sure there is no +* outstanding PRI requests. +*/ + pci_read_config_word(pf, pf->pri_cap + PCI_PRI_STATUS, ); + if (!(status & PCI_PRI_STATUS_STOPPED)) { + ret = -EBUSY; + goto done; + } + + pci_read_config_dword(pf, pf->pri_cap + PCI_PRI_MAX_REQ, _requests); reqs = min(max_requests, reqs); - pdev->pri_reqs_alloc = reqs; - pci_write_config_dword(pdev, pdev->pri_cap + PCI_PRI_ALLOC_REQ, reqs); + pf->pri_reqs_alloc = reqs; + pci_write_config_dword(pf, pf->pri_cap + PCI_PRI_ALLOC_REQ, reqs); - control = PCI_PRI_CTRL_ENABLE; - pci_write_config_word(pdev, pdev->pri_cap + PCI_PRI_CTRL, control); + pci_write_config_word(pf, pf->pri_cap + PCI_PRI_CTRL, + PCI_PRI_CTRL_ENABLE); - pdev->pri_enabled = 1; + /* +* If PRI is not already enabled in PF, increment the PF +* pri_ref_cnt to track the usage of PRI interface. +*/ + if (pdev->is_virtfn && !pf->pri_enabled) { + atomic_inc(>pri_ref_cnt); + pf->pri_enabled = 1; + } - return 0; +update_status: + atomic_inc(>pri_ref_cnt); + pdev->pri_enabled = 1; +done: + pci_physfn_resunlock(pdev); + return ret; } EXPORT_SYMBOL_GPL(pci_enable_pri); @@ -245,18 +278,32 @@ EXPORT_SYMBOL_GPL(pci_enable_pri); void pci_disable_pri(struct pci_dev *pdev) { u16 control; + struct pci_dev *pf = pci_physfn(pdev); if (WARN_ON(!pdev->pri_enabled)) return; - if (!pdev->pri_cap) + if (!pf->pri_cap) return; - pci_read_config_word(pdev, pdev->pri_cap + PCI_PRI_CTRL, ); + pci_physfn_reslock(pdev); + + atomic_dec(>pri_ref_cnt); + + /* +* If pri_ref_cnt is not zero, then don't modify hardware +* registers. +*/ + if (atomic_read(>pri_ref_cnt)) + goto done; + + pci_read_config_word(pf, pf->pri_cap + PCI_PRI_CTRL, ); control &= ~PCI_PRI_CTRL_ENABLE; - pci_write_config_word(pdev, pdev->pri_cap + PCI_PRI_CTRL, control); + pci_write_config_word(pf, pf->pri_cap + PCI_PRI_CTRL, control); +done: pdev->pri_enabled = 0; +
[PATCH v6 3/8] PCI/ATS: Cache PASID capability check result
From: Kuppuswamy Sathyanarayanan Currently, PASID capability checks are repeated across all PASID API's. Instead, cache the capability check result in pci_pasid_init() and use it in other PASID API's. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/ats.c | 50 ++--- include/linux/pci.h | 1 + 2 files changed, 30 insertions(+), 21 deletions(-) diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c index b863562b6702..022698a85c98 100644 --- a/drivers/pci/ats.c +++ b/drivers/pci/ats.c @@ -29,6 +29,19 @@ static void pci_pri_init(struct pci_dev *pdev) #endif } +static void pci_pasid_init(struct pci_dev *pdev) +{ +#ifdef CONFIG_PCI_PASID + int pos; + + pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PASID); + if (!pos) + return; + + pdev->pasid_cap = pos; +#endif +} + void pci_ats_init(struct pci_dev *dev) { int pos; @@ -43,6 +56,8 @@ void pci_ats_init(struct pci_dev *dev) dev->ats_cap = pos; pci_pri_init(dev); + + pci_pasid_init(dev); } /** @@ -303,7 +318,6 @@ EXPORT_SYMBOL_GPL(pci_reset_pri); int pci_enable_pasid(struct pci_dev *pdev, int features) { u16 control, supported; - int pos; if (WARN_ON(pdev->pasid_enabled)) return -EBUSY; @@ -311,11 +325,11 @@ int pci_enable_pasid(struct pci_dev *pdev, int features) if (!pdev->eetlp_prefix_path) return -EINVAL; - pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PASID); - if (!pos) + if (!pdev->pasid_cap) return -EINVAL; - pci_read_config_word(pdev, pos + PCI_PASID_CAP, ); + pci_read_config_word(pdev, pdev->pasid_cap + PCI_PASID_CAP, +); supported &= PCI_PASID_CAP_EXEC | PCI_PASID_CAP_PRIV; /* User wants to enable anything unsupported? */ @@ -325,7 +339,7 @@ int pci_enable_pasid(struct pci_dev *pdev, int features) control = PCI_PASID_CTRL_ENABLE | features; pdev->pasid_features = features; - pci_write_config_word(pdev, pos + PCI_PASID_CTRL, control); + pci_write_config_word(pdev, pdev->pasid_cap + PCI_PASID_CTRL, control); pdev->pasid_enabled = 1; @@ -340,16 +354,14 @@ EXPORT_SYMBOL_GPL(pci_enable_pasid); void pci_disable_pasid(struct pci_dev *pdev) { u16 control = 0; - int pos; if (WARN_ON(!pdev->pasid_enabled)) return; - pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PASID); - if (!pos) + if (!pdev->pasid_cap) return; - pci_write_config_word(pdev, pos + PCI_PASID_CTRL, control); + pci_write_config_word(pdev, pdev->pasid_cap + PCI_PASID_CTRL, control); pdev->pasid_enabled = 0; } @@ -362,17 +374,15 @@ EXPORT_SYMBOL_GPL(pci_disable_pasid); void pci_restore_pasid_state(struct pci_dev *pdev) { u16 control; - int pos; if (!pdev->pasid_enabled) return; - pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PASID); - if (!pos) + if (!pdev->pasid_cap) return; control = PCI_PASID_CTRL_ENABLE | pdev->pasid_features; - pci_write_config_word(pdev, pos + PCI_PASID_CTRL, control); + pci_write_config_word(pdev, pdev->pasid_cap + PCI_PASID_CTRL, control); } EXPORT_SYMBOL_GPL(pci_restore_pasid_state); @@ -389,13 +399,12 @@ EXPORT_SYMBOL_GPL(pci_restore_pasid_state); int pci_pasid_features(struct pci_dev *pdev) { u16 supported; - int pos; - pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PASID); - if (!pos) + if (!pdev->pasid_cap) return -EINVAL; - pci_read_config_word(pdev, pos + PCI_PASID_CAP, ); + pci_read_config_word(pdev, pdev->pasid_cap + PCI_PASID_CAP, +); supported &= PCI_PASID_CAP_EXEC | PCI_PASID_CAP_PRIV; @@ -445,13 +454,12 @@ EXPORT_SYMBOL_GPL(pci_prg_resp_pasid_required); int pci_max_pasids(struct pci_dev *pdev) { u16 supported; - int pos; - pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PASID); - if (!pos) + if (!pdev->pasid_cap) return -EINVAL; - pci_read_config_word(pdev, pos + PCI_PASID_CAP, ); + pci_read_config_word(pdev, pdev->pasid_cap + PCI_PASID_CAP, +); supported = (supported & PASID_NUMBER_MASK) >> PASID_NUMBER_SHIFT; diff --git a/include/linux/pci.h b/include/linux/pci.h index 56b55db099fc..27224c0db849 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -459,6 +459,7 @@ struct pci_dev { u32 pri_reqs_alloc; /* Number of PRI requests allocated */ #endif #ifdef CONFIG_PCI_PASID + u16 pasid_cap; /* PASID Capability offset */ u16 pasid_features; #endif #ifdef CONFIG_PCI_P2PDMA -- 2.21.0
[PATCH v6 4/8] PCI/IOV: Add pci_physfn_reslock/unlock() interfaces
From: Kuppuswamy Sathyanarayanan As per PCIe spec r5.0, sec 9.3.7, in SR-IOV devices, capabilities like PASID, PRI, VC, etc are shared between PF and its associated VFs. So, to prevent race conditions between PF/VF while updating configuration registers of these shared capabilities, a new synchronization mechanism is required. As a first step, create shared resource lock and expose expose pci_physfn_reslock/unlock() API's. Users of these shared capabilities can use these lock/unlock interfaces to synchronize its access. Since the shared capability is always implemented by PF, reslock mutex has been added to pci_sriov structure which only exists for PF. NOTE: Currently this reslock is common for all shared capabilities between PF/VF. In future, if any performance impact has been noticed, we should create individual locks for each of the shared capability. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/iov.c | 1 + drivers/pci/pci.h | 40 2 files changed, 41 insertions(+) diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c index 525fd3f272b3..004e7076b065 100644 --- a/drivers/pci/iov.c +++ b/drivers/pci/iov.c @@ -507,6 +507,7 @@ static int sriov_init(struct pci_dev *dev, int pos) else iov->dev = dev; + mutex_init(>reslock); dev->sriov = iov; dev->is_physfn = 1; rc = compute_max_vf_buses(dev); diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index d22d1b807701..c7fa09f3389a 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -304,6 +304,19 @@ struct pci_sriov { u16 subsystem_device; /* VF subsystem device */ resource_size_t barsz[PCI_SRIOV_NUM_BARS]; /* VF BAR size */ booldrivers_autoprobe; /* Auto probing of VFs by driver */ + /* +* reslock mutex is used for synchronizing updates to resources +* shared between PF and all associated VFs. For example, in +* SRIOV devices, PRI and PASID interfaces are shared between +* PF an all VFs, and hence we need proper locking mechanism to +* prevent both PF and VF update the PRI or PASID configuration +* registers at the same time. +* NOTE: Currently, this lock is shared by all capabilities that +* has shared resource between PF and VFs. If there is any performance +* impact then perhaps we need to create separate lock for each of +* the independent capability that has shared resources. +*/ + struct mutexreslock;/* PF/VF shared resource lock */ }; /** @@ -433,6 +446,27 @@ void pci_iov_update_resource(struct pci_dev *dev, int resno); resource_size_t pci_sriov_resource_alignment(struct pci_dev *dev, int resno); void pci_restore_iov_state(struct pci_dev *dev); int pci_iov_bus_range(struct pci_bus *bus); +static inline void pci_physfn_reslock(struct pci_dev *dev) +{ + struct pci_dev *pf = pci_physfn(dev); + + /* For non SRIOV devices, locking is not needed */ + if (!pf->is_physfn) + return; + + mutex_lock(>sriov->reslock); +} + +static inline void pci_physfn_resunlock(struct pci_dev *dev) +{ + struct pci_dev *pf = pci_physfn(dev); + + /* For non SRIOV devices, reslock is never held */ + if (!pf->is_physfn) + return; + + mutex_unlock(>sriov->reslock); +} #else static inline int pci_iov_init(struct pci_dev *dev) @@ -453,6 +487,12 @@ static inline int pci_iov_bus_range(struct pci_bus *bus) { return 0; } +static inline void pci_physfn_reslock(struct pci_dev *dev) +{ +} +static inline void pci_physfn_resunlock(struct pci_dev *dev) +{ +} #endif /* CONFIG_PCI_IOV */ -- 2.21.0
[PATCH v6 8/8] PCI: Skip Enhanced Allocation (EA) initialization for VF device
From: Kuppuswamy Sathyanarayanan As per PCIe r4.0, sec 9.3.6, VF must not implement Enhanced Allocation Capability. So skip pci_ea_init() for virtual devices. Cc: Ashok Raj Cc: Keith Busch Suggested-by: Ashok Raj Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/pci.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 1b27b5af3d55..266600a11769 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -3025,6 +3025,13 @@ void pci_ea_init(struct pci_dev *dev) int offset; int i; + /* +* Per PCIe r4.0, sec 9.3.6, VF must not implement Enhanced +* Allocation Capability. +*/ + if (dev->is_virtfn) + return; + /* find PCI EA capability in list */ ea = pci_find_capability(dev, PCI_CAP_ID_EA); if (!ea) -- 2.21.0
[PATCH v6 7/8] PCI/ATS: Disable PF/VF ATS service independently
From: Kuppuswamy Sathyanarayanan Currently all VF's needs to be disable their ATS service before disabling the ATS service in corresponding PF device. But this logic is incorrect and does not align with the spec. Also it might lead to some power and performance impact in the system. As per PCIe spec r4.0, sec 9.3.7.8, ATS Capabilities in VFs and their associated PFs may be enabled/disabled independently. So remove this dependency logic in enable/disable code. Cc: Ashok Raj Cc: Keith Busch Suggested-by: Ashok Raj Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/ats.c | 11 --- include/linux/pci.h | 1 - 2 files changed, 12 deletions(-) diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c index ca633482e565..3a3e81630d7c 100644 --- a/drivers/pci/ats.c +++ b/drivers/pci/ats.c @@ -108,8 +108,6 @@ int pci_enable_ats(struct pci_dev *dev, int ps) pdev = pci_physfn(dev); if (pdev->ats_stu != ps) return -EINVAL; - - atomic_inc(>ats_ref_cnt); /* count enabled VFs */ } else { dev->ats_stu = ps; ctrl |= PCI_ATS_CTRL_STU(dev->ats_stu - PCI_ATS_MIN_STU); @@ -127,20 +125,11 @@ EXPORT_SYMBOL_GPL(pci_enable_ats); */ void pci_disable_ats(struct pci_dev *dev) { - struct pci_dev *pdev; u16 ctrl; if (WARN_ON(!dev->ats_enabled)) return; - if (atomic_read(>ats_ref_cnt)) - return; /* VFs still enabled */ - - if (dev->is_virtfn) { - pdev = pci_physfn(dev); - atomic_dec(>ats_ref_cnt); - } - pci_read_config_word(dev, dev->ats_cap + PCI_ATS_CTRL, ); ctrl &= ~PCI_ATS_CTRL_ENABLE; pci_write_config_word(dev, dev->ats_cap + PCI_ATS_CTRL, ctrl); diff --git a/include/linux/pci.h b/include/linux/pci.h index 735dc731e0aa..5c72590df0bf 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -452,7 +452,6 @@ struct pci_dev { }; u16 ats_cap;/* ATS Capability offset */ u8 ats_stu;/* ATS Smallest Translation Unit */ - atomic_tats_ref_cnt;/* Number of VFs with ATS enabled */ #endif #ifdef CONFIG_PCI_PRI u16 pri_cap;/* PRI Capability offset */ -- 2.21.0
[PATCH v6 2/8] PCI/ATS: Cache PRI capability check result
From: Kuppuswamy Sathyanarayanan Currently, PRI capability checks are repeated across all PRI API's. Instead, cache the capability check result in pci_pri_init() and use it in other PRI API's. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/pci/ats.c | 56 + include/linux/pci.h | 1 + 2 files changed, 32 insertions(+), 25 deletions(-) diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c index cdd936d10f68..b863562b6702 100644 --- a/drivers/pci/ats.c +++ b/drivers/pci/ats.c @@ -16,6 +16,19 @@ #include "pci.h" +static void pci_pri_init(struct pci_dev *pdev) +{ +#ifdef CONFIG_PCI_PRI + int pos; + + pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI); + if (!pos) + return; + + pdev->pri_cap = pos; +#endif +} + void pci_ats_init(struct pci_dev *dev) { int pos; @@ -28,6 +41,8 @@ void pci_ats_init(struct pci_dev *dev) return; dev->ats_cap = pos; + + pci_pri_init(dev); } /** @@ -180,26 +195,25 @@ int pci_enable_pri(struct pci_dev *pdev, u32 reqs) { u16 control, status; u32 max_requests; - int pos; if (WARN_ON(pdev->pri_enabled)) return -EBUSY; - pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI); - if (!pos) + if (!pdev->pri_cap) return -EINVAL; - pci_read_config_word(pdev, pos + PCI_PRI_STATUS, ); + pci_read_config_word(pdev, pdev->pri_cap + PCI_PRI_STATUS, ); if (!(status & PCI_PRI_STATUS_STOPPED)) return -EBUSY; - pci_read_config_dword(pdev, pos + PCI_PRI_MAX_REQ, _requests); + pci_read_config_dword(pdev, pdev->pri_cap + PCI_PRI_MAX_REQ, + _requests); reqs = min(max_requests, reqs); pdev->pri_reqs_alloc = reqs; - pci_write_config_dword(pdev, pos + PCI_PRI_ALLOC_REQ, reqs); + pci_write_config_dword(pdev, pdev->pri_cap + PCI_PRI_ALLOC_REQ, reqs); control = PCI_PRI_CTRL_ENABLE; - pci_write_config_word(pdev, pos + PCI_PRI_CTRL, control); + pci_write_config_word(pdev, pdev->pri_cap + PCI_PRI_CTRL, control); pdev->pri_enabled = 1; @@ -216,18 +230,16 @@ EXPORT_SYMBOL_GPL(pci_enable_pri); void pci_disable_pri(struct pci_dev *pdev) { u16 control; - int pos; if (WARN_ON(!pdev->pri_enabled)) return; - pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI); - if (!pos) + if (!pdev->pri_cap) return; - pci_read_config_word(pdev, pos + PCI_PRI_CTRL, ); + pci_read_config_word(pdev, pdev->pri_cap + PCI_PRI_CTRL, ); control &= ~PCI_PRI_CTRL_ENABLE; - pci_write_config_word(pdev, pos + PCI_PRI_CTRL, control); + pci_write_config_word(pdev, pdev->pri_cap + PCI_PRI_CTRL, control); pdev->pri_enabled = 0; } @@ -241,17 +253,15 @@ void pci_restore_pri_state(struct pci_dev *pdev) { u16 control = PCI_PRI_CTRL_ENABLE; u32 reqs = pdev->pri_reqs_alloc; - int pos; if (!pdev->pri_enabled) return; - pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI); - if (!pos) + if (!pdev->pri_cap) return; - pci_write_config_dword(pdev, pos + PCI_PRI_ALLOC_REQ, reqs); - pci_write_config_word(pdev, pos + PCI_PRI_CTRL, control); + pci_write_config_dword(pdev, pdev->pri_cap + PCI_PRI_ALLOC_REQ, reqs); + pci_write_config_word(pdev, pdev->pri_cap + PCI_PRI_CTRL, control); } EXPORT_SYMBOL_GPL(pci_restore_pri_state); @@ -265,17 +275,15 @@ EXPORT_SYMBOL_GPL(pci_restore_pri_state); int pci_reset_pri(struct pci_dev *pdev) { u16 control; - int pos; if (WARN_ON(pdev->pri_enabled)) return -EBUSY; - pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI); - if (!pos) + if (!pdev->pri_cap) return -EINVAL; control = PCI_PRI_CTRL_RESET; - pci_write_config_word(pdev, pos + PCI_PRI_CTRL, control); + pci_write_config_word(pdev, pdev->pri_cap + PCI_PRI_CTRL, control); return 0; } @@ -410,13 +418,11 @@ EXPORT_SYMBOL_GPL(pci_pasid_features); int pci_prg_resp_pasid_required(struct pci_dev *pdev) { u16 status; - int pos; - pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI); - if (!pos) + if (!pdev->pri_cap) return 0; - pci_read_config_word(pdev, pos + PCI_PRI_STATUS, ); + pci_read_config_word(pdev, pdev->pri_cap + PCI_PRI_STATUS, ); if (status & PCI_PRI_STATUS_PASID) return 1; diff --git a/include/linux/pci.h b/include/linux/pci.h index 9e700d9f9f28..56b55db099fc 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -455,6 +455,7 @@ struct pci_dev { atomic_tats_ref_cnt;/* Number of VFs with ATS enabled */ #endif #ifdef CONFIG_PCI_PRI +