| Message ID | 20230125-ira-cxl-events-fixups-2023-01-11-v1-1-1931378515f5@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/cxl: CXL Event processing fixups | expand |
On Wed, 25 Jan 2023 21:37:27 -0800 Ira Weiny <ira.weiny@intel.com> wrote: > CXL 3.0 8.2.9.4.2 Set Timestamp and 8.2.9.4.1 Get Timestamp define the > way for software to set and get the time stamp of a device. Events > should use a time stamp consistent with the Get Timestamp mailbox > command. > > In addition avoid setting the time stamp twice. > > Fixes: fb64c5661d5f ("hw/cxl/events: Wire up get/clear event mailbox commands") > Reported-by: Jonathan Cameron <Jonathan.Cameron@Huawei.com> > Signed-off-by: Ira Weiny <ira.weiny@intel.com> Hi Ira, I'm going to split this patch as I am carrying a very similar utility function for an updated version of the poison list code and I'm not sure what order everything will go upstream in. So I'll split this into: 1) Patch that adds cxl_device_get_timestamp() - adding the use of this in the GET_TIMESTAMP mailbox command. 2) Changes pushed down into the patch you mention above. Given all the code is yours, just split up, I'll keep the SOB. Shout if you mind me doing that. Thanks, Jonathan > --- > hw/cxl/cxl-device-utils.c | 15 +++++++++++++++ > hw/cxl/cxl-events.c | 4 +++- > hw/cxl/cxl-mailbox-utils.c | 11 +---------- > hw/mem/cxl_type3.c | 1 - > include/hw/cxl/cxl_device.h | 2 ++ > 5 files changed, 21 insertions(+), 12 deletions(-) > > diff --git a/hw/cxl/cxl-device-utils.c b/hw/cxl/cxl-device-utils.c > index 7f29d40be04a..5876a3703e85 100644 > --- a/hw/cxl/cxl-device-utils.c > +++ b/hw/cxl/cxl-device-utils.c > @@ -325,3 +325,18 @@ void cxl_device_register_init_swcci(CXLDeviceState *cxl_dstate) > > cxl_initialize_mailbox(cxl_dstate, true); > } > + > +uint64_t cxl_device_get_timestamp(CXLDeviceState *cxl_dstate) > +{ > + uint64_t time, delta; > + uint64_t final_time = 0; > + > + if (cxl_dstate->timestamp.set) { > + /* First find the delta from the last time the host set the time. */ > + time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > + delta = time - cxl_dstate->timestamp.last_set; > + final_time = cxl_dstate->timestamp.host_set + delta; > + } > + > + return final_time; > +} > diff --git a/hw/cxl/cxl-events.c b/hw/cxl/cxl-events.c > index 08fd52b66188..2536aafc55fb 100644 > --- a/hw/cxl/cxl-events.c > +++ b/hw/cxl/cxl-events.c > @@ -100,7 +100,7 @@ bool cxl_event_insert(CXLDeviceState *cxlds, > enum cxl_event_log_type log_type, > struct cxl_event_record_raw *event) > { > - uint64_t time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > + uint64_t time; > struct cxl_event_log *log; > CXLEvent *entry; > > @@ -108,6 +108,8 @@ bool cxl_event_insert(CXLDeviceState *cxlds, > return false; > } > > + time = cxl_device_get_timestamp(cxlds); > + > log = &cxlds->event_logs[log_type]; > > QEMU_LOCK_GUARD(&log->lock); > diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c > index 75703023434b..0e64873c2395 100644 > --- a/hw/cxl/cxl-mailbox-utils.c > +++ b/hw/cxl/cxl-mailbox-utils.c > @@ -394,17 +394,8 @@ static CXLRetCode cmd_timestamp_get(struct cxl_cmd *cmd, > CXLDeviceState *cxl_dstate, > uint16_t *len) > { > - uint64_t time, delta; > - uint64_t final_time = 0; > - > - if (cxl_dstate->timestamp.set) { > - /* First find the delta from the last time the host set the time. */ > - time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > - delta = time - cxl_dstate->timestamp.last_set; > - final_time = cxl_dstate->timestamp.host_set + delta; > - } > + uint64_t final_time = cxl_device_get_timestamp(cxl_dstate); > > - /* Then adjust the actual time */ > stq_le_p(cmd->payload, final_time); > *len = 8; > > diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c > index a7b587780af2..42e291dd9f76 100644 > --- a/hw/mem/cxl_type3.c > +++ b/hw/mem/cxl_type3.c > @@ -1291,7 +1291,6 @@ static void cxl_assign_event_header(struct cxl_event_record_hdr *hdr, > hdr->flags[0] = flags; > hdr->length = length; > memcpy(&hdr->id, uuid, sizeof(hdr->id)); > - hdr->timestamp = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > } > > static const QemuUUID gen_media_uuid = { > diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h > index cbb37c541c44..31579af342f1 100644 > --- a/include/hw/cxl/cxl_device.h > +++ b/include/hw/cxl/cxl_device.h > @@ -426,4 +426,6 @@ CXLRetCode cxl_event_clear_records(CXLDeviceState *cxlds, > > void cxl_event_irq_assert(CXLType3Dev *ct3d); > > +uint64_t cxl_device_get_timestamp(CXLDeviceState *cxlds); > + > #endif >
Jonathan Cameron wrote: > On Wed, 25 Jan 2023 21:37:27 -0800 > Ira Weiny <ira.weiny@intel.com> wrote: > > > CXL 3.0 8.2.9.4.2 Set Timestamp and 8.2.9.4.1 Get Timestamp define the > > way for software to set and get the time stamp of a device. Events > > should use a time stamp consistent with the Get Timestamp mailbox > > command. > > > > In addition avoid setting the time stamp twice. > > > > Fixes: fb64c5661d5f ("hw/cxl/events: Wire up get/clear event mailbox commands") > > Reported-by: Jonathan Cameron <Jonathan.Cameron@Huawei.com> > > Signed-off-by: Ira Weiny <ira.weiny@intel.com> > Hi Ira, > > I'm going to split this patch as I am carrying a very similar > utility function for an updated version of the poison list code > and I'm not sure what order everything will go upstream in. > > So I'll split this into: > 1) Patch that adds cxl_device_get_timestamp() - adding the > use of this in the GET_TIMESTAMP mailbox command. > > 2) Changes pushed down into the patch you mention above. > > Given all the code is yours, just split up, I'll keep the SOB. > Shout if you mind me doing that. That sounds great. I'm just sorry I did not get to all this sooner. Thanks! Ira
diff --git a/hw/cxl/cxl-device-utils.c b/hw/cxl/cxl-device-utils.c index 7f29d40be04a..5876a3703e85 100644 --- a/hw/cxl/cxl-device-utils.c +++ b/hw/cxl/cxl-device-utils.c @@ -325,3 +325,18 @@ void cxl_device_register_init_swcci(CXLDeviceState *cxl_dstate) cxl_initialize_mailbox(cxl_dstate, true); } + +uint64_t cxl_device_get_timestamp(CXLDeviceState *cxl_dstate) +{ + uint64_t time, delta; + uint64_t final_time = 0; + + if (cxl_dstate->timestamp.set) { + /* First find the delta from the last time the host set the time. */ + time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); + delta = time - cxl_dstate->timestamp.last_set; + final_time = cxl_dstate->timestamp.host_set + delta; + } + + return final_time; +} diff --git a/hw/cxl/cxl-events.c b/hw/cxl/cxl-events.c index 08fd52b66188..2536aafc55fb 100644 --- a/hw/cxl/cxl-events.c +++ b/hw/cxl/cxl-events.c @@ -100,7 +100,7 @@ bool cxl_event_insert(CXLDeviceState *cxlds, enum cxl_event_log_type log_type, struct cxl_event_record_raw *event) { - uint64_t time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); + uint64_t time; struct cxl_event_log *log; CXLEvent *entry; @@ -108,6 +108,8 @@ bool cxl_event_insert(CXLDeviceState *cxlds, return false; } + time = cxl_device_get_timestamp(cxlds); + log = &cxlds->event_logs[log_type]; QEMU_LOCK_GUARD(&log->lock); diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c index 75703023434b..0e64873c2395 100644 --- a/hw/cxl/cxl-mailbox-utils.c +++ b/hw/cxl/cxl-mailbox-utils.c @@ -394,17 +394,8 @@ static CXLRetCode cmd_timestamp_get(struct cxl_cmd *cmd, CXLDeviceState *cxl_dstate, uint16_t *len) { - uint64_t time, delta; - uint64_t final_time = 0; - - if (cxl_dstate->timestamp.set) { - /* First find the delta from the last time the host set the time. */ - time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); - delta = time - cxl_dstate->timestamp.last_set; - final_time = cxl_dstate->timestamp.host_set + delta; - } + uint64_t final_time = cxl_device_get_timestamp(cxl_dstate); - /* Then adjust the actual time */ stq_le_p(cmd->payload, final_time); *len = 8; diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c index a7b587780af2..42e291dd9f76 100644 --- a/hw/mem/cxl_type3.c +++ b/hw/mem/cxl_type3.c @@ -1291,7 +1291,6 @@ static void cxl_assign_event_header(struct cxl_event_record_hdr *hdr, hdr->flags[0] = flags; hdr->length = length; memcpy(&hdr->id, uuid, sizeof(hdr->id)); - hdr->timestamp = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); } static const QemuUUID gen_media_uuid = { diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h index cbb37c541c44..31579af342f1 100644 --- a/include/hw/cxl/cxl_device.h +++ b/include/hw/cxl/cxl_device.h @@ -426,4 +426,6 @@ CXLRetCode cxl_event_clear_records(CXLDeviceState *cxlds, void cxl_event_irq_assert(CXLType3Dev *ct3d); +uint64_t cxl_device_get_timestamp(CXLDeviceState *cxlds); + #endif
CXL 3.0 8.2.9.4.2 Set Timestamp and 8.2.9.4.1 Get Timestamp define the way for software to set and get the time stamp of a device. Events should use a time stamp consistent with the Get Timestamp mailbox command. In addition avoid setting the time stamp twice. Fixes: fb64c5661d5f ("hw/cxl/events: Wire up get/clear event mailbox commands") Reported-by: Jonathan Cameron <Jonathan.Cameron@Huawei.com> Signed-off-by: Ira Weiny <ira.weiny@intel.com> --- hw/cxl/cxl-device-utils.c | 15 +++++++++++++++ hw/cxl/cxl-events.c | 4 +++- hw/cxl/cxl-mailbox-utils.c | 11 +---------- hw/mem/cxl_type3.c | 1 - include/hw/cxl/cxl_device.h | 2 ++ 5 files changed, 21 insertions(+), 12 deletions(-)