Re: [PATCH v7 3/7] hw/cxl/events: Wire up get/clear event mailbox commands

[Jonathan Cameron]

On Mon, 22 May 2023 12:50:34 -0700
Fan Ni <nifan@outlook.com> wrote:

> The 05/22/2023 16:09, Jonathan Cameron wrote:
> > From: Ira Weiny <ira.weiny@intel.com>
> > 
> > CXL testing is benefited from an artificial event log injection
> > mechanism.
> > 
> > Add an event log infrastructure to insert, get, and clear events from
> > the various logs available on a device.
> > 
> > Replace the stubbed out CXL Get/Clear Event mailbox commands with
> > commands that operate on the new infrastructure.
> > 
> > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---  
> 
> Reviewed-by: Fan Ni <fan.ni@samsung.com>
> 
> See comments below in cxl_event_insert.

...

> > +    memcpy(&entry->data, event, sizeof(*event));
> > +
> > +    entry->data.hdr.handle = cpu_to_le16(log->next_handle);
> > +    log->next_handle++;
> > +    /* 0 handle is never valid */
> > +    if (log->next_handle == 0) {  
> 
> next_handle is uint16_t, how can it be 0 after next_handle++?

Intent was to deal with wrap around.
We are fine with reusing handles after sufficiently long time that
there can't be a confusion, just not 0 as that has special
meaning when used to 'point' to a related event
(see definition of Related Event Record Handle)

> 
> > +        log->next_handle++;
> > +    }
> > +    entry->data.hdr.timestamp = cpu_to_le64(time);
> > +
> > +    QSIMPLEQ_INSERT_TAIL(&log->events, entry, node);
> > +    cxl_event_set_status(cxlds, log_type, true);
> > +
> > +    /* Count went from 0 to 1 */
> > +    return cxl_event_count(log) == 1;
> > +}