Re: [PATCH v4] scsi: ufs: Zero utp_upiu_req at the beginning of each command

[Bart Van Assche <bvanassche@acm.org>]

On 9/20/24 11:23 PM, Avri Altman wrote:
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 8ea5a82503a9..9187cf5c949c 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -2761,7 +2761,6 @@ void ufshcd_prepare_utp_scsi_cmd_upiu(struct ufshcd_lrb *lrbp, u8 upiu_flags)
>   	ucd_req_ptr->sc.exp_data_transfer_len = cpu_to_be32(cmd->sdb.length);
>   
>   	cdb_len = min_t(unsigned short, cmd->cmd_len, UFS_CDB_SIZE);
> -	memset(ucd_req_ptr->sc.cdb, 0, UFS_CDB_SIZE);
>   	memcpy(ucd_req_ptr->sc.cdb, cmd->cmnd, cdb_len);
>   
>   	memset(lrbp->ucd_rsp_ptr, 0, sizeof(struct utp_upiu_rsp));
> @@ -2864,6 +2863,26 @@ static void ufshcd_comp_scsi_upiu(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
>   	ufshcd_prepare_utp_scsi_cmd_upiu(lrbp, upiu_flags);
>   }
>   
> +static void __ufshcd_setup_cmd(struct ufshcd_lrb *lrbp, struct scsi_cmnd *cmd, u8 lun, int tag)
> +{
> +	memset(lrbp->ucd_req_ptr, 0, sizeof(*lrbp->ucd_req_ptr));
> +
> +	lrbp->cmd = cmd;
> +	lrbp->task_tag = tag;
> +	lrbp->lun = lun;
> +	ufshcd_prepare_lrbp_crypto(cmd ? scsi_cmd_to_rq(cmd) : NULL, lrbp);
> +}
> +
> +static void ufshcd_setup_scsi_cmd(struct ufs_hba *hba, struct ufshcd_lrb *lrbp,
> +				  struct scsi_cmnd *cmd, u8 lun, int tag)
> +{
> +	__ufshcd_setup_cmd(lrbp, cmd, lun, tag);
> +	lrbp->intr_cmd = !ufshcd_is_intr_aggr_allowed(hba);
> +	lrbp->req_abort_skip = false;
> +
> +	ufshcd_comp_scsi_upiu(hba, lrbp);
> +}
> +
>   /**
>    * ufshcd_upiu_wlun_to_scsi_wlun - maps UPIU W-LUN id to SCSI W-LUN ID
>    * @upiu_wlun_id: UPIU W-LUN id
> @@ -2997,16 +3016,8 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
>   	ufshcd_hold(hba);
>   
>   	lrbp = &hba->lrb[tag];
> -	lrbp->cmd = cmd;
> -	lrbp->task_tag = tag;
> -	lrbp->lun = ufshcd_scsi_to_upiu_lun(cmd->device->lun);
> -	lrbp->intr_cmd = !ufshcd_is_intr_aggr_allowed(hba);
>   
> -	ufshcd_prepare_lrbp_crypto(scsi_cmd_to_rq(cmd), lrbp);
> -
> -	lrbp->req_abort_skip = false;
> -
> -	ufshcd_comp_scsi_upiu(hba, lrbp);
> +	ufshcd_setup_scsi_cmd(hba, lrbp, cmd, ufshcd_scsi_to_upiu_lun(cmd->device->lun), tag);
>   
>   	err = ufshcd_map_sg(hba, lrbp);
>   	if (err) {
> @@ -3034,11 +3045,8 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
>   static void ufshcd_setup_dev_cmd(struct ufs_hba *hba, struct ufshcd_lrb *lrbp,
>   			     enum dev_cmd_type cmd_type, u8 lun, int tag)
>   {
> -	lrbp->cmd = NULL;
> -	lrbp->task_tag = tag;
> -	lrbp->lun = lun;
> +	__ufshcd_setup_cmd(lrbp, NULL, lun, tag);
>   	lrbp->intr_cmd = true; /* No interrupt aggregation */
> -	ufshcd_prepare_lrbp_crypto(NULL, lrbp);
>   	hba->dev_cmd.type = cmd_type;
>   }
>   

This should have been two patches: one patch that introduces the
ufshcd_setup_scsi_cmd() function and a second patch that adds the code
for zeroing the UPIU header. Anyway, I'm fine with this patch.

Bart.