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

Frederic Barrat <fbarrat@xxxxxxxxxxxxx> · Fri, 11 May 2018 13:03:41 +0200

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

-----Original Message-----
From: Frederic Barrat <fbarrat@xxxxxxxxxxxxx>
Sent: Friday, 11 May 2018 7:25 PM
To: Alastair D'Silva <alastair@xxxxxxxxxxx>; linuxppc-dev@xxxxxxxxxxxxxxxx
Cc: linux-kernel@xxxxxxxxxxxxxxx; linux-doc@xxxxxxxxxxxxxxx;
mikey@xxxxxxxxxxx; vaibhav@xxxxxxxxxxxxxxxxxx;
aneesh.kumar@xxxxxxxxxxxxxxxxxx; malat@xxxxxxxxxx;
felix@xxxxxxxxxxxxxxxxxx; pombredanne@xxxxxxxx;
sukadev@xxxxxxxxxxxxxxxxxx; npiggin@xxxxxxxxx;
gregkh@xxxxxxxxxxxxxxxxxxx; arnd@xxxxxxxx;
andrew.donnellan@xxxxxxxxxxx; fbarrat@xxxxxxxxxxxxxxxxxx;
corbet@xxxxxxx; Alastair D'Silva <alastair@xxxxxxxxxxx>
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 <alastair@xxxxxxxxxxx>

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 <alastair@xxxxxxxxxxx>
---

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(&ctx->xsl_error_lock);
   	mutex_init(&ctx->irq_lock);
   	idr_init(&ctx->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(&ctx->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 <linux/sched/signal.h>
   #include <linux/uaccess.h>
   #include <uapi/misc/ocxl.h>
+#include <asm/reg.h>
+#include <asm/switch_to.h>
   #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(&arg, 0, sizeof(arg));
+
+	if (cpu_has_feature(CPU_FTR_P9_TIDR)) {
+		enum ocxl_context_status status;
+
+		// Locks both status & tidr
+		mutex_lock(&ctx->status_mutex);
+		if (!ctx->tidr) {
+			if (set_thread_tidr(current))
+				return -ENOENT;
+
+			ctx->tidr = current->thread.tidr;
+		}
+
+		status = ctx->status;
+		mutex_unlock(&ctx->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, &arg, 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)
+		return -EINVAL;
+
+	pe_handle = pasid & SPA_PE_MASK;
+	pe = spa->spa_mem + pe_handle;
+
+	mutex_lock(&spa->spa_lock);
+
+	pe->tid = tid;
+
+	/*
+	 * The barrier makes sure the PE is updated
+	 * before we clear the NPU context cache below, so that the
+	 * old PE cannot be reloaded erroneously.
+	 */
+	mb();
+
+	/*
+	 * hook to platform code
+	 * On powerpc, the entry needs to be cleared from the context
+	 * cache of the NPU.
+	 */
+	rc = pnv_ocxl_spa_remove_pe_from_cache(link->platform_data,
pe_handle);
+	WARN_ON(rc);
+
+	mutex_unlock(&spa->spa_lock);
+	return rc;
+}
+
   int ocxl_link_remove_pe(void *link_handle, int pasid)
   {
   	struct link *link = (struct link *) link_handle; diff --git
a/drivers/misc/ocxl/ocxl_internal.h
b/drivers/misc/ocxl/ocxl_internal.h
index 5d421824afd9..a32f2151029f 100644
--- a/drivers/misc/ocxl/ocxl_internal.h
+++ b/drivers/misc/ocxl/ocxl_internal.h
@@ -77,6 +77,7 @@ struct ocxl_context {
   	struct ocxl_xsl_error xsl_error;
   	struct mutex irq_lock;
   	struct idr irq_idr;
+	u16 tidr; // Thread ID used for P9 wait implementation
   };

   struct ocxl_process_element {
diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h index
51ccf76db293..9ff6ddc28e22 100644
--- a/include/misc/ocxl.h
+++ b/include/misc/ocxl.h
@@ -188,6 +188,15 @@ extern int ocxl_link_add_pe(void *link_handle, int
pasid, u32 pidr, u32 tidr,
   		void (*xsl_err_cb)(void *data, u64 addr, u64 dsisr),
   		void *xsl_err_data);

+/**
+ * Update values within a Process Element
+ *
+ * link_handle: the link handle associated with the process element
+ * pasid: the PASID for the AFU context
+ * tid: the new thread id for the process element  */ extern int
+ocxl_link_update_pe(void *link_handle, int pasid, __u16 tid);
+
   /*
    * Remove a Process Element from the Shared Process Area for a link
    */
diff --git a/include/uapi/misc/ocxl.h b/include/uapi/misc/ocxl.h index
0af83d80fb3e..561e6f0dfcb7 100644
--- a/include/uapi/misc/ocxl.h
+++ b/include/uapi/misc/ocxl.h
@@ -48,6 +48,13 @@ struct ocxl_ioctl_metadata {
   	__u64 reserved[13]; // Total of 16*u64
   };

+struct ocxl_ioctl_p9_wait {
+	__u16 thread_id; // The thread ID required to wake this thread
+	__u16 reserved1;
+	__u32 reserved2;
+	__u64 reserved3[3];
+};
+
   struct ocxl_ioctl_irq_fd {
   	__u64 irq_offset;
   	__s32 eventfd;
@@ -62,5 +69,6 @@ struct ocxl_ioctl_irq_fd {
   #define OCXL_IOCTL_IRQ_FREE	_IOW(OCXL_MAGIC, 0x12, __u64)
   #define OCXL_IOCTL_IRQ_SET_FD	_IOW(OCXL_MAGIC, 0x13, struct
ocxl_ioctl_irq_fd)
   #define OCXL_IOCTL_GET_METADATA _IOR(OCXL_MAGIC, 0x14, struct
ocxl_ioctl_metadata)
+#define OCXL_IOCTL_ENABLE_P9_WAIT	_IOR(OCXL_MAGIC, 0x15,
struct ocxl_ioctl_p9_wait)

   #endif /* _UAPI_MISC_OCXL_H */



---
This email has been checked for viruses by AVG.
http://www.avg.com



--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html






