Skip to content

Commit

Permalink
Merge tag 'pci-v5.16-fixes-1' of git://git.kernel.org/pub/scm/linux/k…
Browse files Browse the repository at this point in the history 
…ernel/git/helgaas/pci

Pull PCI fixes from Bjorn Helgaas:
 "Revert conversion to struct device.driver instead of struct
  pci_dev.driver.

  The device.driver is set earlier, and using it caused the PCI core to
  call driver PM entry points before .probe() and after .remove(), when
  the driver isn't prepared.

  This caused NULL pointer dereferences in i2c_designware_pci and
  probably other driver issues"

* tag 'pci-v5.16-fixes-1' of git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci:
  Revert "PCI: Use to_pci_driver() instead of pci_dev->driver"
  Revert "PCI: Remove struct pci_dev->driver"
  • Loading branch information
@torvalds
torvalds committed Nov 11, 2021
2 parents ca2ef2d + e0217c5 commit 5833291
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 45 deletions.
24 changes: 9 additions & 15 deletions drivers/pci/iov.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,15 +164,13 @@ static ssize_t sriov_vf_total_msix_show(struct device *dev,
char *buf)
{
struct pci_dev *pdev = to_pci_dev(dev);
struct pci_driver *pdrv;
u32 vf_total_msix = 0;

device_lock(dev);
pdrv = to_pci_driver(dev->driver);
if (!pdrv || !pdrv->sriov_get_vf_total_msix)
if (!pdev->driver || !pdev->driver->sriov_get_vf_total_msix)
goto unlock;

vf_total_msix = pdrv->sriov_get_vf_total_msix(pdev);
vf_total_msix = pdev->driver->sriov_get_vf_total_msix(pdev);
unlock:
device_unlock(dev);
return sysfs_emit(buf, "%u\n", vf_total_msix);
Expand All @@ -185,7 +183,6 @@ static ssize_t sriov_vf_msix_count_store(struct device *dev,
{
struct pci_dev *vf_dev = to_pci_dev(dev);
struct pci_dev *pdev = pci_physfn(vf_dev);
struct pci_driver *pdrv;
int val, ret = 0;

if (kstrtoint(buf, 0, &val) < 0)
Expand All @@ -195,14 +192,13 @@ static ssize_t sriov_vf_msix_count_store(struct device *dev,
return -EINVAL;

device_lock(&pdev->dev);
pdrv = to_pci_driver(dev->driver);
if (!pdrv || !pdrv->sriov_set_msix_vec_count) {
if (!pdev->driver || !pdev->driver->sriov_set_msix_vec_count) {
ret = -EOPNOTSUPP;
goto err_pdev;
}

device_lock(&vf_dev->dev);
if (to_pci_driver(vf_dev->dev.driver)) {
if (vf_dev->driver) {
/*
* A driver is already attached to this VF and has configured
* itself based on the current MSI-X vector count. Changing
Expand All @@ -212,7 +208,7 @@ static ssize_t sriov_vf_msix_count_store(struct device *dev,
goto err_dev;
}

ret = pdrv->sriov_set_msix_vec_count(vf_dev, val);
ret = pdev->driver->sriov_set_msix_vec_count(vf_dev, val);

err_dev:
device_unlock(&vf_dev->dev);
Expand Down Expand Up @@ -379,7 +375,6 @@ static ssize_t sriov_numvfs_store(struct device *dev,
const char *buf, size_t count)
{
struct pci_dev *pdev = to_pci_dev(dev);
struct pci_driver *pdrv;
int ret = 0;
u16 num_vfs;

Expand All @@ -395,23 +390,22 @@ static ssize_t sriov_numvfs_store(struct device *dev,
goto exit;

/* is PF driver loaded */
pdrv = to_pci_driver(dev->driver);
if (!pdrv) {
if (!pdev->driver) {
pci_info(pdev, "no driver bound to device; cannot configure SR-IOV\n");
ret = -ENOENT;
goto exit;
}

/* is PF driver loaded w/callback */
if (!pdrv->sriov_configure) {
if (!pdev->driver->sriov_configure) {
pci_info(pdev, "driver does not support SR-IOV configuration via sysfs\n");
ret = -ENOENT;
goto exit;
}

if (num_vfs == 0) {
/* disable VFs */
ret = pdrv->sriov_configure(pdev, 0);
ret = pdev->driver->sriov_configure(pdev, 0);
goto exit;
}

Expand All @@ -423,7 +417,7 @@ static ssize_t sriov_numvfs_store(struct device *dev,
goto exit;
}

ret = pdrv->sriov_configure(pdev, num_vfs);
ret = pdev->driver->sriov_configure(pdev, num_vfs);
if (ret < 0)
goto exit;

Expand Down
37 changes: 20 additions & 17 deletions drivers/pci/pci-driver.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -319,10 +319,12 @@ static long local_pci_probe(void *_ddi)
* its remove routine.
*/
pm_runtime_get_sync(dev);
pci_dev->driver = pci_drv;
rc = pci_drv->probe(pci_dev, ddi->id);
if (!rc)
return rc;
if (rc < 0) {
pci_dev->driver = NULL;
pm_runtime_put_sync(dev);
return rc;
}
Expand Down Expand Up @@ -388,6 +390,7 @@ static int pci_call_probe(struct pci_driver *drv, struct pci_dev *dev,
* @pci_dev: PCI device being probed
*
* returns 0 on success, else error.
* side-effect: pci_dev->driver is set to drv when drv claims pci_dev.
*/
static int __pci_device_probe(struct pci_driver *drv, struct pci_dev *pci_dev)
{
Expand Down Expand Up @@ -454,14 +457,15 @@ static int pci_device_probe(struct device *dev)
static void pci_device_remove(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
struct pci_driver *drv = to_pci_driver(dev->driver);
struct pci_driver *drv = pci_dev->driver;

if (drv->remove) {
pm_runtime_get_sync(dev);
drv->remove(pci_dev);
pm_runtime_put_noidle(dev);
}
pcibios_free_irq(pci_dev);
pci_dev->driver = NULL;
pci_iov_remove(pci_dev);

/* Undo the runtime PM settings in local_pci_probe() */
Expand Down Expand Up @@ -489,7 +493,7 @@ static void pci_device_remove(struct device *dev)
static void pci_device_shutdown(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
struct pci_driver *drv = to_pci_driver(dev->driver);
struct pci_driver *drv = pci_dev->driver;

pm_runtime_resume(dev);

Expand Down Expand Up @@ -585,7 +589,7 @@ static int pci_pm_reenable_device(struct pci_dev *pci_dev)
static int pci_legacy_suspend(struct device *dev, pm_message_t state)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
struct pci_driver *drv = to_pci_driver(dev->driver);
struct pci_driver *drv = pci_dev->driver;

if (drv && drv->suspend) {
pci_power_t prev = pci_dev->current_state;
Expand Down Expand Up @@ -626,7 +630,7 @@ static int pci_legacy_suspend_late(struct device *dev, pm_message_t state)
static int pci_legacy_resume(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
struct pci_driver *drv = to_pci_driver(dev->driver);
struct pci_driver *drv = pci_dev->driver;

pci_fixup_device(pci_fixup_resume, pci_dev);

Expand All @@ -645,7 +649,7 @@ static void pci_pm_default_suspend(struct pci_dev *pci_dev)

static bool pci_has_legacy_pm_support(struct pci_dev *pci_dev)
{
struct pci_driver *drv = to_pci_driver(pci_dev->dev.driver);
struct pci_driver *drv = pci_dev->driver;
bool ret = drv && (drv->suspend || drv->resume);

/*
Expand Down Expand Up @@ -1238,11 +1242,11 @@ static int pci_pm_runtime_suspend(struct device *dev)
int error;

/*
* If the device has no driver, we leave it in D0, but it may go to
* D3cold when the bridge above it runtime suspends. Save its
* config space in case that happens.
* If pci_dev->driver is not set (unbound), we leave the device in D0,
* but it may go to D3cold when the bridge above it runtime suspends.
* Save its config space in case that happens.
*/
if (!to_pci_driver(dev->driver)) {
if (!pci_dev->driver) {
pci_save_state(pci_dev);
return 0;
}
Expand Down Expand Up @@ -1299,7 +1303,7 @@ static int pci_pm_runtime_resume(struct device *dev)
*/
pci_restore_standard_config(pci_dev);

if (!to_pci_driver(dev->driver))
if (!pci_dev->driver)
return 0;

pci_fixup_device(pci_fixup_resume_early, pci_dev);
Expand All @@ -1318,13 +1322,14 @@ static int pci_pm_runtime_resume(struct device *dev)

static int pci_pm_runtime_idle(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;

/*
* If the device has no driver, it should always remain in D0
* regardless of the runtime PM status
* If pci_dev->driver is not set (unbound), the device should
* always remain in D0 regardless of the runtime PM status
*/
if (!to_pci_driver(dev->driver))
if (!pci_dev->driver)
return 0;

if (!pm)
Expand Down Expand Up @@ -1431,10 +1436,8 @@ static struct pci_driver pci_compat_driver = {
*/
struct pci_driver *pci_dev_driver(const struct pci_dev *dev)
{
struct pci_driver *drv = to_pci_driver(dev->dev.driver);

if (drv)
return drv;
if (dev->driver)
return dev->driver;
else {
int i;
for (i = 0; i <= PCI_ROM_RESOURCE; i++)
Expand Down
17 changes: 8 additions & 9 deletions drivers/pci/pci.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5135,14 +5135,13 @@ EXPORT_SYMBOL_GPL(pci_dev_unlock);

static void pci_dev_save_and_disable(struct pci_dev *dev)
{
struct pci_driver *drv = to_pci_driver(dev->dev.driver);
const struct pci_error_handlers *err_handler =
drv ? drv->err_handler : NULL;
dev->driver ? dev->driver->err_handler : NULL;

/*
* drv->err_handler->reset_prepare() is protected against races
* with ->remove() by the device lock, which must be held by the
* caller.
* dev->driver->err_handler->reset_prepare() is protected against
* races with ->remove() by the device lock, which must be held by
* the caller.
*/
if (err_handler && err_handler->reset_prepare)
err_handler->reset_prepare(dev);
Expand All @@ -5167,15 +5166,15 @@ static void pci_dev_save_and_disable(struct pci_dev *dev)

static void pci_dev_restore(struct pci_dev *dev)
{
struct pci_driver *drv = to_pci_driver(dev->dev.driver);
const struct pci_error_handlers *err_handler =
drv ? drv->err_handler : NULL;
dev->driver ? dev->driver->err_handler : NULL;

pci_restore_state(dev);

/*
* drv->err_handler->reset_done() is protected against races with
* ->remove() by the device lock, which must be held by the caller.
* dev->driver->err_handler->reset_done() is protected against
* races with ->remove() by the device lock, which must be held by
* the caller.
*/
if (err_handler && err_handler->reset_done)
err_handler->reset_done(dev);
Expand Down
8 changes: 4 additions & 4 deletions drivers/pci/pcie/err.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ static int report_error_detected(struct pci_dev *dev,
const struct pci_error_handlers *err_handler;

device_lock(&dev->dev);
pdrv = to_pci_driver(dev->dev.driver);
pdrv = dev->driver;
if (!pci_dev_set_io_state(dev, state) ||
!pdrv ||
!pdrv->err_handler ||
Expand Down Expand Up @@ -98,7 +98,7 @@ static int report_mmio_enabled(struct pci_dev *dev, void *data)
const struct pci_error_handlers *err_handler;

device_lock(&dev->dev);
pdrv = to_pci_driver(dev->dev.driver);
pdrv = dev->driver;
if (!pdrv ||
!pdrv->err_handler ||
!pdrv->err_handler->mmio_enabled)
Expand All @@ -119,7 +119,7 @@ static int report_slot_reset(struct pci_dev *dev, void *data)
const struct pci_error_handlers *err_handler;

device_lock(&dev->dev);
pdrv = to_pci_driver(dev->dev.driver);
pdrv = dev->driver;
if (!pdrv ||
!pdrv->err_handler ||
!pdrv->err_handler->slot_reset)
Expand All @@ -139,7 +139,7 @@ static int report_resume(struct pci_dev *dev, void *data)
const struct pci_error_handlers *err_handler;

device_lock(&dev->dev);
pdrv = to_pci_driver(dev->dev.driver);
pdrv = dev->driver;
if (!pci_dev_set_io_state(dev, pci_channel_io_normal) ||
!pdrv ||
!pdrv->err_handler ||
Expand Down
1 change: 1 addition & 0 deletions include/linux/pci.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,7 @@ struct pci_dev {
u16 pcie_flags_reg; /* Cached PCIe Capabilities Register */
unsigned long *dma_alias_mask;/* Mask of enabled devfn aliases */

struct pci_driver *driver; /* Driver bound to this device */
u64 dma_mask; /* Mask of the bits of bus address this
device implements. Normally this is
0xffffffff. You only need to change
Expand Down

0 comments on commit 5833291

Please sign in to comment.