diff mbox

[1/2] cxl: Add mechanism for delivering AFU driver specific events

Message ID 1439957971-30483-1-git-send-email-imunsie@au.ibm.com (mailing list archive)
State Changes Requested
Headers show

Commit Message

Ian Munsie Aug. 19, 2015, 4:19 a.m. UTC
From: Ian Munsie <imunsie@au1.ibm.com>

This adds an afu_driver_ops structure with event_pending and
deliver_event callbacks. An AFU driver can fill these out and associate
it with a context to enable passing custom AFU specific events to
userspace.

The cxl driver will call event_pending() during poll, select, read, etc.
calls to check if an AFU driver specific event is pending, and will call
deliver_event() to deliver that event. This way, the cxl driver takes
care of all the usual locking semantics around these calls and handles
all the generic cxl events, so that the AFU driver only needs to worry
about it's own events.

The deliver_event() call is passed a struct cxl_event buffer to fill in.
The header will already be filled in for an AFU driver event, and the
AFU driver is expected to expand the header.size as necessary (up to
max_size, defined by struct cxl_event_afu_driver_reserved) and fill out
it's own information.

Conflicts between AFU specific events are not expected, due to the fact
that each AFU specific driver has it's own mechanism to deliver an AFU
file descriptor to userspace.

Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
---
 drivers/misc/cxl/Kconfig |  5 +++++
 drivers/misc/cxl/api.c   |  7 +++++++
 drivers/misc/cxl/cxl.h   |  6 +++++-
 drivers/misc/cxl/file.c  | 37 +++++++++++++++++++++++++++----------
 include/misc/cxl.h       | 29 +++++++++++++++++++++++++++++
 include/uapi/misc/cxl.h  | 13 +++++++++++++
 6 files changed, 86 insertions(+), 11 deletions(-)

Comments

Michael Ellerman Aug. 19, 2015, 5:30 a.m. UTC | #1
On Wed, 2015-08-19 at 14:19 +1000, Ian Munsie wrote:
> From: Ian Munsie <imunsie@au1.ibm.com>
> 
> This adds an afu_driver_ops structure with event_pending and
> deliver_event callbacks. An AFU driver can fill these out and associate
> it with a context to enable passing custom AFU specific events to
> userspace.

What's an AFU driver? Give me an example.

> The cxl driver will call event_pending() during poll, select, read, etc.
> calls to check if an AFU driver specific event is pending, and will call
> deliver_event() to deliver that event. This way, the cxl driver takes
> care of all the usual locking semantics around these calls and handles
> all the generic cxl events, so that the AFU driver only needs to worry
> about it's own events.
> 
> The deliver_event() call is passed a struct cxl_event buffer to fill in.
> The header will already be filled in for an AFU driver event, and the
> AFU driver is expected to expand the header.size as necessary (up to
> max_size, defined by struct cxl_event_afu_driver_reserved) and fill out
> it's own information.
> 
> Conflicts between AFU specific events are not expected, due to the fact
> that each AFU specific driver has it's own mechanism to deliver an AFU
> file descriptor to userspace.

I don't grok this bit.

> Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
> ---
>  drivers/misc/cxl/Kconfig |  5 +++++
>  drivers/misc/cxl/api.c   |  7 +++++++
>  drivers/misc/cxl/cxl.h   |  6 +++++-
>  drivers/misc/cxl/file.c  | 37 +++++++++++++++++++++++++++----------
>  include/misc/cxl.h       | 29 +++++++++++++++++++++++++++++
>  include/uapi/misc/cxl.h  | 13 +++++++++++++
>  6 files changed, 86 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/misc/cxl/Kconfig b/drivers/misc/cxl/Kconfig
> index 8756d06..560412c 100644
> --- a/drivers/misc/cxl/Kconfig
> +++ b/drivers/misc/cxl/Kconfig
> @@ -15,12 +15,17 @@ config CXL_EEH
>  	bool
>  	default n
>  
> +config CXL_AFU_DRIVER_OPS
> +	bool
> +	default n
> +
>  config CXL
>  	tristate "Support for IBM Coherent Accelerators (CXL)"
>  	depends on PPC_POWERNV && PCI_MSI && EEH
>  	select CXL_BASE
>  	select CXL_KERNEL_API
>  	select CXL_EEH
> +	select CXL_AFU_DRIVER_OPS
>  	default m
>  	help
>  	  Select this option to enable driver support for IBM Coherent
> diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
> index 6a768a9..e0f0c78 100644
> --- a/drivers/misc/cxl/api.c
> +++ b/drivers/misc/cxl/api.c
> @@ -267,6 +267,13 @@ struct cxl_context *cxl_fops_get_context(struct file *file)
>  }
>  EXPORT_SYMBOL_GPL(cxl_fops_get_context);
>  
> +void cxl_set_driver_ops(struct cxl_context *ctx,
> +			struct cxl_afu_driver_ops *ops)
> +{
> +	ctx->afu_driver_ops = ops;
> +}
> +EXPORT_SYMBOL_GPL(cxl_set_driver_ops);

