[PATCH v2 2/6] cpukit: Add Exception Manager

Gedare Bloom gedare at rtems.org
Fri Oct 1 04:39:40 UTC 2021


You also might separate the exception manager addition away from the
topic of recoverable exceptions. This introduces/extends the classic
API, so it needs to be vetted carefully. Although the new header
claims to be a classic API, it appears to not follow classic API
conventions. You might need to think about what needs to be exposed to
the application level, and how, versus what should be internal. And
follow the conventions for classic API usage (e.g., opaque types not
pointers).

On Wed, Sep 22, 2021 at 6:17 PM Kinsey Moore <kinsey.moore at oarcorp.com> wrote:
>
> This adds the framework necessary to allow more generic handling of
> machine exceptions. This initial patch offers the ability to get the
> class of exception from the CPU_Exception_frame provided. Future
> extensions of the Exception Manager could include the ability to get
> the address of the exception if applicable or to resume execution at
> the next instruction or an arbitrary location.
> ---
>  cpukit/include/rtems/exception.h          | 166 ++++++++++++++++++++++
>  spec/build/cpukit/cpuopts.yml             |   2 +
>  spec/build/cpukit/librtemscpu.yml         |   1 +
>  spec/build/cpukit/optexceptionmanager.yml |  17 +++
>  4 files changed, 186 insertions(+)
>  create mode 100644 cpukit/include/rtems/exception.h
>  create mode 100644 spec/build/cpukit/optexceptionmanager.yml
>
> diff --git a/cpukit/include/rtems/exception.h b/cpukit/include/rtems/exception.h
> new file mode 100644
> index 0000000000..89edfd02b4
> --- /dev/null
> +++ b/cpukit/include/rtems/exception.h
> @@ -0,0 +1,166 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +
> +/**
> + * @file
> + *
> + * @brief This header file defines the Exception Manager API.
> + */
> +
> +/*
> + * Copyright (C) 2021 On-Line Applications Research Corporation (OAR)
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + *    notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + *    notice, this list of conditions and the following disclaimer in the
> + *    documentation and/or other materials provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> + * POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#ifndef _RTEMS_EXCEPTION_H
> +#define _RTEMS_EXCEPTION_H
> +
> +#include <rtems/fatal.h>
> +#include <rtems/score/percpu.h>
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +/**
> + * @defgroup RTEMSAPIClassicException Exception Manager
> + *
> + * @ingroup RTEMSAPIClassic
> + *

If this is an extension of the Classic API, then it should be
following classic api naming conventions, hence I'd expect this all to
be rtems_exception_xxx(). Please clarify.

> + * @brief The Exception Manager processes all machine exceptions and passes
> + *   any unhandled exceptions to the Fatal Error Manager.
> + */
> +
> +/**
> + *  The following lists the generic exception classes supported by the Exception
> + *  Management API.
> + */
> +typedef enum {
> +  EXCEPTION_UNKNOWN,
> +  EXCEPTION_FPU,
> +  EXCEPTION_TAGGED_OVERFLOW,
> +  EXCEPTION_DIV_ZERO,
> +  EXCEPTION_DATA_ABORT_READ,
> +  EXCEPTION_DATA_ABORT_WRITE,
> +  EXCEPTION_DATA_ABORT_UNSPECIFIED,
> +  EXCEPTION_INSTRUCTION_ABORT,
> +  EXCEPTION_MMU_UNSPECIFIED,
> +  EXCEPTION_ACCESS_ALIGNMENT,
> +  EXCEPTION_SUPERVISOR,
> +  EXCEPTION_TRAPPED_INSTRUCTION,
> +  EXCEPTION_PC_ALIGNMENT,
> +  EXCEPTION_SP_ALIGNMENT,
> +  EXCEPTION_BREAKPOINT,
> +  EXCEPTION_BREAK_INSTRUCTION,
> +  EXCEPTION_STEP,
> +  EXCEPTION_WATCHPOINT,
> +  EXCEPTION_MAX
> +} Exception_Class;
> +

How did you derive these? Do they need some more documentation?

