| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 22 May 2024 19:36:38 +0530
From: Athira Rajeev <atrajeev@...ux.vnet.ibm.com>
To: Christophe Leroy <christophe.leroy@...roup.eu>
Cc: "acme@...nel.org" <acme@...nel.org>, "jolsa@...nel.org" <jolsa@...nel.org>,
"adrian.hunter@...el.com" <adrian.hunter@...el.com>,
"irogers@...gle.com" <irogers@...gle.com>,
"namhyung@...nel.org" <namhyung@...nel.org>,
"segher@...nel.crashing.org" <segher@...nel.crashing.org>,
"linux-perf-users@...r.kernel.org" <linux-perf-users@...r.kernel.org>,
"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
"maddy@...ux.ibm.com" <maddy@...ux.ibm.com>,
"kjain@...ux.ibm.com" <kjain@...ux.ibm.com>,
"disgoel@...ux.vnet.ibm.com" <disgoel@...ux.vnet.ibm.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"akanksha@...ux.ibm.com" <akanksha@...ux.ibm.com>
Subject: Re: [PATCH V2 5/9] tools/perf: Update parameters for reg extract
functions to use raw instruction on powerpc
> On 7 May 2024, at 3:18 PM, Christophe Leroy <christophe.leroy@...roup.eu> wrote:
>
>
>
> Le 06/05/2024 à 14:19, Athira Rajeev a écrit :
>> Use the raw instruction code and macros to identify memory instructions,
>> extract register fields and also offset. The implementation addresses
>> the D-form, X-form, DS-form instructions. Two main functions are added.
>> New parse function "load_store__parse" as instruction ops parser for
>> memory instructions. Unlink other parser (like mov__parse), this parser
>> fills in only the "raw" field for source/target and new added "mem_ref"
>> field. This is because, here there is no need to parse the disassembled
>> code and arch specific macros will take care of extracting offset and
>> regs which is easier and will be precise.
>>
>> In powerpc, all instructions with a primary opcode from 32 to 63
>> are memory instructions. Update "ins__find" function to have "opcode"
>> also as a parameter. Don't use the "extract_reg_offset", instead use
>> newly added function "get_arch_regs" which will set these fields: reg1,
>> reg2, offset depending of where it is source or target ops.
>
> Yes all instructions with a primary opcode from 32 to 63 are memory
> instructions, but not all memory instructions have opcode 32 to 63.
>
>>
>> Signed-off-by: Athira Rajeev <atrajeev@...ux.vnet.ibm.com>
>> ---
>> tools/perf/arch/powerpc/util/dwarf-regs.c | 33 +++++++++++++
>> tools/perf/util/annotate.c | 22 ++++++++-
>> tools/perf/util/disasm.c | 59 +++++++++++++++++++++--
>> tools/perf/util/disasm.h | 4 +-
>> tools/perf/util/include/dwarf-regs.h | 4 +-
>> 5 files changed, 114 insertions(+), 8 deletions(-)
>>
>> diff --git a/tools/perf/arch/powerpc/util/dwarf-regs.c b/tools/perf/arch/powerpc/util/dwarf-regs.c
>> index e60a71fd846e..3121c70dc0d3 100644
>> --- a/tools/perf/arch/powerpc/util/dwarf-regs.c
>> +++ b/tools/perf/arch/powerpc/util/dwarf-regs.c
>> @@ -102,6 +102,9 @@ int regs_query_register_offset(const char *name)
>> #define PPC_OP(op) (((op) >> 26) & 0x3F)
>> #define PPC_RA(a) (((a) >> 16) & 0x1f)
>> #define PPC_RT(t) (((t) >> 21) & 0x1f)
>> +#define PPC_RB(b) (((b) >> 11) & 0x1f)
>> +#define PPC_D(D) ((D) & 0xfffe)
>> +#define PPC_DS(DS) ((DS) & 0xfffc)
>>
>> int get_opcode_insn(unsigned int raw_insn)
>> {
>> @@ -117,3 +120,33 @@ int get_target_reg(unsigned int raw_insn)
>> {
>> return PPC_RT(raw_insn);
>> }
>> +
>> +int get_offset_opcode(int raw_insn __maybe_unused)
>> +{
>> + int opcode = PPC_OP(raw_insn);
>> +
>> + /* DS- form */
>> + if ((opcode == 58) || (opcode == 62))
>
> Can you re-use macros from arch/powerpc/include/asm/ppc-opcode.h ?
>
> #define OP_LD 58
> #define OP_STD 62
Hi Christophe
Sure, I will make these changes
>
>
>> + return PPC_DS(raw_insn);
>> + else
>> + return PPC_D(raw_insn);
>> +}
>> +
>> +/*
>> + * Fills the required fields for op_loc depending on if it
>> + * is a source of target.
>> + * D form: ins RT,D(RA) -> src_reg1 = RA, offset = D, dst_reg1 = RT
>> + * DS form: ins RT,DS(RA) -> src_reg1 = RA, offset = DS, dst_reg1 = RT
>> + * X form: ins RT,RA,RB -> src_reg1 = RA, src_reg2 = RB, dst_reg1 = RT
>> + */
>> +void get_arch_regs(int raw_insn __maybe_unused, int is_source __maybe_unused, struct annotated_op_loc *op_loc __maybe_unused)
>> +{
>> + if (is_source)
>> + op_loc->reg1 = get_source_reg(raw_insn);
>> + else
>> + op_loc->reg1 = get_target_reg(raw_insn);
>> + if (op_loc->multi_regs)
>> + op_loc->reg2 = PPC_RB(raw_insn);
>> + if (op_loc->mem_ref)
>> + op_loc->offset = get_offset_opcode(raw_insn);
>> +}
>
>> diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
>> index 85692f73e78f..f41a0fadeab4 100644
>> --- a/tools/perf/util/disasm.c
>> +++ b/tools/perf/util/disasm.c
>> @@ -760,11 +800,20 @@ static void ins__sort(struct arch *arch)
>> qsort(arch->instructions, nmemb, sizeof(struct ins), ins__cmp);
>> }
>>
>> -static struct ins_ops *__ins__find(struct arch *arch, const char *name)
>> +static struct ins_ops *__ins__find(struct arch *arch, const char *name, int opcode)
>> {
>> struct ins *ins;
>> const int nmemb = arch->nr_instructions;
>>
>> + if (arch__is(arch, "powerpc")) {
>> + /*
>> + * Instructions with a primary opcode from 32 to 63
>> + * are memory instructions in powerpc.
>> + */
>> + if ((opcode >= 31) && (opcode <= 63))
>
> Could just be if ((opcode & 0x20)) I guess.
Ok,,
>
> By the way your test is wrong because opcode 31 is not only memory
> instructions, see example below (not exhaustive):
>
> #define OP_31_XOP_TRAP 4 ==> No
> #define OP_31_XOP_LDX 21 ==> Yes
> #define OP_31_XOP_LWZX 23 ==> Yes
> #define OP_31_XOP_LDUX 53 ==> Yes
> #define OP_31_XOP_DCBST 54 ==> No
> #define OP_31_XOP_LWZUX 55 ==> Yes
> #define OP_31_XOP_TRAP_64 68 ==> No
> #define OP_31_XOP_DCBF 86 ==> No
> #define OP_31_XOP_LBZX 87 ==> Yes
> #define OP_31_XOP_STDX 149 ==> Yes
> #define OP_31_XOP_STWX 151 ==> Yes
Thanks for pointing this. I checked through others in this list in arch/powerpc/include/asm/ppc-opcode.h
I will have these filtered in V3
Thanks
Athira
>
>
>
>
>> + return &load_store_ops;
>> + }
>> +
>> if (!arch->sorted_instructions) {
>> ins__sort(arch);
>> arch->sorted_instructions = true;
Powered by blists - more mailing lists