| Message ID | 20240223080021.1692996-1-vidyas@nvidia.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | [V4] PCI: Add support for preserving boot configuration | expand |
On Fri, Feb 23, 2024 at 01:30:21PM +0530, Vidya Sagar wrote: > Add support for preserving the boot configuration done by the > platform firmware per host bridge basis, based on the presence of > 'linux,pci-probe-only' property in the respective PCI host bridge > device-tree node. It also unifies the ACPI and DT based boot flows > in this regard. > > Signed-off-by: Vidya Sagar <vidyas@nvidia.com> > --- > V4: > * Addressed Bjorn's review comments > > V3: > * Unified ACPI and DT flows as part of addressing Bjorn's review comments > > V2: > * Addressed issues reported by kernel test robot <lkp@intel.com> > > drivers/acpi/pci_root.c | 12 ------- > drivers/pci/controller/pci-host-common.c | 4 --- > drivers/pci/of.c | 21 +++++++++++ > drivers/pci/probe.c | 46 ++++++++++++++++++------ > include/linux/of_pci.h | 6 ++++ > 5 files changed, 62 insertions(+), 27 deletions(-) > > diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c > index 84030804a763..ddc2b3e89111 100644 > --- a/drivers/acpi/pci_root.c > +++ b/drivers/acpi/pci_root.c > @@ -1008,7 +1008,6 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, > int node = acpi_get_node(device->handle); > struct pci_bus *bus; > struct pci_host_bridge *host_bridge; > - union acpi_object *obj; > > info->root = root; > info->bridge = device; > @@ -1050,17 +1049,6 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, > if (!(root->osc_ext_control_set & OSC_CXL_ERROR_REPORTING_CONTROL)) > host_bridge->native_cxl_error = 0; > > - /* > - * Evaluate the "PCI Boot Configuration" _DSM Function. If it > - * exists and returns 0, we must preserve any PCI resource > - * assignments made by firmware for this host bridge. > - */ > - obj = acpi_evaluate_dsm(ACPI_HANDLE(bus->bridge), &pci_acpi_dsm_guid, 1, > - DSM_PCI_PRESERVE_BOOT_CONFIG, NULL); > - if (obj && obj->type == ACPI_TYPE_INTEGER && obj->integer.value == 0) > - host_bridge->preserve_config = 1; > - ACPI_FREE(obj); > - > acpi_dev_power_up_children_with_adr(device); > > pci_scan_child_bus(bus); > diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c > index 6be3266cd7b5..e2602e38ae45 100644 > --- a/drivers/pci/controller/pci-host-common.c > +++ b/drivers/pci/controller/pci-host-common.c > @@ -73,10 +73,6 @@ int pci_host_common_probe(struct platform_device *pdev) > if (IS_ERR(cfg)) > return PTR_ERR(cfg); > > - /* Do not reassign resources if probe only */ > - if (!pci_has_flag(PCI_PROBE_ONLY)) > - pci_add_flags(PCI_REASSIGN_ALL_BUS); > - > bridge->sysdata = cfg; > bridge->ops = (struct pci_ops *)&ops->pci_ops; > bridge->msi_domain = true; > diff --git a/drivers/pci/of.c b/drivers/pci/of.c > index 51e3dd0ea5ab..f0f1156040a5 100644 > --- a/drivers/pci/of.c > +++ b/drivers/pci/of.c > @@ -258,6 +258,27 @@ void of_pci_check_probe_only(void) > } > EXPORT_SYMBOL_GPL(of_pci_check_probe_only); > > +/** > + * of_pci_bridge_preserve_resources - Return true if the boot configuration > + * needs to be preserved > + * @node: Device tree node with the domain information. > + * > + * This function looks for "linux,pci-probe-only" property for a given > + * PCI controller's node and returns true if found. Having this property > + * for a PCI controller ensures that the kernel doesn't reconfigure the > + * BARs and bridge windows that are already done by the platform firmware. > + * NOTE: The scope of "linux,pci-probe-only" defined within a PCI bridge device > + * is limited to the hierarchy under that particular bridge device. whereas > + * the scope of "linux,pci-probe-only" defined within chosen node is > + * system wide. > + * > + * Return: true if the property exists false otherwise. > + */ > +bool of_pci_bridge_preserve_resources(struct device_node *node) > +{ > + return of_property_read_bool(node, "linux,pci-probe-only"); This is the wrong type. The existing "linux,pci-probe-only" is a u32 and non-zero value means probe-only. This would return true for 'linux,pci-probe-only = <0>'. Also, this should also check chosen. If you make this work accepting NULL for node, then of_pci_check_probe_only() can be re-implemented to use it. Rob
On Fri, Feb 23, 2024 at 01:30:21PM +0530, Vidya Sagar wrote: > Add support for preserving the boot configuration done by the > platform firmware per host bridge basis, based on the presence of > 'linux,pci-probe-only' property in the respective PCI host bridge > device-tree node. It also unifies the ACPI and DT based boot flows > in this regard. > > Signed-off-by: Vidya Sagar <vidyas@nvidia.com> > --- > V4: > * Addressed Bjorn's review comments > > V3: > * Unified ACPI and DT flows as part of addressing Bjorn's review comments > > V2: > * Addressed issues reported by kernel test robot <lkp@intel.com> > > drivers/acpi/pci_root.c | 12 ------- > drivers/pci/controller/pci-host-common.c | 4 --- > drivers/pci/of.c | 21 +++++++++++ > drivers/pci/probe.c | 46 ++++++++++++++++++------ > include/linux/of_pci.h | 6 ++++ > 5 files changed, 62 insertions(+), 27 deletions(-) One more thing. > @@ -3080,20 +3106,18 @@ int pci_host_probe(struct pci_host_bridge *bridge) > > bus = bridge->bus; > > + /* If we must preserve the resource configuration, claim now */ > + if (pci_has_flag(PCI_PROBE_ONLY) || bridge->preserve_config) > + pci_bus_claim_resources(bus); No reason to check PCI_PROBE_ONLY if you set preserve_config based on /chosen as well. IOW, we should deprecate PCI_PROBE_ONLY flag in favor of the per host bridge setting. Rob
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index 84030804a763..ddc2b3e89111 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -1008,7 +1008,6 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, int node = acpi_get_node(device->handle); struct pci_bus *bus; struct pci_host_bridge *host_bridge; - union acpi_object *obj; info->root = root; info->bridge = device; @@ -1050,17 +1049,6 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, if (!(root->osc_ext_control_set & OSC_CXL_ERROR_REPORTING_CONTROL)) host_bridge->native_cxl_error = 0; - /* - * Evaluate the "PCI Boot Configuration" _DSM Function. If it - * exists and returns 0, we must preserve any PCI resource - * assignments made by firmware for this host bridge. - */ - obj = acpi_evaluate_dsm(ACPI_HANDLE(bus->bridge), &pci_acpi_dsm_guid, 1, - DSM_PCI_PRESERVE_BOOT_CONFIG, NULL); - if (obj && obj->type == ACPI_TYPE_INTEGER && obj->integer.value == 0) - host_bridge->preserve_config = 1; - ACPI_FREE(obj); - acpi_dev_power_up_children_with_adr(device); pci_scan_child_bus(bus); diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c index 6be3266cd7b5..e2602e38ae45 100644 --- a/drivers/pci/controller/pci-host-common.c +++ b/drivers/pci/controller/pci-host-common.c @@ -73,10 +73,6 @@ int pci_host_common_probe(struct platform_device *pdev) if (IS_ERR(cfg)) return PTR_ERR(cfg); - /* Do not reassign resources if probe only */ - if (!pci_has_flag(PCI_PROBE_ONLY)) - pci_add_flags(PCI_REASSIGN_ALL_BUS); - bridge->sysdata = cfg; bridge->ops = (struct pci_ops *)&ops->pci_ops; bridge->msi_domain = true; diff --git a/drivers/pci/of.c b/drivers/pci/of.c index 51e3dd0ea5ab..f0f1156040a5 100644 --- a/drivers/pci/of.c +++ b/drivers/pci/of.c @@ -258,6 +258,27 @@ void of_pci_check_probe_only(void) } EXPORT_SYMBOL_GPL(of_pci_check_probe_only); +/** + * of_pci_bridge_preserve_resources - Return true if the boot configuration + * needs to be preserved + * @node: Device tree node with the domain information. + * + * This function looks for "linux,pci-probe-only" property for a given + * PCI controller's node and returns true if found. Having this property + * for a PCI controller ensures that the kernel doesn't reconfigure the + * BARs and bridge windows that are already done by the platform firmware. + * NOTE: The scope of "linux,pci-probe-only" defined within a PCI bridge device + * is limited to the hierarchy under that particular bridge device. whereas + * the scope of "linux,pci-probe-only" defined within chosen node is + * system wide. + * + * Return: true if the property exists false otherwise. + */ +bool of_pci_bridge_preserve_resources(struct device_node *node) +{ + return of_property_read_bool(node, "linux,pci-probe-only"); +} + /** * devm_of_pci_get_host_bridge_resources() - Resource-managed parsing of PCI * host bridge resources from DT diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 795534589b98..c57648084503 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -15,6 +15,7 @@ #include <linux/cpumask.h> #include <linux/aer.h> #include <linux/acpi.h> +#include <linux/pci-acpi.h> #include <linux/hypervisor.h> #include <linux/irqdomain.h> #include <linux/pm_runtime.h> @@ -877,6 +878,28 @@ static void pci_set_bus_msi_domain(struct pci_bus *bus) dev_set_msi_domain(&bus->dev, d); } +static void pci_check_config_preserve(struct pci_host_bridge *host_bridge) +{ + if (ACPI_HANDLE(&host_bridge->dev)) { + union acpi_object *obj; + + /* + * Evaluate the "PCI Boot Configuration" _DSM Function. If it + * exists and returns 0, we must preserve any PCI resource + * assignments made by firmware for this host bridge. + */ + obj = acpi_evaluate_dsm(ACPI_HANDLE(&host_bridge->dev), &pci_acpi_dsm_guid, 1, + DSM_PCI_PRESERVE_BOOT_CONFIG, NULL); + if (obj && obj->type == ACPI_TYPE_INTEGER && obj->integer.value == 0) + host_bridge->preserve_config = 1; + ACPI_FREE(obj); + } + + if (host_bridge->dev.parent && host_bridge->dev.parent->of_node) + host_bridge->preserve_config = + of_pci_bridge_preserve_resources(host_bridge->dev.parent->of_node); +} + static int pci_register_host_bridge(struct pci_host_bridge *bridge) { struct device *parent = bridge->dev.parent; @@ -971,6 +994,9 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge) if (nr_node_ids > 1 && pcibus_to_node(bus) == NUMA_NO_NODE) dev_warn(&bus->dev, "Unknown NUMA node; performance will be reduced\n"); + /* Check if the boot configuration by FW needs to be preserved */ + pci_check_config_preserve(bridge); + /* Coalesce contiguous windows */ resource_list_for_each_entry_safe(window, n, &resources) { if (list_is_last(&window->node, &resources)) @@ -3080,20 +3106,18 @@ int pci_host_probe(struct pci_host_bridge *bridge) bus = bridge->bus; + /* If we must preserve the resource configuration, claim now */ + if (pci_has_flag(PCI_PROBE_ONLY) || bridge->preserve_config) + pci_bus_claim_resources(bus); + /* - * We insert PCI resources into the iomem_resource and - * ioport_resource trees in either pci_bus_claim_resources() - * or pci_bus_assign_resources(). + * Assign whatever was left unassigned. If we didn't claim above, + * this will reassign everything. */ - if (pci_has_flag(PCI_PROBE_ONLY)) { - pci_bus_claim_resources(bus); - } else { - pci_bus_size_bridges(bus); - pci_bus_assign_resources(bus); + pci_assign_unassigned_root_bus_resources(bus); - list_for_each_entry(child, &bus->children, node) - pcie_bus_configure_settings(child); - } + list_for_each_entry(child, &bus->children, node) + pcie_bus_configure_settings(child); pci_bus_add_devices(bus); return 0; diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h index 29658c0ee71f..3f3909a5d55d 100644 --- a/include/linux/of_pci.h +++ b/include/linux/of_pci.h @@ -13,6 +13,7 @@ struct device_node *of_pci_find_child_device(struct device_node *parent, unsigned int devfn); int of_pci_get_devfn(struct device_node *np); void of_pci_check_probe_only(void); +bool of_pci_bridge_preserve_resources(struct device_node *node); #else static inline struct device_node *of_pci_find_child_device(struct device_node *parent, unsigned int devfn) @@ -26,6 +27,11 @@ static inline int of_pci_get_devfn(struct device_node *np) } static inline void of_pci_check_probe_only(void) { } + +static inline bool of_pci_bridge_preserve_resources(struct device_node *node) +{ + return false; +} #endif #if IS_ENABLED(CONFIG_OF_IRQ)
Add support for preserving the boot configuration done by the platform firmware per host bridge basis, based on the presence of 'linux,pci-probe-only' property in the respective PCI host bridge device-tree node. It also unifies the ACPI and DT based boot flows in this regard. Signed-off-by: Vidya Sagar <vidyas@nvidia.com> --- V4: * Addressed Bjorn's review comments V3: * Unified ACPI and DT flows as part of addressing Bjorn's review comments V2: * Addressed issues reported by kernel test robot <lkp@intel.com> drivers/acpi/pci_root.c | 12 ------- drivers/pci/controller/pci-host-common.c | 4 --- drivers/pci/of.c | 21 +++++++++++ drivers/pci/probe.c | 46 ++++++++++++++++++------ include/linux/of_pci.h | 6 ++++ 5 files changed, 62 insertions(+), 27 deletions(-)