Re: [PATCH v5 7/7] cxl/fwctl: Propagate feature RPC delivery errors

[Dave Jiang]

On 8/12/26 8:45 PM, Richard Cheng wrote:
> FWCTL_RPC requires delivery failures to be returned as ioctl errors,
> while device errors are reported in the output. Get and Set Feature
> instead converted all failures into normal responses, sometimes with a
> SUCCESS device status.
> 
> Initialize the return code to SUCCESS. When the helper fails without a
> device error code, return its errno. Continue reporting actual device
> errors through rpc_out->retval.
> 
> Fixes: 5908f3ed6dc2 ("cxl: Add support to handle user feature commands for get feature")
> Fixes: eb5dfcb9e36d ("cxl: Add support to handle user feature commands for set feature")
> Signed-off-by: Richard Cheng <icheng@xxxxxxxxxx>
> ---
>  drivers/cxl/core/features.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
> index 8d44ce829497..30e00faaf3a9 100644
> --- a/drivers/cxl/core/features.c
> +++ b/drivers/cxl/core/features.c
> @@ -232,7 +232,7 @@ ssize_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
>  	int rc;
>  
>  	if (return_code)
> -		*return_code = CXL_MBOX_CMD_RC_INPUT;
> +		*return_code = CXL_MBOX_CMD_RC_SUCCESS;

With "success" set here, should we drop setting of *return_code at the end of function since it's no longer necessary?

>  
>  	if (!feat_out || !feat_out_size)
>  		return -EINVAL;
> @@ -289,7 +289,7 @@ int cxl_set_feature(struct cxl_mailbox *cxl_mbox,
>  	size_t hdr_size;
>  
>  	if (return_code)
> -		*return_code = CXL_MBOX_CMD_RC_INPUT;
> +		*return_code = CXL_MBOX_CMD_RC_SUCCESS;

I think similar for this function as the previous comment?

DJ

>  
>  	if (feat_data_size > U16_MAX - offset)
>  		return -EINVAL;
> @@ -492,6 +492,9 @@ static void *cxlctl_get_feature(struct cxl_features_state *cxlfs,
>  	data_size = cxl_get_feature(cxl_mbox, &feat_in->uuid,
>  				    feat_in->selection, rpc_out->payload,
>  				    count, offset, &return_code);
> +	if (data_size < 0 &&
> +	    return_code == CXL_MBOX_CMD_RC_SUCCESS)
> +		return ERR_PTR(data_size);
>  	*out_len = sizeof(struct fwctl_rpc_cxl_out);
>  	if (data_size <= 0) {
>  		rpc_out->size = 0;
> @@ -544,6 +547,8 @@ static void *cxlctl_set_feature(struct cxl_features_state *cxlfs,
>  	rc = cxl_set_feature(cxl_mbox, &feat_in->uuid,
>  			     feat_in->version, feat_in->feat_data,
>  			     data_size, flags, offset, &return_code);
> +	if (rc && return_code == CXL_MBOX_CMD_RC_SUCCESS)
> +		return ERR_PTR(rc);
>  	*out_len = sizeof(*rpc_out);
>  	if (rc) {
>  		rpc_out->retval = return_code;