[PATCH] libdebugger: Fix ARMv7-M with -O0

[Chris Johns]

On 19/3/20 7:52 pm, Sebastian Huber wrote:
> The ARMv7-M architecture has no CP-15.  The inline assembly for the
> CP-15 access leads to assembler errors if compiled with -O0.

I would like to understand this one in detail to see why there is cp15 in the build.

> ---
>  cpukit/libdebugger/rtems-debugger-arm.c | 24 +++++++++++++-----------
>  1 file changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/cpukit/libdebugger/rtems-debugger-arm.c b/cpukit/libdebugger/rtems-debugger-arm.c
> index 106cbe8b8a..67f5e2ad90 100644
> --- a/cpukit/libdebugger/rtems-debugger-arm.c
> +++ b/cpukit/libdebugger/rtems-debugger-arm.c
> @@ -269,6 +269,7 @@ do { \
>   */
>  RTEMS_INTERRUPT_LOCK_DEFINE(static, target_lock, "target_lock")
>  
> +#if ARM_CP15
>  /**
>   * An exception offset is added to the return address of the PC on an
>   * exception's stack frame. The PC needs to be adjusted.
> @@ -280,6 +281,7 @@ static const size_t exc_offsets[2][5] =
>    /* TMB undef_ins sup call pref abt data abt */
>    {  0,  2,        0,       4,       8 }
>  };
> +#endif
>  
>  /**
>   * Is a session active?
> @@ -1014,6 +1016,7 @@ arm_debug_break_setup(arm_debug_hwbreak* bp,
>    bp->value = (intptr_t) address;
>  }
>  
> +#if ARM_CP15
>  static void
>  arm_debug_break_write_control(int bp, uint32_t control)
>  {
> @@ -1167,6 +1170,7 @@ arm_debug_method_of_entry(void)
>  {
>    return (arm_debug_dbgdscr_read() >> 2) & 0xf;
>  }
> +#endif

I do not understand why all this code needs to go? How is MMAP or cp14 related
to cp15? I have not looked at the detail and I am looking at the commit comment.

>  static void
>  arm_debug_disable_interrupts(void)
> @@ -1174,6 +1178,7 @@ arm_debug_disable_interrupts(void)
>    debug_disable_ints = true;
>  }
>  
> +#if ARM_CP15
>  static void
>  arm_debug_commit_interrupt_disable(void)
>  {
> @@ -1188,6 +1193,7 @@ arm_debug_enable_interrupts(void)
>  {
>    arm_debug_dbgdscr_write(arm_debug_dbgdscr_read() & ~(1 << 11));
>  }
> +#endif
>  
>  static void
>  arm_debug_break_clear(void)
> @@ -1211,6 +1217,7 @@ arm_debug_set_context_id(const uint32_t id)
>  #endif
>  }
>  
> +#if ARM_CP15
>  /*
>   * You can only load the hardware breaks points when in the SVC mode or the
>   * single step inverted break point will trigger.
> @@ -1237,10 +1244,12 @@ arm_debug_break_load(void)
>    }
>    rtems_interrupt_lock_release(&target_lock, &lock_context);
>  }
> +#endif

I would need to look at this in detail and the hardware doco to see what is
happening. My concern is this may build but does it breaks the needed
functionality if it is to work on this device? The backend has ROM detect
support and support for memory mapped or cp14 support and I am wanting to keep
as much code in play rather than blank disables until it is supported.

Is there no break point support?

>  
>  static void
>  arm_debug_break_unload(void)
>  {
> +#if ARM_CP15
>    rtems_interrupt_lock_context lock_context;
>    arm_debug_hwbreak*           bp = &hw_breaks[0];
>    int                          i;
> @@ -1251,8 +1260,10 @@ arm_debug_break_unload(void)
>      arm_debug_break_write_control(i, 0);
>    }
>    rtems_interrupt_lock_release(&target_lock, &lock_context);
> +#endif
>  }
>  
> +#if ARM_CP15
>  static void
>  arm_debug_break_dump(void)
>  {
> @@ -1267,6 +1278,7 @@ arm_debug_break_dump(void)
>    }
>  #endif
>  }
> +#endif
>  
>  #if NOT_USED_BUT_KEEPING
>  static size_t
> @@ -1295,6 +1307,7 @@ rtems_debugger_target_configure(rtems_debugger_target* target)
>    return arm_debug_probe(target);
>  }
>  
> +#if ARM_CP15
>  static void
>  target_print_frame(CPU_Exception_frame* frame)
>  {
> @@ -1591,7 +1604,6 @@ target_exception(CPU_Exception_frame* frame)
>  /*
>   * This is used to catch faulting accesses.
>   */
> -#if ARM_CP15
>  static void __attribute__((naked))
>  arm_debug_unlock_abort(void)
>  {
> @@ -1601,7 +1613,6 @@ arm_debug_unlock_abort(void)
>    EXCEPTION_ENTRY_THREAD(frame);
>    longjmp(unlock_abort_jmpbuf, -1);
>  }
> -#endif

Hmm I am now getting nervous the patch is to quickly solve a specific issue when
I would prefer it is properly resolved. I could be wrong and I would be happy
about that but it needs more time than I have to look.

Chris

>  
>  static void __attribute__((naked))
>  target_exception_undefined_instruction(void)
> @@ -1677,7 +1688,6 @@ target_exception_data_abort(void)
>    EXCEPTION_EXIT_EXC();
>  }
>  
> -#if ARM_CP15
>  /**
>   * The init value for the text section.
>   */
> @@ -1721,14 +1731,6 @@ rtems_debugger_target_set_vectors(void)
>    /*
>     * Dummy, please add support for your ARM variant.
>     */
> -  void* ui = target_exception_undefined_instruction;
> -  void* sc = target_exception_supervisor_call;
> -  void* pa = target_exception_prefetch_abort;
> -  void* da = target_exception_data_abort;
> -  (void) ui;
> -  (void) sc;
> -  (void) pa;
> -  (void) da;
>  }
>  
>  static void
>