Anshuman Khandual <[email protected]> wrote on 23.03.2015
11:34:30:
> > With that in mind, do we have a way to set the top 32bits of the MSR
> > (which contain the TM bits) when ptracing 32 bit processes? I can't
> > find anything like that in this patch set.
>
> No, we dont have that yet. When ptracing in 32-bit mode the MSR value
> which can be viewed or set from the user space through PTRACE_GETREGS
> PTRACE_SETREGS call is it's lower 32 bits only. Either we can club
> the upper 32 bits of MSR as part of one of the ELF core notes we are
> adding in the patch series or we can create one more separate ELF core
> note for that purpose. Let me know your opinion on this.
I'm not sure I understand this. I thought we had the following:
- If the process calling ptrace is itself 64-bit (which is how GDB is
built on all current Linux distributions), then PTRACE_GETREGS etc.
will *always* operate on 64-bit register sets, even if the target
process is 32-bit.
- If the process calling ptrace is 32-bit, then PTRACE_GETREGS will
operate on 32-bit register sets. However, there is a separate
PTRACE_GETREGS64 / PTRACE_SETREGS64 call that will also provide
the opportunity to operate on the full 64-bit register set. Both
apply independently of whether the target process is 32-bit or
64-bit.
Is this not correct?
Bye,
Ulrich
On Wed, 2015-04-08 at 19:50 +0200, Ulrich Weigand wrote:
> Anshuman Khandual <[email protected]> wrote on 23.03.2015
> 11:34:30:
>
> > > With that in mind, do we have a way to set the top 32bits of the MSR
> > > (which contain the TM bits) when ptracing 32 bit processes? I can't
> > > find anything like that in this patch set.
> >
> > No, we dont have that yet. When ptracing in 32-bit mode the MSR value
> > which can be viewed or set from the user space through PTRACE_GETREGS
> > PTRACE_SETREGS call is it's lower 32 bits only. Either we can club
> > the upper 32 bits of MSR as part of one of the ELF core notes we are
> > adding in the patch series or we can create one more separate ELF core
> > note for that purpose. Let me know your opinion on this.
>
> I'm not sure I understand this. I thought we had the following:
>
> - If the process calling ptrace is itself 64-bit (which is how GDB is
> built on all current Linux distributions), then PTRACE_GETREGS etc.
> will *always* operate on 64-bit register sets, even if the target
> process is 32-bit.
>
> - If the process calling ptrace is 32-bit, then PTRACE_GETREGS will
> operate on 32-bit register sets. However, there is a separate
> PTRACE_GETREGS64 / PTRACE_SETREGS64 call that will also provide
> the opportunity to operate on the full 64-bit register set. Both
> apply independently of whether the target process is 32-bit or
> 64-bit.
>
> Is this not correct?
I think you're correct. We should be right. I'd forgotten about the
GET/SETREGS64 interfaces.
Mikey
On 04/09/2015 04:41 AM, Michael Neuling wrote:
> On Wed, 2015-04-08 at 19:50 +0200, Ulrich Weigand wrote:
>> Anshuman Khandual <[email protected]> wrote on 23.03.2015
>> 11:34:30:
>>
>>>> With that in mind, do we have a way to set the top 32bits of the MSR
>>>> (which contain the TM bits) when ptracing 32 bit processes? I can't
>>>> find anything like that in this patch set.
>>>
>>> No, we dont have that yet. When ptracing in 32-bit mode the MSR value
>>> which can be viewed or set from the user space through PTRACE_GETREGS
>>> PTRACE_SETREGS call is it's lower 32 bits only. Either we can club
>>> the upper 32 bits of MSR as part of one of the ELF core notes we are
>>> adding in the patch series or we can create one more separate ELF core
>>> note for that purpose. Let me know your opinion on this.
>>
>> I'm not sure I understand this. I thought we had the following:
>>
>> - If the process calling ptrace is itself 64-bit (which is how GDB is
>> built on all current Linux distributions), then PTRACE_GETREGS etc.
>> will *always* operate on 64-bit register sets, even if the target
>> process is 32-bit.
>>
>> - If the process calling ptrace is 32-bit, then PTRACE_GETREGS will
>> operate on 32-bit register sets. However, there is a separate
>> PTRACE_GETREGS64 / PTRACE_SETREGS64 call that will also provide
>> the opportunity to operate on the full 64-bit register set. Both
>> apply independently of whether the target process is 32-bit or
>> 64-bit.
>>
>> Is this not correct?
>
> I think you're correct. We should be right. I'd forgotten about the
> GET/SETREGS64 interfaces.
In that case, is the patch series complete and okay ? Is there any thing
else we need to verify other than waiting for the GDB test results which
Edjunior has been working on. But I am not aware of the status on the GDB
test development front.
Edjunior,
Do you have any updates ?
Regards
Anshuman
On Thu, 2015-04-09 at 18:20 +0530, Anshuman Khandual wrote:
> On 04/09/2015 04:41 AM, Michael Neuling wrote:
> > On Wed, 2015-04-08 at 19:50 +0200, Ulrich Weigand wrote:
> >> Anshuman Khandual <[email protected]> wrote on 23.03.2015
> >> 11:34:30:
> >>
> >>>> With that in mind, do we have a way to set the top 32bits of the MSR
> >>>> (which contain the TM bits) when ptracing 32 bit processes? I can't
> >>>> find anything like that in this patch set.
> >>>
> >>> No, we dont have that yet. When ptracing in 32-bit mode the MSR value
> >>> which can be viewed or set from the user space through PTRACE_GETREGS
> >>> PTRACE_SETREGS call is it's lower 32 bits only. Either we can club
> >>> the upper 32 bits of MSR as part of one of the ELF core notes we are
> >>> adding in the patch series or we can create one more separate ELF core
> >>> note for that purpose. Let me know your opinion on this.
> >>
> >> I'm not sure I understand this. I thought we had the following:
> >>
> >> - If the process calling ptrace is itself 64-bit (which is how GDB is
> >> built on all current Linux distributions), then PTRACE_GETREGS etc.
> >> will *always* operate on 64-bit register sets, even if the target
> >> process is 32-bit.
> >>
> >> - If the process calling ptrace is 32-bit, then PTRACE_GETREGS will
> >> operate on 32-bit register sets. However, there is a separate
> >> PTRACE_GETREGS64 / PTRACE_SETREGS64 call that will also provide
> >> the opportunity to operate on the full 64-bit register set. Both
> >> apply independently of whether the target process is 32-bit or
> >> 64-bit.
> >>
> >> Is this not correct?
> >
> > I think you're correct. We should be right. I'd forgotten about the
> > GET/SETREGS64 interfaces.
>
> In that case, is the patch series complete and okay ? Is there any thing
> else we need to verify other than waiting for the GDB test results which
> Edjunior has been working on. But I am not aware of the status on the GDB
> test development front.
I think we are good.
Mikey
>
> Edjunior,
>
> Do you have any updates ?
>
> Regards
> Anshuman
>
>
On 04/10/2015 08:33 AM, Michael Neuling wrote:
> On Thu, 2015-04-09 at 18:20 +0530, Anshuman Khandual wrote:
>> On 04/09/2015 04:41 AM, Michael Neuling wrote:
>>> On Wed, 2015-04-08 at 19:50 +0200, Ulrich Weigand wrote:
>>>> Anshuman Khandual <[email protected]> wrote on 23.03.2015
>>>> 11:34:30:
>>>>
>>>>>> With that in mind, do we have a way to set the top 32bits of the MSR
>>>>>> (which contain the TM bits) when ptracing 32 bit processes? I can't
>>>>>> find anything like that in this patch set.
>>>>>
>>>>> No, we dont have that yet. When ptracing in 32-bit mode the MSR value
>>>>> which can be viewed or set from the user space through PTRACE_GETREGS
>>>>> PTRACE_SETREGS call is it's lower 32 bits only. Either we can club
>>>>> the upper 32 bits of MSR as part of one of the ELF core notes we are
>>>>> adding in the patch series or we can create one more separate ELF core
>>>>> note for that purpose. Let me know your opinion on this.
>>>>
>>>> I'm not sure I understand this. I thought we had the following:
>>>>
>>>> - If the process calling ptrace is itself 64-bit (which is how GDB is
>>>> built on all current Linux distributions), then PTRACE_GETREGS etc.
>>>> will *always* operate on 64-bit register sets, even if the target
>>>> process is 32-bit.
>>>>
>>>> - If the process calling ptrace is 32-bit, then PTRACE_GETREGS will
>>>> operate on 32-bit register sets. However, there is a separate
>>>> PTRACE_GETREGS64 / PTRACE_SETREGS64 call that will also provide
>>>> the opportunity to operate on the full 64-bit register set. Both
>>>> apply independently of whether the target process is 32-bit or
>>>> 64-bit.
>>>>
>>>> Is this not correct?
>>>
>>> I think you're correct. We should be right. I'd forgotten about the
>>> GET/SETREGS64 interfaces.
>>
>> In that case, is the patch series complete and okay ? Is there any thing
>> else we need to verify other than waiting for the GDB test results which
>> Edjunior has been working on. But I am not aware of the status on the GDB
>> test development front.
>
> I think we are good.
I had posted a newer version [V7] of this patch series couple of months back
which got ignored while the discussion continued in this version.
V7: https://lkml.org/lkml/2015/1/14/19
Apart from the last gitignore related patch which already got merged into
mainline separately, all other patches should be as good even today. I will
try rebasing the series, running the base tests again and re post it in some
time.
Anshuman Khandual <[email protected]> wrote on 10.04.2015
11:10:35:
> I had posted a newer version [V7] of this patch series couple of months
back
> which got ignored while the discussion continued in this version.
>
> V7: https://lkml.org/lkml/2015/1/14/19
Ah, with all the back-and-forth on the checkpointed state, I never looked
at this. Unfortunately, there's still a number of issues with this, I
think:
- 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).
- We may have had this discussion in the past, but I still do not like the
notion of a "misc" register set, in particular since the three registers
in it are available at different architecture levels and categories.
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.
If we do have a single regset, at the very least a "get" operation should
set registers unvailable on the machine to a defined state (zero?)
instead of simply leaving memory uninitialized.
- 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.
For example, if we have NT_PPC_DSCR, there should be a NT_PPC_CDSCR for
the checkpointed version etc. (If we do stay with MISC, there should
then
be a CMISC).
- 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 may be misreading kernel code, but it seems the kernel does not
actually
use the ckpt_regs.msr slot at all, and therefore the corresponding slot
of
the NT_PPC_TM_CGPR regset is likewise undefined/unused. Would it not be
more consistent to use that slot to pass the checkpointed MSR?
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?
Bye,
Ulrich
On 04/10/2015 04:03 PM, Ulrich Weigand wrote:
> Anshuman Khandual <[email protected]> wrote on 10.04.2015
> 11:10:35:
>
>> I had posted a newer version [V7] of this patch series couple of months
> back
>> which got ignored while the discussion continued in this version.
>>
>> V7: https://lkml.org/lkml/2015/1/14/19
>
> Ah, with all the back-and-forth on the checkpointed state, I never looked
> at this. Unfortunately, there's still a number of issues with this, I
> think:
>
> - 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.
>
> - We may have had this discussion in the past, but I still do not like the
> notion of a "misc" register set, in particular since the three registers
> in it are available at different architecture levels and categories.
Misc category as always stands for registers which can not be easily classified
into any meaningful categories.
>
> 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.
>
> If we do have a single regset, at the very least a "get" operation should
> set registers unvailable on the machine to a defined state (zero?)
> instead of simply leaving memory uninitialized.
Yeah sure, we can do that.
>
> - 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.
>
> For example, if we have NT_PPC_DSCR, there should be a NT_PPC_CDSCR for
> the checkpointed version etc. (If we do stay with MISC, there should
> then
> be a CMISC).
But then NT_PPC_MISC and NT_PPC_CMISC can contain different set of registers.
NT_PPC_CMISC will contain the orig_msr for now which the other elf core note
does not have and NT_PPC_MISC will grow to have lot more registers in the
future leaving behind NT_PPC_CMISC as it is. Need to take care of these.
>
> - 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.
>
> I may be misreading kernel code, but it seems the kernel does not
> actually
> use the ckpt_regs.msr slot at all, and therefore the corresponding slot
> of
> the NT_PPC_TM_CGPR regset is likewise undefined/unused. Would it not be
> more consistent to use that slot to pass the checkpointed MSR?
Hmm. Its a valid point. Would like to get some more clarity on this from
Mikey whether that slot can be used for check pointed MSR value or not.
Then why did we have these two slots to hold the same check pointed MSR
value in the first place at all. Getting confused a bit.
>
> 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.
On 04/13/2015 02:18 PM, Anshuman Khandual wrote:
> On 04/10/2015 04:03 PM, Ulrich Weigand wrote:
>> Anshuman Khandual <[email protected]> wrote on 10.04.2015
>> 11:10:35:
>
> 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.
tm_orig_msr is used during process context switch only. ckpt_regs
gets used in the signal context where we save all userspace context.
So ptrace would look into the saved MSR value correctly inside
ckpt_regs.msr slot. I believe thats the check pointed MSR value
we are interested in from the ptrace perspective not the tm_orig_msr
which just gets used during process context switch.
>> I may be misreading kernel code, but it seems the kernel does not
>> actually
>> use the ckpt_regs.msr slot at all, and therefore the corresponding slot
>> of
>> the NT_PPC_TM_CGPR regset is likewise undefined/unused. Would it not be
>> more consistent to use that slot to pass the checkpointed MSR?
>
> Hmm. Its a valid point. Would like to get some more clarity on this from
> Mikey whether that slot can be used for check pointed MSR value or not.
> Then why did we have these two slots to hold the same check pointed MSR
> value in the first place at all. Getting confused a bit.
Using ckpt_regs.msr during process context switch instead of tm_orig_msr seems
to be working fine and all the basic TM tests pass, in which case we can drop
tm_orig_msr from thread_struct. Will post a patch on this and see whats the
response from others.
Anshuman Khandual <[email protected]> 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
On 04/20/2015 05:57 PM, Ulrich Weigand wrote:
> Anshuman Khandual <[email protected]> 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.
Yeah right. Started thinking that (NT_PPC_TAR, NT_PPC_CTAR),
(NT_PPC_PPR, NT_PPC_CPPR), (NT_PPC_DSCR, NT_PPC_CDSCR) kind of combinations
make more sense !
>
>>> - 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.
Yeah I agree. Will change it.
>
>>> - 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.
Yeah. The variable tm_orig_msr is used to compute MSR state inside
the kernel or what would be passed to the user space while returning
at various stages of the transaction, where as ckpt_regs.msr contains
the latest check pointed MSR value to be fetched by ptrace. Thats my
understanding as of now.
>
>>> 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.
Agree, will incorporate this change.
In summary, after putting together all the issues that we have
discussed till now regarding the number and scope of all new ELF
core note sections being added, the probable elements there in
can be listed as below.
Changed ELF core note sections
------------------------------
These core note sections need to be changed to accommodate the in
transaction ptrace requests when the running/current value of these
registers will reside some where else instead of the original places
of thread_struct.
/* Running register state */
(1) NT_PRFPREG (Accessible always)
(2) NT_PPC_VMX (Accessible always)
(3) NT_PPC_VSX (Accessible always)
New ELF core note sections
--------------------------
/* TM check pointed register set */
(1) NT_PPC_TM_CGPR --> NT_PRSTATUS (Accessible inside TM)
(2) NT_PPC_TM_CFPR --> NT_PRFPREG (Accessible inside TM)
(3) NT_PPC_TM_CVMX --> NT_PPC_VMX (Accessible inside TM)
(4) NT_PPC_TM_CVSX --> NT_PPC_VSX (Accessible inside TM)
NOTE: The register set data structure for these ELF core not
sections would exactly match with that of the corresponding
running value register sets indicated above.
/* TM SPR set */ (Accessible always)
(5) NT_PPC_TM_SPR thread->tm_tfhar
thread->tm_tfiar
thread->ttm_exasr
/* TM check pointed misc register set */
(6) NT_PPC_TM_TAR thread->tm_tar (Accessible inside TM)
(7) NT_PPC_TM_PPR thread->tm_ppr (Accessible inside TM)
(8) NT_PPC_TM_DSCR thread->tm_dscr (Accessible inside TM)
NOTE: Application can have a different set of TAR, PPR and DSCR
registers inside the transaction compared that of the outside.
Also seems like they are *not* the check pointed ones, will
double check on this. Changed the core note section name from
NT_PPC_TM_CTAR to just NT_PPC_TM_TAR and for all the others.
/* Running misc register set */
(9) NT_PPC_TAR thread->tar (Accessible always)
(10) NT_PPC_PPR thread->ppr (Accessible always)
(11) NT_PPC_DSCR thread->dscr (Accessible always)
NOTE: They are like any other special purpose register which can
be changed from the user space. So the elf core note section name
can be generic. Here are some optional ELF core note sections
which we can also add like the above ones.
(12) NT_PPC_EBBRR thread->ebbrr (Accessible inside EBB)
(13) NT_PPC_EBBHR thread->ebbhr (Accessible inside EBB)
(14) NT_PPC_BESCR thread->bescr (Accessible inside EBB)
(15) NT_PPC_SIAR thread->siar (Accessible inside EBB)
(16) NT_PPC_SDAR thread->sdar (Accessible inside EBB)
(17) NT_PPC_SIER thread->sier (Accessible inside EBB)
(18) NT_PPC_MMCR2 thread->mmcr2 (Accessible inside EBB)
(19) NT_PPC_MMCR0 thread->mmcr0 (Accessible inside EBB)
Ulrich, Mikey, MPE,
Please do let me know your thoughts on this.
Regards
Anshuman
Anshuman Khandual <[email protected]> wrote on 21.04.2015
06:55:24:
> Changed ELF core note sections
> ------------------------------
> These core note sections need to be changed to accommodate the in
> transaction ptrace requests when the running/current value of these
> registers will reside some where else instead of the original places
> of thread_struct.
>
> /* Running register state */
> (1) NT_PRFPREG (Accessible always)
> (2) NT_PPC_VMX (Accessible always)
> (3) NT_PPC_VSX (Accessible always)
>
> New ELF core note sections
> --------------------------
> /* TM check pointed register set */
> (1) NT_PPC_TM_CGPR --> NT_PRSTATUS (Accessible inside TM)
> (2) NT_PPC_TM_CFPR --> NT_PRFPREG (Accessible inside TM)
> (3) NT_PPC_TM_CVMX --> NT_PPC_VMX (Accessible inside TM)
> (4) NT_PPC_TM_CVSX --> NT_PPC_VSX (Accessible inside TM)
>
> NOTE: The register set data structure for these ELF core not
> sections would exactly match with that of the corresponding
> running value register sets indicated above.
OK.
> /* TM SPR set */ (Accessible always)
> (5) NT_PPC_TM_SPR thread->tm_tfhar
> thread->tm_tfiar
> thread->ttm_exasr
OK.
> /* TM check pointed misc register set */
> (6) NT_PPC_TM_TAR thread->tm_tar (Accessible inside TM)
> (7) NT_PPC_TM_PPR thread->tm_ppr (Accessible inside TM)
> (8) NT_PPC_TM_DSCR thread->tm_dscr (Accessible inside TM)
>
> NOTE: Application can have a different set of TAR, PPR and DSCR
> registers inside the transaction compared that of the outside.
> Also seems like they are *not* the check pointed ones, will
> double check on this. Changed the core note section name from
> NT_PPC_TM_CTAR to just NT_PPC_TM_TAR and for all the others.
Huh? How is this not the checkpointed set? I would have
expected that NT_PPC_TAR contains the current tar (which is
the one in the transaction if we're within one), while
NT_PPC_TM_CTAR contains the checkpointed value that will be
restored once the transaction aborts. Why is this not the
case?
> NOTE: They are like any other special purpose register which can
> be changed from the user space. So the elf core note section name
> can be generic. Here are some optional ELF core note sections
> which we can also add like the above ones.
>
> (12) NT_PPC_EBBRR thread->ebbrr (Accessible inside EBB)
> (13) NT_PPC_EBBHR thread->ebbhr (Accessible inside EBB)
> (14) NT_PPC_BESCR thread->bescr (Accessible inside EBB)
> (15) NT_PPC_SIAR thread->siar (Accessible inside EBB)
> (16) NT_PPC_SDAR thread->sdar (Accessible inside EBB)
> (17) NT_PPC_SIER thread->sier (Accessible inside EBB)
> (18) NT_PPC_MMCR2 thread->mmcr2 (Accessible inside EBB)
> (19) NT_PPC_MMCR0 thread->mmcr0 (Accessible inside EBB)
So I'm not really familiar with the EBB stuff. But just as a
general note, if those are in fact related (i.e. on every machine
that has EBB, all those registers will be available), it might
indeed make more sense to collect them into a single note section
(NT_PPC_EBB?) after all.
Bye,
Ulrich
On 04/21/2015 08:11 PM, Ulrich Weigand wrote:
> Anshuman Khandual <[email protected]> wrote on 21.04.2015
> 06:55:24:
>
>> Changed ELF core note sections
>> ------------------------------
>> These core note sections need to be changed to accommodate the in
>> transaction ptrace requests when the running/current value of these
>> registers will reside some where else instead of the original places
>> of thread_struct.
>>
>> /* Running register state */
>> (1) NT_PRFPREG (Accessible always)
>> (2) NT_PPC_VMX (Accessible always)
>> (3) NT_PPC_VSX (Accessible always)
>>
>> New ELF core note sections
>> --------------------------
>> /* TM check pointed register set */
>> (1) NT_PPC_TM_CGPR --> NT_PRSTATUS (Accessible inside TM)
>> (2) NT_PPC_TM_CFPR --> NT_PRFPREG (Accessible inside TM)
>> (3) NT_PPC_TM_CVMX --> NT_PPC_VMX (Accessible inside TM)
>> (4) NT_PPC_TM_CVSX --> NT_PPC_VSX (Accessible inside TM)
>>
>> NOTE: The register set data structure for these ELF core not
>> sections would exactly match with that of the corresponding
>> running value register sets indicated above.
>
> OK.
>
>> /* TM SPR set */ (Accessible always)
>> (5) NT_PPC_TM_SPR thread->tm_tfhar
>> thread->tm_tfiar
>> thread->ttm_exasr
>
> OK.
>
>> /* TM check pointed misc register set */
>> (6) NT_PPC_TM_TAR thread->tm_tar (Accessible inside TM)
>> (7) NT_PPC_TM_PPR thread->tm_ppr (Accessible inside TM)
>> (8) NT_PPC_TM_DSCR thread->tm_dscr (Accessible inside TM)
>>
>> NOTE: Application can have a different set of TAR, PPR and DSCR
>> registers inside the transaction compared that of the outside.
>> Also seems like they are *not* the check pointed ones, will
>> double check on this. Changed the core note section name from
>> NT_PPC_TM_CTAR to just NT_PPC_TM_TAR and for all the others.
>
> Huh? How is this not the checkpointed set? I would have
> expected that NT_PPC_TAR contains the current tar (which is
> the one in the transaction if we're within one), while
> NT_PPC_TM_CTAR contains the checkpointed value that will be
> restored once the transaction aborts. Why is this not the
> case?
Yeah you are right. There is one running value always which is
'thread->tm_tar' when TM is active and it is 'thread->tar' when
TM is not active. This can be accessed *always* with NT_PPC_TAR.
The check pointed TAR is 'thread->tar' only when *TM is active*
which can be accessed via NT_PPC_TM_CTAR. Will update the ELF
core note list with lifetimes.
>
>> NOTE: They are like any other special purpose register which can
>> be changed from the user space. So the elf core note section name
>> can be generic. Here are some optional ELF core note sections
>> which we can also add like the above ones.
>>
>> (12) NT_PPC_EBBRR thread->ebbrr (Accessible inside EBB)
>> (13) NT_PPC_EBBHR thread->ebbhr (Accessible inside EBB)
>> (14) NT_PPC_BESCR thread->bescr (Accessible inside EBB)
>> (15) NT_PPC_SIAR thread->siar (Accessible inside EBB)
>> (16) NT_PPC_SDAR thread->sdar (Accessible inside EBB)
>> (17) NT_PPC_SIER thread->sier (Accessible inside EBB)
>> (18) NT_PPC_MMCR2 thread->mmcr2 (Accessible inside EBB)
>> (19) NT_PPC_MMCR0 thread->mmcr0 (Accessible inside EBB)
>
> So I'm not really familiar with the EBB stuff. But just as a
> general note, if those are in fact related (i.e. on every machine
> that has EBB, all those registers will be available), it might
> indeed make more sense to collect them into a single note section
> (NT_PPC_EBB?) after all.
I agree that would save us precious ELF core note section entry slots
which are 256 in total ever for powerpc arch. Right now all these
register states in thread_struct are getting used for EBB only. But
again these are PMU related registers, in future we may need to access
them for purposes other than EBB. Yes, that will be a problem for us
to solve later on. Right now it makes more sense to group them together
under EBB heading and pass them as a group which saves ELF core note
entries for powerpc. Hopefully Mikey and Michael will agree on this.