forked from mirrors/linux
		
	scsi: sd: Fix TCG OPAL unlock on system resume
Commit3cc2ffe5c1("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:3cc2ffe5c1("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: Damien Le Moal <dlemoal@kernel.org> Link: https://lore.kernel.org/r/20240319071209.1179257-1-dlemoal@kernel.org Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
This commit is contained in:
		
							parent
							
								
									27f58c04a8
								
							
						
					
					
						commit
						0c76106cb9
					
				
					 7 changed files with 69 additions and 5 deletions
				
			
		|  | @ -712,8 +712,10 @@ 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; | ||||||
|  | 			} | ||||||
| 		} | 		} | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
|  | @ -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) | ||||||
| 	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) | ||||||
| 			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, | ||||||
| } | } | ||||||
| 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) | ||||||
| 	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) | ||||||
| 	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) | ||||||
| 		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) | ||||||
| 				  "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 = { | ||||||
| 		.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 { | ||||||
| 
 | 
 | ||||||
| 	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; | ||||||
| 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); | ||||||
| #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 *); | ||||||
|  |  | ||||||
		Loading…
	
		Reference in a new issue
	
	 Damien Le Moal
						Damien Le Moal