This is pointless.

BUT, it wouldn't be if you actually checked the ops. Which you should do,
because then later you can avoid checking them on every event.

IIUI you should never have one op set but not the other, so you check in here
that both are set and error out otherwise.

Then in afu_read() you can change this:

> +	if (ctx->afu_driver_ops
> +			&& ctx->afu_driver_ops->event_pending
> +			&& ctx->afu_driver_ops->deliver_event
> +			&& ctx->afu_driver_ops->event_pending(ctx)) {

to:

> +	if (ctx->afu_driver_ops && ctx->afu_driver_ops->event_pending(ctx)) {


> diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
> index 6f53866..30e44a8 100644
> --- a/drivers/misc/cxl/cxl.h
> +++ b/drivers/misc/cxl/cxl.h
> @@ -24,6 +24,7 @@
>  #include <asm/reg.h>
>  #include <misc/cxl-base.h>
>  
> +#include <misc/cxl.h>
>  #include <uapi/misc/cxl.h>
>  
>  extern uint cxl_verbose;
> @@ -34,7 +35,7 @@ extern uint cxl_verbose;
>   * Bump version each time a user API change is made, whether it is
>   * backwards compatible ot not.
>   */
> -#define CXL_API_VERSION 1
> +#define CXL_API_VERSION 2

I'm not clear on why we're bumping the API version?

Isn't this purely about in-kernel drivers?

I see below you're touching the uapi header, so I guess it's that simple. But
if you can explain it better that would be great.

>  #define CXL_API_VERSION_COMPATIBLE 1
>  
>  /*
> @@ -462,6 +463,9 @@ struct cxl_context {
>  	bool pending_fault;
>  	bool pending_afu_err;
>  
> +	/* Used by AFU drivers for driver specific event delivery */
> +	struct cxl_afu_driver_ops *afu_driver_ops;
> +
>  	struct rcu_head rcu;
>  };
>  
> diff --git a/drivers/misc/cxl/file.c b/drivers/misc/cxl/file.c
> index 57bdb47..2ebaca3 100644
> --- a/drivers/misc/cxl/file.c
> +++ b/drivers/misc/cxl/file.c
> @@ -279,6 +279,22 @@ int afu_mmap(struct file *file, struct vm_area_struct *vm)
>  	return cxl_context_iomap(ctx, vm);
>  }
>  
> +static inline int _ctx_event_pending(struct cxl_context *ctx)

Why isn't this returning bool?

