[PATCH RFC] Interface to set SPRN_TIDR

[PATCH RFC] Interface to set SPRN_TIDR

[Sukadev Bhattiprolu]

We need the SPRN_TIDR to be set for use with fast thread-wakeup
(core-to-core wakeup) in VAS. Each user thread that has a receive
window setup and expects to be notified when a sender issues a paste
needs to have a unique SPRN_TIDR value.

The SPRN_TIDR value only needs to unique within the process but for
now we use a globally unique thread id as described below.

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
Changelog[v2]
	- Michael Ellerman: Use an interface to assign TIDR so it is
	  assigned to only threads that need it; move assignment to
	  restore_sprs(). Drop lint from rebase;


 arch/powerpc/include/asm/processor.h |  4 ++
 arch/powerpc/include/asm/switch_to.h |  3 ++
 arch/powerpc/kernel/process.c        | 97 ++++++++++++++++++++++++++++++++++++
 3 files changed, 104 insertions(+)

diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index fab7ff8..bf6ba63 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -232,6 +232,10 @@ struct debug_reg {
 struct thread_struct {
 	unsigned long	ksp;		/* Kernel stack pointer */
 
+#ifdef CONFIG_PPC_VAS
+	unsigned long	tidr;
+#endif
+
 #ifdef CONFIG_PPC64
 	unsigned long	ksp_vsid;
 #endif
diff --git a/arch/powerpc/include/asm/switch_to.h b/arch/powerpc/include/asm/switch_to.h
index 17c8380..4962455 100644
--- a/arch/powerpc/include/asm/switch_to.h
+++ b/arch/powerpc/include/asm/switch_to.h
@@ -91,4 +91,7 @@ static inline void clear_task_ebb(struct task_struct *t)
 #endif
 }
 
+extern void 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 1f0fd36..13abb22 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1132,6 +1132,10 @@ static inline void restore_sprs(struct thread_struct *old_thread,
 			mtspr(SPRN_TAR, new_thread->tar);
 	}
 #endif
+#ifdef CONFIG_PPC_VAS
+	if (old_thread->tidr != new_thread->tidr)
+		mtspr(SPRN_TIDR, new_thread->tidr);
+#endif
 }
 
 #ifdef CONFIG_PPC_BOOK3S_64
@@ -1446,9 +1450,97 @@ void flush_thread(void)
 #endif /* CONFIG_HAVE_HW_BREAKPOINT */
 }
 
+#ifdef CONFIG_PPC_VAS
+static DEFINE_SPINLOCK(vas_thread_id_lock);
+static DEFINE_IDA(vas_thread_ida);
+
+/*
+ * We need to assign an unique thread id to each thread in a process. This
+ * thread id is intended to be used with the Fast Thread-wakeup (aka Core-
+ * to-core wakeup) mechanism being implemented on top of Virtual Accelerator
+ * Switchboard (VAS).
+ *
+ * To get a unique thread-id per process we could simply use task_pid_nr()
+ * but the problem is that task_pid_nr() is not yet available for the thread
+ * when copy_thread() is called. Fixing that would require changing more
+ * intrusive arch-neutral code in code path in copy_process()?.
+ *
+ * Further, to assign unique thread ids 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 thraed ids for now.
+ *
+ * NOTE: TIDR 0 indicates that the thread does not need a TIDR value.
+ * 	 For now, only threads that expect to be notified by the VAS
+ * 	 hardware need a TIDR value and we assign values > 0 for those.
+ */
+#define MAX_THREAD_CONTEXT	((1 << 15) - 2)
+static int assign_thread_tidr(void)
+{
+	int index;
+	int err;
+
+again:
+	if (!ida_pre_get(&vas_thread_ida, GFP_KERNEL))
+		return -ENOMEM;
+
+	spin_lock(&vas_thread_id_lock);
+	err = ida_get_new_above(&vas_thread_ida, 1, &index);
+	spin_unlock(&vas_thread_id_lock);
+
+	if (err == -EAGAIN)
+		goto again;
+	else if (err)
+		return err;
+
+	if (index > MAX_THREAD_CONTEXT) {
+		spin_lock(&vas_thread_id_lock);
+		ida_remove(&vas_thread_ida, index);
+		spin_unlock(&vas_thread_id_lock);
+		return -ENOMEM;
+	}
+
+	return index;
+}
+
+static void free_thread_tidr(int id)
+{
+	spin_lock(&vas_thread_id_lock);
+	ida_remove(&vas_thread_ida, id);
+	spin_unlock(&vas_thread_id_lock);
+}
+
+void clear_thread_tidr(struct task_struct *t)
+{
+	if (t->thread.tidr) {
+		free_thread_tidr(t->thread.tidr);
+		t->thread.tidr = 0;
+		mtspr(SPRN_TIDR, 0);
+	}
+}
+
+/*
+ * Assign an unique thread id for this thread and set it in the
+ * thread structure. For now, we need this interface only for
+ * the current task.
+ */
+void set_thread_tidr(struct task_struct *t)
+{
+	WARN_ON(t != current);
+	t->thread.tidr = assign_thread_tidr();
+	mtspr(SPRN_TIDR, t->thread.tidr);
+}
+
+#endif /* CONFIG_PPC_VAS */
+
+
 void
 release_thread(struct task_struct *t)
 {
+#ifdef CONFIG_PPC_VAS
+	clear_thread_tidr(t);
+#endif
 }
 
 /*
@@ -1474,6 +1566,8 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
 
 	clear_task_ebb(dst);
 
+	dst->thread.tidr = 0;
+
 	return 0;
 }
 
@@ -1584,6 +1678,9 @@ int copy_thread(unsigned long clone_flags, unsigned long usp,
 #endif
 
 	setup_ksp_vsid(p, sp);
+#ifdef CONFIG_PPC_VAS
+	p->thread.tidr = 0;
+#endif
 
 #ifdef CONFIG_PPC64 
 	if (cpu_has_feature(CPU_FTR_DSCR)) {
-- 
2.7.4

Re: [PATCH RFC] Interface to set SPRN_TIDR

[Michael Neuling]

Suka,

Please CC Christophe who as an alternative way of doing this. We ned to get
agreement across all users of TIDR/AS_notify...

His patch is here:
  https://lists.ozlabs.org/pipermail/linuxppc-dev/2017-August/161582.html

Mikey

On Tue, 2017-08-29 at 19:38 -0700, Sukadev Bhattiprolu wrote:
> We need the SPRN_TIDR to be set for use with fast thread-wakeup
> (core-to-core wakeup) in VAS. Each user thread that has a receive
> window setup and expects to be notified when a sender issues a paste
> needs to have a unique SPRN_TIDR value.
>=20
> The SPRN_TIDR value only needs to unique within the process but for
> now we use a globally unique thread id as described below.
>=20
> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> ---
> Changelog[v2]
> 	- Michael Ellerman: Use an interface to assign TIDR so it is
> 	=C2=A0=C2=A0assigned to only threads that need it; move assignment to
> 	=C2=A0=C2=A0restore_sprs(). Drop lint from rebase;
>=20
>=20
> =C2=A0arch/powerpc/include/asm/processor.h |=C2=A0=C2=A04 ++
> =C2=A0arch/powerpc/include/asm/switch_to.h |=C2=A0=C2=A03 ++
> =C2=A0arch/powerpc/kernel/process.c=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0| 97
> ++++++++++++++++++++++++++++++++++++
> =C2=A03 files changed, 104 insertions(+)
>=20
> diff --git a/arch/powerpc/include/asm/processor.h
> b/arch/powerpc/include/asm/processor.h
> index fab7ff8..bf6ba63 100644
> --- a/arch/powerpc/include/asm/processor.h
> +++ b/arch/powerpc/include/asm/processor.h
> @@ -232,6 +232,10 @@ struct debug_reg {
> =C2=A0struct thread_struct {
> =C2=A0	unsigned long	ksp;		/* Kernel stack pointer */
>=20
> +#ifdef CONFIG_PPC_VAS
> +	unsigned long	tidr;
> +#endif
> +
> =C2=A0#ifdef CONFIG_PPC64
> =C2=A0	unsigned long	ksp_vsid;
> =C2=A0#endif
> diff --git a/arch/powerpc/include/asm/switch_to.h
> b/arch/powerpc/include/asm/switch_to.h
> index 17c8380..4962455 100644
> --- a/arch/powerpc/include/asm/switch_to.h
> +++ b/arch/powerpc/include/asm/switch_to.h
> @@ -91,4 +91,7 @@ static inline void clear_task_ebb(struct task_struct *t=
)
> =C2=A0#endif
> =C2=A0}
>=20
> +extern void set_thread_tidr(struct task_struct *t);
> +extern void clear_thread_tidr(struct task_struct *t);
> +
> =C2=A0#endif /* _ASM_POWERPC_SWITCH_TO_H */
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.=
c
> index 1f0fd36..13abb22 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -1132,6 +1132,10 @@ static inline void restore_sprs(struct thread_stru=
ct
> *old_thread,
> =C2=A0			mtspr(SPRN_TAR, new_thread->tar);
> =C2=A0	}
> =C2=A0#endif
> +#ifdef CONFIG_PPC_VAS
> +	if (old_thread->tidr !=3D new_thread->tidr)
> +		mtspr(SPRN_TIDR, new_thread->tidr);
> +#endif
> =C2=A0}
>=20
> =C2=A0#ifdef CONFIG_PPC_BOOK3S_64
> @@ -1446,9 +1450,97 @@ void flush_thread(void)
> =C2=A0#endif /* CONFIG_HAVE_HW_BREAKPOINT */
> =C2=A0}
>=20
> +#ifdef CONFIG_PPC_VAS
> +static DEFINE_SPINLOCK(vas_thread_id_lock);
> +static DEFINE_IDA(vas_thread_ida);
> +
> +/*
> + * We need to assign an unique thread id to each thread in a process. Th=
is
> + * thread id is intended to be used with the Fast Thread-wakeup (aka Cor=
e-
> + * to-core wakeup) mechanism being implemented on top of Virtual Acceler=
ator
> + * Switchboard (VAS).
> + *
> + * To get a unique thread-id per process we could simply use task_pid_nr=
()
> + * but the problem is that task_pid_nr() is not yet available for the th=
read
> + * when copy_thread() is called. Fixing that would require changing more
> + * intrusive arch-neutral code in code path in copy_process()?.
> + *
> + * Further, to assign unique thread ids within each process, we need an
> + * atomic field (or an IDR) in task_struct, which again intrudes into th=
e
> + * arch-neutral code.
> + *
> + * So try to assign globally unique thraed ids for now.
> + *
> + * NOTE: TIDR 0 indicates that the thread does not need a TIDR value.
> + *=C2=A0	=C2=A0For now, only threads that expect to be notified by the V=
AS
> + *=C2=A0	=C2=A0hardware need a TIDR value and we assign values > 0 for t=
hose.
> + */
> +#define MAX_THREAD_CONTEXT	((1 << 15) - 2)
> +static int assign_thread_tidr(void)
> +{
> +	int index;
> +	int err;
> +
> +again:
> +	if (!ida_pre_get(&vas_thread_ida, GFP_KERNEL))
> +		return -ENOMEM;
> +
> +	spin_lock(&vas_thread_id_lock);
> +	err =3D ida_get_new_above(&vas_thread_ida, 1, &index);
> +	spin_unlock(&vas_thread_id_lock);
> +
> +	if (err =3D=3D -EAGAIN)
> +		goto again;
> +	else if (err)
> +		return err;
> +
> +	if (index > MAX_THREAD_CONTEXT) {
> +		spin_lock(&vas_thread_id_lock);
> +		ida_remove(&vas_thread_ida, index);
> +		spin_unlock(&vas_thread_id_lock);
> +		return -ENOMEM;
> +	}
> +
> +	return index;
> +}
> +
> +static void free_thread_tidr(int id)
> +{
> +	spin_lock(&vas_thread_id_lock);
> +	ida_remove(&vas_thread_ida, id);
> +	spin_unlock(&vas_thread_id_lock);
> +}
> +
> +void clear_thread_tidr(struct task_struct *t)
> +{
> +	if (t->thread.tidr) {
> +		free_thread_tidr(t->thread.tidr);
> +		t->thread.tidr =3D 0;
> +		mtspr(SPRN_TIDR, 0);
> +	}
> +}
> +
> +/*
> + * Assign an unique thread id for this thread and set it in the
> + * thread structure. For now, we need this interface only for
> + * the current task.
> + */
> +void set_thread_tidr(struct task_struct *t)
> +{
> +	WARN_ON(t !=3D current);
> +	t->thread.tidr =3D assign_thread_tidr();
> +	mtspr(SPRN_TIDR, t->thread.tidr);
> +}
> +
> +#endif /* CONFIG_PPC_VAS */
> +
> +
> =C2=A0void
> =C2=A0release_thread(struct task_struct *t)
> =C2=A0{
> +#ifdef CONFIG_PPC_VAS
> +	clear_thread_tidr(t);
> +#endif
> =C2=A0}
>=20
> =C2=A0/*
> @@ -1474,6 +1566,8 @@ int arch_dup_task_struct(struct task_struct *dst, s=
truct
> task_struct *src)
>=20
> =C2=A0	clear_task_ebb(dst);
>=20
> +	dst->thread.tidr =3D 0;
> +
> =C2=A0	return 0;
> =C2=A0}
>=20
> @@ -1584,6 +1678,9 @@ int copy_thread(unsigned long clone_flags, unsigned=
 long
> usp,
> =C2=A0#endif
>=20
> =C2=A0	setup_ksp_vsid(p, sp);
> +#ifdef CONFIG_PPC_VAS
> +	p->thread.tidr =3D 0;
> +#endif
>=20
> =C2=A0#ifdef CONFIG_PPC64=C2=A0
> =C2=A0	if (cpu_has_feature(CPU_FTR_DSCR)) {

Re: [PATCH RFC] Interface to set SPRN_TIDR

[Sukadev Bhattiprolu]

Michael Neuling [mikey@neuling.org] wrote:
> Suka,
>=20
> Please CC Christophe who as an alternative way of doing this. We ned to g=
et
> agreement across all users of TIDR/AS_notify...

Mikey,

Thanks. There is overlap between the two patches. I will send a patch on
top of Christophe's for the interfaces to assign/clear the TIDR value and
clear the thread->tidr during arch_dup_task_struct(). I will also drop the
CONFIG_VAS check since its not only for VAS.

Christophe, can you let me know of any other comments on this patch?

Suka
>=20
> His patch is here:
>   https://lists.ozlabs.org/pipermail/linuxppc-dev/2017-August/161582.html
>=20
> Mikey
>=20
> On Tue, 2017-08-29 at 19:38 -0700, Sukadev Bhattiprolu wrote:
> > We need the SPRN_TIDR to be set for use with fast thread-wakeup
> > (core-to-core wakeup) in VAS. Each user thread that has a receive
> > window setup and expects to be notified when a sender issues a paste
> > needs to have a unique SPRN_TIDR value.
> >=20
> > The SPRN_TIDR value only needs to unique within the process but for
> > now we use a globally unique thread id as described below.
> >=20
> > Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> > ---
> > Changelog[v2]
> > 	- Michael Ellerman: Use an interface to assign TIDR so it is
> > 	=A0=A0assigned to only threads that need it; move assignment to
> > 	=A0=A0restore_sprs(). Drop lint from rebase;
> >=20
> >=20
> > =A0arch/powerpc/include/asm/processor.h |=A0=A04 ++
> > =A0arch/powerpc/include/asm/switch_to.h |=A0=A03 ++
> > =A0arch/powerpc/kernel/process.c=A0=A0=A0=A0=A0=A0=A0=A0| 97
> > ++++++++++++++++++++++++++++++++++++
> > =A03 files changed, 104 insertions(+)
> >=20
> > diff --git a/arch/powerpc/include/asm/processor.h
> > b/arch/powerpc/include/asm/processor.h
> > index fab7ff8..bf6ba63 100644
> > --- a/arch/powerpc/include/asm/processor.h
> > +++ b/arch/powerpc/include/asm/processor.h
> > @@ -232,6 +232,10 @@ struct debug_reg {
> > =A0struct thread_struct {
> > =A0	unsigned long	ksp;		/* Kernel stack pointer */
> >=20
> > +#ifdef CONFIG_PPC_VAS
> > +	unsigned long	tidr;
> > +#endif
> > +
> > =A0#ifdef CONFIG_PPC64
> > =A0	unsigned long	ksp_vsid;
> > =A0#endif
> > diff --git a/arch/powerpc/include/asm/switch_to.h
> > b/arch/powerpc/include/asm/switch_to.h
> > index 17c8380..4962455 100644
> > --- a/arch/powerpc/include/asm/switch_to.h
> > +++ b/arch/powerpc/include/asm/switch_to.h
> > @@ -91,4 +91,7 @@ static inline void clear_task_ebb(struct task_struct =
*t)
> > =A0#endif
> > =A0}
> >=20
> > +extern void set_thread_tidr(struct task_struct *t);
> > +extern void clear_thread_tidr(struct task_struct *t);
> > +
> > =A0#endif /* _ASM_POWERPC_SWITCH_TO_H */
> > diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/proces=
s.c
> > index 1f0fd36..13abb22 100644
> > --- a/arch/powerpc/kernel/process.c
> > +++ b/arch/powerpc/kernel/process.c
> > @@ -1132,6 +1132,10 @@ static inline void restore_sprs(struct thread_st=
ruct
> > *old_thread,
> > =A0			mtspr(SPRN_TAR, new_thread->tar);
> > =A0	}
> > =A0#endif
> > +#ifdef CONFIG_PPC_VAS
> > +	if (old_thread->tidr !=3D new_thread->tidr)
> > +		mtspr(SPRN_TIDR, new_thread->tidr);
> > +#endif
> > =A0}
> >=20
> > =A0#ifdef CONFIG_PPC_BOOK3S_64
> > @@ -1446,9 +1450,97 @@ void flush_thread(void)
> > =A0#endif /* CONFIG_HAVE_HW_BREAKPOINT */
> > =A0}
> >=20
> > +#ifdef CONFIG_PPC_VAS
> > +static DEFINE_SPINLOCK(vas_thread_id_lock);
> > +static DEFINE_IDA(vas_thread_ida);
> > +
> > +/*
> > + * We need to assign an unique thread id to each thread in a process. =
This
> > + * thread id is intended to be used with the Fast Thread-wakeup (aka C=
ore-
> > + * to-core wakeup) mechanism being implemented on top of Virtual Accel=
erator
> > + * Switchboard (VAS).
> > + *
> > + * To get a unique thread-id per process we could simply use task_pid_=
nr()
> > + * but the problem is that task_pid_nr() is not yet available for the =
thread
> > + * when copy_thread() is called. Fixing that would require changing mo=
re
> > + * intrusive arch-neutral code in code path in copy_process()?.
> > + *
> > + * Further, to assign unique thread ids 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 thraed ids for now.
> > + *
> > + * NOTE: TIDR 0 indicates that the thread does not need a TIDR value.
> > + *=A0	=A0For now, only threads that expect to be notified by the VAS
> > + *=A0	=A0hardware need a TIDR value and we assign values > 0 for those.
> > + */
> > +#define MAX_THREAD_CONTEXT	((1 << 15) - 2)
> > +static int assign_thread_tidr(void)
> > +{
> > +	int index;
> > +	int err;
> > +
> > +again:
> > +	if (!ida_pre_get(&vas_thread_ida, GFP_KERNEL))
> > +		return -ENOMEM;
> > +
> > +	spin_lock(&vas_thread_id_lock);
> > +	err =3D ida_get_new_above(&vas_thread_ida, 1, &index);
> > +	spin_unlock(&vas_thread_id_lock);
> > +
> > +	if (err =3D=3D -EAGAIN)
> > +		goto again;
> > +	else if (err)
> > +		return err;
> > +
> > +	if (index > MAX_THREAD_CONTEXT) {
> > +		spin_lock(&vas_thread_id_lock);
> > +		ida_remove(&vas_thread_ida, index);
> > +		spin_unlock(&vas_thread_id_lock);
> > +		return -ENOMEM;
> > +	}
> > +
> > +	return index;
> > +}
> > +
> > +static void free_thread_tidr(int id)
> > +{
> > +	spin_lock(&vas_thread_id_lock);
> > +	ida_remove(&vas_thread_ida, id);
> > +	spin_unlock(&vas_thread_id_lock);
> > +}
> > +
> > +void clear_thread_tidr(struct task_struct *t)
> > +{
> > +	if (t->thread.tidr) {
> > +		free_thread_tidr(t->thread.tidr);
> > +		t->thread.tidr =3D 0;
> > +		mtspr(SPRN_TIDR, 0);
> > +	}
> > +}
> > +
> > +/*
> > + * Assign an unique thread id for this thread and set it in the
> > + * thread structure. For now, we need this interface only for
> > + * the current task.
> > + */
> > +void set_thread_tidr(struct task_struct *t)
> > +{
> > +	WARN_ON(t !=3D current);
> > +	t->thread.tidr =3D assign_thread_tidr();
> > +	mtspr(SPRN_TIDR, t->thread.tidr);
> > +}
> > +
> > +#endif /* CONFIG_PPC_VAS */
> > +
> > +
> > =A0void
> > =A0release_thread(struct task_struct *t)
> > =A0{
> > +#ifdef CONFIG_PPC_VAS
> > +	clear_thread_tidr(t);
> > +#endif
> > =A0}
> >=20
> > =A0/*
> > @@ -1474,6 +1566,8 @@ int arch_dup_task_struct(struct task_struct *dst,=
 struct
> > task_struct *src)
> >=20
> > =A0	clear_task_ebb(dst);
> >=20
> > +	dst->thread.tidr =3D 0;
> > +
> > =A0	return 0;
> > =A0}
> >=20
> > @@ -1584,6 +1678,9 @@ int copy_thread(unsigned long clone_flags, unsign=
ed long
> > usp,
> > =A0#endif
> >=20
> > =A0	setup_ksp_vsid(p, sp);
> > +#ifdef CONFIG_PPC_VAS
> > +	p->thread.tidr =3D 0;
> > +#endif
> >=20
> > =A0#ifdef CONFIG_PPC64=A0
> > =A0	if (cpu_has_feature(CPU_FTR_DSCR)) {

Re: [PATCH RFC] Interface to set SPRN_TIDR

[felix]

On 31/08/2017 01:32, Sukadev Bhattiprolu wrote:
> Michael Neuling [mikey@neuling.org] wrote:
>> Suka,
>>
>> Please CC Christophe who as an alternative way of doing this. We ned to get
>> agreement across all users of TIDR/AS_notify...
> Mikey,
>
> Thanks. There is overlap between the two patches. I will send a patch on
> top of Christophe's for the interfaces to assign/clear the TIDR value and
> clear the thread->tidr during arch_dup_task_struct(). I will also drop the
> CONFIG_VAS check since its not only for VAS.
>
> Christophe, can you let me know of any other comments on this patch?
>
> Suka
Suka,

I am seconding Christophe on this matter. I think that your patch now 
fulfills the CAPI use case requirements, with one exception: CAPI does 
not restrict assigning a thread id to the current task. Please find a 
few minor questions below.

Philippe

>> His patch is here:
>>    https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.ozlabs.org_pipermail_linuxppc-2Ddev_2017-2DAugust_161582.html&d=DwIFAw&c=jf_iaSHvJObTbx-siA1ZOg&r=KC0fX9VGJYXlSiH9qN2ZONDbh8vUCZFX8GUhF3rHAvg&m=XQenBfWewOBjWopgf1Fh2UAVGnlzq766MNuzx7jYfuA&s=07WOVTh9f_4IBZfCJes4lvc7LWenBlqVfAXIXxL2QH4&e=
>>
>> Mikey
>>
>> On Tue, 2017-08-29 at 19:38 -0700, Sukadev Bhattiprolu wrote:
>>> We need the SPRN_TIDR to be set for use with fast thread-wakeup
>>> (core-to-core wakeup) in VAS. Each user thread that has a receive
>>> window setup and expects to be notified when a sender issues a paste
>>> needs to have a unique SPRN_TIDR value.
>>>
>>> The SPRN_TIDR value only needs to unique within the process but for
>>> now we use a globally unique thread id as described below.
>>>
>>> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
>>> ---
>>> Changelog[v2]
>>> 	- Michael Ellerman: Use an interface to assign TIDR so it is
>>> 	  assigned to only threads that need it; move assignment to
>>> 	  restore_sprs(). Drop lint from rebase;
>>>
>>>
>>>   arch/powerpc/include/asm/processor.h |  4 ++
>>>   arch/powerpc/include/asm/switch_to.h |  3 ++
>>>   arch/powerpc/kernel/process.c        | 97
>>> ++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 104 insertions(+)
>>>
>>> diff --git a/arch/powerpc/include/asm/processor.h
>>> b/arch/powerpc/include/asm/processor.h
>>> index fab7ff8..bf6ba63 100644
>>> --- a/arch/powerpc/include/asm/processor.h
>>> +++ b/arch/powerpc/include/asm/processor.h
>>> @@ -232,6 +232,10 @@ struct debug_reg {
>>>   struct thread_struct {
>>>   	unsigned long	ksp;		/* Kernel stack pointer */
>>>
>>> +#ifdef CONFIG_PPC_VAS
>>> +	unsigned long	tidr;
>>> +#endif
>>> +
>>>   #ifdef CONFIG_PPC64
>>>   	unsigned long	ksp_vsid;
>>>   #endif
>>> diff --git a/arch/powerpc/include/asm/switch_to.h
>>> b/arch/powerpc/include/asm/switch_to.h
>>> index 17c8380..4962455 100644
>>> --- a/arch/powerpc/include/asm/switch_to.h
>>> +++ b/arch/powerpc/include/asm/switch_to.h
>>> @@ -91,4 +91,7 @@ static inline void clear_task_ebb(struct task_struct *t)
>>>   #endif
>>>   }
>>>
>>> +extern void 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 1f0fd36..13abb22 100644
>>> --- a/arch/powerpc/kernel/process.c
>>> +++ b/arch/powerpc/kernel/process.c
>>> @@ -1132,6 +1132,10 @@ static inline void restore_sprs(struct thread_struct
>>> *old_thread,
>>>   			mtspr(SPRN_TAR, new_thread->tar);
>>>   	}
>>>   #endif
>>> +#ifdef CONFIG_PPC_VAS
>>> +	if (old_thread->tidr != new_thread->tidr)
>>> +		mtspr(SPRN_TIDR, new_thread->tidr);
>>> +#endif
>>>   }
>>>
>>>   #ifdef CONFIG_PPC_BOOK3S_64
>>> @@ -1446,9 +1450,97 @@ void flush_thread(void)
>>>   #endif /* CONFIG_HAVE_HW_BREAKPOINT */
>>>   }
>>>
>>> +#ifdef CONFIG_PPC_VAS
>>> +static DEFINE_SPINLOCK(vas_thread_id_lock);
>>> +static DEFINE_IDA(vas_thread_ida);
>>> +
>>> +/*
>>> + * We need to assign an unique thread id to each thread in a process. This
>>> + * thread id is intended to be used with the Fast Thread-wakeup (aka Core-
>>> + * to-core wakeup) mechanism being implemented on top of Virtual Accelerator
>>> + * Switchboard (VAS).
>>> + *
>>> + * To get a unique thread-id per process we could simply use task_pid_nr()
>>> + * but the problem is that task_pid_nr() is not yet available for the thread
>>> + * when copy_thread() is called. Fixing that would require changing more
>>> + * intrusive arch-neutral code in code path in copy_process()?.
>>> + *
>>> + * Further, to assign unique thread ids 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 thraed ids for now.
>>> + *
>>> + * NOTE: TIDR 0 indicates that the thread does not need a TIDR value.
>>> + * 	 For now, only threads that expect to be notified by the VAS
>>> + * 	 hardware need a TIDR value and we assign values > 0 for those.
>>> + */
>>> +#define MAX_THREAD_CONTEXT	((1 << 15) - 2)
Why are you excluding ((1 << 15) - 1)?
>>> +static int assign_thread_tidr(void)
>>> +{
>>> +	int index;
>>> +	int err;
>>> +
>>> +again:
>>> +	if (!ida_pre_get(&vas_thread_ida, GFP_KERNEL))
>>> +		return -ENOMEM;
>>> +
>>> +	spin_lock(&vas_thread_id_lock);
>>> +	err = ida_get_new_above(&vas_thread_ida, 1, &index);
Why are you excluding 1?
>>> +	spin_unlock(&vas_thread_id_lock);
>>> +
>>> +	if (err == -EAGAIN)
>>> +		goto again;
>>> +	else if (err)
>>> +		return err;
>>> +
>>> +	if (index > MAX_THREAD_CONTEXT) {
>>> +		spin_lock(&vas_thread_id_lock);
>>> +		ida_remove(&vas_thread_ida, index);
>>> +		spin_unlock(&vas_thread_id_lock);
>>> +		return -ENOMEM;
>>> +	}
>>> +
>>> +	return index;
>>> +}
>>> +
>>> +static void free_thread_tidr(int id)
>>> +{
>>> +	spin_lock(&vas_thread_id_lock);
>>> +	ida_remove(&vas_thread_ida, id);
>>> +	spin_unlock(&vas_thread_id_lock);
>>> +}
>>> +
>>> +void clear_thread_tidr(struct task_struct *t)
>>> +{
>>> +	if (t->thread.tidr) {
>>> +		free_thread_tidr(t->thread.tidr);
>>> +		t->thread.tidr = 0;
>>> +		mtspr(SPRN_TIDR, 0);
>>> +	}
>>> +}
>>> +
>>> +/*
>>> + * Assign an unique thread id for this thread and set it in the
>>> + * thread structure. For now, we need this interface only for
>>> + * the current task.
>>> + */
>>> +void set_thread_tidr(struct task_struct *t)
>>> +{
>>> +	WARN_ON(t != current);
CAPI does not have this restriction. It should be possible to assign a 
thread id to an arbitrary task.
>>> +	t->thread.tidr = assign_thread_tidr();
>>> +	mtspr(SPRN_TIDR, t->thread.tidr);
>>> +}
>>> +
>>> +#endif /* CONFIG_PPC_VAS */
>>> +
>>> +
>>>   void
>>>   release_thread(struct task_struct *t)
>>>   {
>>> +#ifdef CONFIG_PPC_VAS
>>> +	clear_thread_tidr(t);
>>> +#endif
>>>   }
>>>
>>>   /*
>>> @@ -1474,6 +1566,8 @@ int arch_dup_task_struct(struct task_struct *dst, struct
>>> task_struct *src)
>>>
>>>   	clear_task_ebb(dst);
>>>
>>> +	dst->thread.tidr = 0;
>>> +
>>>   	return 0;
>>>   }
>>>
>>> @@ -1584,6 +1678,9 @@ int copy_thread(unsigned long clone_flags, unsigned long
>>> usp,
>>>   #endif
>>>
>>>   	setup_ksp_vsid(p, sp);
>>> +#ifdef CONFIG_PPC_VAS
>>> +	p->thread.tidr = 0;
>>> +#endif
>>>
>>>   #ifdef CONFIG_PPC64
>>>   	if (cpu_has_feature(CPU_FTR_DSCR)) {

Re: [PATCH RFC] Interface to set SPRN_TIDR

[Sukadev Bhattiprolu]

felix [felix@linux.vnet.ibm.com] wrote:
> On 31/08/2017 01:32, Sukadev Bhattiprolu wrote:
> > Michael Neuling [mikey@neuling.org] wrote:
> > > Suka,
> > > 
> > > Please CC Christophe who as an alternative way of doing this. We ned to get
> > > agreement across all users of TIDR/AS_notify...
> > Mikey,
> > 
> > Thanks. There is overlap between the two patches. I will send a patch on
> > top of Christophe's for the interfaces to assign/clear the TIDR value and
> > clear the thread->tidr during arch_dup_task_struct(). I will also drop the
> > CONFIG_VAS check since its not only for VAS.
> > 
> > Christophe, can you let me know of any other comments on this patch?
> > 
> > Suka
> Suka,
> 
> I am seconding Christophe on this matter. I think that your patch now
> fulfills the CAPI use case requirements, with one exception: CAPI does not
> restrict assigning a thread id to the current task. Please find a few minor
> questions below.
> 
> Philippe
> 
> > > His patch is here:
> > >    https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.ozlabs.org_pipermail_linuxppc-2Ddev_2017-2DAugust_161582.html&d=DwIFAw&c=jf_iaSHvJObTbx-siA1ZOg&r=KC0fX9VGJYXlSiH9qN2ZONDbh8vUCZFX8GUhF3rHAvg&m=XQenBfWewOBjWopgf1Fh2UAVGnlzq766MNuzx7jYfuA&s=07WOVTh9f_4IBZfCJes4lvc7LWenBlqVfAXIXxL2QH4&e=
> > > 
> > > Mikey
> > > 
> > > On Tue, 2017-08-29 at 19:38 -0700, Sukadev Bhattiprolu wrote:
> > > > We need the SPRN_TIDR to be set for use with fast thread-wakeup
> > > > (core-to-core wakeup) in VAS. Each user thread that has a receive
> > > > window setup and expects to be notified when a sender issues a paste
> > > > needs to have a unique SPRN_TIDR value.
> > > > 
> > > > The SPRN_TIDR value only needs to unique within the process but for
> > > > now we use a globally unique thread id as described below.
> > > > 
> > > > Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> > > > ---
> > > > Changelog[v2]
> > > > 	- Michael Ellerman: Use an interface to assign TIDR so it is
> > > > 	  assigned to only threads that need it; move assignment to
> > > > 	  restore_sprs(). Drop lint from rebase;
> > > > 
> > > > 
> > > >   arch/powerpc/include/asm/processor.h |  4 ++
> > > >   arch/powerpc/include/asm/switch_to.h |  3 ++
> > > >   arch/powerpc/kernel/process.c        | 97
> > > > ++++++++++++++++++++++++++++++++++++
> > > >   3 files changed, 104 insertions(+)
> > > > 
> > > > diff --git a/arch/powerpc/include/asm/processor.h
> > > > b/arch/powerpc/include/asm/processor.h
> > > > index fab7ff8..bf6ba63 100644
> > > > --- a/arch/powerpc/include/asm/processor.h
> > > > +++ b/arch/powerpc/include/asm/processor.h
> > > > @@ -232,6 +232,10 @@ struct debug_reg {
> > > >   struct thread_struct {
> > > >   	unsigned long	ksp;		/* Kernel stack pointer */
> > > > 
> > > > +#ifdef CONFIG_PPC_VAS
> > > > +	unsigned long	tidr;
> > > > +#endif
> > > > +
> > > >   #ifdef CONFIG_PPC64
> > > >   	unsigned long	ksp_vsid;
> > > >   #endif
> > > > diff --git a/arch/powerpc/include/asm/switch_to.h
> > > > b/arch/powerpc/include/asm/switch_to.h
> > > > index 17c8380..4962455 100644
> > > > --- a/arch/powerpc/include/asm/switch_to.h
> > > > +++ b/arch/powerpc/include/asm/switch_to.h
> > > > @@ -91,4 +91,7 @@ static inline void clear_task_ebb(struct task_struct *t)
> > > >   #endif
> > > >   }
> > > > 
> > > > +extern void 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 1f0fd36..13abb22 100644
> > > > --- a/arch/powerpc/kernel/process.c
> > > > +++ b/arch/powerpc/kernel/process.c
> > > > @@ -1132,6 +1132,10 @@ static inline void restore_sprs(struct thread_struct
> > > > *old_thread,
> > > >   			mtspr(SPRN_TAR, new_thread->tar);
> > > >   	}
> > > >   #endif
> > > > +#ifdef CONFIG_PPC_VAS
> > > > +	if (old_thread->tidr != new_thread->tidr)
> > > > +		mtspr(SPRN_TIDR, new_thread->tidr);
> > > > +#endif
> > > >   }
> > > > 
> > > >   #ifdef CONFIG_PPC_BOOK3S_64
> > > > @@ -1446,9 +1450,97 @@ void flush_thread(void)
> > > >   #endif /* CONFIG_HAVE_HW_BREAKPOINT */
> > > >   }
> > > > 
> > > > +#ifdef CONFIG_PPC_VAS
> > > > +static DEFINE_SPINLOCK(vas_thread_id_lock);
> > > > +static DEFINE_IDA(vas_thread_ida);
> > > > +
> > > > +/*
> > > > + * We need to assign an unique thread id to each thread in a process. This
> > > > + * thread id is intended to be used with the Fast Thread-wakeup (aka Core-
> > > > + * to-core wakeup) mechanism being implemented on top of Virtual Accelerator
> > > > + * Switchboard (VAS).
> > > > + *
> > > > + * To get a unique thread-id per process we could simply use task_pid_nr()
> > > > + * but the problem is that task_pid_nr() is not yet available for the thread
> > > > + * when copy_thread() is called. Fixing that would require changing more
> > > > + * intrusive arch-neutral code in code path in copy_process()?.
> > > > + *
> > > > + * Further, to assign unique thread ids 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 thraed ids for now.
> > > > + *
> > > > + * NOTE: TIDR 0 indicates that the thread does not need a TIDR value.
> > > > + * 	 For now, only threads that expect to be notified by the VAS
> > > > + * 	 hardware need a TIDR value and we assign values > 0 for those.
> > > > + */
> > > > +#define MAX_THREAD_CONTEXT	((1 << 15) - 2)
> Why are you excluding ((1 << 15) - 1)?

You are right. I don't need to exclude that. Also, TIDR is a 16-bit (0:15 in
VAS's Local Notify TID) value right? So, will change to

	#define MAX_THREAD_CONTEXT	((1 << 16) - 1)

> > > > +static int assign_thread_tidr(void)
> > > > +{
> > > > +	int index;
> > > > +	int err;
> > > > +
> > > > +again:
> > > > +	if (!ida_pre_get(&vas_thread_ida, GFP_KERNEL))
> > > > +		return -ENOMEM;
> > > > +
> > > > +	spin_lock(&vas_thread_id_lock);
> > > > +	err = ida_get_new_above(&vas_thread_ida, 1, &index);
> Why are you excluding 1?

I think the "_above" in the name is misleading. The function header for
ida_get_new_above() says "above or equal" so 1 is not excluded?

> > > > +	spin_unlock(&vas_thread_id_lock);
> > > > +
> > > > +	if (err == -EAGAIN)
> > > > +		goto again;
> > > > +	else if (err)
> > > > +		return err;
> > > > +
> > > > +	if (index > MAX_THREAD_CONTEXT) {
> > > > +		spin_lock(&vas_thread_id_lock);
> > > > +		ida_remove(&vas_thread_ida, index);
> > > > +		spin_unlock(&vas_thread_id_lock);
> > > > +		return -ENOMEM;
> > > > +	}
> > > > +
> > > > +	return index;
> > > > +}
> > > > +
> > > > +static void free_thread_tidr(int id)
> > > > +{
> > > > +	spin_lock(&vas_thread_id_lock);
> > > > +	ida_remove(&vas_thread_ida, id);
> > > > +	spin_unlock(&vas_thread_id_lock);
> > > > +}
> > > > +
> > > > +void clear_thread_tidr(struct task_struct *t)
> > > > +{
> > > > +	if (t->thread.tidr) {
> > > > +		free_thread_tidr(t->thread.tidr);
> > > > +		t->thread.tidr = 0;
> > > > +		mtspr(SPRN_TIDR, 0);
> > > > +	}
> > > > +}
> > > > +
> > > > +/*
> > > > + * Assign an unique thread id for this thread and set it in the
> > > > + * thread structure. For now, we need this interface only for
> > > > + * the current task.
> > > > + */
> > > > +void set_thread_tidr(struct task_struct *t)
> > > > +{
> > > > +	WARN_ON(t != current);
> CAPI does not have this restriction. It should be possible to assign a
> thread id to an arbitrary task.

I see. We (or caller) just have to figure out the locking for the task.

> > > > +	t->thread.tidr = assign_thread_tidr();
> > > > +	mtspr(SPRN_TIDR, t->thread.tidr);
> > > > +}
> > > > +

