[PATCH v5 0/7] ocxl: Implement Power9 as_notify/wait for OpenCAPI

[PATCH v5 0/7] ocxl: Implement Power9 as_notify/wait for OpenCAPI

[Alastair D'Silva]

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:
v5:
  Remove stray brace
  Fix bad rename of ocxl_ioctl_platform to ocxl_ioctl_features
v4:
  Remove the "unique" statement from the set_thread_tidr function and
    move the text explaining why it is safe from the commit message
    to the function description
v3:
  Fix references to POWER9
  Remove stray whitespace edit from docs
  Add more details to commit message for "use task_pid_nr()"
  Retitle patch 6 to indicate OCXL rather than CPU features
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 POWER9
  ocxl: Add an IOCTL so userspace knows what OCXL features are available
  ocxl: Document new OCXL IOCTLs

 Documentation/accelerators/ocxl.rst   |  11 ++++
 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, 163 insertions(+), 105 deletions(-)

-- 
2.14.3

[PATCH v5 0/7] ocxl: Implement Power9 as_notify/wait for OpenCAPI

[Alastair D'Silva]

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:
v5:
  Remove stray brace
  Fix bad rename of ocxl_ioctl_platform to ocxl_ioctl_features
v4:
  Remove the "unique" statement from the set_thread_tidr function and
    move the text explaining why it is safe from the commit message
    to the function description
v3:
  Fix references to POWER9
  Remove stray whitespace edit from docs
  Add more details to commit message for "use task_pid_nr()"
  Retitle patch 6 to indicate OCXL rather than CPU features
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 POWER9
  ocxl: Add an IOCTL so userspace knows what OCXL features are available
  ocxl: Document new OCXL IOCTLs

 Documentation/accelerators/ocxl.rst   |  11 ++++
 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, 163 insertions(+), 105 deletions(-)

-- 
2.14.3

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

[PATCH v5 1/7] powerpc: Add TIDR CPU feature for POWER9

[Alastair D'Silva]

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 66fcab13c8b4..9c0a3083571b 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 8ab51f6ca03a..415555e5b69f 100644
--- a/arch/powerpc/kernel/dt_cpu_ftrs.c
+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
@@ -716,6 +716,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.17.0

[PATCH v5 1/7] powerpc: Add TIDR CPU feature for POWER9

[Alastair D'Silva]

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 66fcab13c8b4..9c0a3083571b 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 8ab51f6ca03a..415555e5b69f 100644
--- a/arch/powerpc/kernel/dt_cpu_ftrs.c
+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
@@ -716,6 +716,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.17.0

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

Re: [PATCH v5 1/7] powerpc: Add TIDR CPU feature for POWER9

[Frederic Barrat]

Le 11/05/2018 à 08:12, Alastair D'Silva a écrit :
> 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>
> ---

Reviewed-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>



>   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 66fcab13c8b4..9c0a3083571b 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 8ab51f6ca03a..415555e5b69f 100644
> --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
> +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
> @@ -716,6 +716,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;
>   	}
> 
>   	/*
> 

Re: [PATCH v5 1/7] powerpc: Add TIDR CPU feature for POWER9

[Frederic Barrat]

Le 11/05/2018 à 08:12, Alastair D'Silva a écrit :
> 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>
> ---

Reviewed-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>



>   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 66fcab13c8b4..9c0a3083571b 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 8ab51f6ca03a..415555e5b69f 100644
> --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
> +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
> @@ -716,6 +716,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;
>   	}
> 
>   	/*
> 

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

Re: [PATCH v5 1/7] powerpc: Add TIDR CPU feature for POWER9

[Andrew Donnellan]

On 11/05/18 16:12, Alastair D'Silva wrote:
> 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>

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

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

Re: [PATCH v5 1/7] powerpc: Add TIDR CPU feature for POWER9

[Andrew Donnellan]

On 11/05/18 16:12, Alastair D'Silva wrote:
> 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>

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

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

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

Re: [v5,1/7] powerpc: Add TIDR CPU feature for POWER9

[Michael Ellerman]

On Fri, 2018-05-11 at 06:12:57 UTC, "Alastair D'Silva" wrote:
> 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>
> Reviewed-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

Series applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/819844285ef2b5d15466f5b5062514

cheers

Re: [v5,1/7] powerpc: Add TIDR CPU feature for POWER9

[Michael Ellerman]

On Fri, 2018-05-11 at 06:12:57 UTC, "Alastair D'Silva" wrote:
> 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>
> Reviewed-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

Series applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/819844285ef2b5d15466f5b5062514

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

[PATCH v5 2/7] powerpc: Use TIDR CPU feature to control TIDR allocation

[Alastair D'Silva]

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

[PATCH v5 2/7] powerpc: Use TIDR CPU feature to control TIDR allocation

[Alastair D'Silva]

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

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

Re: [PATCH v5 2/7] powerpc: Use TIDR CPU feature to control TIDR allocation

[Frederic Barrat]

Le 11/05/2018 à 08:12, Alastair D'Silva a écrit :
> 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>
> ---

Reviewed-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>


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

Re: [PATCH v5 2/7] powerpc: Use TIDR CPU feature to control TIDR allocation

[Frederic Barrat]

Le 11/05/2018 à 08:12, Alastair D'Silva a écrit :
> 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>
> ---

Reviewed-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>


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

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

Re: [PATCH v5 2/7] powerpc: Use TIDR CPU feature to control TIDR allocation

[Andrew Donnellan]

On 11/05/18 16:12, Alastair D'Silva wrote:
> 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>

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

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

Re: [PATCH v5 2/7] powerpc: Use TIDR CPU feature to control TIDR allocation

[Andrew Donnellan]

On 11/05/18 16:12, Alastair D'Silva wrote:
> 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>

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

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

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

[PATCH v5 3/7] powerpc: use task_pid_nr() for TID allocation

[Alastair D'Silva]

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.

In the unlikely event that 2 threads share the TID and are waiting,
all potential outcomes have been determined safe.

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 arch/powerpc/include/asm/switch_to.h |   1 -
 arch/powerpc/kernel/process.c        | 122 ++++++---------------------
 2 files changed, 28 insertions(+), 95 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..c5b8e53acbae 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1496,103 +1496,41 @@ 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.
+/**
+ * Assign a TIDR (thread ID) for task @t and set it in the thread
+ * structure. For now, we only support setting TIDR for 'current' task.
  *
- * 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.
+ * Since the TID value is a truncated form of it PID, it is possible
+ * (but unlikely) for 2 threads to have the same TID. In the unlikely event
+ * that 2 threads share the same TID and are waiting, one of the following
+ * cases will happen:
  *
- * 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.
+ * 1. The correct thread is running, the wrong thread is not
+ * In this situation, the correct thread is woken and proceeds to pass it's
+ * condition check.
  *
- * 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()?.
+ * 2. Neither threads are running
+ * In this situation, neither thread will be woken. When scheduled, the waiting
+ * threads will execute either a wait, which will return immediately, followed
+ * by a condition check, which will pass for the correct thread and fail
+ * for the wrong thread, or they will execute the condition check immediately.
  *
- * 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.
+ * 3. The wrong thread is running, the correct thread is not
+ * The wrong thread will be woken, but will fail it's condition check and
+ * re-execute wait. The correct thread, when scheduled, will execute either
+ * it's condition check (which will pass), or wait, which returns immediately
+ * when called the first time after the thread is scheduled, followed by it's
+ * condition check (which will pass).
  *
- * 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.
+ * 4. Both threads are running
+ * Both threads will be woken. The wrong thread will fail it's condition check
+ * and execute another wait, while the correct thread will pass it's condition
+ * check.
+ *
+ * @t: the task to set the thread ID for
  */
 int set_thread_tidr(struct task_struct *t)
 {
-	int rc;
-
 	if (!cpu_has_feature(CPU_FTR_P9_TIDR))
 		return -EINVAL;
 
@@ -1602,11 +1540,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.17.0

[PATCH v5 3/7] powerpc: use task_pid_nr() for TID allocation

[Alastair D'Silva]

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.

In the unlikely event that 2 threads share the TID and are waiting,
all potential outcomes have been determined safe.

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 arch/powerpc/include/asm/switch_to.h |   1 -
 arch/powerpc/kernel/process.c        | 122 ++++++---------------------
 2 files changed, 28 insertions(+), 95 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..c5b8e53acbae 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1496,103 +1496,41 @@ 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.
+/**
+ * Assign a TIDR (thread ID) for task @t and set it in the thread
+ * structure. For now, we only support setting TIDR for 'current' task.
  *
- * 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.
+ * Since the TID value is a truncated form of it PID, it is possible
+ * (but unlikely) for 2 threads to have the same TID. In the unlikely event
+ * that 2 threads share the same TID and are waiting, one of the following
+ * cases will happen:
  *
- * 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.
+ * 1. The correct thread is running, the wrong thread is not
+ * In this situation, the correct thread is woken and proceeds to pass it's
+ * condition check.
  *
- * 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()?.
+ * 2. Neither threads are running
+ * In this situation, neither thread will be woken. When scheduled, the waiting
+ * threads will execute either a wait, which will return immediately, followed
+ * by a condition check, which will pass for the correct thread and fail
+ * for the wrong thread, or they will execute the condition check immediately.
  *
- * 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.
+ * 3. The wrong thread is running, the correct thread is not
+ * The wrong thread will be woken, but will fail it's condition check and
+ * re-execute wait. The correct thread, when scheduled, will execute either
+ * it's condition check (which will pass), or wait, which returns immediately
+ * when called the first time after the thread is scheduled, followed by it's
+ * condition check (which will pass).
  *
- * 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.
+ * 4. Both threads are running
+ * Both threads will be woken. The wrong thread will fail it's condition check
+ * and execute another wait, while the correct thread will pass it's condition
+ * check.
+ *
+ * @t: the task to set the thread ID for
  */
 int set_thread_tidr(struct task_struct *t)
 {
-	int rc;
-
 	if (!cpu_has_feature(CPU_FTR_P9_TIDR))
 		return -EINVAL;
 
@@ -1602,11 +1540,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.17.0

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

Re: [PATCH v5 3/7] powerpc: use task_pid_nr() for TID allocation

[Frederic Barrat]

Le 11/05/2018 à 08:12, Alastair D'Silva a écrit :
> 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.
> 
> In the unlikely event that 2 threads share the TID and are waiting,
> all potential outcomes have been determined safe.
> 
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---

Thanks for adding the comment. It assumes the reader is aware that the 
TIDR value is only used for the notification using the 'wait' 
instruction, but that's likely to be the case.

Reviewed-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>


>   arch/powerpc/include/asm/switch_to.h |   1 -
>   arch/powerpc/kernel/process.c        | 122 ++++++---------------------
>   2 files changed, 28 insertions(+), 95 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..c5b8e53acbae 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -1496,103 +1496,41 @@ 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.
> +/**
> + * Assign a TIDR (thread ID) for task @t and set it in the thread
> + * structure. For now, we only support setting TIDR for 'current' task.
>    *
> - * 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.
> + * Since the TID value is a truncated form of it PID, it is possible
> + * (but unlikely) for 2 threads to have the same TID. In the unlikely event
> + * that 2 threads share the same TID and are waiting, one of the following
> + * cases will happen:
>    *
> - * 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.
> + * 1. The correct thread is running, the wrong thread is not
> + * In this situation, the correct thread is woken and proceeds to pass it's
> + * condition check.
>    *
> - * 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()?.
> + * 2. Neither threads are running
> + * In this situation, neither thread will be woken. When scheduled, the waiting
> + * threads will execute either a wait, which will return immediately, followed
> + * by a condition check, which will pass for the correct thread and fail
> + * for the wrong thread, or they will execute the condition check immediately.
>    *
> - * 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.
> + * 3. The wrong thread is running, the correct thread is not
> + * The wrong thread will be woken, but will fail it's condition check and
> + * re-execute wait. The correct thread, when scheduled, will execute either
> + * it's condition check (which will pass), or wait, which returns immediately
> + * when called the first time after the thread is scheduled, followed by it's
> + * condition check (which will pass).
>    *
> - * 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.
> + * 4. Both threads are running
> + * Both threads will be woken. The wrong thread will fail it's condition check
> + * and execute another wait, while the correct thread will pass it's condition
> + * check.
> + *
> + * @t: the task to set the thread ID for
>    */
>   int set_thread_tidr(struct task_struct *t)
>   {
> -	int rc;
> -
>   	if (!cpu_has_feature(CPU_FTR_P9_TIDR))
>   		return -EINVAL;
> 
> @@ -1602,11 +1540,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;
> 

Re: [PATCH v5 3/7] powerpc: use task_pid_nr() for TID allocation

[Frederic Barrat]

Le 11/05/2018 à 08:12, Alastair D'Silva a écrit :
> 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.
> 
> In the unlikely event that 2 threads share the TID and are waiting,
> all potential outcomes have been determined safe.
> 
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---

Thanks for adding the comment. It assumes the reader is aware that the 
TIDR value is only used for the notification using the 'wait' 
instruction, but that's likely to be the case.

Reviewed-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>


>   arch/powerpc/include/asm/switch_to.h |   1 -
>   arch/powerpc/kernel/process.c        | 122 ++++++---------------------
>   2 files changed, 28 insertions(+), 95 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..c5b8e53acbae 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -1496,103 +1496,41 @@ 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.
> +/**
> + * Assign a TIDR (thread ID) for task @t and set it in the thread
> + * structure. For now, we only support setting TIDR for 'current' task.
>    *
> - * 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.
> + * Since the TID value is a truncated form of it PID, it is possible
> + * (but unlikely) for 2 threads to have the same TID. In the unlikely event
> + * that 2 threads share the same TID and are waiting, one of the following
> + * cases will happen:
>    *
> - * 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.
> + * 1. The correct thread is running, the wrong thread is not
> + * In this situation, the correct thread is woken and proceeds to pass it's
> + * condition check.
>    *
> - * 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()?.
> + * 2. Neither threads are running
> + * In this situation, neither thread will be woken. When scheduled, the waiting
> + * threads will execute either a wait, which will return immediately, followed
> + * by a condition check, which will pass for the correct thread and fail
> + * for the wrong thread, or they will execute the condition check immediately.
>    *
> - * 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.
> + * 3. The wrong thread is running, the correct thread is not
> + * The wrong thread will be woken, but will fail it's condition check and
> + * re-execute wait. The correct thread, when scheduled, will execute either
> + * it's condition check (which will pass), or wait, which returns immediately
> + * when called the first time after the thread is scheduled, followed by it's
> + * condition check (which will pass).
>    *
> - * 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.
> + * 4. Both threads are running
> + * Both threads will be woken. The wrong thread will fail it's condition check
> + * and execute another wait, while the correct thread will pass it's condition
> + * check.
> + *
> + * @t: the task to set the thread ID for
>    */
>   int set_thread_tidr(struct task_struct *t)
>   {
> -	int rc;
> -
>   	if (!cpu_has_feature(CPU_FTR_P9_TIDR))
>   		return -EINVAL;
> 
> @@ -1602,11 +1540,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;
> 

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

Re: [PATCH v5 3/7] powerpc: use task_pid_nr() for TID allocation

[Andrew Donnellan]

On 11/05/18 16:12, Alastair D'Silva wrote:
> 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.
> 
> In the unlikely event that 2 threads share the TID and are waiting,
> all potential outcomes have been determined safe.
> 
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>

Thanks for the clarifying comment. The diff is painful to read but I 
think it makes sense :)

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

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

Re: [PATCH v5 3/7] powerpc: use task_pid_nr() for TID allocation

[Andrew Donnellan]

On 11/05/18 16:12, Alastair D'Silva wrote:
> 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.
> 
> In the unlikely event that 2 threads share the TID and are waiting,
> all potential outcomes have been determined safe.
> 
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>

Thanks for the clarifying comment. The diff is painful to read but I 
think it makes sense :)

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

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

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

[PATCH v5 4/7] ocxl: Rename pnv_ocxl_spa_remove_pe to clarify it's action

[Alastair D'Silva]

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

[PATCH v5 4/7] ocxl: Rename pnv_ocxl_spa_remove_pe to clarify it's action

[Alastair D'Silva]

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

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

Re: [PATCH v5 4/7] ocxl: Rename pnv_ocxl_spa_remove_pe to clarify it's action

[Frederic Barrat]

Le 11/05/2018 à 08:13, Alastair D'Silva a écrit :
> 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>
> ---

Acked-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>

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

Re: [PATCH v5 4/7] ocxl: Rename pnv_ocxl_spa_remove_pe to clarify it's action

[Frederic Barrat]

Le 11/05/2018 à 08:13, Alastair D'Silva a écrit :
> 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>
> ---

Acked-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>

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

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

Re: [PATCH v5 4/7] ocxl: Rename pnv_ocxl_spa_remove_pe to clarify it's action

[Andrew Donnellan]

On 11/05/18 16:13, Alastair D'Silva wrote:
> 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>

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


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

Re: [PATCH v5 4/7] ocxl: Rename pnv_ocxl_spa_remove_pe to clarify it's action

[Andrew Donnellan]

On 11/05/18 16:13, Alastair D'Silva wrote:
> 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>

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


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

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

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

[Alastair D'Silva]

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          |  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 */
-- 
2.17.0

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

[Alastair D'Silva]

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          |  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 */
-- 
2.17.0

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

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

[Frederic Barrat]

Le 11/05/2018 à 08:13, Alastair D'Silva a écrit :
> 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>
> ---

Ok, so we keep the limitation of having only one thread per context able 
to call 'wait', even though we don't have to worry about depleting the 
pool of TIDs any more. I think that's acceptable, though we don't really 
have a reason to justify it any more. Any reason you want to keep it 
that way?

   Fred


>   drivers/misc/ocxl/context.c       |  5 ++-
>   drivers/misc/ocxl/file.c          | 53 +++++++++++++++++++++++++++++++
>   drivers/misc/ocxl/link.c          | 36 +++++++++++++++++++++
>   drivers/misc/ocxl/ocxl_internal.h |  1 +
>   include/misc/ocxl.h               |  9 ++++++
>   include/uapi/misc/ocxl.h          |  8 +++++
>   6 files changed, 111 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/misc/ocxl/context.c b/drivers/misc/ocxl/context.c
> index 909e8807824a..95f74623113e 100644
> --- a/drivers/misc/ocxl/context.c
> +++ b/drivers/misc/ocxl/context.c
> @@ -34,6 +34,8 @@ int ocxl_context_init(struct ocxl_context *ctx, struct ocxl_afu *afu,
>   	mutex_init(&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 */
> 

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

[Frederic Barrat]

Le 11/05/2018 à 08:13, Alastair D'Silva a écrit :
> 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>
> ---

Ok, so we keep the limitation of having only one thread per context able 
to call 'wait', even though we don't have to worry about depleting the 
pool of TIDs any more. I think that's acceptable, though we don't really 
have a reason to justify it any more. Any reason you want to keep it 
that way?

   Fred


>   drivers/misc/ocxl/context.c       |  5 ++-
>   drivers/misc/ocxl/file.c          | 53 +++++++++++++++++++++++++++++++
>   drivers/misc/ocxl/link.c          | 36 +++++++++++++++++++++
>   drivers/misc/ocxl/ocxl_internal.h |  1 +
>   include/misc/ocxl.h               |  9 ++++++
>   include/uapi/misc/ocxl.h          |  8 +++++
>   6 files changed, 111 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/misc/ocxl/context.c b/drivers/misc/ocxl/context.c
> index 909e8807824a..95f74623113e 100644
> --- a/drivers/misc/ocxl/context.c
> +++ b/drivers/misc/ocxl/context.c
> @@ -34,6 +34,8 @@ int ocxl_context_init(struct ocxl_context *ctx, struct ocxl_afu *afu,
>   	mutex_init(&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 */
> 

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

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

[Alastair D'Silva]

> -----Original Message-----
> From: Frederic Barrat <fbarrat@linux.ibm.com>
> Sent: Friday, 11 May 2018 7:25 PM
> To: Alastair D'Silva <alastair@au1.ibm.com>; linuxppc-dev@lists.ozlabs.org
> Cc: linux-kernel@vger.kernel.org; linux-doc@vger.kernel.org;
> mikey@neuling.org; vaibhav@linux.vnet.ibm.com;
> aneesh.kumar@linux.vnet.ibm.com; malat@debian.org;
> felix@linux.vnet.ibm.com; pombredanne@nexb.com;
> sukadev@linux.vnet.ibm.com; npiggin@gmail.com;
> gregkh@linuxfoundation.org; arnd@arndb.de;
> andrew.donnellan@au1.ibm.com; fbarrat@linux.vnet.ibm.com;
> corbet@lwn.net; Alastair D'Silva <alastair@d-silva.org>
> 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@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>
> > ---
> 
> Ok, so we keep the limitation of having only one thread per context able to
> call 'wait', even though we don't have to worry about depleting the pool of
> TIDs any more. I think that's acceptable, though we don't really have a reason
> to justify it any more. Any reason you want to keep it that way?
> 

No strong reason, just trying to minimise the amount of changes. We can always expand the scope later, if we have a use-case for it.

>    Fred
> 
> 
> >   drivers/misc/ocxl/context.c       |  5 ++-
> >   drivers/misc/ocxl/file.c          | 53 +++++++++++++++++++++++++++++++
> >   drivers/misc/ocxl/link.c          | 36 +++++++++++++++++++++
> >   drivers/misc/ocxl/ocxl_internal.h |  1 +
> >   include/misc/ocxl.h               |  9 ++++++
> >   include/uapi/misc/ocxl.h          |  8 +++++
> >   6 files changed, 111 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/misc/ocxl/context.c b/drivers/misc/ocxl/context.c
> > index 909e8807824a..95f74623113e 100644
> > --- a/drivers/misc/ocxl/context.c
> > +++ b/drivers/misc/ocxl/context.c
> > @@ -34,6 +34,8 @@ int ocxl_context_init(struct ocxl_context *ctx, struct
> ocxl_afu *afu,
> >   	mutex_init(&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

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

[Alastair D'Silva]

> -----Original Message-----
> From: Frederic Barrat <fbarrat@linux.ibm.com>
> Sent: Friday, 11 May 2018 7:25 PM
> To: Alastair D'Silva <alastair@au1.ibm.com>; =
linuxppc-dev@lists.ozlabs.org
> Cc: linux-kernel@vger.kernel.org; linux-doc@vger.kernel.org;
> mikey@neuling.org; vaibhav@linux.vnet.ibm.com;
> aneesh.kumar@linux.vnet.ibm.com; malat@debian.org;
> felix@linux.vnet.ibm.com; pombredanne@nexb.com;
> sukadev@linux.vnet.ibm.com; npiggin@gmail.com;
> gregkh@linuxfoundation.org; arnd@arndb.de;
> andrew.donnellan@au1.ibm.com; fbarrat@linux.vnet.ibm.com;
> corbet@lwn.net; Alastair D'Silva <alastair@d-silva.org>
> Subject: Re: [PATCH v5 5/7] ocxl: Expose the thread_id needed for wait =
on
> POWER9
>=20
>=20
>=20
> Le 11/05/2018 =C3=A0 08:13, Alastair D'Silva a =C3=A9crit :
> > 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>
> > ---
>=20
> 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?
>=20

No strong reason, just trying to minimise the amount of changes. We can =
always expand the scope later, if we have a use-case for it.

>    Fred
>=20
>=20
> >   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 =3D 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 !=3D OPENED) {
> >   		rc =3D -EIO;
> > @@ -72,7 +75,7 @@ int ocxl_context_attach(struct ocxl_context *ctx, =
u64
> amr)
> >   	}
> >
> >   	rc =3D 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 =3D current->thread.tidr;
> > +		}
> > +
> > +		status =3D ctx->status;
> > +		mutex_unlock(&ctx->status_mutex);
> > +
> > +		if (status =3D=3D ATTACHED) {
> > +			int rc;
> > +			struct link *link =3D ctx->afu->fn->link;
> > +
> > +			rc =3D ocxl_link_update_pe(link, ctx->pasid, ctx->tidr);
> > +			if (rc)
> > +				return rc;
> > +		}
> > +
> > +		arg.thread_id =3D ctx->tidr;
> > +	} else
> > +		return -ENOENT;
> > +
> > +	if (copy_to_user(uarg, &arg, sizeof(arg)))
> > +		return -EFAULT;
> > +
> > +	return 0;
> > +}
> > +#endif
> > +
> >   #define CMD_STR(x) (x =3D=3D OCXL_IOCTL_ATTACH ? "ATTACH" :
> 		\
> >   			x =3D=3D OCXL_IOCTL_IRQ_ALLOC ? "IRQ_ALLOC" :	\
> >   			x =3D=3D OCXL_IOCTL_IRQ_FREE ? "IRQ_FREE" :
> 	\
> >   			x =3D=3D OCXL_IOCTL_IRQ_SET_FD ? "IRQ_SET_FD" :
> 	\
> >   			x =3D=3D OCXL_IOCTL_GET_METADATA ?
> "GET_METADATA" :	\
> > +			x =3D=3D 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 =3D afu_ioctl_enable_p9_wait(ctx,
> > +				(struct ocxl_ioctl_p9_wait __user *) args);
> > +		break;
> > +#endif
> > +
> >   	default:
> >   		rc =3D -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 =3D (struct link *) link_handle;
> > +	struct spa *spa =3D link->spa;
> > +	struct ocxl_process_element *pe;
> > +	int pe_handle, rc;
> > +
> > +	if (pasid > SPA_PASID_MAX)
> > +		return -EINVAL;
> > +
> > +	pe_handle =3D pasid & SPA_PE_MASK;
> > +	pe =3D spa->spa_mem + pe_handle;
> > +
> > +	mutex_lock(&spa->spa_lock);
> > +
> > +	pe->tid =3D 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 =3D 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 =3D (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 */
> >
>=20
>=20
> ---
> This email has been checked for viruses by AVG.
> http://www.avg.com

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

[Alastair D'Silva]

> -----Original Message-----
> From: Frederic Barrat <fbarrat@linux.ibm.com>
> Sent: Friday, 11 May 2018 7:25 PM
> To: Alastair D'Silva <alastair@au1.ibm.com>; linuxppc-dev@lists.ozlabs.org
> Cc: linux-kernel@vger.kernel.org; linux-doc@vger.kernel.org;
> mikey@neuling.org; vaibhav@linux.vnet.ibm.com;
> aneesh.kumar@linux.vnet.ibm.com; malat@debian.org;
> felix@linux.vnet.ibm.com; pombredanne@nexb.com;
> sukadev@linux.vnet.ibm.com; npiggin@gmail.com;
> gregkh@linuxfoundation.org; arnd@arndb.de;
> andrew.donnellan@au1.ibm.com; fbarrat@linux.vnet.ibm.com;
> corbet@lwn.net; Alastair D'Silva <alastair@d-silva.org>
> 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@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>
> > ---
> 
> Ok, so we keep the limitation of having only one thread per context able to
> call 'wait', even though we don't have to worry about depleting the pool of
> TIDs any more. I think that's acceptable, though we don't really have a reason
> to justify it any more. Any reason you want to keep it that way?
> 

No strong reason, just trying to minimise the amount of changes. We can always expand the scope later, if we have a use-case for it.

>    Fred
> 
> 
> >   drivers/misc/ocxl/context.c       |  5 ++-
> >   drivers/misc/ocxl/file.c          | 53 +++++++++++++++++++++++++++++++
> >   drivers/misc/ocxl/link.c          | 36 +++++++++++++++++++++
> >   drivers/misc/ocxl/ocxl_internal.h |  1 +
> >   include/misc/ocxl.h               |  9 ++++++
> >   include/uapi/misc/ocxl.h          |  8 +++++
> >   6 files changed, 111 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/misc/ocxl/context.c b/drivers/misc/ocxl/context.c
> > index 909e8807824a..95f74623113e 100644
> > --- a/drivers/misc/ocxl/context.c
> > +++ b/drivers/misc/ocxl/context.c
> > @@ -34,6 +34,8 @@ int ocxl_context_init(struct ocxl_context *ctx, struct
> ocxl_afu *afu,
> >   	mutex_init(&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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Frederic Barrat]

Le 11/05/2018 à 12:06, Alastair D'Silva a écrit :
> 
>> -----Original Message-----
>> From: Frederic Barrat <fbarrat@linux.ibm.com>
>> Sent: Friday, 11 May 2018 7:25 PM
>> To: Alastair D'Silva <alastair@au1.ibm.com>; linuxppc-dev@lists.ozlabs.org
>> Cc: linux-kernel@vger.kernel.org; linux-doc@vger.kernel.org;
>> mikey@neuling.org; vaibhav@linux.vnet.ibm.com;
>> aneesh.kumar@linux.vnet.ibm.com; malat@debian.org;
>> felix@linux.vnet.ibm.com; pombredanne@nexb.com;
>> sukadev@linux.vnet.ibm.com; npiggin@gmail.com;
>> gregkh@linuxfoundation.org; arnd@arndb.de;
>> andrew.donnellan@au1.ibm.com; fbarrat@linux.vnet.ibm.com;
>> corbet@lwn.net; Alastair D'Silva <alastair@d-silva.org>
>> 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@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>
>>> ---
>>
>> 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
> 
> 

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

[Frederic Barrat]

Le 11/05/2018 à 12:06, Alastair D'Silva a écrit :
> 
>> -----Original Message-----
>> From: Frederic Barrat <fbarrat@linux.ibm.com>
>> Sent: Friday, 11 May 2018 7:25 PM
>> To: Alastair D'Silva <alastair@au1.ibm.com>; linuxppc-dev@lists.ozlabs.org
>> Cc: linux-kernel@vger.kernel.org; linux-doc@vger.kernel.org;
>> mikey@neuling.org; vaibhav@linux.vnet.ibm.com;
>> aneesh.kumar@linux.vnet.ibm.com; malat@debian.org;
>> felix@linux.vnet.ibm.com; pombredanne@nexb.com;
>> sukadev@linux.vnet.ibm.com; npiggin@gmail.com;
>> gregkh@linuxfoundation.org; arnd@arndb.de;
>> andrew.donnellan@au1.ibm.com; fbarrat@linux.vnet.ibm.com;
>> corbet@lwn.net; Alastair D'Silva <alastair@d-silva.org>
>> 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@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>
>>> ---
>>
>> 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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Andrew Donnellan]

On 11/05/18 16:13, Alastair D'Silva wrote:
> 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>

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

Comments below.

> +#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;

I didn't pick this up before - please please please use braces on both 
sides of the if here.

> +
> +	if (copy_to_user(uarg, &arg, sizeof(arg)))
> +		return -EFAULT;
> +
> +	return 0;
> +}
> +#endif

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

My earlier comment about __u16 vs u16 applies for this declaration (and 
the body declaration) as well

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

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

[Andrew Donnellan]

On 11/05/18 16:13, Alastair D'Silva wrote:
> 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>

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

Comments below.

> +#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;

I didn't pick this up before - please please please use braces on both 
sides of the if here.

> +
> +	if (copy_to_user(uarg, &arg, sizeof(arg)))
> +		return -EFAULT;
> +
> +	return 0;
> +}
> +#endif

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

My earlier comment about __u16 vs u16 applies for this declaration (and 
the body declaration) as well

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

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

[PATCH v5 6/7] ocxl: Add an IOCTL so userspace knows what OCXL features are available

[Alastair D'Silva]

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>
---
 drivers/misc/ocxl/file.c | 25 +++++++++++++++++++++++++
 include/uapi/misc/ocxl.h |  6 ++++++
 2 files changed, 31 insertions(+)

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 561e6f0dfcb7..97937cfa3baa 100644
--- a/include/uapi/misc/ocxl.h
+++ b/include/uapi/misc/ocxl.h
@@ -55,6 +55,11 @@ 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 {
 	__u64 irq_offset;
 	__s32 eventfd;
@@ -70,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_features)
 
 #endif /* _UAPI_MISC_OCXL_H */
-- 
2.17.0

[PATCH v5 6/7] ocxl: Add an IOCTL so userspace knows what OCXL features are available

[Alastair D'Silva]

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>
---
 drivers/misc/ocxl/file.c | 25 +++++++++++++++++++++++++
 include/uapi/misc/ocxl.h |  6 ++++++
 2 files changed, 31 insertions(+)

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 561e6f0dfcb7..97937cfa3baa 100644
--- a/include/uapi/misc/ocxl.h
+++ b/include/uapi/misc/ocxl.h
@@ -55,6 +55,11 @@ 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 {
 	__u64 irq_offset;
 	__s32 eventfd;
@@ -70,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_features)
 
 #endif /* _UAPI_MISC_OCXL_H */
-- 
2.17.0

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

Re: [PATCH v5 6/7] ocxl: Add an IOCTL so userspace knows what OCXL features are available

[Frederic Barrat]

Le 11/05/2018 à 08:13, Alastair D'Silva a écrit :
> 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>
> ---

Acked-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>


>   drivers/misc/ocxl/file.c | 25 +++++++++++++++++++++++++
>   include/uapi/misc/ocxl.h |  6 ++++++
>   2 files changed, 31 insertions(+)
> 
> 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 561e6f0dfcb7..97937cfa3baa 100644
> --- a/include/uapi/misc/ocxl.h
> +++ b/include/uapi/misc/ocxl.h
> @@ -55,6 +55,11 @@ 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 {
>   	__u64 irq_offset;
>   	__s32 eventfd;
> @@ -70,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_features)
> 
>   #endif /* _UAPI_MISC_OCXL_H */
> 

Re: [PATCH v5 6/7] ocxl: Add an IOCTL so userspace knows what OCXL features are available

[Frederic Barrat]

Le 11/05/2018 à 08:13, Alastair D'Silva a écrit :
> 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>
> ---

Acked-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>


>   drivers/misc/ocxl/file.c | 25 +++++++++++++++++++++++++
>   include/uapi/misc/ocxl.h |  6 ++++++
>   2 files changed, 31 insertions(+)
> 
> 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 561e6f0dfcb7..97937cfa3baa 100644
> --- a/include/uapi/misc/ocxl.h
> +++ b/include/uapi/misc/ocxl.h
> @@ -55,6 +55,11 @@ 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 {
>   	__u64 irq_offset;
>   	__s32 eventfd;
> @@ -70,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_features)
> 
>   #endif /* _UAPI_MISC_OCXL_H */
> 

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

Re: [PATCH v5 6/7] ocxl: Add an IOCTL so userspace knows what OCXL features are available

[Andrew Donnellan]

On 11/05/18 16:13, Alastair D'Silva wrote:
> 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>

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

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

Re: [PATCH v5 6/7] ocxl: Add an IOCTL so userspace knows what OCXL features are available

[Andrew Donnellan]

On 11/05/18 16:13, Alastair D'Silva wrote:
> 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>

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

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

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

[PATCH v5 7/7] ocxl: Document new OCXL IOCTLs

[Alastair D'Silva]

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 ddcc58d01cfb..14cefc020e2d 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 POWER9.
+
+OCXL_IOCTL_GET_FEATURES:
+
+  Reports on which CPU features that affect OpenCAPI are usable from
+  userspace.
+
 
 mmap
 ----
-- 
2.17.0

[PATCH v5 7/7] ocxl: Document new OCXL IOCTLs

[Alastair D'Silva]

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 ddcc58d01cfb..14cefc020e2d 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 POWER9.
+
+OCXL_IOCTL_GET_FEATURES:
+
+  Reports on which CPU features that affect OpenCAPI are usable from
+  userspace.
+
 
 mmap
 ----
-- 
2.17.0

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

Re: [PATCH v5 7/7] ocxl: Document new OCXL IOCTLs

[Frederic Barrat]

Le 11/05/2018 à 08:13, Alastair D'Silva a écrit :
> From: Alastair D'Silva <alastair@d-silva.org>
> 
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---

Acked-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>


>   Documentation/accelerators/ocxl.rst | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/Documentation/accelerators/ocxl.rst b/Documentation/accelerators/ocxl.rst
> index ddcc58d01cfb..14cefc020e2d 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 POWER9.
> +
> +OCXL_IOCTL_GET_FEATURES:
> +
> +  Reports on which CPU features that affect OpenCAPI are usable from
> +  userspace.
> +
> 
>   mmap
>   ----
> 

Re: [PATCH v5 7/7] ocxl: Document new OCXL IOCTLs

[Frederic Barrat]

Le 11/05/2018 à 08:13, Alastair D'Silva a écrit :
> From: Alastair D'Silva <alastair@d-silva.org>
> 
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---

Acked-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>


>   Documentation/accelerators/ocxl.rst | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/Documentation/accelerators/ocxl.rst b/Documentation/accelerators/ocxl.rst
> index ddcc58d01cfb..14cefc020e2d 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 POWER9.
> +
> +OCXL_IOCTL_GET_FEATURES:
> +
> +  Reports on which CPU features that affect OpenCAPI are usable from
> +  userspace.
> +
> 
>   mmap
>   ----
> 

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

Re: [PATCH v5 7/7] ocxl: Document new OCXL IOCTLs

[Andrew Donnellan]

On 11/05/18 16:13, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
> 
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>

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


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

Re: [PATCH v5 7/7] ocxl: Document new OCXL IOCTLs

[Andrew Donnellan]

On 11/05/18 16:13, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
> 
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>

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


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

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