> +{
> +	bool afu_driver_event_pending = false;
> +
> +	if (ctx->afu_driver_ops && ctx->afu_driver_ops->event_pending)
> +		afu_driver_event_pending = ctx->afu_driver_ops->event_pending(ctx);

You can drop the 2nd test in the if, if you enforce sane ops in cxl_set_driver_ops().

> +
> +	return (ctx->pending_irq || ctx->pending_fault ||
> +	    ctx->pending_afu_err || afu_driver_event_pending);

This would be nicer if you could short-circuit and avoid the function call when
the easy & cheap bool tests are true.

eg.

	if (ctx->pending_irq || ctx->pending_fault || ctx->pending_afu_err)
		return 1;

	if (ctx->afu_driver_ops && ctx->afu_driver_ops->event_pending)
		return ctx->afu_driver_ops->event_pending(ctx);

	return 0;

Or you could do it all in a single return if you wanted to.

	return ctx->pending_irq || ctx->pending_fault || ctx->pending_afu_err ||
	       (ctx->afu_driver_ops ? ctx->afu_driver_ops->event_pending(ctx) : 0);


> +static inline int ctx_event_pending(struct cxl_context *ctx)
> +{
> +	return _ctx_event_pending(ctx) || (ctx->status == CLOSED);
> +}

It's not at all clear why you would call this version vs the underscore version
_ctx_event_pending(), can they be named better to make it clear?

> @@ -360,7 +369,15 @@ 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->afu_driver_ops->deliver_event
> +			&& ctx->afu_driver_ops->event_pending(ctx)) {

As I said above this would be much less gross if you enforced the ops being
sane when they're registered.

> +		pr_devel("afu_read delivering AFU driver specific event\n");
> +		event.header.type = CXL_EVENT_AFU_DRIVER;
> +		ctx->afu_driver_ops->deliver_event(&event, ctx, sizeof(event));
> +		WARN_ON(event.header.size > sizeof(event));

Some newlines in here would really help my eyes.


> +	} 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;
> @@ -538,7 +555,7 @@ int __init cxl_file_init(void)
>  	 * If these change we really need to update API.  Either change some
>  	 * flags or update API version number CXL_API_VERSION.
>  	 */
> -	BUILD_BUG_ON(CXL_API_VERSION != 1);
> +	BUILD_BUG_ON(CXL_API_VERSION != 2);
>  	BUILD_BUG_ON(sizeof(struct cxl_ioctl_start_work) != 64);
>  	BUILD_BUG_ON(sizeof(struct cxl_event_header) != 8);
>  	BUILD_BUG_ON(sizeof(struct cxl_event_afu_interrupt) != 8);
> diff --git a/include/misc/cxl.h b/include/misc/cxl.h
> index f2ffe5b..73e03a6 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
> + * 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).
> + *
> + * 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 (which can be increased if necessary by reserving more space 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 *cxl);
> +	void (*deliver_event) (struct cxl_event *event,
> +			       struct cxl_context *cxl, size_t max_size);

context should always be the first param IMHO.

> +};
> +
> +/* 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 99a8ca1..97f20b3 100644
> --- a/include/uapi/misc/cxl.h
> +++ b/include/uapi/misc/cxl.h
> @@ -67,6 +67,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 {
> @@ -98,12 +99,24 @@ struct cxl_event_afu_error {
>  	__u64 error;
>  };
>  
> +struct cxl_event_afu_driver_reserved {
> +	/*
> +	 * Reserves space for AFU driver specific events. If an AFU driver
> +	 * needs a larger buffer they should increase this to match so the cxl
> +	 * driver will allocate enough space.

This is odd, or at least oddly worded.

An AFU driver can't increase this. The author of an AFU driver can ask us to
increase it, which requires a source change and a recompile.

> +	 *
> +	 * This is not ABI. The contents will be, but that's up the AFU driver.
> +	 */
> +	__u64 reserved[4];

It is ABI. An old client is within its rights to assume header.size will only
ever be <= sizeof(cxl_event).

It looks like by choosing 4 here you've not actually increased the maximum
possible size of cxl_event. But if you make it any bigger that would break ABI.

> +};
> +
>  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;
>  	};
>  };
>  

cheers
Ian Munsie Aug. 19, 2015, 7:03 a.m. UTC | #2
Excerpts from Michael Ellerman's message of 2015-08-19 15:30:46 +1000:
> On Wed, 2015-08-19 at 14:19 +1000, Ian Munsie wrote:
> > From: Ian Munsie <imunsie@au1.ibm.com>
> > 
> > This adds an afu_driver_ops structure with event_pending and
> > deliver_event callbacks. An AFU driver can fill these out and associate
> > it with a context to enable passing custom AFU specific events to
> > userspace.
> 
> What's an AFU driver? Give me an example.

Maybe I should say user of the in-kernel cxl API? I've been referring to
these as AFU drivers because they are binding to an AFU - somehow I
thought we had already used that terminology in the API, but it appears
that I must have imagined that ;-)

The only real example currently (cxlflash) is still trying to get merged
(and their current patches do not depend on this functionality).

