Re: [PATCH v2 00/30] Optimize the hot path in the UFS driver

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

On 8/14/25 1:40 AM, John Garry wrote:
> Some further initial points on this series:
> - the driver still has the separate tmf tag set. Why cannot the tmf be 
> allocated as a reserved command in the shost tagset?

The UFSHCI standard defines different tag spaces for TMFs and for UFS
commands: [0..hba->nutmrs) for TMFs and [0..hba->nutrs) for UFS
commands. These ranges have to be kept separate because of how UFS host
controllers use these numbers. The TMF tag is passed to UFS host
controllers by writing it into a bitwise register. From
__ufshcd_issue_tm_cmd():

  	ufshcd_writel(hba, 1 << task_tag, REG_UTP_TASK_REQ_DOOR_BELL);

The UFS command tag ends up in a protocol header. From 
ufshcd_prepare_utp_scsi_cmd_upiu():

	ucd_req_ptr->header = (struct utp_upiu_header){
		[ ... ]
		.task_tag = lrbp->task_tag,
		[ ... ]
	};

Mixing up TMFs and the reserved UFS command and allocating all these
from a single tag space would require a additional code and data
structures to translate reserved tags from the [0..hba->nutmrs]
range to the [0..hba->nutmrs) range. In other words, code would become
more complex and harder to maintain. Hence, I prefer to keep the TMF tag
set.

> - I like that you are using blk_execute_rq(), but why do we need the 
> pseudo sdev (and not the ufs sdev)? The idea of the psuedo sdev was 
> originally for sending reserved commands for the host.

In the UFS driver several reserved commands are sent before
ufshcd_scsi_add_wlus() and scsi_scan_host() are called. Hence, the
pseudo sdev is the only sdev that is available when sending reserved
commands like the initial NOP OUT. Allocating the well-known LUNs before
sending the initial NOP OUT is not possible because ufshcd_sdev_init() 
gets called while adding WLUNs and there is code in that function that 
is based on the assumption that UFS device initialization has completed
(ufshcd_lu_init() calls ufshcd_read_unit_desc_param()).

> - IIRC, I was advised to have a check in the scsi core dispatch command 
> patch to check for a reserved command, and have a separate handler for 
> that, i.e. don't use sht->queuecommand for reserved commands. I can try 
> to find the exact discussion if you like.

It would be appreciated if a link to that conversation could be shared.

Thanks,

Bart.