All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v10 0/5] Simplify PCIe native ownership detection logic
@ 2020-10-26 18:56 Kuppuswamy Sathyanarayanan
  2020-10-26 18:56 ` [PATCH v10 1/5] PCI: Conditionally initialize host bridge native_* members Kuppuswamy Sathyanarayanan
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Kuppuswamy Sathyanarayanan @ 2020-10-26 18:56 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 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           | 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


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

* [PATCH v10 1/5] PCI: Conditionally initialize host bridge native_* members
  2020-10-26 18:56 [PATCH v10 0/5] Simplify PCIe native ownership detection logic Kuppuswamy Sathyanarayanan
@ 2020-10-26 18:56 ` Kuppuswamy Sathyanarayanan
  2020-10-26 18:56 ` [PATCH v10 2/5] ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set Kuppuswamy Sathyanarayanan
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Kuppuswamy Sathyanarayanan @ 2020-10-26 18:56 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 related	[flat|nested] 9+ messages in thread

* [PATCH v10 2/5] ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set.
  2020-10-26 18:56 [PATCH v10 0/5] Simplify PCIe native ownership detection logic Kuppuswamy Sathyanarayanan
  2020-10-26 18:56 ` [PATCH v10 1/5] PCI: Conditionally initialize host bridge native_* members Kuppuswamy Sathyanarayanan
@ 2020-10-26 18:56 ` Kuppuswamy Sathyanarayanan
  2020-10-27  1:01     ` kernel test robot
  2020-10-26 18:56 ` [PATCH v10 3/5] ACPI/PCI: Ignore _OSC DPC negotiation result if pcie_ports_dpc_native " Kuppuswamy Sathyanarayanan
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Kuppuswamy Sathyanarayanan @ 2020-10-26 18:56 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           | 33 +++++++++++++++++++++----------
 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, 30 insertions(+), 21 deletions(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index c12b5fb3e8fb..da925bef5bcf 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -41,6 +41,10 @@ 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) \
+	if (!(ctrl & bit)) \
+		flag = 0;
+
 static const struct acpi_device_id root_device_ids[] = {
 	{"PNP0A03", 0},
 	{"", 0},
@@ -887,6 +891,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 +917,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 related	[flat|nested] 9+ messages in thread

* [PATCH v10 3/5] ACPI/PCI: Ignore _OSC DPC negotiation result if pcie_ports_dpc_native is set.
  2020-10-26 18:56 [PATCH v10 0/5] Simplify PCIe native ownership detection logic Kuppuswamy Sathyanarayanan
  2020-10-26 18:56 ` [PATCH v10 1/5] PCI: Conditionally initialize host bridge native_* members Kuppuswamy Sathyanarayanan
  2020-10-26 18:56 ` [PATCH v10 2/5] ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set Kuppuswamy Sathyanarayanan
@ 2020-10-26 18:56 ` Kuppuswamy Sathyanarayanan
  2020-10-26 18:56 ` [PATCH v10 4/5] PCI/portdrv: Remove redundant pci_aer_available() check in DPC enable logic Kuppuswamy Sathyanarayanan
  2020-10-26 18:56 ` [PATCH v10 5/5] PCI/DPC: Move AER/DPC dependency checks out of DPC driver Kuppuswamy Sathyanarayanan
  4 siblings, 0 replies; 9+ messages in thread
From: Kuppuswamy Sathyanarayanan @ 2020-10-26 18:56 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 da925bef5bcf..b5230643e50a 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_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))
 		host_bridge->native_shpc_hotplug = 0;
 
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 related	[flat|nested] 9+ messages in thread

* [PATCH v10 4/5] PCI/portdrv: Remove redundant pci_aer_available() check in DPC enable logic
  2020-10-26 18:56 [PATCH v10 0/5] Simplify PCIe native ownership detection logic Kuppuswamy Sathyanarayanan
                   ` (2 preceding siblings ...)
  2020-10-26 18:56 ` [PATCH v10 3/5] ACPI/PCI: Ignore _OSC DPC negotiation result if pcie_ports_dpc_native " Kuppuswamy Sathyanarayanan
@ 2020-10-26 18:56 ` Kuppuswamy Sathyanarayanan
  2020-10-26 18:56 ` [PATCH v10 5/5] PCI/DPC: Move AER/DPC dependency checks out of DPC driver Kuppuswamy Sathyanarayanan
  4 siblings, 0 replies; 9+ messages in thread
