[PATCH 0/6] Add support for the root PCI bus device-tree node creation.

[PATCH 0/6] Add support for the root PCI bus device-tree node creation.

[Herve Codina]

Hi,

This series adds support for creating a device-tree node for a root PCI
bus on non device-tree based system.

Creating device-tree nodes for PCI devices already exists upstream. It
was added in commit 407d1a51921e ("PCI: Create device tree node for
bridge"). Created device-tree nodes need a parent node to be attached
to. For the first level devices, on device-tree based system, this
parent node (i.e. the root PCI bus) is described in the base device-tree
(PCI controller).

The LAN966x PCI device driver was recently accepted [1] and relies on
this feature.

On system where the base hardware is not described by a device-tree, the
root PCI bus node to which first level created PCI devices need to be
attach to does not exist. This is the case for instance on ACPI
described systems such as x86.

This series goal is to handle this case.

In order to have the root PCI bus device-tree node available even on
x86, this top level node is created (if not already present) based on
information computed by the PCI core. It follows the same mechanism as
the one used for PCI devices device-tree node creation.

In order to have this feature available, a number of changes are needed:
  - Patch 1 and 2: Introduce and use device_{add,remove}_of_node().
    This function will also be used in the root PCI bus node creation.

  - Patch 3 and 4: Improve existing functions to reuse them in the root
    PCI bus node creation.

  - Patch 5: Update the default value used when #address-cells is not
    available in the device-tree root node.

  - Patch 6: The root PCI bus device-tree node creation itself.

With those modifications, the LAN966x PCI device is working on x86 systems.

[1] https://lore.kernel.org/lkml/7512cbb7911b8395d926e9e9e390fbb55ce3aea9.camel@pengutronix.de/

Best regards,
Hervé Codina

Herve Codina (6):
  driver core: Introduce device_{add,remove}_of_node()
  PCI: of: Use device_{add,remove}_of_node() to attach of_node to
    existing device
  PCI: of_property: Add support for NULL pdev in of_pci_set_address()
  PCI: of_property: Constify parameter in of_pci_get_addr_flags()
  of: Use the standards compliant default address cells value for x86
  PCI: of: Create device-tree root bus node

 drivers/base/core.c       |  52 ++++++++++++++++++
 drivers/of/of_private.h   |   2 +-
 drivers/pci/of.c          |  89 +++++++++++++++++++++++++++++-
 drivers/pci/of_property.c | 113 ++++++++++++++++++++++++++++++++++++--
 drivers/pci/pci.h         |   6 ++
 drivers/pci/probe.c       |   2 +
 drivers/pci/remove.c      |   2 +
 include/linux/device.h    |   2 +
 8 files changed, 260 insertions(+), 8 deletions(-)

-- 
2.46.2

[PATCH 1/6] driver core: Introduce device_{add,remove}_of_node()

[Herve Codina]

An of_node can be set to a device using device_set_node().
This function cannot prevent any of_node and/or fwnode overwrites.

When adding an of_node on an already present device, the following
operations need to be done:
- Attach the of_node if no of_node were already attached
- Attach the of_node as a fwnode if no fwnode were already attached

This is the purpose of device_add_of_node().
device_remove_of_node() reverts the operations done by
device_add_of_node().

Cc: stable@vger.kernel.org
Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/base/core.c    | 52 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/device.h |  2 ++
 2 files changed, 54 insertions(+)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 24c572031403..0aa63371f55d 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -5118,6 +5118,58 @@ void set_secondary_fwnode(struct device *dev, struct fwnode_handle *fwnode)
 }
 EXPORT_SYMBOL_GPL(set_secondary_fwnode);
 
