From 6cb2c176d54a5e0342f8320542807c48a08477d4 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/gtest/px4_add_gtest.cmake | 1 + 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 +++++++++++++++ 13 files changed, 233 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 741e8e574a..b1587b157a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -414,6 +414,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) @@ -440,8 +442,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/gtest/px4_add_gtest.cmake b/cmake/gtest/px4_add_gtest.cmake index 1a54456194..c93b15734a 100644 --- a/cmake/gtest/px4_add_gtest.cmake +++ b/cmake/gtest/px4_add_gtest.cmake @@ -107,6 +107,7 @@ function(px4_add_functional_gtest) px4_daemon work_queue parameters + events perf tinybson uorb_msgs diff --git a/cmake/px4_add_module.cmake b/cmake/px4_add_module.cmake index 5c0ee1153d..19d2c41da1 100644 --- a/cmake/px4_add_module.cmake +++ b/cmake/px4_add_module.cmake @@ -168,10 +168,11 @@ function(px4_add_module) if(NOT DYNAMIC) target_link_libraries(${MODULE} PRIVATE prebuild_targets px4_platform systemlib perf) if (${PX4_PLATFORM} STREQUAL "nuttx" AND NOT CONFIG_BUILD_FLAT AND KERNEL) - target_link_libraries(${MODULE} PRIVATE kernel_parameters_interface px4_kernel_layer uORB_kernel) + target_link_libraries(${MODULE} PRIVATE + kernel_events_interface kernel_parameters_interface px4_kernel_layer uORB_kernel) set_property(GLOBAL APPEND PROPERTY PX4_KERNEL_MODULE_LIBRARIES ${MODULE}) else() - target_link_libraries(${MODULE} PRIVATE parameters_interface px4_layer uORB) + target_link_libraries(${MODULE} PRIVATE events_interface parameters_interface px4_layer uORB) 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 a6e06b9d47..d2e08a405b 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 9e1654451c..b85da67e07 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 1581ddfc35..d22905cd38 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 @@ -125,6 +126,7 @@ int px4_platform_init() #if !defined(CONFIG_BUILD_FLAT) hrt_ioctl_init(); + events_ioctl_init(); #endif /* configure CPU load estimation */ diff --git a/platforms/nuttx/src/px4/common/px4_protected_layers.cmake b/platforms/nuttx/src/px4/common/px4_protected_layers.cmake index 7f76502e68..9c57b488b6 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_kc nuttx_karch nuttx_kmm + PRIVATE + kernel_events_interface # events_ioctl_init ) target_link_libraries(px4_kernel_layer diff --git a/src/lib/events/CMakeLists.txt b/src/lib/events/CMakeLists.txt index b3540ffb3d..eea3e9b2ec 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 0000000000..929dbc4982 --- /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 0000000000..a3c8260ddd --- /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 0000000000..7b97c66aa4 --- /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 0000000000..a2f076d065 --- /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 */