LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 7/7] ocxl: Document new OCXL IOCTLs
From: Alastair D'Silva @ 2018-04-18  1:08 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: linux-kernel, linux-doc, mikey, vaibhav, aneesh.kumar, malat,
	felix, pombredanne, sukadev, npiggin, gregkh, arnd,
	andrew.donnellan, fbarrat, corbet, Alastair D'Silva
In-Reply-To: <20180418010810.30937-1-alastair@au1.ibm.com>

From: Alastair D'Silva <alastair@d-silva.org>

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 Documentation/accelerators/ocxl.rst | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/accelerators/ocxl.rst b/Documentation/accelerators/ocxl.rst
index 7904adcc07fd..3b8d3b99795c 100644
--- a/Documentation/accelerators/ocxl.rst
+++ b/Documentation/accelerators/ocxl.rst
@@ -157,6 +157,17 @@ OCXL_IOCTL_GET_METADATA:
   Obtains configuration information from the card, such at the size of
   MMIO areas, the AFU version, and the PASID for the current context.
 
+OCXL_IOCTL_ENABLE_P9_WAIT:
+
+  Allows the AFU to wake a userspace thread executing 'wait'. Returns
+  information to userspace to allow it to configure the AFU. Note that
+  this is only available on Power 9.
+
+OCXL_IOCTL_GET_FEATURES:
+
+  Reports on which CPU features that affect OpenCAPI are usable from
+  userspace.
+
 mmap
 ----
 
-- 
2.14.3

^ permalink raw reply related

* [PATCH v2 1/7] powerpc: Add TIDR CPU feature for Power9
From: Alastair D'Silva @ 2018-04-18  1:08 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: linux-kernel, linux-doc, mikey, vaibhav, aneesh.kumar, malat,
	felix, pombredanne, sukadev, npiggin, gregkh, arnd,
	andrew.donnellan, fbarrat, corbet, Alastair D'Silva
In-Reply-To: <20180418010810.30937-1-alastair@au1.ibm.com>

From: Alastair D'Silva <alastair@d-silva.org>

This patch adds a CPU feature bit to show whether the CPU has
the TIDR register available, enabling as_notify/wait in userspace.

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 arch/powerpc/include/asm/cputable.h | 3 ++-
 arch/powerpc/kernel/dt_cpu_ftrs.c   | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
index 4e332f3531c5..54c4cbbe57b4 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -215,6 +215,7 @@ static inline void cpu_feature_keys_init(void) { }
 #define CPU_FTR_P9_TM_HV_ASSIST		LONG_ASM_CONST(0x0000100000000000)
 #define CPU_FTR_P9_TM_XER_SO_BUG	LONG_ASM_CONST(0x0000200000000000)
 #define CPU_FTR_P9_TLBIE_BUG		LONG_ASM_CONST(0x0000400000000000)
+#define CPU_FTR_P9_TIDR			LONG_ASM_CONST(0x0000800000000000)
 
 #ifndef __ASSEMBLY__
 
@@ -462,7 +463,7 @@ static inline void cpu_feature_keys_init(void) { }
 	    CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY | \
 	    CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_ARCH_207S | \
 	    CPU_FTR_TM_COMP | CPU_FTR_ARCH_300 | CPU_FTR_PKEY | \
-	    CPU_FTR_P9_TLBIE_BUG)
+	    CPU_FTR_P9_TLBIE_BUG | CPU_FTR_P9_TIDR)
 #define CPU_FTRS_POWER9_DD1 ((CPU_FTRS_POWER9 | CPU_FTR_POWER9_DD1) & \
 			     (~CPU_FTR_SAO))
 #define CPU_FTRS_POWER9_DD2_0 CPU_FTRS_POWER9
diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
index 11a3a4fed3fb..10f8b7f55637 100644
--- a/arch/powerpc/kernel/dt_cpu_ftrs.c
+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
@@ -722,6 +722,7 @@ static __init void cpufeatures_cpu_quirks(void)
 	if ((version & 0xffff0000) == 0x004e0000) {
 		cur_cpu_spec->cpu_features &= ~(CPU_FTR_DAWR);
 		cur_cpu_spec->cpu_features |= CPU_FTR_P9_TLBIE_BUG;
+		cur_cpu_spec->cpu_features |= CPU_FTR_P9_TIDR;
 	}
 }
 
-- 
2.14.3

^ permalink raw reply related

* [PATCH v2 6/7] ocxl: Add an IOCTL so userspace knows what CPU features are available
From: Alastair D'Silva @ 2018-04-18  1:08 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: linux-kernel, linux-doc, mikey, vaibhav, aneesh.kumar, malat,
	felix, pombredanne, sukadev, npiggin, gregkh, arnd,
	andrew.donnellan, fbarrat, corbet, Alastair D'Silva
In-Reply-To: <20180418010810.30937-1-alastair@au1.ibm.com>

From: Alastair D'Silva <alastair@d-silva.org>

In order for a userspace AFU driver to call the Power9 specific
OCXL_IOCTL_ENABLE_P9_WAIT, it needs to verify that it can actually
make that call.

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 Documentation/accelerators/ocxl.rst |  1 -
 drivers/misc/ocxl/file.c            | 25 +++++++++++++++++++++++++
 include/uapi/misc/ocxl.h            |  4 ++++
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/Documentation/accelerators/ocxl.rst b/Documentation/accelerators/ocxl.rst
index ddcc58d01cfb..7904adcc07fd 100644
--- a/Documentation/accelerators/ocxl.rst
+++ b/Documentation/accelerators/ocxl.rst
@@ -157,7 +157,6 @@ OCXL_IOCTL_GET_METADATA:
   Obtains configuration information from the card, such at the size of
   MMIO areas, the AFU version, and the PASID for the current context.
 
-
 mmap
 ----
 
diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
index eb409a469f21..33ae46ce0a8a 100644
--- a/drivers/misc/ocxl/file.c
+++ b/drivers/misc/ocxl/file.c
@@ -168,12 +168,32 @@ static long afu_ioctl_enable_p9_wait(struct ocxl_context *ctx,
 }
 #endif
 
+
+static long afu_ioctl_get_features(struct ocxl_context *ctx,
+		struct ocxl_ioctl_features __user *uarg)
+{
+	struct ocxl_ioctl_features arg;
+
+	memset(&arg, 0, sizeof(arg));
+
+#ifdef CONFIG_PPC64
+	if (cpu_has_feature(CPU_FTR_P9_TIDR))
+		arg.flags[0] |= OCXL_IOCTL_FEATURES_FLAGS0_P9_WAIT;
+#endif
+
+	if (copy_to_user(uarg, &arg, sizeof(arg)))
+		return -EFAULT;
+
+	return 0;
+}
+
 #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" :	\
