ehmry/genode
ehmry
/
genode
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

hw: fix race in core's pager code (fix #2301) 

* Acknowledge receive of page-fault signal with ack_signal,
  but restart thread execution separately
* use kill_signal_context when disolving a pager_object to prevent race
* Remove bureaucracy in form of Thread_event and Signal_ack_handler
* remove dead code in riscv, namely Thread_base definition
* translation_table_insertions function for ARM drops out,
  which was overcautious
This commit is contained in:
Stefan Kalkowski 2017-03-01 15:28:13 +01:00 committed by Christian Helmuth
parent 40f319e9e9
commit 96eb82574a
22 changed files with 65 additions and 378 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
repos/base-hw/include/spec/arm/kernel/interface_support.h
View File

@ -21,14 +21,6 @@ namespace Kernel
{
typedef Genode::uint32_t Call_arg;
typedef Genode::uint32_t Call_ret;
/**
* Events that are provided by a kernel thread-object for user handling
*/
struct Thread_event_id
{
enum { FAULT = 0 };
};
}
#endif /* _INCLUDE__SPEC__ARM__KERNEL__INTERFACE_SUPPORT_H_ */

5
repos/base-hw/include/spec/riscv/kernel/interface_support.h
View File

@ -21,11 +21,6 @@ namespace Kernel
{
typedef Genode::uint64_t Call_arg;
typedef Genode::uint64_t Call_ret;
/**
* Events that are provided by a kernel thread-object for user handling
*/
struct Thread_event_id { enum { FAULT = 0 }; };
}
#endif /* _KERNEL__INTERFACE_SUPPORT_H_ */

8
repos/base-hw/include/spec/x86_64/kernel/interface_support.h
View File

@ -21,14 +21,6 @@ namespace Kernel
{
typedef Genode::uint64_t Call_arg;
typedef Genode::uint64_t Call_ret;
/**
* Events that are provided by a kernel thread-object for user handling
*/
struct Thread_event_id
{
enum { FAULT = 0 };
};
}
#endif /* _INCLUDE__SPEC__X86_64__KERNEL__INTERFACE_SUPPORT_H_ */

16
repos/base-hw/src/core/include/kernel/core_interface.h
View File

@ -40,7 +40,7 @@ namespace Kernel
constexpr Call_arg call_id_start_thread() { return 102; }
constexpr Call_arg call_id_pause_thread() { return 103; }
constexpr Call_arg call_id_resume_thread() { return 104; }
constexpr Call_arg call_id_route_thread_event() { return 105; }
constexpr Call_arg call_id_thread_pager() { return 105; }
constexpr Call_arg call_id_thread_quota() { return 106; }
constexpr Call_arg call_id_update_pd() { return 107; }
constexpr Call_arg call_id_new_pd() { return 108; }
@ -167,18 +167,12 @@ namespace Kernel
* Set or unset the handler of an event that can be triggered by a thread
*
* \param thread pointer to thread kernel object
* \param event_id capability id of the targeted thread event
* \param signal_context_id capability id of the handlers signal context
*
* \retval 0 succeeded
* \retval -1 failed
* \param signal_context_id capability id of the page-fault handler
*/
inline int route_thread_event(Thread * const thread,
capid_t const event_id,
capid_t const signal_context_id)
inline void thread_pager(Thread * const thread,
capid_t const signal_context_id)
{
return call(call_id_route_thread_event(), (Call_arg)thread,
event_id, signal_context_id);
call(call_id_thread_pager(), (Call_arg)thread, signal_context_id);
}

53
repos/base-hw/src/core/include/kernel/signal_receiver.h
View File

