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

[Richard Cheng <icheng@nvidia.com>]

On Tue, Aug 18, 2026 at 04:51:56PM +0800, Dave Jiang wrote:
> 
> 
> 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@nvidia.com>
> > ---
> >  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?
>

Hi Dave,

Thanks for pointing this out.
Yes, I think those assignments are redundant after init return_code to
CXL_MBOX_CMD_RC_SUCCESS.
I'll remove both in v6, thanks.

Best regards,
Richard Cheng.
 
> >  
> >  	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;
>