From a74ae300f6b164f960a57d7dd7712a6d6e5f1e91 Mon Sep 17 00:00:00 2001 From: Ville Juven Date: Thu, 21 Sep 2023 10:33:05 +0300 Subject: [PATCH] events: Move implementation of events::send() to lib/events Events have a global, system-wide sequence number, which must be handled atomically, (fetching and incrementing the sequence AND sending the event to uORB must be atomic). Currently in FLAT mode, only one instance of this sequence number exists, so it is OK to have it in px4_platform. However, in PROTECTED mode px4_platform is instantiated both in kernel- and user spaces, which makes two instances of this sequence number, which causes problems in the mavlink event handling logic. When mavlink receives and handles events, it expects that: - The sequence numbers arrive in order (seq n is followed by n+1 etc) - It increments by 1 - There is only one instance of the sequence number In PROTECTED mode this is violated, as the kernel and user sequence numbers run freely on their own. This patch fixes the issue by moving the event backend to the kernel and by providing user access to it via ioctl. --- CMakeLists.txt | 5 ++ cmake/px4_add_module.cmake | 5 +- platforms/common/CMakeLists.txt | 1 - .../common/include/px4_platform/board_ctrl.h | 3 +- platforms/nuttx/src/px4/common/px4_init.cpp | 2 + .../src/px4/common/px4_protected_layers.cmake | 2 + src/lib/events/CMakeLists.txt | 10 +++ .../common => src/lib/events}/events.cpp | 0 src/lib/events/events.h | 36 ++++++++++ src/lib/events/events_ioctl.cpp | 68 +++++++++++++++++++ src/lib/events/events_ioctl.h | 51 ++++++++++++++ src/lib/events/usr_events.cpp | 53 +++++++++++++++ 12 files changed, 232 insertions(+), 4 deletions(-) rename {platforms/common => src/lib/events}/events.cpp (100%) create mode 100644 src/lib/events/events.h create mode 100644 src/lib/events/events_ioctl.cpp create mode 100644 src/lib/events/events_ioctl.h create mode 100644 src/lib/events/usr_events.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index bd9f84f2b793..a7bd965aea10 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -397,6 +397,8 @@ endif() # add_library(parameters_interface INTERFACE) add_library(kernel_parameters_interface INTERFACE) +add_library(events_interface INTERFACE) +add_library(kernel_events_interface INTERFACE) include(px4_add_library) add_subdirectory(src/lib EXCLUDE_FROM_ALL) @@ -423,8 +425,11 @@ add_subdirectory(src/lib/parameters EXCLUDE_FROM_ALL) if(${PX4_PLATFORM} STREQUAL "nuttx" AND NOT CONFIG_BUILD_FLAT) target_link_libraries(parameters_interface INTERFACE usr_parameters) target_link_libraries(kernel_parameters_interface INTERFACE parameters) + target_link_libraries(events_interface INTERFACE usr_events) + target_link_libraries(kernel_events_interface INTERFACE events) else() target_link_libraries(parameters_interface INTERFACE parameters) + target_link_libraries(events_interface INTERFACE events) endif() # firmware added last to generate the builtin for included modules diff --git a/cmake/px4_add_module.cmake b/cmake/px4_add_module.cmake index 7098a51c65bb..ec86722cb139 100644 --- a/cmake/px4_add_module.cmake +++ b/cmake/px4_add_module.cmake @@ -168,13 +168,14 @@ function(px4_add_module) if(NOT DYNAMIC) target_link_libraries(${MODULE} PRIVATE prebuild_targets px4_platform systemlib perf uORB) if (${PX4_PLATFORM} STREQUAL "nuttx" AND NOT CONFIG_BUILD_FLAT AND KERNEL) - target_link_libraries(${MODULE} PRIVATE kernel_parameters_interface px4_kernel_layer) + target_link_libraries(${MODULE} PRIVATE + kernel_events_interface kernel_parameters_interface px4_kernel_layer) set_property(GLOBAL APPEND PROPERTY PX4_KERNEL_MODULE_LIBRARIES ${MODULE}) elseif(${PX4_PLATFORM} STREQUAL "qurt") target_link_libraries(${MODULE} PRIVATE px4_layer uORB) set_property(GLOBAL APPEND PROPERTY PX4_MODULE_LIBRARIES ${MODULE}) else() - target_link_libraries(${MODULE} PRIVATE parameters_interface px4_layer) + target_link_libraries(${MODULE} PRIVATE events_interface parameters_interface px4_layer) set_property(GLOBAL APPEND PROPERTY PX4_MODULE_LIBRARIES ${MODULE}) endif() set_property(GLOBAL APPEND PROPERTY PX4_MODULE_PATHS ${CMAKE_CURRENT_SOURCE_DIR}) diff --git a/platforms/common/CMakeLists.txt b/platforms/common/CMakeLists.txt index a15836015e5b..29b8122916cc 100644 --- a/platforms/common/CMakeLists.txt +++ b/platforms/common/CMakeLists.txt @@ -43,7 +43,6 @@ endif() add_library(px4_platform STATIC board_common.c board_identity.c - events.cpp external_reset_lockout.cpp i2c.cpp i2c_spi_buses.cpp diff --git a/platforms/nuttx/src/px4/common/include/px4_platform/board_ctrl.h b/platforms/nuttx/src/px4/common/include/px4_platform/board_ctrl.h index 9e1654451c55..b85da67e0744 100644 --- a/platforms/nuttx/src/px4/common/include/px4_platform/board_ctrl.h +++ b/platforms/nuttx/src/px4/common/include/px4_platform/board_ctrl.h @@ -53,7 +53,8 @@ #define _CRYPTOIOCBASE IOCTL_IDX_TO_BASE(2) #define _PARAMIOCBASE IOCTL_IDX_TO_BASE(3) #define _PLATFORMIOCBASE IOCTL_IDX_TO_BASE(4) -#define MAX_IOCTL_PTRS 5 +#define _EVENTSIOCBASE IOCTL_IDX_TO_BASE(5) +#define MAX_IOCTL_PTRS 6 /* The PLATFORMIOCLAUNCH IOCTL is used to launch kernel side modules * from the user side code diff --git a/platforms/nuttx/src/px4/common/px4_init.cpp b/platforms/nuttx/src/px4/common/px4_init.cpp index 3c91d3a04e75..a9e9f5e3bb5a 100644 --- a/platforms/nuttx/src/px4/common/px4_init.cpp +++ b/platforms/nuttx/src/px4/common/px4_init.cpp @@ -38,6 +38,7 @@ #include #include #include +#include #include #include #include @@ -132,6 +133,7 @@ int px4_platform_init() #if !defined(CONFIG_BUILD_FLAT) hrt_ioctl_init(); + events_ioctl_init(); #endif #ifdef CONFIG_SYSTEMCMDS_PARAM diff --git a/platforms/nuttx/src/px4/common/px4_protected_layers.cmake b/platforms/nuttx/src/px4/common/px4_protected_layers.cmake index fb810633c522..7f54024709d6 100644 --- a/platforms/nuttx/src/px4/common/px4_protected_layers.cmake +++ b/platforms/nuttx/src/px4/common/px4_protected_layers.cmake @@ -52,6 +52,8 @@ target_link_libraries(px4_kernel_layer nuttx_kxx nuttx_karch nuttx_kmm + PRIVATE + kernel_events_interface # events_ioctl_init ) if (DEFINED PX4_CRYPTO) diff --git a/src/lib/events/CMakeLists.txt b/src/lib/events/CMakeLists.txt index b3540ffb3d78..eea3e9b2ecec 100644 --- a/src/lib/events/CMakeLists.txt +++ b/src/lib/events/CMakeLists.txt @@ -110,3 +110,13 @@ add_custom_command(OUTPUT ${generated_events_header} ) add_custom_target(events_header DEPENDS ${generated_events_header}) add_dependencies(prebuild_targets events_header) + +# Build the interface(s) +if (${PX4_PLATFORM} STREQUAL "nuttx" AND NOT CONFIG_BUILD_FLAT) + list(APPEND EXTRA_SRCS events_ioctl.cpp) + add_library(usr_events usr_events.cpp) + add_dependencies(usr_events prebuild_targets) +endif() + +add_library(events events.cpp ${EXTRA_SRCS}) +add_dependencies(events prebuild_targets) diff --git a/platforms/common/events.cpp b/src/lib/events/events.cpp similarity index 100% rename from platforms/common/events.cpp rename to src/lib/events/events.cpp diff --git a/src/lib/events/events.h b/src/lib/events/events.h new file mode 100644 index 000000000000..929dbc498279 --- /dev/null +++ b/src/lib/events/events.h @@ -0,0 +1,36 @@ +/**************************************************************************** + * + * Copyright (c) 2023 Technology Innovation Institute. All rights reserved. + * + * 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. + * 3. Neither the name PX4 nor the names of its contributors may be + * used to endorse or promote products derived from this software + * without specific prior written permission. + * + * 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. + * + ****************************************************************************/ + +#pragma once + +void events_ioctl_init(void); diff --git a/src/lib/events/events_ioctl.cpp b/src/lib/events/events_ioctl.cpp new file mode 100644 index 000000000000..a3c8260ddd8b --- /dev/null +++ b/src/lib/events/events_ioctl.cpp @@ -0,0 +1,68 @@ +/**************************************************************************** + * + * Copyright (c) 2023 Technology Innovation Institute. All rights reserved. + * + * 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. + * 3. Neither the name PX4 nor the names of its contributors may be + * used to endorse or promote products derived from this software + * without specific prior written permission. + * + * 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. + * + ****************************************************************************/ + +/** + * @file events_ioctl.cpp + * + * Interface to send events from user space. + */ + +#include + +#include + +#include "events_ioctl.h" + +static int events_ioctl(unsigned int cmd, unsigned long arg) +{ + int ret = OK; + + switch (cmd) { + case EVENTSIOCSEND: { + eventiocsend_t *data = (eventiocsend_t *)arg; + events::send(data->event); + } + break; + + default: + ret = -ENOTTY; + break; + } + + return ret; +} + +void events_ioctl_init(void) +{ + px4_register_boardct_ioctl(_EVENTSIOCBASE, events_ioctl); +} diff --git a/src/lib/events/events_ioctl.h b/src/lib/events/events_ioctl.h new file mode 100644 index 000000000000..7b97c66aa41a --- /dev/null +++ b/src/lib/events/events_ioctl.h @@ -0,0 +1,51 @@ +/**************************************************************************** + * + * Copyright (c) 2023 Technology Innovation Institute. All rights reserved. + * + * 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. + * 3. Neither the name PX4 nor the names of its contributors may be + * used to endorse or promote products derived from this software + * without specific prior written permission. + * + * 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. + * + ****************************************************************************/ + +/** + * @file events_ioctl.h + * + * User space - kernel space interface for dispatching events. + */ + +#pragma once + +#include +#include +#include + +#define _EVENTSIOC(_n) (_PX4_IOC(_EVENTSIOCBASE, _n)) + +#define EVENTSIOCSEND _EVENTSIOC(1) +typedef struct eventiocsend { + events::EventType &event; +} eventiocsend_t; diff --git a/src/lib/events/usr_events.cpp b/src/lib/events/usr_events.cpp new file mode 100644 index 000000000000..a2f076d06555 --- /dev/null +++ b/src/lib/events/usr_events.cpp @@ -0,0 +1,53 @@ +/**************************************************************************** + * + * Copyright (c) 2023 Technology Innovation Institute. All rights reserved. + * + * 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. + * 3. Neither the name PX4 nor the names of its contributors may be + * used to endorse or promote products derived from this software + * without specific prior written permission. + * + * 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. + * + ****************************************************************************/ + +/** + * @file usr_events.cpp + * + * User space interface for dispatching events. + */ + +#include + +#include "events_ioctl.h" + +namespace events +{ + +void send(EventType &event) +{ + eventiocsend_t data = {event}; + boardctl(EVENTSIOCSEND, reinterpret_cast(&data)); +} + +} /* namespace events */