| Message ID | 1609246561-5474-2-git-send-email-yangtiezhu@loongson.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add some perf support for mips | expand |
On Tue, Dec 29, 2020 at 08:55:59PM +0800, Tiezhu Yang wrote: > +u64 perf_reg_abi(struct task_struct *tsk) > +{ > + if (test_tsk_thread_flag(tsk, TIF_32BIT_REGS)) > + return PERF_SAMPLE_REGS_ABI_32; > + else > + return PERF_SAMPLE_REGS_ABI_64; > +} So we recently changed this on x86 to not rely on TIF flags. IIRC the problem is that on x86 you can change the mode of a task without the kernel being aware of it. Is something like that possible on MIPS as well? The thing x86 does today is look at it's pt_regs state to determine the actual state.
在 2021/1/4 下午6:59, Peter Zijlstra 写道: > On Tue, Dec 29, 2020 at 08:55:59PM +0800, Tiezhu Yang wrote: >> +u64 perf_reg_abi(struct task_struct *tsk) >> +{ >> + if (test_tsk_thread_flag(tsk, TIF_32BIT_REGS)) >> + return PERF_SAMPLE_REGS_ABI_32; >> + else >> + return PERF_SAMPLE_REGS_ABI_64; >> +} > So we recently changed this on x86 to not rely on TIF flags. IIRC the > problem is that on x86 you can change the mode of a task without the > kernel being aware of it. Is something like that possible on MIPS as > well? Hi all, In MIPS world it's impossible to raise a thread to 64bit without kernel aware. Without STATUS.UX set it will trigger reserved instruction exception when trying to run 64bit instructions. However it may be possible to run with 32bit ABI without TIF_32BIT_REGS if user program didn't get ELF ABI right. I think that's out of our current consideration. > The thing x86 does today is look at it's pt_regs state to determine the > actual state. It is possible to look at pt_regs Status.UX bit on MIPS. But it seems unnecessary as user can't change it. Thanks. - Jiaxun
On Tue, Jan 05, 2021 at 11:45:37AM +0800, Jiaxun Yang wrote: > 在 2021/1/4 下午6:59, Peter Zijlstra 写道: > > On Tue, Dec 29, 2020 at 08:55:59PM +0800, Tiezhu Yang wrote: > > > +u64 perf_reg_abi(struct task_struct *tsk) > > > +{ > > > + if (test_tsk_thread_flag(tsk, TIF_32BIT_REGS)) > > > + return PERF_SAMPLE_REGS_ABI_32; > > > + else > > > + return PERF_SAMPLE_REGS_ABI_64; > > > +} > > So we recently changed this on x86 to not rely on TIF flags. IIRC the > > problem is that on x86 you can change the mode of a task without the > > kernel being aware of it. Is something like that possible on MIPS as > > well? > > Hi all, > > In MIPS world it's impossible to raise a thread to 64bit without kernel > aware. > Without STATUS.UX set it will trigger reserved instruction exception when > trying > to run 64bit instructions. The other way around is the case on x86, a 64bit program can create and execute 32bit code sections without the kernel being aware. But if clearing STATUS.UX has the same issue as setting it, that should not be a problem for you. > However it may be possible to run with 32bit ABI without > TIF_32BIT_REGS if user program didn't get ELF ABI right. I think > that's out of our current consideration. Fair enough. > > The thing x86 does today is look at it's pt_regs state to determine the > > actual state. > It is possible to look at pt_regs Status.UX bit on MIPS. But it seems > unnecessary > as user can't change it. Ok, good. Then no objection, proceed! :-)
On Tue, Jan 05, 2021 at 11:18:06AM +0100, Peter Zijlstra wrote: > On Tue, Jan 05, 2021 at 11:45:37AM +0800, Jiaxun Yang wrote: > > 在 2021/1/4 下午6:59, Peter Zijlstra 写道: > > > On Tue, Dec 29, 2020 at 08:55:59PM +0800, Tiezhu Yang wrote: > > > > +u64 perf_reg_abi(struct task_struct *tsk) > > > > +{ > > > > + if (test_tsk_thread_flag(tsk, TIF_32BIT_REGS)) > > > > + return PERF_SAMPLE_REGS_ABI_32; > > > > + else > > > > + return PERF_SAMPLE_REGS_ABI_64; > > > > +} > > > So we recently changed this on x86 to not rely on TIF flags. IIRC the > > > problem is that on x86 you can change the mode of a task without the > > > kernel being aware of it. Is something like that possible on MIPS as > > > well? > > > > Hi all, > > > > In MIPS world it's impossible to raise a thread to 64bit without kernel > > aware. > > Without STATUS.UX set it will trigger reserved instruction exception when > > trying > > to run 64bit instructions. > > The other way around is the case on x86, a 64bit program can create and > execute 32bit code sections without the kernel being aware. But if > clearing STATUS.UX has the same issue as setting it, that should not be > a problem for you. > > > However it may be possible to run with 32bit ABI without > > TIF_32BIT_REGS if user program didn't get ELF ABI right. I think > > that's out of our current consideration. > > Fair enough. > > > > The thing x86 does today is look at it's pt_regs state to determine the > > > actual state. > > It is possible to look at pt_regs Status.UX bit on MIPS. But it seems > > unnecessary > > as user can't change it. > > Ok, good. Then no objection, proceed! :-) this patch aims more to mips-next, while patch 2 and 3 are targeting tools/perf. Should I take them into mips-next, too ? Thomas.
On 01/28/2021 05:15 AM, Thomas Bogendoerfer wrote: > On Tue, Jan 05, 2021 at 11:18:06AM +0100, Peter Zijlstra wrote: >> On Tue, Jan 05, 2021 at 11:45:37AM +0800, Jiaxun Yang wrote: >>> 在 2021/1/4 下午6:59, Peter Zijlstra 写道: >>>> On Tue, Dec 29, 2020 at 08:55:59PM +0800, Tiezhu Yang wrote: >>>>> +u64 perf_reg_abi(struct task_struct *tsk) >>>>> +{ >>>>> + if (test_tsk_thread_flag(tsk, TIF_32BIT_REGS)) >>>>> + return PERF_SAMPLE_REGS_ABI_32; >>>>> + else >>>>> + return PERF_SAMPLE_REGS_ABI_64; >>>>> +} >>>> So we recently changed this on x86 to not rely on TIF flags. IIRC the >>>> problem is that on x86 you can change the mode of a task without the >>>> kernel being aware of it. Is something like that possible on MIPS as >>>> well? >>> Hi all, >>> >>> In MIPS world it's impossible to raise a thread to 64bit without kernel >>> aware. >>> Without STATUS.UX set it will trigger reserved instruction exception when >>> trying >>> to run 64bit instructions. >> The other way around is the case on x86, a 64bit program can create and >> execute 32bit code sections without the kernel being aware. But if >> clearing STATUS.UX has the same issue as setting it, that should not be >> a problem for you. >> >>> However it may be possible to run with 32bit ABI without >>> TIF_32BIT_REGS if user program didn't get ELF ABI right. I think >>> that's out of our current consideration. >> Fair enough. >> >>>> The thing x86 does today is look at it's pt_regs state to determine the >>>> actual state. >>> It is possible to look at pt_regs Status.UX bit on MIPS. But it seems >>> unnecessary >>> as user can't change it. >> Ok, good. Then no objection, proceed! :-) > this patch aims more to mips-next, while patch 2 and 3 are targeting > tools/perf. Should I take them into mips-next, too ? If it is possible, I prefer to merge this three patches together through mips-next tree. Thanks, Tiezhu > > Thomas. >
Em Fri, Jan 29, 2021 at 10:48:52AM +0800, Tiezhu Yang escreveu: > On 01/28/2021 05:15 AM, Thomas Bogendoerfer wrote: > > On Tue, Jan 05, 2021 at 11:18:06AM +0100, Peter Zijlstra wrote: > > > On Tue, Jan 05, 2021 at 11:45:37AM +0800, Jiaxun Yang wrote: > > > > 在 2021/1/4 下午6:59, Peter Zijlstra 写道: > > > > > On Tue, Dec 29, 2020 at 08:55:59PM +0800, Tiezhu Yang wrote: > > > > > > +u64 perf_reg_abi(struct task_struct *tsk) > > > > > > +{ > > > > > > + if (test_tsk_thread_flag(tsk, TIF_32BIT_REGS)) > > > > > > + return PERF_SAMPLE_REGS_ABI_32; > > > > > > + else > > > > > > + return PERF_SAMPLE_REGS_ABI_64; > > > > > > +} > > > > > So we recently changed this on x86 to not rely on TIF flags. IIRC the > > > > > problem is that on x86 you can change the mode of a task without the > > > > > kernel being aware of it. Is something like that possible on MIPS as > > > > > well? > > > > Hi all, > > > > > > > > In MIPS world it's impossible to raise a thread to 64bit without kernel > > > > aware. > > > > Without STATUS.UX set it will trigger reserved instruction exception when > > > > trying > > > > to run 64bit instructions. > > > The other way around is the case on x86, a 64bit program can create and > > > execute 32bit code sections without the kernel being aware. But if > > > clearing STATUS.UX has the same issue as setting it, that should not be > > > a problem for you. > > > > > > > However it may be possible to run with 32bit ABI without > > > > TIF_32BIT_REGS if user program didn't get ELF ABI right. I think > > > > that's out of our current consideration. > > > Fair enough. > > > > > > > > The thing x86 does today is look at it's pt_regs state to determine the > > > > > actual state. > > > > It is possible to look at pt_regs Status.UX bit on MIPS. But it seems > > > > unnecessary > > > > as user can't change it. > > > Ok, good. Then no objection, proceed! :-) > > this patch aims more to mips-next, while patch 2 and 3 are targeting > > tools/perf. Should I take them into mips-next, too ? > > If it is possible, I prefer to merge this three patches together > through mips-next tree. The kernel part should go via the mips-next tree, the tooling I can process, that is how these things go in other cases where kernel and tooling changes for some new feature are needed. This helps making sure tooling is not in lockstep with the kernel, one should be able to use a new tool in an old kernel and vice-versa. - Arnaldo
Em Wed, Jan 27, 2021 at 10:15:06PM +0100, Thomas Bogendoerfer escreveu: > On Tue, Jan 05, 2021 at 11:18:06AM +0100, Peter Zijlstra wrote: > > On Tue, Jan 05, 2021 at 11:45:37AM +0800, Jiaxun Yang wrote: > > > 在 2021/1/4 下午6:59, Peter Zijlstra 写道: > > > > On Tue, Dec 29, 2020 at 08:55:59PM +0800, Tiezhu Yang wrote: > > > > > +u64 perf_reg_abi(struct task_struct *tsk) > > > > > +{ > > > > > + if (test_tsk_thread_flag(tsk, TIF_32BIT_REGS)) > > > > > + return PERF_SAMPLE_REGS_ABI_32; > > > > > + else > > > > > + return PERF_SAMPLE_REGS_ABI_64; > > > > > +} > > > > So we recently changed this on x86 to not rely on TIF flags. IIRC the > > > > problem is that on x86 you can change the mode of a task without the > > > > kernel being aware of it. Is something like that possible on MIPS as > > > > well? > > > > > > Hi all, > > > > > > In MIPS world it's impossible to raise a thread to 64bit without kernel > > > aware. > > > Without STATUS.UX set it will trigger reserved instruction exception when > > > trying > > > to run 64bit instructions. > > > > The other way around is the case on x86, a 64bit program can create and > > execute 32bit code sections without the kernel being aware. But if > > clearing STATUS.UX has the same issue as setting it, that should not be > > a problem for you. > > > > > However it may be possible to run with 32bit ABI without > > > TIF_32BIT_REGS if user program didn't get ELF ABI right. I think > > > that's out of our current consideration. > > > > Fair enough. > > > > > > The thing x86 does today is look at it's pt_regs state to determine the > > > > actual state. > > > It is possible to look at pt_regs Status.UX bit on MIPS. But it seems > > > unnecessary > > > as user can't change it. > > > > Ok, good. Then no objection, proceed! :-) > > this patch aims more to mips-next, while patch 2 and 3 are targeting > tools/perf. Should I take them into mips-next, too ? I'll process the tools/perf ones, if you took the time to actually review them, please say so and I'll add a Reviewed-by tag stating that. I've replied to another message in this thread with reasoning about the value of processing kernel bits in the relevant arch tree while the tooling one via my perf/core branch. - Arnaldo
On Tue, Dec 29, 2020 at 08:55:59PM +0800, Tiezhu Yang wrote: > +++ b/arch/mips/include/uapi/asm/perf_regs.h > @@ -0,0 +1,42 @@ > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ > +#ifndef _ASM_MIPS_PERF_REGS_H > +#define _ASM_MIPS_PERF_REGS_H > + > +enum perf_event_mips_regs { > + PERF_REG_MIPS_PC, > + PERF_REG_MIPS_R1, > + PERF_REG_MIPS_R2, > + PERF_REG_MIPS_R3, > + PERF_REG_MIPS_R4, > + PERF_REG_MIPS_R5, > + PERF_REG_MIPS_R6, > + PERF_REG_MIPS_R7, > + PERF_REG_MIPS_R8, > + PERF_REG_MIPS_R9, > + PERF_REG_MIPS_R10, > + PERF_REG_MIPS_R11, > + PERF_REG_MIPS_R12, > + PERF_REG_MIPS_R13, > + PERF_REG_MIPS_R14, > + PERF_REG_MIPS_R15, > + PERF_REG_MIPS_R16, > + PERF_REG_MIPS_R17, > + PERF_REG_MIPS_R18, > + PERF_REG_MIPS_R19, > + PERF_REG_MIPS_R20, > + PERF_REG_MIPS_R21, > + PERF_REG_MIPS_R22, > + PERF_REG_MIPS_R23, > + PERF_REG_MIPS_R24, > + PERF_REG_MIPS_R25, > + /* > + * 26 and 27 are k0 and k1, they are always clobbered thus not > + * stored. > + */ haveing this hole here make all code more complicated. Does it hurt to have R26 and R27 in the list ? Thomas.
On 02/01/2021 06:43 PM, Thomas Bogendoerfer wrote: > On Tue, Dec 29, 2020 at 08:55:59PM +0800, Tiezhu Yang wrote: >> +++ b/arch/mips/include/uapi/asm/perf_regs.h >> @@ -0,0 +1,42 @@ >> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ >> +#ifndef _ASM_MIPS_PERF_REGS_H >> +#define _ASM_MIPS_PERF_REGS_H >> + >> +enum perf_event_mips_regs { >> + PERF_REG_MIPS_PC, >> + PERF_REG_MIPS_R1, >> + PERF_REG_MIPS_R2, >> + PERF_REG_MIPS_R3, >> + PERF_REG_MIPS_R4, >> + PERF_REG_MIPS_R5, >> + PERF_REG_MIPS_R6, >> + PERF_REG_MIPS_R7, >> + PERF_REG_MIPS_R8, >> + PERF_REG_MIPS_R9, >> + PERF_REG_MIPS_R10, >> + PERF_REG_MIPS_R11, >> + PERF_REG_MIPS_R12, >> + PERF_REG_MIPS_R13, >> + PERF_REG_MIPS_R14, >> + PERF_REG_MIPS_R15, >> + PERF_REG_MIPS_R16, >> + PERF_REG_MIPS_R17, >> + PERF_REG_MIPS_R18, >> + PERF_REG_MIPS_R19, >> + PERF_REG_MIPS_R20, >> + PERF_REG_MIPS_R21, >> + PERF_REG_MIPS_R22, >> + PERF_REG_MIPS_R23, >> + PERF_REG_MIPS_R24, >> + PERF_REG_MIPS_R25, >> + /* >> + * 26 and 27 are k0 and k1, they are always clobbered thus not >> + * stored. >> + */ > haveing this hole here make all code more complicated. Does it hurt > to have R26 and R27 in the list ? I think there is no effect if have R26 and R27 in the list. In the perf_reg_value(), PERF_REG_MIPS_R{26,27} are default case. +u64 perf_reg_value(struct pt_regs *regs, int idx) +{ + long v; + + switch (idx) { + case PERF_REG_MIPS_PC: + v = regs->cp0_epc; + break; + case PERF_REG_MIPS_R1 ... PERF_REG_MIPS_R25: + v = regs->regs[idx - PERF_REG_MIPS_R1 + 1]; + break; + case PERF_REG_MIPS_R28 ... PERF_REG_MIPS_R31: + v = regs->regs[idx - PERF_REG_MIPS_R28 + 28]; + break; + + default: + WARN_ON_ONCE(1); + return 0; + } + + return (s64)v; /* Sign extend if 32-bit. */ +} Should I modify enum perf_event_mips_regs to add R26 and R27, and then send v2? Thanks, Tiezhu > > Thomas. >
On Mon, Feb 01, 2021 at 08:56:06PM +0800, Tiezhu Yang wrote: > On 02/01/2021 06:43 PM, Thomas Bogendoerfer wrote: > > On Tue, Dec 29, 2020 at 08:55:59PM +0800, Tiezhu Yang wrote: > > > +++ b/arch/mips/include/uapi/asm/perf_regs.h > > > @@ -0,0 +1,42 @@ > > > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ > > > +#ifndef _ASM_MIPS_PERF_REGS_H > > > +#define _ASM_MIPS_PERF_REGS_H > > > + > > > +enum perf_event_mips_regs { > > > + PERF_REG_MIPS_PC, > > > + PERF_REG_MIPS_R1, > > > + PERF_REG_MIPS_R2, > > > + PERF_REG_MIPS_R3, > > > + PERF_REG_MIPS_R4, > > > + PERF_REG_MIPS_R5, > > > + PERF_REG_MIPS_R6, > > > + PERF_REG_MIPS_R7, > > > + PERF_REG_MIPS_R8, > > > + PERF_REG_MIPS_R9, > > > + PERF_REG_MIPS_R10, > > > + PERF_REG_MIPS_R11, > > > + PERF_REG_MIPS_R12, > > > + PERF_REG_MIPS_R13, > > > + PERF_REG_MIPS_R14, > > > + PERF_REG_MIPS_R15, > > > + PERF_REG_MIPS_R16, > > > + PERF_REG_MIPS_R17, > > > + PERF_REG_MIPS_R18, > > > + PERF_REG_MIPS_R19, > > > + PERF_REG_MIPS_R20, > > > + PERF_REG_MIPS_R21, > > > + PERF_REG_MIPS_R22, > > > + PERF_REG_MIPS_R23, > > > + PERF_REG_MIPS_R24, > > > + PERF_REG_MIPS_R25, > > > + /* > > > + * 26 and 27 are k0 and k1, they are always clobbered thus not > > > + * stored. > > > + */ > > haveing this hole here make all code more complicated. Does it hurt > > to have R26 and R27 in the list ? > > I think there is no effect if have R26 and R27 in the list. > > In the perf_reg_value(), PERF_REG_MIPS_R{26,27} are default case. why make them special ? After all they are real registers and are only defined special by current ABIs. > Should I modify enum perf_event_mips_regs to add R26 and R27, > and then send v2? yes please. Thomas.
On 2/3/21 6:40 PM, Thomas Bogendoerfer wrote: > On Mon, Feb 01, 2021 at 08:56:06PM +0800, Tiezhu Yang wrote: >> On 02/01/2021 06:43 PM, Thomas Bogendoerfer wrote: >>> On Tue, Dec 29, 2020 at 08:55:59PM +0800, Tiezhu Yang wrote: >>>> +++ b/arch/mips/include/uapi/asm/perf_regs.h >>>> @@ -0,0 +1,42 @@ >>>> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ >>>> +#ifndef _ASM_MIPS_PERF_REGS_H >>>> +#define _ASM_MIPS_PERF_REGS_H >>>> + >>>> +enum perf_event_mips_regs { >>>> + PERF_REG_MIPS_PC, >>>> + PERF_REG_MIPS_R1, >>>> + PERF_REG_MIPS_R2, >>>> + PERF_REG_MIPS_R3, >>>> + PERF_REG_MIPS_R4, >>>> + PERF_REG_MIPS_R5, >>>> + PERF_REG_MIPS_R6, >>>> + PERF_REG_MIPS_R7, >>>> + PERF_REG_MIPS_R8, >>>> + PERF_REG_MIPS_R9, >>>> + PERF_REG_MIPS_R10, >>>> + PERF_REG_MIPS_R11, >>>> + PERF_REG_MIPS_R12, >>>> + PERF_REG_MIPS_R13, >>>> + PERF_REG_MIPS_R14, >>>> + PERF_REG_MIPS_R15, >>>> + PERF_REG_MIPS_R16, >>>> + PERF_REG_MIPS_R17, >>>> + PERF_REG_MIPS_R18, >>>> + PERF_REG_MIPS_R19, >>>> + PERF_REG_MIPS_R20, >>>> + PERF_REG_MIPS_R21, >>>> + PERF_REG_MIPS_R22, >>>> + PERF_REG_MIPS_R23, >>>> + PERF_REG_MIPS_R24, >>>> + PERF_REG_MIPS_R25, >>>> + /* >>>> + * 26 and 27 are k0 and k1, they are always clobbered thus not >>>> + * stored. >>>> + */ >>> haveing this hole here make all code more complicated. Does it hurt >>> to have R26 and R27 in the list ? >> I think there is no effect if have R26 and R27 in the list. >> >> In the perf_reg_value(), PERF_REG_MIPS_R{26,27} are default case. > why make them special ? After all they are real registers and are only > defined special by current ABIs. By convention, $26 and $27 are k registers which are reserved for use by the OS kernel. Here is an explanation [1]: "An interrupt handler must save any general - purpose registers that it is going to use (to be restored at return). But to do so requires you to modify at least one register first (something like sw $t0, saved_t0 expands to two machine instructions using $at). This situation is resolved by forbidding user programs from using two general - purpose registers, $k0 and $k1 (The k stands for kernel, which an exception handler is part of). The interrupt handler is allowed to use $k0 and $k1 without having to save or restore their values. This allows just enough leeway to start saving registers, as well as making returning from the interrupt handler possible." [1] https://stackoverflow.com/questions/27922315/how-to-use-mips-k0-and-k1-registers > >> Should I modify enum perf_event_mips_regs to add R26 and R27, >> and then send v2? > yes please. > > Thomas. >
On Wed, Feb 03, 2021 at 09:12:28PM +0800, Tiezhu Yang wrote: > On 2/3/21 6:40 PM, Thomas Bogendoerfer wrote: > > On Mon, Feb 01, 2021 at 08:56:06PM +0800, Tiezhu Yang wrote: > > > On 02/01/2021 06:43 PM, Thomas Bogendoerfer wrote: > > > > On Tue, Dec 29, 2020 at 08:55:59PM +0800, Tiezhu Yang wrote: > > > > > +++ b/arch/mips/include/uapi/asm/perf_regs.h > > > > > @@ -0,0 +1,42 @@ > > > > > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ > > > > > +#ifndef _ASM_MIPS_PERF_REGS_H > > > > > +#define _ASM_MIPS_PERF_REGS_H > > > > > + > > > > > +enum perf_event_mips_regs { > > > > > + PERF_REG_MIPS_PC, > > > > > + PERF_REG_MIPS_R1, > > > > > + PERF_REG_MIPS_R2, > > > > > + PERF_REG_MIPS_R3, > > > > > + PERF_REG_MIPS_R4, > > > > > + PERF_REG_MIPS_R5, > > > > > + PERF_REG_MIPS_R6, > > > > > + PERF_REG_MIPS_R7, > > > > > + PERF_REG_MIPS_R8, > > > > > + PERF_REG_MIPS_R9, > > > > > + PERF_REG_MIPS_R10, > > > > > + PERF_REG_MIPS_R11, > > > > > + PERF_REG_MIPS_R12, > > > > > + PERF_REG_MIPS_R13, > > > > > + PERF_REG_MIPS_R14, > > > > > + PERF_REG_MIPS_R15, > > > > > + PERF_REG_MIPS_R16, > > > > > + PERF_REG_MIPS_R17, > > > > > + PERF_REG_MIPS_R18, > > > > > + PERF_REG_MIPS_R19, > > > > > + PERF_REG_MIPS_R20, > > > > > + PERF_REG_MIPS_R21, > > > > > + PERF_REG_MIPS_R22, > > > > > + PERF_REG_MIPS_R23, > > > > > + PERF_REG_MIPS_R24, > > > > > + PERF_REG_MIPS_R25, > > > > > + /* > > > > > + * 26 and 27 are k0 and k1, they are always clobbered thus not > > > > > + * stored. > > > > > + */ > > > > haveing this hole here make all code more complicated. Does it hurt > > > > to have R26 and R27 in the list ? > > > I think there is no effect if have R26 and R27 in the list. > > > > > > In the perf_reg_value(), PERF_REG_MIPS_R{26,27} are default case. > > why make them special ? After all they are real registers and are only > > defined special by current ABIs. > > > By convention, $26 and $27 are k registers which are reserved for use > by the OS kernel. believe me, I knew that already. But from a CPU standpoint they are just registers. Anyway I'm fine with just adding R26 and R27 to the enum. Thomas.
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig index 0a17bed..092c876 100644 --- a/arch/mips/Kconfig +++ b/arch/mips/Kconfig @@ -76,6 +76,8 @@ config MIPS select HAVE_NMI select HAVE_OPROFILE select HAVE_PERF_EVENTS + select HAVE_PERF_REGS + select HAVE_PERF_USER_STACK_DUMP select HAVE_REGS_AND_STACK_ACCESS_API select HAVE_RSEQ select HAVE_SPARSE_SYSCALL_NR diff --git a/arch/mips/include/uapi/asm/perf_regs.h b/arch/mips/include/uapi/asm/perf_regs.h new file mode 100644 index 0000000..f3cef08 --- /dev/null +++ b/arch/mips/include/uapi/asm/perf_regs.h @@ -0,0 +1,42 @@ +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ +#ifndef _ASM_MIPS_PERF_REGS_H +#define _ASM_MIPS_PERF_REGS_H + +enum perf_event_mips_regs { + PERF_REG_MIPS_PC, + PERF_REG_MIPS_R1, + PERF_REG_MIPS_R2, + PERF_REG_MIPS_R3, + PERF_REG_MIPS_R4, + PERF_REG_MIPS_R5, + PERF_REG_MIPS_R6, + PERF_REG_MIPS_R7, + PERF_REG_MIPS_R8, + PERF_REG_MIPS_R9, + PERF_REG_MIPS_R10, + PERF_REG_MIPS_R11, + PERF_REG_MIPS_R12, + PERF_REG_MIPS_R13, + PERF_REG_MIPS_R14, + PERF_REG_MIPS_R15, + PERF_REG_MIPS_R16, + PERF_REG_MIPS_R17, + PERF_REG_MIPS_R18, + PERF_REG_MIPS_R19, + PERF_REG_MIPS_R20, + PERF_REG_MIPS_R21, + PERF_REG_MIPS_R22, + PERF_REG_MIPS_R23, + PERF_REG_MIPS_R24, + PERF_REG_MIPS_R25, + /* + * 26 and 27 are k0 and k1, they are always clobbered thus not + * stored. + */ + PERF_REG_MIPS_R28, + PERF_REG_MIPS_R29, + PERF_REG_MIPS_R30, + PERF_REG_MIPS_R31, + PERF_REG_MIPS_MAX = PERF_REG_MIPS_R31 + 1, +}; +#endif /* _ASM_MIPS_PERF_REGS_H */ diff --git a/arch/mips/kernel/Makefile b/arch/mips/kernel/Makefile index 2a05b92..120075a 100644 --- a/arch/mips/kernel/Makefile +++ b/arch/mips/kernel/Makefile @@ -104,7 +104,7 @@ obj-$(CONFIG_MIPSR2_TO_R6_EMULATOR) += mips-r2-to-r6-emul.o CFLAGS_cpu-bugs64.o = $(shell if $(CC) $(KBUILD_CFLAGS) -Wa,-mdaddi -c -o /dev/null -x c /dev/null >/dev/null 2>&1; then echo "-DHAVE_AS_SET_DADDI"; fi) -obj-$(CONFIG_PERF_EVENTS) += perf_event.o +obj-$(CONFIG_PERF_EVENTS) += perf_event.o perf_regs.o obj-$(CONFIG_HW_PERF_EVENTS) += perf_event_mipsxx.o obj-$(CONFIG_JUMP_LABEL) += jump_label.o diff --git a/arch/mips/kernel/perf_regs.c b/arch/mips/kernel/perf_regs.c new file mode 100644 index 0000000..e686780 --- /dev/null +++ b/arch/mips/kernel/perf_regs.c @@ -0,0 +1,68 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * This file is subject to the terms and conditions of the GNU General Public + * License. See the file "COPYING" in the main directory of this archive + * for more details. + * + * Some parts derived from x86 version of this file. + * + * Copyright (C) 2013 Cavium, Inc. + */ + +#include <linux/perf_event.h> + +#include <asm/ptrace.h> + +#ifdef CONFIG_32BIT +u64 perf_reg_abi(struct task_struct *tsk) +{ + return PERF_SAMPLE_REGS_ABI_32; +} +#else /* Must be CONFIG_64BIT */ +u64 perf_reg_abi(struct task_struct *tsk) +{ + if (test_tsk_thread_flag(tsk, TIF_32BIT_REGS)) + return PERF_SAMPLE_REGS_ABI_32; + else + return PERF_SAMPLE_REGS_ABI_64; +} +#endif /* CONFIG_32BIT */ + +int perf_reg_validate(u64 mask) +{ + if (!mask) + return -EINVAL; + if (mask & ~((1ull << PERF_REG_MIPS_MAX) - 1)) + return -EINVAL; + return 0; +} + +u64 perf_reg_value(struct pt_regs *regs, int idx) +{ + long v; + + switch (idx) { + case PERF_REG_MIPS_PC: + v = regs->cp0_epc; + break; + case PERF_REG_MIPS_R1 ... PERF_REG_MIPS_R25: + v = regs->regs[idx - PERF_REG_MIPS_R1 + 1]; + break; + case PERF_REG_MIPS_R28 ... PERF_REG_MIPS_R31: + v = regs->regs[idx - PERF_REG_MIPS_R28 + 28]; + break; + + default: + WARN_ON_ONCE(1); + return 0; + } + + return (s64)v; /* Sign extend if 32-bit. */ +} + +void perf_get_regs_user(struct perf_regs *regs_user, + struct pt_regs *regs) +{ + regs_user->regs = task_pt_regs(current); + regs_user->abi = perf_reg_abi(current); +}
Add perf_event_mips_regs/perf_reg_value/perf_reg_validate to support features HAVE_PERF_REGS/HAVE_PERF_USER_STACK_DUMP in kernel. [ayan@wavecomp.com: Repick this patch for unwinding userstack backtrace by perf and libunwind on MIPS based CPU.] [ralf@linux-mips.org: Add perf_get_regs_user() which is required after 'commit 88a7c26af8da ("perf: Move task_pt_regs sampling into arch code")'.] [yangtiezhu@loongson.cn: Fix build error about perf_get_regs_user() after commit 76a4efa80900 ("perf/arch: Remove perf_sample_data::regs_user_copy"), and also separate the original patches into two parts (MIPS kernel and perf tools) to merge easily.] The original patches: https://lore.kernel.org/patchwork/patch/1126521/ https://lore.kernel.org/patchwork/patch/1126520/ Signed-off-by: David Daney <david.daney@cavium.com> Signed-off-by: Ralf Baechle <ralf@linux-mips.org> Signed-off-by: Archer Yan <ayan@wavecomp.com> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn> --- arch/mips/Kconfig | 2 + arch/mips/include/uapi/asm/perf_regs.h | 42 +++++++++++++++++++++ arch/mips/kernel/Makefile | 2 +- arch/mips/kernel/perf_regs.c | 68 ++++++++++++++++++++++++++++++++++ 4 files changed, 113 insertions(+), 1 deletion(-) create mode 100644 arch/mips/include/uapi/asm/perf_regs.h create mode 100644 arch/mips/kernel/perf_regs.c