Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [smartpqi updates PATCH V2 05/11] smartpqi: add tur check for
 sanitize operation
To:     Don Brace <don.brace@microchip.com>, hch@infradead.org,
        martin.petersen@oracle.com, jejb@linux.vnet.ibm.com,
        linux-scsi@vger.kernel.org
Cc:     Kevin.Barnett@microchip.com, scott.teel@microchip.com,
        Justin.Lindley@microchip.com, scott.benesh@microchip.com,
        gerry.morong@microchip.com, mahesh.rajashekhara@microchip.com,
        mike.mcgowen@microchip.com, murthy.bhat@microchip.com,
        balsundar.p@microchip.com, joseph.szczypek@hpe.com,
        jeff@canonical.com, POSWALD@suse.com, mwilck@suse.com,
        pmenzel@molgen.mpg.de, linux-kernel@vger.kernel.org
References: <20210928235442.201875-1-don.brace@microchip.com>
 <20210928235442.201875-6-don.brace@microchip.com>
From:   john.p.donnelly@oracle.com
Message-ID: <ac3f4d4b-b1f2-dfa0-f042-d158ee8b0e95@oracle.com>
Date:   Thu, 30 Sep 2021 13:23:12 -0500
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0)
 Gecko/20100101 Thunderbird/78.14.0
In-Reply-To: <20210928235442.201875-6-don.brace@microchip.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?N0dZdHhLNkg5QjFadkwxTVI0RXdRZllYRExYZkhRMlRGVTlXSUU1dkloQ0hl?=
 =?utf-8?B?SzZmM3pzVlZsaHN4bTV0K1Y3NnhrSkdwb2Rab2JERlJpaGlDaUVHT2ZibGxB?=
 =?utf-8?B?RGdJdHVHbDd2ckhabGFSVzAvK2l2RFQ0VWNlUWdVbFdXdlo1VzFqYS9tcHNw?=
 =?utf-8?B?bGdxTTdLWjNVaXJiU24rS2lDbFlVYkhQeHM1Q0hRUk8wbFpIOVRYaVlmSW1B?=
 =?utf-8?B?SmRoeGFkdmFKQkorc1RLb2lzSmJ6VmdJZ0UvWnFJb3JodEdCNEhVeVZqSzJ4?=
 =?utf-8?B?dGJrbVhxeHJ6RE40eHpNNUtKeXVYQmhkVitKSGRyL3NzTVV2bi9QdndXS284?=
 =?utf-8?B?RGpkYmJtR2RKRTE5cWdJUzdUR3lFb2w0UEEyK1hTTGk3czEwUEtIaWdFYWRh?=
 =?utf-8?B?Slh4dWtlYndHcEcyVE01TVorbER0aFR2MG5qS3FBYUZCQXJhVFdhSVFXa29h?=
 =?utf-8?B?NGU1VEFIUWFGd1NBOVV5WTAzYWsrTENxc2hBWXRPY1ozbUpBd21VaHhPRnlh?=
 =?utf-8?B?bS9GcDJPdSs4T0FiWVp3azZPSjZoUGhsVVBTVHpqdzFNZW1ub1pUd2lWZldK?=
 =?utf-8?B?TitKMDVXcUQ1ZEtyVnNGWmdYYjVEcVVtWkt3NDEyTHh3RjJHbVZpT2JUSzlt?=
 =?utf-8?B?UnZKKzAyNmQrS1BRVngrQlZVNEh5bC91SjlhT3ZiWlNnUStWTWZoalJNY0hX?=
 =?utf-8?B?SHkrOGxOMDJRWUZhaEphMlI1enl5RXpOc0JDWGZWSm41YURjYjEvTVgrcXBZ?=
 =?utf-8?B?ZXVPSFZ4RjR4M1FKd1VrYVQyUTNGYmxod2ZGY2I3WlJLL2E1YjlobkxSb0lx?=
 =?utf-8?B?Q3luSEtjVmVOYWVsaEN3SkJrUEUrNmkzR3lUdTQvZEt0N1NQUGJMUDMwNGU1?=
 =?utf-8?B?ODJWZG41RlJMVWlQRlVReUt0YnQzVVJnMWswYSsydkdjNVVDUDUrNi9NalE5?=
 =?utf-8?B?QlVCWC8xWTg4cWlzYmFhdkY0dlU1ZDJkU0hsTjdXd0d1aDFMK2Z1b0J1NjI4?=
 =?utf-8?B?QmRzNFpuOGRySUFsYk9XR2N5SEhzMkZrVE5Jb2tSMVc0TTBwU1NLd0ZSc2da?=
 =?utf-8?B?VXVYOTZCVmZqREovcjRzcmJmck9JdEFqN0QvUTRyWVFqWUhBTTF1bXlqUW1W?=
 =?utf-8?B?OGNka0R0RGtyV1NtNWZyRFB5d25ORjVaSzR4TzVGcnh4UEZaNFo2RkdTSkxp?=
 =?utf-8?B?OUF6VWVaTVlnV1UwU0NsS2xKcUltTm1ZVHJVQm5hM0l2OStxWEJMUGUvdmtR?=
 =?utf-8?B?dTNpMFlGNmhOY040a1BET0xUT1B2N0NTOE9nNFRGdGZDZmgvVHF4b3phSmNI?=
 =?utf-8?B?aUUyQ1hIT1JYd2xWMEdDNFFvaXdaVmxDa2VkVnJFQU03WmkwR1pYbUZCV0lh?=
 =?utf-8?B?dkRNdHRaNW85dzBUVEp5NEMwdy8yZU1qRnpDblh6Um01eDVuUFg1S09tRWIr?=
 =?utf-8?B?WDBnT0JrYnpDQVROTzlVTG43ZDVXT0w1Sk5aN3VLT0tZN1d6T3I2dnVQS2NW?=
 =?utf-8?B?bzBQRC9ZM1ZlbVhyZnplaU11L25KZ2Fha0Q0V1JMSUhielVuYkVCd3NBL1dz?=
 =?utf-8?B?Z1BYcmN2N0grS0ZSNzdOWW1MUDdRN2F1MVJmVXNLRjhISExaU3dIaUF6V1h4?=
 =?utf-8?B?Mm91OFQ2Q1h1a1JHZ1ZqbSttait0ZTRhTkZ3cDBsSHBjUjVXVCtxem5ZeUNM?=
 =?utf-8?B?OFB1RFZIOVRocmJCcjlMbDcwZE5vd2M3RFRlbEZSTlVoT01TUkdiRUlOMEhy?=
 =?utf-8?Q?yrf1Y61lE5sg4dD7PHE8i86xbP7hzcFhPAGDgoM?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: efd145da-6e27-4301-720c-08d9843f5fcd
X-MS-Exchange-CrossTenant-AuthSource: CO1PR10MB4722.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Sep 2021 18:23:17.6055
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: cZ3cvrsWhHX3zXLDQGQGIJ4t5vd0aDFOxENBLx1k3bDdAW2+vvdb4vcQ0VvcJTOVw00hUxgVO1F3k8XdOeXQZ3DbQkURodYDTvcn23ITQUk=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO6PR10MB5539
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10123 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 suspectscore=0
 mlxscore=0 spamscore=0 adultscore=0 bulkscore=0 phishscore=0
 malwarescore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2109230001 definitions=main-2109300112
X-Proofpoint-GUID: 3PnH6ow0D4s445K1CSAWmci2JSf2U0VS
X-Proofpoint-ORIG-GUID: 3PnH6ow0D4s445K1CSAWmci2JSf2U0VS
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 9/28/21 6:54 PM, Don Brace wrote:
> Add in a TUR to HBA disks and do not present them to the OS if
> 0x02/0x04/0x1b (sanitize in progress) is returned.
> 
> During boot-up, some OSes appear to hang when there are one or
> more disks undergoing sanitize.
> 
> According to SCSI SBC4 specification
> section 4.11.2 Commands allowed during sanitize,
> some SCSI commands are permitted, but read/write operations are not.
> 
> When the OS attempts to read the disk partition table a
> CHECK CONDITION ASC 0x04 ASCQ 0x1b is returned which causes the OS
> to retry the read until sanitize has completed. This can take hours.
> 
> Note: According to document HPE Smart Storage Administrator User Guide
> Link: https://support.hpe.com/hpesc/public/docDisplay?docLocale=en_US&docId=c03909334
> 
> During the sanitize erase operation, the drive is unusable.
> I.E.
>        The expected behavior for sanitize is the that disk remains
>        offline even after sanitize has completed. The customer is
>        expected to re-enable the disk using the management utility.
> 
> Reviewed-by: Scott Benesh <scott.benesh@microchip.com>
> Reviewed-by: Scott Teel <scott.teel@microchip.com>
> Reviewed-by: Mike McGowen <mike.mcgowen@microchip.com>
> Signed-off-by: Don Brace <don.brace@microchip.com>
> ---

Acked-by: John Donnelly <john.p.donnelly@oracle.com>


