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

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Fri, 27 Jan 2023 09:59:06 +0000
Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:

> On Thu, 26 Jan 2023 11:59:21 -0800
> Alison Schofield <alison.schofield@intel.com> wrote:
> 
> > 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()?  
> 
> Dave raised that as well.  Makes sense for now. I was wildly conjecturing
> that we might want clock skew tweaking sometime in the future if the timestamps
> ever matter for much beyond error records.
Davidlohr that is!

> 
> Thanks,
> 
> Jonathan
> 
> 
> > 
> > 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
> > >     
>