PCI: Record an fwnode associated with root PCI buses, optionally

From: Jake Oshins <[email protected]>

This patch allows a PCI front-end implementation to supply an fwnode_handle associated with a root PCI bus, optionally. If supplied, the PCI driver records this.

This patch supports the next patch in the series, which looks up an IRQ domain through this handle.

Signed-off-by Jake Oshins <[email protected]>

---
 arch/sparc/kernel/pci.c       | 2 +-
 drivers/acpi/pci_root.c       | 2 +-
 drivers/parisc/lba_pci.c      | 2 +-
 drivers/pci/host/pci-xgene.c  | 2 +-
 drivers/pci/host/pcie-iproc.c | 3 ++-
 drivers/pci/probe.c           | 8 +++++---
 include/linux/pci.h           | 4 +++-
 7 files changed, 14 insertions(+), 9 deletions(-)

arch/sparc/kernel/pci.c +1 -1

index b91d7f1..3d4e9f9 100644
@@ -660,7 +660,7 @@ struct pci_bus *pci_scan_one_pbm(struct pci_pbm_info *pbm,
 	pbm->busn.flags	= IORESOURCE_BUS;
 	pci_add_resource(&resources, &pbm->busn);
 	bus = pci_create_root_bus(parent, pbm->pci_first_busno, pbm->pci_ops,
-				  pbm, &resources);
+				  pbm, &resources, NULL);
 	if (!bus) {
 		printk(KERN_ERR "Failed to create bus for %s\n",
 		       node->full_name);

drivers/acpi/pci_root.c +1 -1

index 850d7bf..eab95bc 100644
@@ -840,7 +840,7 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
 	pci_acpi_root_add_resources(info);
 	pci_add_resource(&info->resources, &root->secondary);
 	bus = pci_create_root_bus(NULL, busnum, ops->pci_ops,
-				  sysdata, &info->resources);
+				  sysdata, &info->resources, NULL);
 	if (!bus)
 		goto out_release_info;
 

drivers/parisc/lba_pci.c +1 -1

index a32c1f6..a7b9d5c 100644
@@ -1567,7 +1567,7 @@ lba_driver_probe(struct parisc_device *dev)
 	dev->dev.platform_data = lba_dev;
 	lba_bus = lba_dev->hba.hba_bus =
 		pci_create_root_bus(&dev->dev, lba_dev->hba.bus_num.start,
-				    cfg_ops, NULL, &resources);
+				    cfg_ops, NULL, &resources, NULL);
 	if (!lba_bus) {
 		pci_free_resource_list(&resources);
 		return 0;

drivers/pci/host/pci-xgene.c +1 -1

index ae00ce2..95c20c8 100644
@@ -545,7 +545,7 @@ static int xgene_pcie_probe_bridge(struct platform_device *pdev)
 		return ret;
 
 	bus = pci_create_root_bus(&pdev->dev, 0,
-					&xgene_pcie_ops, port, &res);
+					&xgene_pcie_ops, port, &res, NULL);
 	if (!bus)
 		return -ENOMEM;
 

drivers/pci/host/pcie-iproc.c +2 -1

index 9193951..bc999b7 100644
@@ -357,7 +357,8 @@ int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res)
 	sysdata = pcie;
 #endif
 
-	bus = pci_create_root_bus(pcie->dev, 0, &iproc_pcie_ops, sysdata, res);
+	bus = pci_create_root_bus(pcie->dev, 0, &iproc_pcie_ops, sysdata, res,
+				  NULL);
 	if (!bus) {
 		dev_err(pcie->dev, "unable to create PCI root bus\n");
 		ret = -ENOMEM;

drivers/pci/probe.c +5 -3

index f441d1b..c0f2e44 100644
@@ -2039,7 +2039,8 @@ void __weak pcibios_remove_bus(struct pci_bus *bus)
 }
 
 struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
