From 71cd7b9d2eac910f6e25dbe10715f191739c83d0 Mon Sep 17 00:00:00 2001 From: Norman Feske Date: Tue, 14 May 2013 22:40:30 +0200 Subject: [PATCH] base-hw: Avoid early calls of cmpxchg This patch eliminates calls of 'cmpxchg' prior enabling the MMU. This is needed because the 'ldrex' and 'strex' instructions do not always work with MMU and L1 cache disabled, i.e., on Raspberry Pi. --- base-hw/src/base/console.cc | 8 +++-- base-hw/src/base/singleton.h | 53 ++++++++++++++++++++++++++++++++ base-hw/src/core/kernel.cc | 21 ++++++++++--- base-hw/src/core/kernel/thread.h | 9 +++--- base-hw/src/core/target.inc | 4 +++ base-hw/src/core/tlb/arm.h | 9 +++++- 6 files changed, 91 insertions(+), 13 deletions(-) create mode 100644 base-hw/src/base/singleton.h diff --git a/base-hw/src/base/console.cc b/base-hw/src/base/console.cc index 046a5e61d..ddb863675 100644 --- a/base-hw/src/base/console.cc +++ b/base-hw/src/base/console.cc @@ -1,4 +1,4 @@ -/** +/* * \brief Genode-console backend * \author Martin Stein * \date 2011-10-17 @@ -16,6 +16,9 @@ #include #include +/* base-hw includes */ +#include "singleton.h" + namespace Genode { /** @@ -63,8 +66,7 @@ using namespace Genode; */ static Platform_console * platform_console() { - static Platform_console static_platform_console; - return &static_platform_console; + return unsynchronized_singleton(); } diff --git a/base-hw/src/base/singleton.h b/base-hw/src/base/singleton.h new file mode 100644 index 000000000..c1fb05487 --- /dev/null +++ b/base-hw/src/base/singleton.h @@ -0,0 +1,53 @@ +/* + * \brief Helper for creating singleton objects + * \author Norman Feske + * \date 2013-05-14 + * + * Before enabling the MMU on ARM, the 'cmpxchg' implementation is not always + * guaranteed to work. For example, on the Raspberry Pi, the 'ldrex' as used by + * 'cmpxchg' causes the machine to reboot. After enabling the MMU, everything + * is fine. Hence, we need to avoid executing 'cmpxchg' prior this point. + * Unfortunately, 'cmpxchg' is implicitly called each time when creating a + * singleton object via a local-static object pattern. In this case, the + * compiler generates code that calls the '__cxa_guard_acquire' function of the + * C++ runtime, which, in turn, relies 'cmpxchg' for synchronization. + * + * The utility provided herein is an alternative way to create single object + * instances without implicitly calling 'cmpxchg'. Because object creation is + * not synchronized via a spin lock, it must not be used in scenarios where + * multiple threads may contend. + */ + +/* + * Copyright (C) 2013 Genode Labs GmbH + * + * This file is part of the Genode OS framework, which is distributed + * under the terms of the GNU General Public License version 2. + */ + +#ifndef _SINGLETON_H_ +#define _SINGLETON_H_ + +inline void *operator new(Genode::size_t, void *at) { return at; } + + +template +static inline T *unsynchronized_singleton(Args... args) +{ + /* + * Each instantiation of the function template with a different type 'T' + * yields a dedicated instance of the local static variables, thereby + * creating the living space for the singleton objects. + */ + static bool initialized; + static int inst[sizeof(T)/sizeof(int) + 1] __attribute__((aligned(ALIGN))); + + /* execute constructor on first call */ + if (!initialized) { + initialized = true; + new (&inst) T(args...); + } + return reinterpret_cast(inst); +} + +#endif /* _SINGLETON_H_ */ diff --git a/base-hw/src/core/kernel.cc b/base-hw/src/core/kernel.cc index 6e6e21977..372aeeecc 100644 --- a/base-hw/src/core/kernel.cc +++ b/base-hw/src/core/kernel.cc @@ -33,6 +33,9 @@ #include #include +/* base-hw includes */ +#include + using namespace Kernel; /* get core configuration */ @@ -262,7 +265,7 @@ namespace Kernel * Static mode transition control */ static Mode_transition_control * mtc() - { static Mode_transition_control _object; return &_object; } + { return unsynchronized_singleton(); } /** * Kernel object that represents a Genode PD @@ -333,7 +336,7 @@ namespace Kernel /** * Access to static interrupt-controller */ - static Pic * pic() { static Pic _object; return &_object; } + static Pic * pic() { return unsynchronized_singleton(); } } @@ -407,9 +410,11 @@ namespace Kernel */ static Pd * core() { - static Core_tlb tlb; - static Pd _pd(&tlb, 0); - return &_pd; + constexpr int tlb_align = 1 << Core_tlb::ALIGNM_LOG2; + + Core_tlb *core_tlb = unsynchronized_singleton(); + Pd *pd = unsynchronized_singleton(core_tlb, nullptr); + return pd; } @@ -1421,6 +1426,12 @@ extern "C" void kernel() /* switch to core address space */ Cpu::init_virt_kernel(core()->tlb()->base(), core_id()); + /* + * From this point on, it is safe to use 'cmpxchg', i.e., to create + * singleton objects via the static-local object pattern. See + * the comment in 'src/base/singleton.h'. + */ + /* create the core main thread */ static Native_utcb cm_utcb; static char cm_stack[DEFAULT_STACK_SIZE] diff --git a/base-hw/src/core/kernel/thread.h b/base-hw/src/core/kernel/thread.h index 1ef64829b..e3c7e8e70 100644 --- a/base-hw/src/core/kernel/thread.h +++ b/base-hw/src/core/kernel/thread.h @@ -25,6 +25,9 @@ #include #include +/* base-hw includes */ +#include + namespace Genode { class Platform_thread; @@ -211,8 +214,7 @@ namespace Kernel */ static Id_alloc * _id_alloc() { - static Id_alloc _id_alloc; - return &_id_alloc; + return unsynchronized_singleton(); } public: @@ -224,8 +226,7 @@ namespace Kernel */ static Pool * pool() { - static Pool _pool; - return &_pool; + return unsynchronized_singleton(); } /** diff --git a/base-hw/src/core/target.inc b/base-hw/src/core/target.inc index fea0b9649..b10dd597b 100644 --- a/base-hw/src/core/target.inc +++ b/base-hw/src/core/target.inc @@ -13,11 +13,15 @@ CC_OPT += -DCORE_MAIN=_main # add library dependencies LIBS += base-common +# enable C++11 support +CC_CXX_OPT += -std=gnu++11 + # add include paths INC_DIR += $(REP_DIR)/src/core \ $(REP_DIR)/src/core/include \ $(REP_DIR)/include \ $(REP_DIR)/src/platform \ + $(REP_DIR)/src/base \ $(BASE_DIR)/src/core/include \ $(BASE_DIR)/include \ $(BASE_DIR)/src/platform diff --git a/base-hw/src/core/tlb/arm.h b/base-hw/src/core/tlb/arm.h index dd96aaec7..0c788c259 100644 --- a/base-hw/src/core/tlb/arm.h +++ b/base-hw/src/core/tlb/arm.h @@ -106,7 +106,14 @@ namespace Arm /* lookup table for AP bitfield values according to 'w' and 'k' flag */ typedef typename T::Ap_1_0 Ap_1_0; typedef typename T::Ap_2 Ap_2; - static typename T::access_t const ap_bits[2][2] = {{ + + /* + * Note: Don't make 'ap_bits' static to avoid implicit use of 'cmpxchg' + * prior enabling the MMU. + * + * XXX Replace array with a simpler-to-grasp switch statement. + */ + typename T::access_t const ap_bits[2][2] = {{ Ap_1_0::bits(Ap_1_0::USER_RO_ACCESS) | /* -- */ Ap_2::bits(Ap_2::KERNEL_RW_OR_NO_ACCESS),