| Message ID | 9a3fcebd55cc6ae2856eee8c4edd52e972adbd0b.1603738449.git.sathyanarayanan.kuppuswamy@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | Simplify PCIe native ownership detection logic | expand |
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
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
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(-)