From: Kuppuswamy Sathyanarayanan @ 2020-10-26 18:56 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 related	[flat|nested] 9+ messages in thread

* [PATCH v10 5/5] PCI/DPC: Move AER/DPC dependency checks out of DPC driver
  2020-10-26 18:56 [PATCH v10 0/5] Simplify PCIe native ownership detection logic Kuppuswamy Sathyanarayanan
                   ` (3 preceding siblings ...)
  2020-10-26 18:56 ` [PATCH v10 4/5] PCI/portdrv: Remove redundant pci_aer_available() check in DPC enable logic Kuppuswamy Sathyanarayanan
@ 2020-10-26 18:56 ` Kuppuswamy Sathyanarayanan
  2020-10-28  5:42   ` Ethan Zhao
  4 siblings, 1 reply; 9+ messages in thread
From: Kuppuswamy Sathyanarayanan @ 2020-10-26 18:56 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 related	[flat|nested] 9+ messages in thread

* Re: [PATCH v10 2/5] ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set.
  2020-10-26 18:56 ` [PATCH v10 2/5] ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set Kuppuswamy Sathyanarayanan
@ 2020-10-27  1:01     ` kernel test robot
  0 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2020-10-27  1:01 UTC (permalink / raw)
  To: Kuppuswamy Sathyanarayanan, bhelgaas
  Cc: kbuild-all, linux-pci, linux-kernel, ashok.raj,
	sathyanarayanan.kuppuswamy, knsathya

[-- Attachment #1: Type: text/plain, Size: 4136 bytes --]

Hi Kuppuswamy,

I love your patch! Perhaps something to improve:

[auto build test WARNING on pci/next]
[also build test WARNING on pm/linux-next linus/master linux/master v5.10-rc1 next-20201026]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Kuppuswamy-Sathyanarayanan/Simplify-PCIe-native-ownership-detection-logic/20201027-030011
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: i386-randconfig-m021-20201026 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

smatch warnings:
drivers/acpi/pci_root.c:923 acpi_pci_root_create() warn: inconsistent indenting

vim +923 drivers/acpi/pci_root.c

   884	
   885	struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
   886					     struct acpi_pci_root_ops *ops,
   887					     struct acpi_pci_root_info *info,
   888					     void *sysdata)
   889	{
   890		int ret, busnum = root->secondary.start;
   891		struct acpi_device *device = root->device;
   892		int node = acpi_get_node(device->handle);
   893		struct pci_bus *bus;
   894		struct pci_host_bridge *host_bridge;
   895		union acpi_object *obj;
   896		u32 ctrl;
   897	
   898		info->root = root;
   899		info->bridge = device;
   900		info->ops = ops;
   901		INIT_LIST_HEAD(&info->resources);
   902		snprintf(info->name, sizeof(info->name), "PCI Bus %04x:%02x",
   903			 root->segment, busnum);
   904	
   905		if (ops->init_info && ops->init_info(info))
   906			goto out_release_info;
   907		if (ops->prepare_resources)
   908			ret = ops->prepare_resources(info);
   909		else
   910			ret = acpi_pci_probe_root_resources(info);
   911		if (ret < 0)
   912			goto out_release_info;
   913	
   914		pci_acpi_root_add_resources(info);
   915		pci_add_resource(&info->resources, &root->secondary);
   916		bus = pci_create_root_bus(NULL, busnum, ops->pci_ops,
   917					  sysdata, &info->resources);
   918		if (!bus)
   919			goto out_release_info;
   920	
   921		host_bridge = to_pci_host_bridge(bus->bridge);
   922	
 > 923		  if (pcie_ports_native) {
   924			  decode_osc_control(root, "OS forcibly taking over",
   925					     OSC_PCI_EXPRESS_CONTROL_MASKS);
   926		  } else {
   927			  ctrl = root->osc_control_set;
   928			  OSC_OWNER(ctrl, OSC_PCI_EXPRESS_NATIVE_HP_CONTROL,
   929				    host_bridge->native_pcie_hotplug);
   930			  OSC_OWNER(ctrl, OSC_PCI_EXPRESS_AER_CONTROL,
   931				    host_bridge->native_aer);
   932			  OSC_OWNER(ctrl, OSC_PCI_EXPRESS_PME_CONTROL,
   933				    host_bridge->native_pme);
   934			  OSC_OWNER(ctrl, OSC_PCI_EXPRESS_LTR_CONTROL,
   935				    host_bridge->native_ltr);
   936			  OSC_OWNER(ctrl, OSC_PCI_EXPRESS_DPC_CONTROL,
   937				    host_bridge->native_dpc);
   938		  }
   939	
   940		if (!(root->osc_control_set & OSC_PCI_SHPC_NATIVE_HP_CONTROL))
   941			host_bridge->native_shpc_hotplug = 0;
   942	
   943		/*
   944		 * Evaluate the "PCI Boot Configuration" _DSM Function.  If it
   945		 * exists and returns 0, we must preserve any PCI resource
   946		 * assignments made by firmware for this host bridge.
   947		 */
   948		obj = acpi_evaluate_dsm(ACPI_HANDLE(bus->bridge), &pci_acpi_dsm_guid, 1,
   949					DSM_PCI_PRESERVE_BOOT_CONFIG, NULL);
   950		if (obj && obj->type == ACPI_TYPE_INTEGER && obj->integer.value == 0)
   951			host_bridge->preserve_config = 1;
   952		ACPI_FREE(obj);
   953	
   954		pci_scan_child_bus(bus);
   955		pci_set_host_bridge_release(host_bridge, acpi_pci_root_release_info,
   956					    info);
   957		if (node != NUMA_NO_NODE)
   958			dev_printk(KERN_DEBUG, &bus->dev, "on NUMA node %d\n", node);
   959		return bus;
   960	
   961	out_release_info:
   962		__acpi_pci_root_release_info(info);
   963		return NULL;
   964	}
   965	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 39290 bytes --]

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

