Re: [PATCH v2 2/6] cpukit: Add Exception Manager
On 01/10/2021 21:39, Kinsey Moore wrote:
Sebastian,
Could you be more specific about which parts of the
architecture-dependent interface still seem tied to the AArch64 port? I
thought I had made them sufficiently abstract by relying on
architecture-specific functions to manipulate and take action on the
exception frame. The signal mapping code written on top of this should
work for any architecture that implements those functions and contains
no reliance on architecture that I can see. The set of exception classes
in particular includes exceptions that don't exist on AArch64, but were
added because they're known to be present on SPARC and need to be
handled. I'm only really familiar with ARM and AArch64, so that could be
why I'm missing it.
I would provide a new CPU feature flag
#define CPU_HAS_EXCEPTION_TO_SIGNAL_MAPPING TRUE|FALSE
If it is TRUE, then the CPU port shall provide:
/**
* @retval -1 There is no signal associated with the exception.
* @retval SIGSEGV, SIGFPE, ...
*/
int _CPU_Exception_frame_get_signal( const CPU_Exception_frame *frame );
RTEMS_NO_RETURN _CPU_Exception_resume( CPU_Exception_frame *frame );
The architecture-independent part just needs to provide a fatal extension:
_Thread_local Thread_Action _Exception_Raise_signal_action;
void _Exception_Raise_signal( source, true, code )
{
int sig;
if ( source != RTEMS_FATAL_SOURCE_EXCEPTION ) {
return;
}
sig = _CPU_Exception_frame_get_signal( code );
switch ( sig ) {
case SIGSEGV:
handler = _Exception_Raise_sigsegv;
break;
default:
return;
}
_Thread_Prepend_post_switch_action( executing,
&_Exception_Raise_signal_action, handler );
_CPU_Exception_resume( code );
}
Using a post-switch extension has the benefit that you don't have to
call the complicated raise() function in the exception context. It is
called in thread context.
--
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.hu...@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax: +49-89-18 94 741 - 08
Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH v2 2/6] cpukit: Add Exception Manager
On 10/1/2021 08:29, Sebastian Huber wrote: On 01/10/2021 06:39, Gedare Bloom wrote: 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). Yes, the is reserved for API header files. The architecture-independent implementation part (Exception Handler) should go to . The architecture-dependent part should go in . If something really (!) needs to be visible to the API, then it should go in . The architecture-dependent parts should be documented in the no_cpu header files. Please make the architecture-dependent interface more abstract. From my point of view your current approach contains to many details of the aarch64 port. There should be documentation in the RTEMS CPU Supplement with a status for each architecture (similar to the TLS support). At the moment I only see one API element and this is the new application configuration option. If you really need new rtems_* interfaces, then please add a proper documentation for them so that it could be included in the RTEMS Classic API Guide. I would prefer to wait with new API elements until at this is implemented on at least another architecture. Thanks for the feedback, Gedare and Sebastian. I'll get this refactored as suggested and keep everything but the configuration option out of the API. Sebastian, Could you be more specific about which parts of the architecture-dependent interface still seem tied to the AArch64 port? I thought I had made them sufficiently abstract by relying on architecture-specific functions to manipulate and take action on the exception frame. The signal mapping code written on top of this should work for any architecture that implements those functions and contains no reliance on architecture that I can see. The set of exception classes in particular includes exceptions that don't exist on AArch64, but were added because they're known to be present on SPARC and need to be handled. I'm only really familiar with ARM and AArch64, so that could be why I'm missing it. Kinsey ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH v2 2/6] cpukit: Add Exception Manager
On 01/10/2021 06:39, Gedare Bloom wrote: 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). Yes, the is reserved for API header files. The architecture-independent implementation part (Exception Handler) should go to . The architecture-dependent part should go in . If something really (!) needs to be visible to the API, then it should go in . The architecture-dependent parts should be documented in the no_cpu header files. Please make the architecture-dependent interface more abstract. From my point of view your current approach contains to many details of the aarch64 port. There should be documentation in the RTEMS CPU Supplement with a status for each architecture (similar to the TLS support). At the moment I only see one API element and this is the new application configuration option. If you really need new rtems_* interfaces, then please add a proper documentation for them so that it could be included in the RTEMS Classic API Guide. I would prefer to wait with new API elements until at this is implemented on at least another architecture. -- embedded brains GmbH Herr Sebastian HUBER Dornierstr. 4 82178 Puchheim Germany email: sebastian.hu...@embedded-brains.de phone: +49-89-18 94 741 - 16 fax: +49-89-18 94 741 - 08 Registergericht: Amtsgericht München Registernummer: HRB 157899 Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler Unsere Datenschutzerklärung finden Sie hier: https://embedded-brains.de/datenschutzerklaerung/ ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH v2 2/6] cpukit: Add Exception Manager
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 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 00..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
> +#include
> +
> +#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
> + *
