Re: [PATCH V4] PCI: Add support for preserving boot configuration

Rob Herring
2024-03-05T14:10:22Z

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