linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/5] Remove AER HEST table parser
@ 2020-05-26 23:18 sathyanarayanan.kuppuswamy
  2020-05-26 23:18 ` [PATCH v4 1/5] PCI/AER: Remove redundant pci_is_pcie() checks sathyanarayanan.kuppuswamy
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: sathyanarayanan.kuppuswamy @ 2020-05-26 23:18 UTC (permalink / raw)
  To: bhelgaas; +Cc: linux-pci, linux-kernel, ashok.raj, sathyanarayanan.kuppuswamy

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

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


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

* [PATCH v4 1/5] PCI/AER: Remove redundant pci_is_pcie() checks.
  2020-05-26 23:18 [PATCH v4 0/5] Remove AER HEST table parser sathyanarayanan.kuppuswamy
@ 2020-05-26 23:18 ` sathyanarayanan.kuppuswamy
  2020-05-26 23:18 ` [PATCH v4 2/5] PCI/AER: Remove redundant dev->aer_cap checks sathyanarayanan.kuppuswamy
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: sathyanarayanan.kuppuswamy @ 2020-05-26 23:18 UTC (permalink / raw)
  To: bhelgaas; +Cc: linux-pci, linux-kernel, ashok.raj, sathyanarayanan.kuppuswamy

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

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


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

* [PATCH v4 2/5] PCI/AER: Remove redundant dev->aer_cap checks.
  2020-05-26 23:18 [PATCH v4 0/5] Remove AER HEST table parser sathyanarayanan.kuppuswamy
  2020-05-26 23:18 ` [PATCH v4 1/5] PCI/AER: Remove redundant pci_is_pcie() checks sathyanarayanan.kuppuswamy
@ 2020-05-26 23:18 ` sathyanarayanan.kuppuswamy
  2020-05-26 23:18 ` [PATCH v4 3/5] ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set sathyanarayanan.kuppuswamy
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: sathyanarayanan.kuppuswamy @ 2020-05-26 23:18 UTC (permalink / raw)
  To: bhelgaas; +Cc: linux-pci, linux-kernel, ashok.raj, sathyanarayanan.kuppuswamy

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

pcie_aer_get_firmware_first() includes check for dev->aer_cap.
So remove redundant dev->aer_cap checks.

Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
 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


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

* [PATCH v4 3/5] ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set.
  2020-05-26 23:18 [PATCH v4 0/5] Remove AER HEST table parser sathyanarayanan.kuppuswamy
  2020-05-26 23:18 ` [PATCH v4 1/5] PCI/AER: Remove redundant pci_is_pcie() checks sathyanarayanan.kuppuswamy
  2020-05-26 23:18 ` [PATCH v4 2/5] PCI/AER: Remove redundant dev->aer_cap checks sathyanarayanan.kuppuswamy
@ 2020-05-26 23:18 ` sathyanarayanan.kuppuswamy
  2020-05-29 23:12   ` Bjorn Helgaas
  2020-05-26 23:18 ` [PATCH v4 4/5] ACPI/PCI: Ignore _OSC DPC negotiation result if pcie_ports_dpc_native " sathyanarayanan.kuppuswamy
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 9+ messages in thread
From: sathyanarayanan.kuppuswamy @ 2020-05-26 23:18 UTC (permalink / raw)
  To: bhelgaas; +Cc: linux-pci, linux-kernel, ashok.raj, sathyanarayanan.kuppuswamy

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

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


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

* [PATCH v4 4/5] ACPI/PCI: Ignore _OSC DPC negotiation result if pcie_ports_dpc_native is set.
  2020-05-26 23:18 [PATCH v4 0/5] Remove AER HEST table parser sathyanarayanan.kuppuswamy
                   ` (2 preceding siblings ...)
  2020-05-26 23:18 ` [PATCH v4 3/5] ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set sathyanarayanan.kuppuswamy
@ 2020-05-26 23:18 ` sathyanarayanan.kuppuswamy
  2020-05-26 23:18 ` [PATCH v4 5/5] PCI/AER: Replace pcie_aer_get_firmware_first() with pcie_aer_is_native() sathyanarayanan.kuppuswamy
  2020-05-29 23:09 ` [PATCH v4 0/5] Remove AER HEST table parser Bjorn Helgaas
  5 siblings, 0 replies; 9+ messages in thread
From: sathyanarayanan.kuppuswamy @ 2020-05-26 23:18 UTC (permalink / raw)
  To: bhelgaas; +Cc: linux-pci, linux-kernel, ashok.raj, sathyanarayanan.kuppuswamy

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

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    | 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_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 v4 5/5] PCI/AER: Replace pcie_aer_get_firmware_first() with pcie_aer_is_native()
  2020-05-26 23:18 [PATCH v4 0/5] Remove AER HEST table parser sathyanarayanan.kuppuswamy
                   ` (3 preceding siblings ...)
  2020-05-26 23:18 ` [PATCH v4 4/5] ACPI/PCI: Ignore _OSC DPC negotiation result if pcie_ports_dpc_native " sathyanarayanan.kuppuswamy
@ 2020-05-26 23:18 ` sathyanarayanan.kuppuswamy
  2020-05-29 23:09 ` [PATCH v4 0/5] Remove AER HEST table parser Bjorn Helgaas
  5 siblings, 0 replies; 9+ messages in thread
From: sathyanarayanan.kuppuswamy @ 2020-05-26 23:18 UTC (permalink / raw)
  To: bhelgaas; +Cc: linux-pci, linux-kernel, ashok.raj, sathyanarayanan.kuppuswamy

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

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 <sathyanarayanan.kuppuswamy@linux.intel.com>
---
 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;