> > Conflicts between AFU specific events are not expected, due to the fact
> > that each AFU specific driver has it's own mechanism to deliver an AFU
> > file descriptor to userspace.
> 
> I don't grok this bit.

So, cxlflash might define a set of AFU specific events that they want to
use and all is fine, but later we might get a second AFU driver (maybe a
gzip, maybe a video codec, maybe networking, whatever) that also wants
to deliver some AFU specific events. If a userspace application were
reading from the AFU file descriptor and got one of these events it
couldn't tell just from the event alone whether it was a cxlflash event,
a gzip event or something else.

Except, the application must know what AFU driver it's talking to
because it has to have used whatever user API that driver exported to
obtain the AFU file descriptor (e.g. cxlflash is handing it out through
an ioctl on their scsi device). Any userspace application that obtains
an AFU file descriptor through the generic cxl driver's user API won't
get any AFU specific events. Hence, the user application already knows
what AFU driver specific events it could expect and therefore there is
no conflict between events from different drivers.

The alternative is adding something similar to an ioctl number...

> > +void cxl_set_driver_ops(struct cxl_context *ctx,
> > +            struct cxl_afu_driver_ops *ops)
> > +{
> > +    ctx->afu_driver_ops = ops;
> > +}
> > +EXPORT_SYMBOL_GPL(cxl_set_driver_ops);
> 
> This is pointless.

Not entirely as the contents of the struct cxl_context is not intended
to be exposed to the AFU driver.

> BUT, it wouldn't be if you actually checked the ops. Which you should do,
> because then later you can avoid checking them on every event.

ok

> IIUI you should never have one op set but not the other, so you check in here
> that both are set and error out otherwise.

I was thinking about the possibility of extending the ops later so I put
the checks where it was used, but ok - that can come later if/when we
actually need it.

> > -#define CXL_API_VERSION 1
> > +#define CXL_API_VERSION 2
> 
> I'm not clear on why we're bumping the API version?
> 
> Isn't this purely about in-kernel drivers?
> 
> I see below you're touching the uapi header, so I guess it's that simple. But
> if you can explain it better that would be great.

