Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 44bda4b7 authored by Hari Vyas's avatar Hari Vyas Committed by Bjorn Helgaas
Browse files

PCI: Fix is_added/is_busmaster race condition 

When a PCI device is detected, pdev->is_added is set to 1 and proc and
sysfs entries are created.

When the device is removed, pdev->is_added is checked for one and then
device is detached with clearing of proc and sys entries and at end,
pdev->is_added is set to 0.

is_added and is_busmaster are bit fields in pci_dev structure sharing same
memory location.

A strange issue was observed with multiple removal and rescan of a PCIe
NVMe device using sysfs commands where is_added flag was observed as zero
instead of one while removing device and proc,sys entries are not cleared.
This causes issue in later device addition with warning message
"proc_dir_entry" already registered.

Debugging revealed a race condition between the PCI core setting the
is_added bit in pci_bus_add_device() and the NVMe driver reset work-queue
setting the is_busmaster bit in pci_set_master().  As these fields are not
handled atomically, that clears the is_added bit.

Move the is_added bit to a separate private flag variable and use atomic
functions to set and retrieve the device addition state.  This avoids the
race because is_added no longer shares a memory location with is_busmaster.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=200283


Signed-off-by: default avatarHari Vyas <hari.vyas@broadcom.com>
Signed-off-by: default avatarBjorn Helgaas <bhelgaas@google.com>
Reviewed-by: default avatarLukas Wunner <lukas@wunner.de>
Acked-by: default avatarMichael Ellerman <mpe@ellerman.id.au>
parent a54e43f9

arch/powerpc/kernel/pci-common.c

+3 −1
Original line number Original line Diff line number Diff line
@@ -42,6 +42,8 @@ 
#include <asm/ppc-pci.h>

#include <asm/ppc-pci.h>

#include <asm/eeh.h>

#include <asm/eeh.h>





#include "../../../drivers/pci/pci.h"



/* hose_spinlock protects accesses to the the phb_bitmap. */

/* hose_spinlock protects accesses to the the phb_bitmap. */

static DEFINE_SPINLOCK(hose_spinlock);

static DEFINE_SPINLOCK(hose_spinlock);

LIST_HEAD(hose_list);

LIST_HEAD(hose_list);
@@ -1014,7 +1016,7 @@ void pcibios_setup_bus_devices(struct pci_bus *bus) 
		/* Cardbus can call us to add new devices to a bus, so ignore

		/* Cardbus can call us to add new devices to a bus, so ignore

		 * those who are already fully discovered

		 * those who are already fully discovered

		 */

		 */

		if (dev->is_added)

		if (pci_dev_is_added(dev))

			continue;

			continue;





		pcibios_setup_device(dev);

		pcibios_setup_device(dev);

arch/powerpc/platforms/powernv/pci-ioda.c

+2 −1
Original line number Original line Diff line number Diff line
@@ -46,6 +46,7 @@ 




#include "powernv.h"

#include "powernv.h"

#include "pci.h"

#include "pci.h"

#include "../../../../drivers/pci/pci.h"





#define PNV_IODA1_M64_NUM	16	/* Number of M64 BARs	*/

#define PNV_IODA1_M64_NUM	16	/* Number of M64 BARs	*/

#define PNV_IODA1_M64_SEGS	8	/* Segments per M64 BAR	*/

#define PNV_IODA1_M64_SEGS	8	/* Segments per M64 BAR	*/
@@ -3138,7 +3139,7 @@ static void pnv_pci_ioda_fixup_iov_resources(struct pci_dev *pdev) 
	struct pci_dn *pdn;

	struct pci_dn *pdn;

	int mul, total_vfs;

	int mul, total_vfs;





	if (!pdev->is_physfn || pdev->is_added)

	if (!pdev->is_physfn || pci_dev_is_added(pdev))

		return;

		return;





	pdn = pci_get_pdn(pdev);

	pdn = pci_get_pdn(pdev);

arch/powerpc/platforms/pseries/setup.c

+2 −1
Original line number Original line Diff line number Diff line
@@ -71,6 +71,7 @@ 
#include <asm/security_features.h>

#include <asm/security_features.h>





#include "pseries.h"

#include "pseries.h"

#include "../../../../drivers/pci/pci.h"





int CMO_PrPSP = -1;

int CMO_PrPSP = -1;

int CMO_SecPSP = -1;

int CMO_SecPSP = -1;
@@ -664,7 +665,7 @@ static void pseries_pci_fixup_iov_resources(struct pci_dev *pdev) 
	const int *indexes;

	const int *indexes;

	struct device_node *dn = pci_device_to_OF_node(pdev);

	struct device_node *dn = pci_device_to_OF_node(pdev);





	if (!pdev->is_physfn || pdev->is_added)

	if (!pdev->is_physfn || pci_dev_is_added(pdev))

		return;

		return;

	/*Firmware must support open sriov otherwise dont configure*/

	/*Firmware must support open sriov otherwise dont configure*/

	indexes = of_get_property(dn, "ibm,open-sriov-vf-bar-info", NULL);

	indexes = of_get_property(dn, "ibm,open-sriov-vf-bar-info", NULL);

drivers/pci/bus.c

+3 −3
Original line number Original line Diff line number Diff line
@@ -330,7 +330,7 @@ void pci_bus_add_device(struct pci_dev *dev) 
		return;

		return;

	}

	}





	dev->is_added = 1;

	pci_dev_assign_added(dev, true);

}

}

EXPORT_SYMBOL_GPL(pci_bus_add_device);

EXPORT_SYMBOL_GPL(pci_bus_add_device);
@@ -347,14 +347,14 @@ void pci_bus_add_devices(const struct pci_bus *bus) 




	list_for_each_entry(dev, &bus->devices, bus_list) {

	list_for_each_entry(dev, &bus->devices, bus_list) {

		/* Skip already-added devices */

		/* Skip already-added devices */

		if (dev->is_added)

		if (pci_dev_is_added(dev))

			continue;

			continue;

		pci_bus_add_device(dev);

		pci_bus_add_device(dev);

	}

	}





	list_for_each_entry(dev, &bus->devices, bus_list) {

	list_for_each_entry(dev, &bus->devices, bus_list) {

		/* Skip if device attach failed */

		/* Skip if device attach failed */

		if (!dev->is_added)

		if (!pci_dev_is_added(dev))

			continue;

			continue;

		child = dev->subordinate;

		child = dev->subordinate;

		if (child)

		if (child)

drivers/pci/hotplug/acpiphp_glue.c

+1 −1
Original line number Original line Diff line number Diff line
@@ -509,7 +509,7 @@ static void enable_slot(struct acpiphp_slot *slot) 




	list_for_each_entry(dev, &bus->devices, bus_list) {

	list_for_each_entry(dev, &bus->devices, bus_list) {

		/* Assume that newly added devices are powered on already. */

		/* Assume that newly added devices are powered on already. */

		if (!dev->is_added)

		if (!pci_dev_is_added(dev))

			dev->current_state = PCI_D0;

			dev->current_state = PCI_D0;

	}

	}