+/**
+ * device_remove_of_node - Remove an of_node from a device
+ * @dev: device whose device-tree node is being removed
+ */
+void device_remove_of_node(struct device *dev)
+{
+	dev = get_device(dev);
+	if (!dev)
+		return;
+
+	if (!dev->of_node)
+		goto end;
+
+	if (dev->fwnode == of_fwnode_handle(dev->of_node))
+		dev->fwnode = NULL;
+
+	of_node_put(dev->of_node);
+	dev->of_node = NULL;
+
+end:
+	put_device(dev);
+}
+EXPORT_SYMBOL_GPL(device_remove_of_node);
+
+/**
+ * device_add_of_node - Add an of_node to an existing device
+ * @dev: device whose device-tree node is being added
+ * @of_node: of_node to add
+ */
+void device_add_of_node(struct device *dev, struct device_node *of_node)
+{
+	if (!of_node)
+		return;
+
+	dev = get_device(dev);
+	if (!dev)
+		return;
+
+	if (WARN(dev->of_node, "%s: Cannot replace node %pOF with %pOF\n",
+		 dev_name(dev), dev->of_node, of_node))
+		goto end;
+
+	dev->of_node = of_node_get(of_node);
+
+	if (!dev->fwnode)
+		dev->fwnode = of_fwnode_handle(of_node);
+
+end:
+	put_device(dev);
+}
+EXPORT_SYMBOL_GPL(device_add_of_node);
+
 /**
  * device_set_of_node_from_dev - reuse device-tree node of another device
  * @dev: device whose device-tree node is being set
diff --git a/include/linux/device.h b/include/linux/device.h
index b4bde8d22697..e3aa25ce1f90 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1146,6 +1146,8 @@ int device_online(struct device *dev);
 void set_primary_fwnode(struct device *dev, struct fwnode_handle *fwnode);
 void set_secondary_fwnode(struct device *dev, struct fwnode_handle *fwnode);
 void device_set_node(struct device *dev, struct fwnode_handle *fwnode);
+void device_add_of_node(struct device *dev, struct device_node *of_node);
+void device_remove_of_node(struct device *dev);
 void device_set_of_node_from_dev(struct device *dev, const struct device *dev2);
 
 static inline struct device_node *dev_of_node(struct device *dev)
-- 
2.46.2

[PATCH 2/6] PCI: of: Use device_{add,remove}_of_node() to attach of_node to existing device

[Herve Codina]

The commit 407d1a51921e ("PCI: Create device tree node for bridge")
creates of_node for PCI devices. The newly created of_node is attached
to an existing device. This is done setting directly pdev->dev.of_node
in the code.

Even if pdev->dev.of_node cannot be previously set, this doesn't handle
the fwnode field of the struct device. Indeed, this field needs to be
set if it hasn't already been set.

device_{add,remove}_of_node() have been introduced to handle this case.

Use them instead of the direct setting.

Fixes: 407d1a51921e ("PCI: Create device tree node for bridge")
Cc: stable@vger.kernel.org
Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/pci/of.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index dacea3fc5128..141ffbb1b3e6 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -655,8 +655,8 @@ void of_pci_remove_node(struct pci_dev *pdev)
 	np = pci_device_to_OF_node(pdev);
 	if (!np || !of_node_check_flag(np, OF_DYNAMIC))
 		return;
-	pdev->dev.of_node = NULL;
 
+	device_remove_of_node(&pdev->dev);
 	of_changeset_revert(np->data);
 	of_changeset_destroy(np->data);
 	of_node_put(np);
@@ -713,7 +713,7 @@ void of_pci_make_dev_node(struct pci_dev *pdev)
 		goto out_free_node;
 
 	np->data = cset;
-	pdev->dev.of_node = np;
+	device_add_of_node(&pdev->dev, np);
 	kfree(name);
 
 	return;
-- 
2.46.2

Re: [PATCH 2/6] PCI: of: Use device_{add,remove}_of_node() to attach of_node to existing device

[Rob Herring]

On Mon, Nov 04, 2024 at 06:19:56PM +0100, Herve Codina wrote:
> The commit 407d1a51921e ("PCI: Create device tree node for bridge")
> creates of_node for PCI devices. The newly created of_node is attached
> to an existing device. This is done setting directly pdev->dev.of_node
> in the code.
> 
> Even if pdev->dev.of_node cannot be previously set, this doesn't handle
> the fwnode field of the struct device. Indeed, this field needs to be
> set if it hasn't already been set.
> 
> device_{add,remove}_of_node() have been introduced to handle this case.
> 
> Use them instead of the direct setting.
> 
> Fixes: 407d1a51921e ("PCI: Create device tree node for bridge")
> Cc: stable@vger.kernel.org

I don't think this is stable material. What exactly would is broken 
which would be fixed by just the first 2 patches?


> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  drivers/pci/of.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index dacea3fc5128..141ffbb1b3e6 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -655,8 +655,8 @@ void of_pci_remove_node(struct pci_dev *pdev)
>  	np = pci_device_to_OF_node(pdev);
>  	if (!np || !of_node_check_flag(np, OF_DYNAMIC))
>  		return;
> -	pdev->dev.of_node = NULL;
>  
> +	device_remove_of_node(&pdev->dev);
>  	of_changeset_revert(np->data);
>  	of_changeset_destroy(np->data);
>  	of_node_put(np);
> @@ -713,7 +713,7 @@ void of_pci_make_dev_node(struct pci_dev *pdev)
>  		goto out_free_node;
>  
>  	np->data = cset;
> -	pdev->dev.of_node = np;
> +	device_add_of_node(&pdev->dev, np);
>  	kfree(name);
>  
>  	return;
> -- 
> 2.46.2
> 

Re: [PATCH 2/6] PCI: of: Use device_{add,remove}_of_node() to attach of_node to existing device

[Herve Codina]

Hi Rob,

On Mon, 4 Nov 2024 14:20:08 -0600
Rob Herring <robh@kernel.org> wrote:

> On Mon, Nov 04, 2024 at 06:19:56PM +0100, Herve Codina wrote:
> > The commit 407d1a51921e ("PCI: Create device tree node for bridge")
> > creates of_node for PCI devices. The newly created of_node is attached
> > to an existing device. This is done setting directly pdev->dev.of_node
> > in the code.
> > 
> > Even if pdev->dev.of_node cannot be previously set, this doesn't handle
> > the fwnode field of the struct device. Indeed, this field needs to be
> > set if it hasn't already been set.
> > 
> > device_{add,remove}_of_node() have been introduced to handle this case.
> > 
> > Use them instead of the direct setting.
> > 
> > Fixes: 407d1a51921e ("PCI: Create device tree node for bridge")
> > Cc: stable@vger.kernel.org  
> 
> I don't think this is stable material. What exactly would is broken 
> which would be fixed by just the first 2 patches?

Hum indeed, I haven't observed a broken behavior in current kernel.
I will remove Fixes and Cc in the next iteration.

Best regards,
Hervé

[PATCH 3/6] PCI: of_property: Add support for NULL pdev in of_pci_set_address()

[Herve Codina]

The pdev (pointer to a struct pci_dev) parameter of of_pci_set_address()
cannot be NULL.

In order to reuse of_pci_set_address() when creating the PCI root bus
node, this function needs to support a NULL pdev parameter. Indeed, in
the case of the PCI root bus node creation, no pdev are available and
of_pci_set_address() will be used with the bridge windows.

Allow to call of_pci_set_address() with a NULL pdev.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/pci/of_property.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/of_property.c b/drivers/pci/of_property.c
index 5a0b98e69795..59cc5c851fc3 100644
--- a/drivers/pci/of_property.c
+++ b/drivers/pci/of_property.c
@@ -54,9 +54,13 @@ enum of_pci_prop_compatible {
 static void of_pci_set_address(struct pci_dev *pdev, u32 *prop, u64 addr,
 			       u32 reg_num, u32 flags, bool reloc)
 {
-	prop[0] = FIELD_PREP(OF_PCI_ADDR_FIELD_BUS, pdev->bus->number) |
-		FIELD_PREP(OF_PCI_ADDR_FIELD_DEV, PCI_SLOT(pdev->devfn)) |
-		FIELD_PREP(OF_PCI_ADDR_FIELD_FUNC, PCI_FUNC(pdev->devfn));
+	if (pdev)
+		prop[0] = FIELD_PREP(OF_PCI_ADDR_FIELD_BUS, pdev->bus->number) |
+			  FIELD_PREP(OF_PCI_ADDR_FIELD_DEV, PCI_SLOT(pdev->devfn)) |
+			  FIELD_PREP(OF_PCI_ADDR_FIELD_FUNC, PCI_FUNC(pdev->devfn));
+	else
+		prop[0] = 0;
+
 	prop[0] |= flags | reg_num;
 	if (!reloc) {
 		prop[0] |= OF_PCI_ADDR_FIELD_NONRELOC;
-- 
2.46.2

[PATCH 4/6] PCI: of_property: Constify parameter in of_pci_get_addr_flags()

[Herve Codina]

The res parameter has no reason to be a pointer to an un-const struct
resource. Indeed, struct resource is not supposed to be modified by the
function.

Constify the res parameter.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/pci/of_property.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/of_property.c b/drivers/pci/of_property.c
index 59cc5c851fc3..e56159cc48e8 100644
--- a/drivers/pci/of_property.c
+++ b/drivers/pci/of_property.c
@@ -69,7 +69,7 @@ static void of_pci_set_address(struct pci_dev *pdev, u32 *prop, u64 addr,
 	}
 }
 
-static int of_pci_get_addr_flags(struct resource *res, u32 *flags)
+static int of_pci_get_addr_flags(const struct resource *res, u32 *flags)
 {
 	u32 ss;
 
-- 
2.46.2

[PATCH 5/6] of: Use the standards compliant default address cells value for x86

[Herve Codina]

The default address cells value is 1.

According to the devicetree specification and the OpenFirmware standard
(IEEE 1275-1994) this default value should be 2.

The device tree compiler already use 2 as default value and the powerpc
PROM code also use 2 as default value. Modern implementation should have
the #address-cells property set and should not rely on this default
value.

On x86, an empty root device-tree node is created but as the hardware
is not described by a device-tree passed by the bootloader, this root
device-tree remains empty and so the #address-cells default value is
used.

In preparation of the support for device-tree overlay on PCI devices
feature on x86 (i.e. the creation of the PCI root bus device-tree node),
this default value needs to be updated. Indeed, on x86_64, addresses are
on 64bits and the upper part of an address is needed for correct address
translations. On x86_32 having the default value updated does not lead
to issues while the uppert part of a 64bits address is zero.

Changing the default value for all architectures may break device-tree
compatibility. Indeed, existing dts file without the #address-cells
property set in the root node will not be compatible with this
modification. Restrict the modification to the x86 architecture.

Instead of having 1 for this default value, be consistent with standards
and set the default address cells value to 2 in case of x86.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/of/of_private.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 04aa2a91f851..d8353f04af0a 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -29,7 +29,7 @@ struct alias_prop {
 	char stem[];
 };
 
-#if defined(CONFIG_SPARC)
+#if defined(CONFIG_SPARC) || defined(CONFIG_X86)
 #define OF_ROOT_NODE_ADDR_CELLS_DEFAULT 2
 #else
 #define OF_ROOT_NODE_ADDR_CELLS_DEFAULT 1
-- 
2.46.2

Re: [PATCH 5/6] of: Use the standards compliant default address cells value for x86

[Rob Herring]

On Mon, Nov 4, 2024 at 11:20 AM Herve Codina <herve.codina@bootlin.com> wrote:
>
> The default address cells value is 1.
>
> According to the devicetree specification and the OpenFirmware standard
> (IEEE 1275-1994) this default value should be 2.
>
> The device tree compiler already use 2 as default value and the powerpc
> PROM code also use 2 as default value. Modern implementation should have
> the #address-cells property set and should not rely on this default
> value.

It's a mess.

Relying on defaults and inheriting from parent nodes in dtc has been a
warning as far back as I could trace (2005-2006).


> On x86, an empty root device-tree node is created but as the hardware
> is not described by a device-tree passed by the bootloader, this root
> device-tree remains empty and so the #address-cells default value is
> used.
>
> In preparation of the support for device-tree overlay on PCI devices
> feature on x86 (i.e. the creation of the PCI root bus device-tree node),
> this default value needs to be updated. Indeed, on x86_64, addresses are
> on 64bits and the upper part of an address is needed for correct address
> translations. On x86_32 having the default value updated does not lead
> to issues while the uppert part of a 64bits address is zero.
>
> Changing the default value for all architectures may break device-tree
> compatibility. Indeed, existing dts file without the #address-cells
> property set in the root node will not be compatible with this
> modification. Restrict the modification to the x86 architecture.
>
> Instead of having 1 for this default value, be consistent with standards
> and set the default address cells value to 2 in case of x86.

I prefer the default to just be wrong. My intention is to get rid of
the defaults (at least for all FDT, not OF) and walking up parent
nodes. My first step was to add warnings[1] and see who complained.

The base tree needs to be populated with #address-cells/#size-cells.
I'd be fine if we say those are always 2 to keep it simple.

Rob

[1] https://lore.kernel.org/all/20240530185049.2851617-1-robh@kernel.org/

Re: [PATCH 5/6] of: Use the standards compliant default address cells value for x86

[Herve Codina]

On Mon, 4 Nov 2024 14:07:59 -0600
Rob Herring <robh@kernel.org> wrote:

...
> 
> I prefer the default to just be wrong. My intention is to get rid of
> the defaults (at least for all FDT, not OF) and walking up parent
> nodes. My first step was to add warnings[1] and see who complained.
> 
> The base tree needs to be populated with #address-cells/#size-cells.
> I'd be fine if we say those are always 2 to keep it simple.
> 

Right in that case, I could add the #address-cells/#size-cells in the empty
node (empty_node.dts) used on all architecture when ACPI is enabled or the
bootloader doesn't provide a fdt.

Best regards,
Hervé

[PATCH 6/6] PCI: of: Create device-tree root bus node

[Herve Codina]

PCI devices device-tree nodes can be already created. This was
introduced by commit 407d1a51921e ("PCI: Create device tree node for
bridge").

In order to have device-tree nodes related to PCI devices attached on
their PCI root bus, a root bus device-tree node is needed. This root bus
node will be used as the parent node of the first level devices scanned
on the bus.

On non device-tree based system (such as ACPI), a device-tree node for
the PCI root bus does not exist. Indeed, this component is not described
in a device-tree used at boot.

The device-tree PCI root bus node creation needs to be done at runtime.
This is done in the same way as for the creation of the PCI device
nodes. I.e. node and properties are created based on computed
information done by the PCI core.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/pci/of.c          |  85 +++++++++++++++++++++++++++++++-
 drivers/pci/of_property.c | 101 ++++++++++++++++++++++++++++++++++++++
 drivers/pci/pci.h         |   6 +++
 drivers/pci/probe.c       |   2 +
 drivers/pci/remove.c      |   2 +
 5 files changed, 195 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 141ffbb1b3e6..46733a293c3f 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -726,7 +726,90 @@ void of_pci_make_dev_node(struct pci_dev *pdev)
 out_free_name:
 	kfree(name);
 }
-#endif
+
+void of_pci_remove_root_bus_node(struct pci_bus *bus)
+{
+	struct device_node *np;
+
+	np = pci_bus_to_OF_node(bus);
+	if (!np || !of_node_check_flag(np, OF_DYNAMIC))
+		return;
+
+	device_remove_of_node(&bus->dev);
+	of_changeset_revert(np->data);
+	of_changeset_destroy(np->data);
+	of_node_put(np);
+}
+
+void of_pci_make_root_bus_node(struct pci_bus *bus)
+{
+	struct device_node *np = NULL;
+	struct of_changeset *cset;
+	const char *name;
+	int ret;
+
+	/*
+	 * If there is already a device tree node linked to this device,
+	 * return immediately.
+	 */
+	if (pci_bus_to_OF_node(bus))
+		return;
+
+	/* Check if there is a DT root node to attach this created node */
+	if (!of_root) {
+		pr_err("of_root node is NULL, cannot create PCI root bus node");
+		return;
+	}
+
+	name = kasprintf(GFP_KERNEL, "pci-root@%x,%x", pci_domain_nr(bus),
+			 bus->number);
+	if (!name)
+		return;
+
+	cset = kmalloc(sizeof(*cset), GFP_KERNEL);
+	if (!cset)
+		goto out_free_name;
+	of_changeset_init(cset);
+
+	np = of_changeset_create_node(cset, of_root, name);
+	if (!np)
+		goto out_destroy_cset;
+
+	ret = of_pci_add_root_bus_properties(bus, cset, np);
+	if (ret)
+		goto out_free_node;
+
+	/*
+	 * This of_node will be added to an existing device. The of_node parent
+	 * is the root OF node and so this node will be handled by the platform
+	 * bus. Avoid any new device creation.
+	 */
+	of_node_set_flag(np, OF_POPULATED);
+	np->fwnode.dev = &bus->dev;
+	fwnode_dev_initialized(&np->fwnode, true);
+
+	ret = of_changeset_apply(cset);
+	if (ret)
+		goto out_free_node;
+
+	np->data = cset;
+
+	/* Add the of_node to the existing device */
+	device_add_of_node(&bus->dev, np);
+	kfree(name);
+
+	return;
+
+out_free_node:
+	of_node_put(np);
+out_destroy_cset:
+	of_changeset_destroy(cset);
+	kfree(cset);
+out_free_name:
+	kfree(name);
+}
+
+#endif /* CONFIG_PCI_DYNAMIC_OF_NODES */
 
 #endif /* CONFIG_PCI */
 