We are defining a new event type in the generic cxl driver (though
notably it will never be used by the generic cxl driver's user API). I
figured it was safer to bump it just in case, but arguably we could
leave it alone as the changes should only be visible to userspace code
that is using an AFU driver's user API and not the generic cxl user API.

What do you think?

Either way I left the compatible version number alone.

> > +static inline int _ctx_event_pending(struct cxl_context *ctx)
> 
> Why isn't this returning bool?

No reason, will change it.

> > +    if (ctx->afu_driver_ops && ctx->afu_driver_ops->event_pending)
> > +        afu_driver_event_pending = ctx->afu_driver_ops->event_pending(ctx);
> 
> You can drop the 2nd test in the if, if you enforce sane ops in cxl_set_driver_ops().

ok

> > +    return (ctx->pending_irq || ctx->pending_fault ||
> > +        ctx->pending_afu_err || afu_driver_event_pending);
> 
> This would be nicer if you could short-circuit and avoid the function call when
> the easy & cheap bool tests are true.

ok

> > +static inline int ctx_event_pending(struct cxl_context *ctx)
> > +{
> > +    return _ctx_event_pending(ctx) || (ctx->status == CLOSED);
> > +}
> 
> It's not at all clear why you would call this version vs the underscore version
> _ctx_event_pending(), can they be named better to make it clear?

I might rename it (cxl_event_pending_or_error?) - the difference here is
only because the poll call will want to set POLLERR if status == CLOSED
and no other events are pending, so it checks that case explicitly.

> > -    if (ctx->pending_irq) {
> > +    if (ctx->afu_driver_ops
> > +            && ctx->afu_driver_ops->event_pending
> > +            && ctx->afu_driver_ops->deliver_event
> > +            && ctx->afu_driver_ops->event_pending(ctx)) {
> 
> As I said above this would be much less gross if you enforced the ops being
> sane when they're registered.

ok

> > +        pr_devel("afu_read delivering AFU driver specific event\n");
> > +        event.header.type = CXL_EVENT_AFU_DRIVER;
> > +        ctx->afu_driver_ops->deliver_event(&event, ctx, sizeof(event));
> > +        WARN_ON(event.header.size > sizeof(event));
> 
> Some newlines in here would really help my eyes.

ok

> > +struct cxl_afu_driver_ops {
> > +    bool (*event_pending) (struct cxl_context *cxl);
> > +    void (*deliver_event) (struct cxl_event *event,
> > +                   struct cxl_context *cxl, size_t max_size);
> 
> context should always be the first param IMHO.

ok

> > +struct cxl_event_afu_driver_reserved {
> > +    /*
> > +     * Reserves space for AFU driver specific events. If an AFU driver
> > +     * needs a larger buffer they should increase this to match so the cxl
> > +     * driver will allocate enough space.
> 
> This is odd, or at least oddly worded.
> 
> An AFU driver can't increase this. The author of an AFU driver can ask us to
> increase it, which requires a source change and a recompile.

ok, will reword (I really meant that they can increase it when they send
the patch for their driver, not dynamically at runtime).

> > +     *
> > +     * This is not ABI. The contents will be, but that's up the AFU driver.
> > +     */
> > +    __u64 reserved[4];
> 
> It is ABI. An old client is within its rights to assume header.size will only
> ever be <= sizeof(cxl_event).
> 
> It looks like by choosing 4 here you've not actually increased the maximum
> possible size of cxl_event. But if you make it any bigger that would break ABI.

I intentionally haven't increased the size as allocated, because I have
no idea how much they will want to put in there - I could reserve more
space here, but how long is a piece of string? I could go for the max
of 4k, but that seemed a bit excessive as my gut feeling is that it is
more likely going to be small. Rather, I opted to just let the AFU
driver author them send us a patch if they need more, but if you have a
better idea to handle this I'm listening :)

It shouldn't be an ABI change to increase this - header.size passed to
the user will not change for existing events (as that is set in the read
call to sizeof(cxl_event_header) + sizeof(whatever event is being
dispatched), regardless of what the overall size of struct cxl_event
is). Plus, the user is already required to use a 4k buffer for reads, so
the size of the struct is only tangentially related to how much data we
actually hand them.

Cheers,
-Ian
diff mbox

Patch

diff --git a/drivers/misc/cxl/Kconfig b/drivers/misc/cxl/Kconfig
index 8756d06..560412c 100644
--- a/drivers/misc/cxl/Kconfig
+++ b/drivers/misc/cxl/Kconfig
@@ -15,12 +15,17 @@  config CXL_EEH
 	bool
 	default n
 
+config CXL_AFU_DRIVER_OPS
+	bool
+	default n
+
 config CXL
 	tristate "Support for IBM Coherent Accelerators (CXL)"
 	depends on PPC_POWERNV && PCI_MSI && EEH
 	select CXL_BASE
 	select CXL_KERNEL_API
 	select CXL_EEH
+	select CXL_AFU_DRIVER_OPS
 	default m
 	help
 	  Select this option to enable driver support for IBM Coherent
diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
index 6a768a9..e0f0c78 100644
--- a/drivers/misc/cxl/api.c
+++ b/drivers/misc/cxl/api.c
@@ -267,6 +267,13 @@  struct cxl_context *cxl_fops_get_context(struct file *file)
 }
 EXPORT_SYMBOL_GPL(cxl_fops_get_context);
 
+void cxl_set_driver_ops(struct cxl_context *ctx,
+			struct cxl_afu_driver_ops *ops)
+{
+	ctx->afu_driver_ops = ops;
+}
+EXPORT_SYMBOL_GPL(cxl_set_driver_ops);
+
 int cxl_start_work(struct cxl_context *ctx,
 		   struct cxl_ioctl_start_work *work)
 {
diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
index 6f53866..30e44a8 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -24,6 +24,7 @@ 
 #include <asm/reg.h>
 #include <misc/cxl-base.h>
 
+#include <misc/cxl.h>
 #include <uapi/misc/cxl.h>
 
 extern uint cxl_verbose;
@@ -34,7 +35,7 @@  extern uint cxl_verbose;
  * Bump version each time a user API change is made, whether it is
  * backwards compatible ot not.
  */
-#define CXL_API_VERSION 1
+#define CXL_API_VERSION 2
 #define CXL_API_VERSION_COMPATIBLE 1
 
 /*
@@ -462,6 +463,9 @@  struct cxl_context {
 	bool pending_fault;
 	bool pending_afu_err;
 
+	/* Used by AFU drivers for driver specific event delivery */
+	struct cxl_afu_driver_ops *afu_driver_ops;
+
 	struct rcu_head rcu;
 };
 
diff --git a/drivers/misc/cxl/file.c b/drivers/misc/cxl/file.c
index 57bdb47..2ebaca3 100644
--- a/drivers/misc/cxl/file.c
+++ b/drivers/misc/cxl/file.c
@@ -279,6 +279,22 @@  int afu_mmap(struct file *file, struct vm_area_struct *vm)
 	return cxl_context_iomap(ctx, vm);
 }
 
+static inline int _ctx_event_pending(struct cxl_context *ctx)
+{
+	bool afu_driver_event_pending = false;
+
+	if (ctx->afu_driver_ops && ctx->afu_driver_ops->event_pending)
+		afu_driver_event_pending = ctx->afu_driver_ops->event_pending(ctx);
+
+	return (ctx->pending_irq || ctx->pending_fault ||
+	    ctx->pending_afu_err || afu_driver_event_pending);
+}
+
+static inline int ctx_event_pending(struct cxl_context *ctx)
+{
+	return _ctx_event_pending(ctx) || (ctx->status == CLOSED);
+}
+
 unsigned int afu_poll(struct file *file, struct poll_table_struct *poll)
 {
 	struct cxl_context *ctx = file->private_data;
@@ -291,8 +307,7 @@  unsigned int afu_poll(struct file *file, struct poll_table_struct *poll)
 	pr_devel("afu_poll wait done pe: %i\n", ctx->pe);
 
 	spin_lock_irqsave(&ctx->lock, flags);
-	if (ctx->pending_irq || ctx->pending_fault ||
-	    ctx->pending_afu_err)
+	if (_ctx_event_pending(ctx))
 		mask |= POLLIN | POLLRDNORM;
 	else if (ctx->status == CLOSED)
 		/* Only error on closed when there are no futher events pending
@@ -305,12 +320,6 @@  unsigned int afu_poll(struct file *file, struct poll_table_struct *poll)
 	return mask;
 }
 
-static inline int ctx_event_pending(struct cxl_context *ctx)
-{
-	return (ctx->pending_irq || ctx->pending_fault ||
-	    ctx->pending_afu_err || (ctx->status == CLOSED));
-}
-
 ssize_t afu_read(struct file *file, char __user *buf, size_t count,
 			loff_t *off)
 {
@@ -360,7 +369,15 @@  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->afu_driver_ops->deliver_event
+			&& 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(&event, ctx, 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;
@@ -538,7 +555,7 @@  int __init cxl_file_init(void)
 	 * If these change we really need to update API.  Either change some
 	 * flags or update API version number CXL_API_VERSION.
 	 */
-	BUILD_BUG_ON(CXL_API_VERSION != 1);
+	BUILD_BUG_ON(CXL_API_VERSION != 2);
 	BUILD_BUG_ON(sizeof(struct cxl_ioctl_start_work) != 64);
 	BUILD_BUG_ON(sizeof(struct cxl_event_header) != 8);
 	BUILD_BUG_ON(sizeof(struct cxl_event_afu_interrupt) != 8);
diff --git a/include/misc/cxl.h b/include/misc/cxl.h
index f2ffe5b..73e03a6 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
+ * 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).
+ *
+ * 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 (which can be increased if necessary by reserving more space 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 *cxl);
+	void (*deliver_event) (struct cxl_event *event,
+			       struct cxl_context *cxl, 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 99a8ca1..97f20b3 100644
--- a/include/uapi/misc/cxl.h
+++ b/include/uapi/misc/cxl.h
@@ -67,6 +67,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 {
@@ -98,12 +99,24 @@  struct cxl_event_afu_error {
 	__u64 error;
 };
 
+struct cxl_event_afu_driver_reserved {
+	/*
+	 * Reserves space for AFU driver specific events. If an AFU driver
+	 * needs a larger buffer they should increase this to match so the cxl
+	 * driver will allocate enough space.
+	 *
+	 * This is not ABI. The contents will be, 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;
 	};
 };