>   drivers/scsi/smartpqi/smartpqi_init.c | 87 +++++++++++++++++++++++++++
>   1 file changed, 87 insertions(+)
> 
> diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c
> index 01330fd67500..838274d8fadf 100644
> --- a/drivers/scsi/smartpqi/smartpqi_init.c
> +++ b/drivers/scsi/smartpqi/smartpqi_init.c
> @@ -555,6 +555,10 @@ static int pqi_build_raid_path_request(struct pqi_ctrl_info *ctrl_info,
>   	cdb = request->cdb;
>   
>   	switch (cmd) {
> +	case TEST_UNIT_READY:
> +		request->data_direction = SOP_READ_FLAG;
> +		cdb[0] = TEST_UNIT_READY;
> +		break;
>   	case INQUIRY:
>   		request->data_direction = SOP_READ_FLAG;
>   		cdb[0] = INQUIRY;
> @@ -1575,6 +1579,85 @@ static int pqi_get_logical_device_info(struct pqi_ctrl_info *ctrl_info,
>   	return rc;
>   }
>   
> +/*
> + * Prevent adding drive to OS for some corner cases such as a drive
> + * undergoing a sanitize operation. Some OSes will continue to poll
> + * the drive until the sanitize completes, which can take hours,
> + * resulting in long bootup delays. Commands such as TUR, READ_CAP
> + * are allowed, but READ/WRITE cause check condition. So the OS
> + * cannot check/read the partition table.
> + * Note: devices that have completed sanitize must be re-enabled
> + *       using the management utility.
> + */
> +static bool pqi_keep_device_offline(struct pqi_ctrl_info *ctrl_info,
> +	struct pqi_scsi_dev *device)
> +{
> +	u8 scsi_status;
> +	int rc;
> +	enum dma_data_direction dir;
> +	char *buffer;
> +	int buffer_length = 64;
> +	size_t sense_data_length;
> +	struct scsi_sense_hdr sshdr;
> +	struct pqi_raid_path_request request;
> +	struct pqi_raid_error_info error_info;
> +	bool offline = false; /* Assume keep online */
> +
> +	/* Do not check controllers. */
> +	if (pqi_is_hba_lunid(device->scsi3addr))
> +		return false;
> +
> +	/* Do not check LVs. */
> +	if (pqi_is_logical_device(device))
> +		return false;
> +
> +	buffer = kmalloc(buffer_length, GFP_KERNEL);
> +	if (!buffer)
> +		return false; /* Assume not offline */
> +
> +	/* Check for SANITIZE in progress using TUR */
> +	rc = pqi_build_raid_path_request(ctrl_info, &request,
> +		TEST_UNIT_READY, RAID_CTLR_LUNID, buffer,
> +		buffer_length, 0, &dir);
> +	if (rc)
> +		goto out; /* Assume not offline */
> +
> +	memcpy(request.lun_number, device->scsi3addr, sizeof(request.lun_number));
> +
> +	rc = pqi_submit_raid_request_synchronous(ctrl_info, &request.header, 0, &error_info);
> +
> +	if (rc)
> +		goto out; /* Assume not offline */
> +
> +	scsi_status = error_info.status;
> +	sense_data_length = get_unaligned_le16(&error_info.sense_data_length);
> +	if (sense_data_length == 0)
> +		sense_data_length =
> +			get_unaligned_le16(&error_info.response_data_length);
> +	if (sense_data_length) {
> +		if (sense_data_length > sizeof(error_info.data))
> +			sense_data_length = sizeof(error_info.data);
> +
> +		/*
> +		 * Check for sanitize in progress: asc:0x04, ascq: 0x1b
> +		 */
> +		if (scsi_status == SAM_STAT_CHECK_CONDITION &&
> +			scsi_normalize_sense(error_info.data,
> +				sense_data_length, &sshdr) &&
> +				sshdr.sense_key == NOT_READY &&
> +				sshdr.asc == 0x04 &&
> +				sshdr.ascq == 0x1b) {
> +			device->device_offline = true;
> +			offline = true;
> +			goto out; /* Keep device offline */
> +		}
> +	}
> +
> +out:
> +	kfree(buffer);
> +	return offline;
> +}
> +
>   static int pqi_get_device_info(struct pqi_ctrl_info *ctrl_info,
>   	struct pqi_scsi_dev *device,
>   	struct bmic_identify_physical_device *id_phys)
> @@ -2296,6 +2379,10 @@ static int pqi_update_scsi_devices(struct pqi_ctrl_info *ctrl_info)
>   		if (!pqi_is_supported_device(device))
>   			continue;
>   
> +		/* Do not present disks that the OS cannot fully probe */
> +		if (pqi_keep_device_offline(ctrl_info, device))
> +			continue;
> +
>   		/* Gather information about the device. */
>   		rc = pqi_get_device_info(ctrl_info, device, id_phys);
>   		if (rc == -ENOMEM) {
>