On Thu, 27 Jan 2022 11:50:08 +0000
Alex Bennée <alex.ben...@linaro.org> wrote:
> Jonathan Cameron <jonathan.came...@huawei.com> writes:
>
> > From: Ben Widawsky <ben.widaw...@intel.com>
> >
> > Per spec, timestamp appears to be a free-running counter from a value
> > set by the host via the Set Timestamp command (0301h). There are
> > references to the epoch, which seem like a red herring. Therefore, the
> > implementation implements the timestamp as freerunning counter from the
> > last value that was issued by the Set Timestamp command.
> >
> > Signed-off-by: Ben Widawsky <ben.widaw...@intel.com>
> > Signed-off-by: Jonathan Cameron <jonathan.came...@huawei.com>
> > ---
> > hw/cxl/cxl-mailbox-utils.c | 53 +++++++++++++++++++++++++++++++++++++
> > include/hw/cxl/cxl_device.h | 6 +++++
> > 2 files changed, 59 insertions(+)
> >
> > diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> > index 1a87846356..cea4b2a59c 100644
> > --- a/hw/cxl/cxl-mailbox-utils.c
> > +++ b/hw/cxl/cxl-mailbox-utils.c
> > @@ -43,6 +43,9 @@ enum {
> > #define CLEAR_RECORDS 0x1
> > #define GET_INTERRUPT_POLICY 0x2
> > #define SET_INTERRUPT_POLICY 0x3
> > + TIMESTAMP = 0x03,
> > + #define GET 0x0
> > + #define SET 0x1
> > };
> >
> > /* 8.2.8.4.5.1 Command Return Codes */
> > @@ -117,8 +120,11 @@ define_mailbox_handler_zeroed(EVENTS_GET_RECORDS,
> > 0x20);
> > define_mailbox_handler_nop(EVENTS_CLEAR_RECORDS);
> > define_mailbox_handler_zeroed(EVENTS_GET_INTERRUPT_POLICY, 4);
> > define_mailbox_handler_nop(EVENTS_SET_INTERRUPT_POLICY);
> > +declare_mailbox_handler(TIMESTAMP_GET);
> > +declare_mailbox_handler(TIMESTAMP_SET);
> >
> > #define IMMEDIATE_CONFIG_CHANGE (1 << 1)
> > +#define IMMEDIATE_POLICY_CHANGE (1 << 3)
> > #define IMMEDIATE_LOG_CHANGE (1 << 4)
> >
> > #define CXL_CMD(s, c, in, cel_effect) \
> > @@ -129,10 +135,57 @@ static struct cxl_cmd cxl_cmd_set[256][256] = {
> > CXL_CMD(EVENTS, CLEAR_RECORDS, ~0, IMMEDIATE_LOG_CHANGE),
> > CXL_CMD(EVENTS, GET_INTERRUPT_POLICY, 0, 0),
> > CXL_CMD(EVENTS, SET_INTERRUPT_POLICY, 4, IMMEDIATE_CONFIG_CHANGE),
> > + CXL_CMD(TIMESTAMP, GET, 0, 0),
> > + CXL_CMD(TIMESTAMP, SET, 8, IMMEDIATE_POLICY_CHANGE),
> > };
> >
> > #undef CXL_CMD
> >
> > +/*
> > + * 8.2.9.3.1
> > + */
> > +define_mailbox_handler(TIMESTAMP_GET)
> > +{
> > + struct timespec ts;
> > + uint64_t delta;
> > +
> > + if (!cxl_dstate->timestamp.set) {
> > + *(uint64_t *)cmd->payload = 0;
> > + goto done;
> > + }
> > +
> > + /* First find the delta from the last time the host set the time. */
> > + clock_gettime(CLOCK_REALTIME, &ts);
>
> Could you consider using qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)?
> Otherwise by introducing a dependency on real time you'll loose the
> ability to get deterministic execution via icount.
I don't have a strong opinion either way, so Ill go with
qemu_clock_get_ns() and see if any comments on it in v5.
I've also updated the commit message as the catch all errata
which was F4 included a clarification that the timestamp
was as implemented here.
Thanks,
Jonathan
