Re: [PATCH v5 5/7] ocxl: Expose the thread_id needed for wait on POWER9

2018-05-11 Thread Frederic Barrat



Le 11/05/2018 à 12:06, Alastair D'Silva a écrit :



-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.


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

2018-05-11 Thread Alastair D'Silva

> -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

2018-05-11 Thread Frederic Barrat



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?


  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

2018-05-11 Thread Alastair D'Silva
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 
---
 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)