A couple of minor nits below... > On Mar 7, 2016, at 12:59 PM, Ian Munsie <imun...@au1.ibm.com> wrote: > > @@ -346,7 +350,7 @@ ssize_t afu_read(struct file *file, char __user *buf, > size_t count, > > for (;;) { > prepare_to_wait(&ctx->wq, &wait, TASK_INTERRUPTIBLE); > - if (ctx_event_pending(ctx)) > + if (ctx_event_pending(ctx) || (ctx->status == CLOSED)) > break; > > if (!cxl_adapter_link_ok(ctx->afu->adapter)) { > @@ -376,7 +380,14 @@ ssize_t afu_read(struct file *file, char __user *buf, > size_t count, > memset(&event, 0, sizeof(event)); > event.header.process_element = ctx->pe; > event.header.size = sizeof(struct cxl_event_header); > - if (ctx->pending_irq) { > + > + if (ctx->afu_driver_ops && ctx->afu_driver_ops->event_pending(ctx)) { > + pr_devel("afu_read delivering AFU driver specific event\n"); > + event.header.type = CXL_EVENT_AFU_DRIVER; > + ctx->afu_driver_ops->deliver_event(ctx, &event, sizeof(event)); > + WARN_ON(event.header.size > sizeof(event)); > + > + } else if (ctx->pending_irq) { > pr_devel("afu_read delivering AFU interrupt\n"); > event.header.size += sizeof(struct cxl_event_afu_interrupt); > event.header.type = CXL_EVENT_AFU_INTERRUPT; > @@ -384,6 +395,7 @@ ssize_t afu_read(struct file *file, char __user *buf, > size_t count, > clear_bit(event.irq.irq - 1, ctx->irq_bitmap); > if (bitmap_empty(ctx->irq_bitmap, ctx->irq_count)) > ctx->pending_irq = false; > + > } else if (ctx->pending_fault) { > pr_devel("afu_read delivering data storage fault\n"); > event.header.size += sizeof(struct cxl_event_data_storage); > @@ -391,12 +403,14 @@ ssize_t afu_read(struct file *file, char __user *buf, > size_t count, > event.fault.addr = ctx->fault_addr; > event.fault.dsisr = ctx->fault_dsisr; > ctx->pending_fault = false; > + > } else if (ctx->pending_afu_err) { > pr_devel("afu_read delivering afu error\n"); > event.header.size += sizeof(struct cxl_event_afu_error); > event.header.type = CXL_EVENT_AFU_ERROR; > event.afu_error.error = ctx->afu_err; > ctx->pending_afu_err = false; > +
Any reason for adding these extra lines as part of this commit? > diff --git a/include/misc/cxl.h b/include/misc/cxl.h > index f2ffe5b..f198a42 100644 > --- a/include/misc/cxl.h > +++ b/include/misc/cxl.h > @@ -210,4 +210,33 @@ ssize_t cxl_fd_read(struct file *file, char __user *buf, > size_t count, > void cxl_perst_reloads_same_image(struct cxl_afu *afu, > bool perst_reloads_same_image); > > +/* > + * AFU driver ops allows an AFU driver to create their own events to pass to > + * userspace through the file descriptor as a simpler alternative to > overriding > + * the read() and poll() calls that works with the generic cxl events. These > + * events are given priority over the generic cxl events, so will be > delivered so _they_ will be delivered > + * first if multiple types of events are pending. > + * > + * even_pending() will be called by the cxl driver to check if an event is > + * pending (e.g. in select/poll/read calls). event_pending() <- missing 't' > + * > + * deliver_event() will be called to fill out a cxl_event structure with the > + * driver specific event. The header will already have the type and > + * process_element fields filled in, and header.size will be set to > + * sizeof(struct cxl_event_header). The AFU driver can extend that size up to > + * max_size (if an afu driver requires more space, they should submit a patch > + * increasing the size in the struct cxl_event_afu_driver_reserved > definition). > + * > + * Both of these calls are made with a spin lock held, so they must not > sleep. > + */ > +struct cxl_afu_driver_ops { > + bool (*event_pending) (struct cxl_context *ctx); > + void (*deliver_event) (struct cxl_context *ctx, > + struct cxl_event *event, size_t max_size); > +}; > + > +/* Associate the above driver ops with a specific context */ > +void cxl_set_driver_ops(struct cxl_context *ctx, > + struct cxl_afu_driver_ops *ops); > + > #endif /* _MISC_CXL_H */ > diff --git a/include/uapi/misc/cxl.h b/include/uapi/misc/cxl.h > index 1e889aa..8b097db 100644 > --- a/include/uapi/misc/cxl.h > +++ b/include/uapi/misc/cxl.h > @@ -69,6 +69,7 @@ enum cxl_event_type { > CXL_EVENT_AFU_INTERRUPT = 1, > CXL_EVENT_DATA_STORAGE = 2, > CXL_EVENT_AFU_ERROR = 3, > + CXL_EVENT_AFU_DRIVER = 4, > }; > > struct cxl_event_header { > @@ -100,12 +101,33 @@ struct cxl_event_afu_error { > __u64 error; > }; > > +struct cxl_event_afu_driver_reserved { > + /* > + * Reserves space for AFU driver specific events. Not actually > + * reserving any more space compared to other events as we can't know > + * how much an AFU driver will need (but it is likely to be small). If > + * your AFU driver needs more than this, please submit a patch > + * increasing it as part of your driver submission. > + * > + * This is not ABI since the event header.size passed to the user for > + * existing events is set in the read call to sizeof(cxl_event_header) > + * + sizeof(whatever event is being dispatched) and will not increase > + * just because this is, and the user is already required to use a 4K > + * buffer on the read call. This is merely the size of the buffer > + * passed between the cxl and AFU drivers. > + * > + * Of course the contents will be ABI, but that's up the AFU driver. > + */ > + __u64 reserved[4]; > +}; > + > struct cxl_event { > struct cxl_event_header header; > union { > struct cxl_event_afu_interrupt irq; > struct cxl_event_data_storage fault; > struct cxl_event_afu_error afu_error; > + struct cxl_event_afu_driver_reserved afu_driver_event; > }; > }; > > -- > 2.1.4 > > _______________________________________________ > Linuxppc-dev mailing list > linuxppc-...@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/linuxppc-dev