Re: [PATCH v1 15/19] cxl: Add support to handle user feature commands for get feature

[Jonathan Cameron <Jonathan.Cameron@huawei.com>]

On Wed, 22 Jan 2025 16:50:46 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> Add helper function to parse the user data from fwctl RPC ioctl and
> send the parsed input parameters to cxl_get_feature() call.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Trivial comment inline.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  drivers/cxl/features.c | 46 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/drivers/cxl/features.c b/drivers/cxl/features.c
> index e65fc8479d21..7c726f1512d0 100644
> --- a/drivers/cxl/features.c
> +++ b/drivers/cxl/features.c
> @@ -146,6 +146,51 @@ static void *cxlctl_get_supported_features(struct cxl_features_state *cfs,
>  	return no_free_ptr(rpc_out);
>  }
>  
> +static void *cxlctl_get_feature(struct cxl_features_state *cfs,
> +				const struct fwctl_rpc_cxl *rpc_in,
> +				size_t *out_len)
> +{
> +	struct cxl_mbox_get_feat_in feat_in;
> +	u16 offset, count, return_code;
> +	size_t out_size = *out_len;
> +	void *feat_out;
> +
> +	if (rpc_in->op_size != sizeof(feat_in))
> +		return ERR_PTR(-EINVAL);
> +
> +	if (copy_from_user(&feat_in, u64_to_user_ptr(rpc_in->in_payload),
> +			   rpc_in->op_size))
> +		return ERR_PTR(-EFAULT);
> +
> +	offset = le16_to_cpu(feat_in.offset);
> +	count = le16_to_cpu(feat_in.count);
> +
> +	if (!count)
> +		return ERR_PTR(-EINVAL);
> +
> +	struct fwctl_rpc_cxl_out *rpc_out __free(kvfree) =
> +		kvzalloc(out_size, GFP_KERNEL);
> +	if (!rpc_out)
> +		return ERR_PTR(-ENOMEM);
> +
> +	feat_out = (void *)rpc_out->payload;

Fairly sure no need for the cast.  Should be fine given it's a void *
that you are assigning it to from another pointer type.
As such, maybe just use it inline in next call?

> +	out_size = cxl_get_feature(cfs->features, feat_in.uuid,
> +				   feat_in.selection, feat_out,
> +				   count, offset, &return_code);
> +	*out_len = sizeof(struct fwctl_rpc_cxl_out);
> +	if (!out_size) {
> +		rpc_out->size = 0;
> +		rpc_out->retval = return_code;
> +		return no_free_ptr(rpc_out);
> +	}
> +
> +	rpc_out->size = out_size;
> +	rpc_out->retval = CXL_MBOX_CMD_RC_SUCCESS;
> +	*out_len += out_size;
> +
> +	return no_free_ptr(rpc_out);
> +}