-		struct pci_ops *ops, void *sysdata, struct list_head *resources)
+		struct pci_ops *ops, void *sysdata, struct list_head *resources,
+		struct fwnode_handle *fwnode)
 {
 	int error;
 	struct pci_host_bridge *bridge;

@@ -2069,6 +2070,7 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 	if (!bridge)
 		goto err_out;
 
+	bridge->fwnode = fwnode;
 	bridge->dev.parent = parent;
 	bridge->dev.release = pci_release_host_bridge_dev;
 	dev_set_name(&bridge->dev, "pci%04x:%02x", pci_domain_nr(b), bus);

@@ -2223,7 +2225,7 @@ struct pci_bus *pci_scan_root_bus_msi(struct device *parent, int bus,
 			break;
 		}
 
-	b = pci_create_root_bus(parent, bus, ops, sysdata, resources);
+	b = pci_create_root_bus(parent, bus, ops, sysdata, resources, NULL);
 	if (!b)
 		return NULL;
 

@@ -2261,7 +2263,7 @@ struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops,
 	pci_add_resource(&resources, &ioport_resource);
 	pci_add_resource(&resources, &iomem_resource);
 	pci_add_resource(&resources, &busn_resource);
-	b = pci_create_root_bus(NULL, bus, ops, sysdata, &resources);
+	b = pci_create_root_bus(NULL, bus, ops, sysdata, &resources, NULL);
 	if (b) {
 		pci_scan_child_bus(b);
 	} else {

include/linux/pci.h +3 -1

index b54fbf1..86ce5fa 100644
@@ -409,6 +409,7 @@ struct pci_host_bridge {
 	struct device dev;
 	struct pci_bus *bus;		/* root bus */
 	struct list_head windows;	/* resource_entry */
+	struct fwnode_handle *fwnode;
 	void (*release_fn)(struct pci_host_bridge *);
 	void *release_data;
 	unsigned int ignore_reset_delay:1;	/* for entire hierarchy */

@@ -789,7 +790,8 @@ void pci_bus_add_devices(const struct pci_bus *bus);
 struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops, void *sysdata);
 struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 				    struct pci_ops *ops, void *sysdata,
-				    struct list_head *resources);
+				    struct list_head *resources,
+				    struct fwnode_handle *fwnode);
 int pci_bus_insert_busn_res(struct pci_bus *b, int bus, int busmax);
 int pci_bus_update_busn_res_end(struct pci_bus *b, int busmax);
 void pci_bus_release_busn_res(struct pci_bus *b);

-----Original Message-----
From: Jiang Liu [mailto:[email protected]]
Sent: Tuesday, October 27, 2015 12:10 AM
To: Jake Oshins <[email protected]>; [email protected]; KY Srinivasan <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Haiyang Zhang <[email protected]>; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH v3 4/7] PCI: Record an fwnode associated with root PCI buses, optionally

Quoted 3 lines · On 2015/10/27 7:15, [email protected] wrote:

From: Jake Oshins <[email protected]>

This patch allows a PCI front-end implementation to supply an

fwnode_handle

Quoted 4 lines

associated with a root PCI bus, optionally. If supplied, the PCI driver records this.

This patch supports the next patch in the series, which looks up an IRQ

domain

Quoted 1 line

through this handle.

Hi JaKeo,
Instead of changing the pci_create_root_bus() interface,
how about packing fwnode into sysdata, then we may either 1) introduce a helper to retrieve fwnode from sysdata or 2) set host_bridge->fwnode = sysdata in function pcibios_root_bridge_prepare.

Thanks,
Gerry

From: Jake Oshins <[email protected]>

The existing PCI code looks for an IRQ domain associated with a root PCI bus by looking in the Open Firmware tree. This patch introduces a second way to identify the associated IRQ domain, if the lookup in the OF tree fails. The handle used for the IRQ domain lookup was introduced in the previous patch in the series.

Signed-off-by Jake Oshins <[email protected]>

---
 drivers/pci/probe.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

drivers/pci/probe.c +1 -0

index c0f2e44..62c9ac7 100644
@@ -664,6 +664,7 @@ static void pci_set_bus_speed(struct pci_bus *bus)
 static struct irq_domain *pci_host_bridge_msi_domain(struct pci_bus *bus)
 {
 	struct irq_domain *d;
+	struct pci_host_bridge *host_bridge;
 
 	/*
 	 * Any firmware interface that can resolve the msi_domain

@@ -671,6 +672,18 @@ static struct irq_domain *pci_host_bridge_msi_domain(struct pci_bus *bus)
 	 */
 	d = pci_host_bridge_of_msi_domain(bus);

drivers/pci/probe.c +3 -0

+		}
+	}
+
 	return d;
 }
 

From: Jake Oshins <[email protected]>

This patch introduces a new driver which exposes a root PCI bus whenever a PCI Express device is passed through to a guest VM under Hyper-V. The device can be single- or multi-function. The interrupts for the devices are managed by an IRQ domain, implemented within the driver.

Signed-off-by Jake Oshins <[email protected]>

---

(quoted diff) +5 -0

