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

[Dave Jiang <dave.jiang@intel.com>]

On 8/25/26 6: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@nvidia.com>
> ---
> Changelog:
> 
> v1 -> v2:
> 	- Remove redundant CXL_MBOX_CMD_RC_SUCCESS assignments. (Dave Jiang)
> 
> ---
>  drivers/cxl/core/features.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
> index 8d44ce829497..28b326df3398 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;
>  
>  	if (!feat_out || !feat_out_size)
>  		return -EINVAL;
> @@ -267,9 +267,6 @@ ssize_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
>  		data_rcvd_size += mbox_cmd.size_out;
>  	} while (data_rcvd_size < feat_out_size);
>  
> -	if (return_code)
> -		*return_code = CXL_MBOX_CMD_RC_SUCCESS;
> -
>  	return data_rcvd_size;
>  }
>  
> @@ -289,7 +286,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;
>  
>  	if (feat_data_size > U16_MAX - offset)
>  		return -EINVAL;
> @@ -341,8 +338,6 @@ int cxl_set_feature(struct cxl_mailbox *cxl_mbox,
>  
>  		data_sent_size += data_in_size;
>  		if (data_sent_size >= feat_data_size) {
> -			if (return_code)
> -				*return_code = CXL_MBOX_CMD_RC_SUCCESS;
>  			return 0;
>  		}

Minor. the {} can now go.

>  
> @@ -492,6 +487,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 &&

Do we need to handle the 'data_size == 0' case here?

> +	    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 +542,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;


We may also need to add something like this because of the change of default return code. The code segment is similar to cxl_xfer_log().

diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
index 28b326df3398..2e62f3720072 100644
--- a/drivers/cxl/core/features.c
+++ b/drivers/cxl/core/features.c
@@ -259,6 +259,17 @@ ssize_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
                        .min_out = data_to_rd_size,
                };
                rc = cxl_internal_send_cmd(cxl_mbox, &mbox_cmd);
+               /*
+                * Per CXL r4.0 8.2.10.6.2, when Offset + Count runs past the
+                * end of the Feature the device returns only the bytes up to
+                * the Feature size. cxl_internal_send_cmd() reports that as
+                * -EIO with a short payload, so stop and return what arrived.
+                */
+               if (rc == -EIO && mbox_cmd.size_out &&
+                   mbox_cmd.size_out < data_to_rd_size) {
+                       data_rcvd_size += mbox_cmd.size_out;
+                       break;
+               }
                if (rc < 0 || !mbox_cmd.size_out) {
                        if (return_code)
                                *return_code = mbox_cmd.return_code;