-}
+#define	PCI_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, &info);
+	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
-
-#define	PCI_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 pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_AER_FLAGS);
@@ -328,7 +235,7 @@ EXPORT_SYMBOL_GPL(pci_enable_pcie_error_reporting);
 
 int pci_disable_pcie_error_reporting(struct pci_dev *dev)
 {
-	if (pcie_aer_get_firmware_first(dev))
+	if (!pcie_aer_is_native(dev))
 		return -EIO;
 
 	return pcie_capability_clear_word(dev, PCI_EXP_DEVCTL,
@@ -349,7 +256,7 @@ int pci_aer_clear_nonfatal_status(struct pci_dev *dev)
 	int pos = dev->aer_cap;
 	u32 status, sev;
 
-	if (pcie_aer_get_firmware_first(dev))
+	if (!pcie_aer_is_native(dev))
 		return -EIO;
 
 	/* Clear status bits for ERR_NONFATAL errors only */
@@ -368,7 +275,7 @@ void pci_aer_clear_fatal_status(struct pci_dev *dev)
 	int pos = dev->aer_cap;
 	u32 status, sev;
 
-	if (pcie_aer_get_firmware_first(dev))
+	if (!pcie_aer_is_native(dev))
 		return;
 
 	/* Clear status bits for ERR_FATAL errors only */
@@ -415,7 +322,7 @@ int pci_aer_raw_clear_status(struct pci_dev *dev)
 
 int pci_aer_clear_status(struct pci_dev *dev)
 {
-	if (pcie_aer_get_firmware_first(dev))
+	if (!pcie_aer_is_native(dev))
 		return -EIO;
 
 	return pci_aer_raw_clear_status(dev);
diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
index 762170423fdd..0993d51abf03 100644
--- a/drivers/pci/pcie/dpc.c
+++ b/drivers/pci/pcie/dpc.c
@@ -284,7 +284,7 @@ static int dpc_probe(struct pcie_device *dev)
 	int status;
 	u16 ctl, cap;
 
-	if (pcie_aer_get_firmware_first(pdev) && !pcie_ports_dpc_native)
+	if (!pcie_aer_is_native(pdev) && !pcie_ports_dpc_native)
 		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 e4999f24ad92..0ac20feef24e 100644
--- a/drivers/pci/pcie/portdrv.h
+++ b/drivers/pci/pcie/portdrv.h
@@ -27,8 +27,10 @@
 
 #ifdef CONFIG_PCIEAER
 int pcie_aer_init(void);
+int pcie_aer_is_native(struct pci_dev *dev);
 #else
 static inline int pcie_aer_init(void) { return 0; }
+static inline int pcie_aer_is_native(struct pci_dev *dev) { return 0; }
 #endif
 
 #ifdef CONFIG_HOTPLUG_PCI_PCIE
@@ -145,16 +147,5 @@ static inline bool pcie_pme_no_msi(void) { return false; }
 static inline void pcie_pme_interrupt_enable(struct pci_dev *dev, bool en) {}
 #endif /* !CONFIG_PCIE_PME */
 
-#ifdef CONFIG_ACPI_APEI
-int pcie_aer_get_firmware_first(struct pci_dev *pci_dev);
-#else
-static inline int pcie_aer_get_firmware_first(struct pci_dev *pci_dev)
-{
-	if (pci_dev->__aer_firmware_first_valid)
-		return pci_dev->__aer_firmware_first;
-	return 0;
-}
-#endif
-
 struct device *pcie_port_find_device(struct pci_dev *dev, u32 service);
 #endif /* _PORTDRV_H_ */
-- 
2.17.1


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

* Re: [PATCH v4 0/5] Remove AER HEST table parser
  2020-05-26 23:18 [PATCH v4 0/5] Remove AER HEST table parser sathyanarayanan.kuppuswamy
                   ` (4 preceding siblings ...)
  2020-05-26 23:18 ` [PATCH v4 5/5] PCI/AER: Replace pcie_aer_get_firmware_first() with pcie_aer_is_native() sathyanarayanan.kuppuswamy
@ 2020-05-29 23:09 ` Bjorn Helgaas
  2020-05-29 23:15   ` Kuppuswamy, Sathyanarayanan
  5 siblings, 1 reply; 9+ messages in thread
From: Bjorn Helgaas @ 2020-05-29 23:09 UTC (permalink / raw)
  To: sathyanarayanan.kuppuswamy; +Cc: bhelgaas, linux-pci, linux-kernel, ashok.raj

On Tue, May 26, 2020 at 04:18:24PM -0700, sathyanarayanan.kuppuswamy@linux.intel.com wrote:
> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> 
> 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()

I reordered these a bit and applied them as follows for v5.8:

  ("PCI/AER: Remove HEST/FIRMWARE_FIRST parsing for AER ownership")
  ("PCI/AER: Remove redundant dev->aer_cap checks")
  ("PCI/AER: Remove redundant pci_is_pcie() checks")

I added the trivial patch below on top.

>  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(-)

commit 643a9f8854f9 ("PCI/AER: Use "aer" variable for capability offset")
Author: Bjorn Helgaas <bhelgaas@google.com>
Date:   Fri May 29 17:56:09 2020 -0500

    PCI/AER: Use "aer" variable for capability offset
    
    Previously we used "pos" or "aer_pos" for the offset of the AER Capability.
    Use "aer" consistently and initialize it the same way everywhere.  No
    functional change intended.
    
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 61e8cb23e98b..3acf56683915 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -136,19 +136,18 @@ static const char * const ecrc_policy_str[] = {
  */
 static int enable_ecrc_checking(struct pci_dev *dev)
 {
-	int pos;
+	int aer = dev->aer_cap;
 	u32 reg32;
 
-	pos = dev->aer_cap;
-	if (!pos)
+	if (!aer)
 		return -ENODEV;
 
-	pci_read_config_dword(dev, pos + PCI_ERR_CAP, &reg32);
+	pci_read_config_dword(dev, aer + PCI_ERR_CAP, &reg32);
 	if (reg32 & PCI_ERR_CAP_ECRC_GENC)
 		reg32 |= PCI_ERR_CAP_ECRC_GENE;
 	if (reg32 & PCI_ERR_CAP_ECRC_CHKC)
 		reg32 |= PCI_ERR_CAP_ECRC_CHKE;
-	pci_write_config_dword(dev, pos + PCI_ERR_CAP, reg32);
+	pci_write_config_dword(dev, aer + PCI_ERR_CAP, reg32);
 
 	return 0;
 }
@@ -161,16 +160,15 @@ static int enable_ecrc_checking(struct pci_dev *dev)
  */
 static int disable_ecrc_checking(struct pci_dev *dev)
 {
-	int pos;
+	int aer = dev->aer_cap;
 	u32 reg32;
 
-	pos = dev->aer_cap;
-	if (!pos)
+	if (!aer)
 		return -ENODEV;
 
-	pci_read_config_dword(dev, pos + PCI_ERR_CAP, &reg32);
+	pci_read_config_dword(dev, aer + PCI_ERR_CAP, &reg32);
 	reg32 &= ~(PCI_ERR_CAP_ECRC_GENE | PCI_ERR_CAP_ECRC_CHKE);
-	pci_write_config_dword(dev, pos + PCI_ERR_CAP, reg32);
+	pci_write_config_dword(dev, aer + PCI_ERR_CAP, reg32);
 
 	return 0;
 }
@@ -253,18 +251,18 @@ void pci_aer_clear_device_status(struct pci_dev *dev)
 
 int pci_aer_clear_nonfatal_status(struct pci_dev *dev)
 {
-	int pos = dev->aer_cap;
+	int aer = dev->aer_cap;
 	u32 status, sev;
 
 	if (!pcie_aer_is_native(dev))
 		return -EIO;
 
 	/* Clear status bits for ERR_NONFATAL errors only */
-	pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_STATUS, &status);
-	pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_SEVER, &sev);
+	pci_read_config_dword(dev, aer + PCI_ERR_UNCOR_STATUS, &status);
+	pci_read_config_dword(dev, aer + PCI_ERR_UNCOR_SEVER, &sev);
 	status &= ~sev;
 	if (status)
-		pci_write_config_dword(dev, pos + PCI_ERR_UNCOR_STATUS, status);
+		pci_write_config_dword(dev, aer + PCI_ERR_UNCOR_STATUS, status);
 
 	return 0;
 }
@@ -272,18 +270,18 @@ EXPORT_SYMBOL_GPL(pci_aer_clear_nonfatal_status);
 
 void pci_aer_clear_fatal_status(struct pci_dev *dev)
 {
-	int pos = dev->aer_cap;
+	int aer = dev->aer_cap;
 	u32 status, sev;
 
 	if (!pcie_aer_is_native(dev))
 		return;
 
 	/* Clear status bits for ERR_FATAL errors only */
-	pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_STATUS, &status);
-	pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_SEVER, &sev);
+	pci_read_config_dword(dev, aer + PCI_ERR_UNCOR_STATUS, &status);
+	pci_read_config_dword(dev, aer + PCI_ERR_UNCOR_SEVER, &sev);
 	status &= sev;
 	if (status)
-		pci_write_config_dword(dev, pos + PCI_ERR_UNCOR_STATUS, status);
+		pci_write_config_dword(dev, aer + PCI_ERR_UNCOR_STATUS, status);
 }
 
 /**
@@ -297,25 +295,24 @@ void pci_aer_clear_fatal_status(struct pci_dev *dev)
  */
 int pci_aer_raw_clear_status(struct pci_dev *dev)
 {
-	int pos;
+	int aer = dev->aer_cap;
 	u32 status;
 	int port_type;
 
-	pos = dev->aer_cap;
-	if (!pos)
+	if (!aer)
 		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, &status);
