Linux-PCI Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v11 0/5] Simplify PCIe native ownership detection logic
@ 2020-10-27  2:57 Kuppuswamy Sathyanarayanan
  2020-10-27  2:57 ` [PATCH v11 1/5] PCI: Conditionally initialize host bridge native_* members Kuppuswamy Sathyanarayanan
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Kuppuswamy Sathyanarayanan @ 2020-10-27  2:57 UTC (permalink / raw)
  To: bhelgaas
  Cc: linux-pci, linux-kernel, ashok.raj, sathyanarayanan.kuppuswamy, knsathya

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 v10:
 * Addressed format issue reported by lkp test.

Changes since v9:
 * Rebased on top of v5.10-rc1

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           | 39 +++++++++++++++++++++++--------
 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, 44 insertions(+), 30 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v11 1/5] PCI: Conditionally initialize host bridge native_* members
  2020-10-27  2:57 [PATCH v11 0/5] Simplify PCIe native ownership detection logic Kuppuswamy Sathyanarayanan
@ 2020-10-27  2:57 ` Kuppuswamy Sathyanarayanan
  2020-10-28  6:09   ` Ethan Zhao
  2020-10-27  2:57 ` [PATCH v11 2/5] ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set Kuppuswamy Sathyanarayanan
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Kuppuswamy Sathyanarayanan @ 2020-10-27  2:57 UTC (permalink / raw)
  To: bhelgaas
  Cc: linux-pci, linux-kernel, ashok.raj, sathyanarayanan.kuppuswamy, knsathya

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 <sathyanarayanan.kuppuswamy@linux.intel.com>
---
 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 4289030b0fff..756fa60ca708 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(&bridge->dev);
 }