diff --git a/drivers/pci/of_property.c b/drivers/pci/of_property.c
index e56159cc48e8..527fc51565f3 100644
--- a/drivers/pci/of_property.c
+++ b/drivers/pci/of_property.c
@@ -394,3 +394,104 @@ int of_pci_add_properties(struct pci_dev *pdev, struct of_changeset *ocs,
 
 	return 0;
 }
+
+static bool of_pci_is_range_resource(const struct resource *res, u32 *flags)
+{
+	if (!(resource_type(res) & IORESOURCE_MEM) &&
+	    !(resource_type(res) & IORESOURCE_MEM_64))
+		return false;
+
+	if (of_pci_get_addr_flags(res, flags))
+		return false;
+
+	return true;
+}
+
+static int of_pci_root_bus_prop_ranges(struct pci_bus *bus,
+				       struct of_changeset *ocs,
+				       struct device_node *np)
+{
+	struct pci_host_bridge *bridge = to_pci_host_bridge(bus->bridge);
+	struct resource_entry *window;
+	unsigned int ranges_sz = 0;
+	unsigned int n_range = 0;
+	struct resource *res;
+	int n_addr_cells;
+	u32 *ranges;
+	u64 val64;
+	u32 flags;
+	int ret;
+
+	n_addr_cells = of_n_addr_cells(np);
+	if (n_addr_cells <= 0 || n_addr_cells > 2)
+		return -EINVAL;
+
+	resource_list_for_each_entry(window, &bridge->windows) {
+		res = window->res;
+		if (!of_pci_is_range_resource(res, &flags))
+			continue;
+		n_range++;
+	}
+
+	if (!n_range)
+		return 0;
+
+	ranges = kcalloc(n_range,
+			 (OF_PCI_ADDRESS_CELLS + OF_PCI_SIZE_CELLS +
+			  n_addr_cells) * sizeof(*ranges),
+			 GFP_KERNEL);
+	if (!ranges)
+		return -ENOMEM;
+
+	resource_list_for_each_entry(window, &bridge->windows) {
+		res = window->res;
+		if (!of_pci_is_range_resource(res, &flags))
+			continue;
+
+		/* PCI bus address */
+		val64 = res->start;
+		of_pci_set_address(NULL, &ranges[ranges_sz], val64, 0, flags, false);
+		ranges_sz += OF_PCI_ADDRESS_CELLS;
+
+		/* Host bus address */
+		if (n_addr_cells == 2)
+			ranges[ranges_sz++] = upper_32_bits(val64);
+		ranges[ranges_sz++] = lower_32_bits(val64);
+
+		/* Size */
+		val64 = resource_size(res);
+		ranges[ranges_sz] = upper_32_bits(val64);
+		ranges[ranges_sz + 1] = lower_32_bits(val64);
+		ranges_sz += OF_PCI_SIZE_CELLS;
+	}
+
+	ret = of_changeset_add_prop_u32_array(ocs, np, "ranges", ranges, ranges_sz);
+	kfree(ranges);
+	return ret;
+}
+
+int of_pci_add_root_bus_properties(struct pci_bus *bus, struct of_changeset *ocs,
+				   struct device_node *np)
+{
+	int ret;
+
+	ret = of_changeset_add_prop_string(ocs, np, "device_type", "pci");
+	if (ret)
+		return ret;
+
+	ret = of_changeset_add_prop_u32(ocs, np, "#address-cells",
+					OF_PCI_ADDRESS_CELLS);
+	if (ret)
+		return ret;
+
+	ret = of_changeset_add_prop_u32(ocs, np, "#size-cells",
+					OF_PCI_SIZE_CELLS);
+	if (ret)
+		return ret;
+
+	ret = of_pci_root_bus_prop_ranges(bus, ocs, np);
+	if (ret)
+		return ret;
+
+	return 0;
+}
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 14d00ce45bfa..56e450807d5d 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -802,9 +802,15 @@ void of_pci_make_dev_node(struct pci_dev *pdev);
 void of_pci_remove_node(struct pci_dev *pdev);
 int of_pci_add_properties(struct pci_dev *pdev, struct of_changeset *ocs,
 			  struct device_node *np);
