Re: [RFC PATCH v1 3/3] powerpc: WIP draft support to objtool check

2023-06-16 Thread Peter Zijlstra


Few comments..

On Fri, Jun 16, 2023 at 03:47:52PM +0200, Christophe Leroy wrote:

> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index 0fcf99c91400..f945fe271706 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -236,6 +236,7 @@ static bool __dead_end_function(struct objtool_file 
> *file, struct symbol *func,
>   "x86_64_start_reservations",
>   "xen_cpu_bringup_again",
>   "xen_start_kernel",
> + "longjmp",
>   };
>  
>   if (!func)
> @@ -2060,13 +2061,12 @@ static int add_jump_table(struct objtool_file *file, 
> struct instruction *insn,
>* instruction.
>*/
>   list_for_each_entry_from(reloc, >sec->reloc_list, list) {
> -
>   /* Check for the end of the table: */
>   if (reloc != table && reloc->jump_table_start)
>   break;
>  
>   /* Make sure the table entries are consecutive: */
> - if (prev_offset && reloc->offset != prev_offset + 8)
> + if (prev_offset && reloc->offset != prev_offset + 4)

Do we want a global variable (from elf.c) called elf_sizeof_long or so?

>   break;
>  
>   /* Detect function pointers from contiguous objects: */
> @@ -2074,7 +2074,10 @@ static int add_jump_table(struct objtool_file *file, 
> struct instruction *insn,
>   reloc->addend == pfunc->offset)
>   break;
>  
> - dest_insn = find_insn(file, reloc->sym->sec, reloc->addend);
> + if (table->jump_table_is_rel)
> + dest_insn = find_insn(file, reloc->sym->sec, 
> reloc->addend + table->offset - reloc->offset);
> + else
> + dest_insn = find_insn(file, reloc->sym->sec, 
> reloc->addend);

offset = reloc->addend;
if (table->jump_table_is_rel)
offset += table->offset - reloc->offset;
dest_insn = find_insn(file, reloc->sym->sec, offset);

perhaps?

>   if (!dest_insn)
>   break;
>  
> @@ -4024,6 +4022,11 @@ static bool ignore_unreachable_insn(struct 
> objtool_file *file, struct instructio
>   if (insn->ignore || insn->type == INSN_NOP || insn->type == INSN_TRAP)
>   return true;
>  
> + /* powerpc relocatable files have a word in front of each relocatable 
> function */
> + if ((file->elf->ehdr.e_machine == EM_PPC || file->elf->ehdr.e_machine 
> == EM_PPC64) &&
> + (file->elf->ehdr.e_flags & EF_PPC_RELOCATABLE_LIB) &&
> + insn_func(next_insn_same_sec(file, insn)))
> + return true;

Can't you simply decode that word to INSN_NOP or so?


[RFC PATCH v1 3/3] powerpc: WIP draft support to objtool check

2023-06-16 Thread Christophe Leroy
This draft messy patch is first try to add support of objtool
check for powerpc. This is in preparation of doing uaccess
validation for powerpc.

For the time being, this is implemented for PPC32 only breaking
support for other targets eventually. Will be reworked to be more
generic once a final working status has been achieved.

All assembly files have been deactivated as they require huge
work and are not really needed at the first place for uaccess validation.

Signed-off-by: Christophe Leroy 
---
 arch/powerpc/Kconfig  |  1 +
 scripts/Makefile.lib  |  2 +-
 tools/objtool/arch/powerpc/decode.c   | 60 +--
 .../arch/powerpc/include/arch/special.h   |  2 +-
 tools/objtool/arch/powerpc/special.c  | 44 +-
 tools/objtool/check.c | 29 +
 tools/objtool/include/objtool/elf.h   |  1 +
 tools/objtool/include/objtool/special.h   |  2 +-
 8 files changed, 118 insertions(+), 23 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 542be1c3c315..3bd244784af1 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -259,6 +259,7 @@ config PPC
select HAVE_OPTPROBES
select HAVE_OBJTOOL if PPC32 || MPROFILE_KERNEL
select HAVE_OBJTOOL_MCOUNT  if HAVE_OBJTOOL
+   select HAVE_UACCESS_VALIDATION  if HAVE_OBJTOOL
select HAVE_PERF_EVENTS
select HAVE_PERF_EVENTS_NMI if PPC64
select HAVE_PERF_REGS
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 100a386fcd71..298e2656e911 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -267,7 +267,7 @@ objtool-args-$(CONFIG_RETHUNK)  
+= --rethunk
 objtool-args-$(CONFIG_SLS) += --sls
 objtool-args-$(CONFIG_STACK_VALIDATION)+= --stackval
 objtool-args-$(CONFIG_HAVE_STATIC_CALL_INLINE) += --static-call
-objtool-args-$(CONFIG_HAVE_UACCESS_VALIDATION) += --uaccess
+objtool-args-$(CONFIG_HAVE_UACCESS_VALIDATION) += --uaccess 
--sec-address
 objtool-args-$(CONFIG_GCOV_KERNEL) += --no-unreachable
 objtool-args-$(CONFIG_PREFIX_SYMBOLS)  += 
--prefix=$(CONFIG_FUNCTION_PADDING_BYTES)
 
diff --git a/tools/objtool/arch/powerpc/decode.c 
b/tools/objtool/arch/powerpc/decode.c
index 53b55690f320..e95c0470e34b 100644
--- a/tools/objtool/arch/powerpc/decode.c
+++ b/tools/objtool/arch/powerpc/decode.c
@@ -43,24 +43,72 @@ int arch_decode_instruction(struct objtool_file *file, 
const struct section *sec
unsigned long offset, unsigned int maxlen,
struct instruction *insn)
 {
-   unsigned int opcode;
+   unsigned int opcode, xop;
+   unsigned int rs, ra, rb, bo, bi, to, uimm, l;
enum insn_type typ;
unsigned long imm;
u32 ins;
 
ins = bswap_if_needed(file->elf, *(u32 *)(sec->data->d_buf + offset));
opcode = ins >> 26;
-   typ = INSN_OTHER;
-   imm = 0;
+   xop = (ins >> 1) & 0x3ff;
+   rs = bo = to = (ins >> 21) & 0x1f;
+   ra = bi = (ins >> 16) & 0x1f;
+   rb = (ins >> 11) & 0x1f;
+   uimm = (ins >> 0) & 0x;
+   l = ins & 1;
 
switch (opcode) {
+   case 16: /* bc[l][a] */
+   if (ins & 1)/* bcl[a] */
+   typ = INSN_OTHER;
+   else/* bc[a] */
+   typ = INSN_JUMP_CONDITIONAL;
+
+   imm = ins & 0xfffc;
+   if (imm & 0x8000)
+   imm -= 0x1;
+   imm |= ins & 2; /* AA flag */
+   insn->immediate = imm;
+   break;
case 18: /* b[l][a] */
-   if ((ins & 3) == 1) /* bl */
+   if (ins & 1)/* bl[a] */
typ = INSN_CALL;
+   else/* b[a] */
+   typ = INSN_JUMP_UNCONDITIONAL;
 
imm = ins & 0x3fc;
if (imm & 0x200)
imm -= 0x400;
+   imm |= ins & 2; /* AA flag */
+   insn->immediate = imm;
+   break;
+   case 19:
+   if (xop == 16 && bo == 20 && bi == 0)   /* blr */
+   typ = INSN_RETURN;
+   else if (xop == 50) /* rfi */
+   typ = INSN_JUMP_DYNAMIC;
+   else if (xop == 528 && bo == 20 && bi ==0 && !l)/* bctr 
*/
+   typ = INSN_JUMP_DYNAMIC;
+   else if (xop == 528 && bo == 20 && bi ==0 && l) /* 
bctrl */
+   typ = INSN_CALL_DYNAMIC;
+   else
+   typ = INSN_OTHER;
+   break;
+   case 24:
+   if (rs == 0 && ra == 0 && uimm == 0)
+   typ = INSN_NOP;
+