-		pci_write_config_dword(dev, pos + PCI_ERR_ROOT_STATUS, status);
+		pci_read_config_dword(dev, aer + PCI_ERR_ROOT_STATUS, &status);
+		pci_write_config_dword(dev, aer + PCI_ERR_ROOT_STATUS, status);
 	}
 
-	pci_read_config_dword(dev, pos + PCI_ERR_COR_STATUS, &status);
-	pci_write_config_dword(dev, pos + PCI_ERR_COR_STATUS, status);
+	pci_read_config_dword(dev, aer + PCI_ERR_COR_STATUS, &status);
+	pci_write_config_dword(dev, aer + PCI_ERR_COR_STATUS, status);
 
-	pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_STATUS, &status);
-	pci_write_config_dword(dev, pos + PCI_ERR_UNCOR_STATUS, status);
+	pci_read_config_dword(dev, aer + PCI_ERR_UNCOR_STATUS, &status);
+	pci_write_config_dword(dev, aer + PCI_ERR_UNCOR_STATUS, status);
 
 	return 0;
 }
@@ -330,12 +327,11 @@ int pci_aer_clear_status(struct pci_dev *dev)
 
 void pci_save_aer_state(struct pci_dev *dev)
 {
+	int aer = dev->aer_cap;
 	struct pci_cap_saved_state *save_state;
 	u32 *cap;
-	int pos;
 
-	pos = dev->aer_cap;
-	if (!pos)
+	if (!aer)
 		return;
 
 	save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_ERR);
@@ -343,22 +339,21 @@ void pci_save_aer_state(struct pci_dev *dev)
 		return;
 
 	cap = &save_state->cap.data[0];
-	pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_MASK, cap++);
-	pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_SEVER, cap++);
-	pci_read_config_dword(dev, pos + PCI_ERR_COR_MASK, cap++);
-	pci_read_config_dword(dev, pos + PCI_ERR_CAP, cap++);
+	pci_read_config_dword(dev, aer + PCI_ERR_UNCOR_MASK, cap++);
+	pci_read_config_dword(dev, aer + PCI_ERR_UNCOR_SEVER, cap++);
+	pci_read_config_dword(dev, aer + PCI_ERR_COR_MASK, cap++);
+	pci_read_config_dword(dev, aer + PCI_ERR_CAP, cap++);
 	if (pcie_cap_has_rtctl(dev))
-		pci_read_config_dword(dev, pos + PCI_ERR_ROOT_COMMAND, cap++);
+		pci_read_config_dword(dev, aer + PCI_ERR_ROOT_COMMAND, cap++);
 }
 
 void pci_restore_aer_state(struct pci_dev *dev)
 {
+	int aer = dev->aer_cap;
 	struct pci_cap_saved_state *save_state;
 	u32 *cap;
-	int pos;
 
-	pos = dev->aer_cap;
-	if (!pos)
+	if (!aer)
 		return;
 
 	save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_ERR);
@@ -366,12 +361,12 @@ void pci_restore_aer_state(struct pci_dev *dev)
 		return;
 
 	cap = &save_state->cap.data[0];
-	pci_write_config_dword(dev, pos + PCI_ERR_UNCOR_MASK, *cap++);
-	pci_write_config_dword(dev, pos + PCI_ERR_UNCOR_SEVER, *cap++);
-	pci_write_config_dword(dev, pos + PCI_ERR_COR_MASK, *cap++);
-	pci_write_config_dword(dev, pos + PCI_ERR_CAP, *cap++);
+	pci_write_config_dword(dev, aer + PCI_ERR_UNCOR_MASK, *cap++);
+	pci_write_config_dword(dev, aer + PCI_ERR_UNCOR_SEVER, *cap++);
+	pci_write_config_dword(dev, aer + PCI_ERR_COR_MASK, *cap++);
+	pci_write_config_dword(dev, aer + PCI_ERR_CAP, *cap++);
 	if (pcie_cap_has_rtctl(dev))
-		pci_write_config_dword(dev, pos + PCI_ERR_ROOT_COMMAND, *cap++);
+		pci_write_config_dword(dev, aer + PCI_ERR_ROOT_COMMAND, *cap++);
 }
 
 void pci_aer_init(struct pci_dev *dev)
@@ -802,7 +797,7 @@ static int add_error_device(struct aer_err_info *e_info, struct pci_dev *dev)
  */
 static bool is_error_source(struct pci_dev *dev, struct aer_err_info *e_info)
 {
-	int pos;
+	int aer = dev->aer_cap;
 	u32 status, mask;
 	u16 reg16;
 
@@ -837,17 +832,16 @@ static bool is_error_source(struct pci_dev *dev, struct aer_err_info *e_info)
 	if (!(reg16 & PCI_EXP_AER_FLAGS))
 		return false;
 
-	pos = dev->aer_cap;
-	if (!pos)
+	if (!aer)
 		return false;
 
 	/* Check if error is recorded */
 	if (e_info->severity == AER_CORRECTABLE) {
-		pci_read_config_dword(dev, pos + PCI_ERR_COR_STATUS, &status);
-		pci_read_config_dword(dev, pos + PCI_ERR_COR_MASK, &mask);
+		pci_read_config_dword(dev, aer + PCI_ERR_COR_STATUS, &status);
+		pci_read_config_dword(dev, aer + PCI_ERR_COR_MASK, &mask);
 	} else {
-		pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_STATUS, &status);
-		pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_MASK, &mask);
+		pci_read_config_dword(dev, aer + PCI_ERR_UNCOR_STATUS, &status);
+		pci_read_config_dword(dev, aer + PCI_ERR_UNCOR_MASK, &mask);
 	}
 	if (status & ~mask)
 		return true;
