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

[Richard Cheng <icheng@nvidia.com>]

On Thu, Aug 27, 2026 at 03:11:09PM +0800, Dave Jiang wrote:
> 
> 
> 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.
>

Ahh sure, thanks, I'll remove it in v7.

Best regards,
Richard Cheng.

 
> >  
> > @@ -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;