+			x == OCXL_IOCTL_GET_FEATURES ? "GET_FEATURES" :	\
 			"UNKNOWN")
 
 static long afu_ioctl(struct file *file, unsigned int cmd,
@@ -239,6 +259,11 @@ static long afu_ioctl(struct file *file, unsigned int cmd,
 		break;
 #endif
 
+	case OCXL_IOCTL_GET_FEATURES:
+		rc = afu_ioctl_get_features(ctx,
+				(struct ocxl_ioctl_features __user *) args);
+		break;
+
 	default:
 		rc = -EINVAL;
 	}
diff --git a/include/uapi/misc/ocxl.h b/include/uapi/misc/ocxl.h
index 8d2748e69c84..bb80f294b429 100644
--- a/include/uapi/misc/ocxl.h
+++ b/include/uapi/misc/ocxl.h
@@ -55,6 +55,9 @@ struct ocxl_ioctl_p9_wait {
 	__u64 reserved3[3];
 };
 
+#define OCXL_IOCTL_FEATURES_FLAGS0_P9_WAIT	0x01
+struct ocxl_ioctl_features {
+	__u64 flags[4];
 };
 
 struct ocxl_ioctl_irq_fd {
@@ -72,5 +75,6 @@ struct ocxl_ioctl_irq_fd {
 #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)
+#define OCXL_IOCTL_GET_FEATURES _IOR(OCXL_MAGIC, 0x16, struct ocxl_ioctl_platform)
 
 #endif /* _UAPI_MISC_OCXL_H */
-- 
2.14.3

^ permalink raw reply related

* [PATCH v2 5/7] ocxl: Expose the thread_id needed for wait on p9
From: Alastair D'Silva @ 2018-04-18  1:08 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: linux-kernel, linux-doc, mikey, vaibhav, aneesh.kumar, malat,
	felix, pombredanne, sukadev, npiggin, gregkh, arnd,
	andrew.donnellan, fbarrat, corbet, Alastair D'Silva
In-Reply-To: <20180418010810.30937-1-alastair@au1.ibm.com>

From: Alastair D'Silva <alastair@d-silva.org>

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@d-silva.org>
---
 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          | 10 ++++++++
 6 files changed, 113 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..6c6d4e61888e 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..8d2748e69c84 100644
--- a/include/uapi/misc/ocxl.h
+++ b/include/uapi/misc/ocxl.h
@@ -48,6 +48,15 @@ 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 +71,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 */
-- 
2.14.3

^ permalink raw reply related

* [PATCH v2 3/7] powerpc: use task_pid_nr() for TID allocation
From: Alastair D'Silva @ 2018-04-18  1:08 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: linux-kernel, linux-doc, mikey, vaibhav, aneesh.kumar, malat,
	felix, pombredanne, sukadev, npiggin, gregkh, arnd,
	andrew.donnellan, fbarrat, corbet, Alastair D'Silva
In-Reply-To: <20180418010810.30937-1-alastair@au1.ibm.com>

From: Alastair D'Silva <alastair@d-silva.org>

The current implementation of TID allocation, using a global IDR, may
result in an errant process starving the system of available TIDs.
Instead, use task_pid_nr(), as mentioned by the original author. The
scenario described which prevented it's use is not applicable, as
set_thread_tidr can only be called after the task struct has been
populated.

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 arch/powerpc/include/asm/switch_to.h |  1 -
 arch/powerpc/kernel/process.c        | 97 +-----------------------------------
 2 files changed, 1 insertion(+), 97 deletions(-)

diff --git a/arch/powerpc/include/asm/switch_to.h b/arch/powerpc/include/asm/switch_to.h
index be8c9fa23983..5b03d8a82409 100644
--- a/arch/powerpc/include/asm/switch_to.h
+++ b/arch/powerpc/include/asm/switch_to.h
@@ -94,6 +94,5 @@ static inline void clear_task_ebb(struct task_struct *t)
 extern int set_thread_uses_vas(void);
 
 extern int set_thread_tidr(struct task_struct *t);
-extern void clear_thread_tidr(struct task_struct *t);
 
 #endif /* _ASM_POWERPC_SWITCH_TO_H */
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 3b00da47699b..87f047fd2762 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1496,103 +1496,12 @@ int set_thread_uses_vas(void)
 }
 
 #ifdef CONFIG_PPC64
-static DEFINE_SPINLOCK(vas_thread_id_lock);
-static DEFINE_IDA(vas_thread_ida);
-
-/*
- * We need to assign a unique thread id to each thread in a process.
- *
- * This thread id, referred to as TIDR, and separate from the Linux's tgid,
- * is intended to be used to direct an ASB_Notify from the hardware to the
- * thread, when a suitable event occurs in the system.
- *
- * One such event is a "paste" instruction in the context of Fast Thread
- * Wakeup (aka Core-to-core wake up in the Virtual Accelerator Switchboard
- * (VAS) in POWER9.
- *
- * To get a unique TIDR per process we could simply reuse task_pid_nr() but
- * the problem is that task_pid_nr() is not yet available copy_thread() is
- * called. Fixing that would require changing more intrusive arch-neutral
- * code in code path in copy_process()?.
- *
- * Further, to assign unique TIDRs within each process, we need an atomic
- * field (or an IDR) in task_struct, which again intrudes into the arch-
- * neutral code. So try to assign globally unique TIDRs for now.
- *
- * NOTE: TIDR 0 indicates that the thread does not need a TIDR value.
- *	 For now, only threads that expect to be notified by the VAS
- *	 hardware need a TIDR value and we assign values > 0 for those.
- */
-#define MAX_THREAD_CONTEXT	((1 << 16) - 1)
-static int assign_thread_tidr(void)
-{
-	int index;
-	int err;
-	unsigned long flags;
-
-again:
-	if (!ida_pre_get(&vas_thread_ida, GFP_KERNEL))
-		return -ENOMEM;
-
-	spin_lock_irqsave(&vas_thread_id_lock, flags);
-	err = ida_get_new_above(&vas_thread_ida, 1, &index);
-	spin_unlock_irqrestore(&vas_thread_id_lock, flags);
-
-	if (err == -EAGAIN)
-		goto again;
-	else if (err)
-		return err;
-
-	if (index > MAX_THREAD_CONTEXT) {
-		spin_lock_irqsave(&vas_thread_id_lock, flags);
-		ida_remove(&vas_thread_ida, index);
-		spin_unlock_irqrestore(&vas_thread_id_lock, flags);
-		return -ENOMEM;
-	}
-
-	return index;
-}
-
-static void free_thread_tidr(int id)
-{
-	unsigned long flags;
-
-	spin_lock_irqsave(&vas_thread_id_lock, flags);
-	ida_remove(&vas_thread_ida, id);
-	spin_unlock_irqrestore(&vas_thread_id_lock, flags);
-}
-
-/*
- * Clear any TIDR value assigned to this thread.
- */
-void clear_thread_tidr(struct task_struct *t)
-{
-	if (!t->thread.tidr)
-		return;
-
-	if (!cpu_has_feature(CPU_FTR_P9_TIDR)) {
-		WARN_ON_ONCE(1);
-		return;
-	}
-
-	mtspr(SPRN_TIDR, 0);
-	free_thread_tidr(t->thread.tidr);
-	t->thread.tidr = 0;
-}
-
-void arch_release_task_struct(struct task_struct *t)
-{
-	clear_thread_tidr(t);
-}
-
 /*
  * Assign a unique TIDR (thread id) for task @t and set it in the thread
  * structure. For now, we only support setting TIDR for 'current' task.
  */
 int set_thread_tidr(struct task_struct *t)
 {
-	int rc;
-
 	if (!cpu_has_feature(CPU_FTR_P9_TIDR))
 		return -EINVAL;
 
@@ -1602,11 +1511,7 @@ int set_thread_tidr(struct task_struct *t)
 	if (t->thread.tidr)
 		return 0;
 
-	rc = assign_thread_tidr();
-	if (rc < 0)
-		return rc;
-
-	t->thread.tidr = rc;
+	t->thread.tidr = (u16)task_pid_nr(t);
 	mtspr(SPRN_TIDR, t->thread.tidr);
 
 	return 0;
-- 
2.14.3

^ permalink raw reply related

* [PATCH v2 0/7] ocxl: Implement Power9 as_notify/wait for OpenCAPI
From: Alastair D'Silva @ 2018-04-18  1:08 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: linux-kernel, linux-doc, mikey, vaibhav, aneesh.kumar, malat,
	felix, pombredanne, sukadev, npiggin, gregkh, arnd,
	andrew.donnellan, fbarrat, corbet, Alastair D'Silva
In-Reply-To: <20180417020950.21446-1-alastair@au1.ibm.com>

From: Alastair D'Silva <alastair@d-silva.org>

The Power 9 as_notify/wait feature provides a lower latency way to
signal a thread that work is complete. This series enables the use of
this feature from OpenCAPI adapters, as well as addressing a potential
starvation issue when allocating thread IDs.

Changelog:
v2:
  Rename get_platform IOCTL to get_features
  Move stray edit from patch 1 to patch 3

Alastair D'Silva (7):
  powerpc: Add TIDR CPU feature for Power9
  powerpc: Use TIDR CPU feature to control TIDR allocation
  powerpc: use task_pid_nr() for TID allocation
  ocxl: Rename pnv_ocxl_spa_remove_pe to clarify it's action
  ocxl: Expose the thread_id needed for wait on p9
  ocxl: Add an IOCTL so userspace knows what CPU features are available
  ocxl: Document new OCXL IOCTLs

 Documentation/accelerators/ocxl.rst   |  10 ++++
 arch/powerpc/include/asm/cputable.h   |   3 +-
 arch/powerpc/include/asm/pnv-ocxl.h   |   2 +-
 arch/powerpc/include/asm/switch_to.h  |   1 -
 arch/powerpc/kernel/dt_cpu_ftrs.c     |   1 +
 arch/powerpc/kernel/process.c         | 101 +---------------------------------
 arch/powerpc/platforms/powernv/ocxl.c |   4 +-
 drivers/misc/ocxl/context.c           |   5 +-
 drivers/misc/ocxl/file.c              |  78 ++++++++++++++++++++++++++
 drivers/misc/ocxl/link.c              |  38 ++++++++++++-
 drivers/misc/ocxl/ocxl_internal.h     |   1 +
 include/misc/ocxl.h                   |   9 +++
 include/uapi/misc/ocxl.h              |  14 +++++
 13 files changed, 162 insertions(+), 105 deletions(-)

-- 
2.14.3

^ permalink raw reply

* [PATCH v2 4/7] ocxl: Rename pnv_ocxl_spa_remove_pe to clarify it's action
From: Alastair D'Silva @ 2018-04-18  1:08 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: linux-kernel, linux-doc, mikey, vaibhav, aneesh.kumar, malat,
	felix, pombredanne, sukadev, npiggin, gregkh, arnd,
	andrew.donnellan, fbarrat, corbet, Alastair D'Silva
In-Reply-To: <20180418010810.30937-1-alastair@au1.ibm.com>

From: Alastair D'Silva <alastair@d-silva.org>

The function removes the process element from NPU cache.

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 arch/powerpc/include/asm/pnv-ocxl.h   | 2 +-
 arch/powerpc/platforms/powernv/ocxl.c | 4 ++--
 drivers/misc/ocxl/link.c              | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/include/asm/pnv-ocxl.h b/arch/powerpc/include/asm/pnv-ocxl.h
index f6945d3bc971..208b5503f4ed 100644
--- a/arch/powerpc/include/asm/pnv-ocxl.h
+++ b/arch/powerpc/include/asm/pnv-ocxl.h
@@ -28,7 +28,7 @@ extern int pnv_ocxl_map_xsl_regs(struct pci_dev *dev, void __iomem **dsisr,
 extern int pnv_ocxl_spa_setup(struct pci_dev *dev, void *spa_mem, int PE_mask,
 			void **platform_data);
 extern void pnv_ocxl_spa_release(void *platform_data);
-extern int pnv_ocxl_spa_remove_pe(void *platform_data, int pe_handle);
+extern int pnv_ocxl_spa_remove_pe_from_cache(void *platform_data, int pe_handle);
 
 extern int pnv_ocxl_alloc_xive_irq(u32 *irq, u64 *trigger_addr);
 extern void pnv_ocxl_free_xive_irq(u32 irq);
diff --git a/arch/powerpc/platforms/powernv/ocxl.c b/arch/powerpc/platforms/powernv/ocxl.c
index fa9b53af3c7b..8c65aacda9c8 100644
--- a/arch/powerpc/platforms/powernv/ocxl.c
+++ b/arch/powerpc/platforms/powernv/ocxl.c
@@ -475,7 +475,7 @@ void pnv_ocxl_spa_release(void *platform_data)
 }
 EXPORT_SYMBOL_GPL(pnv_ocxl_spa_release);
 
-int pnv_ocxl_spa_remove_pe(void *platform_data, int pe_handle)
+int pnv_ocxl_spa_remove_pe_from_cache(void *platform_data, int pe_handle)
 {
 	struct spa_data *data = (struct spa_data *) platform_data;
 	int rc;
@@ -483,7 +483,7 @@ int pnv_ocxl_spa_remove_pe(void *platform_data, int pe_handle)
 	rc = opal_npu_spa_clear_cache(data->phb_opal_id, data->bdfn, pe_handle);
 	return rc;
 }
-EXPORT_SYMBOL_GPL(pnv_ocxl_spa_remove_pe);
+EXPORT_SYMBOL_GPL(pnv_ocxl_spa_remove_pe_from_cache);
 
 int pnv_ocxl_alloc_xive_irq(u32 *irq, u64 *trigger_addr)
 {
diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c
index f30790582dc0..656e8610eec2 100644
--- a/drivers/misc/ocxl/link.c
+++ b/drivers/misc/ocxl/link.c
@@ -599,7 +599,7 @@ int ocxl_link_remove_pe(void *link_handle, int pasid)
 	 * On powerpc, the entry needs to be cleared from the context
 	 * cache of the NPU.
 	 */
-	rc = pnv_ocxl_spa_remove_pe(link->platform_data, pe_handle);
+	rc = pnv_ocxl_spa_remove_pe_from_cache(link->platform_data, pe_handle);
 	WARN_ON(rc);
 
 	pe_data = radix_tree_delete(&spa->pe_tree, pe_handle);
-- 
2.14.3

^ permalink raw reply related

* [PATCH v2 2/7] powerpc: Use TIDR CPU feature to control TIDR allocation
From: Alastair D'Silva @ 2018-04-18  1:08 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: linux-kernel, linux-doc, mikey, vaibhav, aneesh.kumar, malat,
	felix, pombredanne, sukadev, npiggin, gregkh, arnd,
	andrew.donnellan, fbarrat, corbet, Alastair D'Silva
In-Reply-To: <20180418010810.30937-1-alastair@au1.ibm.com>

From: Alastair D'Silva <alastair@d-silva.org>

Switch the use of TIDR on it's CPU feature, rather than assuming it
is available based on architecture.

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 arch/powerpc/kernel/process.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 1237f13fed51..3b00da47699b 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1154,7 +1154,7 @@ static inline void restore_sprs(struct thread_struct *old_thread,
 			mtspr(SPRN_TAR, new_thread->tar);
 	}
 
-	if (cpu_has_feature(CPU_FTR_ARCH_300) &&
+	if (cpu_has_feature(CPU_FTR_P9_TIDR) &&
 	    old_thread->tidr != new_thread->tidr)
 		mtspr(SPRN_TIDR, new_thread->tidr);
 #endif
@@ -1570,7 +1570,7 @@ void clear_thread_tidr(struct task_struct *t)
 	if (!t->thread.tidr)
 		return;
 
-	if (!cpu_has_feature(CPU_FTR_ARCH_300)) {
+	if (!cpu_has_feature(CPU_FTR_P9_TIDR)) {
 		WARN_ON_ONCE(1);
 		return;
 	}
@@ -1593,7 +1593,7 @@ int set_thread_tidr(struct task_struct *t)
 {
 	int rc;
 
-	if (!cpu_has_feature(CPU_FTR_ARCH_300))
+	if (!cpu_has_feature(CPU_FTR_P9_TIDR))
 		return -EINVAL;
 
 	if (t != current)
-- 
2.14.3

^ permalink raw reply related

* Re: [PATCH] misc: cxl: Change return type to vm_fault_t
From: Andrew Donnellan @ 2018-04-17 23:51 UTC (permalink / raw)
  To: Souptick Joarder, fbarrat, arnd, gregkh; +Cc: linuxppc-dev, linux-kernel, willy
In-Reply-To: <20180417145354.GA31451@jordon-HP-15-Notebook-PC>

On 18/04/18 00:53, Souptick Joarder wrote:
> Use new return type vm_fault_t for fault handler. For
> now, this is just documenting that the function returns
> a VM_FAULT value rather than an errno. Once all instances
> are converted, vm_fault_t will become a distinct type.
> 
> Reference id -> 1c8f422059ae ("mm: change return type to
> vm_fault_t")
> 
> previously cxl_mmap_fault returns VM_FAULT_NOPAGE as
> default value irrespective of vm_insert_pfn() return
> value. This bug is fixed with new vmf_insert_pfn()
> which will return VM_FAULT_ type based on err.
> 
> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>

This looks good to me

Acked-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited

^ permalink raw reply

* Re: [PATCH V1 00/11] powerpc/mm/book3s64: Support for split pmd ptlock
From: Balbir Singh @ 2018-04-17 22:43 UTC (permalink / raw)
  To: Aneesh Kumar K.V; +Cc: benh, paulus, mpe, linuxppc-dev
In-Reply-To: <20180416112724.9677-1-aneesh.kumar@linux.ibm.com>

On Mon, 16 Apr 2018 16:57:12 +0530
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> wrote:

> This patch series add split pmd pagetable lock for book3s64. nohash64 also should
> be able to switch to this. I need to workout the code dependency. This series
> also migh have broken the build on platforms otherthan book3s64. I am sending this early
> to get feedback on whether we should continue with the approach.
> 
> We switch the pmd allocator to use something similar to what we already use for
> level 4 pagetable allocation. We get an order 0 page and divide that to fragments
> and hand over fragments when we get request for a pmd pagetable. The pmd lock is
> now stashed in the struct page backing the allocated page.

That's only for the THP case right?

> 
> The series helps in reducing lock contention on mm->page_table_lock.
>

The numbers look good.

^ permalink raw reply

* Re: [PATCH 1/2] powernv/npu: Do a PID GPU TLB flush when invalidating a large address range
From: Balbir Singh @ 2018-04-17 22:25 UTC (permalink / raw)
  To: Alistair Popple
  Cc: open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), Michael Ellerman,
	Mark Hairgrove, arbab
In-Reply-To: <CAKTCnz=nMDksZq_1pWXGqKFTQ8x5pO=Ojw=TQdPkT+ZqBwEGwA@mail.gmail.com>

On Tue, Apr 17, 2018 at 7:17 PM, Balbir Singh <bsingharora@gmail.com> wrote:
> On Tue, Apr 17, 2018 at 7:11 PM, Alistair Popple <alistair@popple.id.au> wrote:
>> The NPU has a limited number of address translation shootdown (ATSD)
>> registers and the GPU has limited bandwidth to process ATSDs. This can
>> result in contention of ATSD registers leading to soft lockups on some
>> threads, particularly when invalidating a large address range in
>> pnv_npu2_mn_invalidate_range().
>>
>> At some threshold it becomes more efficient to flush the entire GPU TLB for
>> the given MM context (PID) than individually flushing each address in the
>> range. This patch will result in ranges greater than 2MB being converted
>> from 32+ ATSDs into a single ATSD which will flush the TLB for the given
>> PID on each GPU.
>>
>> Signed-off-by: Alistair Popple <alistair@popple.id.au>
>> +       }
>>  }
>>
>
> Acked-by: Balbir Singh <bsingharora@gmail.com>
Tested-by: Balbir Singh <bsingharora@gmail.com>

^ permalink raw reply

* Re: [PATCH 2/2] powernv/npu: Add a debugfs setting to change ATSD threshold
From: Balbir Singh @ 2018-04-17 21:45 UTC (permalink / raw)
  To: Alistair Popple; +Cc: linuxppc-dev, mpe, mhairgrove, arbab
In-Reply-To: <20180417091129.23069-2-alistair@popple.id.au>

On Tue, 17 Apr 2018 19:11:29 +1000
Alistair Popple <alistair@popple.id.au> wrote:

> The threshold at which it becomes more efficient to coalesce a range of
> ATSDs into a single per-PID ATSD is currently not well understood due to a
> lack of real-world work loads. This patch adds a debugfs parameter allowing
> the threshold to be altered at runtime in order to aid future development
> and refinement of the value.
> 
> Signed-off-by: Alistair Popple <alistair@popple.id.au>
> ---
>  arch/powerpc/platforms/powernv/npu-dma.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
> index dc34662e9df9..a765bf576c14 100644
> --- a/arch/powerpc/platforms/powernv/npu-dma.c
> +++ b/arch/powerpc/platforms/powernv/npu-dma.c
> @@ -17,7 +17,9 @@
>  #include <linux/pci.h>
>  #include <linux/memblock.h>
>  #include <linux/iommu.h>
> +#include <linux/debugfs.h>
>  
> +#include <asm/debugfs.h>
>  #include <asm/tlb.h>
>  #include <asm/powernv.h>
>  #include <asm/reg.h>
> @@ -44,7 +46,8 @@ DEFINE_SPINLOCK(npu_context_lock);
>   * entire TLB on the GPU for the given PID rather than each specific address in
>   * the range.
>   */
> -#define ATSD_THRESHOLD (2*1024*1024)
> +static uint64_t atsd_threshold = 2 * 1024 * 1024;
> +static struct dentry *atsd_threshold_dentry;
>  
>  /*
>   * Other types of TCE cache invalidation are not functional in the
> @@ -682,7 +685,7 @@ static void pnv_npu2_mn_invalidate_range(struct mmu_notifier *mn,
>  	struct npu_context *npu_context = mn_to_npu_context(mn);
>  	unsigned long address;
>  
> -	if (end - start > ATSD_THRESHOLD) {
> +	if (end - start > atsd_threshold) {
>  		/*
>  		 * Just invalidate the entire PID if the address range is too
>  		 * large.
> @@ -956,6 +959,11 @@ int pnv_npu2_init(struct pnv_phb *phb)
>  	static int npu_index;
>  	uint64_t rc = 0;
>  
> +	if (!atsd_threshold_dentry) {
> +		atsd_threshold_dentry = debugfs_create_x64("atsd_threshold",

Nit-picking can we call this atsd_threshold_in_bytes?

> +				   0600, powerpc_debugfs_root, &atsd_threshold);
> +	}
> +
>  	phb->npu.nmmu_flush =
>  		of_property_read_bool(phb->hose->dn, "ibm,nmmu-flush");
>  	for_each_child_of_node(phb->hose->dn, dn) {

Acked-by: Balbir Singh <bsingharora@gmail.com>

^ permalink raw reply

* Re: [PATCH] powerpc: platform: cell: spufs: Change return type to vm_fault_t
From: Matthew Wilcox @ 2018-04-17 20:47 UTC (permalink / raw)
  To: Souptick Joarder; +Cc: jk, arnd, benh, paulus, mpe, linuxppc-dev
In-Reply-To: <20180417192038.GA22918@jordon-HP-15-Notebook-PC>

On Wed, Apr 18, 2018 at 12:50:38AM +0530, Souptick Joarder wrote:
> Use new return type vm_fault_t for fault handler. For
> now, this is just documenting that the function returns
> a VM_FAULT value rather than an errno. Once all instances
> are converted, vm_fault_t will become a distinct type.
> 
> Reference id -> 1c8f422059ae ("mm: change return type to
> vm_fault_t")
> 
> Previously vm_insert_pfn() returns err but driver returns 
> VM_FAULT_NOPAGE as default. The new function vmf_insert_pfn()
> will replace this inefficiency by returning correct VM_FAULT_*
> type.
> 
> vmf_handle_error is a inline wrapper function which
> will convert error number to vm_fault_t type err.

I think you sent the wrong version of this one ...

The commit message should mention that we're fixing a minor bug, that
the error from vm_insert_pfn() was being ignored and the effect of this
is likely to be only felt in OOM situations.

> @@ -256,11 +257,11 @@ static ssize_t spufs_attr_write(struct file *file, const char __user *buf,
>  		vma->vm_page_prot = pgprot_noncached_wc(vma->vm_page_prot);
>  		pfn = (ctx->spu->local_store_phys + offset) >> PAGE_SHIFT;
>  	}
> -	vm_insert_pfn(vma, vmf->address, pfn);
> +	ret = vmf_insert_pfn(vma, vmf->address, pfn);
>  
>  	spu_release(ctx);
>  
> -	return VM_FAULT_NOPAGE;
> +	return ret;
>  }

I thought I said not to introduce vmf_handle_error(), because it's too
trivial and obfuscates what's actually going on.

> -static int spufs_ps_fault(struct vm_fault *vmf,
> +static inline vm_fault_t vmf_handle_error(int err)
> +{
> +	return VM_FAULT_NOPAGE;
> +}
> +

Re-reading spufs_ps_fault(), I wouldn't change anything inside it.  Just
change its return type to vm_fault_t and call it done.

^ permalink raw reply

* Re: [PATCH] powerpc: platform: cell: spufs: Change return type to vm_fault_t
From: Arnd Bergmann @ 2018-04-17 19:43 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: Jeremy Kerr, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, linuxppc-dev, Matthew Wilcox
In-Reply-To: <20180417192038.GA22918@jordon-HP-15-Notebook-PC>

On Tue, Apr 17, 2018 at 9:20 PM, Souptick Joarder <jrdr.linux@gmail.com> wrote:
> Use new return type vm_fault_t for fault handler. For
> now, this is just documenting that the function returns
> a VM_FAULT value rather than an errno. Once all instances
> are converted, vm_fault_t will become a distinct type.
>
> Reference id -> 1c8f422059ae ("mm: change return type to
> vm_fault_t")
>
> Previously vm_insert_pfn() returns err but driver returns
> VM_FAULT_NOPAGE as default. The new function vmf_insert_pfn()
> will replace this inefficiency by returning correct VM_FAULT_*
> type.
>
> vmf_handle_error is a inline wrapper function which
> will convert error number to vm_fault_t type err.
>
> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> Reviewed-by: Matthew Wilcox <mawilcox@microsoft.com>

Acked-by: Arnd Bergmann <arnd@arndb.de>

^ permalink raw reply

* Re: [RFC PATCH 1/3] signal: Ensure every siginfo we send has all bits initialized
From: Eric W. Biederman @ 2018-04-17 19:37 UTC (permalink / raw)
  To: Dave Martin
  Cc: Linus Torvalds, linux-arch, Linux Kernel Mailing List,
	Dmitry V. Levin, sparclinux, Russell King - ARM Linux, ppc-dev,
	linux-arm-kernel
In-Reply-To: <20180417132328.GF16308@e103592.cambridge.arm.com>

Dave Martin <Dave.Martin@arm.com> writes:

> Hmmm
>
> memset()/clear_siginfo() may ensure that there are no uninitialised
> explicit fields except for those in inactive union members, but I'm not
> sure that this approach is guaranteed to sanitise the padding seen by
> userspace.
>
> Rationale below, though it's a bit theoretical...
>
> With this in mind, I tend agree with Linus that hiding memset() calls
> from the maintainer may be a bad idea unless they are also hidden from
> the compiler.  If the compiler sees the memset() it may be able to
> optimise it in ways that wouldn't be possible for some other random
> external function call, including optimising all or part of the call
> out.
>
> As a result, the breakdown into individual put_user()s etc. in
> copy_siginfo_to_user() may still be valuable even if all paths have the
> memset().

The breakdown into individual put_user()s is known to be problematically
slow, and is actually wrong.

Even exclusing the SI_USER duplication in a small number of cases the
fields filled out in siginfo by architecture code are not the fields
that copy_siginfo_to_user is copying.  Which is much worse.  The code
looks safe but is not.

My intention is to leave 0 instances of clear_siginfo in the
architecture specific code.  Ideally struct siginfo will be limited to
kernel/signal.c but I am not certain I can quite get that far.
The function do_coredump appears to have a legit need for siginfo.


> (Rationale for an arch/arm example:)
>
>> diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c
>> index 4c375e11ae95..adda3fc2dde8 100644
>> --- a/arch/arm/vfp/vfpmodule.c
>> +++ b/arch/arm/vfp/vfpmodule.c
>> @@ -218,8 +218,7 @@ static void vfp_raise_sigfpe(unsigned int sicode, struct pt_regs *regs)
>>  {
>>  	siginfo_t info;
>>  
>> -	memset(&info, 0, sizeof(info));
>> -
>> +	clear_siginfo(&info);
>>  	info.si_signo = SIGFPE;
>
> /* by c11 (n1570) 6.2.6.1 para 6 [1], all padding bytes in info now take
>    unspecified values */
>
>>  	info.si_code = sicode;
>>  	info.si_addr = (void __user *)(instruction_pointer(regs) - 4);
>
> /* by c11 (n1570) 6.2.6.1 para 7 [2], all bytes of the union info._sifields
>    other than than those corresponding to _sigfault take unspecified
>    values */
>
> So I don't see why the compiler needs to ensure that any of the affected
> bytes are zero: it could potentially skip a lot of the memset() as a
> result, in theory.
>
> I've not seen a compiler actually take advantage of that, but I'm now
> not sure what forbids it.

I took a quick look at gcc-4.9 which I have handy.

The passes -f-no-strict-aliasing which helps, and gcc actually
documents that if you access things through the union it will
not take advantage of c11.

gcc-4.9 Documents it this way:

> -fstrict-aliasing'
>      Allow the compiler to assume the strictest aliasing rules
>      applicable to the language being compiled.  For C (and C++), this
>      activates optimizations based on the type of expressions.  In
>      particular, an object of one type is assumed never to reside at the
>      same address as an object of a different type, unless the types are
>      almost the same.  For example, an 'unsigned int' can alias an
>      'int', but not a 'void*' or a 'double'.  A character type may alias
>      any other type.
> 
>      Pay special attention to code like this:
>           union a_union {
>             int i;
>             double d;
>           };
> 
>           int f() {
>             union a_union t;
>             t.d = 3.0;
>             return t.i;
>           }
>      The practice of reading from a different union member than the one
>      most recently written to (called "type-punning") is common.  Even
>      with '-fstrict-aliasing', type-punning is allowed, provided the
>      memory is accessed through the union type.  So, the code above
>      works as expected.


> If this can happen, I only see two watertight workarounds:
>
> 1) Ensure that there is no implicit padding in any UAPI structure, e.g.
> aeb1f39d814b: ("arm64/ptrace: Avoid uninitialised struct padding in
> fpr_set()").  This would include tail-padding of any union member that
> is smaller than the containing union.
>
> It would be significantly more effort to ensure this for siginfo though.
>
> 2) Poke all values directly into allocated or user memory directly
> via pointers to paddingless types; never assign to objects on the kernel
> stack if you care what ends up in the padding, e.g., what your
> copy_siginfo_to_user() does prior to this series.
>
>
> If I'm not barking up the wrong tree, memset() cannot generally be
> used to determine the value of padding bytes, but it may still be
> useful for forcing otherwise uninitialised members to sane initial
> values.
>
> This likely affects many more things than just siginfo.

Unless gcc has changed it's stance on type-punning through unions
or it's semantics with -fno-strict_aliasing we should be good.

Eric

^ permalink raw reply

* [PATCH] powerpc: platform: cell: spufs: Change return type to vm_fault_t
From: Souptick Joarder @ 2018-04-17 19:20 UTC (permalink / raw)
  To: jk, arnd, benh, paulus, mpe; +Cc: linuxppc-dev, willy

Use new return type vm_fault_t for fault handler. For
now, this is just documenting that the function returns
a VM_FAULT value rather than an errno. Once all instances
are converted, vm_fault_t will become a distinct type.

Reference id -> 1c8f422059ae ("mm: change return type to
vm_fault_t")

Previously vm_insert_pfn() returns err but driver returns 
VM_FAULT_NOPAGE as default. The new function vmf_insert_pfn()
will replace this inefficiency by returning correct VM_FAULT_*
type.

vmf_handle_error is a inline wrapper function which
will convert error number to vm_fault_t type err.

Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
Reviewed-by: Matthew Wilcox <mawilcox@microsoft.com>
---
 arch/powerpc/platforms/cell/spufs/file.c | 37 ++++++++++++++++++++------------
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/platforms/cell/spufs/file.c b/arch/powerpc/platforms/cell/spufs/file.c
index 469bdd0..a1dca9a 100644
--- a/arch/powerpc/platforms/cell/spufs/file.c
+++ b/arch/powerpc/platforms/cell/spufs/file.c
@@ -232,12 +232,13 @@ static ssize_t spufs_attr_write(struct file *file, const char __user *buf,
 	return size;
 }
 
-static int
+static vm_fault_t
 spufs_mem_mmap_fault(struct vm_fault *vmf)
 {
 	struct vm_area_struct *vma = vmf->vma;
 	struct spu_context *ctx	= vma->vm_file->private_data;
 	unsigned long pfn, offset;
+	vm_fault_t ret;
 
 	offset = vmf->pgoff << PAGE_SHIFT;
 	if (offset >= LS_SIZE)
@@ -256,11 +257,11 @@ static ssize_t spufs_attr_write(struct file *file, const char __user *buf,
 		vma->vm_page_prot = pgprot_noncached_wc(vma->vm_page_prot);
 		pfn = (ctx->spu->local_store_phys + offset) >> PAGE_SHIFT;
 	}
-	vm_insert_pfn(vma, vmf->address, pfn);
+	ret = vmf_insert_pfn(vma, vmf->address, pfn);
 
 	spu_release(ctx);
 
-	return VM_FAULT_NOPAGE;
+	return ret;
 }
 
 static int spufs_mem_mmap_access(struct vm_area_struct *vma,
@@ -312,13 +313,19 @@ static int spufs_mem_mmap(struct file *file, struct vm_area_struct *vma)
 	.mmap			= spufs_mem_mmap,
 };
 
-static int spufs_ps_fault(struct vm_fault *vmf,
+static inline vm_fault_t vmf_handle_error(int err)
+{
+	return VM_FAULT_NOPAGE;
+}
+
+static vm_fault_t spufs_ps_fault(struct vm_fault *vmf,
 				    unsigned long ps_offs,
 				    unsigned long ps_size)
 {
 	struct spu_context *ctx = vmf->vma->vm_file->private_data;
 	unsigned long area, offset = vmf->pgoff << PAGE_SHIFT;
-	int ret = 0;
+	int err = 0;
+	vm_fault_t ret = VM_FAULT_NOPAGE;
 
 	spu_context_nospu_trace(spufs_ps_fault__enter, ctx);
 
@@ -349,12 +356,14 @@ static int spufs_ps_fault(struct vm_fault *vmf,
 	if (ctx->state == SPU_STATE_SAVED) {
 		up_read(&current->mm->mmap_sem);
 		spu_context_nospu_trace(spufs_ps_fault__sleep, ctx);
-		ret = spufs_wait(ctx->run_wq, ctx->state == SPU_STATE_RUNNABLE);
+		err = spufs_wait(ctx->run_wq, ctx->state == SPU_STATE_RUNNABLE);
+		ret = vmf_handle_error(err);
 		spu_context_trace(spufs_ps_fault__wake, ctx, ctx->spu);
 		down_read(&current->mm->mmap_sem);
 	} else {
 		area = ctx->spu->problem_phys + ps_offs;
-		vm_insert_pfn(vmf->vma, vmf->address, (area + offset) >> PAGE_SHIFT);
+		ret = vmf_insert_pfn(vmf->vma, vmf->address,
+				(area + offset) >> PAGE_SHIFT);
 		spu_context_trace(spufs_ps_fault__insert, ctx, ctx->spu);
 	}
 
@@ -363,11 +372,11 @@ static int spufs_ps_fault(struct vm_fault *vmf,
 
 refault:
 	put_spu_context(ctx);
-	return VM_FAULT_NOPAGE;
+	return ret;
 }
 
 #if SPUFS_MMAP_4K
-static int spufs_cntl_mmap_fault(struct vm_fault *vmf)
+static vm_fault_t spufs_cntl_mmap_fault(struct vm_fault *vmf)
 {
 	return spufs_ps_fault(vmf, 0x4000, SPUFS_CNTL_MAP_SIZE);
 }
@@ -1040,7 +1049,7 @@ static ssize_t spufs_signal1_write(struct file *file, const char __user *buf,
 	return 4;
 }
 
-static int
+static vm_fault_t
 spufs_signal1_mmap_fault(struct vm_fault *vmf)
 {
 #if SPUFS_SIGNAL_MAP_SIZE == 0x1000
@@ -1178,7 +1187,7 @@ static ssize_t spufs_signal2_write(struct file *file, const char __user *buf,
 }
 
 #if SPUFS_MMAP_4K
-static int
+static vm_fault_t
 spufs_signal2_mmap_fault(struct vm_fault *vmf)
 {
 #if SPUFS_SIGNAL_MAP_SIZE == 0x1000
@@ -1307,7 +1316,7 @@ static u64 spufs_signal2_type_get(struct spu_context *ctx)
 		       spufs_signal2_type_set, "%llu\n", SPU_ATTR_ACQUIRE);
 
 #if SPUFS_MMAP_4K
-static int
+static vm_fault_t
 spufs_mss_mmap_fault(struct vm_fault *vmf)
 {
 	return spufs_ps_fault(vmf, 0x0000, SPUFS_MSS_MAP_SIZE);
@@ -1369,7 +1378,7 @@ static int spufs_mss_open(struct inode *inode, struct file *file)
 	.llseek  = no_llseek,
 };
 
-static int
+static vm_fault_t
 spufs_psmap_mmap_fault(struct vm_fault *vmf)
 {
 	return spufs_ps_fault(vmf, 0x0000, SPUFS_PS_MAP_SIZE);
@@ -1429,7 +1438,7 @@ static int spufs_psmap_open(struct inode *inode, struct file *file)
 
 
 #if SPUFS_MMAP_4K
-static int
+static vm_fault_t
 spufs_mfc_mmap_fault(struct vm_fault *vmf)
 {
 	return spufs_ps_fault(vmf, 0x3000, SPUFS_MFC_MAP_SIZE);
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH] powerpc: Allow selection of CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
From: Mathieu Malaterre @ 2018-04-17 17:10 UTC (permalink / raw)
  To: Christophe LEROY
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	linuxppc-dev, LKML
In-Reply-To: <e49e2296-11f8-a2fa-6b64-db18ba5db641@c-s.fr>

On Tue, Apr 17, 2018 at 6:49 PM, Christophe LEROY
<christophe.leroy@c-s.fr> wrote:
>
>
> Le 17/04/2018 =C3=A0 18:45, Mathieu Malaterre a =C3=A9crit :
>>
>> On Tue, Apr 17, 2018 at 12:49 PM, Christophe Leroy
>> <christophe.leroy@c-s.fr> wrote:
>>>
>>> This option does dead code and data elimination with the linker by
>>> compiling with -ffunction-sections -fdata-sections and linking with
>>> --gc-sections.
>>>
>>> By selecting this option on mpc885_ads_defconfig,
>>> vmlinux LOAD segment size gets reduced by 10%
>>>
>>> Program Header before the patch:
>>>      LOAD off    0x00010000 vaddr 0xc0000000 paddr 0x00000000 align 2**=
16
>>>           filesz 0x0036eda4 memsz 0x0038de04 flags rwx
>>>
>>> Program Header after the patch:
>>>      LOAD off    0x00010000 vaddr 0xc0000000 paddr 0x00000000 align 2**=
16
>>>           filesz 0x00316da4 memsz 0x00334268 flags rwx
>>>
>>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>>> ---
>>>   arch/powerpc/Kconfig | 8 ++++++++
>>>   1 file changed, 8 insertions(+)
>>>
>>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>>> index 8fe4353be5e3..e1fac49cf465 100644
>>> --- a/arch/powerpc/Kconfig
>>> +++ b/arch/powerpc/Kconfig
>>> @@ -888,6 +888,14 @@ config PPC_MEM_KEYS
>>>
>>>            If unsure, say y.
>>>
>>> +config PPC_UNUSED_ELIMINATION
>>> +       bool "Eliminate unused functions and data from vmlinux"
>>> +       default n
>>> +       select LD_DEAD_CODE_DATA_ELIMINATION
>>> +       help
>>> +         Select this to do dead code and data elimination with the
>>> linker
>>> +         by compiling with -ffunction-sections -fdata-sections and
>>> linking
>>> +         with --gc-sections.
>>>   endmenu
>>>
>>
>> Just for reference, I cannot boot my Mac Mini G4 anymore (yaboot). The
>> messages I can see (prom_init) are:
>
>
> Which version of GCC do you use ?

$ powerpc-linux-gnu-gcc --version
powerpc-linux-gnu-gcc (Debian 6.3.0-18) 6.3.0 20170516
Copyright (C) 2016 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

this is simply coming from:

$ apt-cache policy crossbuild-essential-powerpc
crossbuild-essential-powerpc:
  Installed: 12.3
  Candidate: 12.3
  Version table:
 *** 12.3 500
        500 http://ftp.fr.debian.org/debian stretch/main amd64 Packages
        500 http://ftp.fr.debian.org/debian stretch/main i386 Packages
        100 /var/lib/dpkg/status


> Can you provide the generated System.map with and without that option act=
ive
> ?

$ du -sh g4/System.map.*
1.7M g4/System.map.with
1.8M g4/System.map.without

Will send them by private emails.

> Thanks
> Christophe
>
>
>>
>> ---
>> done
>> copying OF device tree...
>> Building dt strings...
>> Building dt structure...
>> Device tree strings 0x01100000 -> 0x01100e02
>> Device tree struct   0x01101000 -> 0x01109000
>> Quiescing Open Firmware ...
>> Booting Linux via __start() @ 0x00140000 ...
>> _
>> ---
>>
>>
>>
>>>   config ISA_DMA_API
>>> --
>>> 2.13.3
>>>
>

^ permalink raw reply

* [PATCH v2 2/2] powerpc/32be: use stmw/lmw for registers save/restore in asm
From: Christophe Leroy @ 2018-04-17 17:08 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linux-kernel, linuxppc-dev
In-Reply-To: <7fbae252f24ec4d30f52f57a549901fa3f799f8f.1523984745.git.christophe.leroy@c-s.fr>

arch/powerpc/Makefile activates -mmultiple on BE PPC32 configs
in order to use multiple word instructions in functions entry/exit

The patch does the same for the asm parts, for consistency

On processors like the 8xx on which insn fetching is pretty slow,
this speeds up registers save/restore

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 v2: Swapped both patches in the serie to reduce number of impacted
     lines and added the same modification in ppc_save_regs()

 arch/powerpc/include/asm/ppc_asm.h  |  5 +++++
 arch/powerpc/kernel/misc.S          | 10 ++++++++++
 arch/powerpc/kernel/ppc_save_regs.S |  4 ++++
 3 files changed, 19 insertions(+)

diff --git a/arch/powerpc/include/asm/ppc_asm.h b/arch/powerpc/include/asm/ppc_asm.h
index 13f7f4c0e1ea..4bb765d0b758 100644
--- a/arch/powerpc/include/asm/ppc_asm.h
+++ b/arch/powerpc/include/asm/ppc_asm.h
@@ -80,11 +80,16 @@ END_FW_FTR_SECTION_IFSET(FW_FEATURE_SPLPAR)
 #else
 #define SAVE_GPR(n, base)	stw	n,GPR0+4*(n)(base)
 #define REST_GPR(n, base)	lwz	n,GPR0+4*(n)(base)
+#ifdef CONFIG_CPU_BIG_ENDIAN
+#define SAVE_NVGPRS(base)	stmw	13, GPR0+4*13(base)
+#define REST_NVGPRS(base)	lmw	13, GPR0+4*13(base)
+#else
 #define SAVE_NVGPRS(base)	SAVE_GPR(13, base); SAVE_8GPRS(14, base); \
 				SAVE_10GPRS(22, base)
 #define REST_NVGPRS(base)	REST_GPR(13, base); REST_8GPRS(14, base); \
 				REST_10GPRS(22, base)
 #endif
+#endif
 
 #define SAVE_2GPRS(n, base)	SAVE_GPR(n, base); SAVE_GPR(n+1, base)
 #define SAVE_4GPRS(n, base)	SAVE_2GPRS(n, base); SAVE_2GPRS(n+2, base)
diff --git a/arch/powerpc/kernel/misc.S b/arch/powerpc/kernel/misc.S
index 746ee0320ad4..a316d90a5c26 100644
--- a/arch/powerpc/kernel/misc.S
+++ b/arch/powerpc/kernel/misc.S
@@ -49,6 +49,10 @@ _GLOBAL(setjmp)
 	PPC_STL	r0,0(r3)
 	PPC_STL	r1,SZL(r3)
 	PPC_STL	r2,2*SZL(r3)
+#if defined(CONFIG_PPC32) && defined(CONFIG_CPU_BIG_ENDIAN)
+	mfcr	r12
+	stmw	r12, 3*SZL(r3)
+#else
 	mfcr	r0
 	PPC_STL	r0,3*SZL(r3)
 	PPC_STL	r13,4*SZL(r3)
@@ -70,10 +74,15 @@ _GLOBAL(setjmp)
 	PPC_STL	r29,20*SZL(r3)
 	PPC_STL	r30,21*SZL(r3)
 	PPC_STL	r31,22*SZL(r3)
+#endif
 	li	r3,0
 	blr
 
 _GLOBAL(longjmp)
+#if defined(CONFIG_PPC32) && defined(CONFIG_CPU_BIG_ENDIAN)
+	lmw	r12, 3*SZL(r3)
+	mtcrf	0x38, r12
+#else
 	PPC_LL	r13,4*SZL(r3)
 	PPC_LL	r14,5*SZL(r3)
 	PPC_LL	r15,6*SZL(r3)
@@ -95,6 +104,7 @@ _GLOBAL(longjmp)
 	PPC_LL	r31,22*SZL(r3)
 	PPC_LL	r0,3*SZL(r3)
 	mtcrf	0x38,r0
+#endif
 	PPC_LL	r0,0(r3)
 	PPC_LL	r1,SZL(r3)
 	PPC_LL	r2,2*SZL(r3)
diff --git a/arch/powerpc/kernel/ppc_save_regs.S b/arch/powerpc/kernel/ppc_save_regs.S
index 1b1787d52896..d60316e70514 100644
--- a/arch/powerpc/kernel/ppc_save_regs.S
+++ b/arch/powerpc/kernel/ppc_save_regs.S
@@ -25,6 +25,9 @@
  */
 _GLOBAL(ppc_save_regs)
 	PPC_STL	r0,0*SZL(r3)
+#if defined(CONFIG_PPC32) && defined(CONFIG_CPU_BIG_ENDIAN)
+	stmw	r2, 2*SZL(r3)
+#else
 	PPC_STL	r2,2*SZL(r3)
 	PPC_STL	r3,3*SZL(r3)
 	PPC_STL	r4,4*SZL(r3)
@@ -55,6 +58,7 @@ _GLOBAL(ppc_save_regs)
 	PPC_STL	r29,29*SZL(r3)
 	PPC_STL	r30,30*SZL(r3)
 	PPC_STL	r31,31*SZL(r3)
+#endif
 	/* go up one stack frame for SP */
 	PPC_LL	r4,0(r1)
 	PPC_STL	r4,1*SZL(r3)
-- 
2.13.3

^ permalink raw reply related

* [PATCH v2 1/2] powerpc: avoid an unnecessary test and branch in longjmp()
From: Christophe Leroy @ 2018-04-17 17:08 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linux-kernel, linuxppc-dev

Doing the test at exit of the function avoids an unnecessary
test and branch inside longjmp()

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 v2: Swapped both patches in the serie to reduce number of impacted lines

 arch/powerpc/kernel/misc.S | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/kernel/misc.S b/arch/powerpc/kernel/misc.S
index e1f3a5d054c4..746ee0320ad4 100644
--- a/arch/powerpc/kernel/misc.S
+++ b/arch/powerpc/kernel/misc.S
@@ -74,10 +74,7 @@ _GLOBAL(setjmp)
 	blr
 
 _GLOBAL(longjmp)
-	PPC_LCMPI r4,0
-	bne	1f
-	li	r4,1
-1:	PPC_LL	r13,4*SZL(r3)
+	PPC_LL	r13,4*SZL(r3)
 	PPC_LL	r14,5*SZL(r3)
 	PPC_LL	r15,6*SZL(r3)
 	PPC_LL	r16,7*SZL(r3)
@@ -102,7 +99,9 @@ _GLOBAL(longjmp)
 	PPC_LL	r1,SZL(r3)
 	PPC_LL	r2,2*SZL(r3)
 	mtlr	r0
-	mr	r3,r4
+	mr.	r3, r4
+	bnelr
+	li	r3, 1
 	blr
 
 _GLOBAL(current_stack_pointer)
-- 
2.13.3

^ permalink raw reply related

* Re: [PATCH 2/6 v2] iommu: of: make of_pci_map_rid() available for other devices too
From: Robin Murphy @ 2018-04-17 16:52 UTC (permalink / raw)
  To: Nipun Gupta, robh+dt, frowand.list
  Cc: will.deacon, mark.rutland, catalin.marinas, hch, gregkh, joro,
	m.szyprowski, shawnguo, bhelgaas, iommu, linux-kernel, devicetree,
	linux-arm-kernel, linuxppc-dev, linux-pci, bharat.bhushan,
	stuyoder, laurentiu.tudor, leoyang.li
In-Reply-To: <1523960514-25457-3-git-send-email-nipun.gupta@nxp.com>

On 17/04/18 11:21, Nipun Gupta wrote:
> iommu-map property is also used by devices with fsl-mc. This
> patch moves the of_pci_map_rid to generic location, so that it
> can be used by other busses too.
> 
> Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
> ---
>   drivers/iommu/of_iommu.c | 106 +++++++++++++++++++++++++++++++++++++++++++++--

Doesn't this break "msi-parent" parsing for !CONFIG_OF_IOMMU? I guess 
you don't want fsl-mc to have to depend on PCI, but this looks like a 
step in the wrong direction.

I'm not entirely sure where of_map_rid() fits best, but from a quick 
look around the least-worst option might be drivers/of/of_address.c, 
unless Rob and Frank have a better idea of where generic DT-based ID 
translation routines could live?

>   drivers/of/irq.c         |   6 +--
>   drivers/pci/of.c         | 101 --------------------------------------------
>   include/linux/of_iommu.h |  11 +++++
>   include/linux/of_pci.h   |  10 -----
>   5 files changed, 117 insertions(+), 117 deletions(-)
> 
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index 5c36a8b..4e7712f 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -138,6 +138,106 @@ static int of_iommu_xlate(struct device *dev,
>   	return ops->of_xlate(dev, iommu_spec);
>   }
>   
> +/**
> + * of_map_rid - Translate a requester ID through a downstream mapping.
> + * @np: root complex device node.
> + * @rid: device requester ID to map.
> + * @map_name: property name of the map to use.
> + * @map_mask_name: optional property name of the mask to use.
> + * @target: optional pointer to a target device node.
> + * @id_out: optional pointer to receive the translated ID.
> + *
> + * Given a device requester ID, look up the appropriate implementation-defined
> + * platform ID and/or the target device which receives transactions on that
> + * ID, as per the "iommu-map" and "msi-map" bindings. Either of @target or
> + * @id_out may be NULL if only the other is required. If @target points to
> + * a non-NULL device node pointer, only entries targeting that node will be
> + * matched; if it points to a NULL value, it will receive the device node of
> + * the first matching target phandle, with a reference held.
> + *
> + * Return: 0 on success or a standard error code on failure.
> + */
> +int of_map_rid(struct device_node *np, u32 rid,
> +		   const char *map_name, const char *map_mask_name,
> +		   struct device_node **target, u32 *id_out)
> +{
> +	u32 map_mask, masked_rid;
> +	int map_len;
> +	const __be32 *map = NULL;
> +
> +	if (!np || !map_name || (!target && !id_out))
> +		return -EINVAL;
> +
> +	map = of_get_property(np, map_name, &map_len);
> +	if (!map) {
> +		if (target)
> +			return -ENODEV;
> +		/* Otherwise, no map implies no translation */
> +		*id_out = rid;
> +		return 0;
> +	}
> +
> +	if (!map_len || map_len % (4 * sizeof(*map))) {
> +		pr_err("%pOF: Error: Bad %s length: %d\n", np,
> +			map_name, map_len);
> +		return -EINVAL;
> +	}
> +
> +	/* The default is to select all bits. */
> +	map_mask = 0xffffffff;
> +
> +	/*
> +	 * Can be overridden by "{iommu,msi}-map-mask" property.
> +	 */
> +	if (map_mask_name)
> +		of_property_read_u32(np, map_mask_name, &map_mask);
> +
> +	masked_rid = map_mask & rid;
> +	for ( ; map_len > 0; map_len -= 4 * sizeof(*map), map += 4) {
> +		struct device_node *phandle_node;
> +		u32 rid_base = be32_to_cpup(map + 0);
> +		u32 phandle = be32_to_cpup(map + 1);
> +		u32 out_base = be32_to_cpup(map + 2);
> +		u32 rid_len = be32_to_cpup(map + 3);
> +
> +		if (rid_base & ~map_mask) {
> +			pr_err("%pOF: Invalid %s translation - %s-mask (0x%x) ignores rid-base (0x%x)\n",
> +				np, map_name, map_name,
> +				map_mask, rid_base);
> +			return -EFAULT;
> +		}
> +
> +		if (masked_rid < rid_base || masked_rid >= rid_base + rid_len)
> +			continue;
> +
> +		phandle_node = of_find_node_by_phandle(phandle);
> +		if (!phandle_node)
> +			return -ENODEV;
> +
> +		if (target) {
> +			if (*target)
> +				of_node_put(phandle_node);
> +			else
> +				*target = phandle_node;
> +
> +			if (*target != phandle_node)
> +				continue;
> +		}
> +
> +		if (id_out)
> +			*id_out = masked_rid - rid_base + out_base;
> +
> +		pr_debug("%pOF: %s, using mask %08x, rid-base: %08x, out-base: %08x, length: %08x, rid: %08x -> %08x\n",
> +			np, map_name, map_mask, rid_base, out_base,
> +			rid_len, rid, masked_rid - rid_base + out_base);
> +		return 0;
> +	}
> +
> +	pr_err("%pOF: Invalid %s translation - no match for rid 0x%x on %pOF\n",
> +		np, map_name, rid, target && *target ? *target : NULL);
> +	return -EFAULT;
> +}
> +
>   struct of_pci_iommu_alias_info {
>   	struct device *dev;
>   	struct device_node *np;
> @@ -149,9 +249,9 @@ static int of_pci_iommu_init(struct pci_dev *pdev, u16 alias, void *data)
>   	struct of_phandle_args iommu_spec = { .args_count = 1 };
>   	int err;
>   
> -	err = of_pci_map_rid(info->np, alias, "iommu-map",
> -			     "iommu-map-mask", &iommu_spec.np,
> -			     iommu_spec.args);
> +	err = of_map_rid(info->np, alias, "iommu-map",
> +			 "iommu-map-mask", &iommu_spec.np,
> +			 iommu_spec.args);

Super-nit: Apparently I missed rewrapping this to 2 lines in 
d87beb749281, but if it's being touched again, that would be nice ;)

Robin.

>   	if (err)
>   		return err == -ENODEV ? NO_IOMMU : err;
>   
> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
> index 02ad93a..b72eeec 100644
> --- a/drivers/of/irq.c
> +++ b/drivers/of/irq.c
> @@ -22,7 +22,7 @@
>   #include <linux/module.h>
>   #include <linux/of.h>
>   #include <linux/of_irq.h>
> -#include <linux/of_pci.h>
> +#include <linux/of_iommu.h>
>   #include <linux/string.h>
>   #include <linux/slab.h>
>   
> @@ -588,8 +588,8 @@ static u32 __of_msi_map_rid(struct device *dev, struct device_node **np,
>   	 * "msi-map" property.
>   	 */
>   	for (parent_dev = dev; parent_dev; parent_dev = parent_dev->parent)
> -		if (!of_pci_map_rid(parent_dev->of_node, rid_in, "msi-map",
> -				    "msi-map-mask", np, &rid_out))
> +		if (!of_map_rid(parent_dev->of_node, rid_in, "msi-map",
> +				"msi-map-mask", np, &rid_out))
>   			break;
>   	return rid_out;
>   }
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index a28355c..d2cebbe 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -362,107 +362,6 @@ int of_pci_get_host_bridge_resources(struct device_node *dev,
>   EXPORT_SYMBOL_GPL(of_pci_get_host_bridge_resources);
>   #endif /* CONFIG_OF_ADDRESS */
>   
> -/**
> - * of_pci_map_rid - Translate a requester ID through a downstream mapping.
> - * @np: root complex device node.
> - * @rid: PCI requester ID to map.
> - * @map_name: property name of the map to use.
> - * @map_mask_name: optional property name of the mask to use.
> - * @target: optional pointer to a target device node.
> - * @id_out: optional pointer to receive the translated ID.
> - *
> - * Given a PCI requester ID, look up the appropriate implementation-defined
> - * platform ID and/or the target device which receives transactions on that
> - * ID, as per the "iommu-map" and "msi-map" bindings. Either of @target or
> - * @id_out may be NULL if only the other is required. If @target points to
> - * a non-NULL device node pointer, only entries targeting that node will be
> - * matched; if it points to a NULL value, it will receive the device node of
> - * the first matching target phandle, with a reference held.
> - *
> - * Return: 0 on success or a standard error code on failure.
> - */
> -int of_pci_map_rid(struct device_node *np, u32 rid,
> -		   const char *map_name, const char *map_mask_name,
> -		   struct device_node **target, u32 *id_out)
> -{
> -	u32 map_mask, masked_rid;
> -	int map_len;
> -	const __be32 *map = NULL;
> -
> -	if (!np || !map_name || (!target && !id_out))
> -		return -EINVAL;
> -
> -	map = of_get_property(np, map_name, &map_len);
> -	if (!map) {
> -		if (target)
> -			return -ENODEV;
> -		/* Otherwise, no map implies no translation */
> -		*id_out = rid;
> -		return 0;
> -	}
> -
> -	if (!map_len || map_len % (4 * sizeof(*map))) {
> -		pr_err("%pOF: Error: Bad %s length: %d\n", np,
> -			map_name, map_len);
> -		return -EINVAL;
> -	}
> -
> -	/* The default is to select all bits. */
> -	map_mask = 0xffffffff;
> -
> -	/*
> -	 * Can be overridden by "{iommu,msi}-map-mask" property.
> -	 * If of_property_read_u32() fails, the default is used.
> -	 */
> -	if (map_mask_name)
> -		of_property_read_u32(np, map_mask_name, &map_mask);
> -
> -	masked_rid = map_mask & rid;
> -	for ( ; map_len > 0; map_len -= 4 * sizeof(*map), map += 4) {
> -		struct device_node *phandle_node;
> -		u32 rid_base = be32_to_cpup(map + 0);
> -		u32 phandle = be32_to_cpup(map + 1);
> -		u32 out_base = be32_to_cpup(map + 2);
> -		u32 rid_len = be32_to_cpup(map + 3);
> -
> -		if (rid_base & ~map_mask) {
> -			pr_err("%pOF: Invalid %s translation - %s-mask (0x%x) ignores rid-base (0x%x)\n",
> -				np, map_name, map_name,
> -				map_mask, rid_base);
> -			return -EFAULT;
> -		}
> -
> -		if (masked_rid < rid_base || masked_rid >= rid_base + rid_len)
> -			continue;
> -
> -		phandle_node = of_find_node_by_phandle(phandle);
> -		if (!phandle_node)
> -			return -ENODEV;
> -
> -		if (target) {
> -			if (*target)
> -				of_node_put(phandle_node);
> -			else
> -				*target = phandle_node;
> -
> -			if (*target != phandle_node)
> -				continue;
> -		}
> -
> -		if (id_out)
> -			*id_out = masked_rid - rid_base + out_base;
> -
> -		pr_debug("%pOF: %s, using mask %08x, rid-base: %08x, out-base: %08x, length: %08x, rid: %08x -> %08x\n",
> -			np, map_name, map_mask, rid_base, out_base,
> -			rid_len, rid, masked_rid - rid_base + out_base);
> -		return 0;
> -	}
> -
> -	pr_err("%pOF: Invalid %s translation - no match for rid 0x%x on %pOF\n",
> -		np, map_name, rid, target && *target ? *target : NULL);
> -	return -EFAULT;
> -}
> -
>   #if IS_ENABLED(CONFIG_OF_IRQ)
>   /**
>    * of_irq_parse_pci - Resolve the interrupt for a PCI device
> diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
> index 4fa654e..432b53c 100644
> --- a/include/linux/of_iommu.h
> +++ b/include/linux/of_iommu.h
> @@ -15,6 +15,10 @@ extern int of_get_dma_window(struct device_node *dn, const char *prefix,
>   extern const struct iommu_ops *of_iommu_configure(struct device *dev,
>   					struct device_node *master_np);
>   
> +int of_map_rid(struct device_node *np, u32 rid,
> +	       const char *map_name, const char *map_mask_name,
> +	       struct device_node **target, u32 *id_out);
> +
>   #else
>   
>   static inline int of_get_dma_window(struct device_node *dn, const char *prefix,
> @@ -30,6 +34,13 @@ static inline const struct iommu_ops *of_iommu_configure(struct device *dev,
>   	return NULL;
>   }
>   
> +static inline int of_map_rid(struct device_node *np, u32 rid,
> +			     const char *map_name, const char *map_mask_name,
> +			     struct device_node **target, u32 *id_out)
> +{
> +	return -EINVAL;
> +}
> +
>   #endif	/* CONFIG_OF_IOMMU */
>   
>   extern struct of_device_id __iommu_of_table;
> diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
> index 091033a..a23b44a 100644
> --- a/include/linux/of_pci.h
> +++ b/include/linux/of_pci.h
> @@ -17,9 +17,6 @@ struct device_node *of_pci_find_child_device(struct device_node *parent,
>   int of_get_pci_domain_nr(struct device_node *node);
>   int of_pci_get_max_link_speed(struct device_node *node);
>   void of_pci_check_probe_only(void);
> -int of_pci_map_rid(struct device_node *np, u32 rid,
> -		   const char *map_name, const char *map_mask_name,
> -		   struct device_node **target, u32 *id_out);
>   #else
>   static inline struct device_node *of_pci_find_child_device(struct device_node *parent,
>   					     unsigned int devfn)
> @@ -44,13 +41,6 @@ static inline int of_pci_get_devfn(struct device_node *np)
>   	return -1;
>   }
>   
> -static inline int of_pci_map_rid(struct device_node *np, u32 rid,
> -			const char *map_name, const char *map_mask_name,
> -			struct device_node **target, u32 *id_out)
> -{
> -	return -EINVAL;
> -}
> -
>   static inline int
>   of_pci_get_max_link_speed(struct device_node *node)
>   {
> 

^ permalink raw reply

* Re: [PATCH] powerpc: Allow selection of CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
From: Christophe LEROY @ 2018-04-17 16:49 UTC (permalink / raw)
  To: Mathieu Malaterre
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	linuxppc-dev, LKML
In-Reply-To: <CA+7wUszKbPKu86jSprEBJsxJOmyAc42x5S4XmPce_RFXTsHTfQ@mail.gmail.com>



Le 17/04/2018 à 18:45, Mathieu Malaterre a écrit :
> On Tue, Apr 17, 2018 at 12:49 PM, Christophe Leroy
> <christophe.leroy@c-s.fr> wrote:
>> This option does dead code and data elimination with the linker by
>> compiling with -ffunction-sections -fdata-sections and linking with
>> --gc-sections.
>>
>> By selecting this option on mpc885_ads_defconfig,
>> vmlinux LOAD segment size gets reduced by 10%
>>
>> Program Header before the patch:
>>      LOAD off    0x00010000 vaddr 0xc0000000 paddr 0x00000000 align 2**16
>>           filesz 0x0036eda4 memsz 0x0038de04 flags rwx
>>
>> Program Header after the patch:
>>      LOAD off    0x00010000 vaddr 0xc0000000 paddr 0x00000000 align 2**16
>>           filesz 0x00316da4 memsz 0x00334268 flags rwx
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> ---
>>   arch/powerpc/Kconfig | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> index 8fe4353be5e3..e1fac49cf465 100644
>> --- a/arch/powerpc/Kconfig
>> +++ b/arch/powerpc/Kconfig
>> @@ -888,6 +888,14 @@ config PPC_MEM_KEYS
>>
>>            If unsure, say y.
>>
>> +config PPC_UNUSED_ELIMINATION
>> +       bool "Eliminate unused functions and data from vmlinux"
>> +       default n
>> +       select LD_DEAD_CODE_DATA_ELIMINATION
>> +       help
>> +         Select this to do dead code and data elimination with the linker
>> +         by compiling with -ffunction-sections -fdata-sections and linking
>> +         with --gc-sections.
>>   endmenu
>>
> 
> Just for reference, I cannot boot my Mac Mini G4 anymore (yaboot). The
> messages I can see (prom_init) are:

Which version of GCC do you use ?

Can you provide the generated System.map with and without that option 
active ?

Thanks
Christophe

> 
> ---
> done
> copying OF device tree...
> Building dt strings...
> Building dt structure...
> Device tree strings 0x01100000 -> 0x01100e02
> Device tree struct   0x01101000 -> 0x01109000
> Quiescing Open Firmware ...
> Booting Linux via __start() @ 0x00140000 ...
> _
> ---
> 
> 
> 
>>   config ISA_DMA_API
>> --
>> 2.13.3
>>

^ permalink raw reply

* Re: [PATCH] powerpc: Allow selection of CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
From: Mathieu Malaterre @ 2018-04-17 16:45 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	linuxppc-dev, LKML
In-Reply-To: <20180417104909.22D416C07D@po15720vm.idsi0.si.c-s.fr>

On Tue, Apr 17, 2018 at 12:49 PM, Christophe Leroy
<christophe.leroy@c-s.fr> wrote:
> This option does dead code and data elimination with the linker by
> compiling with -ffunction-sections -fdata-sections and linking with
> --gc-sections.
>
> By selecting this option on mpc885_ads_defconfig,
> vmlinux LOAD segment size gets reduced by 10%
>
> Program Header before the patch:
>     LOAD off    0x00010000 vaddr 0xc0000000 paddr 0x00000000 align 2**16
>          filesz 0x0036eda4 memsz 0x0038de04 flags rwx
>
> Program Header after the patch:
>     LOAD off    0x00010000 vaddr 0xc0000000 paddr 0x00000000 align 2**16
>          filesz 0x00316da4 memsz 0x00334268 flags rwx
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
>  arch/powerpc/Kconfig | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 8fe4353be5e3..e1fac49cf465 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -888,6 +888,14 @@ config PPC_MEM_KEYS
>
>           If unsure, say y.
>
> +config PPC_UNUSED_ELIMINATION
> +       bool "Eliminate unused functions and data from vmlinux"
> +       default n
> +       select LD_DEAD_CODE_DATA_ELIMINATION
> +       help
> +         Select this to do dead code and data elimination with the linker
> +         by compiling with -ffunction-sections -fdata-sections and linking
> +         with --gc-sections.
>  endmenu
>

Just for reference, I cannot boot my Mac Mini G4 anymore (yaboot). The
messages I can see (prom_init) are:

---
done
copying OF device tree...
Building dt strings...
Building dt structure...
Device tree strings 0x01100000 -> 0x01100e02
Device tree struct   0x01101000 -> 0x01109000
Quiescing Open Firmware ...
Booting Linux via __start() @ 0x00140000 ...
_
---



>  config ISA_DMA_API
> --
> 2.13.3
>

^ permalink raw reply

* [PATCH] misc: cxl: Change return type to vm_fault_t
From: Souptick Joarder @ 2018-04-17 14:53 UTC (permalink / raw)
  To: fbarrat, andrew.donnellan, arnd, gregkh; +Cc: linuxppc-dev, linux-kernel, willy

Use new return type vm_fault_t for fault handler. For
now, this is just documenting that the function returns
a VM_FAULT value rather than an errno. Once all instances
are converted, vm_fault_t will become a distinct type.

Reference id -> 1c8f422059ae ("mm: change return type to
vm_fault_t")

previously cxl_mmap_fault returns VM_FAULT_NOPAGE as
default value irrespective of vm_insert_pfn() return
value. This bug is fixed with new vmf_insert_pfn()
which will return VM_FAULT_ type based on err.

Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
---
 drivers/misc/cxl/context.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c
index 7ff315a..c6ec872 100644
--- a/drivers/misc/cxl/context.c
+++ b/drivers/misc/cxl/context.c
@@ -128,11 +128,12 @@ void cxl_context_set_mapping(struct cxl_context *ctx,
 	mutex_unlock(&ctx->mapping_lock);
 }

-static int cxl_mmap_fault(struct vm_fault *vmf)
+static vm_fault_t cxl_mmap_fault(struct vm_fault *vmf)
 {
 	struct vm_area_struct *vma = vmf->vma;
 	struct cxl_context *ctx = vma->vm_file->private_data;
 	u64 area, offset;
+	vm_fault_t ret;

 	offset = vmf->pgoff << PAGE_SHIFT;

@@ -169,11 +170,11 @@ static int cxl_mmap_fault(struct vm_fault *vmf)
 		return VM_FAULT_SIGBUS;
 	}

-	vm_insert_pfn(vma, vmf->address, (area + offset) >> PAGE_SHIFT);
+	ret = vmf_insert_pfn(vma, vmf->address, (area + offset) >> PAGE_SHIFT);

 	mutex_unlock(&ctx->status_mutex);

-	return VM_FAULT_NOPAGE;
+	return ret;
 }

 static const struct vm_operations_struct cxl_mmap_vmops = {
--
1.9.1

^ permalink raw reply related

* Re: powerpc/modules: Fix crashes by adding CONFIG_RELOCATABLE to vermagic
From: Ard Biesheuvel @ 2018-04-17 14:42 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: Michael Ellerman, linuxppc-dev
In-Reply-To: <87in8ri6nf.fsf@concordia.ellerman.id.au>

On 16 April 2018 at 16:10, Michael Ellerman <mpe@ellerman.id.au> wrote:
> Ard Biesheuvel <ard.biesheuvel@linaro.org> writes:
>
>> On 11 April 2018 at 16:49, Michael Ellerman
>> <patch-notifications@ellerman.id.au> wrote:
>>> On Tue, 2018-04-10 at 01:22:06 UTC, Michael Ellerman wrote:
>>>> If you build the kernel with CONFIG_RELOCATABLE=n, then install the
>>>> modules, rebuild the kernel with CONFIG_RELOCATABLE=y and leave the
>>>> old modules installed, we crash something like:
>>>>
>>>>   Unable to handle kernel paging request for data at address 0xd000000018d66cef
>>>>   Faulting instruction address: 0xc0000000021ddd08
>>>>   Oops: Kernel access of bad area, sig: 11 [#1]
>>>>   Modules linked in: x_tables autofs4
>>>>   CPU: 2 PID: 1 Comm: systemd Not tainted 4.16.0-rc6-gcc_ubuntu_le-g99fec39 #1
>>>>   ...
>>>>   NIP check_version.isra.22+0x118/0x170
>>>>   Call Trace:
>>>>     __ksymtab_xt_unregister_table+0x58/0xfffffffffffffcb8 [x_tables] (unreliable)
>>>>     resolve_symbol+0xb4/0x150
>>>>     load_module+0x10e8/0x29a0
>>>>     SyS_finit_module+0x110/0x140
>>>>     system_call+0x58/0x6c
>>>>
>>>> This happens because since commit 71810db27c1c ("modversions: treat
>>>> symbol CRCs as 32 bit quantities"), a relocatable kernel encodes and
>>>> handles symbol CRCs differently from a non-relocatable kernel.
>>>>
>>>> Although it's possible we could try and detect this situation and
>>>> handle it, it's much more robust to simply make the state of
>>>> CONFIG_RELOCATABLE part of the module vermagic.
>>>>
>>>> Fixes: 71810db27c1c ("modversions: treat symbol CRCs as 32 bit quantities")
>>>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>>>
>>> Applied to powerpc fixes.
>>>
>>> https://git.kernel.org/powerpc/c/73aca179d78eaa11604ba0783a6d8b
>>
>> Thanks for the cc. I guess this only affects powerpc, given that it is
>> the only arch that switches between CRC immediate values and CRC
>> offsets depending on the configuration.
>
> No worries.
>
> Is there any reason we shouldn't always turn on CONFIG_MODULE_REL_CRCS?
> It seems to work, but I wanted to test it more before switching to that,
> hence the quick fix above.
>
>
> arch/um looks like it might be switching based on config, but I don't
> know enough to say:
>
>   config LD_SCRIPT_STATIC
>         bool
>         default y
>         depends on STATIC_LINK
>
>   config LD_SCRIPT_DYN
>         bool
>         default y
>         depends on !LD_SCRIPT_STATIC
>           select MODULE_REL_CRCS if MODVERSIONS
>

The only reason not to enable it is that it ends up taking more space
on a 32-bit architecture with CONFIG_RELOCATABLE=n, given that you
need to record both the relative offset and the actual CRC value (both
32-bit quantities) rather than just the CRC itself. On a 64-bit arch
with CONFIG_RELOCATABLE=n, you end up replacing a single 64-bit
quantity with two 32-bit quantities, so it doesn't really matter.

^ permalink raw reply

* [PATCH v10 22/25] mm: speculative page fault handler return VMA
From: Laurent Dufour @ 2018-04-17 14:33 UTC (permalink / raw)
  To: akpm, mhocko, peterz, kirill, ak, dave, jack, Matthew Wilcox,
	benh, mpe, paulus, Thomas Gleixner, Ingo Molnar, hpa, Will Deacon,
	Sergey Senozhatsky, Andrea Arcangeli, Alexei Starovoitov,
	kemi.wang, sergey.senozhatsky.work, Daniel Jordan, David Rientjes,
	Jerome Glisse, Ganesh Mahendran
  Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
	paulmck, Tim Chen, linuxppc-dev, x86
In-Reply-To: <1523975611-15978-1-git-send-email-ldufour@linux.vnet.ibm.com>

When the speculative page fault handler is returning VM_RETRY, there is a
chance that VMA fetched without grabbing the mmap_sem can be reused by the
legacy page fault handler.  By reusing it, we avoid calling find_vma()
again. To achieve, that we must ensure that the VMA structure will not be
freed in our back. This is done by getting the reference on it (get_vma())
and by assuming that the caller will call the new service
can_reuse_spf_vma() once it has grabbed the mmap_sem.

can_reuse_spf_vma() is first checking that the VMA is still in the RB tree
, and then that the VMA's boundaries matched the passed address and release
the reference on the VMA so that it can be freed if needed.

In the case the VMA is freed, can_reuse_spf_vma() will have returned false
as the VMA is no more in the RB tree.

In the architecture page fault handler, the call to the new service
reuse_spf_or_find_vma() should be made in place of find_vma(), this will
handle the check on the spf_vma and if needed call find_vma().

Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
---
 include/linux/mm.h |  22 +++++++--
 mm/memory.c        | 140 ++++++++++++++++++++++++++++++++---------------------
 2 files changed, 103 insertions(+), 59 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 08540c98d63b..50b6fd3bf9e2 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1382,25 +1382,37 @@ extern int handle_mm_fault(struct vm_area_struct *vma, unsigned long address,
 #ifdef CONFIG_SPECULATIVE_PAGE_FAULT
 extern int __handle_speculative_fault(struct mm_struct *mm,
 				      unsigned long address,
-				      unsigned int flags);
+				      unsigned int flags,
+				      struct vm_area_struct **vma);
 static inline int handle_speculative_fault(struct mm_struct *mm,
 					   unsigned long address,
-					   unsigned int flags)
+					   unsigned int flags,
+					   struct vm_area_struct **vma)
 {
 	/*
 	 * Try speculative page fault for multithreaded user space task only.
 	 */
-	if (!(flags & FAULT_FLAG_USER) || atomic_read(&mm->mm_users) == 1)
+	if (!(flags & FAULT_FLAG_USER) || atomic_read(&mm->mm_users) == 1) {
+		*vma = NULL;
 		return VM_FAULT_RETRY;
-	return __handle_speculative_fault(mm, address, flags);
+	}
+	return __handle_speculative_fault(mm, address, flags, vma);
 }
+extern bool can_reuse_spf_vma(struct vm_area_struct *vma,
+			      unsigned long address);
 #else
 static inline int handle_speculative_fault(struct mm_struct *mm,
 					   unsigned long address,
-					   unsigned int flags)
+					   unsigned int flags,
+					   struct vm_area_struct **vma)
 {
 	return VM_FAULT_RETRY;
 }
+static inline bool can_reuse_spf_vma(struct vm_area_struct *vma,
+				     unsigned long address)
+{
+	return false;
+}
 #endif /* CONFIG_SPECULATIVE_PAGE_FAULT */
 
 extern int fixup_user_fault(struct task_struct *tsk, struct mm_struct *mm,
diff --git a/mm/memory.c b/mm/memory.c
index 76178feff000..425f07e0bf38 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -4311,13 +4311,22 @@ static int __handle_mm_fault(struct vm_area_struct *vma, unsigned long address,
 /* This is required by vm_normal_page() */
 #error "Speculative page fault handler requires __HAVE_ARCH_PTE_SPECIAL"
 #endif
-
 /*
  * vm_normal_page() adds some processing which should be done while
  * hodling the mmap_sem.
  */
+
+/*
+ * Tries to handle the page fault in a speculative way, without grabbing the
+ * mmap_sem.
+ * When VM_FAULT_RETRY is returned, the vma pointer is valid and this vma must
+ * be checked later when the mmap_sem has been grabbed by calling
+ * can_reuse_spf_vma().
+ * This is needed as the returned vma is kept in memory until the call to
+ * can_reuse_spf_vma() is made.
+ */
 int __handle_speculative_fault(struct mm_struct *mm, unsigned long address,
-			       unsigned int flags)
+			       unsigned int flags, struct vm_area_struct **vma)
 {
 	struct vm_fault vmf = {
 		.address = address,
@@ -4325,21 +4334,22 @@ int __handle_speculative_fault(struct mm_struct *mm, unsigned long address,
 	pgd_t *pgd, pgdval;
 	p4d_t *p4d, p4dval;
 	pud_t pudval;
-	int seq, ret = VM_FAULT_RETRY;
-	struct vm_area_struct *vma;
+	int seq, ret;
 
 	/* Clear flags that may lead to release the mmap_sem to retry */
 	flags &= ~(FAULT_FLAG_ALLOW_RETRY|FAULT_FLAG_KILLABLE);
 	flags |= FAULT_FLAG_SPECULATIVE;
 
-	vma = get_vma(mm, address);
-	if (!vma)
-		return ret;
+	*vma = get_vma(mm, address);
+	if (!*vma)
+		return VM_FAULT_RETRY;
+	vmf.vma = *vma;
 
-	seq = raw_read_seqcount(&vma->vm_sequence); /* rmb <-> seqlock,vma_rb_erase() */
+	/* rmb <-> seqlock,vma_rb_erase() */
+	seq = raw_read_seqcount(&vmf.vma->vm_sequence);
 	if (seq & 1) {
-		trace_spf_vma_changed(_RET_IP_, vma, address);
-		goto out_put;
+		trace_spf_vma_changed(_RET_IP_, vmf.vma, address);
+		return VM_FAULT_RETRY;
 	}
 
 	/*
@@ -4347,9 +4357,9 @@ int __handle_speculative_fault(struct mm_struct *mm, unsigned long address,
 	 * with the VMA.
 	 * This include huge page from hugetlbfs.
 	 */
-	if (vma->vm_ops) {
-		trace_spf_vma_notsup(_RET_IP_, vma, address);
-		goto out_put;
+	if (vmf.vma->vm_ops) {
+		trace_spf_vma_notsup(_RET_IP_, vmf.vma, address);
+		return VM_FAULT_RETRY;
 	}
 
 	/*
@@ -4357,18 +4367,18 @@ int __handle_speculative_fault(struct mm_struct *mm, unsigned long address,
 	 * because vm_next and vm_prev must be safe. This can't be guaranteed
 	 * in the speculative path.
 	 */
-	if (unlikely(!vma->anon_vma)) {
-		trace_spf_vma_notsup(_RET_IP_, vma, address);
-		goto out_put;
+	if (unlikely(!vmf.vma->anon_vma)) {
+		trace_spf_vma_notsup(_RET_IP_, vmf.vma, address);
+		return VM_FAULT_RETRY;
 	}
 
-	vmf.vma_flags = READ_ONCE(vma->vm_flags);
-	vmf.vma_page_prot = READ_ONCE(vma->vm_page_prot);
+	vmf.vma_flags = READ_ONCE(vmf.vma->vm_flags);
+	vmf.vma_page_prot = READ_ONCE(vmf.vma->vm_page_prot);
 
 	/* Can't call userland page fault handler in the speculative path */
 	if (unlikely(vmf.vma_flags & VM_UFFD_MISSING)) {
-		trace_spf_vma_notsup(_RET_IP_, vma, address);
-		goto out_put;
+		trace_spf_vma_notsup(_RET_IP_, vmf.vma, address);
+		return VM_FAULT_RETRY;
 	}
 
 	if (vmf.vma_flags & VM_GROWSDOWN || vmf.vma_flags & VM_GROWSUP) {
@@ -4377,36 +4387,27 @@ int __handle_speculative_fault(struct mm_struct *mm, unsigned long address,
 		 * boundaries but we want to trace it as not supported instead
 		 * of changed.
 		 */
-		trace_spf_vma_notsup(_RET_IP_, vma, address);
-		goto out_put;
+		trace_spf_vma_notsup(_RET_IP_, vmf.vma, address);
+		return VM_FAULT_RETRY;
 	}
 
-	if (address < READ_ONCE(vma->vm_start)
-	    || READ_ONCE(vma->vm_end) <= address) {
-		trace_spf_vma_changed(_RET_IP_, vma, address);
-		goto out_put;
+	if (address < READ_ONCE(vmf.vma->vm_start)
+	    || READ_ONCE(vmf.vma->vm_end) <= address) {
+		trace_spf_vma_changed(_RET_IP_, vmf.vma, address);
+		return VM_FAULT_RETRY;
 	}
 
-	if (!arch_vma_access_permitted(vma, flags & FAULT_FLAG_WRITE,
+	if (!arch_vma_access_permitted(vmf.vma, flags & FAULT_FLAG_WRITE,
 				       flags & FAULT_FLAG_INSTRUCTION,
-				       flags & FAULT_FLAG_REMOTE)) {
-		trace_spf_vma_access(_RET_IP_, vma, address);
-		ret = VM_FAULT_SIGSEGV;
-		goto out_put;
-	}
+				       flags & FAULT_FLAG_REMOTE))
+		goto out_segv;
 
 	/* This is one is required to check that the VMA has write access set */
 	if (flags & FAULT_FLAG_WRITE) {
-		if (unlikely(!(vmf.vma_flags & VM_WRITE))) {
-			trace_spf_vma_access(_RET_IP_, vma, address);
-			ret = VM_FAULT_SIGSEGV;
-			goto out_put;
-		}
-	} else if (unlikely(!(vmf.vma_flags & (VM_READ|VM_EXEC|VM_WRITE)))) {
-		trace_spf_vma_access(_RET_IP_, vma, address);
-		ret = VM_FAULT_SIGSEGV;
-		goto out_put;
-	}
+		if (unlikely(!(vmf.vma_flags & VM_WRITE)))
+			goto out_segv;
+	} else if (unlikely(!(vmf.vma_flags & (VM_READ|VM_EXEC|VM_WRITE))))
+		goto out_segv;
 
 	if (IS_ENABLED(CONFIG_NUMA)) {
 		struct mempolicy *pol;
@@ -4416,12 +4417,12 @@ int __handle_speculative_fault(struct mm_struct *mm, unsigned long address,
 		 * mpol_misplaced() which are not compatible with the
 		 *speculative page fault processing.
 		 */
-		pol = __get_vma_policy(vma, address);
+		pol = __get_vma_policy(vmf.vma, address);
 		if (!pol)
 			pol = get_task_policy(current);
 		if (pol && pol->mode == MPOL_INTERLEAVE) {
-			trace_spf_vma_notsup(_RET_IP_, vma, address);
-			goto out_put;
+			trace_spf_vma_notsup(_RET_IP_, vmf.vma, address);
+			return VM_FAULT_RETRY;
 		}
 	}
 
@@ -4483,9 +4484,8 @@ int __handle_speculative_fault(struct mm_struct *mm, unsigned long address,
 		vmf.pte = NULL;
 	}
 
-	vmf.vma = vma;
-	vmf.pgoff = linear_page_index(vma, address);
-	vmf.gfp_mask = __get_fault_gfp_mask(vma);
+	vmf.pgoff = linear_page_index(vmf.vma, address);
+	vmf.gfp_mask = __get_fault_gfp_mask(vmf.vma);
 	vmf.sequence = seq;
 	vmf.flags = flags;
 
@@ -4495,16 +4495,22 @@ int __handle_speculative_fault(struct mm_struct *mm, unsigned long address,
 	 * We need to re-validate the VMA after checking the bounds, otherwise
 	 * we might have a false positive on the bounds.
 	 */
-	if (read_seqcount_retry(&vma->vm_sequence, seq)) {
-		trace_spf_vma_changed(_RET_IP_, vma, address);
-		goto out_put;
+	if (read_seqcount_retry(&vmf.vma->vm_sequence, seq)) {
+		trace_spf_vma_changed(_RET_IP_, vmf.vma, address);
+		return VM_FAULT_RETRY;
 	}
 
 	mem_cgroup_oom_enable();
 	ret = handle_pte_fault(&vmf);
 	mem_cgroup_oom_disable();
 
-	put_vma(vma);
+	/*
+	 * If there is no need to retry, don't return the vma to the caller.
+	 */
+	if (ret != VM_FAULT_RETRY) {
+		put_vma(vmf.vma);
+		*vma = NULL;
+	}
 
 	/*
 	 * The task may have entered a memcg OOM situation but
@@ -4517,9 +4523,35 @@ int __handle_speculative_fault(struct mm_struct *mm, unsigned long address,
 	return ret;
 
 out_walk:
-	trace_spf_vma_notsup(_RET_IP_, vma, address);
+	trace_spf_vma_notsup(_RET_IP_, vmf.vma, address);
 	local_irq_enable();
-out_put:
+	return VM_FAULT_RETRY;
+
+out_segv:
+	trace_spf_vma_access(_RET_IP_, vmf.vma, address);
+	/*
+	 * We don't return VM_FAULT_RETRY so the caller is not expected to
+	 * retrieve the fetched VMA.
+	 */
+	put_vma(vmf.vma);
+	*vma = NULL;
+	return VM_FAULT_SIGSEGV;
+}
+
+/*
+ * This is used to know if the vma fetch in the speculative page fault handler
+ * is still valid when trying the regular fault path while holding the
+ * mmap_sem.
+ * The call to put_vma(vma) must be made after checking the vma's fields, as
+ * the vma may be freed by put_vma(). In such a case it is expected that false
+ * is returned.
+ */
+bool can_reuse_spf_vma(struct vm_area_struct *vma, unsigned long address)
+{
+	bool ret;
+
+	ret = !RB_EMPTY_NODE(&vma->vm_rb) &&
+		vma->vm_start <= address && address < vma->vm_end;
 	put_vma(vma);
 	return ret;
 }
-- 
2.7.4

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox