[PATCH v3 1/5] cpukit: Add exception extensions

[Kinsey Moore]

On 10/28/2021 01:00, Sebastian Huber wrote:
>
>
> On 27/10/2021 23:44, Kinsey Moore wrote:
>> This adds the set of functions necessary to allow more generic handling
>> of machine exceptions. This initial patch offers the ability to
>> manipulate a CPU_Exception_frame and resume execution using that
>> exception information with or without thread dispatch. These functions
>> are gated behind the RTEMS_EXCEPTION_EXTENSIONS configuration option.
>> ---
>>   .../cpu/no_cpu/include/rtems/score/cpu.h      | 78 +++++++++++++++++++
>>   spec/build/cpukit/cpuopts.yml                 |  2 +
>>   spec/build/cpukit/optexceptionextensions.yml  | 18 +++++
>>   3 files changed, 98 insertions(+)
>>   create mode 100644 spec/build/cpukit/optexceptionextensions.yml
>>
>> diff --git a/cpukit/score/cpu/no_cpu/include/rtems/score/cpu.h 
>> b/cpukit/score/cpu/no_cpu/include/rtems/score/cpu.h
>> index e224a5e56e..f9afbe7243 100644
>> --- a/cpukit/score/cpu/no_cpu/include/rtems/score/cpu.h
>> +++ b/cpukit/score/cpu/no_cpu/include/rtems/score/cpu.h
>> @@ -1142,6 +1142,84 @@ typedef struct {
>>    */
>>   void _CPU_Exception_frame_print( const CPU_Exception_frame *frame );
>>   +#ifdef RTEMS_EXCEPTION_EXTENSIONS
>
> What is the purpose of this option? Who controls it. Who is supposed 
> to use it?
>
> The way this feature is implemented, an application which doesn't use 
> it will not get it linked in. So, from my point of view we don't need 
> an API visible option.
This is an option built similarly to RTEMS_SMP and is enabled on a 
per-architecture basis. It is primarily used to control whether the 
tests that use this functionality get built, but since it was available 
I applied it here, too, so warnings are generated if someone tries to 
use any of these functions on an architecture that doesn't support it.
>
>> +  /**
>> +   * @brief Resumes normal execution using the provided exception 
>> frame.
>> +   *
>> +   * This routine helps to avoid dead code in the exception handler 
>> epilogue and
>> +   * does not return. This routine may assume that the provided 
>> pointer is valid
>> +   * for resetting the exception stack.
>> +   *
>> +   * @param exception_frame The CPU_Exception_frame describing the 
>> machine
>> +   * exception.
>> +   */
>> +  RTEMS_NO_RETURN void _CPU_Exception_resume( CPU_Exception_frame 
>> *ef );
>
> The parameter name in the function signature and the documentation 
> should match. Also abbreviations are normally not used in the score. 
> Maybe just use "frame".
Thanks, I'll fix this.
>
>> +
>> +  /**
>> +   * @brief Performs thread dispatch and resumes normal execution.
>> +   *
>> +   * This routine helps to avoid dead code in the exception handler 
>> epilogue and
>> +   * does not return. This routine may assume that the provided 
>> pointer is valid
>> +   * for resetting the exception stack. This function is expected to 
>> decrement
>> +   * the ISR nest level and thread dispatch disable level in the 
>> Per_CPU_Control
>> +   * structure.
>> +   *
>> +   * @param exception_frame The CPU_Exception_frame describing the 
>> machine
>> +   * exception.
>> +   */
>> +  RTEMS_NO_RETURN void _CPU_Exception_dispatch_and_resume( 
>> CPU_Exception_frame *ef );
>> +
>> +  /**
>> +   * @brief Disables thread dispatch.
>> +   *
>> +   * This must be called before calling 
>> _CPU_Exception_dispatch_and_resume
>> +   * since that function is expected to reduce the levels 
>> incremented below.
>> +   *
>> +   * @param exception_frame The CPU_Exception_frame describing the 
>> machine
>> +   * exception.
>> +   */
>> +  void _CPU_Exception_disable_thread_dispatch( void );
>> +
>> +  /**
>> +   * @brief Retrieves the generic exception class of the machine 
>> exception.
>> +   *
>> +   * @param exception_frame The CPU_Exception_frame describing the 
>> machine
>> +   * exception.
>> +   * @return The signal associated with the CPU_Exception_frame.
>> +   */
>> +  int _CPU_Exception_frame_get_signal( CPU_Exception_frame *ef );
>> +
>> +  /**
>> +   * @brief Sets the execution address of the exception frame.
>> +   *
>> +   * @param exception_frame The CPU_Exception_frame describing the 
>> machine
>> +   * exception.
>> +   */
>> +  void _CPU_Exception_frame_set_resume( CPU_Exception_frame *ef, 
>> void *address );
>> +
>> +  /**
>> +   * @brief Sets the execution address of the exception frame to the 
>> next
>> +   * instruction.
>> +   *
>> +   * @param exception_frame The CPU_Exception_frame describing the 
>> machine
>> +   * exception.
>> +   */
>> +  void _CPU_Exception_frame_set_resume_next_instruction( 
>> CPU_Exception_frame *ef );
>
> Mabye name it _CPU_Exception_frame_make_resume_next_instruction() 
> since there is no parameter with a value.
I'll update this name.
>
>> +
>> +  /**
>> +   * @brief Copies data to the new exception frame from the old 
>> exception frame.
>> +   *
>> +   * @param old_ef The existing CPU_Exception_frame describing the 
>> machine
>> +   * exception.
>> +   * @param new_ef The new CPU_Exception_frame describing the machine
>> +   * exception.
>> +   */
>> +  void _CPU_Exception_frame_copy(
>> +    CPU_Exception_frame *new_ef,
>> +    CPU_Exception_frame *old_ef
>> +  );
>
> If not used, then please remove it.
This is used from the assembly implementation of the thread dispatch 
epilogue variant to copy the exception frame on to the thread stack. I 
suppose it could be done with memcpy instead since that is what the 
AArch64 implementation compiles down to.


Kinsey