Re: [PATCH] cxl: drop duplicate header sched.h
Acked-by: Ian Munsie <imun...@au1.ibm.com>
Re: [PATCH] cxl: drop duplicate header sched.h
Acked-by: Ian Munsie
Re: [PATCH] cxl: Fix memory allocation failure test
Acked-by: Ian Munsie <imun...@au1.ibm.com>
Re: [PATCH] cxl: Fix memory allocation failure test
Acked-by: Ian Munsie
Re: [PATCH] cxl: Fix error handling
Acked-by: Ian Munsie <imun...@au1.ibm.com>
Re: [PATCH] cxl: Fix error handling
Acked-by: Ian Munsie
Re: [PATCH] cxl: Fix error handling
Acked-by: Ian Munsie <imun...@au1.ibm.com>
Re: [PATCH] cxl: Fix error handling
Acked-by: Ian Munsie
Re: [PATCH -next] cxl: Use for_each_compatible_node() macro
Acked-by: Ian Munsie <imun...@au1.ibm.com>
Re: [PATCH -next] cxl: Use for_each_compatible_node() macro
Acked-by: Ian Munsie
Re: [PATCH] cxl: make base more explicitly non-modular
Acked-by: Ian Munsie <imun...@au1.ibm.com>
Re: [PATCH] cxl: make base more explicitly non-modular
Acked-by: Ian Munsie
Re: [PATCH v3 1/2] cxl: Add mechanism for delivering AFU driver specific events
Excerpts from Frederic Barrat's message of 2016-03-09 20:27:20 +1100: > It would also avoid entering > WARN(1, "afu_read must be buggy\n"); > if the driver changes its mind between the 2 calls :-) Honestly, it had better not - that would be a gross violation of the poll & read semantics and the kind of thing that leads to application hangs. Cheers, -Ian
Re: [PATCH v3 1/2] cxl: Add mechanism for delivering AFU driver specific events
Excerpts from Frederic Barrat's message of 2016-03-09 20:27:20 +1100: > It would also avoid entering > WARN(1, "afu_read must be buggy\n"); > if the driver changes its mind between the 2 calls :-) Honestly, it had better not - that would be a gross violation of the poll & read semantics and the kind of thing that leads to application hangs. Cheers, -Ian
Re: [PATCH v3 1/2] cxl: Add mechanism for delivering AFU driver specific events
Excerpts from Vaibhav Jain's message of 2016-03-10 01:37:56 +1100:
> > +select CXL_AFU_DRIVER_OPS
> I suggest wrapping the driver_ops struct definition and other related
> functions inside a #ifdef CONFIG_CXL_AFU_DRIVER_OPS.
No, the kconfig option is there so that cxlflash can add support for
this and not have to worry about breaking any builds if their code is
merged into the scsi tree that doesn't have our code yet.
There is nothing optional about this within our driver, which is why
this is a select and has no configuration choice of it's own.
On a related matter, we should send a patch to remove some of the
leftover config options that were added to smooth the merging of
cxlflash in the first place (CXL_KERNEL_API, CXL_EEH).
> > +void cxl_set_driver_ops(struct cxl_context *ctx,
> > +struct cxl_afu_driver_ops *ops)
> > +{
> > +WARN_ON(!ops->event_pending || !ops->deliver_event);
> > +ctx->afu_driver_ops = ops;
> > +}
> I would recommend adding a "struct module *" member to afu_driver_ops
> and doing a __module_get on to it here and module_put when we destroy
> the context. Since these callbacks will be residing within an external
> module .text region hence it should stay in the memory until the context
> is alive.
ok
> > diff --git a/drivers/misc/cxl/file.c b/drivers/misc/cxl/file.c
> > index 783337d..d1cc297 100644
> > --- a/drivers/misc/cxl/file.c
> > +++ b/drivers/misc/cxl/file.c
> > @@ -295,6 +295,17 @@ int afu_mmap(struct file *file, struct vm_area_struct
> > *vm)
> > return cxl_context_iomap(ctx, vm);
> > }
> >
> > +static inline bool ctx_event_pending(struct cxl_context *ctx)
> > +{
> > +if (ctx->pending_irq || ctx->pending_fault || ctx->pending_afu_err)
> > +return true;
> > +
> > +if (ctx->afu_driver_ops)
> > +return ctx->afu_driver_ops->event_pending(ctx);
> Should also check if ctx->afu_driver_ops->event_pending is NULL before
> calling it.
The v1 patch did exactly that and mpe rejected it as it made this code
too ugly - we now check that event_pending field is valid when it is
registered and WARN if it is not.
> I would propose these two apis.
>
> /*
> * fetches an event from the driver event queue. NULL means that queue
> * is empty. Can sleep if needed. The memory for cxl_event is allocated
> * by module being called. Hence it can be potentially be larger then
> * sizeof(struct cxl_event). Multiple calls to this should return same
> * pointer untill ack_event is called.
> */
> struct cxl_event * fetch_event(struct cxl_context * ctx);
>
> /*
> * Returns and acknowledge the struct cxl_event * back to the driver
> * which can then free it or maybe put it back in a kmem_cache. This
> * should be called once we have completely returned the current
> * struct cxl_event from the readcall
> */
> void ack_event(struct cxl_context * ctx, struct cxl_event *);
>
> I think above apis would give us more flexbility in the future when
> drivers would want to send larger events without breaking the abi.
I'm very reluctant to make this kind of change - while nice on paper,
poll() and read() are already very easy calls to screw up, and we have
seen that happen countless times in the past from different drivers that
e.g. and end up in a situation where poll says there is an event but
then read blocks, or poll blocks even though there is an event already
pending.
The API at the moment fits into the poll() / read() model and has
appropriate locking and the correct waiting semantics to avoid those
kind of issues (provided that the afu driver doesn't do something that
violates these semantics like sleep in one of these calls, but the
kernel has debug features to detect that), but any deviation from this
is too risky in my view.
Cheers,
-Ian
Re: [PATCH v3 1/2] cxl: Add mechanism for delivering AFU driver specific events
Excerpts from Vaibhav Jain's message of 2016-03-10 01:37:56 +1100:
> > +select CXL_AFU_DRIVER_OPS
> I suggest wrapping the driver_ops struct definition and other related
> functions inside a #ifdef CONFIG_CXL_AFU_DRIVER_OPS.
No, the kconfig option is there so that cxlflash can add support for
this and not have to worry about breaking any builds if their code is
merged into the scsi tree that doesn't have our code yet.
There is nothing optional about this within our driver, which is why
this is a select and has no configuration choice of it's own.
On a related matter, we should send a patch to remove some of the
leftover config options that were added to smooth the merging of
cxlflash in the first place (CXL_KERNEL_API, CXL_EEH).
> > +void cxl_set_driver_ops(struct cxl_context *ctx,
> > +struct cxl_afu_driver_ops *ops)
> > +{
> > +WARN_ON(!ops->event_pending || !ops->deliver_event);
> > +ctx->afu_driver_ops = ops;
> > +}
> I would recommend adding a "struct module *" member to afu_driver_ops
> and doing a __module_get on to it here and module_put when we destroy
> the context. Since these callbacks will be residing within an external
> module .text region hence it should stay in the memory until the context
> is alive.
ok
> > diff --git a/drivers/misc/cxl/file.c b/drivers/misc/cxl/file.c
> > index 783337d..d1cc297 100644
> > --- a/drivers/misc/cxl/file.c
> > +++ b/drivers/misc/cxl/file.c
> > @@ -295,6 +295,17 @@ int afu_mmap(struct file *file, struct vm_area_struct
> > *vm)
> > return cxl_context_iomap(ctx, vm);
> > }
> >
> > +static inline bool ctx_event_pending(struct cxl_context *ctx)
> > +{
> > +if (ctx->pending_irq || ctx->pending_fault || ctx->pending_afu_err)
> > +return true;
> > +
> > +if (ctx->afu_driver_ops)
> > +return ctx->afu_driver_ops->event_pending(ctx);
> Should also check if ctx->afu_driver_ops->event_pending is NULL before
> calling it.
The v1 patch did exactly that and mpe rejected it as it made this code
too ugly - we now check that event_pending field is valid when it is
registered and WARN if it is not.
> I would propose these two apis.
>
> /*
> * fetches an event from the driver event queue. NULL means that queue
> * is empty. Can sleep if needed. The memory for cxl_event is allocated
> * by module being called. Hence it can be potentially be larger then
> * sizeof(struct cxl_event). Multiple calls to this should return same
> * pointer untill ack_event is called.
> */
> struct cxl_event * fetch_event(struct cxl_context * ctx);
>
> /*
> * Returns and acknowledge the struct cxl_event * back to the driver
> * which can then free it or maybe put it back in a kmem_cache. This
> * should be called once we have completely returned the current
> * struct cxl_event from the readcall
> */
> void ack_event(struct cxl_context * ctx, struct cxl_event *);
>
> I think above apis would give us more flexbility in the future when
> drivers would want to send larger events without breaking the abi.
I'm very reluctant to make this kind of change - while nice on paper,
poll() and read() are already very easy calls to screw up, and we have
seen that happen countless times in the past from different drivers that
e.g. and end up in a situation where poll says there is an event but
then read blocks, or poll blocks even though there is an event already
pending.
The API at the moment fits into the poll() / read() model and has
appropriate locking and the correct waiting semantics to avoid those
kind of issues (provided that the afu driver doesn't do something that
violates these semantics like sleep in one of these calls, but the
kernel has debug features to detect that), but any deviation from this
is too risky in my view.
Cheers,
-Ian
Re: [PATCH v3 1/2] cxl: Add mechanism for delivering AFU driver specific events
Excerpts from Frederic Barrat's message of 2016-03-09 20:27:20 +1100: > So on afu_read(), we may call afu_driver_ops->event_pending() twice > before calling afu_driver_ops->deliver_event(). Actually, in the > (likely) scenario where there's only an afu_driver event pending, we > *will* call afu_driver_ops->event_pending() twice. Wouldn't it make > sense to cache it then? Yep, will change it.
Re: [PATCH v3 1/2] cxl: Add mechanism for delivering AFU driver specific events
Excerpts from Frederic Barrat's message of 2016-03-09 20:27:20 +1100: > So on afu_read(), we may call afu_driver_ops->event_pending() twice > before calling afu_driver_ops->deliver_event(). Actually, in the > (likely) scenario where there's only an afu_driver event pending, we > *will* call afu_driver_ops->event_pending() twice. Wouldn't it make > sense to cache it then? Yep, will change it.
[PATCH v3 1/2] cxl: Add mechanism for delivering AFU driver specific events
From: Ian Munsie <imun...@au1.ibm.com>
This adds an afu_driver_ops structure with event_pending and
deliver_event callbacks. An AFU driver such as cxlflash 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.
Since AFU drivers provide their own means for userspace to obtain the
AFU file descriptor (i.e. cxlflash uses an ioctl on their scsi file
descriptor to obtain the AFU file descriptor) and the generic cxl driver
will never use this event, the ABI of the event is up to each individual
AFU driver.
Signed-off-by: Ian Munsie <imun...@au1.ibm.com>
---
Changes since v2:
- Fixed some typos spotted by Matt Ochs
Changes since v1:
- Rebased on upstream
- Bumped cxl api version to 3
- Addressed comments from mpe:
- Clarified commit message & some comments
- Mentioned 'cxlflash' as a possible user of this event
- Check driver ops on registration and warn if missing calls
- Remove redundant checks where driver ops is used
- Simplified ctx_event_pending and removed underscore version
- Changed deliver_event to take the context as the first argument
drivers/misc/cxl/Kconfig | 5 +
drivers/misc/cxl/api.c | 8
drivers/misc/cxl/cxl.h | 6 +-
drivers/misc/cxl/file.c | 36 +---
include/misc/cxl.h | 29 +
include/uapi/misc/cxl.h | 22 ++
6 files changed, 94 insertions(+), 12 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 ea3eeb7..eebc9c3 100644
--- a/drivers/misc/cxl/api.c
+++ b/drivers/misc/cxl/api.c
@@ -296,6 +296,14 @@ 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)
+{
+ WARN_ON(!ops->event_pending || !ops->deliver_event);
+ 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 a521bc7..64e8e0a 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -24,6 +24,7 @@
#include
#include
+#include
#include
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 2
+#define CXL_API_VERSION 3
#define CXL_API_VERSION_COMPATIBLE 1
/*
@@ -485,6 +486,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 783337d..d1cc297 100644
--- a/drivers/misc/cxl/file.c
+++ b/drivers/misc/cxl/file.c
@@ -295,6 +295,17 @@ int afu_mmap(struct file *file, struct vm_area_struct *vm)
return cxl_context_iomap(ctx, vm);
}
+static inline bool ctx_event_pending(struct cxl_context *ctx)
+{
+ if (ctx->pending_irq || ctx->pending_fault || ctx->pending_afu_err)
+ return true;
+
+ if (ctx->afu_driver_ops)
+ return ctx->afu_driver_ops->event_pending(ctx);
+
+ return false;
+}
+
unsigned int afu_poll(struct file *file, struct poll_table_struct *poll)
{
struct cxl_context *ctx = file->private_data;
@@ -307,8 +318,7 @@ unsigned int afu_poll(struct
[PATCH v3 1/2] cxl: Add mechanism for delivering AFU driver specific events
From: Ian Munsie
This adds an afu_driver_ops structure with event_pending and
deliver_event callbacks. An AFU driver such as cxlflash 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.
Since AFU drivers provide their own means for userspace to obtain the
AFU file descriptor (i.e. cxlflash uses an ioctl on their scsi file
descriptor to obtain the AFU file descriptor) and the generic cxl driver
will never use this event, the ABI of the event is up to each individual
AFU driver.
Signed-off-by: Ian Munsie
---
Changes since v2:
- Fixed some typos spotted by Matt Ochs
Changes since v1:
- Rebased on upstream
- Bumped cxl api version to 3
- Addressed comments from mpe:
- Clarified commit message & some comments
- Mentioned 'cxlflash' as a possible user of this event
- Check driver ops on registration and warn if missing calls
- Remove redundant checks where driver ops is used
- Simplified ctx_event_pending and removed underscore version
- Changed deliver_event to take the context as the first argument
drivers/misc/cxl/Kconfig | 5 +
drivers/misc/cxl/api.c | 8
drivers/misc/cxl/cxl.h | 6 +-
drivers/misc/cxl/file.c | 36 +---
include/misc/cxl.h | 29 +
include/uapi/misc/cxl.h | 22 ++
6 files changed, 94 insertions(+), 12 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 ea3eeb7..eebc9c3 100644
--- a/drivers/misc/cxl/api.c
+++ b/drivers/misc/cxl/api.c
@@ -296,6 +296,14 @@ 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)
+{
+ WARN_ON(!ops->event_pending || !ops->deliver_event);
+ 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 a521bc7..64e8e0a 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -24,6 +24,7 @@
#include
#include
+#include
#include
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 2
+#define CXL_API_VERSION 3
#define CXL_API_VERSION_COMPATIBLE 1
/*
@@ -485,6 +486,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 783337d..d1cc297 100644
--- a/drivers/misc/cxl/file.c
+++ b/drivers/misc/cxl/file.c
@@ -295,6 +295,17 @@ int afu_mmap(struct file *file, struct vm_area_struct *vm)
return cxl_context_iomap(ctx, vm);
}
+static inline bool ctx_event_pending(struct cxl_context *ctx)
+{
+ if (ctx->pending_irq || ctx->pending_fault || ctx->pending_afu_err)
+ return true;
+
+ if (ctx->afu_driver_ops)
+ return ctx->afu_driver_ops->event_pending(ctx);
+
+ return false;
+}
+
unsigned int afu_poll(struct file *file, struct poll_table_struct *poll)
{
struct cxl_context *ctx = file->private_data;
@@ -307,8 +318,7 @@ unsigned int afu_poll(struct file *file, struct
poll_table_struct *p
[PATCH v3 2/2] cxl: add set/get private data to context struct
From: Michael Neuling <mi...@neuling.org>
This provides AFU drivers a means to associate private data with a cxl
context. This is particularly intended for make the new callbacks for
driver specific events easier for AFU drivers to use, as they can easily
get back to any private data structures they may use.
Signed-off-by: Michael Neuling <mi...@neuling.org>
Signed-off-by: Ian Munsie <imun...@au1.ibm.com>
Reviewed-by: Matthew R. Ochs <mro...@linux.vnet.ibm.com>
---
No changes since v1, added Matt Ochs reviewed-by tag.
drivers/misc/cxl/api.c | 21 +
drivers/misc/cxl/cxl.h | 3 +++
include/misc/cxl.h | 7 +++
3 files changed, 31 insertions(+)
diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
index eebc9c3..93b270c 100644
--- a/drivers/misc/cxl/api.c
+++ b/drivers/misc/cxl/api.c
@@ -91,6 +91,27 @@ int cxl_release_context(struct cxl_context *ctx)
}
EXPORT_SYMBOL_GPL(cxl_release_context);
+
+int cxl_set_priv(struct cxl_context *ctx, void *priv)
+{
+ if (!ctx)
+ return -EINVAL;
+
+ ctx->priv = priv;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(cxl_set_priv);
+
+void *cxl_get_priv(struct cxl_context *ctx)
+{
+ if (!ctx)
+ return ERR_PTR(-EINVAL);
+
+ return ctx->priv;
+}
+EXPORT_SYMBOL_GPL(cxl_get_priv);
+
int cxl_allocate_afu_irqs(struct cxl_context *ctx, int num)
{
if (num == 0)
diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
index 64e8e0a..71f66e7 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -454,6 +454,9 @@ struct cxl_context {
/* Only used in PR mode */
u64 process_token;
+ /* driver private data */
+ void *priv;
+
unsigned long *irq_bitmap; /* Accessed from IRQ context */
struct cxl_irq_ranges irqs;
struct list_head irq_names;
diff --git a/include/misc/cxl.h b/include/misc/cxl.h
index 01d66a3..76c08cb 100644
--- a/include/misc/cxl.h
+++ b/include/misc/cxl.h
@@ -89,6 +89,13 @@ struct cxl_context *cxl_dev_context_init(struct pci_dev
*dev);
int cxl_release_context(struct cxl_context *ctx);
/*
+ * Set and get private data associated with a context. Allows drivers to have a
+ * back pointer to some useful structure.
+ */
+int cxl_set_priv(struct cxl_context *ctx, void *priv);
+void *cxl_get_priv(struct cxl_context *ctx);
+
+/*
* Allocate AFU interrupts for this context. num=0 will allocate the default
* for this AFU as given in the AFU descriptor. This number doesn't include the
* interrupt 0 (CAIA defines AFU IRQ 0 for page faults). Each interrupt to be
--
2.1.4
[PATCH v3 2/2] cxl: add set/get private data to context struct
From: Michael Neuling
This provides AFU drivers a means to associate private data with a cxl
context. This is particularly intended for make the new callbacks for
driver specific events easier for AFU drivers to use, as they can easily
get back to any private data structures they may use.
Signed-off-by: Michael Neuling
Signed-off-by: Ian Munsie
Reviewed-by: Matthew R. Ochs
---
No changes since v1, added Matt Ochs reviewed-by tag.
drivers/misc/cxl/api.c | 21 +
drivers/misc/cxl/cxl.h | 3 +++
include/misc/cxl.h | 7 +++
3 files changed, 31 insertions(+)
diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
index eebc9c3..93b270c 100644
--- a/drivers/misc/cxl/api.c
+++ b/drivers/misc/cxl/api.c
@@ -91,6 +91,27 @@ int cxl_release_context(struct cxl_context *ctx)
}
EXPORT_SYMBOL_GPL(cxl_release_context);
+
+int cxl_set_priv(struct cxl_context *ctx, void *priv)
+{
+ if (!ctx)
+ return -EINVAL;
+
+ ctx->priv = priv;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(cxl_set_priv);
+
+void *cxl_get_priv(struct cxl_context *ctx)
+{
+ if (!ctx)
+ return ERR_PTR(-EINVAL);
+
+ return ctx->priv;
+}
+EXPORT_SYMBOL_GPL(cxl_get_priv);
+
int cxl_allocate_afu_irqs(struct cxl_context *ctx, int num)
{
if (num == 0)
diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
index 64e8e0a..71f66e7 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -454,6 +454,9 @@ struct cxl_context {
/* Only used in PR mode */
u64 process_token;
+ /* driver private data */
+ void *priv;
+
unsigned long *irq_bitmap; /* Accessed from IRQ context */
struct cxl_irq_ranges irqs;
struct list_head irq_names;
diff --git a/include/misc/cxl.h b/include/misc/cxl.h
index 01d66a3..76c08cb 100644
--- a/include/misc/cxl.h
+++ b/include/misc/cxl.h
@@ -89,6 +89,13 @@ struct cxl_context *cxl_dev_context_init(struct pci_dev
*dev);
int cxl_release_context(struct cxl_context *ctx);
/*
+ * Set and get private data associated with a context. Allows drivers to have a
+ * back pointer to some useful structure.
+ */
+int cxl_set_priv(struct cxl_context *ctx, void *priv);
+void *cxl_get_priv(struct cxl_context *ctx);
+
+/*
* Allocate AFU interrupts for this context. num=0 will allocate the default
* for this AFU as given in the AFU descriptor. This number doesn't include the
* interrupt 0 (CAIA defines AFU IRQ 0 for page faults). Each interrupt to be
--
2.1.4
Re: [PATCH v2 1/2] cxl: Add mechanism for delivering AFU driver specific events
Excerpts from Matt Ochs's message of 2016-03-08 11:26:55 +1100: > Any reason for adding these extra lines as part of this commit? mpe asked for some newlines here in the v1 submission, and it only really made sense to do so if all the related sections had consistent whitespace as well. > > +/* > > + * 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 thanks for spotting that... > > + * > > + * 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' ...and that. Cheers, -Ian
Re: [PATCH v2 1/2] cxl: Add mechanism for delivering AFU driver specific events
Excerpts from Matt Ochs's message of 2016-03-08 11:26:55 +1100: > Any reason for adding these extra lines as part of this commit? mpe asked for some newlines here in the v1 submission, and it only really made sense to do so if all the related sections had consistent whitespace as well. > > +/* > > + * 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 thanks for spotting that... > > + * > > + * 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' ...and that. Cheers, -Ian
[PATCH v2 1/2] cxl: Add mechanism for delivering AFU driver specific events
From: Ian Munsie <imun...@au1.ibm.com>
This adds an afu_driver_ops structure with event_pending and
deliver_event callbacks. An AFU driver such as cxlflash 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.
Since AFU drivers provide their own means for userspace to obtain the
AFU file descriptor (i.e. cxlflash uses an ioctl on their scsi file
descriptor to obtain the AFU file descriptor) and the generic cxl driver
will never use this event, the ABI of the event is up to each individual
AFU driver.
Signed-off-by: Ian Munsie <imun...@au1.ibm.com>
---
Changes since v1:
- Rebased on upstream
- Bumped cxl api version to 3
- Addressed comments from mpe:
- Clarified commit message & some comments
- Mentioned 'cxlflash' as a possible user of this event
- Check driver ops on registration and warn if missing calls
- Remove redundant checks where driver ops is used
- Simplified ctx_event_pending and removed underscore version
- Changed deliver_event to take the context as the first argument
drivers/misc/cxl/Kconfig | 5 +
drivers/misc/cxl/api.c | 8
drivers/misc/cxl/cxl.h | 6 +-
drivers/misc/cxl/file.c | 36 +---
include/misc/cxl.h | 29 +
include/uapi/misc/cxl.h | 22 ++
6 files changed, 94 insertions(+), 12 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 ea3eeb7..eebc9c3 100644
--- a/drivers/misc/cxl/api.c
+++ b/drivers/misc/cxl/api.c
@@ -296,6 +296,14 @@ 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)
+{
+ WARN_ON(!ops->event_pending || !ops->deliver_event);
+ 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 a521bc7..64e8e0a 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -24,6 +24,7 @@
#include
#include
+#include
#include
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 2
+#define CXL_API_VERSION 3
#define CXL_API_VERSION_COMPATIBLE 1
/*
@@ -485,6 +486,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 783337d..d1cc297 100644
--- a/drivers/misc/cxl/file.c
+++ b/drivers/misc/cxl/file.c
@@ -295,6 +295,17 @@ int afu_mmap(struct file *file, struct vm_area_struct *vm)
return cxl_context_iomap(ctx, vm);
}
+static inline bool ctx_event_pending(struct cxl_context *ctx)
+{
+ if (ctx->pending_irq || ctx->pending_fault || ctx->pending_afu_err)
+ return true;
+
+ if (ctx->afu_driver_ops)
+ return ctx->afu_driver_ops->event_pending(ctx);
+
+ return false;
+}
+
unsigned int afu_poll(struct file *file, struct poll_table_struct *poll)
{
struct cxl_context *ctx = file->private_data;
@@ -307,8 +318,7 @@ unsigned int afu_poll(struct file *file, struct
poll_table_struct *poll)
pr_de
[PATCH v2 2/2] cxl: add set/get private data to context struct
From: Michael Neuling <mi...@neuling.org>
This provides AFU drivers a means to associate private data with a cxl
context. This is particularly intended for make the new callbacks for
driver specific events easier for AFU drivers to use, as they can easily
get back to any private data structures they may use.
Signed-off-by: Michael Neuling <mi...@neuling.org>
Signed-off-by: Ian Munsie <imun...@au1.ibm.com>
---
No changes since v1
drivers/misc/cxl/api.c | 21 +
drivers/misc/cxl/cxl.h | 3 +++
include/misc/cxl.h | 7 +++
3 files changed, 31 insertions(+)
diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
index eebc9c3..93b270c 100644
--- a/drivers/misc/cxl/api.c
+++ b/drivers/misc/cxl/api.c
@@ -91,6 +91,27 @@ int cxl_release_context(struct cxl_context *ctx)
}
EXPORT_SYMBOL_GPL(cxl_release_context);
+
+int cxl_set_priv(struct cxl_context *ctx, void *priv)
+{
+ if (!ctx)
+ return -EINVAL;
+
+ ctx->priv = priv;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(cxl_set_priv);
+
+void *cxl_get_priv(struct cxl_context *ctx)
+{
+ if (!ctx)
+ return ERR_PTR(-EINVAL);
+
+ return ctx->priv;
+}
+EXPORT_SYMBOL_GPL(cxl_get_priv);
+
int cxl_allocate_afu_irqs(struct cxl_context *ctx, int num)
{
if (num == 0)
diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
index 64e8e0a..71f66e7 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -454,6 +454,9 @@ struct cxl_context {
/* Only used in PR mode */
u64 process_token;
+ /* driver private data */
+ void *priv;
+
unsigned long *irq_bitmap; /* Accessed from IRQ context */
struct cxl_irq_ranges irqs;
struct list_head irq_names;
diff --git a/include/misc/cxl.h b/include/misc/cxl.h
index f198a42..f99b383 100644
--- a/include/misc/cxl.h
+++ b/include/misc/cxl.h
@@ -89,6 +89,13 @@ struct cxl_context *cxl_dev_context_init(struct pci_dev
*dev);
int cxl_release_context(struct cxl_context *ctx);
/*
+ * Set and get private data associated with a context. Allows drivers to have a
+ * back pointer to some useful structure.
+ */
+int cxl_set_priv(struct cxl_context *ctx, void *priv);
+void *cxl_get_priv(struct cxl_context *ctx);
+
+/*
* Allocate AFU interrupts for this context. num=0 will allocate the default
* for this AFU as given in the AFU descriptor. This number doesn't include the
* interrupt 0 (CAIA defines AFU IRQ 0 for page faults). Each interrupt to be
--
2.1.4
[PATCH v2 1/2] cxl: Add mechanism for delivering AFU driver specific events
From: Ian Munsie
This adds an afu_driver_ops structure with event_pending and
deliver_event callbacks. An AFU driver such as cxlflash 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.
Since AFU drivers provide their own means for userspace to obtain the
AFU file descriptor (i.e. cxlflash uses an ioctl on their scsi file
descriptor to obtain the AFU file descriptor) and the generic cxl driver
will never use this event, the ABI of the event is up to each individual
AFU driver.
Signed-off-by: Ian Munsie
---
Changes since v1:
- Rebased on upstream
- Bumped cxl api version to 3
- Addressed comments from mpe:
- Clarified commit message & some comments
- Mentioned 'cxlflash' as a possible user of this event
- Check driver ops on registration and warn if missing calls
- Remove redundant checks where driver ops is used
- Simplified ctx_event_pending and removed underscore version
- Changed deliver_event to take the context as the first argument
drivers/misc/cxl/Kconfig | 5 +
drivers/misc/cxl/api.c | 8
drivers/misc/cxl/cxl.h | 6 +-
drivers/misc/cxl/file.c | 36 +---
include/misc/cxl.h | 29 +
include/uapi/misc/cxl.h | 22 ++
6 files changed, 94 insertions(+), 12 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 ea3eeb7..eebc9c3 100644
--- a/drivers/misc/cxl/api.c
+++ b/drivers/misc/cxl/api.c
@@ -296,6 +296,14 @@ 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)
+{
+ WARN_ON(!ops->event_pending || !ops->deliver_event);
+ 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 a521bc7..64e8e0a 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -24,6 +24,7 @@
#include
#include
+#include
#include
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 2
+#define CXL_API_VERSION 3
#define CXL_API_VERSION_COMPATIBLE 1
/*
@@ -485,6 +486,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 783337d..d1cc297 100644
--- a/drivers/misc/cxl/file.c
+++ b/drivers/misc/cxl/file.c
@@ -295,6 +295,17 @@ int afu_mmap(struct file *file, struct vm_area_struct *vm)
return cxl_context_iomap(ctx, vm);
}
+static inline bool ctx_event_pending(struct cxl_context *ctx)
+{
+ if (ctx->pending_irq || ctx->pending_fault || ctx->pending_afu_err)
+ return true;
+
+ if (ctx->afu_driver_ops)
+ return ctx->afu_driver_ops->event_pending(ctx);
+
+ return false;
+}
+
unsigned int afu_poll(struct file *file, struct poll_table_struct *poll)
{
struct cxl_context *ctx = file->private_data;
@@ -307,8 +318,7 @@ unsigned int afu_poll(struct file *file, struct
poll_table_struct *poll)
pr_devel("afu_poll wait done pe: %i\n"
[PATCH v2 2/2] cxl: add set/get private data to context struct
From: Michael Neuling
This provides AFU drivers a means to associate private data with a cxl
context. This is particularly intended for make the new callbacks for
driver specific events easier for AFU drivers to use, as they can easily
get back to any private data structures they may use.
Signed-off-by: Michael Neuling
Signed-off-by: Ian Munsie
---
No changes since v1
drivers/misc/cxl/api.c | 21 +
drivers/misc/cxl/cxl.h | 3 +++
include/misc/cxl.h | 7 +++
3 files changed, 31 insertions(+)
diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
index eebc9c3..93b270c 100644
--- a/drivers/misc/cxl/api.c
+++ b/drivers/misc/cxl/api.c
@@ -91,6 +91,27 @@ int cxl_release_context(struct cxl_context *ctx)
}
EXPORT_SYMBOL_GPL(cxl_release_context);
+
+int cxl_set_priv(struct cxl_context *ctx, void *priv)
+{
+ if (!ctx)
+ return -EINVAL;
+
+ ctx->priv = priv;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(cxl_set_priv);
+
+void *cxl_get_priv(struct cxl_context *ctx)
+{
+ if (!ctx)
+ return ERR_PTR(-EINVAL);
+
+ return ctx->priv;
+}
+EXPORT_SYMBOL_GPL(cxl_get_priv);
+
int cxl_allocate_afu_irqs(struct cxl_context *ctx, int num)
{
if (num == 0)
diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
index 64e8e0a..71f66e7 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -454,6 +454,9 @@ struct cxl_context {
/* Only used in PR mode */
u64 process_token;
+ /* driver private data */
+ void *priv;
+
unsigned long *irq_bitmap; /* Accessed from IRQ context */
struct cxl_irq_ranges irqs;
struct list_head irq_names;
diff --git a/include/misc/cxl.h b/include/misc/cxl.h
index f198a42..f99b383 100644
--- a/include/misc/cxl.h
+++ b/include/misc/cxl.h
@@ -89,6 +89,13 @@ struct cxl_context *cxl_dev_context_init(struct pci_dev
*dev);
int cxl_release_context(struct cxl_context *ctx);
/*
+ * Set and get private data associated with a context. Allows drivers to have a
+ * back pointer to some useful structure.
+ */
+int cxl_set_priv(struct cxl_context *ctx, void *priv);
+void *cxl_get_priv(struct cxl_context *ctx);
+
+/*
* Allocate AFU interrupts for this context. num=0 will allocate the default
* for this AFU as given in the AFU descriptor. This number doesn't include the
* interrupt 0 (CAIA defines AFU IRQ 0 for page faults). Each interrupt to be
--
2.1.4
Re: [PATCH] cxl: Delete an unnecessary check before the function call "kfree"
Acked-by: Ian Munsie -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] cxl: Delete an unnecessary check before the function call "kfree"
Acked-by: Ian Munsie <imun...@au1.ibm.com> -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] cxl: Fix + cleanup error paths in cxl_dev_context_init
From: Ian Munsie
If the cxl_context_alloc() call fails, we return immediately without
releasing the reference on the AFU device, allowing it to leak.
This patch switches to using goto style error handling so that the
device is released in common code for both error paths, and will also
simplify things if we add additional initialisation in this function in
the future.
Signed-off-by: Ian Munsie
---
drivers/misc/cxl/api.c | 19 ---
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
index f49e3e5..005adc7 100644
--- a/drivers/misc/cxl/api.c
+++ b/drivers/misc/cxl/api.c
@@ -25,19 +25,24 @@ struct cxl_context *cxl_dev_context_init(struct pci_dev
*dev)
get_device(>dev);
ctx = cxl_context_alloc();
- if (IS_ERR(ctx))
- return ctx;
+ if (IS_ERR(ctx)) {
+ rc = PTR_ERR(ctx);
+ goto err_dev;
+ }
/* Make it a slave context. We can promote it later? */
rc = cxl_context_init(ctx, afu, false, NULL);
- if (rc) {
- kfree(ctx);
- put_device(>dev);
- return ERR_PTR(-ENOMEM);
- }
+ if (rc)
+ goto err_ctx;
cxl_assign_psn_space(ctx);
return ctx;
+
+err_ctx:
+ kfree(ctx);
+err_dev:
+ put_device(>dev);
+ return ERR_PTR(rc);
}
EXPORT_SYMBOL_GPL(cxl_dev_context_init);
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] cxl: Fix force unmapping mmaps of contexts allocated through the kernel api
From: Ian Munsie
The cxl user api uses the address_space associated with the file when we
need to force unmap all cxl mmap regions (e.g. on eeh, driver detach,
etc). Currently, contexts allocated through the kernel api do not do
this and instead skip the mmap invalidation, potentially allowing them
to poke at the hardware after such an event, which may cause all sorts
of trouble.
This patch allocates an address_space for cxl contexts allocated through
the kernel api so that the same invalidate path will for these contexts
as well. We don't use the anonymous inode's address_space, as doing so
could invalidate any mmaps of completely unrelated drivers using
anonymous file descriptors.
This patch also introduces a kernelapi flag, so we know when freeing the
context if the address_space was allocated by us and needs to be freed.
Signed-off-by: Ian Munsie
---
drivers/misc/cxl/api.c | 33 ++---
drivers/misc/cxl/context.c | 2 ++
drivers/misc/cxl/cxl.h | 1 +
3 files changed, 33 insertions(+), 3 deletions(-)
diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
index 005adc7..8af12c8 100644
--- a/drivers/misc/cxl/api.c
+++ b/drivers/misc/cxl/api.c
@@ -12,11 +12,13 @@
#include
#include
#include
+#include
#include "cxl.h"
struct cxl_context *cxl_dev_context_init(struct pci_dev *dev)
{
+ struct address_space *mapping;
struct cxl_afu *afu;
struct cxl_context *ctx;
int rc;
@@ -30,14 +32,32 @@ struct cxl_context *cxl_dev_context_init(struct pci_dev
*dev)
goto err_dev;
}
+ ctx->kernelapi = true;
+
+ /*
+* Make our own address space since we won't have one from the
+* filesystem like the user api has, and even if we do associate a file
+* with this context we don't want to use the global anonymous inode's
+* address space as that can invalidate unrelated users:
+*/
+ mapping = kmalloc(sizeof(struct address_space), GFP_KERNEL);
+ if (!mapping) {
+ rc = -ENOMEM;
+ goto err_ctx;
+ }
+ address_space_init_once(mapping);
+
/* Make it a slave context. We can promote it later? */
- rc = cxl_context_init(ctx, afu, false, NULL);
+ rc = cxl_context_init(ctx, afu, false, mapping);
if (rc)
- goto err_ctx;
+ goto err_mapping;
+
cxl_assign_psn_space(ctx);
return ctx;
+err_mapping:
+ kfree(mapping);
err_ctx:
kfree(ctx);
err_dev:
@@ -260,9 +280,16 @@ struct file *cxl_get_fd(struct cxl_context *ctx, struct
file_operations *fops,
file = anon_inode_getfile("cxl", fops, ctx, flags);
if (IS_ERR(file))
- put_unused_fd(fdtmp);
+ goto err_fd;
+
+ file->f_mapping = ctx->mapping;
+
*fd = fdtmp;
return file;
+
+err_fd:
+ put_unused_fd(fdtmp);
+ return NULL;
}
EXPORT_SYMBOL_GPL(cxl_get_fd);
diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c
index 941fda0..e762f85 100644
--- a/drivers/misc/cxl/context.c
+++ b/drivers/misc/cxl/context.c
@@ -272,6 +272,8 @@ static void reclaim_ctx(struct rcu_head *rcu)
if (ctx->ff_page)
__free_page(ctx->ff_page);
ctx->sstp = NULL;
+ if (ctx->kernelapi)
+ kfree(ctx->mapping);
kfree(ctx);
}
diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
index e7af256..d6566c6 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -420,6 +420,7 @@ struct cxl_context {
struct mutex mapping_lock;
struct page *ff_page;
bool mmio_err_ff;
+ bool kernelapi;
spinlock_t sste_lock; /* Protects segment table entries */
struct cxl_sste *sstp;
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] cxl: Fix force unmapping mmaps of contexts allocated through the kernel api
From: Ian Munsie imun...@au1.ibm.com
The cxl user api uses the address_space associated with the file when we
need to force unmap all cxl mmap regions (e.g. on eeh, driver detach,
etc). Currently, contexts allocated through the kernel api do not do
this and instead skip the mmap invalidation, potentially allowing them
to poke at the hardware after such an event, which may cause all sorts
of trouble.
This patch allocates an address_space for cxl contexts allocated through
the kernel api so that the same invalidate path will for these contexts
as well. We don't use the anonymous inode's address_space, as doing so
could invalidate any mmaps of completely unrelated drivers using
anonymous file descriptors.
This patch also introduces a kernelapi flag, so we know when freeing the
context if the address_space was allocated by us and needs to be freed.
Signed-off-by: Ian Munsie imun...@au1.ibm.com
---
drivers/misc/cxl/api.c | 33 ++---
drivers/misc/cxl/context.c | 2 ++
drivers/misc/cxl/cxl.h | 1 +
3 files changed, 33 insertions(+), 3 deletions(-)
diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
index 005adc7..8af12c8 100644
--- a/drivers/misc/cxl/api.c
+++ b/drivers/misc/cxl/api.c
@@ -12,11 +12,13 @@
#include linux/anon_inodes.h
#include linux/file.h
#include misc/cxl.h
+#include linux/fs.h
#include cxl.h
struct cxl_context *cxl_dev_context_init(struct pci_dev *dev)
{
+ struct address_space *mapping;
struct cxl_afu *afu;
struct cxl_context *ctx;
int rc;
@@ -30,14 +32,32 @@ struct cxl_context *cxl_dev_context_init(struct pci_dev
*dev)
goto err_dev;
}
+ ctx-kernelapi = true;
+
+ /*
+* Make our own address space since we won't have one from the
+* filesystem like the user api has, and even if we do associate a file
+* with this context we don't want to use the global anonymous inode's
+* address space as that can invalidate unrelated users:
+*/
+ mapping = kmalloc(sizeof(struct address_space), GFP_KERNEL);
+ if (!mapping) {
+ rc = -ENOMEM;
+ goto err_ctx;
+ }
+ address_space_init_once(mapping);
+
/* Make it a slave context. We can promote it later? */
- rc = cxl_context_init(ctx, afu, false, NULL);
+ rc = cxl_context_init(ctx, afu, false, mapping);
if (rc)
- goto err_ctx;
+ goto err_mapping;
+
cxl_assign_psn_space(ctx);
return ctx;
+err_mapping:
+ kfree(mapping);
err_ctx:
kfree(ctx);
err_dev:
@@ -260,9 +280,16 @@ struct file *cxl_get_fd(struct cxl_context *ctx, struct
file_operations *fops,
file = anon_inode_getfile(cxl, fops, ctx, flags);
if (IS_ERR(file))
- put_unused_fd(fdtmp);
+ goto err_fd;
+
+ file-f_mapping = ctx-mapping;
+
*fd = fdtmp;
return file;
+
+err_fd:
+ put_unused_fd(fdtmp);
+ return NULL;
}
EXPORT_SYMBOL_GPL(cxl_get_fd);
diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c
index 941fda0..e762f85 100644
--- a/drivers/misc/cxl/context.c
+++ b/drivers/misc/cxl/context.c
@@ -272,6 +272,8 @@ static void reclaim_ctx(struct rcu_head *rcu)
if (ctx-ff_page)
__free_page(ctx-ff_page);
ctx-sstp = NULL;
+ if (ctx-kernelapi)
+ kfree(ctx-mapping);
kfree(ctx);
}
diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
index e7af256..d6566c6 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -420,6 +420,7 @@ struct cxl_context {
struct mutex mapping_lock;
struct page *ff_page;
bool mmio_err_ff;
+ bool kernelapi;
spinlock_t sste_lock; /* Protects segment table entries */
struct cxl_sste *sstp;
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] cxl: Fix + cleanup error paths in cxl_dev_context_init
From: Ian Munsie imun...@au1.ibm.com
If the cxl_context_alloc() call fails, we return immediately without
releasing the reference on the AFU device, allowing it to leak.
This patch switches to using goto style error handling so that the
device is released in common code for both error paths, and will also
simplify things if we add additional initialisation in this function in
the future.
Signed-off-by: Ian Munsie imun...@au1.ibm.com
---
drivers/misc/cxl/api.c | 19 ---
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
index f49e3e5..005adc7 100644
--- a/drivers/misc/cxl/api.c
+++ b/drivers/misc/cxl/api.c
@@ -25,19 +25,24 @@ struct cxl_context *cxl_dev_context_init(struct pci_dev
*dev)
get_device(afu-dev);
ctx = cxl_context_alloc();
- if (IS_ERR(ctx))
- return ctx;
+ if (IS_ERR(ctx)) {
+ rc = PTR_ERR(ctx);
+ goto err_dev;
+ }
/* Make it a slave context. We can promote it later? */
rc = cxl_context_init(ctx, afu, false, NULL);
- if (rc) {
- kfree(ctx);
- put_device(afu-dev);
- return ERR_PTR(-ENOMEM);
- }
+ if (rc)
+ goto err_ctx;
cxl_assign_psn_space(ctx);
return ctx;
+
+err_ctx:
+ kfree(ctx);
+err_dev:
+ put_device(afu-dev);
+ return ERR_PTR(rc);
}
EXPORT_SYMBOL_GPL(cxl_dev_context_init);
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] cxl: Add mechanism for delivering AFU driver specific events
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
> >
> > 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 pe
Re: [PATCH 1/2] cxl: Add mechanism for delivering AFU driver specific events
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 imun...@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
Re: [PATCH 2/2] cxl: add set/get private data to context struct
Excerpts from Michael Ellerman's message of 2015-08-19 14:49:30 +1000: > Do we really need the accessors? They don't buy anything I can see over just > using ctx->priv directly. The reasoning there is because we don't currently expose the contents of stuct cxl_context to afu drivers, rather they just treat it as an opaque type. We could potentially change this to expose the details, but there's a lot of junk in there that's just internal details of the cxl driver that isn't of interest to an afu driver that I'd rather not expose. We also already have another accessor function (cxl_process_element) in the api, so it's not out of place. FWIW I'm not opposed to changing how this api works if it ultimately makes things better, but I want to wait until the cxlflash superpipe support is merged so any patches that change the api can change it at the same time. Cheers, -Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] cxl: add set/get private data to context struct
From: Michael Neuling
This provides AFU drivers a means to associate private data with a cxl
context. This is particularly intended for make the new callbacks for
driver specific events easier for AFU drivers to use, as they can easily
get back to any private data structures they may use.
Signed-off-by: Michael Neuling
Signed-off-by: Ian Munsie
---
drivers/misc/cxl/api.c | 21 +
drivers/misc/cxl/cxl.h | 3 +++
include/misc/cxl.h | 7 +++
3 files changed, 31 insertions(+)
diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
index e0f0c78..5f0b22e 100644
--- a/drivers/misc/cxl/api.c
+++ b/drivers/misc/cxl/api.c
@@ -70,6 +70,27 @@ int cxl_release_context(struct cxl_context *ctx)
}
EXPORT_SYMBOL_GPL(cxl_release_context);
+
+int cxl_set_priv(struct cxl_context *ctx, void *priv)
+{
+ if (!ctx)
+ return -EINVAL;
+
+ ctx->priv = priv;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(cxl_set_priv);
+
+void *cxl_get_priv(struct cxl_context *ctx)
+{
+ if (!ctx)
+ return ERR_PTR(-EINVAL);
+
+ return ctx->priv;
+}
+EXPORT_SYMBOL_GPL(cxl_get_priv);
+
int cxl_allocate_afu_irqs(struct cxl_context *ctx, int num)
{
if (num == 0)
diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
index 30e44a8..93db76a 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -431,6 +431,9 @@ struct cxl_context {
/* Only used in PR mode */
u64 process_token;
+ /* driver private data */
+ void *priv;
+
unsigned long *irq_bitmap; /* Accessed from IRQ context */
struct cxl_irq_ranges irqs;
struct list_head irq_names;
diff --git a/include/misc/cxl.h b/include/misc/cxl.h
index 73e03a6..3f5edbe 100644
--- a/include/misc/cxl.h
+++ b/include/misc/cxl.h
@@ -89,6 +89,13 @@ struct cxl_context *cxl_dev_context_init(struct pci_dev
*dev);
int cxl_release_context(struct cxl_context *ctx);
/*
+ * Set and get private data associated with a context. Allows drivers to have a
+ * back pointer to some useful structure.
+ */
+int cxl_set_priv(struct cxl_context *ctx, void *priv);
+void *cxl_get_priv(struct cxl_context *ctx);
+
+/*
* Allocate AFU interrupts for this context. num=0 will allocate the default
* for this AFU as given in the AFU descriptor. This number doesn't include the
* interrupt 0 (CAIA defines AFU IRQ 0 for page faults). Each interrupt to be
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] cxl: Add mechanism for delivering AFU driver specific events
From: Ian Munsie
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
---
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);
+
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
#include
+#include
#include
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(>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,
[PATCH 1/2] cxl: Add mechanism for delivering AFU driver specific events
From: Ian Munsie imun...@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 imun...@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);
+
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
[PATCH 2/2] cxl: add set/get private data to context struct
From: Michael Neuling mi...@neuling.org
This provides AFU drivers a means to associate private data with a cxl
context. This is particularly intended for make the new callbacks for
driver specific events easier for AFU drivers to use, as they can easily
get back to any private data structures they may use.
Signed-off-by: Michael Neuling mi...@neuling.org
Signed-off-by: Ian Munsie imun...@au1.ibm.com
---
drivers/misc/cxl/api.c | 21 +
drivers/misc/cxl/cxl.h | 3 +++
include/misc/cxl.h | 7 +++
3 files changed, 31 insertions(+)
diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
index e0f0c78..5f0b22e 100644
--- a/drivers/misc/cxl/api.c
+++ b/drivers/misc/cxl/api.c
@@ -70,6 +70,27 @@ int cxl_release_context(struct cxl_context *ctx)
}
EXPORT_SYMBOL_GPL(cxl_release_context);
+
+int cxl_set_priv(struct cxl_context *ctx, void *priv)
+{
+ if (!ctx)
+ return -EINVAL;
+
+ ctx-priv = priv;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(cxl_set_priv);
+
+void *cxl_get_priv(struct cxl_context *ctx)
+{
+ if (!ctx)
+ return ERR_PTR(-EINVAL);
+
+ return ctx-priv;
+}
+EXPORT_SYMBOL_GPL(cxl_get_priv);
+
int cxl_allocate_afu_irqs(struct cxl_context *ctx, int num)
{
if (num == 0)
diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
index 30e44a8..93db76a 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -431,6 +431,9 @@ struct cxl_context {
/* Only used in PR mode */
u64 process_token;
+ /* driver private data */
+ void *priv;
+
unsigned long *irq_bitmap; /* Accessed from IRQ context */
struct cxl_irq_ranges irqs;
struct list_head irq_names;
diff --git a/include/misc/cxl.h b/include/misc/cxl.h
index 73e03a6..3f5edbe 100644
--- a/include/misc/cxl.h
+++ b/include/misc/cxl.h
@@ -89,6 +89,13 @@ struct cxl_context *cxl_dev_context_init(struct pci_dev
*dev);
int cxl_release_context(struct cxl_context *ctx);
/*
+ * Set and get private data associated with a context. Allows drivers to have a
+ * back pointer to some useful structure.
+ */
+int cxl_set_priv(struct cxl_context *ctx, void *priv);
+void *cxl_get_priv(struct cxl_context *ctx);
+
+/*
* Allocate AFU interrupts for this context. num=0 will allocate the default
* for this AFU as given in the AFU descriptor. This number doesn't include the
* interrupt 0 (CAIA defines AFU IRQ 0 for page faults). Each interrupt to be
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] cxl: add set/get private data to context struct
Excerpts from Michael Ellerman's message of 2015-08-19 14:49:30 +1000: Do we really need the accessors? They don't buy anything I can see over just using ctx-priv directly. The reasoning there is because we don't currently expose the contents of stuct cxl_context to afu drivers, rather they just treat it as an opaque type. We could potentially change this to expose the details, but there's a lot of junk in there that's just internal details of the cxl driver that isn't of interest to an afu driver that I'd rather not expose. We also already have another accessor function (cxl_process_element) in the api, so it's not out of place. FWIW I'm not opposed to changing how this api works if it ultimately makes things better, but I want to wait until the cxlflash superpipe support is merged so any patches that change the api can change it at the same time. Cheers, -Ian -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] cxl: Add alternate MMIO error handling
From: Ian Munsie
userspace programs using cxl currently have to use two strategies for
dealing with MMIO errors simultaneously. They have to check every read
for a return of all Fs in case the adapter has gone away and the kernel
has not yet noticed, and they have to deal with SIGBUS in case the
kernel has already noticed, invalidated the mapping and marked the
context as failed.
In order to simplify things, this patch adds an alternative approach
where the kernel will return a page filled with Fs instead of delivering
a SIGBUS. This allows userspace to only need to deal with one of these
two error paths, and is intended for use in libraries that use cxl
transparently and may not be able to safely install a signal handler.
This approach will only work if certain constraints are met. Namely, if
the application is both reading and writing to an address in the problem
state area it cannot assume that a non-FF read is OK, as it may just be
reading out a value it has previously written. Further - since only one
page is used per context a write to a given offset would be visible when
reading the same offset from a different page in the mapping (this only
applies within a single context, not between contexts).
An application could deal with this by e.g. making sure it also reads
from a read-only offset after any reads to a read/write offset.
Due to these constraints, this functionality must be explicitly
requested by userspace when starting the context by passing in the
CXL_START_WORK_ERR_FF flag.
Signed-off-by: Ian Munsie
---
drivers/misc/cxl/context.c | 14 ++
drivers/misc/cxl/cxl.h | 4 +++-
drivers/misc/cxl/file.c| 4 +++-
include/uapi/misc/cxl.h| 4 +++-
4 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c
index 1287148..6570f10 100644
--- a/drivers/misc/cxl/context.c
+++ b/drivers/misc/cxl/context.c
@@ -126,6 +126,18 @@ static int cxl_mmap_fault(struct vm_area_struct *vma,
struct vm_fault *vmf)
if (ctx->status != STARTED) {
mutex_unlock(>status_mutex);
pr_devel("%s: Context not started, failing problem state
access\n", __func__);
+ if (ctx->mmio_err_ff) {
+ if (!ctx->ff_page) {
+ ctx->ff_page = alloc_page(GFP_USER);
+ if (!ctx->ff_page)
+ return VM_FAULT_OOM;
+ memset(page_address(ctx->ff_page), 0xff,
PAGE_SIZE);
+ }
+ get_page(ctx->ff_page);
+ vmf->page = ctx->ff_page;
+ vma->vm_page_prot = pgprot_cached(vma->vm_page_prot);
+ return 0;
+ }
return VM_FAULT_SIGBUS;
}
@@ -253,6 +265,8 @@ static void reclaim_ctx(struct rcu_head *rcu)
struct cxl_context *ctx = container_of(rcu, struct cxl_context, rcu);
free_page((u64)ctx->sstp);
+ if (ctx->ff_page)
+ __free_page(ctx->ff_page);
ctx->sstp = NULL;
kfree(ctx);
diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
index 4fd66ca..b7293a4 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -34,7 +34,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
/*
@@ -418,6 +418,8 @@ struct cxl_context {
/* Used to unmap any mmaps when force detaching */
struct address_space *mapping;
struct mutex mapping_lock;
+ struct page *ff_page;
+ bool mmio_err_ff;
spinlock_t sste_lock; /* Protects segment table entries */
struct cxl_sste *sstp;
diff --git a/drivers/misc/cxl/file.c b/drivers/misc/cxl/file.c
index e3f4b69..34c7a5e 100644
--- a/drivers/misc/cxl/file.c
+++ b/drivers/misc/cxl/file.c
@@ -179,6 +179,8 @@ static long afu_ioctl_start_work(struct cxl_context *ctx,
if (work.flags & CXL_START_WORK_AMR)
amr = work.amr & mfspr(SPRN_UAMOR);
+ ctx->mmio_err_ff = !!(work.flags & CXL_START_WORK_ERR_FF);
+
/*
* We grab the PID here and not in the file open to allow for the case
* where a process (master, some daemon, etc) has opened the chardev on
@@ -519,7 +521,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_
[PATCH] cxl: Add alternate MMIO error handling
From: Ian Munsie imun...@au1.ibm.com
userspace programs using cxl currently have to use two strategies for
dealing with MMIO errors simultaneously. They have to check every read
for a return of all Fs in case the adapter has gone away and the kernel
has not yet noticed, and they have to deal with SIGBUS in case the
kernel has already noticed, invalidated the mapping and marked the
context as failed.
In order to simplify things, this patch adds an alternative approach
where the kernel will return a page filled with Fs instead of delivering
a SIGBUS. This allows userspace to only need to deal with one of these
two error paths, and is intended for use in libraries that use cxl
transparently and may not be able to safely install a signal handler.
This approach will only work if certain constraints are met. Namely, if
the application is both reading and writing to an address in the problem
state area it cannot assume that a non-FF read is OK, as it may just be
reading out a value it has previously written. Further - since only one
page is used per context a write to a given offset would be visible when
reading the same offset from a different page in the mapping (this only
applies within a single context, not between contexts).
An application could deal with this by e.g. making sure it also reads
from a read-only offset after any reads to a read/write offset.
Due to these constraints, this functionality must be explicitly
requested by userspace when starting the context by passing in the
CXL_START_WORK_ERR_FF flag.
Signed-off-by: Ian Munsie imun...@au1.ibm.com
---
drivers/misc/cxl/context.c | 14 ++
drivers/misc/cxl/cxl.h | 4 +++-
drivers/misc/cxl/file.c| 4 +++-
include/uapi/misc/cxl.h| 4 +++-
4 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c
index 1287148..6570f10 100644
--- a/drivers/misc/cxl/context.c
+++ b/drivers/misc/cxl/context.c
@@ -126,6 +126,18 @@ static int cxl_mmap_fault(struct vm_area_struct *vma,
struct vm_fault *vmf)
if (ctx-status != STARTED) {
mutex_unlock(ctx-status_mutex);
pr_devel(%s: Context not started, failing problem state
access\n, __func__);
+ if (ctx-mmio_err_ff) {
+ if (!ctx-ff_page) {
+ ctx-ff_page = alloc_page(GFP_USER);
+ if (!ctx-ff_page)
+ return VM_FAULT_OOM;
+ memset(page_address(ctx-ff_page), 0xff,
PAGE_SIZE);
+ }
+ get_page(ctx-ff_page);
+ vmf-page = ctx-ff_page;
+ vma-vm_page_prot = pgprot_cached(vma-vm_page_prot);
+ return 0;
+ }
return VM_FAULT_SIGBUS;
}
@@ -253,6 +265,8 @@ static void reclaim_ctx(struct rcu_head *rcu)
struct cxl_context *ctx = container_of(rcu, struct cxl_context, rcu);
free_page((u64)ctx-sstp);
+ if (ctx-ff_page)
+ __free_page(ctx-ff_page);
ctx-sstp = NULL;
kfree(ctx);
diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
index 4fd66ca..b7293a4 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -34,7 +34,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
/*
@@ -418,6 +418,8 @@ struct cxl_context {
/* Used to unmap any mmaps when force detaching */
struct address_space *mapping;
struct mutex mapping_lock;
+ struct page *ff_page;
+ bool mmio_err_ff;
spinlock_t sste_lock; /* Protects segment table entries */
struct cxl_sste *sstp;
diff --git a/drivers/misc/cxl/file.c b/drivers/misc/cxl/file.c
index e3f4b69..34c7a5e 100644
--- a/drivers/misc/cxl/file.c
+++ b/drivers/misc/cxl/file.c
@@ -179,6 +179,8 @@ static long afu_ioctl_start_work(struct cxl_context *ctx,
if (work.flags CXL_START_WORK_AMR)
amr = work.amr mfspr(SPRN_UAMOR);
+ ctx-mmio_err_ff = !!(work.flags CXL_START_WORK_ERR_FF);
+
/*
* We grab the PID here and not in the file open to allow for the case
* where a process (master, some daemon, etc) has opened the chardev on
@@ -519,7 +521,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/uapi/misc
Re: [PATCH] cxl: Remove use of macro DEFINE_PCI_DEVICE_TABLE
Acked-by: Ian Munsie -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] cxl: Remove use of macro DEFINE_PCI_DEVICE_TABLE
Acked-by: Ian Munsie imun...@au1.ibm.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: BUG: perf error on syscalls for powerpc64.
Excerpts from Sukadev Bhattiprolu's message of 2015-07-17 11:51:04 +1000: > Are you seeing this on big-endian or little-endian system? > > IIRC, I saw the opposite behavior on an LE system a few months ago. > i.e. without 1028ccf5, 'perf listf|grep syscall' failed. > > Applying 1028ccf5, seemed to fix it. You could be on to something there - IIRC the ABI was changed for LE to remove the dot symbols. Might be worth testing on both. Cheers, -Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: BUG: perf error on syscalls for powerpc64.
Excerpts from Sukadev Bhattiprolu's message of 2015-07-17 11:51:04 +1000: Are you seeing this on big-endian or little-endian system? IIRC, I saw the opposite behavior on an LE system a few months ago. i.e. without 1028ccf5, 'perf listf|grep syscall' failed. Applying 1028ccf5, seemed to fix it. You could be on to something there - IIRC the ABI was changed for LE to remove the dot symbols. Might be worth testing on both. Cheers, -Ian -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] cxl: Destroy afu->contexts_idr on release of an afu
Acked-by: Ian Munsie -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] cxl: Destroy afu-contexts_idr on release of an afu
Acked-by: Ian Munsie imun...@au1.ibm.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] cxl: Destroy cxl_adapter_idr on module_exit
Acked-by: Ian Munsie We are probably also missing an idr_destroy(>contexts_idr); in cxl_release_afu() as well if you wanted to send a patch for that. Cheers, -Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] cxl: Destroy cxl_adapter_idr on module_exit
Acked-by: Ian Munsie imun...@au1.ibm.com We are probably also missing an idr_destroy(afu-contexts_idr); in cxl_release_afu() as well if you wanted to send a patch for that. Cheers, -Ian -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] cxl: Fix off by one error allowing subsequent mmap page to be accessed
From: Ian Munsie
It was discovered that if a process mmaped their problem state area they
were able to access one page more than expected, potentially allowing
them to access the problem state area of an unrelated process.
This was due to a simple off by one error in the mmap fault handler
introduced in 0712dc7e73e59d79bcead5d5520acf4e9e917e87 ("cxl: Fix issues
when unmapping contexts"), which is fixed in this patch.
Cc: sta...@vger.kernel.org
Fixes: 0712dc7e73e5 ("cxl: Fix issues when unmapping contexts")
Signed-off-by: Ian Munsie
---
drivers/misc/cxl/context.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c
index 2a4c80a..6c1ce51 100644
--- a/drivers/misc/cxl/context.c
+++ b/drivers/misc/cxl/context.c
@@ -113,11 +113,11 @@ static int cxl_mmap_fault(struct vm_area_struct *vma,
struct vm_fault *vmf)
if (ctx->afu->current_mode == CXL_MODE_DEDICATED) {
area = ctx->afu->psn_phys;
- if (offset > ctx->afu->adapter->ps_size)
+ if (offset >= ctx->afu->adapter->ps_size)
return VM_FAULT_SIGBUS;
} else {
area = ctx->psn_phys;
- if (offset > ctx->psn_size)
+ if (offset >= ctx->psn_size)
return VM_FAULT_SIGBUS;
}
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] cxl: Fail mmap if requested mapping is larger than assigned problem state area
From: Ian Munsie
This patch makes the mmap call fail outright if the requested region is
larger than the problem state area assigned to the context so the error
is reported immediately rather than waiting for an attempt to access an
address out of bounds.
Although we never expect users to map more than the assigned problem
state area and are not aware of anyone doing this (other than for
testing), this does have the potential to break users if someone has
used a larger range regardless. I'm submitting it for consideration, but
if this change is not considered acceptable the previous patch is
sufficient to prevent access out of bounds without breaking anyone.
Signed-off-by: Ian Munsie
---
drivers/misc/cxl/context.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c
index 6c1ce51..1287148 100644
--- a/drivers/misc/cxl/context.c
+++ b/drivers/misc/cxl/context.c
@@ -145,8 +145,16 @@ static const struct vm_operations_struct cxl_mmap_vmops = {
*/
int cxl_context_iomap(struct cxl_context *ctx, struct vm_area_struct *vma)
{
+ u64 start = vma->vm_pgoff << PAGE_SHIFT;
u64 len = vma->vm_end - vma->vm_start;
- len = min(len, ctx->psn_size);
+
+ if (ctx->afu->current_mode == CXL_MODE_DEDICATED) {
+ if (start + len > ctx->afu->adapter->ps_size)
+ return -EINVAL;
+ } else {
+ if (start + len > ctx->psn_size)
+ return -EINVAL;
+ }
if (ctx->afu->current_mode != CXL_MODE_DEDICATED) {
/* make sure there is a valid per process space for this AFU */
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Doc: powerpc: Fix typos in Documentation/powerpc
Acked-by: Ian Munsie -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Doc: powerpc: Fix typos in Documentation/powerpc
Acked-by: Ian Munsie imun...@au1.ibm.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] cxl: Fix off by one error allowing subsequent mmap page to be accessed
From: Ian Munsie imun...@au1.ibm.com
It was discovered that if a process mmaped their problem state area they
were able to access one page more than expected, potentially allowing
them to access the problem state area of an unrelated process.
This was due to a simple off by one error in the mmap fault handler
introduced in 0712dc7e73e59d79bcead5d5520acf4e9e917e87 (cxl: Fix issues
when unmapping contexts), which is fixed in this patch.
Cc: sta...@vger.kernel.org
Fixes: 0712dc7e73e5 (cxl: Fix issues when unmapping contexts)
Signed-off-by: Ian Munsie imun...@au1.ibm.com
---
drivers/misc/cxl/context.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c
index 2a4c80a..6c1ce51 100644
--- a/drivers/misc/cxl/context.c
+++ b/drivers/misc/cxl/context.c
@@ -113,11 +113,11 @@ static int cxl_mmap_fault(struct vm_area_struct *vma,
struct vm_fault *vmf)
if (ctx-afu-current_mode == CXL_MODE_DEDICATED) {
area = ctx-afu-psn_phys;
- if (offset ctx-afu-adapter-ps_size)
+ if (offset = ctx-afu-adapter-ps_size)
return VM_FAULT_SIGBUS;
} else {
area = ctx-psn_phys;
- if (offset ctx-psn_size)
+ if (offset = ctx-psn_size)
return VM_FAULT_SIGBUS;
}
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] cxl: Fail mmap if requested mapping is larger than assigned problem state area
From: Ian Munsie imun...@au1.ibm.com
This patch makes the mmap call fail outright if the requested region is
larger than the problem state area assigned to the context so the error
is reported immediately rather than waiting for an attempt to access an
address out of bounds.
Although we never expect users to map more than the assigned problem
state area and are not aware of anyone doing this (other than for
testing), this does have the potential to break users if someone has
used a larger range regardless. I'm submitting it for consideration, but
if this change is not considered acceptable the previous patch is
sufficient to prevent access out of bounds without breaking anyone.
Signed-off-by: Ian Munsie imun...@au1.ibm.com
---
drivers/misc/cxl/context.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c
index 6c1ce51..1287148 100644
--- a/drivers/misc/cxl/context.c
+++ b/drivers/misc/cxl/context.c
@@ -145,8 +145,16 @@ static const struct vm_operations_struct cxl_mmap_vmops = {
*/
int cxl_context_iomap(struct cxl_context *ctx, struct vm_area_struct *vma)
{
+ u64 start = vma-vm_pgoff PAGE_SHIFT;
u64 len = vma-vm_end - vma-vm_start;
- len = min(len, ctx-psn_size);
+
+ if (ctx-afu-current_mode == CXL_MODE_DEDICATED) {
+ if (start + len ctx-afu-adapter-ps_size)
+ return -EINVAL;
+ } else {
+ if (start + len ctx-psn_size)
+ return -EINVAL;
+ }
if (ctx-afu-current_mode != CXL_MODE_DEDICATED) {
/* make sure there is a valid per process space for this AFU */
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] cxl/vphb.c: Use phb pointer after NULL check
Acked-by: Ian Munsie -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] cxl/vphb.c: Use phb pointer after NULL check
Acked-by: Ian Munsie imun...@au1.ibm.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 2/2] cxl: use more common format specifier
Acked-by: Ian Munsie -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 2/2] cxl: use more common format specifier
Acked-by: Ian Munsie imun...@au1.ibm.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] cxl: Use call_rcu to reduce latency when releasing the afu fd
From: Ian Munsie
The afu fd release path was identified as a significant bottleneck in
the overall performance of cxl. While an optimal AFU design would
minimise the need to close & reopen the AFU fd, it is not always
practical to avoid.
The bottleneck seems to be down to the call to synchronize_rcu(), which
will block until every other thread is guaranteed to be out of an RCU
critical section. Replace it with call_rcu() to free the context
structures later so we can return to the application sooner.
This reduces the time spent in the fd release path from 13356 usec to
13.3 usec - about a 100x speed up.
Reported-by: Fei K Chen
Signed-off-by: Ian Munsie
---
drivers/misc/cxl/context.c | 15 ++-
drivers/misc/cxl/cxl.h | 2 ++
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c
index 22eb338..cea299e 100644
--- a/drivers/misc/cxl/context.c
+++ b/drivers/misc/cxl/context.c
@@ -243,12 +243,9 @@ void cxl_context_detach_all(struct cxl_afu *afu)
mutex_unlock(>contexts_lock);
}
-void cxl_context_free(struct cxl_context *ctx)
+static void reclaim_ctx(struct rcu_head *rcu)
{
- mutex_lock(>afu->contexts_lock);
- idr_remove(>afu->contexts_idr, ctx->pe);
- mutex_unlock(>afu->contexts_lock);
- synchronize_rcu();
+ struct cxl_context *ctx = container_of(rcu, struct cxl_context, rcu);
free_page((u64)ctx->sstp);
ctx->sstp = NULL;
@@ -256,3 +253,11 @@ void cxl_context_free(struct cxl_context *ctx)
put_pid(ctx->pid);
kfree(ctx);
}
+
+void cxl_context_free(struct cxl_context *ctx)
+{
+ mutex_lock(>afu->contexts_lock);
+ idr_remove(>afu->contexts_idr, ctx->pe);
+ mutex_unlock(>afu->contexts_lock);
+ call_rcu(>rcu, reclaim_ctx);
+}
diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
index 47f655f..ebd2e0d 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -460,6 +460,8 @@ struct cxl_context {
bool pending_irq;
bool pending_fault;
bool pending_afu_err;
+
+ struct rcu_head rcu;
};
struct cxl {
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] cxl: Use call_rcu to reduce latency when releasing the afu fd
From: Ian Munsie imun...@au1.ibm.com
The afu fd release path was identified as a significant bottleneck in
the overall performance of cxl. While an optimal AFU design would
minimise the need to close reopen the AFU fd, it is not always
practical to avoid.
The bottleneck seems to be down to the call to synchronize_rcu(), which
will block until every other thread is guaranteed to be out of an RCU
critical section. Replace it with call_rcu() to free the context
structures later so we can return to the application sooner.
This reduces the time spent in the fd release path from 13356 usec to
13.3 usec - about a 100x speed up.
Reported-by: Fei K Chen uc...@cn.ibm.com
Signed-off-by: Ian Munsie imun...@au1.ibm.com
---
drivers/misc/cxl/context.c | 15 ++-
drivers/misc/cxl/cxl.h | 2 ++
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c
index 22eb338..cea299e 100644
--- a/drivers/misc/cxl/context.c
+++ b/drivers/misc/cxl/context.c
@@ -243,12 +243,9 @@ void cxl_context_detach_all(struct cxl_afu *afu)
mutex_unlock(afu-contexts_lock);
}
-void cxl_context_free(struct cxl_context *ctx)
+static void reclaim_ctx(struct rcu_head *rcu)
{
- mutex_lock(ctx-afu-contexts_lock);
- idr_remove(ctx-afu-contexts_idr, ctx-pe);
- mutex_unlock(ctx-afu-contexts_lock);
- synchronize_rcu();
+ struct cxl_context *ctx = container_of(rcu, struct cxl_context, rcu);
free_page((u64)ctx-sstp);
ctx-sstp = NULL;
@@ -256,3 +253,11 @@ void cxl_context_free(struct cxl_context *ctx)
put_pid(ctx-pid);
kfree(ctx);
}
+
+void cxl_context_free(struct cxl_context *ctx)
+{
+ mutex_lock(ctx-afu-contexts_lock);
+ idr_remove(ctx-afu-contexts_idr, ctx-pe);
+ mutex_unlock(ctx-afu-contexts_lock);
+ call_rcu(ctx-rcu, reclaim_ctx);
+}
diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
index 47f655f..ebd2e0d 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -460,6 +460,8 @@ struct cxl_context {
bool pending_irq;
bool pending_fault;
bool pending_afu_err;
+
+ struct rcu_head rcu;
};
struct cxl {
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] cxl: Add explicit precision specifiers
Excerpts from Joe Perches's message of 2015-02-24 01:59:45 +1100: > On Mon, 2015-02-23 at 11:55 +0100, Rasmus Villemoes wrote: > > 24 of the %.16llx > > matches are in drivers/misc/cxl/, so internal consistency wins. > > I think that's more an argument for changing all of the > cx1 uses to "%016llx". I would not object if someone submitted a patch that makes this change across the whole driver. Cheers, -Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] cxl: Add explicit precision specifiers
Acked-by: Ian Munsie -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] cxl: Add explicit precision specifiers
Acked-by: Ian Munsie imun...@au1.ibm.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] cxl: Add explicit precision specifiers
Excerpts from Joe Perches's message of 2015-02-24 01:59:45 +1100: On Mon, 2015-02-23 at 11:55 +0100, Rasmus Villemoes wrote: 24 of the %.16llx matches are in drivers/misc/cxl/, so internal consistency wins. I think that's more an argument for changing all of the cx1 uses to %016llx. I would not object if someone submitted a patch that makes this change across the whole driver. Cheers, -Ian -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] cxl: Remove useless precision specifiers
Excerpts from Rasmus Villemoes's message of 2015-02-21 00:26:22 +1100: > C99 says that a precision given as simply '.' with no following digits > or * should be interpreted as 0. The kernel's printf implementation, > however, treats this case as if the precision was omitted. C99 also > says that if both the precision and value are 0, no digits should be > printed. Even if the kernel followed C99 to the letter, I don't think > that would be particularly useful in these cases, so just remove the > precision specifiers. Nice catch Rasmus, but I think a better patch would be one that adds the missing precision (%.16llx). Cheers, -Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] cxl: Remove useless precision specifiers
Excerpts from Rasmus Villemoes's message of 2015-02-21 00:26:22 +1100: C99 says that a precision given as simply '.' with no following digits or * should be interpreted as 0. The kernel's printf implementation, however, treats this case as if the precision was omitted. C99 also says that if both the precision and value are 0, no digits should be printed. Even if the kernel followed C99 to the letter, I don't think that would be particularly useful in these cases, so just remove the precision specifiers. Nice catch Rasmus, but I think a better patch would be one that adds the missing precision (%.16llx). Cheers, -Ian -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] cxl: Add missing return statement after handling AFU errror
From: Ian Munsie
We were missing a return statement in the PSL interrupt handler in the
case of an AFU error, which would trigger an "Unhandled CXL PSL IRQ"
warning. We do actually handle these type of errors (by notifying
userspace), so add the missing return IRQ_HANDLED so we don't throw
unecessary warnings.
Signed-off-by: Ian Munsie
---
drivers/misc/cxl/irq.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/misc/cxl/irq.c b/drivers/misc/cxl/irq.c
index ef52af7..2a2e58a 100644
--- a/drivers/misc/cxl/irq.c
+++ b/drivers/misc/cxl/irq.c
@@ -170,6 +170,7 @@ static irqreturn_t cxl_irq(int irq, void *data, struct
cxl_irq_info *irq_info)
}
cxl_ack_irq(ctx, CXL_PSL_TFC_An_A, 0);
+ return IRQ_HANDLED;
}
if (dsisr & CXL_PSL_DSISR_An_OC)
pr_devel("CXL interrupt: OS Context Warning\n");
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] cxl: Export optional AFU configuration record in sysfs
From: Ian Munsie
An AFU may optionally contain one or more PCIe like configuration
records, which can be used to identify the AFU.
This patch adds support for exposing the raw config space and the
vendor, device and class code under sysfs. These will appear in a
subdirectory of the AFU device corresponding with the configuration
record number, e.g.
cat /sys/class/cxl/afu0.0/cr0/vendor
0x1014
cat /sys/class/cxl/afu0.0/cr0/device
0x4350
cat /sys/class/cxl/afu0.0/cr0/class
0x12
hexdump -C /sys/class/cxl/afu0.0/cr0/config
14 10 50 43 00 00 00 00 06 00 00 12 00 00 00 00 |..PC|
0010 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ||
*
0100
These files behave in much the same way as the equivalent files for PCI
devices, with one exception being that the config file is currently
read-only and restricted to the root user. It is not necessarily
required to be this strict, but we currently do not have a compelling
use-case to make it writable and/or world-readable, so I erred on the
side of being restrictive.
Signed-off-by: Ian Munsie
---
Documentation/ABI/testing/sysfs-class-cxl | 37 ++
drivers/misc/cxl/cxl.h| 13 +++
drivers/misc/cxl/pci.c| 23
drivers/misc/cxl/sysfs.c | 179 --
4 files changed, 242 insertions(+), 10 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-class-cxl
b/Documentation/ABI/testing/sysfs-class-cxl
index db31176..23aad38 100644
--- a/Documentation/ABI/testing/sysfs-class-cxl
+++ b/Documentation/ABI/testing/sysfs-class-cxl
@@ -76,6 +76,43 @@ Description:read only
+AFU configuration records (eg. /sys/class/cxl/afu0.0/cr0):
+
+An AFU may optionally export one or more PCIe like configuration records, known
+as AFU configuration records, which will show up here (if present).
+
+What: /sys/class/cxl//cr/vendor
+Date: February 2015
+Contact:linuxppc-...@lists.ozlabs.org
+Description:read only
+ Hexadecimal value of the vendor ID found in this AFU
+ configuration record.
+
+What: /sys/class/cxl//cr/device
+Date: February 2015
+Contact:linuxppc-...@lists.ozlabs.org
+Description:read only
+ Hexadecimal value of the device ID found in this AFU
+ configuration record.
+
+What: /sys/class/cxl//cr/vendor
+Date: February 2015
+Contact:linuxppc-...@lists.ozlabs.org
+Description:read only
+ Hexadecimal value of the class code found in this AFU
+ configuration record.
+
+What: /sys/class/cxl//cr/config
+Date: February 2015
+Contact:linuxppc-...@lists.ozlabs.org
+Description:read only
+ This binary file provides raw access to the AFU configuration
+ record. The format is expected to match the either the standard
+ or extended configuration space defined by the PCIe
+ specification.
+
+
+
Master contexts (eg. /sys/class/cxl/afu0.0m)
What: /sys/class/cxl/m/mmio_size
diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
index 6a6a487..a1cee47 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -382,6 +382,10 @@ struct cxl_afu {
int slice;
int modes_supported;
int current_mode;
+ int crs_num;
+ u64 crs_len;
+ u64 crs_offset;
+ struct list_head crs;
enum prefault_modes prefault_mode;
bool psa;
bool pp_psa;
@@ -551,6 +555,15 @@ static inline void __iomem *_cxl_p2n_addr(struct cxl_afu
*afu, cxl_p2n_reg_t reg
#define cxl_p2n_read(afu, reg) \
in_be64(_cxl_p2n_addr(afu, reg))
+
+#define cxl_afu_cr_read64(afu, cr, off) \
+ in_le64((afu)->afu_desc_mmio + (afu)->crs_offset + ((cr) *
(afu)->crs_len) + (off))
+#define cxl_afu_cr_read32(afu, cr, off) \
+ in_le32((afu)->afu_desc_mmio + (afu)->crs_offset + ((cr) *
(afu)->crs_len) + (off))
+u16 cxl_afu_cr_read16(struct cxl_afu *afu, int cr, u64 off);
+u8 cxl_afu_cr_read8(struct cxl_afu *afu, int cr, u64 off);
+
+
struct cxl_calls {
void (*cxl_slbia)(struct mm_struct *mm);
struct module *owner;
diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index 5137ee5..128481e 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -114,6 +114,24 @@
#define AFUD_EB_LEN(val) EXTRACT_PPC_BITS(val, 8, 63)
#define AFUD_READ_EB_OFF(afu) AFUD_READ(afu, 0x48)
+u16 cxl_afu_cr_read16(struct cxl_afu *afu, int cr, u64 off)
+{
+ u64 aligned_off = off & ~0x3L;
+ u32 val;
+
+ val = cxl_afu_cr_read32(afu, cr, aligned_off);
+ return (val >> ((off & 0x2) * 8)) & 0x;
+}
+
+u8 cxl_afu_cr_read8(struct cxl_afu *afu, int cr, u64 off)
+{
+ u64 aligned_off = off & ~0x3L;
+ u32 val;
+
+ val
[PATCH 2/2] cxl: Fail AFU initialisation if an invalid configuration record is found
From: Ian Munsie
If an AFU claims to have a configuration record but doesn't actually
contain a vendor and device ID, fail the AFU initialisation. Right now
this is just a way of politely letting AFU developers know that they
need to fix their config space, but later on we may expose the AFUs as
actual PCI devices in their own right and don't want to inadvertendly
expose an AFU with a bad config space.
Signed-off-by: Ian Munsie
---
drivers/misc/cxl/pci.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index 128481e..f7c01a9 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -599,6 +599,8 @@ static int cxl_read_afu_descriptor(struct cxl_afu *afu)
static int cxl_afu_descriptor_looks_ok(struct cxl_afu *afu)
{
+ int i;
+
if (afu->psa && afu->adapter->ps_size <
(afu->pp_offset +
afu->pp_size*afu->max_procs_virtualised)) {
dev_err(>dev, "per-process PSA can't fit inside the
PSA!\n");
@@ -608,6 +610,13 @@ static int cxl_afu_descriptor_looks_ok(struct cxl_afu *afu)
if (afu->pp_psa && (afu->pp_size < PAGE_SIZE))
dev_warn(>dev, "AFU uses < PAGE_SIZE per-process PSA!");
+ for (i = 0; i < afu->crs_num; i++) {
+ if ((cxl_afu_cr_read32(afu, i, 0) == 0)) {
+ dev_err(>dev, "ABORTING: AFU configuration record
%i is invalid\n", i);
+ return -EINVAL;
+ }
+ }
+
return 0;
}
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] cxl: Fail AFU initialisation if an invalid configuration record is found
From: Ian Munsie imun...@au1.ibm.com
If an AFU claims to have a configuration record but doesn't actually
contain a vendor and device ID, fail the AFU initialisation. Right now
this is just a way of politely letting AFU developers know that they
need to fix their config space, but later on we may expose the AFUs as
actual PCI devices in their own right and don't want to inadvertendly
expose an AFU with a bad config space.
Signed-off-by: Ian Munsie imun...@au1.ibm.com
---
drivers/misc/cxl/pci.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index 128481e..f7c01a9 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -599,6 +599,8 @@ static int cxl_read_afu_descriptor(struct cxl_afu *afu)
static int cxl_afu_descriptor_looks_ok(struct cxl_afu *afu)
{
+ int i;
+
if (afu-psa afu-adapter-ps_size
(afu-pp_offset +
afu-pp_size*afu-max_procs_virtualised)) {
dev_err(afu-dev, per-process PSA can't fit inside the
PSA!\n);
@@ -608,6 +610,13 @@ static int cxl_afu_descriptor_looks_ok(struct cxl_afu *afu)
if (afu-pp_psa (afu-pp_size PAGE_SIZE))
dev_warn(afu-dev, AFU uses PAGE_SIZE per-process PSA!);
+ for (i = 0; i afu-crs_num; i++) {
+ if ((cxl_afu_cr_read32(afu, i, 0) == 0)) {
+ dev_err(afu-dev, ABORTING: AFU configuration record
%i is invalid\n, i);
+ return -EINVAL;
+ }
+ }
+
return 0;
}
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] cxl: Export optional AFU configuration record in sysfs
From: Ian Munsie imun...@au1.ibm.com
An AFU may optionally contain one or more PCIe like configuration
records, which can be used to identify the AFU.
This patch adds support for exposing the raw config space and the
vendor, device and class code under sysfs. These will appear in a
subdirectory of the AFU device corresponding with the configuration
record number, e.g.
cat /sys/class/cxl/afu0.0/cr0/vendor
0x1014
cat /sys/class/cxl/afu0.0/cr0/device
0x4350
cat /sys/class/cxl/afu0.0/cr0/class
0x12
hexdump -C /sys/class/cxl/afu0.0/cr0/config
14 10 50 43 00 00 00 00 06 00 00 12 00 00 00 00 |..PC|
0010 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ||
*
0100
These files behave in much the same way as the equivalent files for PCI
devices, with one exception being that the config file is currently
read-only and restricted to the root user. It is not necessarily
required to be this strict, but we currently do not have a compelling
use-case to make it writable and/or world-readable, so I erred on the
side of being restrictive.
Signed-off-by: Ian Munsie imun...@au1.ibm.com
---
Documentation/ABI/testing/sysfs-class-cxl | 37 ++
drivers/misc/cxl/cxl.h| 13 +++
drivers/misc/cxl/pci.c| 23
drivers/misc/cxl/sysfs.c | 179 --
4 files changed, 242 insertions(+), 10 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-class-cxl
b/Documentation/ABI/testing/sysfs-class-cxl
index db31176..23aad38 100644
--- a/Documentation/ABI/testing/sysfs-class-cxl
+++ b/Documentation/ABI/testing/sysfs-class-cxl
@@ -76,6 +76,43 @@ Description:read only
+AFU configuration records (eg. /sys/class/cxl/afu0.0/cr0):
+
+An AFU may optionally export one or more PCIe like configuration records, known
+as AFU configuration records, which will show up here (if present).
+
+What: /sys/class/cxl/afu/crconfig num/vendor
+Date: February 2015
+Contact:linuxppc-...@lists.ozlabs.org
+Description:read only
+ Hexadecimal value of the vendor ID found in this AFU
+ configuration record.
+
+What: /sys/class/cxl/afu/crconfig num/device
+Date: February 2015
+Contact:linuxppc-...@lists.ozlabs.org
+Description:read only
+ Hexadecimal value of the device ID found in this AFU
+ configuration record.
+
+What: /sys/class/cxl/afu/crconfig num/vendor
+Date: February 2015
+Contact:linuxppc-...@lists.ozlabs.org
+Description:read only
+ Hexadecimal value of the class code found in this AFU
+ configuration record.
+
+What: /sys/class/cxl/afu/crconfig num/config
+Date: February 2015
+Contact:linuxppc-...@lists.ozlabs.org
+Description:read only
+ This binary file provides raw access to the AFU configuration
+ record. The format is expected to match the either the standard
+ or extended configuration space defined by the PCIe
+ specification.
+
+
+
Master contexts (eg. /sys/class/cxl/afu0.0m)
What: /sys/class/cxl/afum/mmio_size
diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
index 6a6a487..a1cee47 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -382,6 +382,10 @@ struct cxl_afu {
int slice;
int modes_supported;
int current_mode;
+ int crs_num;
+ u64 crs_len;
+ u64 crs_offset;
+ struct list_head crs;
enum prefault_modes prefault_mode;
bool psa;
bool pp_psa;
@@ -551,6 +555,15 @@ static inline void __iomem *_cxl_p2n_addr(struct cxl_afu
*afu, cxl_p2n_reg_t reg
#define cxl_p2n_read(afu, reg) \
in_be64(_cxl_p2n_addr(afu, reg))
+
+#define cxl_afu_cr_read64(afu, cr, off) \
+ in_le64((afu)-afu_desc_mmio + (afu)-crs_offset + ((cr) *
(afu)-crs_len) + (off))
+#define cxl_afu_cr_read32(afu, cr, off) \
+ in_le32((afu)-afu_desc_mmio + (afu)-crs_offset + ((cr) *
(afu)-crs_len) + (off))
+u16 cxl_afu_cr_read16(struct cxl_afu *afu, int cr, u64 off);
+u8 cxl_afu_cr_read8(struct cxl_afu *afu, int cr, u64 off);
+
+
struct cxl_calls {
void (*cxl_slbia)(struct mm_struct *mm);
struct module *owner;
diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index 5137ee5..128481e 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -114,6 +114,24 @@
#define AFUD_EB_LEN(val) EXTRACT_PPC_BITS(val, 8, 63)
#define AFUD_READ_EB_OFF(afu) AFUD_READ(afu, 0x48)
+u16 cxl_afu_cr_read16(struct cxl_afu *afu, int cr, u64 off)
+{
+ u64 aligned_off = off ~0x3L;
+ u32 val;
+
+ val = cxl_afu_cr_read32(afu, cr, aligned_off);
+ return (val ((off 0x2) * 8)) 0x;
+}
+
+u8 cxl_afu_cr_read8(struct cxl_afu *afu, int cr, u64 off)
+{
+ u64 aligned_off
[PATCH] cxl: Add missing return statement after handling AFU errror
From: Ian Munsie imun...@au1.ibm.com We were missing a return statement in the PSL interrupt handler in the case of an AFU error, which would trigger an Unhandled CXL PSL IRQ warning. We do actually handle these type of errors (by notifying userspace), so add the missing return IRQ_HANDLED so we don't throw unecessary warnings. Signed-off-by: Ian Munsie imun...@au1.ibm.com --- drivers/misc/cxl/irq.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/misc/cxl/irq.c b/drivers/misc/cxl/irq.c index ef52af7..2a2e58a 100644 --- a/drivers/misc/cxl/irq.c +++ b/drivers/misc/cxl/irq.c @@ -170,6 +170,7 @@ static irqreturn_t cxl_irq(int irq, void *data, struct cxl_irq_info *irq_info) } cxl_ack_irq(ctx, CXL_PSL_TFC_An_A, 0); + return IRQ_HANDLED; } if (dsisr CXL_PSL_DSISR_An_OC) pr_devel(CXL interrupt: OS Context Warning\n); -- 2.1.4 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] cxl: Add tracepoints
From: Ian Munsie
This patch adds tracepoints throughout the cxl driver, which can provide
insight into:
- Context lifetimes
- Commands sent to the PSL and AFU and their completion status
- Segment and page table misses and their resolution
- PSL and AFU interrupts
- slbia calls from the powerpc copro_fault code
These tracepoints are mostly intended to aid in debugging (particularly
for new AFU designs), and may be useful standalone or in conjunction
with hardware traces collected by the PSL (read out via the trace
interface in debugfs) and AFUs.
Signed-off-by: Ian Munsie
---
drivers/misc/cxl/Makefile | 5 +-
drivers/misc/cxl/fault.c | 5 +
drivers/misc/cxl/file.c | 3 +
drivers/misc/cxl/irq.c| 4 +
drivers/misc/cxl/main.c | 2 +
drivers/misc/cxl/native.c | 38 +++-
drivers/misc/cxl/trace.c | 13 ++
drivers/misc/cxl/trace.h | 459 ++
8 files changed, 520 insertions(+), 9 deletions(-)
create mode 100644 drivers/misc/cxl/trace.c
create mode 100644 drivers/misc/cxl/trace.h
diff --git a/drivers/misc/cxl/Makefile b/drivers/misc/cxl/Makefile
index 165e98f..edb494d 100644
--- a/drivers/misc/cxl/Makefile
+++ b/drivers/misc/cxl/Makefile
@@ -1,3 +1,6 @@
-cxl-y += main.o file.o irq.o fault.o native.o
context.o sysfs.o debugfs.o pci.o
+cxl-y += main.o file.o irq.o fault.o native.o
context.o sysfs.o debugfs.o pci.o trace.o
obj-$(CONFIG_CXL) += cxl.o
obj-$(CONFIG_CXL_BASE) += base.o
+
+# For tracepoints to include our trace.h from tracepoint infrastructure:
+CFLAGS_trace.o := -I$(src)
diff --git a/drivers/misc/cxl/fault.c b/drivers/misc/cxl/fault.c
index e010302..5286b8b 100644
--- a/drivers/misc/cxl/fault.c
+++ b/drivers/misc/cxl/fault.c
@@ -20,6 +20,7 @@
#include
#include "cxl.h"
+#include "trace.h"
static bool sste_matches(struct cxl_sste *sste, struct copro_slb *slb)
{
@@ -75,6 +76,7 @@ static void cxl_load_segment(struct cxl_context *ctx, struct
copro_slb *slb)
pr_devel("CXL Populating SST[%li]: %#llx %#llx\n",
sste - ctx->sstp, slb->vsid, slb->esid);
+ trace_cxl_ste_write(ctx, sste - ctx->sstp, slb->esid, slb->vsid);
sste->vsid_data = cpu_to_be64(slb->vsid);
sste->esid_data = cpu_to_be64(slb->esid);
@@ -116,6 +118,7 @@ static int cxl_handle_segment_miss(struct cxl_context *ctx,
int rc;
pr_devel("CXL interrupt: Segment fault pe: %i ea: %#llx\n", ctx->pe,
ea);
+ trace_cxl_ste_miss(ctx, ea);
if ((rc = cxl_fault_segment(ctx, mm, ea)))
cxl_ack_ae(ctx);
@@ -135,6 +138,8 @@ static void cxl_handle_page_fault(struct cxl_context *ctx,
int result;
unsigned long access, flags, inv_flags = 0;
+ trace_cxl_pte_miss(ctx, dsisr, dar);
+
if ((result = copro_handle_mm_fault(mm, dar, dsisr, ))) {
pr_devel("copro_handle_mm_fault failed: %#x\n", result);
return cxl_ack_ae(ctx);
diff --git a/drivers/misc/cxl/file.c b/drivers/misc/cxl/file.c
index 4e85028..2364bca 100644
--- a/drivers/misc/cxl/file.c
+++ b/drivers/misc/cxl/file.c
@@ -23,6 +23,7 @@
#include
#include "cxl.h"
+#include "trace.h"
#define CXL_NUM_MINORS 256 /* Total to reserve */
#define CXL_DEV_MINORS 13 /* 1 control + 4 AFUs * 3
(dedicated/master/shared) */
@@ -186,6 +187,8 @@ static long afu_ioctl_start_work(struct cxl_context *ctx,
*/
ctx->pid = get_pid(get_task_pid(current, PIDTYPE_PID));
+ trace_cxl_attach(ctx, work.work_element_descriptor,
work.num_interrupts, amr);
+
if ((rc = cxl_attach_process(ctx, false, work.work_element_descriptor,
amr))) {
afu_release_irqs(ctx);
diff --git a/drivers/misc/cxl/irq.c b/drivers/misc/cxl/irq.c
index c294925..ef52af7 100644
--- a/drivers/misc/cxl/irq.c
+++ b/drivers/misc/cxl/irq.c
@@ -17,6 +17,7 @@
#include
#include "cxl.h"
+#include "trace.h"
/* XXX: This is implementation specific */
static irqreturn_t handle_psl_slice_error(struct cxl_context *ctx, u64 dsisr,
u64 errstat)
@@ -100,6 +101,8 @@ static irqreturn_t cxl_irq(int irq, void *data, struct
cxl_irq_info *irq_info)
dsisr = irq_info->dsisr;
dar = irq_info->dar;
+ trace_cxl_psl_irq(ctx, irq, dsisr, dar);
+
pr_devel("CXL interrupt %i for afu pe: %i DSISR: %#llx DAR: %#llx\n",
irq, ctx->pe, dsisr, dar);
if (dsisr & CXL_PSL_DSISR_An_DS) {
@@ -237,6 +240,7 @@ static irqreturn_t cxl_irq_afu(int irq, void *data)
return IRQ_HANDLED;
}
+ trace_cxl_afu_irq(ctx, afu_irq, irq, hwirq);
pr_devel("Received AFU interrupt %i for pe: %i (virq %i hwirq %lx)\n",
afu_irq, ctx->pe, irq, hwirq);
diff
[PATCH] cxl: Add tracepoints
From: Ian Munsie imun...@au1.ibm.com
This patch adds tracepoints throughout the cxl driver, which can provide
insight into:
- Context lifetimes
- Commands sent to the PSL and AFU and their completion status
- Segment and page table misses and their resolution
- PSL and AFU interrupts
- slbia calls from the powerpc copro_fault code
These tracepoints are mostly intended to aid in debugging (particularly
for new AFU designs), and may be useful standalone or in conjunction
with hardware traces collected by the PSL (read out via the trace
interface in debugfs) and AFUs.
Signed-off-by: Ian Munsie imun...@au1.ibm.com
---
drivers/misc/cxl/Makefile | 5 +-
drivers/misc/cxl/fault.c | 5 +
drivers/misc/cxl/file.c | 3 +
drivers/misc/cxl/irq.c| 4 +
drivers/misc/cxl/main.c | 2 +
drivers/misc/cxl/native.c | 38 +++-
drivers/misc/cxl/trace.c | 13 ++
drivers/misc/cxl/trace.h | 459 ++
8 files changed, 520 insertions(+), 9 deletions(-)
create mode 100644 drivers/misc/cxl/trace.c
create mode 100644 drivers/misc/cxl/trace.h
diff --git a/drivers/misc/cxl/Makefile b/drivers/misc/cxl/Makefile
index 165e98f..edb494d 100644
--- a/drivers/misc/cxl/Makefile
+++ b/drivers/misc/cxl/Makefile
@@ -1,3 +1,6 @@
-cxl-y += main.o file.o irq.o fault.o native.o
context.o sysfs.o debugfs.o pci.o
+cxl-y += main.o file.o irq.o fault.o native.o
context.o sysfs.o debugfs.o pci.o trace.o
obj-$(CONFIG_CXL) += cxl.o
obj-$(CONFIG_CXL_BASE) += base.o
+
+# For tracepoints to include our trace.h from tracepoint infrastructure:
+CFLAGS_trace.o := -I$(src)
diff --git a/drivers/misc/cxl/fault.c b/drivers/misc/cxl/fault.c
index e010302..5286b8b 100644
--- a/drivers/misc/cxl/fault.c
+++ b/drivers/misc/cxl/fault.c
@@ -20,6 +20,7 @@
#include asm/mmu.h
#include cxl.h
+#include trace.h
static bool sste_matches(struct cxl_sste *sste, struct copro_slb *slb)
{
@@ -75,6 +76,7 @@ static void cxl_load_segment(struct cxl_context *ctx, struct
copro_slb *slb)
pr_devel(CXL Populating SST[%li]: %#llx %#llx\n,
sste - ctx-sstp, slb-vsid, slb-esid);
+ trace_cxl_ste_write(ctx, sste - ctx-sstp, slb-esid, slb-vsid);
sste-vsid_data = cpu_to_be64(slb-vsid);
sste-esid_data = cpu_to_be64(slb-esid);
@@ -116,6 +118,7 @@ static int cxl_handle_segment_miss(struct cxl_context *ctx,
int rc;
pr_devel(CXL interrupt: Segment fault pe: %i ea: %#llx\n, ctx-pe,
ea);
+ trace_cxl_ste_miss(ctx, ea);
if ((rc = cxl_fault_segment(ctx, mm, ea)))
cxl_ack_ae(ctx);
@@ -135,6 +138,8 @@ static void cxl_handle_page_fault(struct cxl_context *ctx,
int result;
unsigned long access, flags, inv_flags = 0;
+ trace_cxl_pte_miss(ctx, dsisr, dar);
+
if ((result = copro_handle_mm_fault(mm, dar, dsisr, flt))) {
pr_devel(copro_handle_mm_fault failed: %#x\n, result);
return cxl_ack_ae(ctx);
diff --git a/drivers/misc/cxl/file.c b/drivers/misc/cxl/file.c
index 4e85028..2364bca 100644
--- a/drivers/misc/cxl/file.c
+++ b/drivers/misc/cxl/file.c
@@ -23,6 +23,7 @@
#include asm/copro.h
#include cxl.h
+#include trace.h
#define CXL_NUM_MINORS 256 /* Total to reserve */
#define CXL_DEV_MINORS 13 /* 1 control + 4 AFUs * 3
(dedicated/master/shared) */
@@ -186,6 +187,8 @@ static long afu_ioctl_start_work(struct cxl_context *ctx,
*/
ctx-pid = get_pid(get_task_pid(current, PIDTYPE_PID));
+ trace_cxl_attach(ctx, work.work_element_descriptor,
work.num_interrupts, amr);
+
if ((rc = cxl_attach_process(ctx, false, work.work_element_descriptor,
amr))) {
afu_release_irqs(ctx);
diff --git a/drivers/misc/cxl/irq.c b/drivers/misc/cxl/irq.c
index c294925..ef52af7 100644
--- a/drivers/misc/cxl/irq.c
+++ b/drivers/misc/cxl/irq.c
@@ -17,6 +17,7 @@
#include misc/cxl.h
#include cxl.h
+#include trace.h
/* XXX: This is implementation specific */
static irqreturn_t handle_psl_slice_error(struct cxl_context *ctx, u64 dsisr,
u64 errstat)
@@ -100,6 +101,8 @@ static irqreturn_t cxl_irq(int irq, void *data, struct
cxl_irq_info *irq_info)
dsisr = irq_info-dsisr;
dar = irq_info-dar;
+ trace_cxl_psl_irq(ctx, irq, dsisr, dar);
+
pr_devel(CXL interrupt %i for afu pe: %i DSISR: %#llx DAR: %#llx\n,
irq, ctx-pe, dsisr, dar);
if (dsisr CXL_PSL_DSISR_An_DS) {
@@ -237,6 +240,7 @@ static irqreturn_t cxl_irq_afu(int irq, void *data)
return IRQ_HANDLED;
}
+ trace_cxl_afu_irq(ctx, afu_irq, irq, hwirq);
pr_devel(Received AFU interrupt %i for pe: %i (virq %i hwirq %lx)\n,
afu_irq, ctx-pe, irq, hwirq);
diff --git a/drivers/misc/cxl/main.c b/drivers/misc/cxl/main.c
index 4cde9b6..8ccddce 100644
--- a/drivers/misc/cxl/main.c
+++ b/drivers/misc
Re: [PATCH] cxl: remove redundant increment of hwirq
Acked-By: Ian Munsie -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] cxl: remove redundant increment of hwirq
Acked-By: Ian Munsie imun...@au1.ibm.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] CXL: Fix device_node reference counting
From: Ryan Grimm When unbinding and rebinding the driver on a system with a card in PHB0, this error condition is reached after a few attempts: ERROR: Bad of_node_put() on /pciex@3fffe4000 CPU: 0 PID: 3040 Comm: bash Not tainted 3.18.0-rc3-12545-g3627ffe #152 Call Trace: [c00721acb5c0] [c086ef94] .dump_stack+0x84/0xb0 (unreliable) [c00721acb640] [c073a0a8] .of_node_release+0xd8/0xe0 [c00721acb6d0] [c044bc44] .kobject_release+0x74/0xe0 [c00721acb760] [c07394fc] .of_node_put+0x1c/0x30 [c00721acb7d0] [c0545cd8] .cxl_probe+0x1a98/0x1d50 [c00721acb900] [c04845a0] .local_pci_probe+0x40/0xc0 [c00721acb980] [c0484998] .pci_device_probe+0x128/0x170 [c00721acba30] [c052400c] .driver_probe_device+0xac/0x2a0 [c00721acbad0] [c0522468] .bind_store+0x108/0x160 [c00721acbb70] [c0521448] .drv_attr_store+0x38/0x60 [c00721acbbe0] [c0293840] .sysfs_kf_write+0x60/0xa0 [c00721acbc50] [c0292500] .kernfs_fop_write+0x140/0x1d0 [c00721acbcf0] [c0208648] .vfs_write+0xd8/0x260 [c00721acbd90] [c0208b18] .SyS_write+0x58/0x100 [c00721acbe30] [c0009258] syscall_exit+0x0/0x98 of_get_next_parent decrements parent's refcount and we need to call of_node_put after the iteration. But, if while loop is not entered, of_node_put get called on np without an of_node_get. So, call it before the while loop. Signed-off-by: Ryan Grimm Signed-off-by: Ian Munsie --- drivers/misc/cxl/pci.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c index 2ccd0a9..f801c28 100644 --- a/drivers/misc/cxl/pci.c +++ b/drivers/misc/cxl/pci.c @@ -319,6 +319,7 @@ static int init_implementation_adapter_regs(struct cxl *adapter, struct pci_dev if (!(np = pnv_pci_to_phb_node(dev))) return -ENODEV; + of_node_get(np); while (np && !(prop = of_get_property(np, "ibm,chip-id", NULL))) np = of_get_next_parent(np); if (!np) -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] cxl: Fix issues when unmapping contexts
From: Ian Munsie
An issue was introduced with "cxl: Unmap MMIO regions when detaching a
context" (b123429e6a9e8d03aacf888d23262835f0081448) where closing a
context normally could also unmap the problem state area of other
contexts currently using the AFU.
It was also discovered that after a context's MMIO space had been
unmapped it would read 0s when accessing it, whereas the expected
behaviour was for the access to fail altogether.
In order to address these issues, this patch does two things:
- Forced mmap unmapping is only done when we are forcefully detaching
all contexts, and not in the normal detach path. Since the normal
context close path is tied to the file release any mmaps must have
already been released so we don't need to worry in that case.
- The mmap path now uses a vm_operations_struct with a fault handler.
The fault handler ensures that the context is in started state,
otherwise it fails the access attempt with a SIGBUS.
Signed-off-by: Ian Munsie
---
drivers/misc/cxl/context.c | 82 +++---
drivers/misc/cxl/file.c| 14
2 files changed, 71 insertions(+), 25 deletions(-)
diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c
index 51fd6b5..d1b55fe 100644
--- a/drivers/misc/cxl/context.c
+++ b/drivers/misc/cxl/context.c
@@ -100,6 +100,46 @@ int cxl_context_init(struct cxl_context *ctx, struct
cxl_afu *afu, bool master,
return 0;
}
+static int cxl_mmap_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
+{
+ struct cxl_context *ctx = vma->vm_file->private_data;
+ unsigned long address = (unsigned long)vmf->virtual_address;
+ u64 area, offset;
+
+ offset = vmf->pgoff << PAGE_SHIFT;
+
+ pr_devel("%s: pe: %i address: 0x%lx offset: 0x%llx\n",
+ __func__, ctx->pe, address, offset);
+
+ if (ctx->afu->current_mode == CXL_MODE_DEDICATED) {
+ area = ctx->afu->psn_phys;
+ if (offset > ctx->afu->adapter->ps_size)
+ return VM_FAULT_SIGBUS;
+ } else {
+ area = ctx->psn_phys;
+ if (offset > ctx->psn_size)
+ return VM_FAULT_SIGBUS;
+ }
+
+ mutex_lock(>status_mutex);
+
+ if (ctx->status != STARTED) {
+ mutex_unlock(>status_mutex);
+ pr_devel("%s: Context not started, failing problem state
access\n", __func__);
+ return VM_FAULT_SIGBUS;
+ }
+
+ vm_insert_pfn(vma, address, (area + offset) >> PAGE_SHIFT);
+
+ mutex_unlock(>status_mutex);
+
+ return VM_FAULT_NOPAGE;
+}
+
+static const struct vm_operations_struct cxl_mmap_vmops = {
+ .fault = cxl_mmap_fault,
+};
+
/*
* Map a per-context mmio space into the given vma.
*/
@@ -108,26 +148,25 @@ int cxl_context_iomap(struct cxl_context *ctx, struct
vm_area_struct *vma)
u64 len = vma->vm_end - vma->vm_start;
len = min(len, ctx->psn_size);
- if (ctx->afu->current_mode == CXL_MODE_DEDICATED) {
- vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
- return vm_iomap_memory(vma, ctx->afu->psn_phys,
ctx->afu->adapter->ps_size);
- }
+ if (ctx->afu->current_mode != CXL_MODE_DEDICATED) {
+ /* make sure there is a valid per process space for this AFU */
+ if ((ctx->master && !ctx->afu->psa) || (!ctx->afu->pp_psa)) {
+ pr_devel("AFU doesn't support mmio space\n");
+ return -EINVAL;
+ }
- /* make sure there is a valid per process space for this AFU */
- if ((ctx->master && !ctx->afu->psa) || (!ctx->afu->pp_psa)) {
- pr_devel("AFU doesn't support mmio space\n");
- return -EINVAL;
+ /* Can't mmap until the AFU is enabled */
+ if (!ctx->afu->enabled)
+ return -EBUSY;
}
- /* Can't mmap until the AFU is enabled */
- if (!ctx->afu->enabled)
- return -EBUSY;
-
pr_devel("%s: mmio physical: %llx pe: %i master:%i\n", __func__,
ctx->psn_phys, ctx->pe , ctx->master);
+ vma->vm_flags |= VM_IO | VM_PFNMAP;
vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
- return vm_iomap_memory(vma, ctx->psn_phys, len);
+ vma->vm_ops = _mmap_vmops;
+ return 0;
}
/*
@@ -150,12 +189,6 @@ static void __detach_context(struct cxl_context *ctx)
afu_release_irqs(ctx);
flush_work(>fault_work); /* Only needed for dedicated process */
wake_up_all(>wq);
-
- /* Release Problem State Area mapping */
- mutex_lock(>mappin
[PATCH] cxl: Fix issues when unmapping contexts
From: Ian Munsie imun...@au1.ibm.com
An issue was introduced with cxl: Unmap MMIO regions when detaching a
context (b123429e6a9e8d03aacf888d23262835f0081448) where closing a
context normally could also unmap the problem state area of other
contexts currently using the AFU.
It was also discovered that after a context's MMIO space had been
unmapped it would read 0s when accessing it, whereas the expected
behaviour was for the access to fail altogether.
In order to address these issues, this patch does two things:
- Forced mmap unmapping is only done when we are forcefully detaching
all contexts, and not in the normal detach path. Since the normal
context close path is tied to the file release any mmaps must have
already been released so we don't need to worry in that case.
- The mmap path now uses a vm_operations_struct with a fault handler.
The fault handler ensures that the context is in started state,
otherwise it fails the access attempt with a SIGBUS.
Signed-off-by: Ian Munsie imun...@au1.ibm.com
---
drivers/misc/cxl/context.c | 82 +++---
drivers/misc/cxl/file.c| 14
2 files changed, 71 insertions(+), 25 deletions(-)
diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c
index 51fd6b5..d1b55fe 100644
--- a/drivers/misc/cxl/context.c
+++ b/drivers/misc/cxl/context.c
@@ -100,6 +100,46 @@ int cxl_context_init(struct cxl_context *ctx, struct
cxl_afu *afu, bool master,
return 0;
}
+static int cxl_mmap_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
+{
+ struct cxl_context *ctx = vma-vm_file-private_data;
+ unsigned long address = (unsigned long)vmf-virtual_address;
+ u64 area, offset;
+
+ offset = vmf-pgoff PAGE_SHIFT;
+
+ pr_devel(%s: pe: %i address: 0x%lx offset: 0x%llx\n,
+ __func__, ctx-pe, address, offset);
+
+ if (ctx-afu-current_mode == CXL_MODE_DEDICATED) {
+ area = ctx-afu-psn_phys;
+ if (offset ctx-afu-adapter-ps_size)
+ return VM_FAULT_SIGBUS;
+ } else {
+ area = ctx-psn_phys;
+ if (offset ctx-psn_size)
+ return VM_FAULT_SIGBUS;
+ }
+
+ mutex_lock(ctx-status_mutex);
+
+ if (ctx-status != STARTED) {
+ mutex_unlock(ctx-status_mutex);
+ pr_devel(%s: Context not started, failing problem state
access\n, __func__);
+ return VM_FAULT_SIGBUS;
+ }
+
+ vm_insert_pfn(vma, address, (area + offset) PAGE_SHIFT);
+
+ mutex_unlock(ctx-status_mutex);
+
+ return VM_FAULT_NOPAGE;
+}
+
+static const struct vm_operations_struct cxl_mmap_vmops = {
+ .fault = cxl_mmap_fault,
+};
+
/*
* Map a per-context mmio space into the given vma.
*/
@@ -108,26 +148,25 @@ int cxl_context_iomap(struct cxl_context *ctx, struct
vm_area_struct *vma)
u64 len = vma-vm_end - vma-vm_start;
len = min(len, ctx-psn_size);
- if (ctx-afu-current_mode == CXL_MODE_DEDICATED) {
- vma-vm_page_prot = pgprot_noncached(vma-vm_page_prot);
- return vm_iomap_memory(vma, ctx-afu-psn_phys,
ctx-afu-adapter-ps_size);
- }
+ if (ctx-afu-current_mode != CXL_MODE_DEDICATED) {
+ /* make sure there is a valid per process space for this AFU */
+ if ((ctx-master !ctx-afu-psa) || (!ctx-afu-pp_psa)) {
+ pr_devel(AFU doesn't support mmio space\n);
+ return -EINVAL;
+ }
- /* make sure there is a valid per process space for this AFU */
- if ((ctx-master !ctx-afu-psa) || (!ctx-afu-pp_psa)) {
- pr_devel(AFU doesn't support mmio space\n);
- return -EINVAL;
+ /* Can't mmap until the AFU is enabled */
+ if (!ctx-afu-enabled)
+ return -EBUSY;
}
- /* Can't mmap until the AFU is enabled */
- if (!ctx-afu-enabled)
- return -EBUSY;
-
pr_devel(%s: mmio physical: %llx pe: %i master:%i\n, __func__,
ctx-psn_phys, ctx-pe , ctx-master);
+ vma-vm_flags |= VM_IO | VM_PFNMAP;
vma-vm_page_prot = pgprot_noncached(vma-vm_page_prot);
- return vm_iomap_memory(vma, ctx-psn_phys, len);
+ vma-vm_ops = cxl_mmap_vmops;
+ return 0;
}
/*
@@ -150,12 +189,6 @@ static void __detach_context(struct cxl_context *ctx)
afu_release_irqs(ctx);
flush_work(ctx-fault_work); /* Only needed for dedicated process */
wake_up_all(ctx-wq);
-
- /* Release Problem State Area mapping */
- mutex_lock(ctx-mapping_lock);
- if (ctx-mapping)
- unmap_mapping_range(ctx-mapping, 0, 0, 1);
- mutex_unlock(ctx-mapping_lock);
}
/*
@@ -184,6 +217,17 @@ void cxl_context_detach_all(struct cxl_afu *afu)
* created and torn down after the IDR removed
[PATCH] CXL: Fix device_node reference counting
From: Ryan Grimm gr...@linux.vnet.ibm.com When unbinding and rebinding the driver on a system with a card in PHB0, this error condition is reached after a few attempts: ERROR: Bad of_node_put() on /pciex@3fffe4000 CPU: 0 PID: 3040 Comm: bash Not tainted 3.18.0-rc3-12545-g3627ffe #152 Call Trace: [c00721acb5c0] [c086ef94] .dump_stack+0x84/0xb0 (unreliable) [c00721acb640] [c073a0a8] .of_node_release+0xd8/0xe0 [c00721acb6d0] [c044bc44] .kobject_release+0x74/0xe0 [c00721acb760] [c07394fc] .of_node_put+0x1c/0x30 [c00721acb7d0] [c0545cd8] .cxl_probe+0x1a98/0x1d50 [c00721acb900] [c04845a0] .local_pci_probe+0x40/0xc0 [c00721acb980] [c0484998] .pci_device_probe+0x128/0x170 [c00721acba30] [c052400c] .driver_probe_device+0xac/0x2a0 [c00721acbad0] [c0522468] .bind_store+0x108/0x160 [c00721acbb70] [c0521448] .drv_attr_store+0x38/0x60 [c00721acbbe0] [c0293840] .sysfs_kf_write+0x60/0xa0 [c00721acbc50] [c0292500] .kernfs_fop_write+0x140/0x1d0 [c00721acbcf0] [c0208648] .vfs_write+0xd8/0x260 [c00721acbd90] [c0208b18] .SyS_write+0x58/0x100 [c00721acbe30] [c0009258] syscall_exit+0x0/0x98 of_get_next_parent decrements parent's refcount and we need to call of_node_put after the iteration. But, if while loop is not entered, of_node_put get called on np without an of_node_get. So, call it before the while loop. Signed-off-by: Ryan Grimm gr...@linux.vnet.ibm.com Signed-off-by: Ian Munsie imun...@au1.ibm.com --- drivers/misc/cxl/pci.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c index 2ccd0a9..f801c28 100644 --- a/drivers/misc/cxl/pci.c +++ b/drivers/misc/cxl/pci.c @@ -319,6 +319,7 @@ static int init_implementation_adapter_regs(struct cxl *adapter, struct pci_dev if (!(np = pnv_pci_to_phb_node(dev))) return -ENODEV; + of_node_get(np); while (np !(prop = of_get_property(np, ibm,chip-id, NULL))) np = of_get_next_parent(np); if (!np) -- 2.1.4 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/7] CXL: Unmap MMIO regions when detaching a context
Excerpts from Ian Munsie's message of 2014-12-08 19:18:01 +1100: > From: Ian Munsie > > If we need to force detach a context (e.g. due to EEH or simply force > unbinding the driver) we should prevent the userspace contexts from > being able to access the Problem State Area MMIO region further, which > they may have mapped with mmap(). > > This patch unmaps any mapped MMIO regions when detaching a userspace > context. Might want to drop this one for now - Philippe found that it sometimes causes an issue when multiple contexts are using the afu. Seems that unmap_mapping_range() unmapped a bit more than I expected. Cheers, -Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/7] CXL: Unmap MMIO regions when detaching a context
Excerpts from Ian Munsie's message of 2014-12-08 19:18:01 +1100: From: Ian Munsie imun...@au1.ibm.com If we need to force detach a context (e.g. due to EEH or simply force unbinding the driver) we should prevent the userspace contexts from being able to access the Problem State Area MMIO region further, which they may have mapped with mmap(). This patch unmaps any mapped MMIO regions when detaching a userspace context. Might want to drop this one for now - Philippe found that it sometimes causes an issue when multiple contexts are using the afu. Seems that unmap_mapping_range() unmapped a bit more than I expected. Cheers, -Ian -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/7] CXL: Change contexts_lock to a mutex to fix sleep while atomic bug
From: Ian Munsie
We had a known sleep while atomic bug if a CXL device was forcefully
unbound while it was in use. This could occur as a result of EEH, or
manually induced with something like this while the device was in use:
echo :01:00.0 > /sys/bus/pci/drivers/cxl-pci/unbind
The issue was that in this code path we iterated over each context and
forcefully detached it with the contexts_lock spin lock held, however
the detach also needed to take the spu_mutex, and call schedule.
This patch changes the contexts_lock to a mutex so that we are not in
atomic context while doing the detach, thereby avoiding the sleep while
atomic.
Also delete the related TODO comment, which suggested an alternate
solution which turned out to not be workable.
Signed-off-by: Ian Munsie
---
drivers/misc/cxl/context.c | 15 ---
drivers/misc/cxl/cxl.h | 2 +-
drivers/misc/cxl/native.c | 7 ---
drivers/misc/cxl/pci.c | 2 +-
drivers/misc/cxl/sysfs.c | 10 +-
5 files changed, 15 insertions(+), 21 deletions(-)
diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c
index cca4721..4aa31a3 100644
--- a/drivers/misc/cxl/context.c
+++ b/drivers/misc/cxl/context.c
@@ -82,12 +82,12 @@ int cxl_context_init(struct cxl_context *ctx, struct
cxl_afu *afu, bool master)
* Allocating IDR! We better make sure everything's setup that
* dereferences from it.
*/
+ mutex_lock(>contexts_lock);
idr_preload(GFP_KERNEL);
- spin_lock(>contexts_lock);
i = idr_alloc(>afu->contexts_idr, ctx, 0,
ctx->afu->num_procs, GFP_NOWAIT);
- spin_unlock(>contexts_lock);
idr_preload_end();
+ mutex_unlock(>contexts_lock);
if (i < 0)
return i;
@@ -168,21 +168,22 @@ void cxl_context_detach_all(struct cxl_afu *afu)
struct cxl_context *ctx;
int tmp;
- rcu_read_lock();
- idr_for_each_entry(>contexts_idr, ctx, tmp)
+ mutex_lock(>contexts_lock);
+ idr_for_each_entry(>contexts_idr, ctx, tmp) {
/*
* Anything done in here needs to be setup before the IDR is
* created and torn down after the IDR removed
*/
__detach_context(ctx);
- rcu_read_unlock();
+ }
+ mutex_unlock(>contexts_lock);
}
void cxl_context_free(struct cxl_context *ctx)
{
- spin_lock(>afu->contexts_lock);
+ mutex_lock(>afu->contexts_lock);
idr_remove(>afu->contexts_idr, ctx->pe);
- spin_unlock(>afu->contexts_lock);
+ mutex_unlock(>afu->contexts_lock);
synchronize_rcu();
free_page((u64)ctx->sstp);
diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
index b5b6bda..7c05239 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -351,7 +351,7 @@ struct cxl_afu {
struct device *chardev_s, *chardev_m, *chardev_d;
struct idr contexts_idr;
struct dentry *debugfs;
- spinlock_t contexts_lock;
+ struct mutex contexts_lock;
struct mutex spa_mutex;
spinlock_t afu_cntl_lock;
diff --git a/drivers/misc/cxl/native.c b/drivers/misc/cxl/native.c
index 9a5a442..1001cf4 100644
--- a/drivers/misc/cxl/native.c
+++ b/drivers/misc/cxl/native.c
@@ -610,13 +610,6 @@ static inline int detach_process_native_dedicated(struct
cxl_context *ctx)
return 0;
}
-/*
- * TODO: handle case when this is called inside a rcu_read_lock() which may
- * happen when we unbind the driver (ie. cxl_context_detach_all()) . Terminate
- * & remove use a mutex lock and schedule which will not good with lock held.
- * May need to write do_process_element_cmd() that handles outstanding page
- * faults synchronously.
- */
static inline int detach_process_native_afu_directed(struct cxl_context *ctx)
{
if (!ctx->pe_inserted)
diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index 10c98ab..0f2cc9f 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -502,7 +502,7 @@ static struct cxl_afu *cxl_alloc_afu(struct cxl *adapter,
int slice)
afu->dev.release = cxl_release_afu;
afu->slice = slice;
idr_init(>contexts_idr);
- spin_lock_init(>contexts_lock);
+ mutex_init(>contexts_lock);
spin_lock_init(>afu_cntl_lock);
mutex_init(>spa_mutex);
diff --git a/drivers/misc/cxl/sysfs.c b/drivers/misc/cxl/sysfs.c
index ce7ec06..461bdbd 100644
--- a/drivers/misc/cxl/sysfs.c
+++ b/drivers/misc/cxl/sysfs.c
@@ -121,7 +121,7 @@ static ssize_t reset_store_afu(struct device *device,
int rc;
/* Not safe to reset if it is currently in use */
- spin_lock(>contexts_lock);
+ mutex_lock(>contexts_lock);
if (!idr_is_empty(>contexts_idr)) {
rc = -EBUSY;
goto err;
@@ -13
[PATCH 3/7] CXL: Fix leaking interrupts if attach process fails
From: Ian Munsie
In this particular error path we have already allocated the AFU
interrupts, but have not yet set the status to STARTED. The detach
context code will only attempt to release the interrupts if the context
is in state STARTED, so in this case the interrupts would remain
allocated.
This patch releases the AFU interrupts immediately if the attach call
fails to prevent them leaking.
Signed-off-by: Ian Munsie
---
drivers/misc/cxl/file.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/misc/cxl/file.c b/drivers/misc/cxl/file.c
index 378b099..2e067a5 100644
--- a/drivers/misc/cxl/file.c
+++ b/drivers/misc/cxl/file.c
@@ -181,8 +181,10 @@ static long afu_ioctl_start_work(struct cxl_context *ctx,
ctx->pid = get_pid(get_task_pid(current, PIDTYPE_PID));
if ((rc = cxl_attach_process(ctx, false, work.work_element_descriptor,
-amr)))
+amr))) {
+ afu_release_irqs(ctx);
goto out;
+ }
ctx->status = STARTED;
rc = 0;
--
2.1.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 7/7] CXL: Unmap MMIO regions when detaching a context
From: Ian Munsie
If we need to force detach a context (e.g. due to EEH or simply force
unbinding the driver) we should prevent the userspace contexts from
being able to access the Problem State Area MMIO region further, which
they may have mapped with mmap().
This patch unmaps any mapped MMIO regions when detaching a userspace
context.
Signed-off-by: Ian Munsie
---
drivers/misc/cxl/context.c | 11 ++-
drivers/misc/cxl/cxl.h | 7 ++-
drivers/misc/cxl/file.c| 6 +-
3 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c
index 4aa31a3..51fd6b5 100644
--- a/drivers/misc/cxl/context.c
+++ b/drivers/misc/cxl/context.c
@@ -34,7 +34,8 @@ struct cxl_context *cxl_context_alloc(void)
/*
* Initialises a CXL context.
*/
-int cxl_context_init(struct cxl_context *ctx, struct cxl_afu *afu, bool master)
+int cxl_context_init(struct cxl_context *ctx, struct cxl_afu *afu, bool master,
+struct address_space *mapping)
{
int i;
@@ -42,6 +43,8 @@ int cxl_context_init(struct cxl_context *ctx, struct cxl_afu
*afu, bool master)
ctx->afu = afu;
ctx->master = master;
ctx->pid = NULL; /* Set in start work ioctl */
+ mutex_init(>mapping_lock);
+ ctx->mapping = mapping;
/*
* Allocate the segment table before we put it in the IDR so that we
@@ -147,6 +150,12 @@ static void __detach_context(struct cxl_context *ctx)
afu_release_irqs(ctx);
flush_work(>fault_work); /* Only needed for dedicated process */
wake_up_all(>wq);
+
+ /* Release Problem State Area mapping */
+ mutex_lock(>mapping_lock);
+ if (ctx->mapping)
+ unmap_mapping_range(ctx->mapping, 0, 0, 1);
+ mutex_unlock(>mapping_lock);
}
/*
diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
index c1f8aa6..0df0438 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -405,6 +405,10 @@ struct cxl_context {
phys_addr_t psn_phys;
u64 psn_size;
+ /* Used to unmap any mmaps when force detaching */
+ struct address_space *mapping;
+ struct mutex mapping_lock;
+
spinlock_t sste_lock; /* Protects segment table entries */
struct cxl_sste *sstp;
u64 sstp0, sstp1;
@@ -606,7 +610,8 @@ int cxl_alloc_sst(struct cxl_context *ctx);
void init_cxl_native(void);
struct cxl_context *cxl_context_alloc(void);
-int cxl_context_init(struct cxl_context *ctx, struct cxl_afu *afu, bool
master);
+int cxl_context_init(struct cxl_context *ctx, struct cxl_afu *afu, bool master,
+struct address_space *mapping);
void cxl_context_free(struct cxl_context *ctx);
int cxl_context_iomap(struct cxl_context *ctx, struct vm_area_struct *vma);
diff --git a/drivers/misc/cxl/file.c b/drivers/misc/cxl/file.c
index 2e067a5..b09be44 100644
--- a/drivers/misc/cxl/file.c
+++ b/drivers/misc/cxl/file.c
@@ -77,7 +77,7 @@ static int __afu_open(struct inode *inode, struct file *file,
bool master)
goto err_put_afu;
}
- if ((rc = cxl_context_init(ctx, afu, master)))
+ if ((rc = cxl_context_init(ctx, afu, master, inode->i_mapping)))
goto err_put_afu;
pr_devel("afu_open pe: %i\n", ctx->pe);
@@ -113,6 +113,10 @@ static int afu_release(struct inode *inode, struct file
*file)
__func__, ctx->pe);
cxl_context_detach(ctx);
+ mutex_lock(>mapping_lock);
+ ctx->mapping = NULL;
+ mutex_unlock(>mapping_lock);
+
put_device(>afu->dev);
/*
--
2.1.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 5/7] CXL: Disable AFU debug flag
From: Ian Munsie
Upon inspection of the implementation specific registers, it was
discovered that the high bit of the implementation specific RXCTL
register was enabled, which enables the DEADB00F debug feature.
The debug feature causes MMIO reads to a disabled AFU to respond with
0xDEADB00F instead of all Fs. In general this should not be visible as
the kernel will only allow MMIO access to enabled AFUs, but there may be
some circumstances where an AFU may become disabled while it is use.
One such case would be an AFU designed to only be used in the dedicated
process mode and to disable itself after it has completed it's work
(however even in that case the effects of this debug flag would be
limited as the userspace application must have completed any required
MMIO accesses before the AFU disables itself with or without the flag).
This patch removes the debug flag and replaces the magic value
programmed into this register with a preprocessor define so it is
clearer what the rest of this initialisation does.
Signed-off-by: Ian Munsie
---
drivers/misc/cxl/cxl.h | 7 +++
drivers/misc/cxl/pci.c | 2 +-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
index 7c05239..c1f8aa6 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -287,6 +287,13 @@ static const cxl_p2n_reg_t CXL_PSL_WED_An = {0x0A0};
#define CXL_PE_SOFTWARE_STATE_S (1ul << (31 - 30)) /* Suspend */
#define CXL_PE_SOFTWARE_STATE_T (1ul << (31 - 31)) /* Terminate */
+/** CXL_PSL_RXCTL_An (Implementation Specific) **
+ * Controls AFU Hang Pulse, which sets the timeout for the AFU to respond to
+ * the PSL for any response (except MMIO). Timeouts will occur between 1x to 2x
+ * of the hang pulse frequency.
+ */
+#define CXL_PSL_RXCTL_AFUHP_4S 0x7000ULL
+
/* SPA->sw_command_status */
#define CXL_SPA_SW_CMD_MASK 0xULL
#define CXL_SPA_SW_CMD_TERMINATE0x0001ULL
diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index 0f2cc9f..2ccd0a9 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -348,7 +348,7 @@ static int init_implementation_afu_regs(struct cxl_afu *afu)
cxl_p1n_write(afu, CXL_PSL_COALLOC_A, 0xFF00FEFEFEFEULL);
/* for debugging with trace arrays */
cxl_p1n_write(afu, CXL_PSL_SLICE_TRACE, 0xULL);
- cxl_p1n_write(afu, CXL_PSL_RXCTL_A, 0xF000ULL);
+ cxl_p1n_write(afu, CXL_PSL_RXCTL_A, CXL_PSL_RXCTL_AFUHP_4S);
return 0;
}
--
2.1.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 6/7] CXL: Disable SPAP register when freeing SPA
From: Ian Munsie
When we deactivate the AFU directed mode we free the scheduled process
area, but did not clear the register in the hardware that has a pointer
to it.
This should be fine since we will have already cleared out every context
and we won't do anything that would cause the hardware to access it
until after we have allocated a new one, but just to be safe this patch
clears out the register when we free the page.
Signed-off-by: Ian Munsie
---
drivers/misc/cxl/native.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/misc/cxl/native.c b/drivers/misc/cxl/native.c
index f2b37b4..0f24fa5 100644
--- a/drivers/misc/cxl/native.c
+++ b/drivers/misc/cxl/native.c
@@ -185,6 +185,7 @@ static int alloc_spa(struct cxl_afu *afu)
static void release_spa(struct cxl_afu *afu)
{
+ cxl_p1n_write(afu, CXL_PSL_SPAP_An, 0);
free_pages((unsigned long) afu->spa, afu->spa_order);
}
--
2.1.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/7] CXL: Early return from cxl_handle_fault for a shut down context
From: Ian Munsie
If a context is being detached and we get a translation fault for it
there is little point getting it's mm and handling the fault, so just
respond with an address error and return earlier.
Signed-off-by: Ian Munsie
---
drivers/misc/cxl/fault.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/misc/cxl/fault.c b/drivers/misc/cxl/fault.c
index c99e896..3e8c06a 100644
--- a/drivers/misc/cxl/fault.c
+++ b/drivers/misc/cxl/fault.c
@@ -176,6 +176,12 @@ void cxl_handle_fault(struct work_struct *fault_work)
return;
}
+ /* Early return if the context is being / has been detached */
+ if (ctx->status == CLOSED) {
+ cxl_ack_ae(ctx);
+ return;
+ }
+
pr_devel("CXL BOTTOM HALF handling fault for afu pe: %i. "
"DSISR: %#llx DAR: %#llx\n", ctx->pe, dsisr, dar);
--
2.1.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/7] CXL: Add timeout to process element commands
From: Ian Munsie
In the event that something goes wrong in the hardware and it is unable
to complete a process element comment we would end up polling forever,
effectively making the associated process unkillable.
This patch adds a timeout to the process element command code path, so
that we will give up if the hardware does not respond in a reasonable
time.
Signed-off-by: Ian Munsie
---
drivers/misc/cxl/native.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/misc/cxl/native.c b/drivers/misc/cxl/native.c
index 1001cf4..f2b37b4 100644
--- a/drivers/misc/cxl/native.c
+++ b/drivers/misc/cxl/native.c
@@ -277,6 +277,7 @@ static int do_process_element_cmd(struct cxl_context *ctx,
u64 cmd, u64 pe_state)
{
u64 state;
+ unsigned long timeout = jiffies + (HZ * CXL_TIMEOUT);
WARN_ON(!ctx->afu->enabled);
@@ -286,6 +287,10 @@ static int do_process_element_cmd(struct cxl_context *ctx,
smp_mb();
cxl_p1n_write(ctx->afu, CXL_PSL_LLCMD_An, cmd | ctx->pe);
while (1) {
+ if (time_after_eq(jiffies, timeout)) {
+ dev_warn(>afu->dev, "WARNING: Process Element
Command timed out!\n");
+ return -EBUSY;
+ }
state = be64_to_cpup(ctx->afu->sw_command_status);
if (state == ~0ULL) {
pr_err("cxl: Error adding process element to AFU\n");
--
2.1.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/7] CXL: Add timeout to process element commands
From: Ian Munsie imun...@au1.ibm.com
In the event that something goes wrong in the hardware and it is unable
to complete a process element comment we would end up polling forever,
effectively making the associated process unkillable.
This patch adds a timeout to the process element command code path, so
that we will give up if the hardware does not respond in a reasonable
time.
Signed-off-by: Ian Munsie imun...@au1.ibm.com
---
drivers/misc/cxl/native.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/misc/cxl/native.c b/drivers/misc/cxl/native.c
index 1001cf4..f2b37b4 100644
--- a/drivers/misc/cxl/native.c
+++ b/drivers/misc/cxl/native.c
@@ -277,6 +277,7 @@ static int do_process_element_cmd(struct cxl_context *ctx,
u64 cmd, u64 pe_state)
{
u64 state;
+ unsigned long timeout = jiffies + (HZ * CXL_TIMEOUT);
WARN_ON(!ctx-afu-enabled);
@@ -286,6 +287,10 @@ static int do_process_element_cmd(struct cxl_context *ctx,
smp_mb();
cxl_p1n_write(ctx-afu, CXL_PSL_LLCMD_An, cmd | ctx-pe);
while (1) {
+ if (time_after_eq(jiffies, timeout)) {
+ dev_warn(ctx-afu-dev, WARNING: Process Element
Command timed out!\n);
+ return -EBUSY;
+ }
state = be64_to_cpup(ctx-afu-sw_command_status);
if (state == ~0ULL) {
pr_err(cxl: Error adding process element to AFU\n);
--
2.1.3
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/7] CXL: Early return from cxl_handle_fault for a shut down context
From: Ian Munsie imun...@au1.ibm.com
If a context is being detached and we get a translation fault for it
there is little point getting it's mm and handling the fault, so just
respond with an address error and return earlier.
Signed-off-by: Ian Munsie imun...@au1.ibm.com
---
drivers/misc/cxl/fault.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/misc/cxl/fault.c b/drivers/misc/cxl/fault.c
index c99e896..3e8c06a 100644
--- a/drivers/misc/cxl/fault.c
+++ b/drivers/misc/cxl/fault.c
@@ -176,6 +176,12 @@ void cxl_handle_fault(struct work_struct *fault_work)
return;
}
+ /* Early return if the context is being / has been detached */
+ if (ctx-status == CLOSED) {
+ cxl_ack_ae(ctx);
+ return;
+ }
+
pr_devel(CXL BOTTOM HALF handling fault for afu pe: %i.
DSISR: %#llx DAR: %#llx\n, ctx-pe, dsisr, dar);
--
2.1.3
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 6/7] CXL: Disable SPAP register when freeing SPA
From: Ian Munsie imun...@au1.ibm.com
When we deactivate the AFU directed mode we free the scheduled process
area, but did not clear the register in the hardware that has a pointer
to it.
This should be fine since we will have already cleared out every context
and we won't do anything that would cause the hardware to access it
until after we have allocated a new one, but just to be safe this patch
clears out the register when we free the page.
Signed-off-by: Ian Munsie imun...@au1.ibm.com
---
drivers/misc/cxl/native.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/misc/cxl/native.c b/drivers/misc/cxl/native.c
index f2b37b4..0f24fa5 100644
--- a/drivers/misc/cxl/native.c
+++ b/drivers/misc/cxl/native.c
@@ -185,6 +185,7 @@ static int alloc_spa(struct cxl_afu *afu)
static void release_spa(struct cxl_afu *afu)
{
+ cxl_p1n_write(afu, CXL_PSL_SPAP_An, 0);
free_pages((unsigned long) afu-spa, afu-spa_order);
}
--
2.1.3
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 5/7] CXL: Disable AFU debug flag
From: Ian Munsie imun...@au1.ibm.com
Upon inspection of the implementation specific registers, it was
discovered that the high bit of the implementation specific RXCTL
register was enabled, which enables the DEADB00F debug feature.
The debug feature causes MMIO reads to a disabled AFU to respond with
0xDEADB00F instead of all Fs. In general this should not be visible as
the kernel will only allow MMIO access to enabled AFUs, but there may be
some circumstances where an AFU may become disabled while it is use.
One such case would be an AFU designed to only be used in the dedicated
process mode and to disable itself after it has completed it's work
(however even in that case the effects of this debug flag would be
limited as the userspace application must have completed any required
MMIO accesses before the AFU disables itself with or without the flag).
This patch removes the debug flag and replaces the magic value
programmed into this register with a preprocessor define so it is
clearer what the rest of this initialisation does.
Signed-off-by: Ian Munsie imun...@au1.ibm.com
---
drivers/misc/cxl/cxl.h | 7 +++
drivers/misc/cxl/pci.c | 2 +-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
index 7c05239..c1f8aa6 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -287,6 +287,13 @@ static const cxl_p2n_reg_t CXL_PSL_WED_An = {0x0A0};
#define CXL_PE_SOFTWARE_STATE_S (1ul (31 - 30)) /* Suspend */
#define CXL_PE_SOFTWARE_STATE_T (1ul (31 - 31)) /* Terminate */
+/** CXL_PSL_RXCTL_An (Implementation Specific) **
+ * Controls AFU Hang Pulse, which sets the timeout for the AFU to respond to
+ * the PSL for any response (except MMIO). Timeouts will occur between 1x to 2x
+ * of the hang pulse frequency.
+ */
+#define CXL_PSL_RXCTL_AFUHP_4S 0x7000ULL
+
/* SPA-sw_command_status */
#define CXL_SPA_SW_CMD_MASK 0xULL
#define CXL_SPA_SW_CMD_TERMINATE0x0001ULL
diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index 0f2cc9f..2ccd0a9 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -348,7 +348,7 @@ static int init_implementation_afu_regs(struct cxl_afu *afu)
cxl_p1n_write(afu, CXL_PSL_COALLOC_A, 0xFF00FEFEFEFEULL);
/* for debugging with trace arrays */
cxl_p1n_write(afu, CXL_PSL_SLICE_TRACE, 0xULL);
- cxl_p1n_write(afu, CXL_PSL_RXCTL_A, 0xF000ULL);
+ cxl_p1n_write(afu, CXL_PSL_RXCTL_A, CXL_PSL_RXCTL_AFUHP_4S);
return 0;
}
--
2.1.3
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/7] CXL: Fix leaking interrupts if attach process fails
From: Ian Munsie imun...@au1.ibm.com
In this particular error path we have already allocated the AFU
interrupts, but have not yet set the status to STARTED. The detach
context code will only attempt to release the interrupts if the context
is in state STARTED, so in this case the interrupts would remain
allocated.
This patch releases the AFU interrupts immediately if the attach call
fails to prevent them leaking.
Signed-off-by: Ian Munsie imun...@au1.ibm.com
---
drivers/misc/cxl/file.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/misc/cxl/file.c b/drivers/misc/cxl/file.c
index 378b099..2e067a5 100644
--- a/drivers/misc/cxl/file.c
+++ b/drivers/misc/cxl/file.c
@@ -181,8 +181,10 @@ static long afu_ioctl_start_work(struct cxl_context *ctx,
ctx-pid = get_pid(get_task_pid(current, PIDTYPE_PID));
if ((rc = cxl_attach_process(ctx, false, work.work_element_descriptor,
-amr)))
+amr))) {
+ afu_release_irqs(ctx);
goto out;
+ }
ctx-status = STARTED;
rc = 0;
--
2.1.3
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 7/7] CXL: Unmap MMIO regions when detaching a context
From: Ian Munsie imun...@au1.ibm.com
If we need to force detach a context (e.g. due to EEH or simply force
unbinding the driver) we should prevent the userspace contexts from
being able to access the Problem State Area MMIO region further, which
they may have mapped with mmap().
This patch unmaps any mapped MMIO regions when detaching a userspace
context.
Signed-off-by: Ian Munsie imun...@au1.ibm.com
---
drivers/misc/cxl/context.c | 11 ++-
drivers/misc/cxl/cxl.h | 7 ++-
drivers/misc/cxl/file.c| 6 +-
3 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c
index 4aa31a3..51fd6b5 100644
--- a/drivers/misc/cxl/context.c
+++ b/drivers/misc/cxl/context.c
@@ -34,7 +34,8 @@ struct cxl_context *cxl_context_alloc(void)
/*
* Initialises a CXL context.
*/
-int cxl_context_init(struct cxl_context *ctx, struct cxl_afu *afu, bool master)
+int cxl_context_init(struct cxl_context *ctx, struct cxl_afu *afu, bool master,
+struct address_space *mapping)
{
int i;
@@ -42,6 +43,8 @@ int cxl_context_init(struct cxl_context *ctx, struct cxl_afu
*afu, bool master)
ctx-afu = afu;
ctx-master = master;
ctx-pid = NULL; /* Set in start work ioctl */
+ mutex_init(ctx-mapping_lock);
+ ctx-mapping = mapping;
/*
* Allocate the segment table before we put it in the IDR so that we
@@ -147,6 +150,12 @@ static void __detach_context(struct cxl_context *ctx)
afu_release_irqs(ctx);
flush_work(ctx-fault_work); /* Only needed for dedicated process */
wake_up_all(ctx-wq);
+
+ /* Release Problem State Area mapping */
+ mutex_lock(ctx-mapping_lock);
+ if (ctx-mapping)
+ unmap_mapping_range(ctx-mapping, 0, 0, 1);
+ mutex_unlock(ctx-mapping_lock);
}
/*
diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
index c1f8aa6..0df0438 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -405,6 +405,10 @@ struct cxl_context {
phys_addr_t psn_phys;
u64 psn_size;
+ /* Used to unmap any mmaps when force detaching */
+ struct address_space *mapping;
+ struct mutex mapping_lock;
+
spinlock_t sste_lock; /* Protects segment table entries */
struct cxl_sste *sstp;
u64 sstp0, sstp1;
@@ -606,7 +610,8 @@ int cxl_alloc_sst(struct cxl_context *ctx);
void init_cxl_native(void);
struct cxl_context *cxl_context_alloc(void);
-int cxl_context_init(struct cxl_context *ctx, struct cxl_afu *afu, bool
master);
+int cxl_context_init(struct cxl_context *ctx, struct cxl_afu *afu, bool master,
+struct address_space *mapping);
void cxl_context_free(struct cxl_context *ctx);
int cxl_context_iomap(struct cxl_context *ctx, struct vm_area_struct *vma);
diff --git a/drivers/misc/cxl/file.c b/drivers/misc/cxl/file.c
index 2e067a5..b09be44 100644
--- a/drivers/misc/cxl/file.c
+++ b/drivers/misc/cxl/file.c
@@ -77,7 +77,7 @@ static int __afu_open(struct inode *inode, struct file *file,
bool master)
goto err_put_afu;
}
- if ((rc = cxl_context_init(ctx, afu, master)))
+ if ((rc = cxl_context_init(ctx, afu, master, inode-i_mapping)))
goto err_put_afu;
pr_devel(afu_open pe: %i\n, ctx-pe);
@@ -113,6 +113,10 @@ static int afu_release(struct inode *inode, struct file
*file)
__func__, ctx-pe);
cxl_context_detach(ctx);
+ mutex_lock(ctx-mapping_lock);
+ ctx-mapping = NULL;
+ mutex_unlock(ctx-mapping_lock);
+
put_device(ctx-afu-dev);
/*
--
2.1.3
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/7] CXL: Change contexts_lock to a mutex to fix sleep while atomic bug
From: Ian Munsie imun...@au1.ibm.com
We had a known sleep while atomic bug if a CXL device was forcefully
unbound while it was in use. This could occur as a result of EEH, or
manually induced with something like this while the device was in use:
echo :01:00.0 /sys/bus/pci/drivers/cxl-pci/unbind
The issue was that in this code path we iterated over each context and
forcefully detached it with the contexts_lock spin lock held, however
the detach also needed to take the spu_mutex, and call schedule.
This patch changes the contexts_lock to a mutex so that we are not in
atomic context while doing the detach, thereby avoiding the sleep while
atomic.
Also delete the related TODO comment, which suggested an alternate
solution which turned out to not be workable.
Signed-off-by: Ian Munsie imun...@au1.ibm.com
---
drivers/misc/cxl/context.c | 15 ---
drivers/misc/cxl/cxl.h | 2 +-
drivers/misc/cxl/native.c | 7 ---
drivers/misc/cxl/pci.c | 2 +-
drivers/misc/cxl/sysfs.c | 10 +-
5 files changed, 15 insertions(+), 21 deletions(-)
diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c
index cca4721..4aa31a3 100644
--- a/drivers/misc/cxl/context.c
+++ b/drivers/misc/cxl/context.c
@@ -82,12 +82,12 @@ int cxl_context_init(struct cxl_context *ctx, struct
cxl_afu *afu, bool master)
* Allocating IDR! We better make sure everything's setup that
* dereferences from it.
*/
+ mutex_lock(afu-contexts_lock);
idr_preload(GFP_KERNEL);
- spin_lock(afu-contexts_lock);
i = idr_alloc(ctx-afu-contexts_idr, ctx, 0,
ctx-afu-num_procs, GFP_NOWAIT);
- spin_unlock(afu-contexts_lock);
idr_preload_end();
+ mutex_unlock(afu-contexts_lock);
if (i 0)
return i;
@@ -168,21 +168,22 @@ void cxl_context_detach_all(struct cxl_afu *afu)
struct cxl_context *ctx;
int tmp;
- rcu_read_lock();
- idr_for_each_entry(afu-contexts_idr, ctx, tmp)
+ mutex_lock(afu-contexts_lock);
+ idr_for_each_entry(afu-contexts_idr, ctx, tmp) {
/*
* Anything done in here needs to be setup before the IDR is
* created and torn down after the IDR removed
*/
__detach_context(ctx);
- rcu_read_unlock();
+ }
+ mutex_unlock(afu-contexts_lock);
}
void cxl_context_free(struct cxl_context *ctx)
{
- spin_lock(ctx-afu-contexts_lock);
+ mutex_lock(ctx-afu-contexts_lock);
idr_remove(ctx-afu-contexts_idr, ctx-pe);
- spin_unlock(ctx-afu-contexts_lock);
+ mutex_unlock(ctx-afu-contexts_lock);
synchronize_rcu();
free_page((u64)ctx-sstp);
diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
index b5b6bda..7c05239 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -351,7 +351,7 @@ struct cxl_afu {
struct device *chardev_s, *chardev_m, *chardev_d;
struct idr contexts_idr;
struct dentry *debugfs;
- spinlock_t contexts_lock;
+ struct mutex contexts_lock;
struct mutex spa_mutex;
spinlock_t afu_cntl_lock;
diff --git a/drivers/misc/cxl/native.c b/drivers/misc/cxl/native.c
index 9a5a442..1001cf4 100644
--- a/drivers/misc/cxl/native.c
+++ b/drivers/misc/cxl/native.c
@@ -610,13 +610,6 @@ static inline int detach_process_native_dedicated(struct
cxl_context *ctx)
return 0;
}
-/*
- * TODO: handle case when this is called inside a rcu_read_lock() which may
- * happen when we unbind the driver (ie. cxl_context_detach_all()) . Terminate
- * remove use a mutex lock and schedule which will not good with lock held.
- * May need to write do_process_element_cmd() that handles outstanding page
- * faults synchronously.
- */
static inline int detach_process_native_afu_directed(struct cxl_context *ctx)
{
if (!ctx-pe_inserted)
diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index 10c98ab..0f2cc9f 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -502,7 +502,7 @@ static struct cxl_afu *cxl_alloc_afu(struct cxl *adapter,
int slice)
afu-dev.release = cxl_release_afu;
afu-slice = slice;
idr_init(afu-contexts_idr);
- spin_lock_init(afu-contexts_lock);
+ mutex_init(afu-contexts_lock);
spin_lock_init(afu-afu_cntl_lock);
mutex_init(afu-spa_mutex);
diff --git a/drivers/misc/cxl/sysfs.c b/drivers/misc/cxl/sysfs.c
index ce7ec06..461bdbd 100644
--- a/drivers/misc/cxl/sysfs.c
+++ b/drivers/misc/cxl/sysfs.c
@@ -121,7 +121,7 @@ static ssize_t reset_store_afu(struct device *device,
int rc;
/* Not safe to reset if it is currently in use */
- spin_lock(afu-contexts_lock);
+ mutex_lock(afu-contexts_lock);
if (!idr_is_empty(afu-contexts_idr)) {
rc = -EBUSY;
goto err;
@@ -132,7 +132,7 @@ static ssize_t
