Commit 0c76106c authored by Damien Le Moal's avatar Damien Le Moal Committed by Martin K. Petersen

scsi: sd: Fix TCG OPAL unlock on system resume

Commit 3cc2ffe5 ("scsi: sd: Differentiate system and runtime start/stop
management") introduced the manage_system_start_stop scsi_device flag to
allow libata to indicate to the SCSI disk driver that nothing should be
done when resuming a disk on system resume. This change turned the
execution of sd_resume() into a no-op for ATA devices on system
resume. While this solved deadlock issues during device resume, this change
also wrongly removed the execution of opal_unlock_from_suspend().  As a
result, devices with TCG OPAL locking enabled remain locked and
inaccessible after a system resume from sleep.

To fix this issue, introduce the SCSI driver resume method and implement it
with the sd_resume() function calling opal_unlock_from_suspend(). The
former sd_resume() function is renamed to sd_resume_common() and modified
to call the new sd_resume() function. For non-ATA devices, this result in
no functional changes.

In order for libata to explicitly execute sd_resume() when a device is
resumed during system restart, the function scsi_resume_device() is
introduced. libata calls this function from the revalidation work executed
on devie resume, a state that is indicated with the new device flag
ATA_DFLAG_RESUMING. Doing so, locked TCG OPAL enabled devices are unlocked
on resume, allowing normal operation.

Fixes: 3cc2ffe5 ("scsi: sd: Differentiate system and runtime start/stop management")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=218538
Cc: stable@vger.kernel.org
Signed-off-by: default avatarDamien Le Moal <dlemoal@kernel.org>
Link: https://lore.kernel.org/r/20240319071209.1179257-1-dlemoal@kernel.orgSigned-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
parent 27f58c04
...@@ -712,10 +712,12 @@ void ata_scsi_port_error_handler(struct Scsi_Host *host, struct ata_port *ap) ...@@ -712,10 +712,12 @@ void ata_scsi_port_error_handler(struct Scsi_Host *host, struct ata_port *ap)
ehc->saved_ncq_enabled |= 1 << devno; ehc->saved_ncq_enabled |= 1 << devno;
/* If we are resuming, wake up the device */ /* If we are resuming, wake up the device */
if (ap->pflags & ATA_PFLAG_RESUMING) if (ap->pflags & ATA_PFLAG_RESUMING) {
dev->flags |= ATA_DFLAG_RESUMING;
ehc->i.dev_action[devno] |= ATA_EH_SET_ACTIVE; ehc->i.dev_action[devno] |= ATA_EH_SET_ACTIVE;
} }
} }
}
ap->pflags |= ATA_PFLAG_EH_IN_PROGRESS; ap->pflags |= ATA_PFLAG_EH_IN_PROGRESS;
ap->pflags &= ~ATA_PFLAG_EH_PENDING; ap->pflags &= ~ATA_PFLAG_EH_PENDING;
...@@ -3169,6 +3171,7 @@ static int ata_eh_revalidate_and_attach(struct ata_link *link, ...@@ -3169,6 +3171,7 @@ static int ata_eh_revalidate_and_attach(struct ata_link *link,
return 0; return 0;
err: err:
dev->flags &= ~ATA_DFLAG_RESUMING;
*r_failed_dev = dev; *r_failed_dev = dev;
return rc; return rc;
} }
......
...@@ -4730,6 +4730,7 @@ void ata_scsi_dev_rescan(struct work_struct *work) ...@@ -4730,6 +4730,7 @@ void ata_scsi_dev_rescan(struct work_struct *work)
struct ata_link *link; struct ata_link *link;
struct ata_device *dev; struct ata_device *dev;
unsigned long flags; unsigned long flags;
bool do_resume;
int ret = 0; int ret = 0;
mutex_lock(&ap->scsi_scan_mutex); mutex_lock(&ap->scsi_scan_mutex);
...@@ -4751,7 +4752,15 @@ void ata_scsi_dev_rescan(struct work_struct *work) ...@@ -4751,7 +4752,15 @@ void ata_scsi_dev_rescan(struct work_struct *work)
if (scsi_device_get(sdev)) if (scsi_device_get(sdev))
continue; continue;
do_resume = dev->flags & ATA_DFLAG_RESUMING;
spin_unlock_irqrestore(ap->lock, flags); spin_unlock_irqrestore(ap->lock, flags);
if (do_resume) {
ret = scsi_resume_device(sdev);
if (ret == -EWOULDBLOCK)
goto unlock;
dev->flags &= ~ATA_DFLAG_RESUMING;
}
ret = scsi_rescan_device(sdev); ret = scsi_rescan_device(sdev);
scsi_device_put(sdev); scsi_device_put(sdev);
spin_lock_irqsave(ap->lock, flags); spin_lock_irqsave(ap->lock, flags);
......
...@@ -1642,6 +1642,40 @@ int scsi_add_device(struct Scsi_Host *host, uint channel, ...@@ -1642,6 +1642,40 @@ int scsi_add_device(struct Scsi_Host *host, uint channel,
} }
EXPORT_SYMBOL(scsi_add_device); EXPORT_SYMBOL(scsi_add_device);
int scsi_resume_device(struct scsi_device *sdev)
{
struct device *dev = &sdev->sdev_gendev;
int ret = 0;
device_lock(dev);
/*
* Bail out if the device or its queue are not running. Otherwise,
* the rescan may block waiting for commands to be executed, with us
* holding the device lock. This can result in a potential deadlock
* in the power management core code when system resume is on-going.
*/
if (sdev->sdev_state != SDEV_RUNNING ||
blk_queue_pm_only(sdev->request_queue)) {
ret = -EWOULDBLOCK;
goto unlock;
}
if (dev->driver && try_module_get(dev->driver->owner)) {
struct scsi_driver *drv = to_scsi_driver(dev->driver);
if (drv->resume)
ret = drv->resume(dev);
module_put(dev->driver->owner);
}
unlock:
device_unlock(dev);
return ret;
}
EXPORT_SYMBOL(scsi_resume_device);
int scsi_rescan_device(struct scsi_device *sdev) int scsi_rescan_device(struct scsi_device *sdev)
{ {
struct device *dev = &sdev->sdev_gendev; struct device *dev = &sdev->sdev_gendev;
......
...@@ -4108,7 +4108,21 @@ static int sd_suspend_runtime(struct device *dev) ...@@ -4108,7 +4108,21 @@ static int sd_suspend_runtime(struct device *dev)
return sd_suspend_common(dev, true); return sd_suspend_common(dev, true);
} }
static int sd_resume(struct device *dev, bool runtime) static int sd_resume(struct device *dev)
{
struct scsi_disk *sdkp = dev_get_drvdata(dev);
sd_printk(KERN_NOTICE, sdkp, "Starting disk\n");
if (opal_unlock_from_suspend(sdkp->opal_dev)) {
sd_printk(KERN_NOTICE, sdkp, "OPAL unlock failed\n");
return -EIO;
}
return 0;
}
static int sd_resume_common(struct device *dev, bool runtime)
{ {
struct scsi_disk *sdkp = dev_get_drvdata(dev); struct scsi_disk *sdkp = dev_get_drvdata(dev);
int ret; int ret;
...@@ -4124,7 +4138,7 @@ static int sd_resume(struct device *dev, bool runtime) ...@@ -4124,7 +4138,7 @@ static int sd_resume(struct device *dev, bool runtime)
sd_printk(KERN_NOTICE, sdkp, "Starting disk\n"); sd_printk(KERN_NOTICE, sdkp, "Starting disk\n");
ret = sd_start_stop_device(sdkp, 1); ret = sd_start_stop_device(sdkp, 1);
if (!ret) { if (!ret) {
opal_unlock_from_suspend(sdkp->opal_dev); sd_resume(dev);
sdkp->suspended = false; sdkp->suspended = false;
} }
...@@ -4143,7 +4157,7 @@ static int sd_resume_system(struct device *dev) ...@@ -4143,7 +4157,7 @@ static int sd_resume_system(struct device *dev)
return 0; return 0;
} }
return sd_resume(dev, false); return sd_resume_common(dev, false);
} }
static int sd_resume_runtime(struct device *dev) static int sd_resume_runtime(struct device *dev)
...@@ -4170,7 +4184,7 @@ static int sd_resume_runtime(struct device *dev) ...@@ -4170,7 +4184,7 @@ static int sd_resume_runtime(struct device *dev)
"Failed to clear sense data\n"); "Failed to clear sense data\n");
} }
return sd_resume(dev, true); return sd_resume_common(dev, true);
} }
static const struct dev_pm_ops sd_pm_ops = { static const struct dev_pm_ops sd_pm_ops = {
...@@ -4193,6 +4207,7 @@ static struct scsi_driver sd_template = { ...@@ -4193,6 +4207,7 @@ static struct scsi_driver sd_template = {
.pm = &sd_pm_ops, .pm = &sd_pm_ops,
}, },
.rescan = sd_rescan, .rescan = sd_rescan,
.resume = sd_resume,
.init_command = sd_init_command, .init_command = sd_init_command,
.uninit_command = sd_uninit_command, .uninit_command = sd_uninit_command,
.done = sd_done, .done = sd_done,
......
...@@ -107,6 +107,7 @@ enum { ...@@ -107,6 +107,7 @@ enum {
ATA_DFLAG_NCQ_PRIO_ENABLED = (1 << 20), /* Priority cmds sent to dev */ ATA_DFLAG_NCQ_PRIO_ENABLED = (1 << 20), /* Priority cmds sent to dev */
ATA_DFLAG_CDL_ENABLED = (1 << 21), /* cmd duration limits is enabled */ ATA_DFLAG_CDL_ENABLED = (1 << 21), /* cmd duration limits is enabled */
ATA_DFLAG_RESUMING = (1 << 22), /* Device is resuming */
ATA_DFLAG_DETACH = (1 << 24), ATA_DFLAG_DETACH = (1 << 24),
ATA_DFLAG_DETACHED = (1 << 25), ATA_DFLAG_DETACHED = (1 << 25),
ATA_DFLAG_DA = (1 << 26), /* device supports Device Attention */ ATA_DFLAG_DA = (1 << 26), /* device supports Device Attention */
......
...@@ -12,6 +12,7 @@ struct request; ...@@ -12,6 +12,7 @@ struct request;
struct scsi_driver { struct scsi_driver {
struct device_driver gendrv; struct device_driver gendrv;
int (*resume)(struct device *);
void (*rescan)(struct device *); void (*rescan)(struct device *);
blk_status_t (*init_command)(struct scsi_cmnd *); blk_status_t (*init_command)(struct scsi_cmnd *);
void (*uninit_command)(struct scsi_cmnd *); void (*uninit_command)(struct scsi_cmnd *);
......
...@@ -767,6 +767,7 @@ scsi_template_proc_dir(const struct scsi_host_template *sht); ...@@ -767,6 +767,7 @@ scsi_template_proc_dir(const struct scsi_host_template *sht);
#define scsi_template_proc_dir(sht) NULL #define scsi_template_proc_dir(sht) NULL
#endif #endif
extern void scsi_scan_host(struct Scsi_Host *); extern void scsi_scan_host(struct Scsi_Host *);
extern int scsi_resume_device(struct scsi_device *sdev);
extern int scsi_rescan_device(struct scsi_device *sdev); extern int scsi_rescan_device(struct scsi_device *sdev);
extern void scsi_remove_host(struct Scsi_Host *); extern void scsi_remove_host(struct Scsi_Host *);
extern struct Scsi_Host *scsi_host_get(struct Scsi_Host *); extern struct Scsi_Host *scsi_host_get(struct Scsi_Host *);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment