Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1335194 > unrolled thread

Re: [PATCH V10 03/28] powerpc, ptrace: Enable in transaction NT_PRFPREG ptrace requests

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH V10 03/28] powerpc, ptrace: Enable in transaction  NT_PRFPREG ptrace requests Denis Kirjanov <kda@linux-powerpc.org> - 2016-02-16 10:20 +0100
    Re: [PATCH V10 03/28] powerpc, ptrace: Enable in transaction  NT_PRFPREG ptrace requests Michael Ellerman <mpe@ellerman.id.au> - 2016-02-16 11:20 +0100

#1335194 — Re: [PATCH V10 03/28] powerpc, ptrace: Enable in transaction NT_PRFPREG ptrace requests

On 2/16/16, Anshuman Khandual <khandual@linux.vnet.ibm.com> wrote:
> This patch enables in transaction NT_PRFPREG ptrace requests.
> The function fpr_get which gets the running value of all FPR
> registers and the function fpr_set which sets the running
> value of of all FPR registers work on the running set of FPR
> registers whose location will be different if transaction is
> active. This patch makes these functions adapt to situations
> when the transaction is active.
>
> Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
> ---
>  arch/powerpc/kernel/ptrace.c | 93
> ++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 89 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
> index 30a03c0..547a979 100644
> --- a/arch/powerpc/kernel/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace.c
> @@ -358,6 +358,29 @@ static int gpr_set(struct task_struct *target, const
> struct user_regset *regset,
>  	return ret;
>  }
>
> +/*
> + * When the transaction is active, 'transact_fp' holds the current running
> + * value of all FPR registers and 'fp_state' holds the last checkpointed
> + * value of all FPR registers for the current transaction. When transaction
> + * is not active 'fp_state' holds the current running state of all the FPR
> + * registers. So this function which returns the current running values of
> + * all the FPR registers, needs to know whether any transaction is active
> + * or not.
> + *
> + * Userspace interface buffer layout:
> + *
> + * struct data {
> + *	u64	fpr[32];
> + *	u64	fpscr;
> + * };
> + *
> + * There are two config options CONFIG_VSX and CONFIG_PPC_TRANSACTIONAL_MEM
> + * which determines the final code in this function. All the combinations
> of
> + * these two config options are possible except the one below as
> transactional
> + * memory config pulls in CONFIG_VSX automatically.
> + *
> + *	!defined(CONFIG_VSX) && defined(CONFIG_PPC_TRANSACTIONAL_MEM)
> + */
>  static int fpr_get(struct task_struct *target, const struct user_regset
> *regset,
>  		   unsigned int pos, unsigned int count,
>  		   void *kbuf, void __user *ubuf)
> @@ -368,14 +391,31 @@ static int fpr_get(struct task_struct *target, const
> struct user_regset *regset,
>  #endif
>  	flush_fp_to_thread(target);
>
> -#ifdef CONFIG_VSX
> +#if defined(CONFIG_VSX) && defined(CONFIG_PPC_TRANSACTIONAL_MEM)
> +	/* copy to local buffer then write that out */
> +	if (MSR_TM_ACTIVE(target->thread.regs->msr)) {
> +		flush_altivec_to_thread(target);
> +		flush_tmregs_to_thread(target);
> +		for (i = 0; i < 32 ; i++)
use ELF_NFPREG

> +			buf[i] = target->thread.TS_TRANS_FPR(i);
> +		buf[32] = target->thread.transact_fp.fpscr;
> +	} else {
> +		for (i = 0; i < 32 ; i++)
> +			buf[i] = target->thread.TS_FPR(i);
> +		buf[32] = target->thread.fp_state.fpscr;
> +	}
> +	return user_regset_copyout(&pos, &count, &kbuf, &ubuf, buf, 0, -1);
> +#endif
> +
> +#if defined(CONFIG_VSX) && !defined(CONFIG_PPC_TRANSACTIONAL_MEM)
>  	/* copy to local buffer then write that out */
>  	for (i = 0; i < 32 ; i++)
>  		buf[i] = target->thread.TS_FPR(i);
>  	buf[32] = target->thread.fp_state.fpscr;
>  	return user_regset_copyout(&pos, &count, &kbuf, &ubuf, buf, 0, -1);
> +#endif
>
> -#else
> +#if !defined(CONFIG_VSX) && !defined(CONFIG_PPC_TRANSACTIONAL_MEM)
>  	BUILD_BUG_ON(offsetof(struct thread_fp_state, fpscr) !=
>  		     offsetof(struct thread_fp_state, fpr[32][0]));
>
> @@ -384,6 +424,29 @@ static int fpr_get(struct task_struct *target, const
> struct user_regset *regset,
>  #endif
>  }
>
> +/*
> + * When the transaction is active, 'transact_fp' holds the current running
> + * value of all FPR registers and 'fp_state' holds the last checkpointed
> + * value of all FPR registers for the current transaction. When transaction
> + * is not active 'fp_state' holds the current running state of all the FPR
> + * registers. So this function which setss the current running values of
> + * all the FPR registers, needs to know whether any transaction is active
> + * or not.
> + *
> + * Userspace interface buffer layout:
> + *
> + * struct data {
> + *	u64	fpr[32];
> + *	u64	fpscr;
> + * };
> + *
> + * There are two config options CONFIG_VSX and CONFIG_PPC_TRANSACTIONAL_MEM
> + * which determines the final code in this function. All the combinations
> of
> + * these two config options are possible except the one below as
> transactional
> + * memory config pulls in CONFIG_VSX automatically.
> + *
> + *	!defined(CONFIG_VSX) && defined(CONFIG_PPC_TRANSACTIONAL_MEM)
> + */
>  static int fpr_set(struct task_struct *target, const struct user_regset
> *regset,
>  		   unsigned int pos, unsigned int count,
>  		   const void *kbuf, const void __user *ubuf)
> @@ -394,7 +457,27 @@ static int fpr_set(struct task_struct *target, const
> struct user_regset *regset,
>  #endif
>  	flush_fp_to_thread(target);
>
> -#ifdef CONFIG_VSX
> +#if defined(CONFIG_VSX) && defined(CONFIG_PPC_TRANSACTIONAL_MEM)
> +	/* copy to local buffer then write that out */
> +	i = user_regset_copyin(&pos, &count, &kbuf, &ubuf, buf, 0, -1);
> +	if (i)
> +		return i;
> +
> +	if (MSR_TM_ACTIVE(target->thread.regs->msr)) {
> +		flush_altivec_to_thread(target);
> +		flush_tmregs_to_thread(target);
> +		for (i = 0; i < 32 ; i++)
> +			target->thread.TS_TRANS_FPR(i) = buf[i];
> +		target->thread.transact_fp.fpscr = buf[32];
> +	} else {
> +		for (i = 0; i < 32 ; i++)
> +			target->thread.TS_FPR(i) = buf[i];
> +		target->thread.fp_state.fpscr = buf[32];
> +	}
> +	return 0;
> +#endif
> +
> +#if defined(CONFIG_VSX) && !defined(CONFIG_PPC_TRANSACTIONAL_MEM)
>  	/* copy to local buffer then write that out */
>  	i = user_regset_copyin(&pos, &count, &kbuf, &ubuf, buf, 0, -1);
>  	if (i)
> @@ -403,7 +486,9 @@ static int fpr_set(struct task_struct *target, const
> struct user_regset *regset,
>  		target->thread.TS_FPR(i) = buf[i];
>  	target->thread.fp_state.fpscr = buf[32];
>  	return 0;
> -#else
> +#endif
> +
> +#if !defined(CONFIG_VSX) && !defined(CONFIG_PPC_TRANSACTIONAL_MEM)
>  	BUILD_BUG_ON(offsetof(struct thread_fp_state, fpscr) !=
>  		     offsetof(struct thread_fp_state, fpr[32][0]));
>
> --
> 2.1.0
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

[toc] | [next] | [standalone]

#1335256

On Tue, 2016-02-16 at 12:09 +0300, Denis Kirjanov wrote:

> On 2/16/16, Anshuman Khandual <khandual@linux.vnet.ibm.com> wrote:

> > This patch enables in transaction NT_PRFPREG ptrace requests.
> > The function fpr_get which gets the running value of all FPR
> > registers and the function fpr_set which sets the running
> > value of of all FPR registers work on the running set of FPR
> > registers whose location will be different if transaction is
> > active. This patch makes these functions adapt to situations
> > when the transaction is active.
> > 
> > Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
> > ---
> >  arch/powerpc/kernel/ptrace.c | 93
> > ++++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 89 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
> > index 30a03c0..547a979 100644
> > --- a/arch/powerpc/kernel/ptrace.c
> > +++ b/arch/powerpc/kernel/ptrace.c
> > @@ -358,6 +358,29 @@ static int gpr_set(struct task_struct *target, const
> > struct user_regset *regset,
> >  	return ret;
> >  }
> > 
> > +/*
> > + * When the transaction is active, 'transact_fp' holds the current running
> > + * value of all FPR registers and 'fp_state' holds the last checkpointed
> > + * value of all FPR registers for the current transaction. When transaction
> > + * is not active 'fp_state' holds the current running state of all the FPR
> > + * registers. So this function which returns the current running values of
> > + * all the FPR registers, needs to know whether any transaction is active
> > + * or not.
> > + *
> > + * Userspace interface buffer layout:
> > + *
> > + * struct data {
> > + *	u64	fpr[32];
> > + *	u64	fpscr;
> > + * };
> > + *
> > + * There are two config options CONFIG_VSX and CONFIG_PPC_TRANSACTIONAL_MEM
> > + * which determines the final code in this function. All the combinations
> > of
> > + * these two config options are possible except the one below as
> > transactional
> > + * memory config pulls in CONFIG_VSX automatically.
> > + *
> > + *	!defined(CONFIG_VSX) && defined(CONFIG_PPC_TRANSACTIONAL_MEM)
> > + */
> >  static int fpr_get(struct task_struct *target, const struct user_regset
> > *regset,
> >  		   unsigned int pos, unsigned int count,
> >  		   void *kbuf, void __user *ubuf)
> > @@ -368,14 +391,31 @@ static int fpr_get(struct task_struct *target, const
> > struct user_regset *regset,
> >  #endif
> >  	flush_fp_to_thread(target);
> > 
> > -#ifdef CONFIG_VSX
> > +#if defined(CONFIG_VSX) && defined(CONFIG_PPC_TRANSACTIONAL_MEM)
> > +	/* copy to local buffer then write that out */
> > +	if (MSR_TM_ACTIVE(target->thread.regs->msr)) {
> > +		flush_altivec_to_thread(target);
> > +		flush_tmregs_to_thread(target);
> > +		for (i = 0; i < 32 ; i++)

> use ELF_NFPREG

> > +			buf[i] = target->thread.TS_TRANS_FPR(i);
> > +		buf[32] = target->thread.transact_fp.fpscr;

#define ELF_NFPREG	33	/* includes fpscr */

So it's not what he wants here. Unless you mean that he should use i < ELF_NFPREG - 1; ?

We already have several loops over the 32 fprs, I don't think hiding the "32"
behind a macro really buys us anything.

cheers

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web