* Re: [PATCH v10 2/5] ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set.
@ 2020-10-27  1:01     ` kernel test robot
  0 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2020-10-27  1:01 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 4247 bytes --]

Hi Kuppuswamy,

I love your patch! Perhaps something to improve:

[auto build test WARNING on pci/next]
[also build test WARNING on pm/linux-next linus/master linux/master v5.10-rc1 next-20201026]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Kuppuswamy-Sathyanarayanan/Simplify-PCIe-native-ownership-detection-logic/20201027-030011
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: i386-randconfig-m021-20201026 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

smatch warnings:
drivers/acpi/pci_root.c:923 acpi_pci_root_create() warn: inconsistent indenting

vim +923 drivers/acpi/pci_root.c

   884	
   885	struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
   886					     struct acpi_pci_root_ops *ops,
   887					     struct acpi_pci_root_info *info,
   888					     void *sysdata)
   889	{
   890		int ret, busnum = root->secondary.start;
   891		struct acpi_device *device = root->device;
   892		int node = acpi_get_node(device->handle);
   893		struct pci_bus *bus;
   894		struct pci_host_bridge *host_bridge;
   895		union acpi_object *obj;
   896		u32 ctrl;
   897	
   898		info->root = root;
   899		info->bridge = device;
   900		info->ops = ops;
   901		INIT_LIST_HEAD(&info->resources);
   902		snprintf(info->name, sizeof(info->name), "PCI Bus %04x:%02x",
   903			 root->segment, busnum);
   904	
   905		if (ops->init_info && ops->init_info(info))
   906			goto out_release_info;
   907		if (ops->prepare_resources)
   908			ret = ops->prepare_resources(info);
   909		else
   910			ret = acpi_pci_probe_root_resources(info);
   911		if (ret < 0)
   912			goto out_release_info;
   913	
   914		pci_acpi_root_add_resources(info);
   915		pci_add_resource(&info->resources, &root->secondary);
   916		bus = pci_create_root_bus(NULL, busnum, ops->pci_ops,
   917					  sysdata, &info->resources);
   918		if (!bus)
   919			goto out_release_info;
   920	
   921		host_bridge = to_pci_host_bridge(bus->bridge);
   922	
 > 923		  if (pcie_ports_native) {
   924			  decode_osc_control(root, "OS forcibly taking over",
   925					     OSC_PCI_EXPRESS_CONTROL_MASKS);
   926		  } else {
   927			  ctrl = root->osc_control_set;
   928			  OSC_OWNER(ctrl, OSC_PCI_EXPRESS_NATIVE_HP_CONTROL,
   929				    host_bridge->native_pcie_hotplug);
   930			  OSC_OWNER(ctrl, OSC_PCI_EXPRESS_AER_CONTROL,
   931				    host_bridge->native_aer);
   932			  OSC_OWNER(ctrl, OSC_PCI_EXPRESS_PME_CONTROL,
   933				    host_bridge->native_pme);
   934			  OSC_OWNER(ctrl, OSC_PCI_EXPRESS_LTR_CONTROL,
   935				    host_bridge->native_ltr);
   936			  OSC_OWNER(ctrl, OSC_PCI_EXPRESS_DPC_CONTROL,
   937				    host_bridge->native_dpc);
   938		  }
   939	
   940		if (!(root->osc_control_set & OSC_PCI_SHPC_NATIVE_HP_CONTROL))
   941			host_bridge->native_shpc_hotplug = 0;
   942	
   943		/*
   944		 * Evaluate the "PCI Boot Configuration" _DSM Function.  If it
   945		 * exists and returns 0, we must preserve any PCI resource
   946		 * assignments made by firmware for this host bridge.
   947		 */
   948		obj = acpi_evaluate_dsm(ACPI_HANDLE(bus->bridge), &pci_acpi_dsm_guid, 1,
   949					DSM_PCI_PRESERVE_BOOT_CONFIG, NULL);
   950		if (obj && obj->type == ACPI_TYPE_INTEGER && obj->integer.value == 0)
   951			host_bridge->preserve_config = 1;
   952		ACPI_FREE(obj);
   953	
   954		pci_scan_child_bus(bus);
   955		pci_set_host_bridge_release(host_bridge, acpi_pci_root_release_info,
   956					    info);
   957		if (node != NUMA_NO_NODE)
   958			dev_printk(KERN_DEBUG, &bus->dev, "on NUMA node %d\n", node);
   959		return bus;
   960	
   961	out_release_info:
   962		__acpi_pci_root_release_info(info);
   963		return NULL;
   964	}
   965	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 39290 bytes --]

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

* Re: [PATCH v10 5/5] PCI/DPC: Move AER/DPC dependency checks out of DPC driver
  2020-10-26 18:56 ` [PATCH v10 5/5] PCI/DPC: Move AER/DPC dependency checks out of DPC driver Kuppuswamy Sathyanarayanan