@@ -918,16 +912,15 @@ static bool find_source_device(struct pci_dev *parent,
  */
 static void handle_error_source(struct pci_dev *dev, struct aer_err_info *info)
 {
-	int pos;
+	int aer = dev->aer_cap;
 
 	if (info->severity == AER_CORRECTABLE) {
 		/*
 		 * Correctable error does not need software intervention.
 		 * No need to go through error recovery process.
 		 */
-		pos = dev->aer_cap;
-		if (pos)
-			pci_write_config_dword(dev, pos + PCI_ERR_COR_STATUS,
+		if (aer)
+			pci_write_config_dword(dev, aer + PCI_ERR_COR_STATUS,
 					info->status);
 		pci_aer_clear_device_status(dev);
 	} else if (info->severity == AER_NONFATAL)
@@ -1018,22 +1011,21 @@ EXPORT_SYMBOL_GPL(aer_recover_queue);
  */
 int aer_get_device_error_info(struct pci_dev *dev, struct aer_err_info *info)
 {
-	int pos, temp;
+	int aer = dev->aer_cap;
+	int temp;
 
 	/* Must reset in this function */
 	info->status = 0;
 	info->tlp_header_valid = 0;
 
-	pos = dev->aer_cap;
-
 	/* The device might not support AER */
-	if (!pos)
+	if (!aer)
 		return 0;
 
 	if (info->severity == AER_CORRECTABLE) {
-		pci_read_config_dword(dev, pos + PCI_ERR_COR_STATUS,
+		pci_read_config_dword(dev, aer + PCI_ERR_COR_STATUS,
 			&info->status);
-		pci_read_config_dword(dev, pos + PCI_ERR_COR_MASK,
+		pci_read_config_dword(dev, aer + PCI_ERR_COR_MASK,
 			&info->mask);
 		if (!(info->status & ~info->mask))
 			return 0;
@@ -1042,27 +1034,27 @@ int aer_get_device_error_info(struct pci_dev *dev, struct aer_err_info *info)
 		   info->severity == AER_NONFATAL) {
 
 		/* Link is still healthy for IO reads */
-		pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_STATUS,
+		pci_read_config_dword(dev, aer + PCI_ERR_UNCOR_STATUS,
 			&info->status);
-		pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_MASK,
+		pci_read_config_dword(dev, aer + PCI_ERR_UNCOR_MASK,
 			&info->mask);
 		if (!(info->status & ~info->mask))
 			return 0;
 
 		/* Get First Error Pointer */
-		pci_read_config_dword(dev, pos + PCI_ERR_CAP, &temp);
+		pci_read_config_dword(dev, aer + PCI_ERR_CAP, &temp);
 		info->first_error = PCI_ERR_CAP_FEP(temp);
 
 		if (info->status & AER_LOG_TLP_MASKS) {
 			info->tlp_header_valid = 1;
 			pci_read_config_dword(dev,
-				pos + PCI_ERR_HEADER_LOG, &info->tlp.dw0);
+				aer + PCI_ERR_HEADER_LOG, &info->tlp.dw0);
 			pci_read_config_dword(dev,
-				pos + PCI_ERR_HEADER_LOG + 4, &info->tlp.dw1);
+				aer + PCI_ERR_HEADER_LOG + 4, &info->tlp.dw1);
 			pci_read_config_dword(dev,
-				pos + PCI_ERR_HEADER_LOG + 8, &info->tlp.dw2);
+				aer + PCI_ERR_HEADER_LOG + 8, &info->tlp.dw2);
 			pci_read_config_dword(dev,
-				pos + PCI_ERR_HEADER_LOG + 12, &info->tlp.dw3);
+				aer + PCI_ERR_HEADER_LOG + 12, &info->tlp.dw3);
 		}
 	}
 
@@ -1168,15 +1160,15 @@ static irqreturn_t aer_irq(int irq, void *context)
 	struct pcie_device *pdev = (struct pcie_device *)context;
 	struct aer_rpc *rpc = get_service_data(pdev);
 	struct pci_dev *rp = rpc->rpd;
+	int aer = rp->aer_cap;
 	struct aer_err_source e_src = {};
-	int pos = rp->aer_cap;
 
-	pci_read_config_dword(rp, pos + PCI_ERR_ROOT_STATUS, &e_src.status);
+	pci_read_config_dword(rp, aer + PCI_ERR_ROOT_STATUS, &e_src.status);
 	if (!(e_src.status & (PCI_ERR_ROOT_UNCOR_RCV|PCI_ERR_ROOT_COR_RCV)))
 		return IRQ_NONE;
 
-	pci_read_config_dword(rp, pos + PCI_ERR_ROOT_ERR_SRC, &e_src.id);
-	pci_write_config_dword(rp, pos + PCI_ERR_ROOT_STATUS, e_src.status);
+	pci_read_config_dword(rp, aer + PCI_ERR_ROOT_ERR_SRC, &e_src.id);
+	pci_write_config_dword(rp, aer + PCI_ERR_ROOT_STATUS, e_src.status);
 
 	if (!kfifo_put(&rpc->aer_fifo, e_src))
 		return IRQ_HANDLED;
@@ -1228,7 +1220,7 @@ static void set_downstream_devices_error_reporting(struct pci_dev *dev,
 static void aer_enable_rootport(struct aer_rpc *rpc)
 {
 	struct pci_dev *pdev = rpc->rpd;
-	int aer_pos;
+	int aer = pdev->aer_cap;
 	u16 reg16;
 	u32 reg32;
 
@@ -1240,14 +1232,13 @@ static void aer_enable_rootport(struct aer_rpc *rpc)
 	pcie_capability_clear_word(pdev, PCI_EXP_RTCTL,
 				   SYSTEM_ERROR_INTR_ON_MESG_MASK);
 
-	aer_pos = pdev->aer_cap;
 	/* Clear error status */
-	pci_read_config_dword(pdev, aer_pos + PCI_ERR_ROOT_STATUS, &reg32);
-	pci_write_config_dword(pdev, aer_pos + PCI_ERR_ROOT_STATUS, reg32);
-	pci_read_config_dword(pdev, aer_pos + PCI_ERR_COR_STATUS, &reg32);
-	pci_write_config_dword(pdev, aer_pos + PCI_ERR_COR_STATUS, reg32);
-	pci_read_config_dword(pdev, aer_pos + PCI_ERR_UNCOR_STATUS, &reg32);
-	pci_write_config_dword(pdev, aer_pos + PCI_ERR_UNCOR_STATUS, reg32);
+	pci_read_config_dword(pdev, aer + PCI_ERR_ROOT_STATUS, &reg32);
+	pci_write_config_dword(pdev, aer + PCI_ERR_ROOT_STATUS, reg32);
+	pci_read_config_dword(pdev, aer + PCI_ERR_COR_STATUS, &reg32);
+	pci_write_config_dword(pdev, aer + PCI_ERR_COR_STATUS, reg32);
+	pci_read_config_dword(pdev, aer + PCI_ERR_UNCOR_STATUS, &reg32);
+	pci_write_config_dword(pdev, aer + PCI_ERR_UNCOR_STATUS, reg32);
 
 	/*
 	 * Enable error reporting for the root port device and downstream port
@@ -1256,9 +1247,9 @@ static void aer_enable_rootport(struct aer_rpc *rpc)
 	set_downstream_devices_error_reporting(pdev, true);
 
 	/* Enable Root Port's interrupt in response to error messages */
-	pci_read_config_dword(pdev, aer_pos + PCI_ERR_ROOT_COMMAND, &reg32);
+	pci_read_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, &reg32);
 	reg32 |= ROOT_PORT_INTR_ON_MESG_MASK;
-	pci_write_config_dword(pdev, aer_pos + PCI_ERR_ROOT_COMMAND, reg32);
+	pci_write_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, reg32);
 }
 
 /**
@@ -1270,8 +1261,8 @@ static void aer_enable_rootport(struct aer_rpc *rpc)
 static void aer_disable_rootport(struct aer_rpc *rpc)
 {
 	struct pci_dev *pdev = rpc->rpd;
+	int aer = pdev->aer_cap;
 	u32 reg32;
-	int pos;
 
 	/*
 	 * Disable error reporting for the root port device and downstream port
@@ -1279,15 +1270,14 @@ static void aer_disable_rootport(struct aer_rpc *rpc)
 	 */
 	set_downstream_devices_error_reporting(pdev, false);
 
-	pos = pdev->aer_cap;
 	/* Disable Root's interrupt in response to error messages */
-	pci_read_config_dword(pdev, pos + PCI_ERR_ROOT_COMMAND, &reg32);
+	pci_read_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, &reg32);
 	reg32 &= ~ROOT_PORT_INTR_ON_MESG_MASK;
