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

[Davidlohr Bueso <dave@stgolabs.net>]

On Thu, 26 Jan 2023, Jonathan Cameron wrote:

>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).

A few questions (which I'm missing from the spec):

- How to verify if a ts was actually set? I would assume that if SET
is unsupported, GET will also be, and since you set the ts at
bootup, it should never occur in reality.

- Does "unsupported" return type mean the command cannot mean that
flat out the device cannot handle timestamps or is it more that the
device cannot handle nanosecond resolution?

...

>@@ -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,

Why no GET support?

> };
>
> /*
>@@ -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;
>+
>+	pi.timestamp = ts;

Considering we always want "now", maybe just remove the parameter and just
compute ktime_get_real_ns() here?

>+	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);
>+}

Thanks,
Davidlohr