+void of_pci_make_root_bus_node(struct pci_bus *bus);
+void of_pci_remove_root_bus_node(struct pci_bus *bus);
+int of_pci_add_root_bus_properties(struct pci_bus *bus, struct of_changeset *ocs,
+				   struct device_node *np);
 #else
 static inline void of_pci_make_dev_node(struct pci_dev *pdev) { }
 static inline void of_pci_remove_node(struct pci_dev *pdev) { }
+static inline void of_pci_make_root_bus_node(struct pci_bus *bus) { }
+static inline void of_pci_remove_root_bus_node(struct pci_bus *bus) { }
 #endif
 
 #ifdef CONFIG_PCIEAER
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 4f68414c3086..063780bec45e 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1049,6 +1049,8 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge)
 		dev_info(&bus->dev, "root bus resource %pR%s\n", res, addr);
 	}
 
+	of_pci_make_root_bus_node(bus);
+
 	down_write(&pci_bus_sem);
 	list_add_tail(&bus->node, &pci_root_buses);
 	up_write(&pci_bus_sem);
diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index e4ce1145aa3e..80cbe02f66b2 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -160,6 +160,8 @@ void pci_stop_root_bus(struct pci_bus *bus)
 					 &bus->devices, bus_list)
 		pci_stop_bus_device(child);
 
