diff mbox series

[3/5] cxl/mbox: Improve handling of mbox_cmd return codes

Message ID 20220317234049.69323-4-dave@stgolabs.net
State Superseded
Headers show
Series cxl/mbox: Robustify handling of mbox_cmd.return_code | expand

Commit Message

Davidlohr Bueso March 17, 2022, 11:40 p.m. UTC
Upon a completed command the caller is still expected to check
the actual return_code register for to ensure it succeed. This
adds, per the spec, the potential command return codes. It maps
the hardware return code with the kernel's errno style, and by
default continues to use -ENXIO (Command completed, but device
reported an error).

Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
 drivers/cxl/core/mbox.c |  2 +-
 drivers/cxl/cxlmem.h    | 90 ++++++++++++++++++++++++++++++++++++++++-
 drivers/cxl/pci.c       |  2 +-
 3 files changed, 91 insertions(+), 3 deletions(-)

Comments

Adam Manzanares March 22, 2022, 8:56 p.m. UTC | #1
On Thu, Mar 17, 2022 at 04:40:47PM -0700, Davidlohr Bueso wrote:
> Upon a completed command the caller is still expected to check
> the actual return_code register for to ensure it succeed. This

Is  MB Status register == return_code register here. s/for to/to
> adds, per the spec, the potential command return codes. It maps
> the hardware return code with the kernel's errno style, and by
> default continues to use -ENXIO (Command completed, but device
> reported an error).
> 
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
> ---
>  drivers/cxl/core/mbox.c |  2 +-
>  drivers/cxl/cxlmem.h    | 90 ++++++++++++++++++++++++++++++++++++++++-
>  drivers/cxl/pci.c       |  2 +-
>  3 files changed, 91 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index 778b04a0fb0a..d4d4a16820b7 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -171,7 +171,7 @@ int cxl_mbox_send_cmd(struct cxl_dev_state *cxlds, u16 opcode, void *in,
>  		return rc;
>  
>  	/* TODO: Map return code to proper kernel style errno */
> -	if (mbox_cmd.return_code != CXL_MBOX_SUCCESS)
> +	if (mbox_cmd.return_code != CXL_MBOX_CMD_RC_SUCCESS)
>  		return -ENXIO;
>  
>  	/*
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index 5d33ce24fe09..268597d1ff33 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -85,9 +85,97 @@ struct cxl_mbox_cmd {
>  	size_t size_in;
>  	size_t size_out;
>  	u16 return_code;
> -#define CXL_MBOX_SUCCESS 0
>  };
>  
> +/*
> + * Per CXL 2.0 Section 8.2.8.4.5.1
> + */
> +enum {
> +	CXL_MBOX_CMD_RC_SUCCESS = 0,
> +	CXL_MBOX_CMD_RC_BACKGROUND,
> +	CXL_MBOX_CMD_RC_INVINPUT,
> +	CXL_MBOX_CMD_RC_UNSUPPORTED,
> +	CXL_MBOX_CMD_RC_INTERNAL,
> +	CXL_MBOX_CMD_RC_RETRY,
> +	CXL_MBOX_CMD_RC_BUSY,
> +	CXL_MBOX_CMD_RC_MEDIADISABLED,
> +	CXL_MBOX_CMD_RC_FWINPROGRESS,
> +	CXL_MBOX_CMD_RC_FWOOO,
> +	CXL_MBOX_CMD_RC_FWAUTH,
> +	CXL_MBOX_CMD_RC_INVSLOT,

Should we keep the FW prefix for INVSLOT, since this is still related to a 
FW operation?

> +	CXL_MBOX_CMD_RC_FWROLLBACK,
> +	CXL_MBOX_CMD_RC_FWRESET,
> +	CXL_MBOX_CMD_RC_INVHANDLE,
> +	CXL_MBOX_CMD_RC_INVPA,
> +	CXL_MBOX_CMD_RC_INJPOISON,

We should indicate that we hit the poison limit, something like the 
following POISONLMT

> +	CXL_MBOX_CMD_RC_MEDIAFAILURE,
> +	CXL_MBOX_CMD_RC_ABORT,
> +	CXL_MBOX_CMD_RC_INVSECURITY,
> +	CXL_MBOX_CMD_RC_PASSPHRASE,
> +	CXL_MBOX_CMD_RC_MBOXUNSUPPORTED,
> +	CXL_MBOX_CMD_RC_INVPAYLOAD,

Might as well be more descriptive here PAYLOADLEN, looking at this more now
maybe we drop all of the INV prefixes since most of the return codes are error
codes. 

> +};
> +
> +#define CXL_CMD_RC(_value, errno, str)		\
> +	[CXL_MBOX_CMD_RC_##_value] = {		\
> +		.err = errno,			\
> +		.desc =	str,			\
> +	}
> +
> +/*
> + * mbox cmd hardware return_code handling
> + */
> +struct cxl_mbox_cmd_rc {
> +	int err; /* proper kernel style errno */
> +	const char *desc;
> +};
> +
> +static const struct cxl_mbox_cmd_rc cxl_mbox_cmd_rctable[] = {
> +	CXL_CMD_RC(SUCCESS, 0, NULL),
> +	CXL_CMD_RC(BACKGROUND, ENXIO, "background cmd started successfully"),
> +	/* errors here on out */
> +	CXL_CMD_RC(INVINPUT, ENXIO, "cmd input was invalid"),
> +	CXL_CMD_RC(UNSUPPORTED, ENXIO, "cmd is not supported"),
> +	CXL_CMD_RC(INTERNAL, ENXIO, "internal device error"),
> +	CXL_CMD_RC(RETRY, ENXIO, "temporary error, retry once"),
> +	CXL_CMD_RC(BUSY, ENXIO, "ongoing background operation"),
> +	CXL_CMD_RC(MEDIADISABLED, ENXIO, "media access is disabled"),
> +	CXL_CMD_RC(FWINPROGRESS, ENXIO,
> +			"only one FW package can be transferred at a time"),
> +	CXL_CMD_RC(FWOOO, ENXIO,
> +			"FW package content was transferred out of order"),
> +	CXL_CMD_RC(FWAUTH, ENXIO, "FW package authentication failed"),
> +	CXL_CMD_RC(INVSLOT, ENXIO,
> +			"FW slot specified is not supported for requested op"),
> +	CXL_CMD_RC(FWROLLBACK, ENXIO,
> +			"rolled back to the previous active FW"),
> +	CXL_CMD_RC(FWRESET, ENXIO, "FW failed to activate, needs cold reset"),
> +	CXL_CMD_RC(INVHANDLE, ENXIO,
> +			"one or more Event Record Handles were invalid"),
> +	CXL_CMD_RC(INVPA, ENXIO, "physical address specified is invalid"),
> +	CXL_CMD_RC(INJPOISON, ENXIO,
> +			"allowed poison injection has been reached"),

injection limit

> +	CXL_CMD_RC(MEDIAFAILURE, ENXIO, "permanent issue with the media"),
> +	CXL_CMD_RC(ABORT, ENXIO, "background cmd was aborted by device"),
> +	CXL_CMD_RC(INVSECURITY, ENXIO,
> +		   "not valid in the current security state"),
> +	CXL_CMD_RC(PASSPHRASE, ENXIO,
> +			"passphrase doesn't match current passphrase"),

current set passphrase

> +	CXL_CMD_RC(MBOXUNSUPPORTED, ENXIO,
> +			"not supported on the mailbox it was issued on"),
> +	CXL_CMD_RC(INVPAYLOAD, ENXIO, "invalid payload length"),
> +};
> +
> +static inline const char *cxl_mbox_cmd_rc2str(struct cxl_mbox_cmd *mbox_cmd)
> +{
> +	return cxl_mbox_cmd_rctable[mbox_cmd->return_code].desc;
> +}
> +
> +static inline int cxl_mbox_cmd_rc2errno(struct cxl_mbox_cmd *mbox_cmd)
> +{
> +	return cxl_mbox_cmd_rctable[mbox_cmd->return_code].err;
> +}
> +
>  /*
>   * CXL 2.0 - Memory capacity multiplier
>   * See Section 8.2.9.5
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index c77e06aff8dc..0c36d111232b 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -177,7 +177,7 @@ static int __cxl_pci_mbox_send_cmd(struct cxl_dev_state *cxlds,
>  	mbox_cmd->return_code =
>  		FIELD_GET(CXLDEV_MBOX_STATUS_RET_CODE_MASK, status_reg);
>  
> -	if (mbox_cmd->return_code != CXL_MBOX_SUCCESS) {
> +	if (mbox_cmd->return_code != CXL_MBOX_CMD_RC_SUCCESS) {
>  		dev_dbg(dev, "Mailbox operation had an error\n");
>  		return 0;  /* completed but caller must check return_code */
>  	}
> 
> -- 
> 2.26.2
>