Re: [PATCH RFC] Interface to set SPRN_TIDR

[Philippe Bergheaud]

On 31/08/2017 20:06, Sukadev Bhattiprolu wrote:
> felix [felix@linux.vnet.ibm.com] wrote:
>> On 31/08/2017 01:32, Sukadev Bhattiprolu wrote:
>>> Michael Neuling [mikey@neuling.org] wrote:
>>>> Suka,
>>>>
>>>> Please CC Christophe who as an alternative way of doing this. We ned to get
>>>> agreement across all users of TIDR/AS_notify...
>>> Mikey,
>>>
>>> Thanks. There is overlap between the two patches. I will send a patch on
>>> top of Christophe's for the interfaces to assign/clear the TIDR value and
>>> clear the thread->tidr during arch_dup_task_struct(). I will also drop the
>>> CONFIG_VAS check since its not only for VAS.
>>>
>>> Christophe, can you let me know of any other comments on this patch?
>>>
>>> Suka
>> Suka,
>>
>> I am seconding Christophe on this matter. I think that your patch now
>> fulfills the CAPI use case requirements, with one exception: CAPI does not
>> restrict assigning a thread id to the current task. Please find a few minor
>> questions below.
>>
>> Philippe
>>
>>>> His patch is here:
>>>>     https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.ozlabs.org_pipermail_linuxppc-2Ddev_2017-2DAugust_161582.html&d=DwIFAw&c=jf_iaSHvJObTbx-siA1ZOg&r=KC0fX9VGJYXlSiH9qN2ZONDbh8vUCZFX8GUhF3rHAvg&m=XQenBfWewOBjWopgf1Fh2UAVGnlzq766MNuzx7jYfuA&s=07WOVTh9f_4IBZfCJes4lvc7LWenBlqVfAXIXxL2QH4&e=
>>>>
>>>> Mikey
>>>>
>>>> On Tue, 2017-08-29 at 19:38 -0700, Sukadev Bhattiprolu wrote:
>>>>> We need the SPRN_TIDR to be set for use with fast thread-wakeup
>>>>> (core-to-core wakeup) in VAS. Each user thread that has a receive
>>>>> window setup and expects to be notified when a sender issues a paste
>>>>> needs to have a unique SPRN_TIDR value.
>>>>>
>>>>> The SPRN_TIDR value only needs to unique within the process but for
>>>>> now we use a globally unique thread id as described below.
>>>>>
>>>>> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
>>>>> ---
>>>>> Changelog[v2]
>>>>> 	- Michael Ellerman: Use an interface to assign TIDR so it is
>>>>> 	  assigned to only threads that need it; move assignment to
>>>>> 	  restore_sprs(). Drop lint from rebase;
>>>>>
>>>>>
>>>>>    arch/powerpc/include/asm/processor.h |  4 ++
>>>>>    arch/powerpc/include/asm/switch_to.h |  3 ++
>>>>>    arch/powerpc/kernel/process.c        | 97
>>>>> ++++++++++++++++++++++++++++++++++++
>>>>>    3 files changed, 104 insertions(+)
>>>>>
>>>>> diff --git a/arch/powerpc/include/asm/processor.h
>>>>> b/arch/powerpc/include/asm/processor.h
>>>>> index fab7ff8..bf6ba63 100644
>>>>> --- a/arch/powerpc/include/asm/processor.h
>>>>> +++ b/arch/powerpc/include/asm/processor.h
>>>>> @@ -232,6 +232,10 @@ struct debug_reg {
>>>>>    struct thread_struct {
>>>>>    	unsigned long	ksp;		/* Kernel stack pointer */
>>>>>
>>>>> +#ifdef CONFIG_PPC_VAS
>>>>> +	unsigned long	tidr;
>>>>> +#endif
>>>>> +
>>>>>    #ifdef CONFIG_PPC64
>>>>>    	unsigned long	ksp_vsid;
>>>>>    #endif
>>>>> diff --git a/arch/powerpc/include/asm/switch_to.h
>>>>> b/arch/powerpc/include/asm/switch_to.h
>>>>> index 17c8380..4962455 100644
>>>>> --- a/arch/powerpc/include/asm/switch_to.h
>>>>> +++ b/arch/powerpc/include/asm/switch_to.h
>>>>> @@ -91,4 +91,7 @@ static inline void clear_task_ebb(struct task_struct *t)
>>>>>    #endif
>>>>>    }
>>>>>
>>>>> +extern void 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 1f0fd36..13abb22 100644
>>>>> --- a/arch/powerpc/kernel/process.c
>>>>> +++ b/arch/powerpc/kernel/process.c
>>>>> @@ -1132,6 +1132,10 @@ static inline void restore_sprs(struct thread_struct
>>>>> *old_thread,
>>>>>    			mtspr(SPRN_TAR, new_thread->tar);
>>>>>    	}
>>>>>    #endif
>>>>> +#ifdef CONFIG_PPC_VAS
>>>>> +	if (old_thread->tidr != new_thread->tidr)
>>>>> +		mtspr(SPRN_TIDR, new_thread->tidr);
>>>>> +#endif
>>>>>    }
>>>>>
>>>>>    #ifdef CONFIG_PPC_BOOK3S_64
>>>>> @@ -1446,9 +1450,97 @@ void flush_thread(void)
>>>>>    #endif /* CONFIG_HAVE_HW_BREAKPOINT */
>>>>>    }
>>>>>
>>>>> +#ifdef CONFIG_PPC_VAS
>>>>> +static DEFINE_SPINLOCK(vas_thread_id_lock);
>>>>> +static DEFINE_IDA(vas_thread_ida);
>>>>> +
>>>>> +/*
>>>>> + * We need to assign an unique thread id to each thread in a process. This
>>>>> + * thread id is intended to be used with the Fast Thread-wakeup (aka Core-
>>>>> + * to-core wakeup) mechanism being implemented on top of Virtual Accelerator
>>>>> + * Switchboard (VAS).
>>>>> + *
>>>>> + * To get a unique thread-id per process we could simply use task_pid_nr()
>>>>> + * but the problem is that task_pid_nr() is not yet available for the thread
>>>>> + * when copy_thread() is called. Fixing that would require changing more
>>>>> + * intrusive arch-neutral code in code path in copy_process()?.
>>>>> + *
>>>>> + * Further, to assign unique thread ids 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 thraed ids for now.
>>>>> + *
>>>>> + * NOTE: TIDR 0 indicates that the thread does not need a TIDR value.
>>>>> + * 	 For now, only threads that expect to be notified by the VAS
>>>>> + * 	 hardware need a TIDR value and we assign values > 0 for those.
>>>>> + */
>>>>> +#define MAX_THREAD_CONTEXT	((1 << 15) - 2)
>> Why are you excluding ((1 << 15) - 1)?
> You are right. I don't need to exclude that. Also, TIDR is a 16-bit (0:15 in
> VAS's Local Notify TID) value right? So, will change to
>
> 	#define MAX_THREAD_CONTEXT	((1 << 16) - 1)
Yes.
>
>>>>> +static int assign_thread_tidr(void)
>>>>> +{
>>>>> +	int index;
>>>>> +	int err;
>>>>> +
>>>>> +again:
>>>>> +	if (!ida_pre_get(&vas_thread_ida, GFP_KERNEL))
>>>>> +		return -ENOMEM;
>>>>> +
>>>>> +	spin_lock(&vas_thread_id_lock);
>>>>> +	err = ida_get_new_above(&vas_thread_ida, 1, &index);
>> Why are you excluding 1?
> I think the "_above" in the name is misleading. The function header for
> ida_get_new_above() says "above or equal" so 1 is not excluded?
Understood.
>
>>>>> +	spin_unlock(&vas_thread_id_lock);
>>>>> +
>>>>> +	if (err == -EAGAIN)
>>>>> +		goto again;
>>>>> +	else if (err)
>>>>> +		return err;
>>>>> +
>>>>> +	if (index > MAX_THREAD_CONTEXT) {
>>>>> +		spin_lock(&vas_thread_id_lock);
>>>>> +		ida_remove(&vas_thread_ida, index);
>>>>> +		spin_unlock(&vas_thread_id_lock);
>>>>> +		return -ENOMEM;
>>>>> +	}
>>>>> +
>>>>> +	return index;
>>>>> +}
>>>>> +
>>>>> +static void free_thread_tidr(int id)
>>>>> +{
>>>>> +	spin_lock(&vas_thread_id_lock);
>>>>> +	ida_remove(&vas_thread_ida, id);
>>>>> +	spin_unlock(&vas_thread_id_lock);
>>>>> +}
>>>>> +
>>>>> +void clear_thread_tidr(struct task_struct *t)
>>>>> +{
>>>>> +	if (t->thread.tidr) {
>>>>> +		free_thread_tidr(t->thread.tidr);
>>>>> +		t->thread.tidr = 0;
>>>>> +		mtspr(SPRN_TIDR, 0);
>>>>> +	}
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * Assign an unique thread id for this thread and set it in the
>>>>> + * thread structure. For now, we need this interface only for
>>>>> + * the current task.
>>>>> + */
>>>>> +void set_thread_tidr(struct task_struct *t)
>>>>> +{
>>>>> +	WARN_ON(t != current);
>> CAPI does not have this restriction. It should be possible to assign a
>> thread id to an arbitrary task.
> I see. We (or caller) just have to figure out the locking for the task.
The extension to any thread can be implemented in a separate patch. 
'current' will do for now.
>
>>>>> +	t->thread.tidr = assign_thread_tidr();
>>>>> +	mtspr(SPRN_TIDR, t->thread.tidr);
>>>>> +}
>>>>> +