Re: [PATCH v4 04/16] ARM: move generic thumb instruction parsingcode to new files for use by other feature

[David Long]

On 12/20/13 07:46, Jon Medhurst (Tixy) wrote:
> On Sun, 2013-12-15 at 23:08 -0500, David Long wrote:
> > From: "David A. Long" <dave.long@xxxxxxxxxx>
> >
> > Move the thumb version of the kprobes instruction parsing code into more generic
> > files from where it can be used by uprobes and possibly other subsystems. The
> > symbol names will be made more generic in a subsequent part of this patchset.
> >
> > Signed-off-by: David A. Long <dave.long@xxxxxxxxxx>
> > ---
>
> I just have two comments about probes-thumb.h ...
>
> [...]
>
> > diff --git a/arch/arm/kernel/probes-thumb.h b/arch/arm/kernel/probes-thumb.h
> > new file mode 100644
> > index 0000000..3f39210
> > --- /dev/null
> > +++ b/arch/arm/kernel/probes-thumb.h
> > @@ -0,0 +1,136 @@
> > +/*
> > + * arch/arm/kernel/probes-arm.h
> > + *
> > + * Copyright 2013 Linaro Ltd.
> > + * Written by: David A. Long
> > + *
> > + * The code contained herein is licensed under the GNU General Public
> > + * License. You may obtain a copy of the GNU General Public License
> > + * Version 2 or later at the following locations:
> > + *
> > + * http://www.opensource.org/licenses/gpl-license.html
> > + * http://www.gnu.org/copyleft/gpl.html
> > + */
> > +
> > +#ifndef _ARM_KERNEL_PROBES_THUMB_H
> > +#define  _ARM_KERNEL_PROBES_THUMB_H
> > +
> > +/*
> > + * True if current instruction is in an IT block.
> > + */
> > +#define in_it_block(cpsr)	((cpsr & 0x06000c00) != 0x00000000)
> > +
> > +/*
> > + * Return the condition code to check for the currently executing instruction.
> > + * This is in ITSTATE<7:4> which is in CPSR<15:12> but is only valid if
> > + * in_it_block returns true.
> > + */
> > +#define current_cond(cpsr)	((cpsr >> 12) & 0xf)
>
> Looks like you forgot to remove above two #defines from kprobes-thumb.c
> when you moved them to this header file.

Fixed.

> Also...
>
> > +enum probes_t32_action {
> > +	PROBES_T32_EMULATE_NONE,
> > +	PROBES_T32_SIMULATE_NOP,
> > +	PROBES_T32_LDMSTM,
> > +	PROBES_T32_LDRDSTRD,
> > +	PROBES_T32_TABLE_BRANCH,
> > +	PROBES_T32_TST,
> > +	PROBES_T32_CMP,
> > +	PROBES_T32_MOV,
> > +	PROBES_T32_ADDSUB,
> > +	PROBES_T32_LOGICAL,
> > +	PROBES_T32_ADDWSUBW_PC,
> > +	PROBES_T32_ADDWSUBW,
> > +	PROBES_T32_MOVW,
> > +	PROBES_T32_SAT,
> > +	PROBES_T32_BITFIELD,
> > +	PROBES_T32_SEV,
> > +	PROBES_T32_WFE,
> > +	PROBES_T32_MRS,
> > +	PROBES_T32_BRANCH_COND,
> > +	PROBES_T32_BRANCH,
> > +	PROBES_T32_PLDI,
> > +	PROBES_T32_LDR_LIT,
> > +	PROBES_T32_LDRSTR,
> > +	PROBES_T32_SIGN_EXTEND,
> > +	PROBES_T32_MEDIA,
> > +	PROBES_T32_REVERSE,
> > +	PROBES_T32_MUL_ADD,
> > +	PROBES_T32_MUL_ADD2,
> > +	PROBES_T32_MUL_ADD_LONG
> > +};
> > +
> > +enum probes_t16_action {
> > +	PROBES_T16_ADD_SP,
> > +	PROBES_T16_CBZ,
> > +	PROBES_T16_SIGN_EXTEND,
> > +	PROBES_T16_PUSH,
> > +	PROBES_T16_POP,
> > +	PROBES_T16_SEV,
> > +	PROBES_T16_WFE,
> > +	PROBES_T16_IT,
> > +	PROBES_T16_CMP,
> > +	PROBES_T16_ADDSUB,
> > +	PROBES_T16_LOGICAL,
> > +	PROBES_T16_BLX,
> > +	PROBES_T16_HIREGOPS,
> > +	PROBES_T16_LDR_LIT,
> > +	PROBES_T16_LDRHSTRH,
> > +	PROBES_T16_LDRSTR,
> > +	PROBES_T16_ADR,
> > +	PROBES_T16_LDMSTM,
> > +	PROBES_T16_BRANCH_COND,
> > +	PROBES_T16_BRANCH
> > +};
> > +

Moved.

-dl


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/