Re: [PATCH v4 9/9] cxl/mem: Add payload dumping for debug

Jonathan Cameron
2021-02-16T15:50:50Z

On Mon, 15 Feb 2021 17:45:38 -0800
Ben Widawsky <ben.widawsky@intel.com> wrote:

> It's often useful in debug scenarios to see what the hardware has dumped
> out. As it stands today, any device error will result in the payload not
> being copied out, so there is no way to triage commands which weren't
> expected to fail (and sometimes the payload may have that information).
>
> The functionality is protected by normal kernel security mechanisms as
> well as a CONFIG option in the CXL driver.
>
> This was extracted from the original version of the CXL enabling patch
> series.
>
> Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>

My gut feeling here is use a tracepoint rather than spamming the kernel
log. Alternatively just don't bother merging this patch - it's on the list
now anyway so trivial for anyone doing such debug to pick it up.

Jonathan

> ---
> drivers/cxl/Kconfig | 13 +++++++++++++
> drivers/cxl/mem.c | 8 ++++++++
> 2 files changed, 21 insertions(+)
>
> diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
> index 97dc4d751651..3eec9276e586 100644
> --- a/drivers/cxl/Kconfig
> +++ b/drivers/cxl/Kconfig
> @@ -50,4 +50,17 @@ config CXL_MEM_RAW_COMMANDS
> potential impact to memory currently in use by the kernel.
>
> If developing CXL hardware or the driver say Y, otherwise say N.
> +
> +config CXL_MEM_INSECURE_DEBUG
> + bool "CXL.mem debugging"
> + depends on CXL_MEM
> + help
> + Enable debug of all CXL command payloads.
> +
> + Some CXL devices and controllers support encryption and other
> + security features. The payloads for the commands that enable
> + those features may contain sensitive clear-text security
> + material. Disable debug of those command payloads by default.
> + If you are a kernel developer actively working on CXL
> + security enabling say Y, otherwise say N.
> endif
> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> index dc608bb20a31..237b956f0be0 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -342,6 +342,14 @@ static int __cxl_mem_mbox_send_cmd(struct cxl_mem *cxlm,
>
> /* #5 */
> rc = cxl_mem_wait_for_doorbell(cxlm);
> +
> + if (!cxl_is_security_command(mbox_cmd->opcode) ||
> + IS_ENABLED(CONFIG_CXL_MEM_INSECURE_DEBUG)) {
> + print_hex_dump_debug("Payload ", DUMP_PREFIX_OFFSET, 16, 1,
> + mbox_cmd->payload_in, mbox_cmd->size_in,
> + true);
> + }
> +
> if (rc == -ETIMEDOUT) {
> cxl_mem_mbox_timeout(cxlm, mbox_cmd);
> return rc;