Other than the minor nits. 
Looks good.

Reviewed by: Adam Manzanares <a.manzanares@samsung.com>
Davidlohr Bueso March 22, 2022, 9:50 p.m. UTC | #2
On Tue, 22 Mar 2022, Adam Manzanares wrote:

>On Thu, Mar 17, 2022 at 04:40:47PM -0700, Davidlohr Bueso wrote:
>> Upon a completed command the caller is still expected to check
>> the actual return_code register for to ensure it succeed. This
>
>Is  MB Status register == return_code register here. s/for to/to
>> adds, per the spec, the potential command return codes. It maps
>> the hardware return code with the kernel's errno style, and by
>> default continues to use -ENXIO (Command completed, but device
>> reported an error).
>>
>> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
>> ---
>>  drivers/cxl/core/mbox.c |  2 +-
>>  drivers/cxl/cxlmem.h    | 90 ++++++++++++++++++++++++++++++++++++++++-
>>  drivers/cxl/pci.c       |  2 +-
>>  3 files changed, 91 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
>> index 778b04a0fb0a..d4d4a16820b7 100644
>> --- a/drivers/cxl/core/mbox.c
>> +++ b/drivers/cxl/core/mbox.c
>> @@ -171,7 +171,7 @@ int cxl_mbox_send_cmd(struct cxl_dev_state *cxlds, u16 opcode, void *in,
>>		return rc;
>>
>>	/* TODO: Map return code to proper kernel style errno */
>> -	if (mbox_cmd.return_code != CXL_MBOX_SUCCESS)
>> +	if (mbox_cmd.return_code != CXL_MBOX_CMD_RC_SUCCESS)
>>		return -ENXIO;
>>
>>	/*
>> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
>> index 5d33ce24fe09..268597d1ff33 100644
>> --- a/drivers/cxl/cxlmem.h
>> +++ b/drivers/cxl/cxlmem.h
>> @@ -85,9 +85,97 @@ struct cxl_mbox_cmd {
>>	size_t size_in;
>>	size_t size_out;
>>	u16 return_code;
>> -#define CXL_MBOX_SUCCESS 0
>>  };
>>
>> +/*
>> + * Per CXL 2.0 Section 8.2.8.4.5.1
>> + */
>> +enum {
>> +	CXL_MBOX_CMD_RC_SUCCESS = 0,
>> +	CXL_MBOX_CMD_RC_BACKGROUND,
>> +	CXL_MBOX_CMD_RC_INVINPUT,
>> +	CXL_MBOX_CMD_RC_UNSUPPORTED,
>> +	CXL_MBOX_CMD_RC_INTERNAL,
>> +	CXL_MBOX_CMD_RC_RETRY,
>> +	CXL_MBOX_CMD_RC_BUSY,
>> +	CXL_MBOX_CMD_RC_MEDIADISABLED,
>> +	CXL_MBOX_CMD_RC_FWINPROGRESS,
>> +	CXL_MBOX_CMD_RC_FWOOO,
>> +	CXL_MBOX_CMD_RC_FWAUTH,
>> +	CXL_MBOX_CMD_RC_INVSLOT,
>
>Should we keep the FW prefix for INVSLOT, since this is still related to a
>FW operation?
>
>> +	CXL_MBOX_CMD_RC_FWROLLBACK,
>> +	CXL_MBOX_CMD_RC_FWRESET,
>> +	CXL_MBOX_CMD_RC_INVHANDLE,
>> +	CXL_MBOX_CMD_RC_INVPA,
>> +	CXL_MBOX_CMD_RC_INJPOISON,
>
>We should indicate that we hit the poison limit, something like the
>following POISONLMT
>
>> +	CXL_MBOX_CMD_RC_MEDIAFAILURE,
>> +	CXL_MBOX_CMD_RC_ABORT,
>> +	CXL_MBOX_CMD_RC_INVSECURITY,
>> +	CXL_MBOX_CMD_RC_PASSPHRASE,
>> +	CXL_MBOX_CMD_RC_MBOXUNSUPPORTED,
>> +	CXL_MBOX_CMD_RC_INVPAYLOAD,
>
>Might as well be more descriptive here PAYLOADLEN, looking at this more now
>maybe we drop all of the INV prefixes since most of the return codes are error
>codes.

Yes to all the above.

>
>> +};
>> +
>> +#define CXL_CMD_RC(_value, errno, str)		\
>> +	[CXL_MBOX_CMD_RC_##_value] = {		\
>> +		.err = errno,			\
>> +		.desc =	str,			\
>> +	}
>> +
>> +/*
>> + * mbox cmd hardware return_code handling
>> + */
>> +struct cxl_mbox_cmd_rc {
>> +	int err; /* proper kernel style errno */
>> +	const char *desc;
>> +};
>> +
>> +static const struct cxl_mbox_cmd_rc cxl_mbox_cmd_rctable[] = {
>> +	CXL_CMD_RC(SUCCESS, 0, NULL),
>> +	CXL_CMD_RC(BACKGROUND, ENXIO, "background cmd started successfully"),
>> +	/* errors here on out */
>> +	CXL_CMD_RC(INVINPUT, ENXIO, "cmd input was invalid"),
>> +	CXL_CMD_RC(UNSUPPORTED, ENXIO, "cmd is not supported"),
>> +	CXL_CMD_RC(INTERNAL, ENXIO, "internal device error"),
>> +	CXL_CMD_RC(RETRY, ENXIO, "temporary error, retry once"),
>> +	CXL_CMD_RC(BUSY, ENXIO, "ongoing background operation"),
>> +	CXL_CMD_RC(MEDIADISABLED, ENXIO, "media access is disabled"),
>> +	CXL_CMD_RC(FWINPROGRESS, ENXIO,
>> +			"only one FW package can be transferred at a time"),
>> +	CXL_CMD_RC(FWOOO, ENXIO,
>> +			"FW package content was transferred out of order"),
>> +	CXL_CMD_RC(FWAUTH, ENXIO, "FW package authentication failed"),
>> +	CXL_CMD_RC(INVSLOT, ENXIO,
>> +			"FW slot specified is not supported for requested op"),
>> +	CXL_CMD_RC(FWROLLBACK, ENXIO,
>> +			"rolled back to the previous active FW"),
>> +	CXL_CMD_RC(FWRESET, ENXIO, "FW failed to activate, needs cold reset"),
>> +	CXL_CMD_RC(INVHANDLE, ENXIO,
>> +			"one or more Event Record Handles were invalid"),
>> +	CXL_CMD_RC(INVPA, ENXIO, "physical address specified is invalid"),
>> +	CXL_CMD_RC(INJPOISON, ENXIO,
>> +			"allowed poison injection has been reached"),
>
>injection limit
>
>> +	CXL_CMD_RC(MEDIAFAILURE, ENXIO, "permanent issue with the media"),
>> +	CXL_CMD_RC(ABORT, ENXIO, "background cmd was aborted by device"),
>> +	CXL_CMD_RC(INVSECURITY, ENXIO,
>> +		   "not valid in the current security state"),
>> +	CXL_CMD_RC(PASSPHRASE, ENXIO,
>> +			"passphrase doesn't match current passphrase"),
>
>current set passphrase
>
>> +	CXL_CMD_RC(MBOXUNSUPPORTED, ENXIO,
>> +			"not supported on the mailbox it was issued on"),
>> +	CXL_CMD_RC(INVPAYLOAD, ENXIO, "invalid payload length"),
>> +};
>
>Other than the minor nits.
>Looks good.

Thanks for reviewing. I'll add the changes in a v2 but will wait a bit
for any further feedback.
Dan Williams March 30, 2022, 12:18 a.m. UTC | #3
On Thu, Mar 17, 2022 at 4:41 PM Davidlohr Bueso <dave@stgolabs.net> wrote:
>
> Upon a completed command the caller is still expected to check
> the actual return_code register for to ensure it succeed. This
> adds, per the spec, the potential command return codes. It maps
> the hardware return code with the kernel's errno style, and by
> default continues to use -ENXIO (Command completed, but device
> reported an error).
>
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
> ---
>  drivers/cxl/core/mbox.c |  2 +-
>  drivers/cxl/cxlmem.h    | 90 ++++++++++++++++++++++++++++++++++++++++-
>  drivers/cxl/pci.c       |  2 +-
>  3 files changed, 91 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index 778b04a0fb0a..d4d4a16820b7 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -171,7 +171,7 @@ int cxl_mbox_send_cmd(struct cxl_dev_state *cxlds, u16 opcode, void *in,
>                 return rc;
>
>         /* TODO: Map return code to proper kernel style errno */
> -       if (mbox_cmd.return_code != CXL_MBOX_SUCCESS)
> +       if (mbox_cmd.return_code != CXL_MBOX_CMD_RC_SUCCESS)
>                 return -ENXIO;
>
>         /*
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index 5d33ce24fe09..268597d1ff33 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -85,9 +85,97 @@ struct cxl_mbox_cmd {
>         size_t size_in;
>         size_t size_out;
>         u16 return_code;
> -#define CXL_MBOX_SUCCESS 0
>  };
>
> +/*
> + * Per CXL 2.0 Section 8.2.8.4.5.1
> + */
> +enum {
> +       CXL_MBOX_CMD_RC_SUCCESS = 0,
> +       CXL_MBOX_CMD_RC_BACKGROUND,
> +       CXL_MBOX_CMD_RC_INVINPUT,
> +       CXL_MBOX_CMD_RC_UNSUPPORTED,
> +       CXL_MBOX_CMD_RC_INTERNAL,
> +       CXL_MBOX_CMD_RC_RETRY,
> +       CXL_MBOX_CMD_RC_BUSY,
> +       CXL_MBOX_CMD_RC_MEDIADISABLED,
> +       CXL_MBOX_CMD_RC_FWINPROGRESS,
> +       CXL_MBOX_CMD_RC_FWOOO,
> +       CXL_MBOX_CMD_RC_FWAUTH,
> +       CXL_MBOX_CMD_RC_INVSLOT,
> +       CXL_MBOX_CMD_RC_FWROLLBACK,
> +       CXL_MBOX_CMD_RC_FWRESET,
> +       CXL_MBOX_CMD_RC_INVHANDLE,
> +       CXL_MBOX_CMD_RC_INVPA,
> +       CXL_MBOX_CMD_RC_INJPOISON,
> +       CXL_MBOX_CMD_RC_MEDIAFAILURE,
> +       CXL_MBOX_CMD_RC_ABORT,
> +       CXL_MBOX_CMD_RC_INVSECURITY,
> +       CXL_MBOX_CMD_RC_PASSPHRASE,
> +       CXL_MBOX_CMD_RC_MBOXUNSUPPORTED,
> +       CXL_MBOX_CMD_RC_INVPAYLOAD,
> +};
> +
> +#define CXL_CMD_RC(_value, errno, str)         \
> +       [CXL_MBOX_CMD_RC_##_value] = {          \
> +               .err = errno,                   \
> +               .desc = str,                    \
> +       }
> +
> +/*
> + * mbox cmd hardware return_code handling
> + */
> +struct cxl_mbox_cmd_rc {
> +       int err; /* proper kernel style errno */
> +       const char *desc;
> +};
> +
> +static const struct cxl_mbox_cmd_rc cxl_mbox_cmd_rctable[] = {
> +       CXL_CMD_RC(SUCCESS, 0, NULL),
> +       CXL_CMD_RC(BACKGROUND, ENXIO, "background cmd started successfully"),
> +       /* errors here on out */
> +       CXL_CMD_RC(INVINPUT, ENXIO, "cmd input was invalid"),
> +       CXL_CMD_RC(UNSUPPORTED, ENXIO, "cmd is not supported"),
> +       CXL_CMD_RC(INTERNAL, ENXIO, "internal device error"),
> +       CXL_CMD_RC(RETRY, ENXIO, "temporary error, retry once"),
> +       CXL_CMD_RC(BUSY, ENXIO, "ongoing background operation"),
> +       CXL_CMD_RC(MEDIADISABLED, ENXIO, "media access is disabled"),
> +       CXL_CMD_RC(FWINPROGRESS, ENXIO,
> +                       "only one FW package can be transferred at a time"),
> +       CXL_CMD_RC(FWOOO, ENXIO,
> +                       "FW package content was transferred out of order"),
> +       CXL_CMD_RC(FWAUTH, ENXIO, "FW package authentication failed"),
> +       CXL_CMD_RC(INVSLOT, ENXIO,
> +                       "FW slot specified is not supported for requested op"),
> +       CXL_CMD_RC(FWROLLBACK, ENXIO,
> +                       "rolled back to the previous active FW"),
> +       CXL_CMD_RC(FWRESET, ENXIO, "FW failed to activate, needs cold reset"),
> +       CXL_CMD_RC(INVHANDLE, ENXIO,
> +                       "one or more Event Record Handles were invalid"),
> +       CXL_CMD_RC(INVPA, ENXIO, "physical address specified is invalid"),
> +       CXL_CMD_RC(INJPOISON, ENXIO,
> +                       "allowed poison injection has been reached"),
> +       CXL_CMD_RC(MEDIAFAILURE, ENXIO, "permanent issue with the media"),
> +       CXL_CMD_RC(ABORT, ENXIO, "background cmd was aborted by device"),
> +       CXL_CMD_RC(INVSECURITY, ENXIO,
> +                  "not valid in the current security state"),
> +       CXL_CMD_RC(PASSPHRASE, ENXIO,
> +                       "passphrase doesn't match current passphrase"),
> +       CXL_CMD_RC(MBOXUNSUPPORTED, ENXIO,
> +                       "not supported on the mailbox it was issued on"),
> +       CXL_CMD_RC(INVPAYLOAD, ENXIO, "invalid payload length"),
> +};

You can define the table and the enums together with Steven's trick [1]

#define CMD_CMD_RC_TABLE                                                    \
        C(SUCCESS,      0,      NULL),                                   \
        C(BACKGROUND,   -ENXIO, "background cmd started successfully"),  \
                ...
        C(PAYLOADLEN,   -ENXIO, "invalid payload length")
#undef C
#define C(a, b, c) CXL_CMD_RC_##a
enum  { CMD_CMD_RC_TABLE };
#undef C
#define C(a, b, c) { b, c }
static struct {
        int err;
        const char *desc;
} cxl_mbox_cmd_rctable[] = { OPS };
#undef C

Ideally you could align the columns so it's easy to read which
hardware code relates to which errno.

[1]: See "cpp tricks and treats": https://lwn.net/Articles/383362/
diff mbox series

Patch

diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index 778b04a0fb0a..d4d4a16820b7 100644
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -171,7 +171,7 @@  int cxl_mbox_send_cmd(struct cxl_dev_state *cxlds, u16 opcode, void *in,
 		return rc;
 
 	/* TODO: Map return code to proper kernel style errno */
-	if (mbox_cmd.return_code != CXL_MBOX_SUCCESS)
+	if (mbox_cmd.return_code != CXL_MBOX_CMD_RC_SUCCESS)
 		return -ENXIO;
 
 	/*
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 5d33ce24fe09..268597d1ff33 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -85,9 +85,97 @@  struct cxl_mbox_cmd {
 	size_t size_in;
 	size_t size_out;
 	u16 return_code;
-#define CXL_MBOX_SUCCESS 0
 };
 
+/*
+ * Per CXL 2.0 Section 8.2.8.4.5.1
+ */
+enum {
+	CXL_MBOX_CMD_RC_SUCCESS = 0,
+	CXL_MBOX_CMD_RC_BACKGROUND,
+	CXL_MBOX_CMD_RC_INVINPUT,
+	CXL_MBOX_CMD_RC_UNSUPPORTED,
+	CXL_MBOX_CMD_RC_INTERNAL,
+	CXL_MBOX_CMD_RC_RETRY,
+	CXL_MBOX_CMD_RC_BUSY,
+	CXL_MBOX_CMD_RC_MEDIADISABLED,
+	CXL_MBOX_CMD_RC_FWINPROGRESS,
+	CXL_MBOX_CMD_RC_FWOOO,
+	CXL_MBOX_CMD_RC_FWAUTH,
+	CXL_MBOX_CMD_RC_INVSLOT,
+	CXL_MBOX_CMD_RC_FWROLLBACK,
+	CXL_MBOX_CMD_RC_FWRESET,
+	CXL_MBOX_CMD_RC_INVHANDLE,
+	CXL_MBOX_CMD_RC_INVPA,
+	CXL_MBOX_CMD_RC_INJPOISON,
+	CXL_MBOX_CMD_RC_MEDIAFAILURE,
+	CXL_MBOX_CMD_RC_ABORT,
+	CXL_MBOX_CMD_RC_INVSECURITY,
+	CXL_MBOX_CMD_RC_PASSPHRASE,
+	CXL_MBOX_CMD_RC_MBOXUNSUPPORTED,
+	CXL_MBOX_CMD_RC_INVPAYLOAD,
+};
+
+#define CXL_CMD_RC(_value, errno, str)		\
+	[CXL_MBOX_CMD_RC_##_value] = {		\
+		.err = errno,			\
+		.desc =	str,			\
+	}
+
+/*
+ * mbox cmd hardware return_code handling
+ */
+struct cxl_mbox_cmd_rc {
+	int err; /* proper kernel style errno */
+	const char *desc;
+};
+
+static const struct cxl_mbox_cmd_rc cxl_mbox_cmd_rctable[] = {
+	CXL_CMD_RC(SUCCESS, 0, NULL),
+	CXL_CMD_RC(BACKGROUND, ENXIO, "background cmd started successfully"),
+	/* errors here on out */
+	CXL_CMD_RC(INVINPUT, ENXIO, "cmd input was invalid"),
+	CXL_CMD_RC(UNSUPPORTED, ENXIO, "cmd is not supported"),
+	CXL_CMD_RC(INTERNAL, ENXIO, "internal device error"),
+	CXL_CMD_RC(RETRY, ENXIO, "temporary error, retry once"),
+	CXL_CMD_RC(BUSY, ENXIO, "ongoing background operation"),
+	CXL_CMD_RC(MEDIADISABLED, ENXIO, "media access is disabled"),
+	CXL_CMD_RC(FWINPROGRESS, ENXIO,
+			"only one FW package can be transferred at a time"),
+	CXL_CMD_RC(FWOOO, ENXIO,
+			"FW package content was transferred out of order"),
+	CXL_CMD_RC(FWAUTH, ENXIO, "FW package authentication failed"),
+	CXL_CMD_RC(INVSLOT, ENXIO,
+			"FW slot specified is not supported for requested op"),
+	CXL_CMD_RC(FWROLLBACK, ENXIO,
+			"rolled back to the previous active FW"),
+	CXL_CMD_RC(FWRESET, ENXIO, "FW failed to activate, needs cold reset"),
+	CXL_CMD_RC(INVHANDLE, ENXIO,
+			"one or more Event Record Handles were invalid"),
+	CXL_CMD_RC(INVPA, ENXIO, "physical address specified is invalid"),
+	CXL_CMD_RC(INJPOISON, ENXIO,
+			"allowed poison injection has been reached"),
+	CXL_CMD_RC(MEDIAFAILURE, ENXIO, "permanent issue with the media"),
+	CXL_CMD_RC(ABORT, ENXIO, "background cmd was aborted by device"),
+	CXL_CMD_RC(INVSECURITY, ENXIO,
+		   "not valid in the current security state"),
+	CXL_CMD_RC(PASSPHRASE, ENXIO,
+			"passphrase doesn't match current passphrase"),
+	CXL_CMD_RC(MBOXUNSUPPORTED, ENXIO,
+			"not supported on the mailbox it was issued on"),
+	CXL_CMD_RC(INVPAYLOAD, ENXIO, "invalid payload length"),
+};
+
+static inline const char *cxl_mbox_cmd_rc2str(struct cxl_mbox_cmd *mbox_cmd)
+{
+	return cxl_mbox_cmd_rctable[mbox_cmd->return_code].desc;
+}
+
+static inline int cxl_mbox_cmd_rc2errno(struct cxl_mbox_cmd *mbox_cmd)
+{
+	return cxl_mbox_cmd_rctable[mbox_cmd->return_code].err;
+}
+
 /*
  * CXL 2.0 - Memory capacity multiplier
  * See Section 8.2.9.5
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index c77e06aff8dc..0c36d111232b 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -177,7 +177,7 @@  static int __cxl_pci_mbox_send_cmd(struct cxl_dev_state *cxlds,
 	mbox_cmd->return_code =
 		FIELD_GET(CXLDEV_MBOX_STATUS_RET_CODE_MASK, status_reg);
 
-	if (mbox_cmd->return_code != CXL_MBOX_SUCCESS) {
+	if (mbox_cmd->return_code != CXL_MBOX_CMD_RC_SUCCESS) {
 		dev_dbg(dev, "Mailbox operation had an error\n");
 		return 0;  /* completed but caller must check return_code */
 	}