@ -27,11 +27,6 @@ namespace Kernel
*/
class Signal_handler;
/**
* Ability to get informed about signal acks
*/
class Signal_ack_handler;
/**
* Ability to destruct signal contexts
*/
@ -48,31 +43,6 @@ namespace Kernel
class Signal_receiver;
}
class Kernel::Signal_ack_handler
{
friend class Signal_context;
private:
Signal_context * _signal_context;
protected:
/**
* Provide custom handler for acks at a signal context
*/
virtual void _signal_acknowledged() = 0;
/**
* Constructor
*/
Signal_ack_handler() : _signal_context(0) { }
/**
* Destructor
*/
virtual ~Signal_ack_handler();
};
class Kernel::Signal_handler
{
@ -170,20 +140,6 @@ class Kernel::Signal_context : public Kernel::Object
typedef Genode::Fifo_element<Signal_context> Fifo_element;
/**
* Dummy handler that is used every time no other handler is available
*/
class Default_ack_handler : public Signal_ack_handler
{
private:
/************************
** Signal_ack_handler **
************************/
void _signal_acknowledged() { }
};
Fifo_element _deliver_fe;
Fifo_element _contexts_fe;
Signal_receiver * const _receiver;
@ -192,8 +148,6 @@ class Kernel::Signal_context : public Kernel::Object
bool _ack;
bool _killed;
Signal_context_killer * _killer;
Default_ack_handler _default_ack_handler;
Signal_ack_handler * _ack_handler;
/**
* Tell receiver about the submits of the context if any
@ -227,13 +181,6 @@ class Kernel::Signal_context : public Kernel::Object
*/
Signal_context(Signal_receiver * const r, unsigned const imprint);
/**
* Attach or detach a handler for acknowledgments at this context
*
* \param h handler that shall be attached or 0 to detach handler
*/
void ack_handler(Signal_ack_handler * const h);
/**
* Submit the signal
*

62
repos/base-hw/src/core/include/kernel/thread.h
View File

@ -24,54 +24,9 @@
namespace Kernel
{
class Thread;
class Thread_event;
class Core_thread;
}
/**
* Event that is provided by kernel thread-objects for user handling
*/
class Kernel::Thread_event : public Signal_ack_handler
{
private:
Thread * const _thread;
Signal_context * _signal_context;
/************************
** Signal_ack_handler **
************************/
void _signal_acknowledged();
public:
/**
* Constructor
*
* \param t thread that blocks on the event
*/
Thread_event(Thread * const t);
/**
* Submit to listening handlers just like a signal context
*/
void submit();
/**
* Kernel name of assigned signal context or 0 if not assigned
*/
Signal_context * const signal_context() const;
/**
* Override signal context of the event
*
* \param c new signal context or 0 to dissolve current signal context
*/
void signal_context(Signal_context * const c);
};
/**
* Kernel back-end for userland execution-contexts
*/
@ -81,7 +36,6 @@ class Kernel::Thread
public Ipc_node, public Signal_context_killer, public Signal_handler,
private Timeout
{
friend class Thread_event;
friend class Core_thread;
private:
@ -99,11 +53,10 @@ class Kernel::Thread
DEAD = 7,
};
Thread_event _fault;
Signal_context * _pager = nullptr;
addr_t _fault_pd;
addr_t _fault_addr;
addr_t _fault_writes;
addr_t _fault_signal;
State _state;
Signal_receiver * _signal_receiver;
char const * const _label;
@ -130,16 +83,6 @@ class Kernel::Thread
int _route_event(unsigned const event_id,
Signal_context * const signal_context_id);
/**
* Map kernel name of thread event to the corresponding member
*
* \param id kernel name of targeted thread event
*
* \retval 0 failed
* \retval >0 targeted member pointer
*/
Thread_event Thread::* _event(unsigned const id) const;
/**
* Return wether this is a core thread
*/
@ -219,7 +162,7 @@ class Kernel::Thread
void _call_delete_vm();
void _call_run_vm();
void _call_pause_vm();
void _call_route_thread_event();
void _call_pager();
void _call_new_irq();
void _call_ack_irq();
void _call_new_obj();
@ -346,7 +289,6 @@ class Kernel::Thread
addr_t fault_pd() const { return _fault_pd; }
addr_t fault_addr() const { return _fault_addr; }
addr_t fault_writes() const { return _fault_writes; }
addr_t fault_signal() const { return _fault_signal; }
};

2
repos/base-hw/src/core/include/platform_thread.h
View File

