Re: [PATCH] cxl/pci: Set the device timestamp

[Alison Schofield <alison.schofield@intel.com>]

On Thu, Jan 26, 2023 at 06:04:58PM +0000, Jonathan Cameron wrote:
> CXL r3.0 section 8.2.9.4.2 "Set Timestamp" recommends that the host sets
> the timestamp after every Conventional or CXL Reset to ensure accurate
> timestamps. This should include on initial boot up. The time base that
> is being set is used by a device for the poison list overflow timestamp
> and all event timestamps.  Note that the command is optional and if
> not supported and the device cannot return accurate timestamps it will
> fill the fields in with an appropriate marker (see the specification
> description of each timestamp).
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> ---
> 
> Open question: Should we only do this if Linux has control of the
> error handling?  In theory it should be safe anyway given the
> specification is clear that the timestamp base should always be the
> same - so subject to small errors we shouldn't cause any firmware first
> handling to get confused.
> 
>  drivers/cxl/core/mbox.c      | 25 +++++++++++++++++++++++++
>  drivers/cxl/cxlmem.h         |  7 +++++++
>  drivers/cxl/pci.c            |  5 +++++
>  include/uapi/linux/cxl_mem.h |  1 +
>  4 files changed, 38 insertions(+)
> 
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index b03fba212799..a7317bb142ed 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -65,6 +65,7 @@ static struct cxl_mem_command cxl_mem_commands[CXL_MEM_COMMAND_ID_MAX] = {
>  	CXL_CMD(GET_SCAN_MEDIA_CAPS, 0x10, 0x4, 0),
>  	CXL_CMD(SCAN_MEDIA, 0x11, 0, 0),
>  	CXL_CMD(GET_SCAN_MEDIA, 0, CXL_VARIABLE_PAYLOAD, 0),
> +	CXL_CMD(SET_TIMESTAMP, 8, 0, 0),
>  };
>  
>  /*
> @@ -93,6 +94,7 @@ static u16 cxl_disabled_raw_commands[] = {
>  	CXL_MBOX_OP_SET_SHUTDOWN_STATE,
>  	CXL_MBOX_OP_SCAN_MEDIA,
>  	CXL_MBOX_OP_GET_SCAN_MEDIA,
> +	CXL_MBOX_OP_SET_TIMESTAMP,
>  };
>  
>  /*
> @@ -857,6 +859,29 @@ int cxl_mem_create_range_info(struct cxl_dev_state *cxlds)
>  }
>  EXPORT_SYMBOL_NS_GPL(cxl_mem_create_range_info, CXL);
>  
> +int cxl_set_timestamp(struct cxl_dev_state *cxlds, u64 ts)
> +{
> +	struct cxl_mbox_cmd mbox_cmd;
> +	struct cxl_mbox_set_timestamp_in pi;
> +
> +	/*
> +	 * Command is optional and functionality should not be affected if
> +	 * the command is not available.
> +	 */
> +	if (!test_bit(CXL_MEM_COMMAND_ID_SET_TIMESTAMP, cxlds->enabled_cmds))
> +		return 0;

Jonathan,

Would it be more efficient to call ktime_get_real_ns() right here,
rather than pass in the call from cxl_pci_probe()?

Alison
> +
> +	pi.timestamp = ts;
> +	mbox_cmd = (struct cxl_mbox_cmd) {
> +		.opcode = CXL_MBOX_OP_SET_TIMESTAMP,
> +		.size_in = sizeof(pi),
> +		.payload_in = &pi,
> +	};
> +
> +	return cxl_internal_send_cmd(cxlds, &mbox_cmd);
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_set_timestamp, CXL);
> +
>  struct cxl_dev_state *cxl_dev_state_create(struct device *dev)
>  {
>  	struct cxl_dev_state *cxlds;
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index ab138004f644..0ca7a35de893 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -274,6 +274,7 @@ enum cxl_opcode {
>  	CXL_MBOX_OP_RAW			= CXL_MBOX_OP_INVALID,
>  	CXL_MBOX_OP_GET_FW_INFO		= 0x0200,
>  	CXL_MBOX_OP_ACTIVATE_FW		= 0x0202,
> +	CXL_MBOX_OP_SET_TIMESTAMP	= 0x0301,
>  	CXL_MBOX_OP_GET_SUPPORTED_LOGS	= 0x0400,
>  	CXL_MBOX_OP_GET_LOG		= 0x0401,
>  	CXL_MBOX_OP_IDENTIFY		= 0x4000,
> @@ -372,6 +373,11 @@ struct cxl_mbox_set_partition_info {
>  
>  #define  CXL_SET_PARTITION_IMMEDIATE_FLAG	BIT(0)
>  
> +/* Set Timestamp CXL 3.0 Spec 8.2.9.4.2 */
> +struct cxl_mbox_set_timestamp_in {
> +	__le64 timestamp;
> +} __packed;
> +
>  /**
>   * struct cxl_mem_command - Driver representation of a memory device command
>   * @info: Command information as it exists for the UAPI
> @@ -441,6 +447,7 @@ int cxl_mem_create_range_info(struct cxl_dev_state *cxlds);
>  struct cxl_dev_state *cxl_dev_state_create(struct device *dev);
>  void set_exclusive_cxl_commands(struct cxl_dev_state *cxlds, unsigned long *cmds);
>  void clear_exclusive_cxl_commands(struct cxl_dev_state *cxlds, unsigned long *cmds);
> +int cxl_set_timestamp(struct cxl_dev_state *cxlds, u64 ts);
>  #ifdef CONFIG_CXL_SUSPEND
>  void cxl_mem_active_inc(void);
>  void cxl_mem_active_dec(void);
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 2bbebbc7e032..30a2b231d970 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -2,6 +2,7 @@
>  /* Copyright(c) 2020 Intel Corporation. All rights reserved. */
>  #include <linux/io-64-nonatomic-lo-hi.h>
>  #include <linux/moduleparam.h>
> +#include <linux/timekeeping.h>
>  #include <linux/module.h>
>  #include <linux/delay.h>
>  #include <linux/sizes.h>
> @@ -482,6 +483,10 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	if (rc)
>  		return rc;
>  
> +	rc = cxl_set_timestamp(cxlds, ktime_get_real_ns());
> +	if (rc)
> +		return rc;
> +
>  	rc = cxl_dev_state_identify(cxlds);
>  	if (rc)
>  		return rc;
> diff --git a/include/uapi/linux/cxl_mem.h b/include/uapi/linux/cxl_mem.h
> index c71021a2a9ed..333301037b74 100644
> --- a/include/uapi/linux/cxl_mem.h
> +++ b/include/uapi/linux/cxl_mem.h
> @@ -41,6 +41,7 @@
>  	___C(GET_SCAN_MEDIA_CAPS, "Get Scan Media Capabilities"),         \
>  	___C(SCAN_MEDIA, "Scan Media"),                                   \
>  	___C(GET_SCAN_MEDIA, "Get Scan Media Results"),                   \
> +	___C(SET_TIMESTAMP, "Set Device Timestamp"),                      \
>  	___C(MAX, "invalid / last command")
>  
>  #define ___C(a, b) CXL_MEM_COMMAND_ID_##a
> 
> base-commit: 172738bbccdb4ef76bdd72fc72a315c741c39161
> -- 
> 2.37.2
>