+
+/**
+ * hv_pcie_init_irq_domain() - Initialize IRQ domain
+ * @hbus:	The root PCI bus
+ *

(quoted diff) +10 -0

+	hbus->irq_domain = pci_msi_create_irq_domain(hbus->fwnode,
+						     &hbus->msi_info,
+						     x86_vector_domain);
+	if (!hbus->irq_domain) {
+		pr_err("Failed to build an MSI IRQ domain\n");
+		return -ENODEV;
+	}
+
+	return 0;
+}

-----Original Message-----
From: Jiang Liu [mailto:[email protected]]
Sent: Tuesday, October 27, 2015 12:11 AM
To: Jake Oshins <[email protected]>; [email protected]; KY Srinivasan <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Haiyang Zhang <[email protected]>; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH v3 7/7] PCI: hv: New paravirtual PCI front-end for Hyper- V VMs

Quoted 2 lines · On 2015/10/27 7:15, [email protected] wrote:

From: Jake Oshins <[email protected]>

Review

Quoted 13 lines

(quoted diff) +13 -0

+/**
+ * hv_pcie_init_irq_domain() - Initialize IRQ domain
+ * @hbus:	The root PCI bus
+ *
+ * Return: '0' on success and error value on failure
+ */
+static int hv_pcie_init_irq_domain(struct hv_pcibus_device *hbus)
+{
+	hbus->msi_info.chip = &hv_msi_irq_chip;
+	hbus->msi_info.chip_data = hbus;
+	hbus->msi_info.ops = &hv_msi_ops;
+	hbus->msi_info.flags = (MSI_FLAG_USE_DEF_DOM_OPS |
+		MSI_FLAG_USE_DEF_CHIP_OPS |

MSI_FLAG_MULTI_PCI_MSI |

Review

Quoted 1 line

(quoted diff) +1 -0

+		MSI_FLAG_PCI_MSIX);

When interrupt remapping is not supported, x86 vector allocator can't support multiple MSI because it can't allocate continuous vectors yet. So please confirm whether we could enable MSI_FLAG_MULTI_PCI_MSI for HV.

Review

Quoted 3 lines

(quoted diff) +3 -0

+	hbus->msi_info.handler = handle_edge_irq;
+	hbus->msi_info.handler_name = "edge";
+	hbus->msi_info.data = hbus;

How about using following pattern so we could avoid exporting too many interfaces?

struct irq_domain *parent_domain = NULL; hbus->msi_info.chip = &hv_msi_irq_chip; hbus->msi_info.ops = &hv_msi_ops;
// Let arch code to fill in default ops for chip and domain x86_setup_default_msi_irqdomian_info(&hbus->msi_info, &parent_domain);
// Override default ops if not applicable hbus->irq_domain = pci_msi_create_irq_domain(hbus->fwnode,

					     &hbus->msi_info,
					     parent_domain);

Quoted 15 lines

-----Original Message-----
From: Jiang Liu [mailto:[email protected]]
Sent: Tuesday, October 27, 2015 12:11 AM
To: Jake Oshins <[email protected]>; [email protected]; KY Srinivasan <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Haiyang Zhang <[email protected]>; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH v3 7/7] PCI: hv: New paravirtual PCI front-end for Hyper- V VMs

Quoted 2 lines · On 2015/10/27 7:15, [email protected] wrote:

From: Jake Oshins <[email protected]>

(snip)

Quoted 14 lines

Review

Quoted 13 lines

(quoted diff) +13 -0

+/**
+ * hv_pcie_init_irq_domain() - Initialize IRQ domain
+ * @hbus:	The root PCI bus
+ *
+ * Return: '0' on success and error value on failure
+ */
+static int hv_pcie_init_irq_domain(struct hv_pcibus_device *hbus)
+{
+	hbus->msi_info.chip = &hv_msi_irq_chip;
+	hbus->msi_info.chip_data = hbus;
+	hbus->msi_info.ops = &hv_msi_ops;
+	hbus->msi_info.flags = (MSI_FLAG_USE_DEF_DOM_OPS |
+		MSI_FLAG_USE_DEF_CHIP_OPS |

MSI_FLAG_MULTI_PCI_MSI |

Quoted 6 lines

Review

Quoted 1 line

(quoted diff) +1 -0

+		MSI_FLAG_PCI_MSIX);

When interrupt remapping is not supported, x86 vector allocator can't support multiple MSI because it can't allocate continuous vectors yet. So please confirm whether we could enable MSI_FLAG_MULTI_PCI_MSI for HV.

We can actually handle the remapping in the hypervisor. I'll add a comment to that effect.

Quoted 14 lines

Review

Quoted 3 lines

(quoted diff) +3 -0

+	hbus->msi_info.handler = handle_edge_irq;
+	hbus->msi_info.handler_name = "edge";
+	hbus->msi_info.data = hbus;

How about using following pattern so we could avoid exporting too many interfaces?

struct irq_domain *parent_domain = NULL; hbus->msi_info.chip = &hv_msi_irq_chip; hbus->msi_info.ops = &hv_msi_ops;
// Let arch code to fill in default ops for chip and domain x86_setup_default_msi_irqdomian_info(&hbus->msi_info, &parent_domain);
// Override default ops if not applicable hbus->irq_domain = pci_msi_create_irq_domain(hbus->fwnode,

Quoted 3 lines

					     &hbus->msi_info,
					     parent_domain);

I understand your point here, but I'm having trouble making it play out. When I look at this, the only functions or structures which are supplied straight from exports (in my proposed patches) are irq_chip_ack_parent(), pci_msi_prepare() and x86_vector_domain. The other exports either already exist for other reasons or they're needed within functions that I need to supply. If you feel strongly that adding a new function to avoid exporting these is the right way to go, I'll do it, but I want to confirm that first.

Thanks,
Jake Oshins