From de7fdd3e1a1d0a7bc74e1022cf6697e97cd3a3e8 Mon Sep 17 00:00:00 2001 From: Stefan Kalkowski Date: Mon, 2 May 2022 11:37:04 +0200 Subject: [PATCH] platform_drv: wait for device's availability Instead of returning an invalid device capability when a device is (not yet) available, e.g. a PCI device is requested before the PCI bus got parsed accordingly, we check the device capability within the Platform::Connection utilities, and register temporarily an Io_signal_handler to wait for changes of the devices ROM, and try the device aquisition again. Thereby, simple drivers so not have to take the burden to do so. To enable this feature for all drivers, we always have to export a devices ROM, but limit the information about physical resources (I/O memory addresses, IRQ numbers, I/O port ranges) to clients with 'info=yes' in their policy description. Fix genodelabs/genode#4496 --- .../os/include/platform_session/connection.h | 89 +++++++++++++------ repos/os/include/platform_session/device.h | 2 +- .../os/include/platform_session/dma_buffer.h | 2 +- repos/os/run/platform_drv.run | 21 +---- repos/os/src/drivers/platform/README | 7 +- repos/os/src/drivers/platform/device.cc | 11 ++- repos/os/src/drivers/platform/device.h | 2 +- .../src/drivers/platform/session_component.cc | 5 +- repos/os/src/test/platform_drv/main.cc | 9 -- 9 files changed, 78 insertions(+), 70 deletions(-) diff --git a/repos/os/include/platform_session/connection.h b/repos/os/include/platform_session/connection.h index 05809cf5fa..350c8c7a1d 100644 --- a/repos/os/include/platform_session/connection.h +++ b/repos/os/include/platform_session/connection.h @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -34,31 +35,58 @@ class Platform::Connection : public Genode::Connection, { private: - /* 'Device' and 'Dma_buffer' access the '_rm' member */ + /* 'Device' and 'Dma_buffer' access the '_env' member */ friend class Device; friend class Dma_buffer; - Region_map &_rm; - Rom_session_client _rom {devices_rom()}; - Constructible _ds {}; + Env &_env; + Rom_session_client _rom {devices_rom()}; + Constructible _ds {}; + Constructible> _handler {}; void _try_attach() { _ds.destruct(); - try { _ds.construct(_rm, _rom.dataspace()); } + try { _ds.construct(_env.rm(), _rom.dataspace()); } catch (Attached_dataspace::Invalid_dataspace) { warning("Invalid devices rom dataspace returned!");} } + void _handle_io() {} + + template + Capability _wait_for_device(FN const & fn) + { + for (;;) { + /* repeatedly check for availability of device */ + Capability cap = fn(); + if (cap.valid()) { + if (_handler.constructed()) { + sigh(Signal_context_capability()); + _handler.destruct(); + } + return cap; + } + + if (!_handler.constructed()) { + _handler.construct(_env.ep(), *this, + &Connection::_handle_io); + sigh(*_handler); + } + + _env.ep().wait_and_dispatch_one_io_signal(); + } + } + public: Connection(Env &env) : Genode::Connection(env, session(env.parent(), - "ram_quota=%u, cap_quota=%u", - RAM_QUOTA, CAP_QUOTA)), + "ram_quota=%u, cap_quota=%u", + RAM_QUOTA, CAP_QUOTA)), Client(cap()), - _rm(env.rm()) + _env(env) { _try_attach(); } @@ -75,14 +103,18 @@ class Platform::Connection : public Genode::Connection, Capability acquire_device(Device_name const &name) override { - return retry_with_upgrade(Ram_quota{6*1024}, Cap_quota{6}, [&] () { - return Client::acquire_device(name); }); + return _wait_for_device([&] () { + return retry_with_upgrade(Ram_quota{6*1024}, Cap_quota{6}, [&] () { + return Client::acquire_device(name); }); + }); } Capability acquire_device() { - return retry_with_upgrade(Ram_quota{6*1024}, Cap_quota{6}, [&] () { - return Client::acquire_single_device(); }); + return _wait_for_device([&] () { + return retry_with_upgrade(Ram_quota{6*1024}, Cap_quota{6}, [&] () { + return Client::acquire_single_device(); }); + }); } Ram_dataspace_capability alloc_dma_buffer(size_t size, Cache cache) override @@ -105,30 +137,29 @@ class Platform::Connection : public Genode::Connection, Capability device_by_type(char const * type) { - using String = Genode::String<64>; + return _wait_for_device([&] () { - Capability cap; + using String = Genode::String<64>; - with_xml([&] (Xml_node & xml) { - xml.for_each_sub_node("device", [&] (Xml_node node) { + Capability cap; - /* already found a device? */ - if (cap.valid()) - return; + with_xml([&] (Xml_node & xml) { + xml.for_each_sub_node("device", [&] (Xml_node node) { - if (node.attribute_value("type", String()) != type) - return; + /* already found a device? */ + if (cap.valid()) + return; - Device_name name = node.attribute_value("name", Device_name()); - cap = acquire_device(name); + if (node.attribute_value("type", String()) != type) + return; + + Device_name name = node.attribute_value("name", Device_name()); + cap = acquire_device(name); + }); }); - if (!cap.valid()) { - error(__func__, ": type=", type, " not found!"); - error("device ROM content: ", xml); - } - }); - return cap; + return cap; + }); } }; diff --git a/repos/os/include/platform_session/device.h b/repos/os/include/platform_session/device.h index 1d8950b9ee..736fc5ceeb 100644 --- a/repos/os/include/platform_session/device.h +++ b/repos/os/include/platform_session/device.h @@ -59,7 +59,7 @@ class Platform::Device : Interface, Noncopyable return _cap.call(index); } - Region_map &_rm() { return _platform._rm; } + Region_map &_rm() { return _platform._env.rm(); } public: diff --git a/repos/os/include/platform_session/dma_buffer.h b/repos/os/include/platform_session/dma_buffer.h index 9d8100aa30..b80f42d2cd 100644 --- a/repos/os/include/platform_session/dma_buffer.h +++ b/repos/os/include/platform_session/dma_buffer.h @@ -46,7 +46,7 @@ class Platform::Dma_buffer : Noncopyable } _allocation; - Attached_dataspace _ds { _allocation.platform._rm, _allocation.cap }; + Attached_dataspace _ds { _allocation.platform._env.rm(), _allocation.cap }; public: diff --git a/repos/os/run/platform_drv.run b/repos/os/run/platform_drv.run index d4e08442d4..3fdfaf45f2 100644 --- a/repos/os/run/platform_drv.run +++ b/repos/os/run/platform_drv.run @@ -88,7 +88,7 @@ install_config { - + @@ -168,25 +168,6 @@ set good_string { [init -> test-platform_drv] [init -> test-platform_drv] [init -> test-platform_drv] -[init -> test-platform_drv] Error: Device 0 not valid! -[init -> test-platform_drv] -[init -> test-platform_drv] -[init -> test-platform_drv] -[init -> test-platform_drv] -[init -> test-platform_drv] -[init -> test-platform_drv] -[init -> test-platform_drv] -[init -> test-platform_drv] -[init -> test-platform_drv] -[init -> test-platform_drv] -[init -> test-platform_drv] -[init -> test-platform_drv] -[init -> test-platform_drv] -[init -> test-platform_drv] -[init -> test-platform_drv] -[init -> test-platform_drv] -[init -> test-platform_drv] -[init -> test-platform_drv] [init -> test-platform_drv] Found next valid device of dummy type [init -> test-platform_drv] Test has ended! } diff --git a/repos/os/src/drivers/platform/README b/repos/os/src/drivers/platform/README index e059d4efbb..777fac775b 100644 --- a/repos/os/src/drivers/platform/README +++ b/repos/os/src/drivers/platform/README @@ -42,9 +42,10 @@ physical to virtual identical mappings of DMA memory for the device_pd On some systems, e.g., base-hw kernel on certain ARM platforms, it allows the platform driver to call system management firmware via kernel syscalls. -The 'info' attribute in a policy node describe, whether the client gets -permission to access a devices ROM containing detailed information about -the devices of its virtual bus. +The 'info' attribute in a policy node describe, whether the client's devices +ROM will contain detailed information about physical resources of the devices +of its virtual bus. This is only useful when using ported legacy drivers, which +operate with global names of physical resources. Report facilities ----------------- diff --git a/repos/os/src/drivers/platform/device.cc b/repos/os/src/drivers/platform/device.cc index 80446a6cfd..3f530e3e0b 100644 --- a/repos/os/src/drivers/platform/device.cc +++ b/repos/os/src/drivers/platform/device.cc @@ -112,7 +112,8 @@ void Driver::Device::release(Session_component & sc) } -void Driver::Device::report(Xml_generator & xml, Device_model & devices) +void Driver::Device::report(Xml_generator & xml, Device_model & devices, + bool info) { xml.node("device", [&] () { xml.attribute("name", name()); @@ -120,17 +121,23 @@ void Driver::Device::report(Xml_generator & xml, Device_model & devices) xml.attribute("used", _owner.valid()); _io_mem_list.for_each([&] (Io_mem & io_mem) { xml.node("io_mem", [&] () { + if (!info) + return; xml.attribute("phys_addr", String<16>(Hex(io_mem.range.start))); xml.attribute("size", String<16>(Hex(io_mem.range.size))); }); }); _irq_list.for_each([&] (Irq & irq) { xml.node("irq", [&] () { + if (!info) + return; xml.attribute("number", irq.number); }); }); _io_port_range_list.for_each([&] (Io_port_range & io_port_range) { xml.node("io_port_range", [&] () { + if (!info) + return; xml.attribute("phys_addr", String<16>(Hex(io_port_range.addr))); xml.attribute("size", String<16>(Hex(io_port_range.size))); }); @@ -171,7 +178,7 @@ void Driver::Device_model::update_report() if (_reporter.enabled()) { Reporter::Xml_generator xml(_reporter, [&] () { for_each([&] (Device & device) { - device.report(xml, *this); }); + device.report(xml, *this, true); }); }); } } diff --git a/repos/os/src/drivers/platform/device.h b/repos/os/src/drivers/platform/device.h index 728ed3c329..3aa5bdc990 100644 --- a/repos/os/src/drivers/platform/device.h +++ b/repos/os/src/drivers/platform/device.h @@ -167,7 +167,7 @@ class Driver::Device : private List_model::Element fn(idx++, ipr.addr, ipr.size); }); } - void report(Xml_generator &, Device_model &); + void report(Xml_generator &, Device_model &, bool); protected: diff --git a/repos/os/src/drivers/platform/session_component.cc b/repos/os/src/drivers/platform/session_component.cc index 6af99fa901..447832c277 100644 --- a/repos/os/src/drivers/platform/session_component.cc +++ b/repos/os/src/drivers/platform/session_component.cc @@ -97,14 +97,11 @@ void Session_component::update_policy(bool info, Policy_version version) void Session_component::produce_xml(Xml_generator &xml) { - if (!_info) - return; - if (_version.valid()) xml.attribute("version", _version); _devices.for_each([&] (Device & dev) { - if (matches(dev)) dev.report(xml, _devices); }); + if (matches(dev)) dev.report(xml, _devices, _info); }); } diff --git a/repos/os/src/test/platform_drv/main.cc b/repos/os/src/test/platform_drv/main.cc index 522ea5421a..f4ce9192fc 100644 --- a/repos/os/src/test/platform_drv/main.cc +++ b/repos/os/src/test/platform_drv/main.cc @@ -60,7 +60,6 @@ struct Main Reporter device_reporter { env, "devices" }; Reconstructible platform { env }; - Constructible platform_2 { }; Signal_handler
device_rom_handler { env.ep(), *this, &Main::handle_device_update }; @@ -193,14 +192,6 @@ struct Main next_step(1, 1, 0x40000100, 32); return; case 6: - start_driver(0); - platform_2.construct(env); - devices[1].construct(*platform_2, Platform::Device::Name(0)); - stop_driver(1); - platform_2.destruct(); - next_step(4, 4, 0x40000100, 32); - return; - case 7: { Platform::Device dev (*platform, Platform::Device::Type({"dummy-device"})); if (dev._cap.valid()) log("Found next valid device of dummy type");