Anshuman Khandual <khand...@linux.vnet.ibm.com> wrote on 13.04.2015
10:48:57:
> On 04/10/2015 04:03 PM, Ulrich Weigand wrote:
> > - You provide checkpointed FPR and VMX registers, but there doesn't
seem
> > to be any way to get at the checkpointed *VSX* registers (i.e. the
part
> > that is neither covered by FPR or VMX, corresponding to NT_PPC_VSX).
>
> Will change vsr_get, vsr_set functions as we have done for fpr_get and
fpr_set
> functions. Also will add one more ELF core note NT_PPC_TM_CVSX to fetch
the
> check pointed state of VSX register while inside the transaction.
OK.
> > I would much prefer three separate regsets (e.g. NT_PPC_DSCR,
NT_PPC_PPR,
> > and NT_PPC_TAR), each of which is available and valid if and only if
the
> > current processor actually has the register in question.
>
> Thats like adding one ELF core note for every single register
> because we cannot
> put them in any category. Then as Michael Ellerman had pointed out to
include
> a lot more registers in this MISC category (which we are not doing right
now
> in the interest of having minimum support available before we look at the
full
> possible list of MISC registers), we should add one ELF core note section
for
> each of those individual registers ? I am not sure.
This confuses me a bit. My understanding was that ptrace regsets, once
defined, should never change in the future. (GDB will only check whether
or not a regset is supported; if it is, it will expect the contents to be
as it expects them to be.) "Including a lot more registers" would
therefore
seem to require adding new regsets anyway, which is one of the reasons why
I disagree a "MISC" regset is particularly useful.
> > - Similarly, the NT_PPC_TM_SPR regset as currently defined mixes and
> > matches
> > registers with different "lifetimes". The transactional memory
registers
> > (TFHAR, TEXASR, TFIAR) are available *always* on machines that
support
> > transactions. But the other registers in that regset are
checkpointed
> > versions that are only available/valid within a transaction. I think
a
> > better way to faithfully represent this would be to have the
> > NT_PPC_TM_SPR
> > regset only contain the transcational memory registers, and use
separate
> > regsets for the checkpointed registers -- those should parallel the
non-
> > checkpointed register regset.
>
> Right now, we support NT_PPC_TM_SPR only inside the transaction, so we
dont
> have the problem with different "lifetimes" registers accessed together.
But
> yes, I get your point.
Since the transactional SPRs are accessible from user space outside of a
transaction, it would make sense for them to accessible from ptrace as
well.
If the current patch set doesn't do that, I guess it would be better to
change that.
> > - Particularly confusing to me is the "checkpointed original MSR" which
> > currently also resides in NT_PPC_TM_SPR. What exactly is this? How
> > does that differ from the MSR slot in the NT_PPC_TM_CGPR regset?
>
> I believed it stores the check pointed MSR value which was in the
register
> before the transaction started. But then how it is different from the
> ckpt_regs.msr, I am not sure. Mikey or Michael should be able to clarify
> more on this. I can see "orig_msr" getting used in many places to hold
the
> check pointed value of MSR.
Your other mail states that the orig_mst may be irrelevant for ptrace
anyway ... that would be OK with me as well.
> > In any case, it seems the ptrace set-register case currently allows
user
> > space to restore *any* arbitrary value into the checkpointed MSR,
which
> > would presumably get restored into the real MSR at some point, unless
I'm
> > missing something here. Do we not need a check that only safe bits
are
> > modified, just like with ptrace access to the real MSR?
>
> Where and which safe bits do we check before writing any value into the
MSR
> register from ptrace interface ? May be I am missing something here.
All ptrace accesses to *set* the regular msr go via this routine:
static int set_user_msr(struct task_struct *task, unsigned long msr)
{
task->thread.regs->msr &= ~MSR_DEBUGCHANGE;
task->thread.regs->msr |= msr & MSR_DEBUGCHANGE;
return 0;
}
I think we'd need to do the equivalent whenever changing the checkpointed
MSR.
Bye,
Ulrich
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
