Re: [RFC PATCH 10/13] fwctl/cxl: Add support for fwctl RPC command to enable CXL feature commands

[Jason Gunthorpe <jgg@nvidia.com>]

On Thu, Jul 18, 2024 at 02:32:28PM -0700, Dave Jiang wrote:
> diff --git a/include/uapi/fwctl/cxl.h b/include/uapi/fwctl/cxl.h
> index d3a735f1fe4e..de8949a28473 100644
> --- a/include/uapi/fwctl/cxl.h
> +++ b/include/uapi/fwctl/cxl.h
> @@ -22,4 +22,68 @@ struct fwctl_info_cxl {
>  	__u32 uctx_caps;
>  };
>  
> +/*
> + * CXL spec r3.1 Table 8-101 Set Feature Input Payload
> + */
> +struct set_feature_input {
> +	uuid_t uuid;
> +	__u32 flags;
> +	__u16 offset;
> +	__u8 version;
> +	__u8 reserved[9];
> +	__u8 data[];
> +} __packed;

This struct probably needs to be prefixed with fwcl_, but should it be
here I wonder or should the CXL defined structs be someplace else?

> +/**
> + * struct cxl_send_command - Send a command to a memory device.
> + * @id: The command to send to the memory device. This must be one of the
> + *	commands returned by the query command.
> + * @flags: Flags for the command (input).
> + * @raw: Special fields for raw commands
> + * @raw.opcode: Opcode passed to hardware when using the RAW command.
> + * @raw.rsvd: Must be zero.
> + * @rsvd: Must be zero.
> + * @retval: Return value from the memory device (output).
> + * @in: Parameters associated with input payload.
> + * @in.size: Size of the payload to provide to the device (input).
> + * @in.rsvd: Must be zero.
> + * @in.payload: Pointer to memory for payload input, payload is little endian.
> + *
> + * Output payload is defined with 'struct fwctl_rpc' and is the hardware output
> + */
> +struct fwctl_cxl_command {
> +	__u32 id;
> +	__u32 flags;
> +	union {
> +		struct {
> +			__u16 opcode;
> +			__u16 rsvd;
> +		} raw;
> +		__u32 rsvd;
> +	};
> +
> +	struct {
> +		__u32 size;
> +		__u32 rsvd;
> +		__u64 payload;
> +	} in;
> +};

Why all the structs and unions?

> +/**
> + * struct fwctl_rpc_cxl - ioctl(FWCTL_RPC) input
> + */
> +struct fwctl_rpc_cxl {
> +	__u32 rpc_cmd;
> +	__u32 payload_size;
> +	__u32 version;
> +	__u32 rsvd;
> +	struct fwctl_cxl_command send_cmd;
> +};
> +
> +struct fwctl_rpc_cxl_out {
> +	__u32 retval;
> +	__u32 rsvd;

> +	__u8 payload[];

This might be better as 

  __aligned_u64 payload[]

Which will force a 8 byte alignment of the struct which will allow the
payload to be casted to aligned u64 reliably.

How do you know the length of payload?

Jason