Subject: [PATCH v10 0/5] Simplify PCIe native ownership detection logic

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


Subject: [PATCH v10 2/5] ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set.

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 <[email protected]>
---
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

Subject: [PATCH v10 4/5] PCI/portdrv: Remove redundant pci_aer_available() check in DPC enable logic

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 <[email protected]>
---
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

Subject: [PATCH v10 3/5] ACPI/PCI: Ignore _OSC DPC negotiation result if pcie_ports_dpc_native is set.

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 <[email protected]>
---
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

2020-10-27 01:04:41

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v10 2/5] ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set.

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 <[email protected]>

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/[email protected]


Attachments:
(No filename) (4.15 kB)
.config.gz (38.37 kB)
Download all attachments
Subject: [PATCH v10 1/5] PCI: Conditionally initialize host bridge native_* members

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 <[email protected]>
---
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

Subject: [PATCH v10 5/5] PCI/DPC: Move AER/DPC dependency checks out of DPC driver

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 <[email protected]>
---
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

2020-10-29 10:00:11

by Ethan Zhao

[permalink] [raw]
Subject: Re: [PATCH v10 5/5] PCI/DPC: Move AER/DPC dependency checks out of DPC driver

On Tue, Oct 27, 2020 at 11:12 AM Kuppuswamy Sathyanarayanan
<[email protected]> 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 <[email protected]>
> ---
> 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
>