Skip to contentSkip to conversation

sparc/pci: Refactor dev_archdata initialization into pci_init_dev_archdata

PATCH Applied Sowmini Varadhan 3 participants 3 messages Apr 12, 2016 – Apr 27, 2016 SPARC + UltraSPARC (sparc/sparc64)

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 sparc/pci: Refactor dev_archdata initialization into pci_init_dev_archdata

The function pcibios_add_device() added by commit d0c31e020057 ("sparc/PCI: Fix for panic while enabling SR-IOV") initializes the dev_archdata by doing a memcpy from the PF. This has the problem that it erroneously copies the OF device without explicitly refcounting it.

As David Miller pointed out: "Generally speaking we don't really support hot-plug for OF probed devices, but if we did all of the device tree pointers have to be refcounted properly."

To fix this error, and also avoid code duplication, this patch creates a new helper function, pci_init_dev_archdata(), that initializes the fields in dev_archdata, and can be invoked by callers after they have taken the needed refcounts

---
 arch/sparc/kernel/pci.c |   29 +++++++++++++++++++++--------
 1 files changed, 21 insertions(+), 8 deletions(-)
arch/sparc/kernel/pci.c +21 -8
index 9f9614d..c2b202d 100644
@@ -245,6 +245,18 @@ static void pci_parse_of_addrs(struct platform_device *op,
 	}
 }
 
+static void pci_init_dev_archdata(struct dev_archdata *sd, void *iommu,
+				  void *stc, void *host_controller,
+				  struct platform_device  *op,
+				  int numa_node)
+{
+	sd->iommu = iommu;
+	sd->stc = stc;
+	sd->host_controller = host_controller;
+	sd->op = op;
+	sd->numa_node = numa_node;
+}
+
 static struct pci_dev *of_create_pci_dev(struct pci_pbm_info *pbm,
 					 struct device_node *node,
 					 struct pci_bus *bus, int devfn)
@@ -259,13 +271,10 @@ static struct pci_dev *of_create_pci_dev(struct pci_pbm_info *pbm,
 	if (!dev)
 		return NULL;
 
+	op = of_find_device_by_node(node);
 	sd = &dev->dev.archdata;
-	sd->iommu = pbm->iommu;
-	sd->stc = &pbm->stc;
-	sd->host_controller = pbm;
-	sd->op = op = of_find_device_by_node(node);
-	sd->numa_node = pbm->numa_node;
-
+	pci_init_dev_archdata(sd, pbm->iommu, &pbm->stc, pbm, op,
+			      pbm->numa_node);
 	sd = &op->dev.archdata;
 	sd->iommu = pbm->iommu;
 	sd->stc = &pbm->stc;
@@ -1003,9 +1012,13 @@ int pcibios_add_device(struct pci_dev *dev)
 	 * Copy dev_archdata from PF to VF
 	 */
 	if (dev->is_virtfn) {
+		struct dev_archdata *psd;
+
 		pdev = dev->physfn;
-		memcpy(&dev->dev.archdata, &pdev->dev.archdata,
-		       sizeof(struct dev_archdata));
+		psd = &pdev->dev.archdata;
+		pci_init_dev_archdata(&dev->dev.archdata, psd->iommu,
+				      psd->stc, psd->host_controller, NULL,
+				      psd->numa_node);
 	}
 	return 0;
 }
Signature · 1 line
1.7.1
Reviewed-by on v1 1/1
#1 lore raw
Quoted 69 lines · Sowmini Varadhan

The function pcibios_add_device() added by commit d0c31e020057 ("sparc/PCI: Fix for panic while enabling SR-IOV") initializes the dev_archdata by doing a memcpy from the PF. This has the problem that it erroneously copies the OF device without explicitly refcounting it.

As David Miller pointed out: "Generally speaking we don't really support hot-plug for OF probed devices, but if we did all of the device tree pointers have to be refcounted properly."

To fix this error, and also avoid code duplication, this patch creates a new helper function, pci_init_dev_archdata(), that initializes the fields in dev_archdata, and can be invoked by callers after they have taken the needed refcounts

---
  arch/sparc/kernel/pci.c |   29 +++++++++++++++++++++--------
  1 files changed, 21 insertions(+), 8 deletions(-)
arch/sparc/kernel/pci.c +17 -6
index 9f9614d..c2b202d 100644
@@ -245,6 +245,18 @@ static void pci_parse_of_addrs(struct platform_device *op,
  	}
  }
 
+static void pci_init_dev_archdata(struct dev_archdata *sd, void *iommu,
+				  void *stc, void *host_controller,
+				  struct platform_device  *op,
+				  int numa_node)
+{
+	sd->iommu = iommu;
+	sd->stc = stc;
+	sd->host_controller = host_controller;
+	sd->op = op;
+	sd->numa_node = numa_node;
+}
+
  static struct pci_dev *of_create_pci_dev(struct pci_pbm_info *pbm,
  					 struct device_node *node,
  					 struct pci_bus *bus, int devfn)
@@ -259,13 +271,10 @@ static struct pci_dev *of_create_pci_dev(struct pci_pbm_info *pbm,
  	if (!dev)
  		return NULL;
 
+	op = of_find_device_by_node(node);
  	sd = &dev->dev.archdata;
-	sd->iommu = pbm->iommu;
-	sd->stc = &pbm->stc;
-	sd->host_controller = pbm;
-	sd->op = op = of_find_device_by_node(node);
-	sd->numa_node = pbm->numa_node;
-
+	pci_init_dev_archdata(sd, pbm->iommu, &pbm->stc, pbm, op,
+			      pbm->numa_node);
  	sd = &op->dev.archdata;
  	sd->iommu = pbm->iommu;
  	sd->stc = &pbm->stc;
@@ -1003,9 +1012,13 @@ int pcibios_add_device(struct pci_dev *dev)
  	 * Copy dev_archdata from PF to VF
  	 */
  	if (dev->is_virtfn) {
+		struct dev_archdata *psd;
+

arch/sparc/kernel/pci.c int pcibios_add_device(struct pci_dev *dev) · line 1024

Quoted 10 lines
arch/sparc/kernel/pci.c +4 -2
  		pdev = dev->physfn;
-		memcpy(&dev->dev.archdata, &pdev->dev.archdata,
-		       sizeof(struct dev_archdata));
+		psd = &pdev->dev.archdata;
+		pci_init_dev_archdata(&dev->dev.archdata, psd->iommu,
+				      psd->stc, psd->host_controller, NULL,
+				      psd->numa_node);
  	}
  	return 0;
  }

Looks good to me.

--
Khalid

maintainer Applied
#2 lore raw
In reply to Sowmini Varadhan

From: Sowmini Varadhan <[email protected]>
Date: Mon, 11 Apr 2016 17:57:05 -0700

Quoted 10 lines

The function pcibios_add_device() added by commit d0c31e020057 ("sparc/PCI: Fix for panic while enabling SR-IOV") initializes the dev_archdata by doing a memcpy from the PF. This has the problem that it erroneously copies the OF device without explicitly refcounting it.

As David Miller pointed out: "Generally speaking we don't really support hot-plug for OF probed devices, but if we did all of the device tree pointers have to be refcounted properly."

Quoted 7 lines

To fix this error, and also avoid code duplication, this patch creates a new helper function, pci_init_dev_archdata(), that initializes the fields in dev_archdata, and can be invoked by callers after they have taken the needed refcounts

Applied.