From 6912dd62fa020db5f0d9a7727a1d91dd841ee6a4 Mon Sep 17 00:00:00 2001 From: Johannes Schlatow Date: Thu, 6 Jun 2024 22:32:33 +0200 Subject: [PATCH] platform: handle reserved memory on devices update Reserved memory regions must be excluded from the corresponding DMA allocators irrespective of whether the device is in use. Otherwise, an early allocation of DMA buffers may use the reserved memory regions of a late acquired device. Fixes #5232 --- repos/os/src/driver/platform/common.h | 2 +- repos/os/src/driver/platform/device.cc | 17 ++++++--- repos/os/src/driver/platform/device.h | 13 ++++--- .../src/driver/platform/device_component.cc | 9 +---- .../driver/platform/reserved_memory_handler.h | 35 +++++++++++++++++++ repos/os/src/driver/platform/root.cc | 19 ++++++++++ repos/os/src/driver/platform/root.h | 11 +++++- .../src/driver/platform/session_component.cc | 24 ++++++++----- .../src/driver/platform/session_component.h | 1 - 9 files changed, 103 insertions(+), 28 deletions(-) create mode 100644 repos/os/src/driver/platform/reserved_memory_handler.h diff --git a/repos/os/src/driver/platform/common.h b/repos/os/src/driver/platform/common.h index 2cb64868e7..929d2d584b 100644 --- a/repos/os/src/driver/platform/common.h +++ b/repos/os/src/driver/platform/common.h @@ -159,7 +159,7 @@ void Driver::Common::acquire_io_mmu_devices() void Driver::Common::_handle_devices() { _devices_rom.update(); - _devices.update(_devices_rom.xml()); + _devices.update(_devices_rom.xml(), _root); acquire_io_mmu_devices(); update_report(); _root.update_policy(); diff --git a/repos/os/src/driver/platform/device.cc b/repos/os/src/driver/platform/device.cc index f4015b321e..91898bec47 100644 --- a/repos/os/src/driver/platform/device.cc +++ b/repos/os/src/driver/platform/device.cc @@ -184,7 +184,8 @@ void Driver::Device::generate(Xml_generator & xml, bool info) const } -void Driver::Device::update(Allocator &alloc, Xml_node const &node) +void Driver::Device::update(Allocator &alloc, Xml_node const &node, + Reserved_memory_handler & reserved_mem_handler) { using Bar = Device::Pci_bar; @@ -390,11 +391,16 @@ void Driver::Device::update(Allocator &alloc, Xml_node const &node) { addr_t addr = node.attribute_value("address", 0UL); size_t size = node.attribute_value("size", 0UL); + reserved_mem_handler.add_range(*this, {addr, size}); return *(new (alloc) Reserved_memory({addr, size})); }, /* destroy */ - [&] (Reserved_memory &reserved) { destroy(alloc, &reserved); }, + [&] (Reserved_memory &reserved) + { + reserved_mem_handler.remove_range(*this, reserved.range); + destroy(alloc, &reserved); + }, /* update */ [&] (Reserved_memory &, Xml_node const &) { } @@ -445,7 +451,8 @@ void Driver::Device_model::generate(Xml_generator & xml) const } -void Driver::Device_model::update(Xml_node const & node) +void Driver::Device_model::update(Xml_node const & node, + Reserved_memory_handler & reserved_mem_handler) { _model.update_from_xml(node, @@ -461,7 +468,7 @@ void Driver::Device_model::update(Xml_node const & node) /* destroy */ [&] (Device &device) { - device.update(_heap, Xml_node("")); + device.update(_heap, Xml_node(""), reserved_mem_handler); device.release(_owner); destroy(_heap, &device); }, @@ -469,7 +476,7 @@ void Driver::Device_model::update(Xml_node const & node) /* update */ [&] (Device &device, Xml_node const &node) { - device.update(_heap, node); + device.update(_heap, node, reserved_mem_handler); } ); diff --git a/repos/os/src/driver/platform/device.h b/repos/os/src/driver/platform/device.h index 2e072502b4..1780898e2c 100644 --- a/repos/os/src/driver/platform/device.h +++ b/repos/os/src/driver/platform/device.h @@ -28,7 +28,7 @@ #include #include #include -#include +#include namespace Driver { @@ -38,6 +38,7 @@ namespace Driver { struct Device_reporter; struct Device_model; struct Device_owner; + struct Reserved_memory_handler; } @@ -418,7 +419,7 @@ class Driver::Device : private List_model::Element void generate(Xml_generator &, bool) const; - void update(Allocator &, Xml_node const &); + void update(Allocator &, Xml_node const &, Reserved_memory_handler &); /** * List_model::Element @@ -493,7 +494,7 @@ class Driver::Device_model public: void generate(Xml_generator & xml) const; - void update(Xml_node const & node); + void update(Xml_node const & node, Reserved_memory_handler &); void device_status_changed(); Device_model(Env & env, @@ -502,7 +503,11 @@ class Driver::Device_model Device_owner & owner) : _env(env), _heap(heap), _reporter(reporter), _owner(owner) { } - ~Device_model() { update(Xml_node("")); } + ~Device_model() + { + Reserved_memory_handler dummy { }; + update(Xml_node(""), dummy); + } template void for_each(FN const & fn) { _model.for_each(fn); } diff --git a/repos/os/src/driver/platform/device_component.cc b/repos/os/src/driver/platform/device_component.cc index 4caae1ac24..5f7bccb6e3 100644 --- a/repos/os/src/driver/platform/device_component.cc +++ b/repos/os/src/driver/platform/device_component.cc @@ -40,11 +40,7 @@ void Driver::Device_component::_release_resources() }); _reserved_mem_registry.for_each([&] (Io_mem & iomem) { - /* unreserve at dma allocator */ - _session.dma_allocator().unreserve(iomem.range.start, iomem.range.size); - - destroy(_session.heap(), &iomem); - }); + destroy(_session.heap(), &iomem); }); if (_pci_config.constructed()) _pci_config.destruct(); @@ -235,9 +231,6 @@ Device_component::Device_component(Registry & registry, Io_mem(_reserved_mem_registry, {0}, idx, range, false)); iomem.io_mem.construct(_env, iomem.range.start, iomem.range.size, false); - - /* reserve memory at dma allocator */ - session.dma_allocator().reserve(iomem.range.start, iomem.range.size); }); auto add_range_fn = [&] (Driver::Io_mmu::Domain & domain) { diff --git a/repos/os/src/driver/platform/reserved_memory_handler.h b/repos/os/src/driver/platform/reserved_memory_handler.h new file mode 100644 index 0000000000..974d14fd02 --- /dev/null +++ b/repos/os/src/driver/platform/reserved_memory_handler.h @@ -0,0 +1,35 @@ +/* + * \brief Global interface for handling reserved memory regions + * \author Johannes Schlatow + * \date 2024-06-06 + */ + +/* + * Copyright (C) 2024 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 _SRC__DRIVERS__PLATFORM__RESERVED_MEMORY_HANDLER_H_ +#define _SRC__DRIVERS__PLATFORM__RESERVED_MEMORY_HANDLER_H_ + +#include + +namespace Driver +{ + using Range = Platform::Device_interface::Range; + + struct Device; + struct Reserved_memory_handler; +} + + +struct Driver::Reserved_memory_handler +{ + virtual void add_range(Device const &, Range const &) { } + virtual void remove_range(Device const &, Range const &) { } + virtual ~Reserved_memory_handler() { } +}; + +#endif /* _SRC__DRIVERS__PLATFORM__RESERVED_MEMORY_HANDLER_H_ */ diff --git a/repos/os/src/driver/platform/root.cc b/repos/os/src/driver/platform/root.cc index 70f0efdec1..03a905ff57 100644 --- a/repos/os/src/driver/platform/root.cc +++ b/repos/os/src/driver/platform/root.cc @@ -69,6 +69,25 @@ void Driver::Root::_upgrade_session(Session_component * sc, const char * args) } +void Driver::Root::add_range(Device const & dev, Range const & range) +{ + _sessions.for_each([&] (Session_component & sc) { + if (!sc.matches(dev)) return; + sc._dma_allocator.reserve(range.start, range.size); + }); + +} + + +void Driver::Root::remove_range(Device const & dev, Range const & range) +{ + _sessions.for_each([&] (Session_component & sc) { + if (!sc.matches(dev)) return; + sc._dma_allocator.unreserve(range.start, range.size); + }); +} + + Driver::Root::Root(Env & env, Sliced_heap & sliced_heap, Attached_rom_dataspace const & config, diff --git a/repos/os/src/driver/platform/root.h b/repos/os/src/driver/platform/root.h index 364d8ee34c..d9f2bb6c3d 100644 --- a/repos/os/src/driver/platform/root.h +++ b/repos/os/src/driver/platform/root.h @@ -18,12 +18,14 @@ #include #include #include +#include #include namespace Driver { class Root; } -class Driver::Root : public Root_component +class Driver::Root : public Root_component, + public Reserved_memory_handler { public: @@ -50,6 +52,13 @@ class Driver::Root : public Root_component }); } + /***************************** + ** Reserved_memory_handler ** + *****************************/ + + void add_range(Device const &, Range const &) override; + void remove_range(Device const &, Range const &) override; + private: Session_component * _create_session(const char * args) override; diff --git a/repos/os/src/driver/platform/session_component.cc b/repos/os/src/driver/platform/session_component.cc index 52761a7511..7af77553ba 100644 --- a/repos/os/src/driver/platform/session_component.cc +++ b/repos/os/src/driver/platform/session_component.cc @@ -66,13 +66,16 @@ Session_component::_acquire(Device & device) void Session_component::_release_device(Device_component & dc) { - Device::Name name = dc.device(); _env.ep().rpc_ep().dissolve(&dc); - destroy(heap(), &dc); + /* release device (calls disable_device()) before destroying device component */ + Device::Name name = dc.device(); _devices.for_each([&] (Device & dev) { if (name == dev.name()) dev.release(*this); }); + /* destroy device component */ + destroy(heap(), &dc); + /* destroy unused domains */ _domain_registry.for_each([&] (Io_mmu_domain & wrapper) { if (wrapper.domain.devices() == 0) @@ -219,12 +222,6 @@ Driver::Io_mmu_domain_registry & Session_component::domain_registry() } -Driver::Dma_allocator & Session_component::dma_allocator() -{ - return _dma_allocator; -} - - void Session_component::update_devices_rom() { _rom_session.trigger_update(); @@ -468,6 +465,17 @@ Session_component::Session_component(Env & env, _cap_quota_guard()); } }); + + /* + * Iterate matching devices and reserve reserved memory regions at DMA + * allocator. + */ + _devices.for_each([&] (Device const & dev) { + if (!matches(dev)) return; + + dev.for_each_reserved_memory([&] (unsigned, Io_mmu::Range range) { + _dma_allocator.reserve(range.start, range.size); }); + }); } diff --git a/repos/os/src/driver/platform/session_component.h b/repos/os/src/driver/platform/session_component.h index 9c4aed029c..8d1d96b245 100644 --- a/repos/os/src/driver/platform/session_component.h +++ b/repos/os/src/driver/platform/session_component.h @@ -64,7 +64,6 @@ class Driver::Session_component Heap & heap(); Io_mmu_domain_registry & domain_registry(); - Dma_allocator & dma_allocator(); void enable_dma_remapping() { _dma_allocator.enable_remapping(); }