-- 
2.17.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v11 2/5] ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set.
  2020-10-27  2:57 [PATCH v11 0/5] Simplify PCIe native ownership detection logic Kuppuswamy Sathyanarayanan
  2020-10-27  2:57 ` [PATCH v11 1/5] PCI: Conditionally initialize host bridge native_* members Kuppuswamy Sathyanarayanan
@ 2020-10-27  2:57 ` Kuppuswamy Sathyanarayanan
  2020-10-27  2:57 ` [PATCH v11 3/5] ACPI/PCI: Ignore _OSC DPC negotiation result if pcie_ports_dpc_native " Kuppuswamy Sathyanarayanan
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Kuppuswamy Sathyanarayanan @ 2020-10-27  2:57 UTC (permalink / raw)
  To: bhelgaas
  Cc: linux-pci, linux-kernel, ashok.raj, sathyanarayanan.kuppuswamy, knsathya

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 <sathyanarayanan.kuppuswamy@linux.intel.com>
---
 drivers/acpi/pci_root.c           | 35 ++++++++++++++++++++++---------
 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 +++-----
 include/linux/acpi.h              |  2 ++
 6 files changed, 32 insertions(+), 21 deletions(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index c12b5fb3e8fb..a9e6b782622d 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -41,6 +41,12 @@ static int acpi_pci_root_scan_dependent(struct acpi_device *adev)
 				| OSC_PCI_CLOCK_PM_SUPPORT \
 				| OSC_PCI_MSI_SUPPORT)
 
+#define OSC_OWNER(ctrl, bit, flag) \
+	do { \
+		if (!(ctrl & bit)) \
+			flag = 0;  \
+	} while (0)
+
 static const struct acpi_device_id root_device_ids[] = {
 	{"PNP0A03", 0},
 	{"", 0},
@@ -887,6 +893,7 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
 	struct pci_bus *bus;
 	struct pci_host_bridge *host_bridge;
 	union acpi_object *obj;
+	u32 ctrl;
 
 	info->root = root;
 	info->bridge = device;
@@ -912,18 +919,26 @@ 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 (pcie_ports_native) {
+		decode_osc_control(root, "OS forcibly taking over",
+				   OSC_PCI_EXPRESS_CONTROL_MASKS);
+	} else {
+		ctrl = root->osc_control_set;
+		OSC_OWNER(ctrl, OSC_PCI_EXPRESS_NATIVE_HP_CONTROL,
+			  host_bridge->native_pcie_hotplug);
+		OSC_OWNER(ctrl, OSC_PCI_EXPRESS_AER_CONTROL,
+			  host_bridge->native_aer);
+		OSC_OWNER(ctrl, OSC_PCI_EXPRESS_PME_CONTROL,
+			  host_bridge->native_pme);
+		OSC_OWNER(ctrl, OSC_PCI_EXPRESS_LTR_CONTROL,
+			  host_bridge->native_ltr);
+		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;
-	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 ad3393930ecb..d1831e6bf60a 100644
--- a/drivers/pci/hotplug/pciehp_core.c
+++ b/drivers/pci/hotplug/pciehp_core.c
@@ -256,7 +256,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 bf03648c2072..a84f75ec6df8 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 65dff5f3457a..79bb441139c2 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 CONFIG_PCIEAER
-	if (dev->aer_cap && pci_aer_available() &&
-	    (pcie_ports_native || host->native_aer)) {
+	if (dev->aer_cap && pci_aer_available() && host->native_aer) {
 		services |= PCIE_PORT_SERVICE_AER;
 
 		/*
@@ -238,8 +236,7 @@ static int get_port_device_capability(struct pci_dev *dev)
 	 * Event Collectors can also generate PMEs, but we don't handle
 	 * those yet.
 	 */
-	if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT &&
-	    (pcie_ports_native || host->native_pme)) {
+	if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT && host->native_pme) {
 		services |= PCIE_PORT_SERVICE_PME;
 
 		/*
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 39263c6b52e1..35689f4e8e1f 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -569,6 +569,8 @@ extern bool osc_pc_lpi_support_confirmed;
 #define OSC_PCI_EXPRESS_LTR_CONTROL		0x00000020
 #define OSC_PCI_EXPRESS_DPC_CONTROL		0x00000080
 #define OSC_PCI_CONTROL_MASKS			0x000000bf
+/* Masks specific to PCIe Capabilities */
+#define OSC_PCI_EXPRESS_CONTROL_MASKS		0x000000bd
 
 #define ACPI_GSB_ACCESS_ATTRIB_QUICK		0x00000002
 #define ACPI_GSB_ACCESS_ATTRIB_SEND_RCV         0x00000004
-- 
2.17.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v11 3/5] ACPI/PCI: Ignore _OSC DPC negotiation result if pcie_ports_dpc_native is set.
  2020-10-27  2:57 [PATCH v11 0/5] Simplify PCIe native ownership detection logic Kuppuswamy Sathyanarayanan
  2020-10-27  2:57 ` [PATCH v11 1/5] PCI: Conditionally initialize host bridge native_* members Kuppuswamy Sathyanarayanan
  2020-10-27  2:57 ` [PATCH v11 2/5] ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set Kuppuswamy Sathyanarayanan
@ 2020-10-27  2:57 ` Kuppuswamy Sathyanarayanan
  2020-10-27  2:57 ` [PATCH v11 4/5] PCI/portdrv: Remove redundant pci_aer_available() check in DPC enable logic Kuppuswamy Sathyanarayanan
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Kuppuswamy Sathyanarayanan @ 2020-10-27  2:57 UTC (permalink / raw)
  To: bhelgaas
  Cc: linux-pci, linux-kernel, ashok.raj, sathyanarayanan.kuppuswamy, knsathya

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 <sathyanarayanan.kuppuswamy@linux.intel.com>
---
 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 a9e6b782622d..2e2bc80c42fe 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -933,8 +933,12 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
 			  host_bridge->native_pme);
 		OSC_OWNER(ctrl, OSC_PCI_EXPRESS_LTR_CONTROL,
 			  host_bridge->native_ltr);
-		OSC_OWNER(ctrl, OSC_PCI_EXPRESS_DPC_CONTROL,
-			  host_bridge->native_dpc);
+		if (pcie_ports_dpc_native)
+			dev_warn(&bus->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))
diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
index e05aba86a317..21f77420632b 100644
--- a/drivers/pci/pcie/dpc.c
+++ b/drivers/pci/pcie/dpc.c
@@ -283,11 +283,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 = &dev->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 22207a79762c..388121ec88b5 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1559,9 +1559,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_disabled	true
 #define pcie_ports_native	false
+#define pcie_ports_dpc_native	false
 #endif
 
 #define PCIE_LINK_STATE_L0S		BIT(0)
-- 
2.17.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v11 4/5] PCI/portdrv: Remove redundant pci_aer_available() check in DPC enable logic
  2020-10-27  2:57 [PATCH v11 0/5] Simplify PCIe native ownership detection logic Kuppuswamy Sathyanarayanan
                   ` (2 preceding siblings ...)
  2020-10-27  2:57 ` [PATCH v11 3/5] ACPI/PCI: Ignore _OSC DPC negotiation result if pcie_ports_dpc_native " Kuppuswamy Sathyanarayanan
@ 2020-10-27  2:57 ` Kuppuswamy Sathyanarayanan
  2020-10-28  6:00   ` Ethan Zhao
  2020-10-27  2:57 ` [PATCH v11 5/5] PCI/DPC: Move AER/DPC dependency checks out of DPC driver Kuppuswamy Sathyanarayanan
  2020-11-13 21:59 ` [PATCH v11 0/5] Simplify PCIe native ownership detection logic Kuppuswamy, Sathyanarayanan
  5 siblings, 1 reply; 12+ messages in thread
From: Kuppuswamy Sathyanarayanan @ 2020-10-27  2:57 UTC (permalink / raw)
  To: bhelgaas
  Cc: linux-pci, linux-kernel, ashok.raj, sathyanarayanan.kuppuswamy, knsathya

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 <sathyanarayanan.kuppuswamy@linux.intel.com>
---
 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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v11 5/5] PCI/DPC: Move AER/DPC dependency checks out of DPC driver
  2020-10-27  2:57 [PATCH v11 0/5] Simplify PCIe native ownership detection logic Kuppuswamy Sathyanarayanan
                   ` (3 preceding siblings ...)
  2020-10-27  2:57 ` [PATCH v11 4/5] PCI/portdrv: Remove redundant pci_aer_available() check in DPC enable logic Kuppuswamy Sathyanarayanan
@ 2020-10-27  2:57 ` Kuppuswamy Sathyanarayanan
  2020-11-13 21:59 ` [PATCH v11 0/5] Simplify PCIe native ownership detection logic Kuppuswamy, Sathyanarayanan
  5 siblings, 0 replies; 12+ messages in thread
From: Kuppuswamy Sathyanarayanan @ 2020-10-27  2:57 UTC (permalink / raw)
  To: bhelgaas
  Cc: linux-pci, linux-kernel, ashok.raj, sathyanarayanan.kuppuswamy, knsathya

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 <sathyanarayanan.kuppuswamy@linux.intel.com>
---
 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 21f77420632b..a8b922044447 100644
--- a/drivers/pci/pcie/dpc.c
+++ b/drivers/pci/pcie/dpc.c
@@ -283,14 +283,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 = &dev->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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v11 4/5] PCI/portdrv: Remove redundant pci_aer_available() check in DPC enable logic
  2020-10-27  2:57 ` [PATCH v11 4/5] PCI/portdrv: Remove redundant pci_aer_available() check in DPC enable logic Kuppuswamy Sathyanarayanan
