On Wed, Sep 04, 2024 at 11:37:33AM -0500, Ira Weiny wrote:
> Jonathan Cameron wrote:
> > On Tue, 27 Aug 2024 09:40:05 -0700
> > nifan....@gmail.com wrote:
> >
> > > From: Fan Ni <fan...@samsung.com>
> > >
> > > When inserting multiple dynamic capacity event records grouped via More
> > > flag,
> > > we should only trigger interrupt after the last record is inserted into
> > > the
> > > event log. Achieving the goal by letting cxl_event_insert return true only
> > > for the insertion of the last dynamic capacity event record in the
> > > sequence.
> >
> > I'm not sure this one is accurate. We might well have a slow
> > system provisioning capacity one extent at time (and interrupting).
> >
> > The event buffer might also not be large enough to hold all records so
> > the device might 'wait' before figuring out the next extent for there
> > to be somewhere to put the record.
> >
> > Overall I think we can interrupt on each one and it should 'work'
> > as should interrupt only once there are lots of them or
> > every (n).
>
> Indeed I think it should work. But you won't see any extents as they will
> be pending in the memdev.
>
> Did this fail in some way? I'm sorry I did not try and use qemu to test
> the more bit. Rather I used cxl_test for that.
>
> Ira
It works with or without this fix in my test. Until the last extent is
notified to the OS, the extents will be pending as you mentioned.
Fan
>
> >
> > Interrupt only fires on a 0 to >= 1 transition anyway, not
> > on repeats after that unless the log has been cleared.
> > It's up to OS to keep clearing records until it at least
> > momentarily hits 0 if it wants to get any more interrupts.
> >
> > Jonathan
> >
> >
> > >
> > > Signed-off-by: Fan Ni <fan...@samsung.com>
> > > ---
> > > hw/cxl/cxl-events.c | 8 ++++++++
> > > include/hw/cxl/cxl_events.h | 1 +
> > > 2 files changed, 9 insertions(+)
> > >
> > > diff --git a/hw/cxl/cxl-events.c b/hw/cxl/cxl-events.c
> > > index 12dee2e467..90536c0e68 100644
> > > --- a/hw/cxl/cxl-events.c
> > > +++ b/hw/cxl/cxl-events.c
> > > @@ -135,6 +135,14 @@ bool cxl_event_insert(CXLDeviceState *cxlds,
> > > CXLEventLogType log_type,
> > > QSIMPLEQ_INSERT_TAIL(&log->events, entry, node);
> > > cxl_event_set_status(cxlds, log_type, true);
> > >
> > > + /*
> > > + * For dynamic capacity event records grouped via More flag,
> > > + * Only raise interrupt after inserting the last record in the log.
> > > + */
> > > + if (log_type == CXL_EVENT_TYPE_DYNAMIC_CAP) {
> > > + CXLEventDynamicCapacity *dCap = (CXLEventDynamicCapacity *)event;
> > > + return (dCap->flags & MORE_FLAG) ? false : true;
> > > + }
> > > /* Count went from 0 to 1 */
> > > return cxl_event_count(log) == 1;
> >
> > If there are multiple this will fail I think as cxl_event_count(log) will
> > go from 0
> > to X not 1.
> >
> > > }
> > > diff --git a/include/hw/cxl/cxl_events.h b/include/hw/cxl/cxl_events.h
> > > index 38cadaa0f3..b0e5cc89c0 100644
> > > --- a/include/hw/cxl/cxl_events.h
> > > +++ b/include/hw/cxl/cxl_events.h
> > > @@ -170,6 +170,7 @@ typedef struct CXLEventMemoryModule {
> > > * CXL r3.1 section Table 8-50: Dynamic Capacity Event Record
> > > * All fields little endian.
> > > */
> > > +#define MORE_FLAG BIT_MASK(0)
> > > typedef struct CXLEventDynamicCapacity {
> > > CXLEventRecordHdr hdr;
> > > uint8_t type;
> >
>
>
--
Fan Ni
