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

Jason Gunthorpe
2024-07-22T15:29:39Z

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