@ 2020-10-28  6:00   ` Ethan Zhao
  2020-10-28 17:13     ` Kuppuswamy, Sathyanarayanan
  0 siblings, 1 reply; 12+ messages in thread
From: Ethan Zhao @ 2020-10-28  6:00 UTC (permalink / raw)
  To: Kuppuswamy Sathyanarayanan
  Cc: Bjorn Helgaas, linux-pci, Linux Kernel Mailing List, Ashok Raj, knsathya

On Tue, Oct 27, 2020 at 10:00 PM Kuppuswamy Sathyanarayanan
<sathyanarayanan.kuppuswamy@linux.intel.com> wrote:
>
> In DPC service enable logic, check for
> services & PCIE_PORT_SERVICE_AER implies pci_aer_available()
How about PCIE_PORT_SERVICE_AER is not configured, but
pcie_aer_disable == 0 ?
> 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 <sathyanarayanan.kuppuswamy@linux.intel.com>
> ---
>  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
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v11 1/5] PCI: Conditionally initialize host bridge native_* members
  2020-10-27  2:57 ` [PATCH v11 1/5] PCI: Conditionally initialize host bridge native_* members Kuppuswamy Sathyanarayanan
@ 2020-10-28  6:09   ` Ethan Zhao
  2020-10-28 17:14     ` Kuppuswamy, Sathyanarayanan
  0 siblings, 1 reply; 12+ messages in thread
From: Ethan Zhao @ 2020-10-28  6:09 UTC (permalink / raw)
  To: Kuppuswamy Sathyanarayanan
  Cc: Bjorn Helgaas, linux-pci, Linux Kernel Mailing List, Ashok Raj, knsathya

On Tue, Oct 27, 2020 at 10:00 PM Kuppuswamy Sathyanarayanan
<sathyanarayanan.kuppuswamy@linux.intel.com> wrote:
>
> 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 <sathyanarayanan.kuppuswamy@linux.intel.com>
> ---
>  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 4289030b0fff..756fa60ca708 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
  If CONFIG_PCIEPORTBUS wasn't defined, leave them to "unknown" value ?

> +       bridge->native_ltr = 1;
> +       bridge->native_shpc_hotplug = 1;
>
>         device_initialize(&bridge->dev);
>  }
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v11 4/5] PCI/portdrv: Remove redundant pci_aer_available() check in DPC enable logic
  2020-10-28  6:00   ` Ethan Zhao
@ 2020-10-28 17:13     ` Kuppuswamy, Sathyanarayanan
  2020-11-02  9:59       ` Ethan Zhao
  0 siblings, 1 reply; 12+ messages in thread
From: Kuppuswamy, Sathyanarayanan @ 2020-10-28 17:13 UTC (permalink / raw)
  To: Ethan Zhao
  Cc: Bjorn Helgaas, linux-pci, Linux Kernel Mailing List, Ashok Raj, knsathya



On 10/27/20 11:00 PM, Ethan Zhao wrote:
> On Tue, Oct 27, 2020 at 10:00 PM Kuppuswamy Sathyanarayanan
> <sathyanarayanan.kuppuswamy@linux.intel.com> wrote:
>>
>> In DPC service enable logic, check for
>> services & PCIE_PORT_SERVICE_AER implies pci_aer_available()
> How about PCIE_PORT_SERVICE_AER is not configured, but
> pcie_aer_disable == 0 ?
Its not possible in current code flow. DPC service is configured
following AER service configuration.
>> 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 <sathyanarayanan.kuppuswamy@linux.intel.com>
>> ---
>>   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
>>

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v11 1/5] PCI: Conditionally initialize host bridge native_* members
  2020-10-28  6:09   ` Ethan Zhao
@ 2020-10-28 17:14     ` Kuppuswamy, Sathyanarayanan
  0 siblings, 0 replies; 12+ messages in thread
From: Kuppuswamy, Sathyanarayanan @ 2020-10-28 17:14 UTC (permalink / raw)
  To: Ethan Zhao
  Cc: Bjorn Helgaas, linux-pci, Linux Kernel Mailing List, Ashok Raj, knsathya



On 10/27/20 11:09 PM, Ethan Zhao wrote:
> On Tue, Oct 27, 2020 at 10:00 PM Kuppuswamy Sathyanarayanan
> <sathyanarayanan.kuppuswamy@linux.intel.com> wrote:
>>
>> 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 <sathyanarayanan.kuppuswamy@linux.intel.com>
>> ---
>>   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 4289030b0fff..756fa60ca708 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
>    If CONFIG_PCIEPORTBUS wasn't defined, leave them to "unknown" value ?
By default all of them are 0.
> 
>> +       bridge->native_ltr = 1;
>> +       bridge->native_shpc_hotplug = 1;
>>
>>          device_initialize(&bridge->dev);
>>   }
>> --
>> 2.17.1
>>

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v11 4/5] PCI/portdrv: Remove redundant pci_aer_available() check in DPC enable logic
  2020-10-28 17:13     ` Kuppuswamy, Sathyanarayanan
@ 2020-11-02  9:59       ` Ethan Zhao
  0 siblings, 0 replies; 12+ messages in thread
From: Ethan Zhao @ 2020-11-02  9:59 UTC (permalink / raw)
  To: Kuppuswamy, Sathyanarayanan
  Cc: Bjorn Helgaas, linux-pci, Linux Kernel Mailing List, Ashok Raj, knsathya

The current logic is
if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) &&
    pci_aer_available() &&
    (pcie_ports_dpc_native || (services & PCIE_PORT_SERVICE_AER)))
services |= PCIE_PORT_SERVICE_DPC;


if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) &&
    pci_aer_available() &&
    (pcie_ports_dpc_native))
 services |= PCIE_PORT_SERVICE_DPC;

