Re: [PATCH 04/20] ublk: add integrity UAPI

[Ming Lei <ming.lei@redhat.com>]

On Tue, Dec 16, 2025 at 10:34:38PM -0700, Caleb Sander Mateos wrote:
> From: Stanley Zhang <stazhang@purestorage.com>
> 
> Add UAPI definitions for metadata/integrity support in ublk.
> UBLK_PARAM_TYPE_INTEGRITY and struct ublk_param_integrity allow a ublk
> server to specify the integrity params of a ublk device.
> The ublk driver will set UBLK_IO_F_INTEGRITY in the op_flags field of
> struct ublksrv_io_desc for requests with integrity data.
> The ublk server uses user copy with UBLKSRV_IO_INTEGRITY_FLAG set in the
> offset parameter to access a request's integrity buffer.
> 
> Signed-off-by: Stanley Zhang <stazhang@purestorage.com>
> [csander: drop feature flag and redundant pi_tuple_size field,
>  add io_desc flag, use block metadata UAPI constants]
> Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
> ---
>  include/uapi/linux/ublk_cmd.h | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h
> index ec77dabba45b..5bfb9a0521c3 100644
> --- a/include/uapi/linux/ublk_cmd.h
> +++ b/include/uapi/linux/ublk_cmd.h
> @@ -129,11 +129,15 @@
>  #define UBLK_QID_BITS		12
>  #define UBLK_QID_BITS_MASK	((1ULL << UBLK_QID_BITS) - 1)
>  
>  #define UBLK_MAX_NR_QUEUES	(1U << UBLK_QID_BITS)
>  
> -#define UBLKSRV_IO_BUF_TOTAL_BITS	(UBLK_QID_OFF + UBLK_QID_BITS)
> +/* Copy to/from request integrity buffer instead of data buffer */
> +#define UBLK_INTEGRITY_FLAG_OFF		(UBLK_QID_OFF + UBLK_QID_BITS)
> +#define UBLKSRV_IO_INTEGRITY_FLAG	(1ULL << UBLK_INTEGRITY_FLAG_OFF)
> +

I feel it is more readable to move the definition into the patch which uses
them.

> +#define UBLKSRV_IO_BUF_TOTAL_BITS	(UBLK_INTEGRITY_FLAG_OFF + 1)

It is UAPI, UBLKSRV_IO_BUF_TOTAL_BITS shouldn't be changed, or can you
explain this way is safe?

>  #define UBLKSRV_IO_BUF_TOTAL_SIZE	(1ULL << UBLKSRV_IO_BUF_TOTAL_BITS)
>  
>  /*
>   * ublk server can register data buffers for incoming I/O requests with a sparse
>   * io_uring buffer table. The request buffer can then be used as the data buffer
> @@ -406,10 +410,12 @@ struct ublksrv_ctrl_dev_info {
>   *
>   * ublk server has to check this flag if UBLK_AUTO_BUF_REG_FALLBACK is
>   * passed in.
>   */
>  #define		UBLK_IO_F_NEED_REG_BUF		(1U << 17)
> +/* Request has an integrity data buffer */
> +#define		UBLK_IO_F_INTEGRITY		(1U << 18)
>  
>  /*
>   * io cmd is described by this structure, and stored in share memory, indexed
>   * by request tag.
>   *
> @@ -598,10 +604,20 @@ struct ublk_param_segment {
>  	__u32 	max_segment_size;
>  	__u16 	max_segments;
>  	__u8	pad[2];
>  };
>  
> +struct ublk_param_integrity {
> +	__u32	flags; /* LBMD_PI_CAP_* from linux/fs.h */
> +	__u8	interval_exp;
> +	__u8	metadata_size;
> +	__u8	pi_offset;
> +	__u8	csum_type; /* LBMD_PI_CSUM_* from linux/fs.h */
> +	__u8	tag_size;
> +	__u8	pad[7];
> +};
> +

Just be curious, `pi_tuple_size` isn't defined, instead it is hard-coded in
ublk_integrity_pi_tuple_size().

However, both scsi and nvme sets `pi_tuple_size`, so it means that ublk PI
supports one `subset` or scsi/nvme `pi_tuple_size` can be removed too?


Thanks, 
Ming