Re: [PATCH v5 5/7] ocxl: Expose the thread_id needed for wait on POWER9
Le 11/05/2018 à 12:06, Alastair D'Silva a écrit : -Original Message- From: Frederic BarratSent: Friday, 11 May 2018 7:25 PM To: Alastair D'Silva ; linuxppc-...@lists.ozlabs.org Cc: linux-ker...@vger.kernel.org; linux-doc@vger.kernel.org; mi...@neuling.org; vaib...@linux.vnet.ibm.com; aneesh.ku...@linux.vnet.ibm.com; ma...@debian.org; fe...@linux.vnet.ibm.com; pombreda...@nexb.com; suka...@linux.vnet.ibm.com; npig...@gmail.com; gre...@linuxfoundation.org; a...@arndb.de; andrew.donnel...@au1.ibm.com; fbar...@linux.vnet.ibm.com; cor...@lwn.net; Alastair D'Silva Subject: Re: [PATCH v5 5/7] ocxl: Expose the thread_id needed for wait on POWER9 Le 11/05/2018 à 08:13, Alastair D'Silva a écrit : From: Alastair D'Silva In order to successfully issue as_notify, an AFU needs to know the TID to notify, which in turn means that this information should be available in userspace so it can be communicated to the AFU. Signed-off-by: Alastair D'Silva --- Ok, so we keep the limitation of having only one thread per context able to call 'wait', even though we don't have to worry about depleting the pool of TIDs any more. I think that's acceptable, though we don't really have a reason to justify it any more. Any reason you want to keep it that way? No strong reason, just trying to minimise the amount of changes. We can always expand the scope later, if we have a use-case for it. ok. Agreed, it's not worth holding up this series, we can send a follow up patch. Fred Fred drivers/misc/ocxl/context.c | 5 ++- drivers/misc/ocxl/file.c | 53 +++ drivers/misc/ocxl/link.c | 36 + drivers/misc/ocxl/ocxl_internal.h | 1 + include/misc/ocxl.h | 9 ++ include/uapi/misc/ocxl.h | 8 + 6 files changed, 111 insertions(+), 1 deletion(-) diff --git a/drivers/misc/ocxl/context.c b/drivers/misc/ocxl/context.c index 909e8807824a..95f74623113e 100644 --- a/drivers/misc/ocxl/context.c +++ b/drivers/misc/ocxl/context.c @@ -34,6 +34,8 @@ int ocxl_context_init(struct ocxl_context *ctx, struct ocxl_afu *afu, mutex_init(>xsl_error_lock); mutex_init(>irq_lock); idr_init(>irq_idr); + ctx->tidr = 0; + /* * Keep a reference on the AFU to make sure it's valid for the * duration of the life of the context @@ -65,6 +67,7 @@ int ocxl_context_attach(struct ocxl_context *ctx, u64 amr) { int rc; + // Locks both status & tidr mutex_lock(>status_mutex); if (ctx->status != OPENED) { rc = -EIO; @@ -72,7 +75,7 @@ int ocxl_context_attach(struct ocxl_context *ctx, u64 amr) } rc = ocxl_link_add_pe(ctx->afu->fn->link, ctx->pasid, - current->mm->context.id, 0, amr, current->mm, + current->mm->context.id, ctx->tidr, amr, current- mm, xsl_fault_error, ctx); if (rc) goto out; diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c index 038509e5d031..eb409a469f21 100644 --- a/drivers/misc/ocxl/file.c +++ b/drivers/misc/ocxl/file.c @@ -5,6 +5,8 @@ #include #include #include +#include +#include #include "ocxl_internal.h" @@ -123,11 +125,55 @@ static long afu_ioctl_get_metadata(struct ocxl_context *ctx, return 0; } +#ifdef CONFIG_PPC64 +static long afu_ioctl_enable_p9_wait(struct ocxl_context *ctx, + struct ocxl_ioctl_p9_wait __user *uarg) { + struct ocxl_ioctl_p9_wait arg; + + memset(, 0, sizeof(arg)); + + if (cpu_has_feature(CPU_FTR_P9_TIDR)) { + enum ocxl_context_status status; + + // Locks both status & tidr + mutex_lock(>status_mutex); + if (!ctx->tidr) { + if (set_thread_tidr(current)) + return -ENOENT; + + ctx->tidr = current->thread.tidr; + } + + status = ctx->status; + mutex_unlock(>status_mutex); + + if (status == ATTACHED) { + int rc; + struct link *link = ctx->afu->fn->link; + + rc = ocxl_link_update_pe(link, ctx->pasid, ctx->tidr); + if (rc) + return rc; + } + + arg.thread_id = ctx->tidr; + } else + return -ENOENT; + + if (copy_to_user(uarg, , sizeof(arg))) + return -EFAULT; + + return 0; +} +#endif + #define CMD_STR(x) (x == OCXL_IOCTL_ATTACH ? "ATTACH" : \ x == OCXL_IOCTL_IRQ_ALLOC ? "IRQ_ALLOC" : \ x == OCXL_IOCTL_IRQ_FREE ? "IRQ_FREE" : \
RE: [PATCH v5 5/7] ocxl: Expose the thread_id needed for wait on POWER9
> -Original Message- > From: Frederic Barrat> Sent: Friday, 11 May 2018 7:25 PM > To: Alastair D'Silva ; linuxppc-...@lists.ozlabs.org > Cc: linux-ker...@vger.kernel.org; linux-doc@vger.kernel.org; > mi...@neuling.org; vaib...@linux.vnet.ibm.com; > aneesh.ku...@linux.vnet.ibm.com; ma...@debian.org; > fe...@linux.vnet.ibm.com; pombreda...@nexb.com; > suka...@linux.vnet.ibm.com; npig...@gmail.com; > gre...@linuxfoundation.org; a...@arndb.de; > andrew.donnel...@au1.ibm.com; fbar...@linux.vnet.ibm.com; > cor...@lwn.net; Alastair D'Silva > Subject: Re: [PATCH v5 5/7] ocxl: Expose the thread_id needed for wait on > POWER9 > > > > Le 11/05/2018 à 08:13, Alastair D'Silva a écrit : > > From: Alastair D'Silva > > > > In order to successfully issue as_notify, an AFU needs to know the TID > > to notify, which in turn means that this information should be > > available in userspace so it can be communicated to the AFU. > > > > Signed-off-by: Alastair D'Silva > > --- > > Ok, so we keep the limitation of having only one thread per context able to > call 'wait', even though we don't have to worry about depleting the pool of > TIDs any more. I think that's acceptable, though we don't really have a reason > to justify it any more. Any reason you want to keep it that way? > No strong reason, just trying to minimise the amount of changes. We can always expand the scope later, if we have a use-case for it. >Fred > > > > drivers/misc/ocxl/context.c | 5 ++- > > drivers/misc/ocxl/file.c | 53 +++ > > drivers/misc/ocxl/link.c | 36 + > > drivers/misc/ocxl/ocxl_internal.h | 1 + > > include/misc/ocxl.h | 9 ++ > > include/uapi/misc/ocxl.h | 8 + > > 6 files changed, 111 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/misc/ocxl/context.c b/drivers/misc/ocxl/context.c > > index 909e8807824a..95f74623113e 100644 > > --- a/drivers/misc/ocxl/context.c > > +++ b/drivers/misc/ocxl/context.c > > @@ -34,6 +34,8 @@ int ocxl_context_init(struct ocxl_context *ctx, struct > ocxl_afu *afu, > > mutex_init(>xsl_error_lock); > > mutex_init(>irq_lock); > > idr_init(>irq_idr); > > + ctx->tidr = 0; > > + > > /* > > * Keep a reference on the AFU to make sure it's valid for the > > * duration of the life of the context @@ -65,6 +67,7 @@ int > > ocxl_context_attach(struct ocxl_context *ctx, u64 amr) > > { > > int rc; > > > > + // Locks both status & tidr > > mutex_lock(>status_mutex); > > if (ctx->status != OPENED) { > > rc = -EIO; > > @@ -72,7 +75,7 @@ int ocxl_context_attach(struct ocxl_context *ctx, u64 > amr) > > } > > > > rc = ocxl_link_add_pe(ctx->afu->fn->link, ctx->pasid, > > - current->mm->context.id, 0, amr, current->mm, > > + current->mm->context.id, ctx->tidr, amr, current- > >mm, > > xsl_fault_error, ctx); > > if (rc) > > goto out; > > diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c index > > 038509e5d031..eb409a469f21 100644 > > --- a/drivers/misc/ocxl/file.c > > +++ b/drivers/misc/ocxl/file.c > > @@ -5,6 +5,8 @@ > > #include > > #include > > #include > > +#include > > +#include > > #include "ocxl_internal.h" > > > > > > @@ -123,11 +125,55 @@ static long afu_ioctl_get_metadata(struct > ocxl_context *ctx, > > return 0; > > } > > > > +#ifdef CONFIG_PPC64 > > +static long afu_ioctl_enable_p9_wait(struct ocxl_context *ctx, > > + struct ocxl_ioctl_p9_wait __user *uarg) { > > + struct ocxl_ioctl_p9_wait arg; > > + > > + memset(, 0, sizeof(arg)); > > + > > + if (cpu_has_feature(CPU_FTR_P9_TIDR)) { > > + enum ocxl_context_status status; > > + > > + // Locks both status & tidr > > + mutex_lock(>status_mutex); > > + if (!ctx->tidr) { > > + if (set_thread_tidr(current)) > > + return -ENOENT; > > + > > + ctx->tidr = current->thread.tidr; > > + } > > + > > + status = ctx->status; > > + mutex_unlock(>status_mutex); > > + > > + if (status == ATTACHED) { > > + int rc; > > + struct link *link = ctx->afu->fn->link; > > + > > + rc = ocxl_link_update_pe(link, ctx->pasid, ctx->tidr); > > + if (rc) > > + return rc; > > + } > > + > > + arg.thread_id = ctx->tidr; > > + } else > > + return -ENOENT; > > + > > + if (copy_to_user(uarg, , sizeof(arg))) > > + return -EFAULT; > > + > > + return 0; > > +} > > +#endif > > + > > #define CMD_STR(x) (x == OCXL_IOCTL_ATTACH ? "ATTACH" : > \ > > x ==
Re: [PATCH v5 5/7] ocxl: Expose the thread_id needed for wait on POWER9
Le 11/05/2018 à 08:13, Alastair D'Silva a écrit : From: Alastair D'SilvaIn order to successfully issue as_notify, an AFU needs to know the TID to notify, which in turn means that this information should be available in userspace so it can be communicated to the AFU. Signed-off-by: Alastair D'Silva --- Ok, so we keep the limitation of having only one thread per context able to call 'wait', even though we don't have to worry about depleting the pool of TIDs any more. I think that's acceptable, though we don't really have a reason to justify it any more. Any reason you want to keep it that way? Fred drivers/misc/ocxl/context.c | 5 ++- drivers/misc/ocxl/file.c | 53 +++ drivers/misc/ocxl/link.c | 36 + drivers/misc/ocxl/ocxl_internal.h | 1 + include/misc/ocxl.h | 9 ++ include/uapi/misc/ocxl.h | 8 + 6 files changed, 111 insertions(+), 1 deletion(-) diff --git a/drivers/misc/ocxl/context.c b/drivers/misc/ocxl/context.c index 909e8807824a..95f74623113e 100644 --- a/drivers/misc/ocxl/context.c +++ b/drivers/misc/ocxl/context.c @@ -34,6 +34,8 @@ int ocxl_context_init(struct ocxl_context *ctx, struct ocxl_afu *afu, mutex_init(>xsl_error_lock); mutex_init(>irq_lock); idr_init(>irq_idr); + ctx->tidr = 0; + /* * Keep a reference on the AFU to make sure it's valid for the * duration of the life of the context @@ -65,6 +67,7 @@ int ocxl_context_attach(struct ocxl_context *ctx, u64 amr) { int rc; + // Locks both status & tidr mutex_lock(>status_mutex); if (ctx->status != OPENED) { rc = -EIO; @@ -72,7 +75,7 @@ int ocxl_context_attach(struct ocxl_context *ctx, u64 amr) } rc = ocxl_link_add_pe(ctx->afu->fn->link, ctx->pasid, - current->mm->context.id, 0, amr, current->mm, + current->mm->context.id, ctx->tidr, amr, current->mm, xsl_fault_error, ctx); if (rc) goto out; diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c index 038509e5d031..eb409a469f21 100644 --- a/drivers/misc/ocxl/file.c +++ b/drivers/misc/ocxl/file.c @@ -5,6 +5,8 @@ #include #include #include +#include +#include #include "ocxl_internal.h" @@ -123,11 +125,55 @@ static long afu_ioctl_get_metadata(struct ocxl_context *ctx, return 0; } +#ifdef CONFIG_PPC64 +static long afu_ioctl_enable_p9_wait(struct ocxl_context *ctx, + struct ocxl_ioctl_p9_wait __user *uarg) +{ + struct ocxl_ioctl_p9_wait arg; + + memset(, 0, sizeof(arg)); + + if (cpu_has_feature(CPU_FTR_P9_TIDR)) { + enum ocxl_context_status status; + + // Locks both status & tidr + mutex_lock(>status_mutex); + if (!ctx->tidr) { + if (set_thread_tidr(current)) + return -ENOENT; + + ctx->tidr = current->thread.tidr; + } + + status = ctx->status; + mutex_unlock(>status_mutex); + + if (status == ATTACHED) { + int rc; + struct link *link = ctx->afu->fn->link; + + rc = ocxl_link_update_pe(link, ctx->pasid, ctx->tidr); + if (rc) + return rc; + } + + arg.thread_id = ctx->tidr; + } else + return -ENOENT; + + if (copy_to_user(uarg, , sizeof(arg))) + return -EFAULT; + + return 0; +} +#endif + #define CMD_STR(x) (x == OCXL_IOCTL_ATTACH ? "ATTACH" : \ x == OCXL_IOCTL_IRQ_ALLOC ? "IRQ_ALLOC" : \ x == OCXL_IOCTL_IRQ_FREE ? "IRQ_FREE" : \ x == OCXL_IOCTL_IRQ_SET_FD ? "IRQ_SET_FD" : \ x == OCXL_IOCTL_GET_METADATA ? "GET_METADATA" : \ + x == OCXL_IOCTL_ENABLE_P9_WAIT ? "ENABLE_P9_WAIT" : \ "UNKNOWN") static long afu_ioctl(struct file *file, unsigned int cmd, @@ -186,6 +232,13 @@ static long afu_ioctl(struct file *file, unsigned int cmd, (struct ocxl_ioctl_metadata __user *) args); break; +#ifdef CONFIG_PPC64 + case OCXL_IOCTL_ENABLE_P9_WAIT: + rc = afu_ioctl_enable_p9_wait(ctx, + (struct ocxl_ioctl_p9_wait __user *) args); + break; +#endif + default: rc = -EINVAL; } diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c index 656e8610eec2..88876ae8f330 100644 --- a/drivers/misc/ocxl/link.c +++ b/drivers/misc/ocxl/link.c @@ -544,6 +544,42 @@ int
[PATCH v5 5/7] ocxl: Expose the thread_id needed for wait on POWER9
From: Alastair D'SilvaIn order to successfully issue as_notify, an AFU needs to know the TID to notify, which in turn means that this information should be available in userspace so it can be communicated to the AFU. Signed-off-by: Alastair D'Silva --- drivers/misc/ocxl/context.c | 5 ++- drivers/misc/ocxl/file.c | 53 +++ drivers/misc/ocxl/link.c | 36 + drivers/misc/ocxl/ocxl_internal.h | 1 + include/misc/ocxl.h | 9 ++ include/uapi/misc/ocxl.h | 8 + 6 files changed, 111 insertions(+), 1 deletion(-) diff --git a/drivers/misc/ocxl/context.c b/drivers/misc/ocxl/context.c index 909e8807824a..95f74623113e 100644 --- a/drivers/misc/ocxl/context.c +++ b/drivers/misc/ocxl/context.c @@ -34,6 +34,8 @@ int ocxl_context_init(struct ocxl_context *ctx, struct ocxl_afu *afu, mutex_init(>xsl_error_lock); mutex_init(>irq_lock); idr_init(>irq_idr); + ctx->tidr = 0; + /* * Keep a reference on the AFU to make sure it's valid for the * duration of the life of the context @@ -65,6 +67,7 @@ int ocxl_context_attach(struct ocxl_context *ctx, u64 amr) { int rc; + // Locks both status & tidr mutex_lock(>status_mutex); if (ctx->status != OPENED) { rc = -EIO; @@ -72,7 +75,7 @@ int ocxl_context_attach(struct ocxl_context *ctx, u64 amr) } rc = ocxl_link_add_pe(ctx->afu->fn->link, ctx->pasid, - current->mm->context.id, 0, amr, current->mm, + current->mm->context.id, ctx->tidr, amr, current->mm, xsl_fault_error, ctx); if (rc) goto out; diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c index 038509e5d031..eb409a469f21 100644 --- a/drivers/misc/ocxl/file.c +++ b/drivers/misc/ocxl/file.c @@ -5,6 +5,8 @@ #include #include #include +#include +#include #include "ocxl_internal.h" @@ -123,11 +125,55 @@ static long afu_ioctl_get_metadata(struct ocxl_context *ctx, return 0; } +#ifdef CONFIG_PPC64 +static long afu_ioctl_enable_p9_wait(struct ocxl_context *ctx, + struct ocxl_ioctl_p9_wait __user *uarg) +{ + struct ocxl_ioctl_p9_wait arg; + + memset(, 0, sizeof(arg)); + + if (cpu_has_feature(CPU_FTR_P9_TIDR)) { + enum ocxl_context_status status; + + // Locks both status & tidr + mutex_lock(>status_mutex); + if (!ctx->tidr) { + if (set_thread_tidr(current)) + return -ENOENT; + + ctx->tidr = current->thread.tidr; + } + + status = ctx->status; + mutex_unlock(>status_mutex); + + if (status == ATTACHED) { + int rc; + struct link *link = ctx->afu->fn->link; + + rc = ocxl_link_update_pe(link, ctx->pasid, ctx->tidr); + if (rc) + return rc; + } + + arg.thread_id = ctx->tidr; + } else + return -ENOENT; + + if (copy_to_user(uarg, , sizeof(arg))) + return -EFAULT; + + return 0; +} +#endif + #define CMD_STR(x) (x == OCXL_IOCTL_ATTACH ? "ATTACH" : \ x == OCXL_IOCTL_IRQ_ALLOC ? "IRQ_ALLOC" : \ x == OCXL_IOCTL_IRQ_FREE ? "IRQ_FREE" : \ x == OCXL_IOCTL_IRQ_SET_FD ? "IRQ_SET_FD" : \ x == OCXL_IOCTL_GET_METADATA ? "GET_METADATA" : \ + x == OCXL_IOCTL_ENABLE_P9_WAIT ? "ENABLE_P9_WAIT" : \ "UNKNOWN") static long afu_ioctl(struct file *file, unsigned int cmd, @@ -186,6 +232,13 @@ static long afu_ioctl(struct file *file, unsigned int cmd, (struct ocxl_ioctl_metadata __user *) args); break; +#ifdef CONFIG_PPC64 + case OCXL_IOCTL_ENABLE_P9_WAIT: + rc = afu_ioctl_enable_p9_wait(ctx, + (struct ocxl_ioctl_p9_wait __user *) args); + break; +#endif + default: rc = -EINVAL; } diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c index 656e8610eec2..88876ae8f330 100644 --- a/drivers/misc/ocxl/link.c +++ b/drivers/misc/ocxl/link.c @@ -544,6 +544,42 @@ int ocxl_link_add_pe(void *link_handle, int pasid, u32 pidr, u32 tidr, } EXPORT_SYMBOL_GPL(ocxl_link_add_pe); +int ocxl_link_update_pe(void *link_handle, int pasid, __u16 tid) +{ + struct link *link = (struct link *) link_handle; + struct spa *spa = link->spa; + struct ocxl_process_element *pe; + int pe_handle, rc; + + if (pasid > SPA_PASID_MAX)