-	pci_write_config_dword(pdev, pos + PCI_ERR_ROOT_COMMAND, reg32);
+	pci_write_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, reg32);
 
 	/* Clear Root's error status reg */
-	pci_read_config_dword(pdev, pos + PCI_ERR_ROOT_STATUS, &reg32);
-	pci_write_config_dword(pdev, pos + PCI_ERR_ROOT_STATUS, reg32);
+	pci_read_config_dword(pdev, aer + PCI_ERR_ROOT_STATUS, &reg32);
+	pci_write_config_dword(pdev, aer + PCI_ERR_ROOT_STATUS, reg32);
 }
 
 /**
@@ -1344,28 +1334,27 @@ static int aer_probe(struct pcie_device *dev)
  */
 static pci_ers_result_t aer_root_reset(struct pci_dev *dev)
 {
+	int aer = dev->aer_cap;
 	u32 reg32;
-	int pos;
 	int rc;
 
-	pos = dev->aer_cap;
 
 	/* Disable Root's interrupt in response to error messages */
-	pci_read_config_dword(dev, pos + PCI_ERR_ROOT_COMMAND, &reg32);
+	pci_read_config_dword(dev, aer + PCI_ERR_ROOT_COMMAND, &reg32);
 	reg32 &= ~ROOT_PORT_INTR_ON_MESG_MASK;
-	pci_write_config_dword(dev, pos + PCI_ERR_ROOT_COMMAND, reg32);
+	pci_write_config_dword(dev, aer + PCI_ERR_ROOT_COMMAND, reg32);
 
 	rc = pci_bus_error_reset(dev);
 	pci_info(dev, "Root Port link has been reset\n");
 
 	/* Clear Root Error Status */
-	pci_read_config_dword(dev, pos + PCI_ERR_ROOT_STATUS, &reg32);
-	pci_write_config_dword(dev, pos + PCI_ERR_ROOT_STATUS, reg32);
+	pci_read_config_dword(dev, aer + PCI_ERR_ROOT_STATUS, &reg32);
+	pci_write_config_dword(dev, aer + PCI_ERR_ROOT_STATUS, reg32);
 
 	/* Enable Root Port's interrupt in response to error messages */
-	pci_read_config_dword(dev, pos + PCI_ERR_ROOT_COMMAND, &reg32);
+	pci_read_config_dword(dev, aer + PCI_ERR_ROOT_COMMAND, &reg32);
 	reg32 |= ROOT_PORT_INTR_ON_MESG_MASK;
-	pci_write_config_dword(dev, pos + PCI_ERR_ROOT_COMMAND, reg32);
+	pci_write_config_dword(dev, aer + PCI_ERR_ROOT_COMMAND, reg32);
 
 	return rc ? PCI_ERS_RESULT_DISCONNECT : PCI_ERS_RESULT_RECOVERED;
 }

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

* Re: [PATCH v4 3/5] ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set.
  2020-05-26 23:18 ` [PATCH v4 3/5] ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set sathyanarayanan.kuppuswamy
@ 2020-05-29 23:12   ` Bjorn Helgaas
  0 siblings, 0 replies; 9+ messages in thread
From: Bjorn Helgaas @ 2020-05-29 23:12 UTC (permalink / raw)
  To: sathyanarayanan.kuppuswamy; +Cc: bhelgaas, linux-pci, linux-kernel, ashok.raj

On Tue, May 26, 2020 at 04:18:27PM -0700, sathyanarayanan.kuppuswamy@linux.intel.com wrote:
> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> 
> 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 <sathyanarayanan.kuppuswamy@linux.intel.com>
> ---
>  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;
> +	}

I like this, but what I had in mind was to *remove* the tests of
pcie_ports_native elsewhere at the same time.

For example, 

  @@ -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;
   }

and

  @@ -221,7 +221,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;
   }

So I deferred these two _OSC-related things for now.  We can work on
this more in the next cycle.

