* Re: [PATCH] cxl/pci: Set the device timestamp
2023-01-26 18:04 [PATCH] cxl/pci: Set the device timestamp Jonathan Cameron
@ 2023-01-26 18:56 ` Davidlohr Bueso
2023-01-27 9:57 ` Jonathan Cameron
2023-01-26 19:59 ` Alison Schofield
` (4 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Davidlohr Bueso @ 2023-01-26 18:56 UTC (permalink / raw)
To: Jonathan Cameron
Cc: linux-cxl, Alison Schofield, Vishal Verma, Ira Weiny,
Dan Williams, linuxarm
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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH] cxl/pci: Set the device timestamp
2023-01-26 18:56 ` Davidlohr Bueso
@ 2023-01-27 9:57 ` Jonathan Cameron
2023-01-27 12:08 ` Jonathan Cameron
0 siblings, 1 reply; 17+ messages in thread
From: Jonathan Cameron @ 2023-01-27 9:57 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: linux-cxl, Alison Schofield, Vishal Verma, Ira Weiny,
Dan Williams, linuxarm
On Thu, 26 Jan 2023 10:56:20 -0800
Davidlohr Bueso <dave@stgolabs.net> wrote:
> 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.
I don't think we care. If a record comes back with the magic code (all
zeros) that is in the spec. Get can still be supported.
>
> - 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?
Neither - it just means that it cannot be set by this means.
There is no provision for anything other than nanoseconds, but there is also
no statement on accuracy. So if you only do seconds, multiply the number
by 1x10^9
The device might have another means of setting it. MCTP for example
or in theory might have an RTC. The OS can only tell if that is true
when it sees a timestamp.
>
> ...
>
> >@@ -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?
Sure. We could do that, and bring the parameter back if we want to
do anything clever in future.
>
> >+ 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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH] cxl/pci: Set the device timestamp
2023-01-27 9:57 ` Jonathan Cameron
@ 2023-01-27 12:08 ` Jonathan Cameron
0 siblings, 0 replies; 17+ messages in thread
From: Jonathan Cameron @ 2023-01-27 12:08 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: linux-cxl, Alison Schofield, Vishal Verma, Ira Weiny,
Dan Williams, linuxarm
On Fri, 27 Jan 2023 09:57:06 +0000
Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:
> On Thu, 26 Jan 2023 10:56:20 -0800
> Davidlohr Bueso <dave@stgolabs.net> wrote:
>
> > 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.
>
> I don't think we care. If a record comes back with the magic code (all
> zeros) that is in the spec. Get can still be supported.
>
> >
> > - 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?
>
> Neither - it just means that it cannot be set by this means.
> There is no provision for anything other than nanoseconds, but there is also
> no statement on accuracy. So if you only do seconds, multiply the number
> by 1x10^9
>
> The device might have another means of setting it. MCTP for example
> or in theory might have an RTC. The OS can only tell if that is true
> when it sees a timestamp.
>
>
>
> >
> > ...
> >
> > >@@ -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?
Missed this question in first reply.
Original thought was don't provide something we aren't yet using.
After Dan's review I've removed userspace direct access and not restricted the
set here. So adding Get makes even less sense at this stage.
Thanks
Jonathan
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] cxl/pci: Set the device timestamp
2023-01-26 18:04 [PATCH] cxl/pci: Set the device timestamp Jonathan Cameron
2023-01-26 18:56 ` Davidlohr Bueso
@ 2023-01-26 19:59 ` Alison Schofield
2023-01-27 9:59 ` Jonathan Cameron
2023-01-26 20:22 ` Dan Williams
` (3 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Alison Schofield @ 2023-01-26 19:59 UTC (permalink / raw)
To: Jonathan Cameron
Cc: linux-cxl, Vishal Verma, Ira Weiny, Dan Williams, linuxarm
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
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH] cxl/pci: Set the device timestamp
2023-01-26 19:59 ` Alison Schofield
@ 2023-01-27 9:59 ` Jonathan Cameron
2023-01-27 9:59 ` Jonathan Cameron
0 siblings, 1 reply; 17+ messages in thread
From: Jonathan Cameron @ 2023-01-27 9:59 UTC (permalink / raw)
To: Alison Schofield
Cc: linux-cxl, Vishal Verma, Ira Weiny, Dan Williams, linuxarm
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.
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
> >
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH] cxl/pci: Set the device timestamp
2023-01-27 9:59 ` Jonathan Cameron
@ 2023-01-27 9:59 ` Jonathan Cameron
0 siblings, 0 replies; 17+ messages in thread
From: Jonathan Cameron @ 2023-01-27 9:59 UTC (permalink / raw)
To: Alison Schofield
Cc: linux-cxl, Vishal Verma, Ira Weiny, Dan Williams, linuxarm
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
> > >
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH] cxl/pci: Set the device timestamp
2023-01-26 18:04 [PATCH] cxl/pci: Set the device timestamp Jonathan Cameron
2023-01-26 18:56 ` Davidlohr Bueso
2023-01-26 19:59 ` Alison Schofield
@ 2023-01-26 20:22 ` Dan Williams
2023-01-27 10:04 ` Jonathan Cameron
2023-01-28 11:21 ` kernel test robot
` (2 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Dan Williams @ 2023-01-26 20:22 UTC (permalink / raw)
To: Jonathan Cameron, linux-cxl
Cc: Alison Schofield, Vishal Verma, Ira Weiny, Dan Williams, linuxarm
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),
Is there a use case for userspace to need to send its own view
of 'timestamp' to the device? I think it's ok if this only a
kernel-internal thing.
> };
>
> /*
> @@ -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,
The criteria I have in mind for commands that should be added to this
list are things that need to have a kernel control point (like long
running background commands), or commands with data integrity
implications that only the kernel can reasonably manage (like shutdown
state). While it is odd for userspace to send its own timestamps via
debug kernel builds that enable raw commands, the side effects of
allowing this seem benign.
> };
>
> /*
> @@ -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;
cpu_to_le64()?
> + 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")
Modulo the discussion above, may not need this.
>
> #define ___C(a, b) CXL_MEM_COMMAND_ID_##a
>
> base-commit: 172738bbccdb4ef76bdd72fc72a315c741c39161
> --
> 2.37.2
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH] cxl/pci: Set the device timestamp
2023-01-26 20:22 ` Dan Williams
@ 2023-01-27 10:04 ` Jonathan Cameron
2023-01-27 12:10 ` Jonathan Cameron
0 siblings, 1 reply; 17+ messages in thread
From: Jonathan Cameron @ 2023-01-27 10:04 UTC (permalink / raw)
To: Dan Williams
Cc: linux-cxl, Alison Schofield, Vishal Verma, Ira Weiny, linuxarm
On Thu, 26 Jan 2023 12:22:14 -0800
Dan Williams <dan.j.williams@intel.com> wrote:
> 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),
>
> Is there a use case for userspace to need to send its own view
> of 'timestamp' to the device? I think it's ok if this only a
> kernel-internal thing.
Fair enough. Easier to add the interface later if we need to than to rip
it out.
>
> > };
> >
> > /*
> > @@ -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,
>
> The criteria I have in mind for commands that should be added to this
> list are things that need to have a kernel control point (like long
> running background commands), or commands with data integrity
> implications that only the kernel can reasonably manage (like shutdown
> state). While it is odd for userspace to send its own timestamps via
> debug kernel builds that enable raw commands, the side effects of
> allowing this seem benign.
Makes sense I'll drop it from this list.
>
> > };
> >
> > /*
> > @@ -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;
>
> cpu_to_le64()?
Good point.
Thanks,
Jonathan
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH] cxl/pci: Set the device timestamp
2023-01-27 10:04 ` Jonathan Cameron
@ 2023-01-27 12:10 ` Jonathan Cameron
2023-01-27 19:07 ` Dan Williams
0 siblings, 1 reply; 17+ messages in thread
From: Jonathan Cameron @ 2023-01-27 12:10 UTC (permalink / raw)
To: Dan Williams
Cc: linux-cxl, Alison Schofield, Vishal Verma, Ira Weiny, linuxarm
> > > /*
> > > @@ -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;
One side effect of dropping the userspace handling is we loose
the presence in enabled_cmds (based on the CEL). I've replaced
this with specific handling of the Not Supported mailbox return code
and suitable comments on why I'm not considering that an error.
Hopefully that compromise makes sense.
Jonathan
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH] cxl/pci: Set the device timestamp
2023-01-27 12:10 ` Jonathan Cameron
@ 2023-01-27 19:07 ` Dan Williams
2023-01-27 23:50 ` Ira Weiny
0 siblings, 1 reply; 17+ messages in thread
From: Dan Williams @ 2023-01-27 19:07 UTC (permalink / raw)
To: Jonathan Cameron, Dan Williams
Cc: linux-cxl, Alison Schofield, Vishal Verma, Ira Weiny, linuxarm
Jonathan Cameron wrote:
> > > > /*
> > > > @@ -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;
>
> One side effect of dropping the userspace handling is we loose
> the presence in enabled_cmds (based on the CEL). I've replaced
> this with specific handling of the Not Supported mailbox return code
> and suitable comments on why I'm not considering that an error.
>
> Hopefully that compromise makes sense.
That does make sense.
That also has implications for the debug messages that will say that the "set
timestamp" opcode is unsupported.
What do you think of the following to try to clarify the distinction of
supported opcodes that are used internally by the driver and the ones
that are wrapped by a user command?
-- >8 --
From d36b9ce647e615be8241d7fbf86ce555c72cc2a4 Mon Sep 17 00:00:00 2001
From: Dan Williams <dan.j.williams@intel.com>
Date: Fri, 27 Jan 2023 11:00:52 -0800
Subject: [PATCH] cxl/mbox: Indicate internal vs user enabled for command debug
At initial enumeration provide an indication of whether the kernel will
use an opcode internally, and / or make the functionality available via
user ioctl command.
Given that 'enum cxl_opcode' needs to be updated before the kernel can
use a CXL opcode internally, use that list to populate known_opcode().
For the QEMU CXL emulation this results in the following debug messages
at startup:
cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x100 Enabled: 0 User: none
cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x101 Enabled: 0 User: none
cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x102 Enabled: 0 User: none
cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x103 Enabled: 0 User: none
cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x200 Enabled: 1 User: Get FW Info
cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x300 Enabled: 0 User: none
cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x301 Enabled: 0 User: none
cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x400 Enabled: 1 User: Get Supported Logs
cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x401 Enabled: 1 User: Get Log
cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x4000 Enabled: 1 User: Identify Command
cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x4100 Enabled: 1 User: Get Partition Information
cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x4102 Enabled: 1 User: Get Label Storage Area
cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x4103 Enabled: 1 User: Set Label Storage Area
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/cxl/core/mbox.c | 48 ++++++++++++++++++++++++++++++++++-------
drivers/cxl/cxlmem.h | 1 +
2 files changed, 41 insertions(+), 8 deletions(-)
diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index cae43cea00cd..45206465fd4d 100644
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -587,6 +587,40 @@ static int cxl_xfer_log(struct cxl_dev_state *cxlds, uuid_t *uuid, u32 size, u8
return 0;
}
+static bool known_opcode(u16 opcode)
+{
+ switch (opcode) {
+ case CXL_MBOX_OP_GET_FW_INFO:
+ case CXL_MBOX_OP_ACTIVATE_FW:
+ case CXL_MBOX_OP_GET_SUPPORTED_LOGS:
+ case CXL_MBOX_OP_GET_LOG:
+ case CXL_MBOX_OP_IDENTIFY:
+ case CXL_MBOX_OP_GET_PARTITION_INFO:
+ case CXL_MBOX_OP_SET_PARTITION_INFO:
+ case CXL_MBOX_OP_GET_LSA:
+ case CXL_MBOX_OP_SET_LSA:
+ case CXL_MBOX_OP_GET_HEALTH_INFO:
+ case CXL_MBOX_OP_GET_ALERT_CONFIG:
+ case CXL_MBOX_OP_SET_ALERT_CONFIG:
+ case CXL_MBOX_OP_GET_SHUTDOWN_STATE:
+ case CXL_MBOX_OP_SET_SHUTDOWN_STATE:
+ case CXL_MBOX_OP_GET_POISON:
+ case CXL_MBOX_OP_INJECT_POISON:
+ case CXL_MBOX_OP_CLEAR_POISON:
+ case CXL_MBOX_OP_GET_SCAN_MEDIA_CAPS:
+ case CXL_MBOX_OP_SCAN_MEDIA:
+ case CXL_MBOX_OP_GET_SCAN_MEDIA:
+ case CXL_MBOX_OP_GET_SECURITY_STATE:
+ case CXL_MBOX_OP_SET_PASSPHRASE:
+ case CXL_MBOX_OP_DISABLE_PASSPHRASE:
+ case CXL_MBOX_OP_UNLOCK:
+ case CXL_MBOX_OP_FREEZE_SECURITY:
+ case CXL_MBOX_OP_PASSPHRASE_SECURE_ERASE:
+ return true;
+ }
+ return false;
+}
+
/**
* cxl_walk_cel() - Walk through the Command Effects Log.
* @cxlds: The device data for the operation
@@ -607,15 +641,13 @@ static void cxl_walk_cel(struct cxl_dev_state *cxlds, size_t size, u8 *cel)
for (i = 0; i < cel_entries; i++) {
u16 opcode = le16_to_cpu(cel_entry[i].opcode);
struct cxl_mem_command *cmd = cxl_mem_find_command(opcode);
+ bool known = known_opcode(opcode);
- if (!cmd) {
- dev_dbg(cxlds->dev,
- "Opcode 0x%04x unsupported by driver\n", opcode);
- continue;
- }
-
- set_bit(cmd->info.id, cxlds->enabled_cmds);
- dev_dbg(cxlds->dev, "Opcode 0x%04x enabled\n", opcode);
+ if (cmd)
+ set_bit(cmd->info.id, cxlds->enabled_cmds);
+ dev_dbg(cxlds->dev, "Opcode %#04x Enabled: %d User: %s\n",
+ opcode, !!known,
+ cmd ? cxl_command_names[cmd->info.id].name : "none");
}
}
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index ab138004f644..21f97d631823 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -299,6 +299,7 @@ enum cxl_opcode {
CXL_MBOX_OP_FREEZE_SECURITY = 0x4504,
CXL_MBOX_OP_PASSPHRASE_SECURE_ERASE = 0x4505,
CXL_MBOX_OP_MAX = 0x10000
+ /* update known_opcode() when adding opcode support to this list */
};
#define DEFINE_CXL_CEL_UUID \
--
2.38.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH] cxl/pci: Set the device timestamp
2023-01-27 19:07 ` Dan Williams
@ 2023-01-27 23:50 ` Ira Weiny
2023-01-28 0:17 ` Dan Williams
0 siblings, 1 reply; 17+ messages in thread
From: Ira Weiny @ 2023-01-27 23:50 UTC (permalink / raw)
To: Dan Williams, Jonathan Cameron
Cc: linux-cxl, Alison Schofield, Vishal Verma, Ira Weiny, linuxarm
Dan Williams wrote:
> Jonathan Cameron wrote:
> > > > > /*
> > > > > @@ -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;
> >
> > One side effect of dropping the userspace handling is we loose
> > the presence in enabled_cmds (based on the CEL). I've replaced
> > this with specific handling of the Not Supported mailbox return code
> > and suitable comments on why I'm not considering that an error.
> >
> > Hopefully that compromise makes sense.
>
> That does make sense.
>
> That also has implications for the debug messages that will say that the "set
> timestamp" opcode is unsupported.
>
> What do you think of the following to try to clarify the distinction of
> supported opcodes that are used internally by the driver and the ones
> that are wrapped by a user command?
>
> -- >8 --
> From d36b9ce647e615be8241d7fbf86ce555c72cc2a4 Mon Sep 17 00:00:00 2001
> From: Dan Williams <dan.j.williams@intel.com>
> Date: Fri, 27 Jan 2023 11:00:52 -0800
> Subject: [PATCH] cxl/mbox: Indicate internal vs user enabled for command debug
>
> At initial enumeration provide an indication of whether the kernel will
> use an opcode internally, and / or make the functionality available via
> user ioctl command.
>
> Given that 'enum cxl_opcode' needs to be updated before the kernel can
> use a CXL opcode internally, use that list to populate known_opcode().
>
> For the QEMU CXL emulation this results in the following debug messages
> at startup:
>
> cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x100 Enabled: 0 User: none
> cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x101 Enabled: 0 User: none
> cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x102 Enabled: 0 User: none
> cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x103 Enabled: 0 User: none
> cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x200 Enabled: 1 User: Get FW Info
> cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x300 Enabled: 0 User: none
> cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x301 Enabled: 0 User: none
> cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x400 Enabled: 1 User: Get Supported Logs
> cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x401 Enabled: 1 User: Get Log
> cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x4000 Enabled: 1 User: Identify Command
> cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x4100 Enabled: 1 User: Get Partition Information
> cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x4102 Enabled: 1 User: Get Label Storage Area
> cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x4103 Enabled: 1 User: Set Label Storage Area
I think this is nice debug output.
This kind of threw me though. It looks like all enabled commands have
user commands.
Would it be better to s/Enabled/Kernel. Or perhaps driver?
Should we also check anything that is exclusive to the kernel and report
that?
FWIW I've got a patch warming which updates the query command to factor in
the kernel exclusive and hardware support, in case anyone is looking at
this debug output as a way to determine that.
>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
> drivers/cxl/core/mbox.c | 48 ++++++++++++++++++++++++++++++++++-------
> drivers/cxl/cxlmem.h | 1 +
> 2 files changed, 41 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index cae43cea00cd..45206465fd4d 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -587,6 +587,40 @@ static int cxl_xfer_log(struct cxl_dev_state *cxlds, uuid_t *uuid, u32 size, u8
> return 0;
> }
>
> +static bool known_opcode(u16 opcode)
Perhaps driver_known_opcode?
> +{
> + switch (opcode) {
> + case CXL_MBOX_OP_GET_FW_INFO:
> + case CXL_MBOX_OP_ACTIVATE_FW:
> + case CXL_MBOX_OP_GET_SUPPORTED_LOGS:
> + case CXL_MBOX_OP_GET_LOG:
> + case CXL_MBOX_OP_IDENTIFY:
> + case CXL_MBOX_OP_GET_PARTITION_INFO:
> + case CXL_MBOX_OP_SET_PARTITION_INFO:
> + case CXL_MBOX_OP_GET_LSA:
> + case CXL_MBOX_OP_SET_LSA:
> + case CXL_MBOX_OP_GET_HEALTH_INFO:
> + case CXL_MBOX_OP_GET_ALERT_CONFIG:
> + case CXL_MBOX_OP_SET_ALERT_CONFIG:
> + case CXL_MBOX_OP_GET_SHUTDOWN_STATE:
> + case CXL_MBOX_OP_SET_SHUTDOWN_STATE:
> + case CXL_MBOX_OP_GET_POISON:
> + case CXL_MBOX_OP_INJECT_POISON:
> + case CXL_MBOX_OP_CLEAR_POISON:
> + case CXL_MBOX_OP_GET_SCAN_MEDIA_CAPS:
> + case CXL_MBOX_OP_SCAN_MEDIA:
> + case CXL_MBOX_OP_GET_SCAN_MEDIA:
> + case CXL_MBOX_OP_GET_SECURITY_STATE:
> + case CXL_MBOX_OP_SET_PASSPHRASE:
> + case CXL_MBOX_OP_DISABLE_PASSPHRASE:
> + case CXL_MBOX_OP_UNLOCK:
> + case CXL_MBOX_OP_FREEZE_SECURITY:
> + case CXL_MBOX_OP_PASSPHRASE_SECURE_ERASE:
> + return true;
> + }
> + return false;
> +}
> +
> /**
> * cxl_walk_cel() - Walk through the Command Effects Log.
> * @cxlds: The device data for the operation
> @@ -607,15 +641,13 @@ static void cxl_walk_cel(struct cxl_dev_state *cxlds, size_t size, u8 *cel)
> for (i = 0; i < cel_entries; i++) {
> u16 opcode = le16_to_cpu(cel_entry[i].opcode);
> struct cxl_mem_command *cmd = cxl_mem_find_command(opcode);
> + bool known = known_opcode(opcode);
>
> - if (!cmd) {
> - dev_dbg(cxlds->dev,
> - "Opcode 0x%04x unsupported by driver\n", opcode);
> - continue;
> - }
> -
> - set_bit(cmd->info.id, cxlds->enabled_cmds);
> - dev_dbg(cxlds->dev, "Opcode 0x%04x enabled\n", opcode);
> + if (cmd)
> + set_bit(cmd->info.id, cxlds->enabled_cmds);
> + dev_dbg(cxlds->dev, "Opcode %#04x Enabled: %d User: %s\n",
> + opcode, !!known,
> + cmd ? cxl_command_names[cmd->info.id].name : "none");
> }
> }
>
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index ab138004f644..21f97d631823 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -299,6 +299,7 @@ enum cxl_opcode {
> CXL_MBOX_OP_FREEZE_SECURITY = 0x4504,
> CXL_MBOX_OP_PASSPHRASE_SECURE_ERASE = 0x4505,
> CXL_MBOX_OP_MAX = 0x10000
> + /* update known_opcode() when adding opcode support to this list */
I'm wishing there was a way to not have to do this. All I can think of to
make it easier would be to make 'driver_know_opcode()' static inline and
in this header. I'm sure some macro magic could be used too. Although
I'm not always a fan.
Ira
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH] cxl/pci: Set the device timestamp
2023-01-27 23:50 ` Ira Weiny
@ 2023-01-28 0:17 ` Dan Williams
0 siblings, 0 replies; 17+ messages in thread
From: Dan Williams @ 2023-01-28 0:17 UTC (permalink / raw)
To: Ira Weiny, Dan Williams, Jonathan Cameron
Cc: linux-cxl, Alison Schofield, Vishal Verma, Ira Weiny, linuxarm
Ira Weiny wrote:
> Dan Williams wrote:
> > Jonathan Cameron wrote:
> > > > > > /*
> > > > > > @@ -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;
> > >
> > > One side effect of dropping the userspace handling is we loose
> > > the presence in enabled_cmds (based on the CEL). I've replaced
> > > this with specific handling of the Not Supported mailbox return code
> > > and suitable comments on why I'm not considering that an error.
> > >
> > > Hopefully that compromise makes sense.
> >
> > That does make sense.
> >
> > That also has implications for the debug messages that will say that the "set
> > timestamp" opcode is unsupported.
> >
> > What do you think of the following to try to clarify the distinction of
> > supported opcodes that are used internally by the driver and the ones
> > that are wrapped by a user command?
> >
> > -- >8 --
> > From d36b9ce647e615be8241d7fbf86ce555c72cc2a4 Mon Sep 17 00:00:00 2001
> > From: Dan Williams <dan.j.williams@intel.com>
> > Date: Fri, 27 Jan 2023 11:00:52 -0800
> > Subject: [PATCH] cxl/mbox: Indicate internal vs user enabled for command debug
> >
> > At initial enumeration provide an indication of whether the kernel will
> > use an opcode internally, and / or make the functionality available via
> > user ioctl command.
> >
> > Given that 'enum cxl_opcode' needs to be updated before the kernel can
> > use a CXL opcode internally, use that list to populate known_opcode().
> >
> > For the QEMU CXL emulation this results in the following debug messages
> > at startup:
> >
> > cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x100 Enabled: 0 User: none
> > cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x101 Enabled: 0 User: none
> > cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x102 Enabled: 0 User: none
> > cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x103 Enabled: 0 User: none
> > cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x200 Enabled: 1 User: Get FW Info
> > cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x300 Enabled: 0 User: none
> > cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x301 Enabled: 0 User: none
> > cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x400 Enabled: 1 User: Get Supported Logs
> > cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x401 Enabled: 1 User: Get Log
> > cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x4000 Enabled: 1 User: Identify Command
> > cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x4100 Enabled: 1 User: Get Partition Information
> > cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x4102 Enabled: 1 User: Get Label Storage Area
> > cxl_walk_cel: cxl_pci 0000:35:00.0: Opcode 0x4103 Enabled: 1 User: Set Label Storage Area
>
> I think this is nice debug output.
>
> This kind of threw me though. It looks like all enabled commands have
> user commands.
Only because I ran it on my branch without 'Set Timestamp' or 'Get Event
Records'. I would also expect that since the Security Commands are
internal-only that cxl_test output would show those commands enabled,
but without an associated user command. Turns out that's a quirk of
cxl_test where it fails to publish the opcodes in the CEL, but still
attempts to call them nonetheless.
I think that needs fixing. I think cxl_walk_cel() needs to cache the
availablility of the event commands and if not present exit out of
cxl_event_config() without failing. As it stands those will fail the
driver load on any device that does not publish those optional commands.
> Would it be better to s/Enabled/Kernel. Or perhaps driver?
Yeah, "Kernel" makes it clearer.
> Should we also check anything that is exclusive to the kernel and report
> that?
Exlcusivity can be ephemeral, like the label storage commands that are
ok through ioctl as long as nvdimm is not attached. The truly
kernel-internal exclusive commands will be "Kernel: 1 User: none".
> FWIW I've got a patch warming which updates the query command to factor in
> the kernel exclusive and hardware support, in case anyone is looking at
> this debug output as a way to determine that.
Cool.
>
> >
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > ---
> > drivers/cxl/core/mbox.c | 48 ++++++++++++++++++++++++++++++++++-------
> > drivers/cxl/cxlmem.h | 1 +
> > 2 files changed, 41 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> > index cae43cea00cd..45206465fd4d 100644
> > --- a/drivers/cxl/core/mbox.c
> > +++ b/drivers/cxl/core/mbox.c
> > @@ -587,6 +587,40 @@ static int cxl_xfer_log(struct cxl_dev_state *cxlds, uuid_t *uuid, u32 size, u8
> > return 0;
> > }
> >
> > +static bool known_opcode(u16 opcode)
>
> Perhaps driver_known_opcode?
A bit on the nose, this is the driver.
>
> > +{
> > + switch (opcode) {
> > + case CXL_MBOX_OP_GET_FW_INFO:
> > + case CXL_MBOX_OP_ACTIVATE_FW:
> > + case CXL_MBOX_OP_GET_SUPPORTED_LOGS:
> > + case CXL_MBOX_OP_GET_LOG:
> > + case CXL_MBOX_OP_IDENTIFY:
> > + case CXL_MBOX_OP_GET_PARTITION_INFO:
> > + case CXL_MBOX_OP_SET_PARTITION_INFO:
> > + case CXL_MBOX_OP_GET_LSA:
> > + case CXL_MBOX_OP_SET_LSA:
> > + case CXL_MBOX_OP_GET_HEALTH_INFO:
> > + case CXL_MBOX_OP_GET_ALERT_CONFIG:
> > + case CXL_MBOX_OP_SET_ALERT_CONFIG:
> > + case CXL_MBOX_OP_GET_SHUTDOWN_STATE:
> > + case CXL_MBOX_OP_SET_SHUTDOWN_STATE:
> > + case CXL_MBOX_OP_GET_POISON:
> > + case CXL_MBOX_OP_INJECT_POISON:
> > + case CXL_MBOX_OP_CLEAR_POISON:
> > + case CXL_MBOX_OP_GET_SCAN_MEDIA_CAPS:
> > + case CXL_MBOX_OP_SCAN_MEDIA:
> > + case CXL_MBOX_OP_GET_SCAN_MEDIA:
> > + case CXL_MBOX_OP_GET_SECURITY_STATE:
> > + case CXL_MBOX_OP_SET_PASSPHRASE:
> > + case CXL_MBOX_OP_DISABLE_PASSPHRASE:
> > + case CXL_MBOX_OP_UNLOCK:
> > + case CXL_MBOX_OP_FREEZE_SECURITY:
> > + case CXL_MBOX_OP_PASSPHRASE_SECURE_ERASE:
> > + return true;
> > + }
> > + return false;
> > +}
> > +
> > /**
> > * cxl_walk_cel() - Walk through the Command Effects Log.
> > * @cxlds: The device data for the operation
> > @@ -607,15 +641,13 @@ static void cxl_walk_cel(struct cxl_dev_state *cxlds, size_t size, u8 *cel)
> > for (i = 0; i < cel_entries; i++) {
> > u16 opcode = le16_to_cpu(cel_entry[i].opcode);
> > struct cxl_mem_command *cmd = cxl_mem_find_command(opcode);
> > + bool known = known_opcode(opcode);
> >
> > - if (!cmd) {
> > - dev_dbg(cxlds->dev,
> > - "Opcode 0x%04x unsupported by driver\n", opcode);
> > - continue;
> > - }
> > -
> > - set_bit(cmd->info.id, cxlds->enabled_cmds);
> > - dev_dbg(cxlds->dev, "Opcode 0x%04x enabled\n", opcode);
> > + if (cmd)
> > + set_bit(cmd->info.id, cxlds->enabled_cmds);
> > + dev_dbg(cxlds->dev, "Opcode %#04x Enabled: %d User: %s\n",
> > + opcode, !!known,
> > + cmd ? cxl_command_names[cmd->info.id].name : "none");
> > }
> > }
> >
> > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> > index ab138004f644..21f97d631823 100644
> > --- a/drivers/cxl/cxlmem.h
> > +++ b/drivers/cxl/cxlmem.h
> > @@ -299,6 +299,7 @@ enum cxl_opcode {
> > CXL_MBOX_OP_FREEZE_SECURITY = 0x4504,
> > CXL_MBOX_OP_PASSPHRASE_SECURE_ERASE = 0x4505,
> > CXL_MBOX_OP_MAX = 0x10000
> > + /* update known_opcode() when adding opcode support to this list */
>
> I'm wishing there was a way to not have to do this. All I can think of to
> make it easier would be to make 'driver_know_opcode()' static inline and
> in this header. I'm sure some macro magic could be used too. Although
> I'm not always a fan.
Yeah, a val_is_in_enum() helper seems like something someone would have
wrote...
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] cxl/pci: Set the device timestamp
2023-01-26 18:04 [PATCH] cxl/pci: Set the device timestamp Jonathan Cameron
` (2 preceding siblings ...)
2023-01-26 20:22 ` Dan Williams
@ 2023-01-28 11:21 ` kernel test robot
2023-01-28 11:32 ` kernel test robot
2023-01-28 16:01 ` kernel test robot
5 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2023-01-28 11:21 UTC (permalink / raw)
To: Jonathan Cameron, linux-cxl
Cc: oe-kbuild-all, Alison Schofield, Vishal Verma, Ira Weiny,
Dan Williams, linuxarm
Hi Jonathan,
I love your patch! Yet something to improve:
[auto build test ERROR on 172738bbccdb4ef76bdd72fc72a315c741c39161]
url: https://github.com/intel-lab-lkp/linux/commits/Jonathan-Cameron/cxl-pci-Set-the-device-timestamp/20230128-152300
base: 172738bbccdb4ef76bdd72fc72a315c741c39161
patch link: https://lore.kernel.org/r/20230126180458.5145-1-Jonathan.Cameron%40huawei.com
patch subject: [PATCH] cxl/pci: Set the device timestamp
config: mips-allmodconfig (https://download.01.org/0day-ci/archive/20230128/202301281954.CS360mGh-lkp@intel.com/config)
compiler: mips-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/f36cca050cf662048001b7b7686040270e4a8c00
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Jonathan-Cameron/cxl-pci-Set-the-device-timestamp/20230128-152300
git checkout f36cca050cf662048001b7b7686040270e4a8c00
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=mips olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All error/warnings (new ones prefixed by >>):
In file included from drivers/cxl/pci.c:5:
>> include/linux/timekeeping.h:19:43: warning: 'struct timespec64' declared inside parameter list will not be visible outside of this definition or declaration
19 | extern int do_settimeofday64(const struct timespec64 *ts);
| ^~~~~~~~~~
>> include/linux/timekeeping.h:21:47: warning: 'struct timezone' declared inside parameter list will not be visible outside of this definition or declaration
21 | const struct timezone *tz);
| ^~~~~~~~
include/linux/timekeeping.h:20:47: warning: 'struct timespec64' declared inside parameter list will not be visible outside of this definition or declaration
20 | extern int do_sys_settimeofday64(const struct timespec64 *tv,
| ^~~~~~~~~~
include/linux/timekeeping.h:41:39: warning: 'struct timespec64' declared inside parameter list will not be visible outside of this definition or declaration
41 | extern void ktime_get_raw_ts64(struct timespec64 *ts);
| ^~~~~~~~~~
include/linux/timekeeping.h:42:35: warning: 'struct timespec64' declared inside parameter list will not be visible outside of this definition or declaration
42 | extern void ktime_get_ts64(struct timespec64 *ts);
| ^~~~~~~~~~
include/linux/timekeeping.h:43:40: warning: 'struct timespec64' declared inside parameter list will not be visible outside of this definition or declaration
43 | extern void ktime_get_real_ts64(struct timespec64 *tv);
| ^~~~~~~~~~
include/linux/timekeeping.h:44:42: warning: 'struct timespec64' declared inside parameter list will not be visible outside of this definition or declaration
44 | extern void ktime_get_coarse_ts64(struct timespec64 *ts);
| ^~~~~~~~~~
include/linux/timekeeping.h:45:47: warning: 'struct timespec64' declared inside parameter list will not be visible outside of this definition or declaration
45 | extern void ktime_get_coarse_real_ts64(struct timespec64 *ts);
| ^~~~~~~~~~
include/linux/timekeeping.h:47:27: warning: 'struct timespec64' declared inside parameter list will not be visible outside of this definition or declaration
47 | void getboottime64(struct timespec64 *ts);
| ^~~~~~~~~~
>> include/linux/timekeeping.h:52:8: error: unknown type name 'time64_t'
52 | extern time64_t ktime_get_seconds(void);
| ^~~~~~~~
include/linux/timekeeping.h:53:8: error: unknown type name 'time64_t'
53 | extern time64_t __ktime_get_real_seconds(void);
| ^~~~~~~~
include/linux/timekeeping.h:54:8: error: unknown type name 'time64_t'
54 | extern time64_t ktime_get_real_seconds(void);
| ^~~~~~~~
include/linux/timekeeping.h:67:8: error: unknown type name 'ktime_t'
67 | extern ktime_t ktime_get(void);
| ^~~~~~~
include/linux/timekeeping.h:68:8: error: unknown type name 'ktime_t'
68 | extern ktime_t ktime_get_with_offset(enum tk_offsets offs);
| ^~~~~~~
include/linux/timekeeping.h:69:8: error: unknown type name 'ktime_t'
69 | extern ktime_t ktime_get_coarse_with_offset(enum tk_offsets offs);
| ^~~~~~~
include/linux/timekeeping.h:70:8: error: unknown type name 'ktime_t'
70 | extern ktime_t ktime_mono_to_any(ktime_t tmono, enum tk_offsets offs);
| ^~~~~~~
include/linux/timekeeping.h:70:34: error: unknown type name 'ktime_t'; did you mean 'timer_t'?
70 | extern ktime_t ktime_mono_to_any(ktime_t tmono, enum tk_offsets offs);
| ^~~~~~~
| timer_t
include/linux/timekeeping.h:71:8: error: unknown type name 'ktime_t'
71 | extern ktime_t ktime_get_raw(void);
| ^~~~~~~
include/linux/timekeeping.h:77:15: error: unknown type name 'ktime_t'
77 | static inline ktime_t ktime_get_real(void)
| ^~~~~~~
include/linux/timekeeping.h:82:15: error: unknown type name 'ktime_t'
82 | static inline ktime_t ktime_get_coarse_real(void)
| ^~~~~~~
include/linux/timekeeping.h:93:15: error: unknown type name 'ktime_t'
93 | static inline ktime_t ktime_get_boottime(void)
| ^~~~~~~
include/linux/timekeeping.h:98:15: error: unknown type name 'ktime_t'
98 | static inline ktime_t ktime_get_coarse_boottime(void)
| ^~~~~~~
include/linux/timekeeping.h:106:15: error: unknown type name 'ktime_t'
106 | static inline ktime_t ktime_get_clocktai(void)
| ^~~~~~~
include/linux/timekeeping.h:111:15: error: unknown type name 'ktime_t'
111 | static inline ktime_t ktime_get_coarse_clocktai(void)
| ^~~~~~~
include/linux/timekeeping.h:116:15: error: unknown type name 'ktime_t'
116 | static inline ktime_t ktime_get_coarse(void)
| ^~~~~~~
include/linux/timekeeping.h: In function 'ktime_get_coarse':
>> include/linux/timekeeping.h:118:27: error: storage size of 'ts' isn't known
118 | struct timespec64 ts;
| ^~
include/linux/timekeeping.h:121:16: error: implicit declaration of function 'timespec64_to_ktime' [-Werror=implicit-function-declaration]
121 | return timespec64_to_ktime(ts);
| ^~~~~~~~~~~~~~~~~~~
include/linux/timekeeping.h:118:27: warning: unused variable 'ts' [-Wunused-variable]
118 | struct timespec64 ts;
| ^~
include/linux/timekeeping.h: In function 'ktime_get_coarse_ns':
include/linux/timekeeping.h:126:16: error: implicit declaration of function 'ktime_to_ns' [-Werror=implicit-function-declaration]
126 | return ktime_to_ns(ktime_get_coarse());
| ^~~~~~~~~~~
include/linux/timekeeping.h: At top level:
include/linux/timekeeping.h:147:15: error: unknown type name 'ktime_t'
147 | static inline ktime_t ktime_mono_to_real(ktime_t mono)
| ^~~~~~~
include/linux/timekeeping.h:147:42: error: unknown type name 'ktime_t'; did you mean 'timer_t'?
147 | static inline ktime_t ktime_mono_to_real(ktime_t mono)
| ^~~~~~~
| timer_t
include/linux/timekeeping.h:188:51: warning: 'struct timespec64' declared inside parameter list will not be visible outside of this definition or declaration
188 | static inline void ktime_get_boottime_ts64(struct timespec64 *ts)
| ^~~~~~~~~~
include/linux/timekeeping.h: In function 'ktime_get_boottime_ts64':
include/linux/timekeeping.h:190:15: error: implicit declaration of function 'ktime_to_timespec64' [-Werror=implicit-function-declaration]
190 | *ts = ktime_to_timespec64(ktime_get_boottime());
| ^~~~~~~~~~~~~~~~~~~
>> include/linux/timekeeping.h:190:13: error: invalid use of undefined type 'struct timespec64'
190 | *ts = ktime_to_timespec64(ktime_get_boottime());
| ^
include/linux/timekeeping.h: At top level:
include/linux/timekeeping.h:193:58: warning: 'struct timespec64' declared inside parameter list will not be visible outside of this definition or declaration
193 | static inline void ktime_get_coarse_boottime_ts64(struct timespec64 *ts)
| ^~~~~~~~~~
include/linux/timekeeping.h: In function 'ktime_get_coarse_boottime_ts64':
include/linux/timekeeping.h:195:13: error: invalid use of undefined type 'struct timespec64'
195 | *ts = ktime_to_timespec64(ktime_get_coarse_boottime());
| ^
include/linux/timekeeping.h: At top level:
include/linux/timekeeping.h:198:15: error: unknown type name 'time64_t'
198 | static inline time64_t ktime_get_boottime_seconds(void)
| ^~~~~~~~
include/linux/timekeeping.h: In function 'ktime_get_boottime_seconds':
include/linux/timekeeping.h:200:16: error: implicit declaration of function 'ktime_divns'; did you mean 'ktime_get_ns'? [-Werror=implicit-function-declaration]
200 | return ktime_divns(ktime_get_coarse_boottime(), NSEC_PER_SEC);
| ^~~~~~~~~~~
| ktime_get_ns
>> include/linux/timekeeping.h:200:57: error: 'NSEC_PER_SEC' undeclared (first use in this function); did you mean 'CLOCKS_PER_SEC'?
200 | return ktime_divns(ktime_get_coarse_boottime(), NSEC_PER_SEC);
| ^~~~~~~~~~~~
| CLOCKS_PER_SEC
include/linux/timekeeping.h:200:57: note: each undeclared identifier is reported only once for each function it appears in
include/linux/timekeeping.h: At top level:
include/linux/timekeeping.h:203:51: warning: 'struct timespec64' declared inside parameter list will not be visible outside of this definition or declaration
203 | static inline void ktime_get_clocktai_ts64(struct timespec64 *ts)
| ^~~~~~~~~~
include/linux/timekeeping.h: In function 'ktime_get_clocktai_ts64':
include/linux/timekeeping.h:205:13: error: invalid use of undefined type 'struct timespec64'
205 | *ts = ktime_to_timespec64(ktime_get_clocktai());
| ^
include/linux/timekeeping.h: At top level:
include/linux/timekeeping.h:208:58: warning: 'struct timespec64' declared inside parameter list will not be visible outside of this definition or declaration
208 | static inline void ktime_get_coarse_clocktai_ts64(struct timespec64 *ts)
| ^~~~~~~~~~
include/linux/timekeeping.h: In function 'ktime_get_coarse_clocktai_ts64':
include/linux/timekeeping.h:210:13: error: invalid use of undefined type 'struct timespec64'
210 | *ts = ktime_to_timespec64(ktime_get_coarse_clocktai());
| ^
include/linux/timekeeping.h: At top level:
include/linux/timekeeping.h:213:15: error: unknown type name 'time64_t'
213 | static inline time64_t ktime_get_clocktai_seconds(void)
| ^~~~~~~~
include/linux/timekeeping.h: In function 'ktime_get_clocktai_seconds':
include/linux/timekeeping.h:215:57: error: 'NSEC_PER_SEC' undeclared (first use in this function); did you mean 'CLOCKS_PER_SEC'?
215 | return ktime_divns(ktime_get_coarse_clocktai(), NSEC_PER_SEC);
| ^~~~~~~~~~~~
| CLOCKS_PER_SEC
include/linux/timekeeping.h: At top level:
include/linux/timekeeping.h:224:57: warning: 'struct timespec64' declared inside parameter list will not be visible outside of this definition or declaration
224 | extern void timekeeping_inject_sleeptime64(const struct timespec64 *delta);
| ^~~~~~~~~~
include/linux/timekeeping.h:249:9: error: unknown type name 'ktime_t'
249 | ktime_t real;
| ^~~~~~~
include/linux/timekeeping.h:250:9: error: unknown type name 'ktime_t'
250 | ktime_t raw;
| ^~~~~~~
include/linux/timekeeping.h:264:9: error: unknown type name 'ktime_t'
264 | ktime_t device;
| ^~~~~~~
include/linux/timekeeping.h:265:9: error: unknown type name 'ktime_t'
265 | ktime_t sys_realtime;
| ^~~~~~~
include/linux/timekeeping.h:266:9: error: unknown type name 'ktime_t'
266 | ktime_t sys_monoraw;
| ^~~~~~~
include/linux/timekeeping.h:285:44: error: unknown type name 'ktime_t'; did you mean 'timer_t'?
285 | int (*get_time_fn)(ktime_t *device_time,
| ^~~~~~~
| timer_t
include/linux/timekeeping.h:288:25: error: expected ';', ',' or ')' before 'void'
288 | void *ctx,
| ^~~~
include/linux/timekeeping.h:305:44: warning: 'struct timespec64' declared inside parameter list will not be visible outside of this definition or declaration
305 | extern void read_persistent_clock64(struct timespec64 *ts);
| ^~~~~~~~~~
include/linux/timekeeping.h:306:50: warning: 'struct timespec64' declared inside parameter list will not be visible outside of this definition or declaration
306 | void read_persistent_wall_and_boot_offset(struct timespec64 *wall_clock,
| ^~~~~~~~~~
include/linux/timekeeping.h:309:45: warning: 'struct timespec64' declared inside parameter list will not be visible outside of this definition or declaration
309 | extern int update_persistent_clock64(struct timespec64 now);
| ^~~~~~~~~~
In file included from include/linux/timer.h:6,
from include/linux/workqueue.h:9,
from include/linux/srcu.h:21,
from include/linux/notifier.h:16,
from arch/mips/include/asm/uprobes.h:9,
from include/linux/uprobes.h:49,
from include/linux/mm_types.h:16,
from include/linux/buildid.h:5,
from include/linux/module.h:14,
from drivers/cxl/pci.c:6:
include/linux/ktime.h:71:23: error: conflicting types for 'timespec64_to_ktime'; have 'ktime_t(struct timespec64)' {aka 'long long int(struct timespec64)'}
71 | static inline ktime_t timespec64_to_ktime(struct timespec64 ts)
| ^~~~~~~~~~~~~~~~~~~
include/linux/timekeeping.h:121:16: note: previous implicit declaration of 'timespec64_to_ktime' with type 'int()'
121 | return timespec64_to_ktime(ts);
| ^~~~~~~~~~~~~~~~~~~
include/linux/ktime.h:80:19: error: conflicting types for 'ktime_to_ns'; have 's64(const ktime_t)' {aka 'long long int(const long long int)'}
80 | static inline s64 ktime_to_ns(const ktime_t kt)
| ^~~~~~~~~~~
include/linux/timekeeping.h:126:16: note: previous implicit declaration of 'ktime_to_ns' with type 'int()'
126 | return ktime_to_ns(ktime_get_coarse());
| ^~~~~~~~~~~
include/linux/ktime.h:130:19: error: conflicting types for 'ktime_divns'; have 's64(const ktime_t, s64)' {aka 'long long int(const long long int, long long int)'}
130 | static inline s64 ktime_divns(const ktime_t kt, s64 div)
| ^~~~~~~~~~~
include/linux/timekeeping.h:200:16: note: previous implicit declaration of 'ktime_divns' with type 'int()'
200 | return ktime_divns(ktime_get_coarse_boottime(), NSEC_PER_SEC);
| ^~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/time64_t +52 include/linux/timekeeping.h
93b5a9a7051e51 Ingo Molnar 2017-02-05 15
8b094cd03b4a37 Thomas Gleixner 2014-07-16 16 /*
8b094cd03b4a37 Thomas Gleixner 2014-07-16 17 * Get and set timeofday
8b094cd03b4a37 Thomas Gleixner 2014-07-16 18 */
21f7eca555ad14 pang.xunlei 2014-11-18 @19 extern int do_settimeofday64(const struct timespec64 *ts);
86d3473224b004 Baolin Wang 2016-04-08 20 extern int do_sys_settimeofday64(const struct timespec64 *tv,
8b094cd03b4a37 Thomas Gleixner 2014-07-16 @21 const struct timezone *tz);
8758a240e2d74c Baolin Wang 2015-07-29 22
0e3fd810c4f41d Arnd Bergmann 2018-07-10 23 /*
0e3fd810c4f41d Arnd Bergmann 2018-07-10 24 * ktime_get() family: read the current time in a multitude of ways,
0e3fd810c4f41d Arnd Bergmann 2018-07-10 25 *
0e3fd810c4f41d Arnd Bergmann 2018-07-10 26 * The default time reference is CLOCK_MONOTONIC, starting at
0e3fd810c4f41d Arnd Bergmann 2018-07-10 27 * boot time but not counting the time spent in suspend.
0e3fd810c4f41d Arnd Bergmann 2018-07-10 28 * For other references, use the functions with "real", "clocktai",
0e3fd810c4f41d Arnd Bergmann 2018-07-10 29 * "boottime" and "raw" suffixes.
0e3fd810c4f41d Arnd Bergmann 2018-07-10 30 *
0e3fd810c4f41d Arnd Bergmann 2018-07-10 31 * To get the time in a different format, use the ones wit
0e3fd810c4f41d Arnd Bergmann 2018-07-10 32 * "ns", "ts64" and "seconds" suffix.
0e3fd810c4f41d Arnd Bergmann 2018-07-10 33 *
0e3fd810c4f41d Arnd Bergmann 2018-07-10 34 * See Documentation/core-api/timekeeping.rst for more details.
0e3fd810c4f41d Arnd Bergmann 2018-07-10 35 */
0e3fd810c4f41d Arnd Bergmann 2018-07-10 36
0e3fd810c4f41d Arnd Bergmann 2018-07-10 37
8b094cd03b4a37 Thomas Gleixner 2014-07-16 38 /*
6546911ed369af Arnd Bergmann 2017-10-19 39 * timespec64 based interfaces
8b094cd03b4a37 Thomas Gleixner 2014-07-16 40 */
fb7fcc96a86cfa Arnd Bergmann 2018-04-27 41 extern void ktime_get_raw_ts64(struct timespec64 *ts);
d6d29896c665df Thomas Gleixner 2014-07-16 42 extern void ktime_get_ts64(struct timespec64 *ts);
edca71fecb77e2 Arnd Bergmann 2018-04-27 43 extern void ktime_get_real_ts64(struct timespec64 *tv);
fb7fcc96a86cfa Arnd Bergmann 2018-04-27 44 extern void ktime_get_coarse_ts64(struct timespec64 *ts);
fb7fcc96a86cfa Arnd Bergmann 2018-04-27 45 extern void ktime_get_coarse_real_ts64(struct timespec64 *ts);
fb7fcc96a86cfa Arnd Bergmann 2018-04-27 46
fb7fcc96a86cfa Arnd Bergmann 2018-04-27 47 void getboottime64(struct timespec64 *ts);
fb7fcc96a86cfa Arnd Bergmann 2018-04-27 48
fb7fcc96a86cfa Arnd Bergmann 2018-04-27 49 /*
fb7fcc96a86cfa Arnd Bergmann 2018-04-27 50 * time64_t base interfaces
fb7fcc96a86cfa Arnd Bergmann 2018-04-27 51 */
9e3680b1750b9a Heena Sirwani 2014-10-29 @52 extern time64_t ktime_get_seconds(void);
6909e29fdefbb7 Arnd Bergmann 2017-10-12 53 extern time64_t __ktime_get_real_seconds(void);
dbe7aa622db96b Heena Sirwani 2014-10-29 54 extern time64_t ktime_get_real_seconds(void);
d6d29896c665df Thomas Gleixner 2014-07-16 55
8b094cd03b4a37 Thomas Gleixner 2014-07-16 56 /*
8b094cd03b4a37 Thomas Gleixner 2014-07-16 57 * ktime_t based interfaces
8b094cd03b4a37 Thomas Gleixner 2014-07-16 58 */
a3ed0e4393d688 Thomas Gleixner 2018-04-25 59
0077dc60f274b9 Thomas Gleixner 2014-07-16 60 enum tk_offsets {
0077dc60f274b9 Thomas Gleixner 2014-07-16 61 TK_OFFS_REAL,
a3ed0e4393d688 Thomas Gleixner 2018-04-25 62 TK_OFFS_BOOT,
0077dc60f274b9 Thomas Gleixner 2014-07-16 63 TK_OFFS_TAI,
0077dc60f274b9 Thomas Gleixner 2014-07-16 64 TK_OFFS_MAX,
0077dc60f274b9 Thomas Gleixner 2014-07-16 65 };
0077dc60f274b9 Thomas Gleixner 2014-07-16 66
8b094cd03b4a37 Thomas Gleixner 2014-07-16 67 extern ktime_t ktime_get(void);
0077dc60f274b9 Thomas Gleixner 2014-07-16 68 extern ktime_t ktime_get_with_offset(enum tk_offsets offs);
b9ff604cff1135 Arnd Bergmann 2018-04-27 69 extern ktime_t ktime_get_coarse_with_offset(enum tk_offsets offs);
9a6b51976ea3a3 Thomas Gleixner 2014-07-16 70 extern ktime_t ktime_mono_to_any(ktime_t tmono, enum tk_offsets offs);
f519b1a2e08c91 Thomas Gleixner 2014-07-16 71 extern ktime_t ktime_get_raw(void);
6374f9124efea5 Harald Geyer 2015-04-07 72 extern u32 ktime_get_resolution_ns(void);
8b094cd03b4a37 Thomas Gleixner 2014-07-16 73
f5264d5d5a0729 Thomas Gleixner 2014-07-16 74 /**
f5264d5d5a0729 Thomas Gleixner 2014-07-16 75 * ktime_get_real - get the real (wall-) time in ktime_t format
f5264d5d5a0729 Thomas Gleixner 2014-07-16 76 */
f5264d5d5a0729 Thomas Gleixner 2014-07-16 77 static inline ktime_t ktime_get_real(void)
f5264d5d5a0729 Thomas Gleixner 2014-07-16 78 {
f5264d5d5a0729 Thomas Gleixner 2014-07-16 79 return ktime_get_with_offset(TK_OFFS_REAL);
f5264d5d5a0729 Thomas Gleixner 2014-07-16 80 }
f5264d5d5a0729 Thomas Gleixner 2014-07-16 81
b9ff604cff1135 Arnd Bergmann 2018-04-27 82 static inline ktime_t ktime_get_coarse_real(void)
b9ff604cff1135 Arnd Bergmann 2018-04-27 83 {
b9ff604cff1135 Arnd Bergmann 2018-04-27 84 return ktime_get_coarse_with_offset(TK_OFFS_REAL);
b9ff604cff1135 Arnd Bergmann 2018-04-27 85 }
b9ff604cff1135 Arnd Bergmann 2018-04-27 86
a3ed0e4393d688 Thomas Gleixner 2018-04-25 87 /**
a3ed0e4393d688 Thomas Gleixner 2018-04-25 88 * ktime_get_boottime - Returns monotonic time since boot in ktime_t format
a3ed0e4393d688 Thomas Gleixner 2018-04-25 89 *
a3ed0e4393d688 Thomas Gleixner 2018-04-25 90 * This is similar to CLOCK_MONTONIC/ktime_get, but also includes the
a3ed0e4393d688 Thomas Gleixner 2018-04-25 91 * time spent in suspend.
a3ed0e4393d688 Thomas Gleixner 2018-04-25 92 */
a3ed0e4393d688 Thomas Gleixner 2018-04-25 93 static inline ktime_t ktime_get_boottime(void)
a3ed0e4393d688 Thomas Gleixner 2018-04-25 94 {
a3ed0e4393d688 Thomas Gleixner 2018-04-25 95 return ktime_get_with_offset(TK_OFFS_BOOT);
a3ed0e4393d688 Thomas Gleixner 2018-04-25 96 }
a3ed0e4393d688 Thomas Gleixner 2018-04-25 97
b9ff604cff1135 Arnd Bergmann 2018-04-27 98 static inline ktime_t ktime_get_coarse_boottime(void)
b9ff604cff1135 Arnd Bergmann 2018-04-27 99 {
b9ff604cff1135 Arnd Bergmann 2018-04-27 100 return ktime_get_coarse_with_offset(TK_OFFS_BOOT);
b9ff604cff1135 Arnd Bergmann 2018-04-27 101 }
b9ff604cff1135 Arnd Bergmann 2018-04-27 102
afab07c0e91ecf Thomas Gleixner 2014-07-16 103 /**
afab07c0e91ecf Thomas Gleixner 2014-07-16 104 * ktime_get_clocktai - Returns the TAI time of day in ktime_t format
afab07c0e91ecf Thomas Gleixner 2014-07-16 105 */
afab07c0e91ecf Thomas Gleixner 2014-07-16 106 static inline ktime_t ktime_get_clocktai(void)
afab07c0e91ecf Thomas Gleixner 2014-07-16 107 {
afab07c0e91ecf Thomas Gleixner 2014-07-16 108 return ktime_get_with_offset(TK_OFFS_TAI);
afab07c0e91ecf Thomas Gleixner 2014-07-16 109 }
afab07c0e91ecf Thomas Gleixner 2014-07-16 110
b9ff604cff1135 Arnd Bergmann 2018-04-27 111 static inline ktime_t ktime_get_coarse_clocktai(void)
b9ff604cff1135 Arnd Bergmann 2018-04-27 112 {
b9ff604cff1135 Arnd Bergmann 2018-04-27 113 return ktime_get_coarse_with_offset(TK_OFFS_TAI);
b9ff604cff1135 Arnd Bergmann 2018-04-27 114 }
b9ff604cff1135 Arnd Bergmann 2018-04-27 115
4c54294d01e605 Jason A. Donenfeld 2019-06-21 116 static inline ktime_t ktime_get_coarse(void)
4c54294d01e605 Jason A. Donenfeld 2019-06-21 117 {
4c54294d01e605 Jason A. Donenfeld 2019-06-21 @118 struct timespec64 ts;
4c54294d01e605 Jason A. Donenfeld 2019-06-21 119
4c54294d01e605 Jason A. Donenfeld 2019-06-21 120 ktime_get_coarse_ts64(&ts);
4c54294d01e605 Jason A. Donenfeld 2019-06-21 121 return timespec64_to_ktime(ts);
4c54294d01e605 Jason A. Donenfeld 2019-06-21 122 }
4c54294d01e605 Jason A. Donenfeld 2019-06-21 123
4c54294d01e605 Jason A. Donenfeld 2019-06-21 124 static inline u64 ktime_get_coarse_ns(void)
4c54294d01e605 Jason A. Donenfeld 2019-06-21 125 {
4c54294d01e605 Jason A. Donenfeld 2019-06-21 126 return ktime_to_ns(ktime_get_coarse());
4c54294d01e605 Jason A. Donenfeld 2019-06-21 127 }
4c54294d01e605 Jason A. Donenfeld 2019-06-21 128
4c54294d01e605 Jason A. Donenfeld 2019-06-21 129 static inline u64 ktime_get_coarse_real_ns(void)
4c54294d01e605 Jason A. Donenfeld 2019-06-21 130 {
4c54294d01e605 Jason A. Donenfeld 2019-06-21 131 return ktime_to_ns(ktime_get_coarse_real());
4c54294d01e605 Jason A. Donenfeld 2019-06-21 132 }
4c54294d01e605 Jason A. Donenfeld 2019-06-21 133
d48e0cd8fcaf31 Jason A. Donenfeld 2019-06-24 134 static inline u64 ktime_get_coarse_boottime_ns(void)
4c54294d01e605 Jason A. Donenfeld 2019-06-21 135 {
4c54294d01e605 Jason A. Donenfeld 2019-06-21 136 return ktime_to_ns(ktime_get_coarse_boottime());
4c54294d01e605 Jason A. Donenfeld 2019-06-21 137 }
4c54294d01e605 Jason A. Donenfeld 2019-06-21 138
4c54294d01e605 Jason A. Donenfeld 2019-06-21 139 static inline u64 ktime_get_coarse_clocktai_ns(void)
4c54294d01e605 Jason A. Donenfeld 2019-06-21 140 {
4c54294d01e605 Jason A. Donenfeld 2019-06-21 141 return ktime_to_ns(ktime_get_coarse_clocktai());
4c54294d01e605 Jason A. Donenfeld 2019-06-21 142 }
4c54294d01e605 Jason A. Donenfeld 2019-06-21 143
9a6b51976ea3a3 Thomas Gleixner 2014-07-16 144 /**
9a6b51976ea3a3 Thomas Gleixner 2014-07-16 145 * ktime_mono_to_real - Convert monotonic time to clock realtime
9a6b51976ea3a3 Thomas Gleixner 2014-07-16 146 */
9a6b51976ea3a3 Thomas Gleixner 2014-07-16 147 static inline ktime_t ktime_mono_to_real(ktime_t mono)
9a6b51976ea3a3 Thomas Gleixner 2014-07-16 148 {
9a6b51976ea3a3 Thomas Gleixner 2014-07-16 149 return ktime_mono_to_any(mono, TK_OFFS_REAL);
9a6b51976ea3a3 Thomas Gleixner 2014-07-16 150 }
9a6b51976ea3a3 Thomas Gleixner 2014-07-16 151
897994e32b2b0a Thomas Gleixner 2014-07-16 152 static inline u64 ktime_get_ns(void)
897994e32b2b0a Thomas Gleixner 2014-07-16 153 {
897994e32b2b0a Thomas Gleixner 2014-07-16 154 return ktime_to_ns(ktime_get());
897994e32b2b0a Thomas Gleixner 2014-07-16 155 }
897994e32b2b0a Thomas Gleixner 2014-07-16 156
897994e32b2b0a Thomas Gleixner 2014-07-16 157 static inline u64 ktime_get_real_ns(void)
897994e32b2b0a Thomas Gleixner 2014-07-16 158 {
897994e32b2b0a Thomas Gleixner 2014-07-16 159 return ktime_to_ns(ktime_get_real());
897994e32b2b0a Thomas Gleixner 2014-07-16 160 }
897994e32b2b0a Thomas Gleixner 2014-07-16 161
9285ec4c8b61d4 Jason A. Donenfeld 2019-06-21 162 static inline u64 ktime_get_boottime_ns(void)
a3ed0e4393d688 Thomas Gleixner 2018-04-25 163 {
a3ed0e4393d688 Thomas Gleixner 2018-04-25 164 return ktime_to_ns(ktime_get_boottime());
a3ed0e4393d688 Thomas Gleixner 2018-04-25 165 }
a3ed0e4393d688 Thomas Gleixner 2018-04-25 166
9285ec4c8b61d4 Jason A. Donenfeld 2019-06-21 167 static inline u64 ktime_get_clocktai_ns(void)
fe5fba05b46c79 Peter Zijlstra 2015-03-17 168 {
fe5fba05b46c79 Peter Zijlstra 2015-03-17 169 return ktime_to_ns(ktime_get_clocktai());
fe5fba05b46c79 Peter Zijlstra 2015-03-17 170 }
fe5fba05b46c79 Peter Zijlstra 2015-03-17 171
f519b1a2e08c91 Thomas Gleixner 2014-07-16 172 static inline u64 ktime_get_raw_ns(void)
f519b1a2e08c91 Thomas Gleixner 2014-07-16 173 {
f519b1a2e08c91 Thomas Gleixner 2014-07-16 174 return ktime_to_ns(ktime_get_raw());
f519b1a2e08c91 Thomas Gleixner 2014-07-16 175 }
f519b1a2e08c91 Thomas Gleixner 2014-07-16 176
4396e058c52e16 Thomas Gleixner 2014-07-16 177 extern u64 ktime_get_mono_fast_ns(void);
f09cb9a1808e35 Peter Zijlstra 2015-03-19 178 extern u64 ktime_get_raw_fast_ns(void);
a3ed0e4393d688 Thomas Gleixner 2018-04-25 179 extern u64 ktime_get_boot_fast_ns(void);
3dc6ffae2da201 Kurt Kanzenbach 2022-04-14 180 extern u64 ktime_get_tai_fast_ns(void);
4c3711d7fb4763 Thomas Gleixner 2017-08-31 181 extern u64 ktime_get_real_fast_ns(void);
4396e058c52e16 Thomas Gleixner 2014-07-16 182
d6c7270e913db7 Thomas Gleixner 2018-03-01 183 /*
06aa376903b6e8 Arnd Bergmann 2018-04-27 184 * timespec64/time64_t interfaces utilizing the ktime based ones
06aa376903b6e8 Arnd Bergmann 2018-04-27 185 * for API completeness, these could be implemented more efficiently
06aa376903b6e8 Arnd Bergmann 2018-04-27 186 * if needed.
d6c7270e913db7 Thomas Gleixner 2018-03-01 187 */
fb7fcc96a86cfa Arnd Bergmann 2018-04-27 188 static inline void ktime_get_boottime_ts64(struct timespec64 *ts)
a3ed0e4393d688 Thomas Gleixner 2018-04-25 189 {
a3ed0e4393d688 Thomas Gleixner 2018-04-25 @190 *ts = ktime_to_timespec64(ktime_get_boottime());
a3ed0e4393d688 Thomas Gleixner 2018-04-25 191 }
a3ed0e4393d688 Thomas Gleixner 2018-04-25 192
06aa376903b6e8 Arnd Bergmann 2018-04-27 193 static inline void ktime_get_coarse_boottime_ts64(struct timespec64 *ts)
06aa376903b6e8 Arnd Bergmann 2018-04-27 194 {
06aa376903b6e8 Arnd Bergmann 2018-04-27 195 *ts = ktime_to_timespec64(ktime_get_coarse_boottime());
06aa376903b6e8 Arnd Bergmann 2018-04-27 196 }
06aa376903b6e8 Arnd Bergmann 2018-04-27 197
06aa376903b6e8 Arnd Bergmann 2018-04-27 198 static inline time64_t ktime_get_boottime_seconds(void)
06aa376903b6e8 Arnd Bergmann 2018-04-27 199 {
06aa376903b6e8 Arnd Bergmann 2018-04-27 @200 return ktime_divns(ktime_get_coarse_boottime(), NSEC_PER_SEC);
06aa376903b6e8 Arnd Bergmann 2018-04-27 201 }
06aa376903b6e8 Arnd Bergmann 2018-04-27 202
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH] cxl/pci: Set the device timestamp
2023-01-26 18:04 [PATCH] cxl/pci: Set the device timestamp Jonathan Cameron
` (3 preceding siblings ...)
2023-01-28 11:21 ` kernel test robot
@ 2023-01-28 11:32 ` kernel test robot
2023-01-30 15:10 ` Jonathan Cameron
2023-01-28 16:01 ` kernel test robot
5 siblings, 1 reply; 17+ messages in thread
From: kernel test robot @ 2023-01-28 11:32 UTC (permalink / raw)
To: Jonathan Cameron, linux-cxl
Cc: oe-kbuild-all, Alison Schofield, Vishal Verma, Ira Weiny,
Dan Williams, linuxarm
Hi Jonathan,
I love your patch! Yet something to improve:
[auto build test ERROR on 172738bbccdb4ef76bdd72fc72a315c741c39161]
url: https://github.com/intel-lab-lkp/linux/commits/Jonathan-Cameron/cxl-pci-Set-the-device-timestamp/20230128-152300
base: 172738bbccdb4ef76bdd72fc72a315c741c39161
patch link: https://lore.kernel.org/r/20230126180458.5145-1-Jonathan.Cameron%40huawei.com
patch subject: [PATCH] cxl/pci: Set the device timestamp
config: ia64-allyesconfig (https://download.01.org/0day-ci/archive/20230128/202301281915.t1bQicuY-lkp@intel.com/config)
compiler: ia64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/f36cca050cf662048001b7b7686040270e4a8c00
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Jonathan-Cameron/cxl-pci-Set-the-device-timestamp/20230128-152300
git checkout f36cca050cf662048001b7b7686040270e4a8c00
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
In file included from drivers/cxl/pci.c:5:
>> include/linux/timekeeping.h:67:8: error: unknown type name 'ktime_t'
67 | extern ktime_t ktime_get(void);
| ^~~~~~~
include/linux/timekeeping.h:68:8: error: unknown type name 'ktime_t'
68 | extern ktime_t ktime_get_with_offset(enum tk_offsets offs);
| ^~~~~~~
include/linux/timekeeping.h:69:8: error: unknown type name 'ktime_t'
69 | extern ktime_t ktime_get_coarse_with_offset(enum tk_offsets offs);
| ^~~~~~~
include/linux/timekeeping.h:70:8: error: unknown type name 'ktime_t'
70 | extern ktime_t ktime_mono_to_any(ktime_t tmono, enum tk_offsets offs);
| ^~~~~~~
include/linux/timekeeping.h:70:34: error: unknown type name 'ktime_t'; did you mean 'timer_t'?
70 | extern ktime_t ktime_mono_to_any(ktime_t tmono, enum tk_offsets offs);
| ^~~~~~~
| timer_t
include/linux/timekeeping.h:71:8: error: unknown type name 'ktime_t'
71 | extern ktime_t ktime_get_raw(void);
| ^~~~~~~
include/linux/timekeeping.h:77:15: error: unknown type name 'ktime_t'
77 | static inline ktime_t ktime_get_real(void)
| ^~~~~~~
include/linux/timekeeping.h:82:15: error: unknown type name 'ktime_t'
82 | static inline ktime_t ktime_get_coarse_real(void)
| ^~~~~~~
include/linux/timekeeping.h:93:15: error: unknown type name 'ktime_t'
93 | static inline ktime_t ktime_get_boottime(void)
| ^~~~~~~
include/linux/timekeeping.h:98:15: error: unknown type name 'ktime_t'
98 | static inline ktime_t ktime_get_coarse_boottime(void)
| ^~~~~~~
include/linux/timekeeping.h:106:15: error: unknown type name 'ktime_t'
106 | static inline ktime_t ktime_get_clocktai(void)
| ^~~~~~~
include/linux/timekeeping.h:111:15: error: unknown type name 'ktime_t'
111 | static inline ktime_t ktime_get_coarse_clocktai(void)
| ^~~~~~~
include/linux/timekeeping.h:116:15: error: unknown type name 'ktime_t'
116 | static inline ktime_t ktime_get_coarse(void)
| ^~~~~~~
include/linux/timekeeping.h: In function 'ktime_get_coarse':
>> include/linux/timekeeping.h:121:16: error: implicit declaration of function 'timespec64_to_ktime'; did you mean 'timespec64_to_ns'? [-Werror=implicit-function-declaration]
121 | return timespec64_to_ktime(ts);
| ^~~~~~~~~~~~~~~~~~~
| timespec64_to_ns
include/linux/timekeeping.h: In function 'ktime_get_coarse_ns':
>> include/linux/timekeeping.h:126:16: error: implicit declaration of function 'ktime_to_ns' [-Werror=implicit-function-declaration]
126 | return ktime_to_ns(ktime_get_coarse());
| ^~~~~~~~~~~
include/linux/timekeeping.h: At top level:
include/linux/timekeeping.h:147:15: error: unknown type name 'ktime_t'
147 | static inline ktime_t ktime_mono_to_real(ktime_t mono)
| ^~~~~~~
include/linux/timekeeping.h:147:42: error: unknown type name 'ktime_t'; did you mean 'timer_t'?
147 | static inline ktime_t ktime_mono_to_real(ktime_t mono)
| ^~~~~~~
| timer_t
include/linux/timekeeping.h: In function 'ktime_get_boottime_ts64':
>> include/linux/timekeeping.h:190:15: error: implicit declaration of function 'ktime_to_timespec64'; did you mean 'ns_to_timespec64'? [-Werror=implicit-function-declaration]
190 | *ts = ktime_to_timespec64(ktime_get_boottime());
| ^~~~~~~~~~~~~~~~~~~
| ns_to_timespec64
>> include/linux/timekeeping.h:190:15: error: incompatible types when assigning to type 'struct timespec64' from type 'int'
include/linux/timekeeping.h: In function 'ktime_get_coarse_boottime_ts64':
include/linux/timekeeping.h:195:15: error: incompatible types when assigning to type 'struct timespec64' from type 'int'
195 | *ts = ktime_to_timespec64(ktime_get_coarse_boottime());
| ^~~~~~~~~~~~~~~~~~~
include/linux/timekeeping.h: In function 'ktime_get_boottime_seconds':
>> include/linux/timekeeping.h:200:16: error: implicit declaration of function 'ktime_divns'; did you mean 'ktime_get_ns'? [-Werror=implicit-function-declaration]
200 | return ktime_divns(ktime_get_coarse_boottime(), NSEC_PER_SEC);
| ^~~~~~~~~~~
| ktime_get_ns
include/linux/timekeeping.h: In function 'ktime_get_clocktai_ts64':
include/linux/timekeeping.h:205:15: error: incompatible types when assigning to type 'struct timespec64' from type 'int'
205 | *ts = ktime_to_timespec64(ktime_get_clocktai());
| ^~~~~~~~~~~~~~~~~~~
include/linux/timekeeping.h: In function 'ktime_get_coarse_clocktai_ts64':
include/linux/timekeeping.h:210:15: error: incompatible types when assigning to type 'struct timespec64' from type 'int'
210 | *ts = ktime_to_timespec64(ktime_get_coarse_clocktai());
| ^~~~~~~~~~~~~~~~~~~
include/linux/timekeeping.h: At top level:
include/linux/timekeeping.h:249:9: error: unknown type name 'ktime_t'
249 | ktime_t real;
| ^~~~~~~
include/linux/timekeeping.h:250:9: error: unknown type name 'ktime_t'
250 | ktime_t raw;
| ^~~~~~~
include/linux/timekeeping.h:264:9: error: unknown type name 'ktime_t'
264 | ktime_t device;
| ^~~~~~~
include/linux/timekeeping.h:265:9: error: unknown type name 'ktime_t'
265 | ktime_t sys_realtime;
| ^~~~~~~
include/linux/timekeeping.h:266:9: error: unknown type name 'ktime_t'
266 | ktime_t sys_monoraw;
| ^~~~~~~
include/linux/timekeeping.h:285:44: error: unknown type name 'ktime_t'; did you mean 'timer_t'?
285 | int (*get_time_fn)(ktime_t *device_time,
| ^~~~~~~
| timer_t
>> include/linux/timekeeping.h:288:25: error: expected ';', ',' or ')' before 'void'
288 | void *ctx,
| ^~~~
In file included from include/linux/timer.h:6,
from include/linux/workqueue.h:9,
from include/linux/mm_types.h:19,
from include/linux/buildid.h:5,
from include/linux/module.h:14,
from drivers/cxl/pci.c:6:
>> include/linux/ktime.h:71:23: error: conflicting types for 'timespec64_to_ktime'; have 'ktime_t(struct timespec64)' {aka 'long long int(struct timespec64)'}
71 | static inline ktime_t timespec64_to_ktime(struct timespec64 ts)
| ^~~~~~~~~~~~~~~~~~~
include/linux/timekeeping.h:121:16: note: previous implicit declaration of 'timespec64_to_ktime' with type 'int()'
121 | return timespec64_to_ktime(ts);
| ^~~~~~~~~~~~~~~~~~~
>> include/linux/ktime.h:80:19: error: conflicting types for 'ktime_to_ns'; have 's64(const ktime_t)' {aka 'long long int(const long long int)'}
80 | static inline s64 ktime_to_ns(const ktime_t kt)
| ^~~~~~~~~~~
include/linux/timekeeping.h:126:16: note: previous implicit declaration of 'ktime_to_ns' with type 'int()'
126 | return ktime_to_ns(ktime_get_coarse());
| ^~~~~~~~~~~
>> include/linux/ktime.h:148:19: error: conflicting types for 'ktime_divns'; have 's64(const ktime_t, s64)' {aka 'long long int(const long long int, long long int)'}
148 | static inline s64 ktime_divns(const ktime_t kt, s64 div)
| ^~~~~~~~~~~
include/linux/timekeeping.h:200:16: note: previous implicit declaration of 'ktime_divns' with type 'int()'
200 | return ktime_divns(ktime_get_coarse_boottime(), NSEC_PER_SEC);
| ^~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/ktime_t +67 include/linux/timekeeping.h
0077dc60f274b9 Thomas Gleixner 2014-07-16 66
8b094cd03b4a37 Thomas Gleixner 2014-07-16 @67 extern ktime_t ktime_get(void);
0077dc60f274b9 Thomas Gleixner 2014-07-16 68 extern ktime_t ktime_get_with_offset(enum tk_offsets offs);
b9ff604cff1135 Arnd Bergmann 2018-04-27 69 extern ktime_t ktime_get_coarse_with_offset(enum tk_offsets offs);
9a6b51976ea3a3 Thomas Gleixner 2014-07-16 70 extern ktime_t ktime_mono_to_any(ktime_t tmono, enum tk_offsets offs);
f519b1a2e08c91 Thomas Gleixner 2014-07-16 71 extern ktime_t ktime_get_raw(void);
6374f9124efea5 Harald Geyer 2015-04-07 72 extern u32 ktime_get_resolution_ns(void);
8b094cd03b4a37 Thomas Gleixner 2014-07-16 73
f5264d5d5a0729 Thomas Gleixner 2014-07-16 74 /**
f5264d5d5a0729 Thomas Gleixner 2014-07-16 75 * ktime_get_real - get the real (wall-) time in ktime_t format
f5264d5d5a0729 Thomas Gleixner 2014-07-16 76 */
f5264d5d5a0729 Thomas Gleixner 2014-07-16 77 static inline ktime_t ktime_get_real(void)
f5264d5d5a0729 Thomas Gleixner 2014-07-16 78 {
f5264d5d5a0729 Thomas Gleixner 2014-07-16 79 return ktime_get_with_offset(TK_OFFS_REAL);
f5264d5d5a0729 Thomas Gleixner 2014-07-16 80 }
f5264d5d5a0729 Thomas Gleixner 2014-07-16 81
b9ff604cff1135 Arnd Bergmann 2018-04-27 82 static inline ktime_t ktime_get_coarse_real(void)
b9ff604cff1135 Arnd Bergmann 2018-04-27 83 {
b9ff604cff1135 Arnd Bergmann 2018-04-27 84 return ktime_get_coarse_with_offset(TK_OFFS_REAL);
b9ff604cff1135 Arnd Bergmann 2018-04-27 85 }
b9ff604cff1135 Arnd Bergmann 2018-04-27 86
a3ed0e4393d688 Thomas Gleixner 2018-04-25 87 /**
a3ed0e4393d688 Thomas Gleixner 2018-04-25 88 * ktime_get_boottime - Returns monotonic time since boot in ktime_t format
a3ed0e4393d688 Thomas Gleixner 2018-04-25 89 *
a3ed0e4393d688 Thomas Gleixner 2018-04-25 90 * This is similar to CLOCK_MONTONIC/ktime_get, but also includes the
a3ed0e4393d688 Thomas Gleixner 2018-04-25 91 * time spent in suspend.
a3ed0e4393d688 Thomas Gleixner 2018-04-25 92 */
a3ed0e4393d688 Thomas Gleixner 2018-04-25 93 static inline ktime_t ktime_get_boottime(void)
a3ed0e4393d688 Thomas Gleixner 2018-04-25 94 {
a3ed0e4393d688 Thomas Gleixner 2018-04-25 95 return ktime_get_with_offset(TK_OFFS_BOOT);
a3ed0e4393d688 Thomas Gleixner 2018-04-25 96 }
a3ed0e4393d688 Thomas Gleixner 2018-04-25 97
b9ff604cff1135 Arnd Bergmann 2018-04-27 98 static inline ktime_t ktime_get_coarse_boottime(void)
b9ff604cff1135 Arnd Bergmann 2018-04-27 99 {
b9ff604cff1135 Arnd Bergmann 2018-04-27 100 return ktime_get_coarse_with_offset(TK_OFFS_BOOT);
b9ff604cff1135 Arnd Bergmann 2018-04-27 101 }
b9ff604cff1135 Arnd Bergmann 2018-04-27 102
afab07c0e91ecf Thomas Gleixner 2014-07-16 103 /**
afab07c0e91ecf Thomas Gleixner 2014-07-16 104 * ktime_get_clocktai - Returns the TAI time of day in ktime_t format
afab07c0e91ecf Thomas Gleixner 2014-07-16 105 */
afab07c0e91ecf Thomas Gleixner 2014-07-16 106 static inline ktime_t ktime_get_clocktai(void)
afab07c0e91ecf Thomas Gleixner 2014-07-16 107 {
afab07c0e91ecf Thomas Gleixner 2014-07-16 108 return ktime_get_with_offset(TK_OFFS_TAI);
afab07c0e91ecf Thomas Gleixner 2014-07-16 109 }
afab07c0e91ecf Thomas Gleixner 2014-07-16 110
b9ff604cff1135 Arnd Bergmann 2018-04-27 111 static inline ktime_t ktime_get_coarse_clocktai(void)
b9ff604cff1135 Arnd Bergmann 2018-04-27 112 {
b9ff604cff1135 Arnd Bergmann 2018-04-27 113 return ktime_get_coarse_with_offset(TK_OFFS_TAI);
b9ff604cff1135 Arnd Bergmann 2018-04-27 114 }
b9ff604cff1135 Arnd Bergmann 2018-04-27 115
4c54294d01e605 Jason A. Donenfeld 2019-06-21 116 static inline ktime_t ktime_get_coarse(void)
4c54294d01e605 Jason A. Donenfeld 2019-06-21 117 {
4c54294d01e605 Jason A. Donenfeld 2019-06-21 118 struct timespec64 ts;
4c54294d01e605 Jason A. Donenfeld 2019-06-21 119
4c54294d01e605 Jason A. Donenfeld 2019-06-21 120 ktime_get_coarse_ts64(&ts);
4c54294d01e605 Jason A. Donenfeld 2019-06-21 @121 return timespec64_to_ktime(ts);
4c54294d01e605 Jason A. Donenfeld 2019-06-21 122 }
4c54294d01e605 Jason A. Donenfeld 2019-06-21 123
4c54294d01e605 Jason A. Donenfeld 2019-06-21 124 static inline u64 ktime_get_coarse_ns(void)
4c54294d01e605 Jason A. Donenfeld 2019-06-21 125 {
4c54294d01e605 Jason A. Donenfeld 2019-06-21 @126 return ktime_to_ns(ktime_get_coarse());
4c54294d01e605 Jason A. Donenfeld 2019-06-21 127 }
4c54294d01e605 Jason A. Donenfeld 2019-06-21 128
4c54294d01e605 Jason A. Donenfeld 2019-06-21 129 static inline u64 ktime_get_coarse_real_ns(void)
4c54294d01e605 Jason A. Donenfeld 2019-06-21 130 {
4c54294d01e605 Jason A. Donenfeld 2019-06-21 131 return ktime_to_ns(ktime_get_coarse_real());
4c54294d01e605 Jason A. Donenfeld 2019-06-21 132 }
4c54294d01e605 Jason A. Donenfeld 2019-06-21 133
d48e0cd8fcaf31 Jason A. Donenfeld 2019-06-24 134 static inline u64 ktime_get_coarse_boottime_ns(void)
4c54294d01e605 Jason A. Donenfeld 2019-06-21 135 {
4c54294d01e605 Jason A. Donenfeld 2019-06-21 136 return ktime_to_ns(ktime_get_coarse_boottime());
4c54294d01e605 Jason A. Donenfeld 2019-06-21 137 }
4c54294d01e605 Jason A. Donenfeld 2019-06-21 138
4c54294d01e605 Jason A. Donenfeld 2019-06-21 139 static inline u64 ktime_get_coarse_clocktai_ns(void)
4c54294d01e605 Jason A. Donenfeld 2019-06-21 140 {
4c54294d01e605 Jason A. Donenfeld 2019-06-21 141 return ktime_to_ns(ktime_get_coarse_clocktai());
4c54294d01e605 Jason A. Donenfeld 2019-06-21 142 }
4c54294d01e605 Jason A. Donenfeld 2019-06-21 143
9a6b51976ea3a3 Thomas Gleixner 2014-07-16 144 /**
9a6b51976ea3a3 Thomas Gleixner 2014-07-16 145 * ktime_mono_to_real - Convert monotonic time to clock realtime
9a6b51976ea3a3 Thomas Gleixner 2014-07-16 146 */
9a6b51976ea3a3 Thomas Gleixner 2014-07-16 147 static inline ktime_t ktime_mono_to_real(ktime_t mono)
9a6b51976ea3a3 Thomas Gleixner 2014-07-16 148 {
9a6b51976ea3a3 Thomas Gleixner 2014-07-16 149 return ktime_mono_to_any(mono, TK_OFFS_REAL);
9a6b51976ea3a3 Thomas Gleixner 2014-07-16 150 }
9a6b51976ea3a3 Thomas Gleixner 2014-07-16 151
897994e32b2b0a Thomas Gleixner 2014-07-16 152 static inline u64 ktime_get_ns(void)
897994e32b2b0a Thomas Gleixner 2014-07-16 153 {
897994e32b2b0a Thomas Gleixner 2014-07-16 154 return ktime_to_ns(ktime_get());
897994e32b2b0a Thomas Gleixner 2014-07-16 155 }
897994e32b2b0a Thomas Gleixner 2014-07-16 156
897994e32b2b0a Thomas Gleixner 2014-07-16 157 static inline u64 ktime_get_real_ns(void)
897994e32b2b0a Thomas Gleixner 2014-07-16 158 {
897994e32b2b0a Thomas Gleixner 2014-07-16 159 return ktime_to_ns(ktime_get_real());
897994e32b2b0a Thomas Gleixner 2014-07-16 160 }
897994e32b2b0a Thomas Gleixner 2014-07-16 161
9285ec4c8b61d4 Jason A. Donenfeld 2019-06-21 162 static inline u64 ktime_get_boottime_ns(void)
a3ed0e4393d688 Thomas Gleixner 2018-04-25 163 {
a3ed0e4393d688 Thomas Gleixner 2018-04-25 164 return ktime_to_ns(ktime_get_boottime());
a3ed0e4393d688 Thomas Gleixner 2018-04-25 165 }
a3ed0e4393d688 Thomas Gleixner 2018-04-25 166
9285ec4c8b61d4 Jason A. Donenfeld 2019-06-21 167 static inline u64 ktime_get_clocktai_ns(void)
fe5fba05b46c79 Peter Zijlstra 2015-03-17 168 {
fe5fba05b46c79 Peter Zijlstra 2015-03-17 169 return ktime_to_ns(ktime_get_clocktai());
fe5fba05b46c79 Peter Zijlstra 2015-03-17 170 }
fe5fba05b46c79 Peter Zijlstra 2015-03-17 171
f519b1a2e08c91 Thomas Gleixner 2014-07-16 172 static inline u64 ktime_get_raw_ns(void)
f519b1a2e08c91 Thomas Gleixner 2014-07-16 173 {
f519b1a2e08c91 Thomas Gleixner 2014-07-16 174 return ktime_to_ns(ktime_get_raw());
f519b1a2e08c91 Thomas Gleixner 2014-07-16 175 }
f519b1a2e08c91 Thomas Gleixner 2014-07-16 176
4396e058c52e16 Thomas Gleixner 2014-07-16 177 extern u64 ktime_get_mono_fast_ns(void);
f09cb9a1808e35 Peter Zijlstra 2015-03-19 178 extern u64 ktime_get_raw_fast_ns(void);
a3ed0e4393d688 Thomas Gleixner 2018-04-25 179 extern u64 ktime_get_boot_fast_ns(void);
3dc6ffae2da201 Kurt Kanzenbach 2022-04-14 180 extern u64 ktime_get_tai_fast_ns(void);
4c3711d7fb4763 Thomas Gleixner 2017-08-31 181 extern u64 ktime_get_real_fast_ns(void);
4396e058c52e16 Thomas Gleixner 2014-07-16 182
d6c7270e913db7 Thomas Gleixner 2018-03-01 183 /*
06aa376903b6e8 Arnd Bergmann 2018-04-27 184 * timespec64/time64_t interfaces utilizing the ktime based ones
06aa376903b6e8 Arnd Bergmann 2018-04-27 185 * for API completeness, these could be implemented more efficiently
06aa376903b6e8 Arnd Bergmann 2018-04-27 186 * if needed.
d6c7270e913db7 Thomas Gleixner 2018-03-01 187 */
fb7fcc96a86cfa Arnd Bergmann 2018-04-27 188 static inline void ktime_get_boottime_ts64(struct timespec64 *ts)
a3ed0e4393d688 Thomas Gleixner 2018-04-25 189 {
a3ed0e4393d688 Thomas Gleixner 2018-04-25 @190 *ts = ktime_to_timespec64(ktime_get_boottime());
a3ed0e4393d688 Thomas Gleixner 2018-04-25 191 }
a3ed0e4393d688 Thomas Gleixner 2018-04-25 192
06aa376903b6e8 Arnd Bergmann 2018-04-27 193 static inline void ktime_get_coarse_boottime_ts64(struct timespec64 *ts)
06aa376903b6e8 Arnd Bergmann 2018-04-27 194 {
06aa376903b6e8 Arnd Bergmann 2018-04-27 195 *ts = ktime_to_timespec64(ktime_get_coarse_boottime());
06aa376903b6e8 Arnd Bergmann 2018-04-27 196 }
06aa376903b6e8 Arnd Bergmann 2018-04-27 197
06aa376903b6e8 Arnd Bergmann 2018-04-27 198 static inline time64_t ktime_get_boottime_seconds(void)
06aa376903b6e8 Arnd Bergmann 2018-04-27 199 {
06aa376903b6e8 Arnd Bergmann 2018-04-27 @200 return ktime_divns(ktime_get_coarse_boottime(), NSEC_PER_SEC);
06aa376903b6e8 Arnd Bergmann 2018-04-27 201 }
06aa376903b6e8 Arnd Bergmann 2018-04-27 202
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH] cxl/pci: Set the device timestamp
2023-01-28 11:32 ` kernel test robot
@ 2023-01-30 15:10 ` Jonathan Cameron
0 siblings, 0 replies; 17+ messages in thread
From: Jonathan Cameron @ 2023-01-30 15:10 UTC (permalink / raw)
To: kernel test robot
Cc: linux-cxl, oe-kbuild-all, Alison Schofield, Vishal Verma,
Ira Weiny, Dan Williams, linuxarm
On Sat, 28 Jan 2023 19:32:20 +0800
kernel test robot <lkp@intel.com> wrote:
> Hi Jonathan,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on 172738bbccdb4ef76bdd72fc72a315c741c39161]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Jonathan-Cameron/cxl-pci-Set-the-device-timestamp/20230128-152300
> base: 172738bbccdb4ef76bdd72fc72a315c741c39161
> patch link: https://lore.kernel.org/r/20230126180458.5145-1-Jonathan.Cameron%40huawei.com
> patch subject: [PATCH] cxl/pci: Set the device timestamp
> config: ia64-allyesconfig (https://download.01.org/0day-ci/archive/20230128/202301281915.t1bQicuY-lkp@intel.com/config)
> compiler: ia64-linux-gcc (GCC) 12.1.0
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # https://github.com/intel-lab-lkp/linux/commit/f36cca050cf662048001b7b7686040270e4a8c00
> git remote add linux-review https://github.com/intel-lab-lkp/linux
> git fetch --no-tags linux-review Jonathan-Cameron/cxl-pci-Set-the-device-timestamp/20230128-152300
> git checkout f36cca050cf662048001b7b7686040270e4a8c00
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 olddefconfig
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@intel.com>
>
> All errors (new ones prefixed by >>):
>
> In file included from drivers/cxl/pci.c:5:
> >> include/linux/timekeeping.h:67:8: error: unknown type name 'ktime_t'
> 67 | extern ktime_t ktime_get(void);
I should be including ktime.h not timekeeping.h here. Will fix for v2.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] cxl/pci: Set the device timestamp
2023-01-26 18:04 [PATCH] cxl/pci: Set the device timestamp Jonathan Cameron
` (4 preceding siblings ...)
2023-01-28 11:32 ` kernel test robot
@ 2023-01-28 16:01 ` kernel test robot
5 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2023-01-28 16:01 UTC (permalink / raw)
To: Jonathan Cameron, linux-cxl
Cc: oe-kbuild-all, Alison Schofield, Vishal Verma, Ira Weiny,
Dan Williams, linuxarm
Hi Jonathan,
I love your patch! Perhaps something to improve:
[auto build test WARNING on 172738bbccdb4ef76bdd72fc72a315c741c39161]
url: https://github.com/intel-lab-lkp/linux/commits/Jonathan-Cameron/cxl-pci-Set-the-device-timestamp/20230128-152300
base: 172738bbccdb4ef76bdd72fc72a315c741c39161
patch link: https://lore.kernel.org/r/20230126180458.5145-1-Jonathan.Cameron%40huawei.com
patch subject: [PATCH] cxl/pci: Set the device timestamp
config: powerpc-randconfig-s032-20230123 (https://download.01.org/0day-ci/archive/20230128/202301282342.2ZTowY3a-lkp@intel.com/config)
compiler: powerpc-linux-gcc (GCC) 12.1.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-39-gce1a6720-dirty
# https://github.com/intel-lab-lkp/linux/commit/f36cca050cf662048001b7b7686040270e4a8c00
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Jonathan-Cameron/cxl-pci-Set-the-device-timestamp/20230128-152300
git checkout f36cca050cf662048001b7b7686040270e4a8c00
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=powerpc olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=powerpc SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
drivers/cxl/core/mbox.c:834:18: sparse: sparse: cast to non-scalar
drivers/cxl/core/mbox.c:834:18: sparse: sparse: cast from non-scalar
>> drivers/cxl/core/mbox.c:874:22: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __le64 [usertype] timestamp @@ got unsigned long long [usertype] ts @@
drivers/cxl/core/mbox.c:874:22: sparse: expected restricted __le64 [usertype] timestamp
drivers/cxl/core/mbox.c:874:22: sparse: got unsigned long long [usertype] ts
vim +874 drivers/cxl/core/mbox.c
861
862 int cxl_set_timestamp(struct cxl_dev_state *cxlds, u64 ts)
863 {
864 struct cxl_mbox_cmd mbox_cmd;
865 struct cxl_mbox_set_timestamp_in pi;
866
867 /*
868 * Command is optional and functionality should not be affected if
869 * the command is not available.
870 */
871 if (!test_bit(CXL_MEM_COMMAND_ID_SET_TIMESTAMP, cxlds->enabled_cmds))
872 return 0;
873
> 874 pi.timestamp = ts;
875 mbox_cmd = (struct cxl_mbox_cmd) {
876 .opcode = CXL_MBOX_OP_SET_TIMESTAMP,
877 .size_in = sizeof(pi),
878 .payload_in = &pi,
879 };
880
881 return cxl_internal_send_cmd(cxlds, &mbox_cmd);
882 }
883 EXPORT_SYMBOL_NS_GPL(cxl_set_timestamp, CXL);
884
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 17+ messages in thread