+	of_pci_remove_root_bus_node(bus);
+
 	/* stop the host bridge */
 	device_release_driver(&host_bridge->dev);
 }
-- 
2.46.2

Re: [PATCH 6/6] PCI: of: Create device-tree root bus node

[Bjorn Helgaas]

On Mon, Nov 04, 2024 at 06:20:00PM +0100, Herve Codina wrote:
> PCI devices device-tree nodes can be already created. This was
> introduced by commit 407d1a51921e ("PCI: Create device tree node for
> bridge").

I guess 407d1a51921e creates device tree nodes for bridges, including
Root Ports, which are enumerated as PCI-to-PCI bridges, right?

> In order to have device-tree nodes related to PCI devices attached on
> their PCI root bus, a root bus device-tree node is needed. This root bus
> node will be used as the parent node of the first level devices scanned
> on the bus.
>
> On non device-tree based system (such as ACPI), a device-tree node for
> the PCI root bus does not exist.  ...

I'm wondering if "root bus" is the right description for this patch
and whether "PCI host bridge" might be more accurate.  The bus itself
doesn't really have a physical counterpart other than being the
secondary side of a PCI host bridge where the primary side is some
kind of CPU bus.

An ACPI namespace doesn't include a "root bus" object, but it *does*
include a PCI host bridge (PNP0A03) object, which is where any address
translation between the CPU bus and the PCI hierarchy is described.

I suspect this patch is adding a DT node that corresponds to the
PNP0A03 host bridge object, and the "ranges" property of the new node
will describe the mapping from the CPU address space to the PCI
address space.

> Indeed, this component is not described
> in a device-tree used at boot.

But maybe I'm on the wrong track, because obviously PCI host
controllers *are* described in DTs used at boot.

> The device-tree PCI root bus node creation needs to be done at runtime.
> This is done in the same way as for the creation of the PCI device
> nodes. I.e. node and properties are created based on computed
> information done by the PCI core.

See address translation question below.

> +void of_pci_make_root_bus_node(struct pci_bus *bus)
> +{
> +	struct device_node *np = NULL;
> +	struct of_changeset *cset;
> +	const char *name;
> +	int ret;
> +
> +	/*
> +	 * If there is already a device tree node linked to this device,
> +	 * return immediately.
> +	 */
> +	if (pci_bus_to_OF_node(bus))
> +		return;
> +
> +	/* Check if there is a DT root node to attach this created node */
> +	if (!of_root) {
> +		pr_err("of_root node is NULL, cannot create PCI root bus node");
> +		return;
> +	}
> +
> +	name = kasprintf(GFP_KERNEL, "pci-root@%x,%x", pci_domain_nr(bus),
> +			 bus->number);

Should this be "pci%d@%x,%x" to match the typical descriptions of PCI
host bridges in DT?

> +static int of_pci_root_bus_prop_ranges(struct pci_bus *bus,
> +				       struct of_changeset *ocs,
> +				       struct device_node *np)
> +{
> +	struct pci_host_bridge *bridge = to_pci_host_bridge(bus->bridge);
> +	struct resource_entry *window;
> +	unsigned int ranges_sz = 0;
> +	unsigned int n_range = 0;
> +	struct resource *res;
> +	int n_addr_cells;
> +	u32 *ranges;
> +	u64 val64;
> +	u32 flags;
> +	int ret;
> +
> +	n_addr_cells = of_n_addr_cells(np);
> +	if (n_addr_cells <= 0 || n_addr_cells > 2)
> +		return -EINVAL;
> +
> +	resource_list_for_each_entry(window, &bridge->windows) {
> +		res = window->res;
> +		if (!of_pci_is_range_resource(res, &flags))
> +			continue;
> +		n_range++;
> +	}
> +
> +	if (!n_range)
> +		return 0;
> +
> +	ranges = kcalloc(n_range,
> +			 (OF_PCI_ADDRESS_CELLS + OF_PCI_SIZE_CELLS +
> +			  n_addr_cells) * sizeof(*ranges),
> +			 GFP_KERNEL);
> +	if (!ranges)
> +		return -ENOMEM;
> +
> +	resource_list_for_each_entry(window, &bridge->windows) {
> +		res = window->res;
> +		if (!of_pci_is_range_resource(res, &flags))
> +			continue;
> +
> +		/* PCI bus address */
> +		val64 = res->start;
> +		of_pci_set_address(NULL, &ranges[ranges_sz], val64, 0, flags, false);
> +		ranges_sz += OF_PCI_ADDRESS_CELLS;
> +
> +		/* Host bus address */
> +		if (n_addr_cells == 2)
> +			ranges[ranges_sz++] = upper_32_bits(val64);
> +		ranges[ranges_sz++] = lower_32_bits(val64);

IIUC this sets both the parent address (the host bus (CPU) physical
address) and the child address (PCI bus address) to the same value.

I think that's wrong because these addresses need not be identical.

I think the parent address should be the res->start value, and the
child address should be "res->start - window->offset", similar to
what's done by pcibios_resource_to_bus().

> +		/* Size */
> +		val64 = resource_size(res);
> +		ranges[ranges_sz] = upper_32_bits(val64);
> +		ranges[ranges_sz + 1] = lower_32_bits(val64);
> +		ranges_sz += OF_PCI_SIZE_CELLS;
> +	}
> +
> +	ret = of_changeset_add_prop_u32_array(ocs, np, "ranges", ranges, ranges_sz);
> +	kfree(ranges);
> +	return ret;
> +}

Re: [PATCH 6/6] PCI: of: Create device-tree root bus node

[Herve Codina]

On Tue, 5 Nov 2024 12:59:01 -0600
Bjorn Helgaas <helgaas@kernel.org> wrote:

> On Mon, Nov 04, 2024 at 06:20:00PM +0100, Herve Codina wrote:
> > PCI devices device-tree nodes can be already created. This was
> > introduced by commit 407d1a51921e ("PCI: Create device tree node for
> > bridge").  
> 
> I guess 407d1a51921e creates device tree nodes for bridges, including
> Root Ports, which are enumerated as PCI-to-PCI bridges, right?

It creates DT nodes for bridges and devices.

> 
> > In order to have device-tree nodes related to PCI devices attached on
> > their PCI root bus, a root bus device-tree node is needed. This root bus
> > node will be used as the parent node of the first level devices scanned
> > on the bus.
> >
> > On non device-tree based system (such as ACPI), a device-tree node for
> > the PCI root bus does not exist.  ...  
> 
> I'm wondering if "root bus" is the right description for this patch
> and whether "PCI host bridge" might be more accurate.  The bus itself
> doesn't really have a physical counterpart other than being the
> secondary side of a PCI host bridge where the primary side is some
> kind of CPU bus.

Indeed, you're right. I will rename "root bus" to "host bridge" everywhere
in the patch (description but also the code itself).

> 
> An ACPI namespace doesn't include a "root bus" object, but it *does*
> include a PCI host bridge (PNP0A03) object, which is where any address
> translation between the CPU bus and the PCI hierarchy is described.
> 
> I suspect this patch is adding a DT node that corresponds to the
> PNP0A03 host bridge object, and the "ranges" property of the new node
> will describe the mapping from the CPU address space to the PCI
> address space.

Exactly.

> 
> > Indeed, this component is not described
> > in a device-tree used at boot.  
> 
> But maybe I'm on the wrong track, because obviously PCI host
> controllers *are* described in DTs used at boot.

They are described in a device-tree used at boot on device-tree based
systems.
On x86, we are on ACPI based system -> No DT used at boot -> PCI host
controller not described in DT.

I could replace with "Indeed, this component is not described in a
device-tree used at boot because, in that case, device-tree is not
used to describe the hardware"

> 
> > The device-tree PCI root bus node creation needs to be done at runtime.
> > This is done in the same way as for the creation of the PCI device
> > nodes. I.e. node and properties are created based on computed
> > information done by the PCI core.  
> 
> See address translation question below.
> 
> > +void of_pci_make_root_bus_node(struct pci_bus *bus)
> > +{
> > +	struct device_node *np = NULL;
> > +	struct of_changeset *cset;
> > +	const char *name;
> > +	int ret;
> > +
> > +	/*
> > +	 * If there is already a device tree node linked to this device,
> > +	 * return immediately.
> > +	 */
> > +	if (pci_bus_to_OF_node(bus))
> > +		return;
> > +
> > +	/* Check if there is a DT root node to attach this created node */
> > +	if (!of_root) {
> > +		pr_err("of_root node is NULL, cannot create PCI root bus node");
> > +		return;
> > +	}
> > +
> > +	name = kasprintf(GFP_KERNEL, "pci-root@%x,%x", pci_domain_nr(bus),
> > +			 bus->number);  
> 
> Should this be "pci%d@%x,%x" to match the typical descriptions of PCI
> host bridges in DT?

What do you think I should use for the %d you proposed.
Also I supposed your "@%x,%x" is still pci_domain_nr(bus), bus->number.

> 
> > +static int of_pci_root_bus_prop_ranges(struct pci_bus *bus,
> > +				       struct of_changeset *ocs,
> > +				       struct device_node *np)
> > +{
> > +	struct pci_host_bridge *bridge = to_pci_host_bridge(bus->bridge);
> > +	struct resource_entry *window;
> > +	unsigned int ranges_sz = 0;
> > +	unsigned int n_range = 0;
> > +	struct resource *res;
> > +	int n_addr_cells;
> > +	u32 *ranges;
> > +	u64 val64;
> > +	u32 flags;
> > +	int ret;
> > +
> > +	n_addr_cells = of_n_addr_cells(np);
> > +	if (n_addr_cells <= 0 || n_addr_cells > 2)
> > +		return -EINVAL;
> > +
> > +	resource_list_for_each_entry(window, &bridge->windows) {
> > +		res = window->res;
> > +		if (!of_pci_is_range_resource(res, &flags))
> > +			continue;
> > +		n_range++;
> > +	}
> > +
> > +	if (!n_range)
> > +		return 0;
> > +
> > +	ranges = kcalloc(n_range,
> > +			 (OF_PCI_ADDRESS_CELLS + OF_PCI_SIZE_CELLS +
> > +			  n_addr_cells) * sizeof(*ranges),
> > +			 GFP_KERNEL);
> > +	if (!ranges)
> > +		return -ENOMEM;
> > +
> > +	resource_list_for_each_entry(window, &bridge->windows) {
> > +		res = window->res;
> > +		if (!of_pci_is_range_resource(res, &flags))
> > +			continue;
> > +
> > +		/* PCI bus address */
> > +		val64 = res->start;
> > +		of_pci_set_address(NULL, &ranges[ranges_sz], val64, 0, flags, false);
> > +		ranges_sz += OF_PCI_ADDRESS_CELLS;
> > +
> > +		/* Host bus address */
> > +		if (n_addr_cells == 2)
> > +			ranges[ranges_sz++] = upper_32_bits(val64);
> > +		ranges[ranges_sz++] = lower_32_bits(val64);  
> 
> IIUC this sets both the parent address (the host bus (CPU) physical
> address) and the child address (PCI bus address) to the same value.
> 
> I think that's wrong because these addresses need not be identical.
> 
> I think the parent address should be the res->start value, and the
> child address should be "res->start - window->offset", similar to
> what's done by pcibios_resource_to_bus().

I see.
I will update in the next iteration.

Thanks for your feedback.
Best regards,
Hervé

Re: [PATCH 6/6] PCI: of: Create device-tree root bus node

[Bjorn Helgaas]

On Wed, Nov 06, 2024 at 03:53:53PM +0100, Herve Codina wrote:
> On Tue, 5 Nov 2024 12:59:01 -0600
> Bjorn Helgaas <helgaas@kernel.org> wrote:
> > On Mon, Nov 04, 2024 at 06:20:00PM +0100, Herve Codina wrote:
> > > PCI devices device-tree nodes can be already created. This was
> > > introduced by commit 407d1a51921e ("PCI: Create device tree node for
> > > bridge").  
> ...

> > > Indeed, this component is not described
> > > in a device-tree used at boot.  
> > 
> > But maybe I'm on the wrong track, because obviously PCI host
> > controllers *are* described in DTs used at boot.
> 
> They are described in a device-tree used at boot on device-tree based
> systems.
> On x86, we are on ACPI based system -> No DT used at boot -> PCI host
> controller not described in DT.

Right, I was thinking of the devicetree-based systems, where the host
controller must be described in DT.

> > > +	name = kasprintf(GFP_KERNEL, "pci-root@%x,%x", pci_domain_nr(bus),
> > > +			 bus->number);  
> > 
> > Should this be "pci%d@%x,%x" to match the typical descriptions of PCI
> > host bridges in DT?
> 
> What do you think I should use for the %d you proposed.

Based on the .dts files, I think the %d is just an index to
distinguish multiple PCI host bridges.  Maybe that's not relevant
here, I dunno.

> Also I supposed your "@%x,%x" is still pci_domain_nr(bus), bus->number.

Yes.  I think we're basically constructing a DT node to correspond to
an ACPI PNP0A03 device.  ACPI does support updating the root bus
number via _CRS/_PRS/_SRS, but Linux doesn't have support for that, so
the root bus number is basically constant.  The pci_domain_nr(bus)
should be coming from _SEG, and that's definitely constant.

Bjorn

Re: [PATCH 0/6] Add support for the root PCI bus device-tree node creation.

[Rob Herring]

On Mon, Nov 04, 2024 at 06:19:54PM +0100, Herve Codina wrote:
> Hi,
> 
> This series adds support for creating a device-tree node for a root PCI
> bus on non device-tree based system.
> 
> Creating device-tree nodes for PCI devices already exists upstream. It
> was added in commit 407d1a51921e ("PCI: Create device tree node for
> bridge"). Created device-tree nodes need a parent node to be attached
> to. For the first level devices, on device-tree based system, this
> parent node (i.e. the root PCI bus) is described in the base device-tree
> (PCI controller).
> 
> The LAN966x PCI device driver was recently accepted [1] and relies on
> this feature.
> 
> On system where the base hardware is not described by a device-tree, the
> root PCI bus node to which first level created PCI devices need to be
> attach to does not exist. This is the case for instance on ACPI
> described systems such as x86.
> 
> This series goal is to handle this case.
> 
> In order to have the root PCI bus device-tree node available even on
> x86, this top level node is created (if not already present) based on
> information computed by the PCI core. It follows the same mechanism as
> the one used for PCI devices device-tree node creation.
> 
> In order to have this feature available, a number of changes are needed:
>   - Patch 1 and 2: Introduce and use device_{add,remove}_of_node().
>     This function will also be used in the root PCI bus node creation.
> 
>   - Patch 3 and 4: Improve existing functions to reuse them in the root
>     PCI bus node creation.
> 
>   - Patch 5: Update the default value used when #address-cells is not
>     available in the device-tree root node.
> 
>   - Patch 6: The root PCI bus device-tree node creation itself.
> 
> With those modifications, the LAN966x PCI device is working on x86 systems.

That's nice, but I don't have a LAN966x device nor do I want one. We 
already have the QEMU PCI test device working with the existing PCI 
support. Please ensure this series works with it as well.

Rob

Re: [PATCH 0/6] Add support for the root PCI bus device-tree node creation.

[Herve Codina]

Hi Rob,

On Mon, 4 Nov 2024 14:15:07 -0600
Rob Herring <robh@kernel.org> wrote:

...
> > With those modifications, the LAN966x PCI device is working on x86 systems.  
> 
> That's nice, but I don't have a LAN966x device nor do I want one. We 
> already have the QEMU PCI test device working with the existing PCI 
> support. Please ensure this series works with it as well.
> 

I will check.

Can you confirm that you are talking about this test:
  https://elixir.bootlin.com/linux/v6.12-rc6/source/drivers/of/unittest.c#L4188

The test needs QEMU with a specific setup and I found this entry point:
  https://lore.kernel.org/all/fa208013-7bf8-80fc-2732-814f380cebf9@amd.com/

Do you have an "official" QEMU setup on your side to run the test or any
other pointers related to the QEMU command/setup you use?

Best regards,
Hervé

Re: [PATCH 0/6] Add support for the root PCI bus device-tree node creation.

[Rob Herring]

On Tue, Nov 5, 2024 at 11:44 AM Herve Codina <herve.codina@bootlin.com> wrote:
>
> Hi Rob,
>
> On Mon, 4 Nov 2024 14:15:07 -0600
> Rob Herring <robh@kernel.org> wrote:
>
> ...
> > > With those modifications, the LAN966x PCI device is working on x86 systems.
> >
> > That's nice, but I don't have a LAN966x device nor do I want one. We
> > already have the QEMU PCI test device working with the existing PCI
> > support. Please ensure this series works with it as well.
> >
>
> I will check.
>
> Can you confirm that you are talking about this test:
>   https://elixir.bootlin.com/linux/v6.12-rc6/source/drivers/of/unittest.c#L4188
>
> The test needs QEMU with a specific setup and I found this entry point:
>   https://lore.kernel.org/all/fa208013-7bf8-80fc-2732-814f380cebf9@amd.com/

Yes, that's it.

> Do you have an "official" QEMU setup on your side to run the test or any
> other pointers related to the QEMU command/setup you use?

No, it's just something based on what you linked. Here's what I have:

qemu-system-aarch64 -machine virt -cpu cortex-a72 -machine type=virt
-nographic -smp 1 -m 2048 -kernel ../linux.git/arch/arm64/boot/Image
--append console=ttyAMA0 -device
pcie-root-port,port=0x10,chassis=9,id=pci.9,bus=pcie.0,multifunction=on,addr=0x3
-device pcie-root-port,port=0x11,chassis=10,id=pci.10,bus=pcie.0,addr=0x3.0x1
-device x3130-upstream,id=pci.11,bus=pci.9,addr=0x0 -device
xio3130-downstream,port=0x0,chassis=11,id=pci.12,bus=pci.11,multifunction=on,addr=0x0
-device i82801b11-bridge,id=pci.13,bus=pcie.0,addr=0x4 -device
pci-bridge,chassis_nr=14,id=pci.14,bus=pci.13,addr=0x0 -device
pci-testdev,bus=pci.12,addr=0x0

Of course, you'll need a few changes to use ACPI.

Rob

Re: [PATCH 0/6] Add support for the root PCI bus device-tree node creation.

[Herve Codina]

Hi Rob,

On Tue, 5 Nov 2024 13:59:40 -0600
Rob Herring <robh@kernel.org> wrote:

> On Tue, Nov 5, 2024 at 11:44 AM Herve Codina <herve.codina@bootlin.com> wrote:
> >
> > Hi Rob,
> >
> > On Mon, 4 Nov 2024 14:15:07 -0600
> > Rob Herring <robh@kernel.org> wrote:
> >
> > ...  
> > > > With those modifications, the LAN966x PCI device is working on x86 systems.  
> > >
> > > That's nice, but I don't have a LAN966x device nor do I want one. We
> > > already have the QEMU PCI test device working with the existing PCI
> > > support. Please ensure this series works with it as well.
> > >  
> >
> > I will check.
> >
> > Can you confirm that you are talking about this test:
> >   https://elixir.bootlin.com/linux/v6.12-rc6/source/drivers/of/unittest.c#L4188
> >
> > The test needs QEMU with a specific setup and I found this entry point:
> >   https://lore.kernel.org/all/fa208013-7bf8-80fc-2732-814f380cebf9@amd.com/  
> 
> Yes, that's it.
> 
> > Do you have an "official" QEMU setup on your side to run the test or any
> > other pointers related to the QEMU command/setup you use?  
> 
> No, it's just something based on what you linked. Here's what I have:
> 
> qemu-system-aarch64 -machine virt -cpu cortex-a72 -machine type=virt
> -nographic -smp 1 -m 2048 -kernel ../linux.git/arch/arm64/boot/Image
> --append console=ttyAMA0 -device
> pcie-root-port,port=0x10,chassis=9,id=pci.9,bus=pcie.0,multifunction=on,addr=0x3
> -device pcie-root-port,port=0x11,chassis=10,id=pci.10,bus=pcie.0,addr=0x3.0x1
> -device x3130-upstream,id=pci.11,bus=pci.9,addr=0x0 -device
> xio3130-downstream,port=0x0,chassis=11,id=pci.12,bus=pci.11,multifunction=on,addr=0x0
> -device i82801b11-bridge,id=pci.13,bus=pcie.0,addr=0x4 -device
> pci-bridge,chassis_nr=14,id=pci.14,bus=pci.13,addr=0x0 -device
> pci-testdev,bus=pci.12,addr=0x0
> 
> Of course, you'll need a few changes to use ACPI.
> 

I ran the OF kunit tests with the following command:
qemu-system-x86_64 -machine q35 -nographic \
	-kernel arch/x86_64/boot/bzImage --append console=ttyS0 \
	-device pcie-root-port,port=0x10,chassis=9,id=pci.9,bus=pcie.0,multifunction=on,addr=0x3 \
	-device pcie-root-port,port=0x11,chassis=10,id=pci.10,bus=pcie.0,addr=0x3.0x1 \
	-device x3130-upstream,id=pci.11,bus=pci.9,addr=0x0 \
	-device xio3130-downstream,port=0x0,chassis=11,id=pci.12,bus=pci.11,multifunction=on,addr=0x0 \
	-device i82801b11-bridge,id=pci.13,bus=pcie.0,addr=0x4 \
	-device pci-bridge,chassis_nr=14,id=pci.14,bus=pci.13,addr=0x0 \
	-device pci-testdev,bus=pci.12,addr=0x0

Same -device options as the ones present in your command.

Tests are successful:
  [    0.000000] DMI: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-1.fc39 04/01/2014
  ...
  [    0.030500] ACPI: Early table checksum verification disabled
  [    0.030771] ACPI: RSDP 0x00000000000F5250 000014 (v00 BOCHS )
  [    0.030993] ACPI: RSDT 0x0000000007FE4068 000038 (v01 BOCHS  BXPC     00000001 BXPC 00000001)
  [    0.031438] ACPI: FACP 0x0000000007FE3E60 0000F4 (v03 BOCHS  BXPC     00000001 BXPC 00000001)
  [    0.031920] ACPI: DSDT 0x0000000007FE0040 003E20 (v01 BOCHS  BXPC     00000001 BXPC 00000001)
  [    0.031993] ACPI: FACS 0x0000000007FE0000 000040
  [    0.032036] ACPI: APIC 0x0000000007FE3F54 000078 (v03 BOCHS  BXPC     00000001 BXPC 00000001)
  [    0.032060] ACPI: HPET 0x0000000007FE3FCC 000038 (v01 BOCHS  BXPC     00000001 BXPC 00000001)
  [    0.032084] ACPI: MCFG 0x0000000007FE4004 00003C (v01 BOCHS  BXPC     00000001 BXPC 00000001)
  [    0.032105] ACPI: WAET 0x0000000007FE4040 000028 (v01 BOCHS  BXPC     00000001 BXPC 00000001)
  [    0.032171] ACPI: Reserving FACP table memory at [mem 0x7fe3e60-0x7fe3f53]
  [    0.032206] ACPI: Reserving DSDT table memory at [mem 0x7fe0040-0x7fe3e5f]
  [    0.032215] ACPI: Reserving FACS table memory at [mem 0x7fe0000-0x7fe003f]
  [    0.032220] ACPI: Reserving APIC table memory at [mem 0x7fe3f54-0x7fe3fcb]
  [    0.032226] ACPI: Reserving HPET table memory at [mem 0x7fe3fcc-0x7fe4003]
  [    0.032231] ACPI: Reserving MCFG table memory at [mem 0x7fe4004-0x7fe403f]
  [    0.032236] ACPI: Reserving WAET table memory at [mem 0x7fe4040-0x7fe4067]
  ...
  [    3.466693] ### dt-test ### pass of_unittest_pci_node():4202
  [    3.466887] ### dt-test ### pass of_unittest_pci_node_verify():4155
  [    3.467133] ### dt-test ### pass of_unittest_pci_node_verify():4162
  [    3.467278] ### dt-test ### pass of_unittest_pci_node_verify():4169
  [    3.467442] ### dt-test ### pass of_unittest_pci_node():4214
  [    3.467572] ### dt-test ### pass of_unittest_pci_node():4216
  [    3.469993] ### dt-test ### pass of_unittest_pci_node_verify():4155
  [    3.470273] ### dt-test ### pass of_unittest_pci_node_verify():4175
  [    3.470577] ### dt-test ### pass of_unittest_pci_node_verify():4177
  ...
  [    3.513309] ### dt-test ### end of unittest - 387 passed, 0 failed


Best regards,
Hervé