>  	/*
>  	 * Evaluate the "PCI Boot Configuration" _DSM Function.  If it
> -- 
> 2.17.1
> 

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

* Re: [PATCH v4 0/5] Remove AER HEST table parser
  2020-05-29 23:09 ` [PATCH v4 0/5] Remove AER HEST table parser Bjorn Helgaas
@ 2020-05-29 23:15   ` Kuppuswamy, Sathyanarayanan
  0 siblings, 0 replies; 9+ messages in thread
From: Kuppuswamy, Sathyanarayanan @ 2020-05-29 23:15 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: bhelgaas, linux-pci, linux-kernel, ashok.raj



On 5/29/20 4:09 PM, Bjorn Helgaas wrote:
> On Tue, May 26, 2020 at 04:18:24PM -0700, sathyanarayanan.kuppuswamy@linux.intel.com wrote:
>> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>>
>> 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()
> 
> I reordered these a bit and applied them as follows for v5.8:
> 
>    ("PCI/AER: Remove HEST/FIRMWARE_FIRST parsing for AER ownership")
>    ("PCI/AER: Remove redundant dev->aer_cap checks")
>    ("PCI/AER: Remove redundant pci_is_pcie() checks")
> 
> I added the trivial patch below on top.
> 
>>   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(-)
> 
> commit 643a9f8854f9 ("PCI/AER: Use "aer" variable for capability offset")
> Author: Bjorn Helgaas <bhelgaas@google.com>
> Date:   Fri May 29 17:56:09 2020 -0500
> 
>      PCI/AER: Use "aer" variable for capability offset
>      
>      Previously we used "pos" or "aer_pos" for the offset of the AER Capability.
>      Use "aer" consistently and initialize it the same way everywhere.  No
>      functional change intended.
>      
>      Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Looks good to me.
Reviewed-by: Kuppuswamy Sathyanarayanan 
<sathyanarayanan.kuppuswamy@linux.intel.com>
> 
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index 61e8cb23e98b..3acf56683915 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -136,19 +136,18 @@ static const char * const ecrc_policy_str[] = {
>    */
>   static int enable_ecrc_checking(struct pci_dev *dev)
>   {
> -	int pos;
> +	int aer = dev->aer_cap;
>   	u32 reg32;
>   
> -	pos = dev->aer_cap;
> -	if (!pos)
> +	if (!aer)
>   		return -ENODEV;
>   
> -	pci_read_config_dword(dev, pos + PCI_ERR_CAP, &reg32);
> +	pci_read_config_dword(dev, aer + PCI_ERR_CAP, &reg32);
>   	if (reg32 & PCI_ERR_CAP_ECRC_GENC)
>   		reg32 |= PCI_ERR_CAP_ECRC_GENE;
>   	if (reg32 & PCI_ERR_CAP_ECRC_CHKC)
>   		reg32 |= PCI_ERR_CAP_ECRC_CHKE;
> -	pci_write_config_dword(dev, pos + PCI_ERR_CAP, reg32);
> +	pci_write_config_dword(dev, aer + PCI_ERR_CAP, reg32);
>   
>   	return 0;
>   }
> @@ -161,16 +160,15 @@ static int enable_ecrc_checking(struct pci_dev *dev)
>    */
>   static int disable_ecrc_checking(struct pci_dev *dev)
>   {
> -	int pos;
> +	int aer = dev->aer_cap;
>   	u32 reg32;
>   
> -	pos = dev->aer_cap;
> -	if (!pos)
> +	if (!aer)
>   		return -ENODEV;
>   
> -	pci_read_config_dword(dev, pos + PCI_ERR_CAP, &reg32);
> +	pci_read_config_dword(dev, aer + PCI_ERR_CAP, &reg32);
>   	reg32 &= ~(PCI_ERR_CAP_ECRC_GENE | PCI_ERR_CAP_ECRC_CHKE);
> -	pci_write_config_dword(dev, pos + PCI_ERR_CAP, reg32);
> +	pci_write_config_dword(dev, aer + PCI_ERR_CAP, reg32);
>   
>   	return 0;
>   }
> @@ -253,18 +251,18 @@ void pci_aer_clear_device_status(struct pci_dev *dev)
>   
>   int pci_aer_clear_nonfatal_status(struct pci_dev *dev)
>   {
> -	int pos = dev->aer_cap;
> +	int aer = dev->aer_cap;
>   	u32 status, sev;
>   
>   	if (!pcie_aer_is_native(dev))
>   		return -EIO;
>   
>   	/* Clear status bits for ERR_NONFATAL errors only */
> -	pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_STATUS, &status);
> -	pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_SEVER, &sev);
> +	pci_read_config_dword(dev, aer + PCI_ERR_UNCOR_STATUS, &status);
> +	pci_read_config_dword(dev, aer + PCI_ERR_UNCOR_SEVER, &sev);
>   	status &= ~sev;
>   	if (status)
> -		pci_write_config_dword(dev, pos + PCI_ERR_UNCOR_STATUS, status);
> +		pci_write_config_dword(dev, aer + PCI_ERR_UNCOR_STATUS, status);
>   
>   	return 0;
>   }
> @@ -272,18 +270,18 @@ EXPORT_SYMBOL_GPL(pci_aer_clear_nonfatal_status);
>   
>   void pci_aer_clear_fatal_status(struct pci_dev *dev)
>   {
> -	int pos = dev->aer_cap;
> +	int aer = dev->aer_cap;
>   	u32 status, sev;
>   
>   	if (!pcie_aer_is_native(dev))
>   		return;
>   
>   	/* Clear status bits for ERR_FATAL errors only */
> -	pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_STATUS, &status);
> -	pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_SEVER, &sev);
> +	pci_read_config_dword(dev, aer + PCI_ERR_UNCOR_STATUS, &status);
> +	pci_read_config_dword(dev, aer + PCI_ERR_UNCOR_SEVER, &sev);
>   	status &= sev;
>   	if (status)
> -		pci_write_config_dword(dev, pos + PCI_ERR_UNCOR_STATUS, status);
> +		pci_write_config_dword(dev, aer + PCI_ERR_UNCOR_STATUS, status);
>   }
>   
>   /**
> @@ -297,25 +295,24 @@ void pci_aer_clear_fatal_status(struct pci_dev *dev)
>    */
>   int pci_aer_raw_clear_status(struct pci_dev *dev)
>   {
> -	int pos;
> +	int aer = dev->aer_cap;
>   	u32 status;
>   	int port_type;
>   
> -	pos = dev->aer_cap;
> -	if (!pos)
> +	if (!aer)
>   		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, &status);
> -		pci_write_config_dword(dev, pos + PCI_ERR_ROOT_STATUS, status);
> +		pci_read_config_dword(dev, aer + PCI_ERR_ROOT_STATUS, &status);
> +		pci_write_config_dword(dev, aer + PCI_ERR_ROOT_STATUS, status);
>   	}
>   
> -	pci_read_config_dword(dev, pos + PCI_ERR_COR_STATUS, &status);
> -	pci_write_config_dword(dev, pos + PCI_ERR_COR_STATUS, status);
> +	pci_read_config_dword(dev, aer + PCI_ERR_COR_STATUS, &status);
> +	pci_write_config_dword(dev, aer + PCI_ERR_COR_STATUS, status);
>   
> -	pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_STATUS, &status);
> -	pci_write_config_dword(dev, pos + PCI_ERR_UNCOR_STATUS, status);
> +	pci_read_config_dword(dev, aer + PCI_ERR_UNCOR_STATUS, &status);
> +	pci_write_config_dword(dev, aer + PCI_ERR_UNCOR_STATUS, status);
>   
>   	return 0;
>   }
> @@ -330,12 +327,11 @@ int pci_aer_clear_status(struct pci_dev *dev)
>   
>   void pci_save_aer_state(struct pci_dev *dev)
>   {
> +	int aer = dev->aer_cap;
>   	struct pci_cap_saved_state *save_state;
>   	u32 *cap;
> -	int pos;
>   
> -	pos = dev->aer_cap;
> -	if (!pos)
> +	if (!aer)
>   		return;
>   
>   	save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_ERR);
> @@ -343,22 +339,21 @@ void pci_save_aer_state(struct pci_dev *dev)
>   		return;
>   
>   	cap = &save_state->cap.data[0];
> -	pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_MASK, cap++);
> -	pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_SEVER, cap++);
> -	pci_read_config_dword(dev, pos + PCI_ERR_COR_MASK, cap++);
> -	pci_read_config_dword(dev, pos + PCI_ERR_CAP, cap++);
> +	pci_read_config_dword(dev, aer + PCI_ERR_UNCOR_MASK, cap++);
> +	pci_read_config_dword(dev, aer + PCI_ERR_UNCOR_SEVER, cap++);
> +	pci_read_config_dword(dev, aer + PCI_ERR_COR_MASK, cap++);
> +	pci_read_config_dword(dev, aer + PCI_ERR_CAP, cap++);
>   	if (pcie_cap_has_rtctl(dev))
> -		pci_read_config_dword(dev, pos + PCI_ERR_ROOT_COMMAND, cap++);
> +		pci_read_config_dword(dev, aer + PCI_ERR_ROOT_COMMAND, cap++);
>   }
>   
>   void pci_restore_aer_state(struct pci_dev *dev)
>   {
> +	int aer = dev->aer_cap;
>   	struct pci_cap_saved_state *save_state;
>   	u32 *cap;
> -	int pos;
>   
> -	pos = dev->aer_cap;
> -	if (!pos)
> +	if (!aer)
>   		return;
>   
>   	save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_ERR);
> @@ -366,12 +361,12 @@ void pci_restore_aer_state(struct pci_dev *dev)
>   		return;
>   
>   	cap = &save_state->cap.data[0];
> -	pci_write_config_dword(dev, pos + PCI_ERR_UNCOR_MASK, *cap++);
> -	pci_write_config_dword(dev, pos + PCI_ERR_UNCOR_SEVER, *cap++);
> -	pci_write_config_dword(dev, pos + PCI_ERR_COR_MASK, *cap++);
> -	pci_write_config_dword(dev, pos + PCI_ERR_CAP, *cap++);
> +	pci_write_config_dword(dev, aer + PCI_ERR_UNCOR_MASK, *cap++);
> +	pci_write_config_dword(dev, aer + PCI_ERR_UNCOR_SEVER, *cap++);
> +	pci_write_config_dword(dev, aer + PCI_ERR_COR_MASK, *cap++);
> +	pci_write_config_dword(dev, aer + PCI_ERR_CAP, *cap++);
>   	if (pcie_cap_has_rtctl(dev))
> -		pci_write_config_dword(dev, pos + PCI_ERR_ROOT_COMMAND, *cap++);
> +		pci_write_config_dword(dev, aer + PCI_ERR_ROOT_COMMAND, *cap++);
>   }
>   
>   void pci_aer_init(struct pci_dev *dev)
> @@ -802,7 +797,7 @@ static int add_error_device(struct aer_err_info *e_info, struct pci_dev *dev)
>    */
>   static bool is_error_source(struct pci_dev *dev, struct aer_err_info *e_info)
>   {
> -	int pos;
> +	int aer = dev->aer_cap;
>   	u32 status, mask;
>   	u16 reg16;
>   
> @@ -837,17 +832,16 @@ static bool is_error_source(struct pci_dev *dev, struct aer_err_info *e_info)
>   	if (!(reg16 & PCI_EXP_AER_FLAGS))
>   		return false;
>   
> -	pos = dev->aer_cap;
> -	if (!pos)
> +	if (!aer)
>   		return false;
>   
>   	/* Check if error is recorded */
>   	if (e_info->severity == AER_CORRECTABLE) {
> -		pci_read_config_dword(dev, pos + PCI_ERR_COR_STATUS, &status);
> -		pci_read_config_dword(dev, pos + PCI_ERR_COR_MASK, &mask);
> +		pci_read_config_dword(dev, aer + PCI_ERR_COR_STATUS, &status);
> +		pci_read_config_dword(dev, aer + PCI_ERR_COR_MASK, &mask);
>   	} else {
> -		pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_STATUS, &status);
> -		pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_MASK, &mask);
> +		pci_read_config_dword(dev, aer + PCI_ERR_UNCOR_STATUS, &status);
> +		pci_read_config_dword(dev, aer + PCI_ERR_UNCOR_MASK, &mask);
>   	}
>   	if (status & ~mask)
>   		return true;
> @@ -918,16 +912,15 @@ static bool find_source_device(struct pci_dev *parent,
>    */
>   static void handle_error_source(struct pci_dev *dev, struct aer_err_info *info)
>   {
> -	int pos;
> +	int aer = dev->aer_cap;
>   
>   	if (info->severity == AER_CORRECTABLE) {
>   		/*
>   		 * Correctable error does not need software intervention.
>   		 * No need to go through error recovery process.
>   		 */
> -		pos = dev->aer_cap;
> -		if (pos)
> -			pci_write_config_dword(dev, pos + PCI_ERR_COR_STATUS,
> +		if (aer)
> +			pci_write_config_dword(dev, aer + PCI_ERR_COR_STATUS,
>   					info->status);
>   		pci_aer_clear_device_status(dev);
>   	} else if (info->severity == AER_NONFATAL)
> @@ -1018,22 +1011,21 @@ EXPORT_SYMBOL_GPL(aer_recover_queue);
>    */
>   int aer_get_device_error_info(struct pci_dev *dev, struct aer_err_info *info)
>   {
> -	int pos, temp;
> +	int aer = dev->aer_cap;
> +	int temp;
>   
>   	/* Must reset in this function */
>   	info->status = 0;
>   	info->tlp_header_valid = 0;
>   
> -	pos = dev->aer_cap;
> -
>   	/* The device might not support AER */
> -	if (!pos)
> +	if (!aer)
>   		return 0;
>   
>   	if (info->severity == AER_CORRECTABLE) {
> -		pci_read_config_dword(dev, pos + PCI_ERR_COR_STATUS,
> +		pci_read_config_dword(dev, aer + PCI_ERR_COR_STATUS,
>   			&info->status);
> -		pci_read_config_dword(dev, pos + PCI_ERR_COR_MASK,
> +		pci_read_config_dword(dev, aer + PCI_ERR_COR_MASK,
>   			&info->mask);
>   		if (!(info->status & ~info->mask))
>   			return 0;
> @@ -1042,27 +1034,27 @@ int aer_get_device_error_info(struct pci_dev *dev, struct aer_err_info *info)
>   		   info->severity == AER_NONFATAL) {
>   
>   		/* Link is still healthy for IO reads */
> -		pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_STATUS,
> +		pci_read_config_dword(dev, aer + PCI_ERR_UNCOR_STATUS,
>   			&info->status);
> -		pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_MASK,
> +		pci_read_config_dword(dev, aer + PCI_ERR_UNCOR_MASK,
>   			&info->mask);
>   		if (!(info->status & ~info->mask))
>   			return 0;
>   
>   		/* Get First Error Pointer */
> -		pci_read_config_dword(dev, pos + PCI_ERR_CAP, &temp);
> +		pci_read_config_dword(dev, aer + PCI_ERR_CAP, &temp);
>   		info->first_error = PCI_ERR_CAP_FEP(temp);
>   
>   		if (info->status & AER_LOG_TLP_MASKS) {
>   			info->tlp_header_valid = 1;
>   			pci_read_config_dword(dev,
> -				pos + PCI_ERR_HEADER_LOG, &info->tlp.dw0);
> +				aer + PCI_ERR_HEADER_LOG, &info->tlp.dw0);
>   			pci_read_config_dword(dev,
> -				pos + PCI_ERR_HEADER_LOG + 4, &info->tlp.dw1);
> +				aer + PCI_ERR_HEADER_LOG + 4, &info->tlp.dw1);
>   			pci_read_config_dword(dev,
> -				pos + PCI_ERR_HEADER_LOG + 8, &info->tlp.dw2);
> +				aer + PCI_ERR_HEADER_LOG + 8, &info->tlp.dw2);
>   			pci_read_config_dword(dev,
> -				pos + PCI_ERR_HEADER_LOG + 12, &info->tlp.dw3);
> +				aer + PCI_ERR_HEADER_LOG + 12, &info->tlp.dw3);
>   		}
>   	}
>   
> @@ -1168,15 +1160,15 @@ static irqreturn_t aer_irq(int irq, void *context)
>   	struct pcie_device *pdev = (struct pcie_device *)context;
>   	struct aer_rpc *rpc = get_service_data(pdev);
>   	struct pci_dev *rp = rpc->rpd;
> +	int aer = rp->aer_cap;
>   	struct aer_err_source e_src = {};
> -	int pos = rp->aer_cap;
>   
> -	pci_read_config_dword(rp, pos + PCI_ERR_ROOT_STATUS, &e_src.status);
> +	pci_read_config_dword(rp, aer + PCI_ERR_ROOT_STATUS, &e_src.status);
>   	if (!(e_src.status & (PCI_ERR_ROOT_UNCOR_RCV|PCI_ERR_ROOT_COR_RCV)))
>   		return IRQ_NONE;
>   
> -	pci_read_config_dword(rp, pos + PCI_ERR_ROOT_ERR_SRC, &e_src.id);
> -	pci_write_config_dword(rp, pos + PCI_ERR_ROOT_STATUS, e_src.status);
> +	pci_read_config_dword(rp, aer + PCI_ERR_ROOT_ERR_SRC, &e_src.id);
> +	pci_write_config_dword(rp, aer + PCI_ERR_ROOT_STATUS, e_src.status);
>   
>   	if (!kfifo_put(&rpc->aer_fifo, e_src))
>   		return IRQ_HANDLED;
> @@ -1228,7 +1220,7 @@ static void set_downstream_devices_error_reporting(struct pci_dev *dev,
>   static void aer_enable_rootport(struct aer_rpc *rpc)
>   {
>   	struct pci_dev *pdev = rpc->rpd;
> -	int aer_pos;
> +	int aer = pdev->aer_cap;
>   	u16 reg16;
>   	u32 reg32;
>   
> @@ -1240,14 +1232,13 @@ static void aer_enable_rootport(struct aer_rpc *rpc)
>   	pcie_capability_clear_word(pdev, PCI_EXP_RTCTL,
>   				   SYSTEM_ERROR_INTR_ON_MESG_MASK);
>   
> -	aer_pos = pdev->aer_cap;
>   	/* Clear error status */
> -	pci_read_config_dword(pdev, aer_pos + PCI_ERR_ROOT_STATUS, &reg32);
> -	pci_write_config_dword(pdev, aer_pos + PCI_ERR_ROOT_STATUS, reg32);
> -	pci_read_config_dword(pdev, aer_pos + PCI_ERR_COR_STATUS, &reg32);
> -	pci_write_config_dword(pdev, aer_pos + PCI_ERR_COR_STATUS, reg32);
> -	pci_read_config_dword(pdev, aer_pos + PCI_ERR_UNCOR_STATUS, &reg32);
> -	pci_write_config_dword(pdev, aer_pos + PCI_ERR_UNCOR_STATUS, reg32);
> +	pci_read_config_dword(pdev, aer + PCI_ERR_ROOT_STATUS, &reg32);
> +	pci_write_config_dword(pdev, aer + PCI_ERR_ROOT_STATUS, reg32);
> +	pci_read_config_dword(pdev, aer + PCI_ERR_COR_STATUS, &reg32);
> +	pci_write_config_dword(pdev, aer + PCI_ERR_COR_STATUS, reg32);
> +	pci_read_config_dword(pdev, aer + PCI_ERR_UNCOR_STATUS, &reg32);
> +	pci_write_config_dword(pdev, aer + PCI_ERR_UNCOR_STATUS, reg32);
>   
>   	/*
>   	 * Enable error reporting for the root port device and downstream port
> @@ -1256,9 +1247,9 @@ static void aer_enable_rootport(struct aer_rpc *rpc)
>   	set_downstream_devices_error_reporting(pdev, true);
>   
>   	/* Enable Root Port's interrupt in response to error messages */
> -	pci_read_config_dword(pdev, aer_pos + PCI_ERR_ROOT_COMMAND, &reg32);
> +	pci_read_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, &reg32);
>   	reg32 |= ROOT_PORT_INTR_ON_MESG_MASK;
> -	pci_write_config_dword(pdev, aer_pos + PCI_ERR_ROOT_COMMAND, reg32);
> +	pci_write_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, reg32);
>   }
>   
>   /**
> @@ -1270,8 +1261,8 @@ static void aer_enable_rootport(struct aer_rpc *rpc)
>   static void aer_disable_rootport(struct aer_rpc *rpc)
>   {
>   	struct pci_dev *pdev = rpc->rpd;
> +	int aer = pdev->aer_cap;
>   	u32 reg32;
> -	int pos;
>   
>   	/*
>   	 * Disable error reporting for the root port device and downstream port
> @@ -1279,15 +1270,14 @@ static void aer_disable_rootport(struct aer_rpc *rpc)
>   	 */
>   	set_downstream_devices_error_reporting(pdev, false);
>   
> -	pos = pdev->aer_cap;
>   	/* Disable Root's interrupt in response to error messages */
> -	pci_read_config_dword(pdev, pos + PCI_ERR_ROOT_COMMAND, &reg32);
> +	pci_read_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, &reg32);
>   	reg32 &= ~ROOT_PORT_INTR_ON_MESG_MASK;
> -	pci_write_config_dword(pdev, pos + PCI_ERR_ROOT_COMMAND, reg32);
> +	pci_write_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, reg32);
>   
>   	/* Clear Root's error status reg */
> -	pci_read_config_dword(pdev, pos + PCI_ERR_ROOT_STATUS, &reg32);
> -	pci_write_config_dword(pdev, pos + PCI_ERR_ROOT_STATUS, reg32);
> +	pci_read_config_dword(pdev, aer + PCI_ERR_ROOT_STATUS, &reg32);
> +	pci_write_config_dword(pdev, aer + PCI_ERR_ROOT_STATUS, reg32);
>   }
>   
>   /**
> @@ -1344,28 +1334,27 @@ static int aer_probe(struct pcie_device *dev)
>    */
>   static pci_ers_result_t aer_root_reset(struct pci_dev *dev)
>   {
> +	int aer = dev->aer_cap;
>   	u32 reg32;
> -	int pos;
>   	int rc;
>   
> -	pos = dev->aer_cap;
>   
>   	/* Disable Root's interrupt in response to error messages */
> -	pci_read_config_dword(dev, pos + PCI_ERR_ROOT_COMMAND, &reg32);
> +	pci_read_config_dword(dev, aer + PCI_ERR_ROOT_COMMAND, &reg32);
>   	reg32 &= ~ROOT_PORT_INTR_ON_MESG_MASK;
> -	pci_write_config_dword(dev, pos + PCI_ERR_ROOT_COMMAND, reg32);
> +	pci_write_config_dword(dev, aer + PCI_ERR_ROOT_COMMAND, reg32);
>   
>   	rc = pci_bus_error_reset(dev);
>   	pci_info(dev, "Root Port link has been reset\n");
>   
>   	/* Clear Root Error Status */
> -	pci_read_config_dword(dev, pos + PCI_ERR_ROOT_STATUS, &reg32);
> -	pci_write_config_dword(dev, pos + PCI_ERR_ROOT_STATUS, reg32);
> +	pci_read_config_dword(dev, aer + PCI_ERR_ROOT_STATUS, &reg32);
> +	pci_write_config_dword(dev, aer + PCI_ERR_ROOT_STATUS, reg32);
>   
>   	/* Enable Root Port's interrupt in response to error messages */
> -	pci_read_config_dword(dev, pos + PCI_ERR_ROOT_COMMAND, &reg32);
> +	pci_read_config_dword(dev, aer + PCI_ERR_ROOT_COMMAND, &reg32);
>   	reg32 |= ROOT_PORT_INTR_ON_MESG_MASK;
> -	pci_write_config_dword(dev, pos + PCI_ERR_ROOT_COMMAND, reg32);
> +	pci_write_config_dword(dev, aer + PCI_ERR_ROOT_COMMAND, reg32);
>   
>   	return rc ? PCI_ERS_RESULT_DISCONNECT : PCI_ERS_RESULT_RECOVERED;
>   }
> 

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

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

end of thread, other threads:[~2020-05-29 23:16 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-26 23:18 [PATCH v4 0/5] Remove AER HEST table parser sathyanarayanan.kuppuswamy
2020-05-26 23:18 ` [PATCH v4 1/5] PCI/AER: Remove redundant pci_is_pcie() checks sathyanarayanan.kuppuswamy
2020-05-26 23:18 ` [PATCH v4 2/5] PCI/AER: Remove redundant dev->aer_cap checks sathyanarayanan.kuppuswamy
2020-05-26 23:18 ` [PATCH v4 3/5] ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set sathyanarayanan.kuppuswamy
2020-05-29 23:12   ` Bjorn Helgaas
2020-05-26 23:18 ` [PATCH v4 4/5] ACPI/PCI: Ignore _OSC DPC negotiation result if pcie_ports_dpc_native " sathyanarayanan.kuppuswamy
2020-05-26 23:18 ` [PATCH v4 5/5] PCI/AER: Replace pcie_aer_get_firmware_first() with pcie_aer_is_native() sathyanarayanan.kuppuswamy
2020-05-29 23:09 ` [PATCH v4 0/5] Remove AER HEST table parser Bjorn Helgaas
2020-05-29 23:15   ` Kuppuswamy, Sathyanarayanan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).