Re: [PATCH RFC 03/11] nvmet: Add nvmet_fabrics_ops flag to indicate SGLs not supported

[Damien Le Moal <dlemoal@kernel.org>]

On 3/13/25 14:18, Mike Christie wrote:
> The nvmet_mdev_pci driver does not initially support SGLs. In some
> prelim testing I don't think there will be a perf gain (the virt related
> interface may be the major bottleneck so I may not notice) so I wasn't
> sure if they will be required/needed. This adds a nvmet_fabrics_ops flag
> so we can tell nvmet core to tell the host we do not supports SGLS.

That is a major spec violation as NVMe fabrics mandates SGL support.
So at the very least, this needs a big fat warning comment explaining what this
is about.

> 
> Signed-off-by: Mike Christie <michael.christie@oracle.com>
> ---
>  drivers/nvme/target/admin-cmd.c | 13 +++++++------
>  drivers/nvme/target/nvmet.h     |  1 +
>  2 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
> index acc138bbf8f2..486ed6f7b717 100644
> --- a/drivers/nvme/target/admin-cmd.c
> +++ b/drivers/nvme/target/admin-cmd.c
> @@ -755,12 +755,13 @@ static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
>  	id->awun = 0;
>  	id->awupf = 0;
>  
> -	/* we always support SGLs */
> -	id->sgls = cpu_to_le32(NVME_CTRL_SGLS_BYTE_ALIGNED);
> -	if (ctrl->ops->flags & NVMF_KEYED_SGLS)
> -		id->sgls |= cpu_to_le32(NVME_CTRL_SGLS_KSDBDS);
> -	if (req->port->inline_data_size)
> -		id->sgls |= cpu_to_le32(NVME_CTRL_SGLS_SAOS);
> +	if (!(ctrl->ops->flags & NVMF_SGLS_NOT_SUPP)) {
> +		id->sgls = cpu_to_le32(NVME_CTRL_SGLS_BYTE_ALIGNED);
> +		if (ctrl->ops->flags & NVMF_KEYED_SGLS)
> +			id->sgls |= cpu_to_le32(NVME_CTRL_SGLS_KSDBDS);
> +		if (req->port->inline_data_size)
> +			id->sgls |= cpu_to_le32(NVME_CTRL_SGLS_SAOS);
> +	}
>  
>  	strscpy(id->subnqn, ctrl->subsys->subsysnqn, sizeof(id->subnqn));
>  
> diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
> index fcf4f460dc9a..ec3d10eb316a 100644
> --- a/drivers/nvme/target/nvmet.h
> +++ b/drivers/nvme/target/nvmet.h
> @@ -403,6 +403,7 @@ struct nvmet_fabrics_ops {
>  	unsigned int flags;
>  #define NVMF_KEYED_SGLS			(1 << 0)
>  #define NVMF_METADATA_SUPPORTED		(1 << 1)
> +#define NVMF_SGLS_NOT_SUPP		(1 << 2)
>  	void (*queue_response)(struct nvmet_req *req);
>  	int (*add_port)(struct nvmet_port *port);
>  	void (*remove_port)(struct nvmet_port *port);


-- 
Damien Le Moal
Western Digital Research