Re: [PATCH v7 1/3] bsg: add bsg_uring_cmd uapi structure

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

On 3/12/26 2:22 AM, Yang Xiuwei wrote:
> +struct bsg_uring_cmd {
> +	__u64 request;		/* [i], [*i] command descriptor address */
> +	__u32 request_len;	/* [i] command descriptor length in bytes */
> +	__u32 protocol;		/* [i] protocol type (BSG_PROTOCOL_*) */
> +	__u32 subprotocol;	/* [i] subprotocol type (BSG_SUB_PROTOCOL_*) */
> +	__u32 max_response_len;	/* [i] response buffer size in bytes */
> +
> +	__u64 response;		/* [i], [*o] response data address */
> +	__u64 dout_xferp;	/* [i], [*i] */
> +	__u32 dout_xfer_len;	/* [i] bytes to be transferred to device */
> +	__u32 dout_iovec_count;	/* [i] 0 -> "flat" dout transfer else
> +				 * dout_xferp points to array of iovec
> +				 */
> +	__u64 din_xferp;	/* [i], [*o] */
> +	__u32 din_xfer_len;	/* [i] bytes to be transferred from device */
> +	__u32 din_iovec_count;	/* [i] 0 -> "flat" din transfer */
> +
> +	__u32 timeout_ms;	/* [i] timeout in milliseconds */
> +	__u8  reserved[12];	/* reserved for future extension */
> +};

Please consider adding a static_assert() statement that verifies the
size of this data structure at compile time. Such a statement is useful
to document the size of the data structure, helps with verifying that
the size is the same on all architectures and helps with verifying that
the size doesn't change if a reserved byte is taken in use.

> +#define BSG_SCSI_RES2_DEVICE_STATUS(res2)   ((__u8)((__u64)(res2) & 0xff))
> +#define BSG_SCSI_RES2_DRIVER_STATUS(res2)   ((__u8)((__u64)(res2) >> 8))
> +#define BSG_SCSI_RES2_HOST_STATUS(res2)     ((__u8)((__u64)(res2) >> 16))
> +#define BSG_SCSI_RES2_SENSE_LEN(res2)       ((__u8)((__u64)(res2) >> 24))
> +#define BSG_SCSI_RES2_RESID_LEN(res2)       ((__u32)((__u64)(res2) >> 32))
> +
> +#define BSG_SCSI_RES2_BUILD(device_status, driver_status, host_status,   \
> +			    sense_len_wr, resid_len)			\
> +	(((__u64)(__u32)(resid_len) << 32) |				\
> +	 ((__u64)(__u8)(sense_len_wr) << 24) |				\
> +	 ((__u64)(__u8)(host_status) << 16) |				\
> +	 ((__u64)(__u8)(driver_status) << 8) |				\
> +	 ((__u64)(__u8)(device_status)))

Please convert all the above macros into inline functions because that
will result in removal of most of the typecasts. I think that inline
functions are allowed in uapi headers:

$ git grep 'static.*inline' include/uapi | wc -l
354

Thanks,

Bart.