From fb51ccbf217c1c994607b6519c7d85250928553d Mon Sep 17 00:00:00 2001 From: Jan Kiszka Date: Fri, 4 Nov 2011 09:45:59 +0100 Subject: PCI: Rework config space blocking services pci_block_user_cfg_access was designed for the use case that a single context, the IPR driver, temporarily delays user space accesses to the config space via sysfs. This assumption became invalid by the time pci_dev_reset was added as locking instance. Today, if you run two loops in parallel that reset the same device via sysfs, you end up with a kernel BUG as pci_block_user_cfg_access detect the broken assumption. This reworks the pci_block_user_cfg_access to a sleeping service pci_cfg_access_lock and an atomic-compatible variant called pci_cfg_access_trylock. The former not only blocks user space access as before but also waits if access was already locked. The latter service just returns false in this case, allowing the caller to resolve the conflict instead of raising a BUG. Adaptions of the ipr driver were originally written by Brian King. Acked-by: Brian King Acked-by: Michael S. Tsirkin Signed-off-by: Jan Kiszka Signed-off-by: Jesse Barnes --- drivers/scsi/ipr.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++------ drivers/scsi/ipr.h | 1 + 2 files changed, 61 insertions(+), 7 deletions(-) (limited to 'drivers/scsi') diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c index fd860d952b2..67b169b7a5b 100644 --- a/drivers/scsi/ipr.c +++ b/drivers/scsi/ipr.c @@ -7638,8 +7638,12 @@ static int ipr_reset_restore_cfg_space(struct ipr_cmnd *ipr_cmd) **/ static int ipr_reset_bist_done(struct ipr_cmnd *ipr_cmd) { + struct ipr_ioa_cfg *ioa_cfg = ipr_cmd->ioa_cfg; + ENTER; - pci_unblock_user_cfg_access(ipr_cmd->ioa_cfg->pdev); + if (ioa_cfg->cfg_locked) + pci_cfg_access_unlock(ioa_cfg->pdev); + ioa_cfg->cfg_locked = 0; ipr_cmd->job_step = ipr_reset_restore_cfg_space; LEAVE; return IPR_RC_JOB_CONTINUE; @@ -7660,8 +7664,6 @@ static int ipr_reset_start_bist(struct ipr_cmnd *ipr_cmd) int rc = PCIBIOS_SUCCESSFUL; ENTER; - pci_block_user_cfg_access(ioa_cfg->pdev); - if (ioa_cfg->ipr_chip->bist_method == IPR_MMIO) writel(IPR_UPROCI_SIS64_START_BIST, ioa_cfg->regs.set_uproc_interrupt_reg32); @@ -7673,7 +7675,9 @@ static int ipr_reset_start_bist(struct ipr_cmnd *ipr_cmd) ipr_reset_start_timer(ipr_cmd, IPR_WAIT_FOR_BIST_TIMEOUT); rc = IPR_RC_JOB_RETURN; } else { - pci_unblock_user_cfg_access(ipr_cmd->ioa_cfg->pdev); + if (ioa_cfg->cfg_locked) + pci_cfg_access_unlock(ipr_cmd->ioa_cfg->pdev); + ioa_cfg->cfg_locked = 0; ipr_cmd->s.ioasa.hdr.ioasc = cpu_to_be32(IPR_IOASC_PCI_ACCESS_ERROR); rc = IPR_RC_JOB_CONTINUE; } @@ -7716,7 +7720,6 @@ static int ipr_reset_slot_reset(struct ipr_cmnd *ipr_cmd) struct pci_dev *pdev = ioa_cfg->pdev; ENTER; - pci_block_user_cfg_access(pdev); pci_set_pcie_reset_state(pdev, pcie_warm_reset); ipr_cmd->job_step = ipr_reset_slot_reset_done; ipr_reset_start_timer(ipr_cmd, IPR_PCI_RESET_TIMEOUT); @@ -7724,6 +7727,56 @@ static int ipr_reset_slot_reset(struct ipr_cmnd *ipr_cmd) return IPR_RC_JOB_RETURN; } +/** + * ipr_reset_block_config_access_wait - Wait for permission to block config access + * @ipr_cmd: ipr command struct + * + * Description: This attempts to block config access to the IOA. + * + * Return value: + * IPR_RC_JOB_CONTINUE / IPR_RC_JOB_RETURN + **/ +static int ipr_reset_block_config_access_wait(struct ipr_cmnd *ipr_cmd) +{ + struct ipr_ioa_cfg *ioa_cfg = ipr_cmd->ioa_cfg; + int rc = IPR_RC_JOB_CONTINUE; + + if (pci_cfg_access_trylock(ioa_cfg->pdev)) { + ioa_cfg->cfg_locked = 1; + ipr_cmd->job_step = ioa_cfg->reset; + } else { + if (ipr_cmd->u.time_left) { + rc = IPR_RC_JOB_RETURN; + ipr_cmd->u.time_left -= IPR_CHECK_FOR_RESET_TIMEOUT; + ipr_reset_start_timer(ipr_cmd, + IPR_CHECK_FOR_RESET_TIMEOUT); + } else { + ipr_cmd->job_step = ioa_cfg->reset; + dev_err(&ioa_cfg->pdev->dev, + "Timed out waiting to lock config access. Resetting anyway.\n"); + } + } + + return rc; +} + +/** + * ipr_reset_block_config_access - Block config access to the IOA + * @ipr_cmd: ipr command struct + * + * Description: This attempts to block config access to the IOA + * + * Return value: + * IPR_RC_JOB_CONTINUE + **/ +static int ipr_reset_block_config_access(struct ipr_cmnd *ipr_cmd) +{ + ipr_cmd->ioa_cfg->cfg_locked = 0; + ipr_cmd->job_step = ipr_reset_block_config_access_wait; + ipr_cmd->u.time_left = IPR_WAIT_FOR_RESET_TIMEOUT; + return IPR_RC_JOB_CONTINUE; +} + /** * ipr_reset_allowed - Query whether or not IOA can be reset * @ioa_cfg: ioa config struct @@ -7763,7 +7816,7 @@ static int ipr_reset_wait_to_start_bist(struct ipr_cmnd *ipr_cmd) ipr_cmd->u.time_left -= IPR_CHECK_FOR_RESET_TIMEOUT; ipr_reset_start_timer(ipr_cmd, IPR_CHECK_FOR_RESET_TIMEOUT); } else { - ipr_cmd->job_step = ioa_cfg->reset; + ipr_cmd->job_step = ipr_reset_block_config_access; rc = IPR_RC_JOB_CONTINUE; } @@ -7796,7 +7849,7 @@ static int ipr_reset_alert(struct ipr_cmnd *ipr_cmd) writel(IPR_UPROCI_RESET_ALERT, ioa_cfg->regs.set_uproc_interrupt_reg32); ipr_cmd->job_step = ipr_reset_wait_to_start_bist; } else { - ipr_cmd->job_step = ioa_cfg->reset; + ipr_cmd->job_step = ipr_reset_block_config_access; } ipr_cmd->u.time_left = IPR_WAIT_FOR_RESET_TIMEOUT; diff --git a/drivers/scsi/ipr.h b/drivers/scsi/ipr.h index ac84736c1b9..b13f9cc1227 100644 --- a/drivers/scsi/ipr.h +++ b/drivers/scsi/ipr.h @@ -1387,6 +1387,7 @@ struct ipr_ioa_cfg { u8 msi_received:1; u8 sis64:1; u8 dump_timeout:1; + u8 cfg_locked:1; u8 revid; -- cgit v1.2.3