On Tue, 2018-02-20 at 14:59 +1100, Cyril Bur wrote:
> On Tue, 2018-02-20 at 14:00 +1100, Michael Neuling wrote:
> > This needs a description of what you're trying to do. "Correctly" doesn't
> > really mean anything.
> >
> >
> > On Tue, 2018-02-20 at 11:22 +1100, Cyril Bur wrote:
> > > ---
> > > arch/powerpc/kernel/process.c | 57
> > > +++++++++++++++++++++++++++++++++++++++++-
> > > -
> > > arch/powerpc/kernel/ptrace.c | 9 +++----
> > > 2 files changed, 58 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> > > index cd3ae80a6878..674f75c56172 100644
> > > --- a/arch/powerpc/kernel/process.c
> > > +++ b/arch/powerpc/kernel/process.c
> > > @@ -859,6 +859,8 @@ static inline bool tm_enabled(struct task_struct *tsk)
> > > return tsk && tsk->thread.regs && (tsk->thread.regs->msr & MSR_TM);
> > > }
> > >
> > > +static inline void save_sprs(struct thread_struct *t);
> > > +
> > > static void tm_reclaim_thread(struct thread_struct *thr, uint8_t cause)
> > > {
> > > /*
> > > @@ -879,6 +881,8 @@ static void tm_reclaim_thread(struct thread_struct
> > > *thr,
> > > uint8_t cause)
> > > if (!MSR_TM_SUSPENDED(mfmsr()))
> > > return;
> > >
> > > + save_sprs(thr);
> > > +
> > > giveup_all(container_of(thr, struct task_struct, thread));
> > >
> > > tm_reclaim(thr, cause);
> > > @@ -991,6 +995,37 @@ void tm_recheckpoint(struct thread_struct *thread)
> > >
> > > __tm_recheckpoint(thread);
> > >
> > > + /*
> > > + * This is a stripped down restore_sprs(), we need to do this
> > > + * now as we might go straight out to userspace and currently
> > > + * the checkpointed values are on the CPU.
> > > + *
> > > + * TODO: Improve
> > > + */
> > > +#ifdef CONFIG_ALTIVEC
> > > + if (cpu_has_feature(CPU_FTR_ALTIVEC))
> > > + mtspr(SPRN_VRSAVE, thread->vrsave);
> > > +#endif
> > > +#ifdef CONFIG_PPC_BOOK3S_64
> > > + if (cpu_has_feature(CPU_FTR_DSCR)) {
> > > + u64 dscr = get_paca()->dscr_default;
> > > + if (thread->dscr_inherit)
> > > + dscr = thread->dscr;
> > > +
> > > + mtspr(SPRN_DSCR, dscr);
> > > + }
> > > +
> > > + if (cpu_has_feature(CPU_FTR_ARCH_207S)) {
> > > + /* The EBB regs aren't checkpointed */
> > > + mtspr(SPRN_FSCR, thread->fscr);
> > > +
> > > + mtspr(SPRN_TAR, thread->tar);
> > > + }
> > > +
> > > + /* I think we don't need to */
> > > + if (cpu_has_feature(CPU_FTR_ARCH_300))
> > > + mtspr(SPRN_TIDR, thread->tidr);
> > > +#endif
> >
> > Why are you touching all the above hunk?
>
> I copied restore_sprs. I'm tidying that up now - we can't call
> restore_sprs because we don't have a prev and next thread.
Yeah needs to be tided up... we can't have another copy of the code.. obviously.
>
> >
> > > local_irq_restore(flags);
> > > }
> > >
> > > @@ -1193,6 +1228,11 @@ struct task_struct *__switch_to(struct task_struct
> > > *prev,
> > > #endif
> > >
> > > new_thread = &new->thread;
> > > + /*
> > > + * Why not &prev->thread; ?
> > > + * What is the difference between &prev->thread and
> > > + * ¤t->thread ?
> > > + */
> >
> > Why not just work it out and FIX THE CODE, rather than just rabbiting on
> > about
> > it! :-P
>
> Agreed - I started to and then had a mini freakout that things would
> end really badly if they're not the same. So I left that comment as a
> reminder to investigate.
>
> They should be the same though right?
Should be if prev == current.
Mikey