I'm not 100% on calling them "Classes". Types? Sources? I'm not sure.

> +/**
> + * @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 Exception_Manager_resume( CPU_Exception_frame *ef );
> +

Another naming issue, everything in classic is a "Manager" I would
avoid embedding "Manager" in the name. Even if you do, it would be the
name of the "Class" so "Exception_manager" would be more proper (for
an score name), hence, "Exception_manager_Resume" would be the right
way to name things based on how things currently stand. But, again, I
think you might prefer rtems_exception_resume()?

> +/**
> + * @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 Exception_Manager_dispatch_and_resume( CPU_Exception_frame *ef );
> +
> +/**
> + * @brief Disables thread dispatch.
> + *
> + * This must be called before calling Exception_Manager_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.
> + */
> +static inline void Exception_Manager_disable_thread_dispatch( void )
> +{
> +  Per_CPU_Control *cpu_self = _Per_CPU_Get();
> +
> +  /* Increment interrupt nest and thread dispatch disable level */
> +  ++cpu_self->thread_dispatch_disable_level;
> +  ++cpu_self->isr_nest_level;
> +}
> +
> +/**
> + * @brief Retrieves the generic exception class of the machine exception.
> + *
> + * @param exception_frame The CPU_Exception_frame describing the machine
> + * exception.
> + * @return The class of exception represented by the CPU_Exception_frame.
> + */
> +Exception_Class Exception_Manager_Get_class( 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 Exception_Manager_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 Exception_Manager_Set_resume_next_instruction( CPU_Exception_frame *ef );
> +
> +/**
> + * @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 Exception_Manager_Copy_CPU_Exception_frame(
> +  CPU_Exception_frame *new_ef,
> +  CPU_Exception_frame *old_ef
> +);
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* _RTEMS_EXCEPTION_H */
> diff --git a/spec/build/cpukit/cpuopts.yml b/spec/build/cpukit/cpuopts.yml
> index 0a07f6d5e9..5568ed1316 100644
> --- a/spec/build/cpukit/cpuopts.yml
> +++ b/spec/build/cpukit/cpuopts.yml
> @@ -39,6 +39,8 @@ links:
>    uid: optdebug
>  - role: build-dependency
>    uid: optdrvmgr
> +- role: build-dependency
> +  uid: optexceptionmanager
>  - role: build-dependency
>    uid: optmpci
>  - role: build-dependency
> diff --git a/spec/build/cpukit/librtemscpu.yml b/spec/build/cpukit/librtemscpu.yml
> index c0f8f6d670..6f8d223f1e 100644
> --- a/spec/build/cpukit/librtemscpu.yml
> +++ b/spec/build/cpukit/librtemscpu.yml
> @@ -99,6 +99,7 @@ install:
>    - cpukit/include/rtems/dumpbuf.h
>    - cpukit/include/rtems/endian.h
>    - cpukit/include/rtems/error.h
> +  - cpukit/include/rtems/exception.h
>    - cpukit/include/rtems/extension.h
>    - cpukit/include/rtems/extensiondata.h
>    - cpukit/include/rtems/extensionimpl.h
> diff --git a/spec/build/cpukit/optexceptionmanager.yml b/spec/build/cpukit/optexceptionmanager.yml
> new file mode 100644
> index 0000000000..2188e16cf1
> --- /dev/null
> +++ b/spec/build/cpukit/optexceptionmanager.yml
> @@ -0,0 +1,17 @@
> +SPDX-License-Identifier: CC-BY-SA-4.0 OR BSD-2-Clause
> +actions:
> +- get-boolean: null
> +- env-enable: null
> +- define-condition: null
> +build-type: option
> +copyrights:
> +- Copyright (C) 2021 On-Line Applications Research (OAR)
> +default: true
> +default-by-family: []
> +default-by-variant: []
> +description: |
> +  Enable the Exception Manager
> +enabled-by:
> +links: []
> +name: RTEMS_EXCEPTION_MANAGER
> +type: build
> --
> 2.30.2
>
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel


More information about the devel mailing list