Re: [PATCH v2] ide: pci: free PCI BARs on initialization failure

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/03/2017 05:17 AM, Bartlomiej Zolnierkiewicz wrote:
Recent pci_assign_irq() changes uncovered a problem with missing
freeing of PCI BARs on PCI IDE host initialization failure:

ide0: disabled, no IRQ
ide0: failed to initialize IDE interface
ide0: disabling port
cmd64x 0000:00:02.0: IDE controller (0x1095:0x0646 rev 0x07)
CMD64x_IDE 0000:00:02.0: BAR 0: can't reserve [io  0x8050-0x8057]
cmd64x 0000:00:02.0: can't reserve resources
CMD64x_IDE: probe of 0000:00:02.0 failed with error -16

Fix the problem by adding missing freeing of PCI BARs to
ide_setup_pci_controller() and ide_pci_init_two().

Link: http://lkml.kernel.org/r/32ec730f-c1b0-5584-cd35-f8a809122b96@xxxxxxxxxxxx
Reported-by: Guenter Roeck <linux@xxxxxxxxxxxx>
Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
Cc: Richard Henderson <rth@xxxxxxxxxxx>
Cc: Ivan Kokshaysky <ink@xxxxxxxxxxxxxxxxxxxx>
Cc: Guenter Roeck <linux@xxxxxxxxxxxx>
Cc: Matt Turner <mattst88@xxxxxxxxx>
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@xxxxxxxxxxx>

Tested-by: Guenter Roeck <linux@xxxxxxxxxxxx>