@ 2020-10-28  5:42   ` Ethan Zhao
  0 siblings, 0 replies; 9+ messages in thread
From: Ethan Zhao @ 2020-10-28  5:42 UTC (permalink / raw)
  To: Kuppuswamy Sathyanarayanan
  Cc: Bjorn Helgaas, linux-pci, Linux Kernel Mailing List, Ashok Raj, knsathya

On Tue, Oct 27, 2020 at 11:12 AM Kuppuswamy Sathyanarayanan
<sathyanarayanan.kuppuswamy@linux.intel.com> wrote:
>
> 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 &&
What hell the logic is here ?
>             (host->native_dpc || (services & PCIE_PORT_SERVICE_AER)))
>                 services |= PCIE_PORT_SERVICE_DPC;
>
> --
> 2.17.1
>

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

end of thread, other threads:[~2020-10-28 23:10 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-26 18:56 [PATCH v10 0/5] Simplify PCIe native ownership detection logic Kuppuswamy Sathyanarayanan
2020-10-26 18:56 ` [PATCH v10 1/5] PCI: Conditionally initialize host bridge native_* members Kuppuswamy Sathyanarayanan
2020-10-26 18:56 ` [PATCH v10 2/5] ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set Kuppuswamy Sathyanarayanan
2020-10-27  1:01   ` kernel test robot
2020-10-27  1:01     ` kernel test robot
2020-10-26 18:56 ` [PATCH v10 3/5] ACPI/PCI: Ignore _OSC DPC negotiation result if pcie_ports_dpc_native " Kuppuswamy Sathyanarayanan
2020-10-26 18:56 ` [PATCH v10 4/5] PCI/portdrv: Remove redundant pci_aer_available() check in DPC enable logic Kuppuswamy Sathyanarayanan
2020-10-26 18:56 ` [PATCH v10 5/5] PCI/DPC: Move AER/DPC dependency checks out of DPC driver Kuppuswamy Sathyanarayanan
2020-10-28  5:42   ` Ethan Zhao

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.