@ -129,6 +129,8 @@ namespace Genode {
*/
int start(void * const ip, void * const sp);
void restart();
/**
* Pause this thread
*/

10
repos/base-hw/src/core/include/spec/arm_v6/cpu.h
View File

@ -66,16 +66,6 @@ class Genode::Cpu : public Arm
static bool restricted_page_mappings() {
return Ctr::P::get(Ctr::read()); }
/**
* Ensure that TLB insertions get applied
*/
void translation_table_insertions()
{
clean_invalidate_data_cache();
invalidate_instr_cache();
invalidate_tlb();
}
/**
* Post processing after a translation was added to a translation table
*

2
repos/base-hw/src/core/include/spec/cortex_a15/cpu.h
View File

@ -426,8 +426,6 @@ class Genode::Cpu : public Arm_v7
void invalidate_data_cache() {
invalidate_inner_data_cache(); }
void translation_table_insertions() { invalidate_branch_predicts(); }
/**
* Hook function called at the very beginning
* of the local cpu initialization

5
repos/base-hw/src/core/include/spec/cortex_a8/cpu.h
View File

@ -37,11 +37,6 @@ class Genode::Cpu : public Arm_v7
void invalidate_data_cache() {
invalidate_inner_data_cache(); }
/**
* Ensure that TLB insertions get applied
*/
void translation_table_insertions();
/**
* Post processing after a translation was added to a translation table
*

2
repos/base-hw/src/core/include/spec/cortex_a9/cpu_support.h
View File

@ -128,8 +128,6 @@ class Genode::Cortex_a9 : public Arm_v7
Kernel::board().l2_cache().clean_invalidate();
}
void translation_table_insertions() { invalidate_branch_predicts(); }
static unsigned executing_id() { return Mpidr::Aff_0::get(Mpidr::read()); }

43
repos/base-hw/src/core/include/spec/riscv/kernel/thread_base.h
View File

@ -1,43 +0,0 @@
/*
* \brief Hardware specific base of kernel thread-objects
* \author Sebastian Sumpf
* \date 2015-06-02
*/
/*
* Copyright (C) 2015-2017 Genode Labs GmbH
*
* This file is part of the Genode OS framework, which is distributed
* under the terms of the GNU Affero General Public License version 3.
*/
#ifndef _KERNEL__THREAD_BASE_H_
#define _KERNEL__THREAD_BASE_H_
/* core includes */
#include <kernel/thread_event.h>
namespace Kernel { class Thread; }
/**
* Hardware specific base of kernel thread-objects
*/
class Kernel::Thread_base
{
protected:
Thread_event _fault;
addr_t _fault_pd;
addr_t _fault_addr;
addr_t _fault_writes;
addr_t _fault_signal;
/**
* Constructor
*
* \param t generic part of kernel thread-object
*/
Thread_base(Thread * const t);
};
#endif /* _KERNEL__THREAD_BASE_H_ */

21
repos/base-hw/src/core/kernel/signal_receiver.cc
View File

@ -17,16 +17,6 @@
using namespace Kernel;
/************************
** Signal_ack_handler **
************************/
Signal_ack_handler::~Signal_ack_handler()
{
if (_signal_context) { _signal_context->ack_handler(0); }
}
/********************
** Signal_handler **
********************/
@ -83,13 +73,6 @@ void Signal_context::_delivered()
void Signal_context::_killer_cancelled() { _killer = 0; }
void Signal_context::ack_handler(Signal_ack_handler * const h)
{
_ack_handler = h ? h : &_default_ack_handler;
_ack_handler->_signal_context = this;
}
int Signal_context::submit(unsigned const n)
{
if (_killed || _submits >= (unsigned)~0 - n) { return -1; }
@ -101,7 +84,6 @@ int Signal_context::submit(unsigned const n)
void Signal_context::ack()
{
_ack_handler->_signal_acknowledged();
if (_ack) { return; }
if (!_killed) {
_ack = 1;
@ -153,8 +135,7 @@ Signal_context::Signal_context(Signal_receiver * const r, unsigned const imprint
_submits(0),
_ack(1),
_killed(0),
_killer(0),
_ack_handler(&_default_ack_handler)
_killer(0)
{
r->_add_context(this);
}

50
repos/base-hw/src/core/kernel/thread.cc
View File

@ -234,7 +234,7 @@ void Thread::_call_restart_thread()
return; }
Thread * const thread = pd()->cap_tree().find<Thread>(user_arg_1());
if (!thread || pd() != thread->pd()) {
if (!thread || (!_core() && (pd() != thread->pd()))) {
warning(*this, ": failed to lookup thread ", (unsigned)user_arg_1(),
" to restart it");
_die();
@ -288,13 +288,6 @@ void Thread::_cancel_blocking()
}
Thread_event::Thread_event(Thread * const t)
: _thread(t), _signal_context(0) { }
void Thread_event::submit() { if (_signal_context) _signal_context->submit(1); }
void Thread::_call_yield_thread()
{
Cpu_job::_yield();
@ -367,38 +360,14 @@ void Thread::_call_send_reply_msg()
}
void Thread::_call_route_thread_event()
void Thread::_call_pager()
{
/* override event route */
Thread * const t = (Thread*) user_arg_1();
unsigned const event_id = user_arg_2();
Signal_context * c = pd()->cap_tree().find<Signal_context>(user_arg_3());
user_arg_0(t->_route_event(event_id, c));
t->_pager = pd()->cap_tree().find<Signal_context>(user_arg_2());
}
int Thread::_route_event(unsigned const event_id,
Signal_context * c)
{
/* lookup event and assign signal context */
Thread_event Thread::* e = _event(event_id);
if (!e) { return -1; }
(this->*e).signal_context(c);
return 0;
}
void Thread_event::signal_context(Signal_context * const c)
{
_signal_context = c;
if (_signal_context) { _signal_context->ack_handler(this); }
}
Signal_context * const Thread_event::signal_context() const {
return _signal_context; }
void Thread::_call_print_char() { Kernel::log((char)user_arg_1()); }
@ -611,7 +580,7 @@ void Thread::_call()
case call_id_start_thread(): _call_start_thread(); return;
case call_id_resume_thread(): _call_resume_thread(); return;
case call_id_cancel_thread_blocking(): _call_cancel_thread_blocking(); return;
case call_id_route_thread_event(): _call_route_thread_event(); return;
case call_id_thread_pager(): _call_pager(); return;
case call_id_update_pd(): _call_update_pd(); return;
case call_id_new_pd():
_call_new<Pd>((Genode::Translation_table *) user_arg_2(),
@ -647,21 +616,14 @@ void Thread::_call()
Thread::Thread(unsigned const priority, unsigned const quota,
char const * const label)
:
Cpu_job(priority, quota), _fault(this), _fault_pd(0), _fault_addr(0),
_fault_writes(0), _fault_signal(0), _state(AWAITS_START),
Cpu_job(priority, quota), _fault_pd(0), _fault_addr(0),
_fault_writes(0), _state(AWAITS_START),
_signal_receiver(0), _label(label)
{
_init();
}
Thread_event Thread::* Thread::_event(unsigned const id) const
{
static Thread_event Thread::* _events[] = { &Thread::_fault };
return id < sizeof(_events)/sizeof(_events[0]) ? _events[id] : 0;
}
void Thread::print(Genode::Output &out) const
{
Genode::print(out, (_pd) ? _pd->platform_pd()->label() : "?");

11
repos/base-hw/src/core/pager.cc
View File

@ -18,6 +18,7 @@
#include <pager.h>
#include <platform_thread.h>
#include <platform_pd.h>
#include <region_map_component.h>
/* base-internal includes */
#include <base/internal/capability_space.h>
@ -44,8 +45,8 @@ void Ipc_pager::set_reply_mapping(Mapping m) { _mapping = m; }
void Pager_object::wake_up()
{
using Object = Kernel_object<Kernel::Signal_context>;
Kernel::ack_signal(Capability_space::capid(Object::_cap));
Platform_thread * const pt = (Platform_thread *)badge();
if (pt) pt->restart();
}
void Pager_object::start_paging(Kernel::Signal_receiver * receiver)
@ -93,7 +94,10 @@ Pager_object::Pager_object(Cpu_session_capability cpu_session_cap,
void Pager_entrypoint::dissolve(Pager_object * const o)
{
remove(o);
if (o) {
Kernel::kill_signal_context(Capability_space::capid(o->cap()));
remove(o);
}
}
@ -109,4 +113,3 @@ Pager_capability Pager_entrypoint::manage(Pager_object * const o)
insert(o);
return reinterpret_cap_cast<Pager_object>(o->cap());
}

13
repos/base-hw/src/core/platform_thread.cc
View File

@ -186,11 +186,8 @@ void Platform_thread::pager(Pager_object * const pager)
{
using namespace Kernel;
if (route_thread_event(kernel_object(), Thread_event_id::FAULT,
pager ? Capability_space::capid(pager->cap())
: cap_id_invalid()))
error("failed to set pager object for thread ", label());
thread_pager(kernel_object(), pager ? Capability_space::capid(pager->cap())
: cap_id_invalid());
_pager = pager;
}
@ -210,3 +207,9 @@ void Platform_thread::state(Thread_state thread_state)
Cpu_state * cstate = static_cast<Cpu_state *>(kernel_object());
*cstate = static_cast<Cpu_state>(thread_state);
}
void Platform_thread::restart()
{
Kernel::restart_thread(Capability_space::capid(_cap));
}

61
repos/base-hw/src/core/region_map_support.cc
View File

@ -44,50 +44,45 @@ void Rm_client::unmap(addr_t, addr_t virt_base, size_t size)
void Pager_entrypoint::entry()
{
Untyped_capability cap;
while (1)
{
if (cap.valid()) Kernel::ack_signal(Capability_space::capid(cap));
/* receive fault */
if (Kernel::await_signal(Capability_space::capid(_cap))) continue;
Untyped_capability cap =
(*(Pager_object**)Thread::myself()->utcb()->data())->cap();
Pager_object *po = *(Pager_object**)Thread::myself()->utcb()->data();
cap = po->cap();
/*
* Synchronize access and ensure that the object is still managed
*
* FIXME: The implicit lookup of the oject isn't needed.
*/
auto lambda = [&] (Pager_object *po) {
if (!po) return;
if (!po) continue;
/* fetch fault data */
Platform_thread * const pt = (Platform_thread *)po->badge();
if (!pt) {
Genode::warning("failed to get platform thread of faulter");
return;
}
/* fetch fault data */
Platform_thread * const pt = (Platform_thread *)po->badge();
if (!pt) {
Genode::warning("failed to get platform thread of faulter");
continue;
}
_fault.ip = pt->kernel_object()->ip;
_fault.addr = pt->kernel_object()->fault_addr();
_fault.writes = pt->kernel_object()->fault_writes();
_fault.signal = pt->kernel_object()->fault_signal();
_fault.ip = pt->kernel_object()->ip;
_fault.addr = pt->kernel_object()->fault_addr();
_fault.writes = pt->kernel_object()->fault_writes();
/* try to resolve fault directly via local region managers */
if (po->pager(*this)) return;
/* try to resolve fault directly via local region managers */
if (po->pager(*this)) continue;
/* apply mapping that was determined by the local region managers */
{
Locked_ptr<Address_space> locked_ptr(pt->address_space());
if (!locked_ptr.valid()) return;
/* apply mapping that was determined by the local region managers */
{
Locked_ptr<Address_space> locked_ptr(pt->address_space());
if (!locked_ptr.valid()) continue;
Hw::Address_space * as = static_cast<Hw::Address_space*>(&*locked_ptr);
as->insert_translation(_mapping.virt(), _mapping.phys(),
_mapping.size(), _mapping.flags());
}
Hw::Address_space * as = static_cast<Hw::Address_space*>(&*locked_ptr);
as->insert_translation(_mapping.virt(), _mapping.phys(),
_mapping.size(), _mapping.flags());
}
/* let pager object go back to no-fault state */
po->wake_up();
};
apply(cap, lambda);
/* let pager object go back to no-fault state */
po->wake_up();
}
}

20
repos/base-hw/src/core/spec/arm/kernel/thread.cc
View File

@ -58,8 +58,7 @@ void Thread::_mmu_exception()
{
_become_inactive(AWAITS_RESTART);
if (in_fault(_fault_addr, _fault_writes)) {
_fault_pd = (addr_t)_pd->platform_pd();
_fault_signal = (addr_t)_fault.signal_context();
_fault_pd = (addr_t)_pd->platform_pd();
/*
* Core should never raise a page-fault. If this happens, print out an
@ -69,7 +68,7 @@ void Thread::_mmu_exception()
Genode::error("page fault in core thread (", label(), "): "
"ip=", Genode::Hex(ip), " fault=", Genode::Hex(_fault_addr));
_fault.submit();
if (_pager) _pager->submit(1);
return;
}
Genode::error(*this, ": raised unhandled ",
@ -129,18 +128,3 @@ void Kernel::Thread::_call_update_instr_region()
cpu->clean_invalidate_data_cache_by_virt_region(base, size);
cpu->invalidate_instr_cache_by_virt_region(base, size);
}
void Thread_event::_signal_acknowledged()
{
/*
* FIXME: this is currently only called as reply to a page-fault resolution.
* On some ARM platforms, we have to do maintainance operations
* after new page table entries where added. If core runs completely
* in privileged mode, we should move this hook to the mappings
* functions.
*/
cpu_pool()->cpu(Cpu::executing_id())->translation_table_insertions();
_thread->_restart();
}

6
repos/base-hw/src/core/spec/cortex_a8/cpu.cc
View File

@ -16,12 +16,6 @@
#include <kernel/kernel.h>
#include <kernel/cpu.h>
void Genode::Cpu::translation_table_insertions()
{
clean_invalidate_data_cache();
invalidate_tlb();
}
void Genode::Cpu::translation_added(Genode::addr_t const addr,
Genode::size_t const size)

13
repos/base-hw/src/core/spec/riscv/kernel/thread.cc
View File

@ -45,11 +45,10 @@ void Thread::exception(unsigned const cpu)
void Thread::_mmu_exception()
{
_become_inactive(AWAITS_RESTART);
_fault_pd = (addr_t)_pd->platform_pd();
_fault_signal = (addr_t)_fault.signal_context();
_fault_addr = Cpu::sbadaddr();
_fault_pd = (addr_t)_pd->platform_pd();
_fault_addr = Cpu::sbadaddr();
_fault.submit();
if (_pager) _pager->submit(1);
}
@ -66,9 +65,3 @@ void Thread::_call_update_data_region()
void Thread::_call_update_instr_region() { }
void Thread_event::_signal_acknowledged()
{
_thread->_restart();
}

26
repos/base-hw/src/core/spec/riscv/kernel/thread_base.cc
View File

@ -1,26 +0,0 @@
/*
* \brief CPU specific implementations of core
* \author Sebastian Sumpf
* \date 2015-06-02
*/
/*
* Copyright (C) 2015-2017 Genode Labs GmbH
*
* This file is part of the Genode OS framework, which is distributed
* under the terms of the GNU Affero General Public License version 3.
*/
/* core includes */
#include <kernel/thread.h>
using namespace Kernel;
Thread_base::Thread_base(Thread * const t)
:
_fault(t),
_fault_pd(0),
_fault_addr(0),
_fault_writes(0),
_fault_signal(0)
{ }

6
repos/base-hw/src/core/spec/x86_64/kernel/thread.cc
View File

@ -24,14 +24,10 @@ void Kernel::Thread::_call_update_data_region() { }
void Kernel::Thread::_call_update_instr_region() { }
void Kernel::Thread_event::_signal_acknowledged() { _thread->_restart(); }
void Kernel::Thread::_mmu_exception()
{
_become_inactive(AWAITS_RESTART);
_fault_pd = (addr_t)_pd->platform_pd();
_fault_signal = (addr_t)_fault.signal_context();
_fault_addr = Cpu::Cr2::read();
/*
@ -42,7 +38,7 @@ void Kernel::Thread::_mmu_exception()
Genode::error("page fault in core thread (", label(), "): "
"ip=", Genode::Hex(ip), " fault=", Genode::Hex(_fault_addr));
_fault.submit();
if (_pager) _pager->submit(1);
return;
}