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


Reply via email to