or

if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) &&
    pci_aer_available() &&(services & PCIE_PORT_SERVICE_AER))
  services |= PCIE_PORT_SERVICE_DPC;

do you mean one of the possible is
if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) &&
    (pcie_ports_dpc_native))
 services |= PCIE_PORT_SERVICE_DPC;

after your patch ? nothing about AER ?

Thanks,
Ethan

On Thu, Oct 29, 2020 at 1:14 AM Kuppuswamy, Sathyanarayanan
<sathyanarayanan.kuppuswamy@linux.intel.com> wrote:
>
>
>
> On 10/27/20 11:00 PM, Ethan Zhao wrote:
> > On Tue, Oct 27, 2020 at 10:00 PM Kuppuswamy Sathyanarayanan
> > <sathyanarayanan.kuppuswamy@linux.intel.com> wrote:
> >>
> >> In DPC service enable logic, check for
> >> services & PCIE_PORT_SERVICE_AER implies pci_aer_available()
> > How about PCIE_PORT_SERVICE_AER is not configured, but
> > pcie_aer_disable == 0 ?
> Its not possible in current code flow. DPC service is configured
> following AER service configuration.
> >> 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 <sathyanarayanan.kuppuswamy@linux.intel.com>
> >> ---
> >>   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
> >>
>
> --
> Sathyanarayanan Kuppuswamy
> Linux Kernel Developer

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v11 0/5] Simplify PCIe native ownership detection logic
  2020-10-27  2:57 [PATCH v11 0/5] Simplify PCIe native ownership detection logic Kuppuswamy Sathyanarayanan
                   ` (4 preceding siblings ...)
  2020-10-27  2:57 ` [PATCH v11 5/5] PCI/DPC: Move AER/DPC dependency checks out of DPC driver Kuppuswamy Sathyanarayanan
@ 2020-11-13 21:59 ` Kuppuswamy, Sathyanarayanan
  5 siblings, 0 replies; 12+ messages in thread
From: Kuppuswamy, Sathyanarayanan @ 2020-11-13 21:59 UTC (permalink / raw)
  To: bhelgaas; +Cc: linux-pci, linux-kernel, ashok.raj, knsathya

Hi Bjorn,

On 10/26/20 7:57 PM, Kuppuswamy Sathyanarayanan wrote:
> 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.
Any comments on this series?
> 
> Changes since v10:
>   * Addressed format issue reported by lkp test.
> 
> Changes since v9:
>   * Rebased on top of v5.10-rc1
> 
> 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           | 39 +++++++++++++++++++++++--------
>   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, 44 insertions(+), 30 deletions(-)
> 

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, back to index

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-27  2:57 [PATCH v11 0/5] Simplify PCIe native ownership detection logic Kuppuswamy Sathyanarayanan
2020-10-27  2:57 ` [PATCH v11 1/5] PCI: Conditionally initialize host bridge native_* members Kuppuswamy Sathyanarayanan
2020-10-28  6:09   ` Ethan Zhao
2020-10-28 17:14     ` Kuppuswamy, Sathyanarayanan
2020-10-27  2:57 ` [PATCH v11 2/5] ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set Kuppuswamy Sathyanarayanan
2020-10-27  2:57 ` [PATCH v11 3/5] ACPI/PCI: Ignore _OSC DPC negotiation result if pcie_ports_dpc_native " Kuppuswamy Sathyanarayanan
2020-10-27  2:57 ` [PATCH v11 4/5] PCI/portdrv: Remove redundant pci_aer_available() check in DPC enable logic Kuppuswamy Sathyanarayanan
2020-10-28  6:00   ` Ethan Zhao
2020-10-28 17:13     ` Kuppuswamy, Sathyanarayanan
2020-11-02  9:59       ` Ethan Zhao
2020-10-27  2:57 ` [PATCH v11 5/5] PCI/DPC: Move AER/DPC dependency checks out of DPC driver Kuppuswamy Sathyanarayanan
2020-11-13 21:59 ` [PATCH v11 0/5] Simplify PCIe native ownership detection logic Kuppuswamy, Sathyanarayanan

Linux-PCI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-pci/0 linux-pci/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-pci linux-pci/ https://lore.kernel.org/linux-pci \
		linux-pci@vger.kernel.org
	public-inbox-index linux-pci

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-pci


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git