---
This should fix problem with reserving PCI resources on a secondary
PCI device probe attempt (please test when pci_assign_irq() fix is
not present and "[PATCH] ide: add missing hwif->portdev freeing on
hwif_init() failure" is applied).

v2:
- Added missing 'goto out;' in ide_setup_pci_controller()

  drivers/ide/setup-pci.c |   63 ++++++++++++++++++++++++++++++------------------
  1 file changed, 40 insertions(+), 23 deletions(-)

Index: b/drivers/ide/setup-pci.c
===================================================================
--- a/drivers/ide/setup-pci.c	2017-10-03 14:13:02.391779813 +0200
+++ b/drivers/ide/setup-pci.c	2017-10-03 14:13:59.051779867 +0200
@@ -179,6 +179,7 @@ EXPORT_SYMBOL_GPL(ide_setup_pci_noise);
  /**
   *	ide_pci_enable	-	do PCI enables
   *	@dev: PCI device
+ *	@bars: PCI BARs mask
   *	@d: IDE port info
   *
   *	Enable the IDE PCI device. We attempt to enable the device in full
@@ -189,9 +190,10 @@ EXPORT_SYMBOL_GPL(ide_setup_pci_noise);
   *	Returns zero on success or an error code
   */
-static int ide_pci_enable(struct pci_dev *dev, const struct ide_port_info *d)
+static int ide_pci_enable(struct pci_dev *dev, int bars,
+			  const struct ide_port_info *d)
  {
-	int ret, bars;
+	int ret;
if (pci_enable_device(dev)) {
  		ret = pci_enable_device_io(dev);
@@ -216,18 +218,6 @@ static int ide_pci_enable(struct pci_dev
  		goto out;
  	}
- if (d->host_flags & IDE_HFLAG_SINGLE)
-		bars = (1 << 2) - 1;
-	else
-		bars = (1 << 4) - 1;
-
-	if ((d->host_flags & IDE_HFLAG_NO_DMA) == 0) {
-		if (d->host_flags & IDE_HFLAG_CS5520)
-			bars |= (1 << 2);
-		else
-			bars |= (1 << 4);
-	}
-
  	ret = pci_request_selected_regions(dev, bars, d->name);
  	if (ret < 0)
  		printk(KERN_ERR "%s %s: can't reserve resources\n",
@@ -403,6 +393,7 @@ int ide_hwif_setup_dma(ide_hwif_t *hwif,
  /**
   *	ide_setup_pci_controller	-	set up IDE PCI
   *	@dev: PCI device
+ *	@bars: PCI BARs mask
   *	@d: IDE port info
   *	@noisy: verbose flag
   *
@@ -411,7 +402,7 @@ int ide_hwif_setup_dma(ide_hwif_t *hwif,
   *	and enables it if need be
   */
-static int ide_setup_pci_controller(struct pci_dev *dev,
+static int ide_setup_pci_controller(struct pci_dev *dev, int bars,
  				    const struct ide_port_info *d, int noisy)
  {
  	int ret;
@@ -420,7 +411,7 @@ static int ide_setup_pci_controller(stru
  	if (noisy)
  		ide_setup_pci_noise(dev, d);
- ret = ide_pci_enable(dev, d);
+	ret = ide_pci_enable(dev, bars, d);
  	if (ret < 0)
  		goto out;
@@ -428,16 +419,20 @@ static int ide_setup_pci_controller(stru
  	if (ret < 0) {
  		printk(KERN_ERR "%s %s: error accessing PCI regs\n",
  			d->name, pci_name(dev));
-		goto out;
+		goto out_free_bars;
  	}
  	if (!(pcicmd & PCI_COMMAND_IO)) {	/* is device disabled? */
  		ret = ide_pci_configure(dev, d);
  		if (ret < 0)
-			goto out;
+			goto out_free_bars;
  		printk(KERN_INFO "%s %s: device enabled (Linux)\n",
  			d->name, pci_name(dev));
  	}
+ goto out;
+
+out_free_bars:
+	pci_release_selected_regions(dev, bars);
  out:
  	return ret;
  }
@@ -540,13 +535,28 @@ int ide_pci_init_two(struct pci_dev *dev
  {
  	struct pci_dev *pdev[] = { dev1, dev2 };
  	struct ide_host *host;
-	int ret, i, n_ports = dev2 ? 4 : 2;
+	int ret, i, n_ports = dev2 ? 4 : 2, bars;
  	struct ide_hw hw[4], *hws[] = { NULL, NULL, NULL, NULL };
+ if (d->host_flags & IDE_HFLAG_SINGLE)
+		bars = (1 << 2) - 1;
+	else
+		bars = (1 << 4) - 1;
+
+	if ((d->host_flags & IDE_HFLAG_NO_DMA) == 0) {
+		if (d->host_flags & IDE_HFLAG_CS5520)
+			bars |= (1 << 2);
+		else
+			bars |= (1 << 4);
+	}
+
  	for (i = 0; i < n_ports / 2; i++) {
-		ret = ide_setup_pci_controller(pdev[i], d, !i);
-		if (ret < 0)
+		ret = ide_setup_pci_controller(pdev[i], bars, d, !i);
+		if (ret < 0) {
+			if (i == 1)
+				pci_release_selected_regions(pdev[0], bars);
  			goto out;
+		}
ide_pci_setup_ports(pdev[i], d, &hw[i*2], &hws[i*2]);
  	}
@@ -554,7 +564,7 @@ int ide_pci_init_two(struct pci_dev *dev
  	host = ide_host_alloc(d, hws, n_ports);
  	if (host == NULL) {
  		ret = -ENOMEM;
-		goto out;
+		goto out_free_bars;
  	}
host->dev[0] = &dev1->dev;
@@ -576,7 +586,7 @@ int ide_pci_init_two(struct pci_dev *dev
  		 * do_ide_setup_pci_device() on the first device!
  		 */
  		if (ret < 0)
-			goto out;
+			goto out_free_bars;
/* fixup IRQ */
  		if (ide_pci_is_in_compatibility_mode(pdev[i])) {
@@ -589,6 +599,13 @@ int ide_pci_init_two(struct pci_dev *dev
  	ret = ide_host_register(host, d, hws);
  	if (ret)
  		ide_host_free(host);
+	else
+		goto out;
+
+out_free_bars:
+	i = n_ports / 2;
+	while (i--)
+		pci_release_selected_regions(pdev[i], bars);
  out:
  	return ret;
  }



--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux