From: Alison Schofield <alison.schofield@intel.com>
To: Richard Cheng <icheng@nvidia.com>
Cc: <dave@stgolabs.net>, <jic23@kernel.org>, <dave.jiang@intel.com>,
<vishal.l.verma@intel.com>, <iweiny@kernel.org>,
<ming.li@zohomail.com>, <gourry@gourry.net>, <rrichter@amd.com>,
<linux-cxl@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
<kees@kernel.org>, <newtonl@nvidia.com>, <kristinc@nvidia.com>,
<kaihengf@nvidia.com>, <kobak@nvidia.com>
Subject: Re: [PATCH v7 7/7] cxl/fwctl: Propagate feature RPC delivery errors
Date: Thu, 3 Sep 2026 22:21:58 -0700 [thread overview]
Message-ID: <appVdg55jb6Y3zcs@aschofie-mobl2.lan> (raw)
In-Reply-To: <20260902053839.25595-8-icheng@nvidia.com>
On Wed, Sep 02, 2026 at 01:38:39PM +0800, 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.
>
> CXL permits Get Feature to return a nonzero short payload when Offset +
> Count runs past the end of the Feature. cxl_internal_send_cmd() reports
> that response as -EIO, so preserve the returned bytes as a successful
> partial transfer. Fixed-format EDAC callers still require complete
> attribute structures, so reject partial payloads before consuming them.
>
> Map an unexpected zero-length result with a SUCCESS device status to
> -EIO.
Reviewed-by: Alison Schofield <alison.schofield@intel.com>
>
> 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/edac.c | 10 +++++-----
> drivers/cxl/core/features.c | 28 +++++++++++++++++++---------
> 2 files changed, 24 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/cxl/core/edac.c b/drivers/cxl/core/edac.c
> index f1df4b5cfe5b..34b81e8dfbbb 100644
> --- a/drivers/cxl/core/edac.c
> +++ b/drivers/cxl/core/edac.c
> @@ -89,7 +89,7 @@ static int cxl_mem_scrub_get_attrbs(struct cxl_mailbox *cxl_mbox, u8 *cap,
> rd_data_size, 0, NULL);
> if (data_size < 0)
> return data_size;
> - if (!data_size)
> + if ((size_t)data_size != rd_data_size)
> return -EIO;
>
> *cap = rd_attrbs->scrub_cycle_cap;
> @@ -567,7 +567,7 @@ static int cxl_mem_ecs_get_attrbs(struct device *dev,
> rd_data_size, 0, NULL);
> if (data_size < 0)
> return data_size;
> - if (!data_size)
> + if ((size_t)data_size != rd_data_size)
> return -EIO;
>
> fru_rd_attrbs = rd_attrbs->fru_attrbs;
> @@ -602,7 +602,7 @@ static int cxl_mem_ecs_set_attrbs(struct device *dev,
> rd_data_size, 0, NULL);
> if (data_size < 0)
> return data_size;
> - if (!data_size)
> + if ((size_t)data_size != rd_data_size)
> return -EIO;
>
> struct cxl_ecs_wr_attrbs *wr_attrbs __free(kvfree) =
> @@ -1282,7 +1282,7 @@ cxl_mem_sparing_get_attrbs(struct cxl_mem_sparing_context *cxl_sparing_ctx)
> rd_data_size, 0, &return_code);
> if (data_size < 0)
> return data_size;
> - if (!data_size)
> + if ((size_t)data_size != rd_data_size)
> return -EIO;
>
> cxl_sparing_ctx->op_class = rd_attrbs->hdr.op_class;
> @@ -1771,7 +1771,7 @@ static int cxl_mem_ppr_get_attrbs(struct cxl_ppr_context *cxl_ppr_ctx)
> rd_data_size, 0, &return_code);
> if (data_size < 0)
> return data_size;
> - if (!data_size)
> + if ((size_t)data_size != rd_data_size)
> return -EIO;
>
> cxl_ppr_ctx->op_class = rd_attrbs->hdr.op_class;
> diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
> index 8d44ce829497..95f47193fb61 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;
> @@ -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;
> @@ -267,9 +278,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 +297,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;
> @@ -340,11 +348,8 @@ 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;
> + if (data_sent_size >= feat_data_size)
> return 0;
> - }
>
> if ((feat_data_size - data_sent_size) <= (cxl_mbox->payload_size - hdr_size)) {
> data_in_size = feat_data_size - data_sent_size;
> @@ -492,6 +497,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 ?: -EIO);
> *out_len = sizeof(struct fwctl_rpc_cxl_out);
> if (data_size <= 0) {
> rpc_out->size = 0;
> @@ -544,6 +552,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;
> --
> 2.53.0
>
next prev parent reply other threads:[~2026-09-04 5:22 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-02 5:38 [PATCH v7 0/7] cxl: Sashiko bug fixes Richard Cheng
2026-09-02 5:38 ` [PATCH v7 1/7] cxl/features: Reject feature offset that overflows 16-bit field Richard Cheng
2026-09-04 5:23 ` Alison Schofield
2026-09-02 5:38 ` [PATCH v7 2/7] cxl/region: Scan all partitions for unmapped poison Richard Cheng
2026-09-02 5:38 ` [PATCH v7 3/7] cxl/region: Don't leak tolerated RAM -EFAULT from unmapped poison scan Richard Cheng
2026-09-04 5:25 ` Alison Schofield
2026-09-02 5:38 ` [PATCH v7 4/7] cxl/region: Start unmapped poison scan at the committed decoder boundary Richard Cheng
2026-09-02 5:38 ` [PATCH v7 5/7] cxl/memdev: Don't overwrite the error from an earlier partition poison query Richard Cheng
2026-09-04 5:31 ` Alison Schofield
2026-09-02 5:38 ` [PATCH v7 6/7] cxl/region: Reject poison scan for decoder without a partition Richard Cheng
2026-09-02 5:52 ` sashiko-bot
2026-09-04 5:57 ` Alison Schofield
2026-09-02 5:38 ` [PATCH v7 7/7] cxl/fwctl: Propagate feature RPC delivery errors Richard Cheng
2026-09-04 5:21 ` Alison Schofield [this message]
2026-09-04 5:20 ` [PATCH v7 0/7] cxl: Sashiko bug fixes Alison Schofield
2026-09-04 16:19 ` Dave Jiang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=appVdg55jb6Y3zcs@aschofie-mobl2.lan \
--to=alison.schofield@intel.com \
--cc=dave.jiang@intel.com \
--cc=dave@stgolabs.net \
--cc=gourry@gourry.net \
--cc=icheng@nvidia.com \
--cc=iweiny@kernel.org \
--cc=jic23@kernel.org \
--cc=kaihengf@nvidia.com \
--cc=kees@kernel.org \
--cc=kobak@nvidia.com \
--cc=kristinc@nvidia.com \
--cc=linux-cxl@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=ming.li@zohomail.com \
--cc=newtonl@nvidia.com \
--cc=rrichter@amd.com \
--cc=vishal.l.verma@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox