From eefaa070246b1dca6aa578bd34e156e2cb3a2229 Mon Sep 17 00:00:00 2001 From: Johannes Schlatow Date: Tue, 3 Oct 2023 15:37:01 +0200 Subject: [PATCH] base: add irq_type session argument By adding the `irq_type` argument, one can explicitly specify whether to use LEGACY, MSI or MSI-X interrupts. We formerly used the `device_phys_config` to implicitly select MSI, however, with the addition of IOMMU support to the platform driver there is at least one instance where we need an MSI for a non-PCI device. Yet, by adding another session argument to the Irq session, we exceed the character limit for session args. Since not all arguments are relevant for LEGACY interrupts resp. MSI, we can split the Irq_connection constructor to handle the two cases separately and omit unneeded arguments. genodelabs/genode#5002 --- .../src/core/irq_session_component.cc | 5 +++- .../src/core/irq_session_component.cc | 5 ++-- .../base-hw/src/core/irq_session_component.cc | 5 +--- .../src/core/irq_session_component.cc | 12 +++++----- .../src/core/irq_session_component.cc | 4 +++- .../src/core/irq_session_component.cc | 4 +++- .../src/core/irq_session_component.cc | 5 ++-- repos/base/include/irq_session/connection.h | 24 ++++++++++++++++--- repos/base/include/irq_session/irq_session.h | 5 ++++ repos/base/src/core/include/irq_args.h | 24 +++++++++++++++++-- repos/os/src/drivers/platform/device.cc | 7 +++--- repos/os/src/drivers/platform/device.h | 4 +--- .../src/drivers/platform/device_component.cc | 14 ++++++----- .../src/drivers/platform/device_component.h | 4 ++-- repos/os/src/drivers/platform/pci.cc | 6 ++--- repos/os/src/drivers/platform/pci.h | 2 +- 16 files changed, 88 insertions(+), 42 deletions(-) diff --git a/repos/base-fiasco/src/core/irq_session_component.cc b/repos/base-fiasco/src/core/irq_session_component.cc index 87c2cb93a6..46b9bb5cec 100644 --- a/repos/base-fiasco/src/core/irq_session_component.cc +++ b/repos/base-fiasco/src/core/irq_session_component.cc @@ -16,6 +16,7 @@ /* core includes */ #include +#include #include /* L4/Fiasco includes */ @@ -125,7 +126,9 @@ Irq_session_component::Irq_session_component(Range_allocator &irq_alloc, _irq_alloc(irq_alloc), _irq_object(_irq_number) { - long msi = Arg_string::find_arg(args, "device_config_phys").long_value(0); + Irq_args irq_args(args); + bool msi { irq_args.type() != Irq_session::TYPE_LEGACY }; + if (msi) throw Service_denied(); diff --git a/repos/base-foc/src/core/irq_session_component.cc b/repos/base-foc/src/core/irq_session_component.cc index 3bdf10681e..231cb6ec5a 100644 --- a/repos/base-foc/src/core/irq_session_component.cc +++ b/repos/base-foc/src/core/irq_session_component.cc @@ -193,7 +193,8 @@ Irq_session_component::Irq_session_component(Range_allocator &irq_alloc, _irq_number((unsigned)Arg_string::find_arg(args, "irq_number").long_value(-1)), _irq_alloc(irq_alloc), _irq_object() { - long const msi = Arg_string::find_arg(args, "device_config_phys").long_value(0); + Irq_args const irq_args(args); + bool msi { irq_args.type() != TYPE_LEGACY }; if (msi) { if (msi_alloc().get(_irq_number, 1)) { @@ -208,8 +209,6 @@ Irq_session_component::Irq_session_component(Range_allocator &irq_alloc, } } - Irq_args const irq_args(args); - if (_irq_object.associate(_irq_number, msi, irq_args.trigger(), irq_args.polarity())) return; diff --git a/repos/base-hw/src/core/irq_session_component.cc b/repos/base-hw/src/core/irq_session_component.cc index 16d988749a..6c33684f91 100644 --- a/repos/base-hw/src/core/irq_session_component.cc +++ b/repos/base-hw/src/core/irq_session_component.cc @@ -69,10 +69,7 @@ Irq_session_component::Irq_session_component(Range_allocator &irq_alloc, _irq_number((unsigned)Platform::irq(_irq_args.irq_number())), _irq_alloc(irq_alloc), _kobj(), _is_msi(false), _address(0), _value(0) { - long const mmconf = - Arg_string::find_arg(args, "device_config_phys").long_value(0); - - if (mmconf) { + if (_irq_args.type() != Irq_session::TYPE_LEGACY) { _is_msi = Platform::alloc_msi_vector(_address, _value); if (!_is_msi) throw Service_denied(); diff --git a/repos/base-nova/src/core/irq_session_component.cc b/repos/base-nova/src/core/irq_session_component.cc index 166893ca0e..72f304886b 100644 --- a/repos/base-nova/src/core/irq_session_component.cc +++ b/repos/base-nova/src/core/irq_session_component.cc @@ -115,7 +115,7 @@ void Irq_object::sigh(Signal_context_capability cap) /* associate GSI or MSI to device belonging to device_phys */ bool ok = false; - if (_device_phys) + if (_device_phys || (_msi_addr && _msi_data)) ok = associate_msi(irq_sel(), _device_phys, _msi_addr, _msi_data, cap); else ok = associate_gsi(irq_sel(), cap, _gsi_flags); @@ -173,10 +173,10 @@ void Irq_object::start(unsigned irq, addr_t const device_phys, Irq_args const &i /* associate GSI or MSI to device belonging to device_phys */ bool ok = false; - if (device_phys) - ok = associate_msi(irq_sel(), device_phys, _msi_addr, _msi_data, _sigh_cap); - else + if (irq_args.type() == Irq_session::TYPE_LEGACY) ok = associate_gsi(irq_sel(), _sigh_cap, _gsi_flags); + else + ok = associate_msi(irq_sel(), device_phys, _msi_addr, _msi_data, _sigh_cap); if (!ok) throw Service_denied(); @@ -217,8 +217,7 @@ Irq_session_component::Irq_session_component(Range_allocator &irq_alloc, Irq_args const irq_args(args); long irq_number = irq_args.irq_number(); - long device_phys = Arg_string::find_arg(args, "device_config_phys").long_value(0); - if (device_phys) { + if (irq_args.type() != Irq_session::TYPE_LEGACY) { if ((unsigned long)irq_number >= kernel_hip().sel_gsi) throw Service_denied(); @@ -236,6 +235,7 @@ Irq_session_component::Irq_session_component(Range_allocator &irq_alloc, _irq_number = (unsigned)irq_number; + long device_phys = Arg_string::find_arg(args, "device_config_phys").long_value(0); _irq_object.start(_irq_number, device_phys, irq_args); } diff --git a/repos/base-okl4/src/core/irq_session_component.cc b/repos/base-okl4/src/core/irq_session_component.cc index 1dcda2d6b2..09aca4d00c 100644 --- a/repos/base-okl4/src/core/irq_session_component.cc +++ b/repos/base-okl4/src/core/irq_session_component.cc @@ -17,6 +17,7 @@ /* core includes */ #include +#include #include /* base-internal includes */ @@ -133,7 +134,8 @@ Irq_session_component::Irq_session_component(Range_allocator &irq_alloc, _irq_alloc(irq_alloc), _irq_object(_irq_number) { - long msi = Arg_string::find_arg(args, "device_config_phys").long_value(0); + Irq_args irq_args(args); + bool msi { irq_args.type() != Irq_session::TYPE_LEGACY }; if (msi) throw Service_denied(); diff --git a/repos/base-pistachio/src/core/irq_session_component.cc b/repos/base-pistachio/src/core/irq_session_component.cc index 692ce37261..a7ac996ad2 100644 --- a/repos/base-pistachio/src/core/irq_session_component.cc +++ b/repos/base-pistachio/src/core/irq_session_component.cc @@ -16,6 +16,7 @@ /* core includes */ #include +#include #include using namespace Core; @@ -126,7 +127,8 @@ Irq_session_component::Irq_session_component(Range_allocator &irq_alloc, _irq_alloc(irq_alloc), _irq_object(_irq_number) { - long msi = Arg_string::find_arg(args, "device_config_phys").long_value(0); + Irq_args irq_args(args); + bool msi { irq_args.type() != Irq_session::TYPE_LEGACY }; if (msi) throw Service_denied(); diff --git a/repos/base-sel4/src/core/irq_session_component.cc b/repos/base-sel4/src/core/irq_session_component.cc index 80f037cfd8..e9feab77d5 100644 --- a/repos/base-sel4/src/core/irq_session_component.cc +++ b/repos/base-sel4/src/core/irq_session_component.cc @@ -103,7 +103,8 @@ Irq_session_component::Irq_session_component(Range_allocator &irq_alloc, _irq_alloc(irq_alloc), _irq_object(_irq_number) { - long msi = Arg_string::find_arg(args, "device_config_phys").long_value(0); + Irq_args const irq_args(args); + bool msi { irq_args.type() != Irq_session::TYPE_LEGACY }; if (msi) throw Service_denied(); @@ -113,8 +114,6 @@ Irq_session_component::Irq_session_component(Range_allocator &irq_alloc, } - Irq_args const irq_args(args); - if (!_irq_object.associate(irq_args.trigger(), irq_args.polarity())) { error("could not associate with IRQ ", irq_args.irq_number()); throw Service_denied(); diff --git a/repos/base/include/irq_session/connection.h b/repos/base/include/irq_session/connection.h index d502c7a69c..5ad4784e5a 100644 --- a/repos/base/include/irq_session/connection.h +++ b/repos/base/include/irq_session/connection.h @@ -32,14 +32,32 @@ struct Genode::Irq_connection : Connection, Irq_session_client Irq_connection(Env &env, Label const &label, Trigger trigger = Irq_session::TRIGGER_UNCHANGED, - Polarity polarity = Irq_session::POLARITY_UNCHANGED, - addr_t device_config_phys = 0) + Polarity polarity = Irq_session::POLARITY_UNCHANGED) : Connection(env, label, Ram_quota { RAM_QUOTA }, Args("irq_number=", label, ", " "irq_trigger=", unsigned(trigger), ", " "irq_polarity=", unsigned(polarity), ", " - "device_config_phys=", Hex(device_config_phys))), + "irq_type=", unsigned(TYPE_LEGACY))), + Irq_session_client(cap()) + { } + + /** + * Constructor + * + * \param label (virtual) interrupt number + * \param device_config_phys config-space physical address + * \param type interrupt type (e.g., msi/msi-x) + */ + Irq_connection(Env &env, + Label const &label, + addr_t device_config_phys, + Type type = Irq_session::TYPE_MSI) + : + Connection(env, label, Ram_quota { RAM_QUOTA }, + Args("irq_number=", label, ", " + "device_config_phys=", Hex(device_config_phys), ", " + "irq_type=", unsigned(type))), Irq_session_client(cap()) { } }; diff --git a/repos/base/include/irq_session/irq_session.h b/repos/base/include/irq_session/irq_session.h index e0160a4cc5..782547d076 100644 --- a/repos/base/include/irq_session/irq_session.h +++ b/repos/base/include/irq_session/irq_session.h @@ -48,6 +48,11 @@ struct Genode::Irq_session : Session */ enum Polarity { POLARITY_UNCHANGED = 0, POLARITY_HIGH, POLARITY_LOW }; + /** + * Interrupt type + */ + enum Type { TYPE_LEGACY = 0, TYPE_MSI, TYPE_MSIX }; + /** * Destructor */ diff --git a/repos/base/src/core/include/irq_args.h b/repos/base/src/core/include/irq_args.h index 06f9e24480..4630097971 100644 --- a/repos/base/src/core/include/irq_args.h +++ b/repos/base/src/core/include/irq_args.h @@ -29,6 +29,7 @@ class Core::Irq_args Irq_session::Trigger _irq_trigger { Irq_session::TRIGGER_UNCHANGED }; Irq_session::Polarity _irq_polarity { Irq_session::POLARITY_UNCHANGED }; + Irq_session::Type _irq_type { Irq_session::TYPE_LEGACY }; long const _irq_number; @@ -38,8 +39,9 @@ class Core::Irq_args : _irq_number(Arg_string::find_arg(args, "irq_number").long_value(-1)) { - long irq_trg = Arg_string::find_arg(args, "irq_trigger").long_value(-1); - long irq_pol = Arg_string::find_arg(args, "irq_polarity").long_value(-1); + long irq_trg = Arg_string::find_arg(args, "irq_trigger").long_value(-1); + long irq_pol = Arg_string::find_arg(args, "irq_polarity").long_value(-1); + long irq_type = Arg_string::find_arg(args, "irq_type").long_value(-1); switch (irq_trg) { case -1: @@ -74,11 +76,29 @@ class Core::Irq_args _irq_number); throw Service_denied(); } + + switch (irq_type) { + case -1: + case Irq_session::TYPE_LEGACY: + _irq_type = Irq_session::TYPE_LEGACY; + break; + case Irq_session::TYPE_MSI: + _irq_type = Irq_session::TYPE_MSI; + break; + case Irq_session::TYPE_MSIX: + _irq_type = Irq_session::TYPE_MSIX; + break; + default: + error("invalid type ", irq_type, " specified for IRQ ", + _irq_number); + throw Service_denied(); + } } long irq_number() const { return _irq_number; } Irq_session::Trigger trigger() const { return _irq_trigger; } Irq_session::Polarity polarity() const { return _irq_polarity; } + Irq_session::Type type() const { return _irq_type; } }; #endif /* _CORE__INCLUDE__IRQ_ARGS_H_ */ diff --git a/repos/os/src/drivers/platform/device.cc b/repos/os/src/drivers/platform/device.cc index 76410090ac..23fd52e69c 100644 --- a/repos/os/src/drivers/platform/device.cc +++ b/repos/os/src/drivers/platform/device.cc @@ -207,7 +207,8 @@ void Driver::Device::update(Allocator &alloc, Xml_node const &node) irq.mode = (mode == "edge") ? Irq_session::TRIGGER_EDGE : Irq_session::TRIGGER_LEVEL; if (type.valid()) - irq.type = (type == "msi-x") ? Irq::MSIX : Irq::MSI; + irq.type = (type == "msi-x") ? Irq_session::TYPE_MSIX + : Irq_session::TYPE_MSI; return irq; }, @@ -463,7 +464,7 @@ void Driver::Device_model::update(Xml_node const & node) for_each([&] (Device const & device) { device._irq_list.for_each([&] (Device::Irq const & irq) { - if (irq.type != Device::Irq::LEGACY) + if (irq.type != Irq_session::TYPE_LEGACY) return; if (detected_irqs.get(irq.number, 1)) { @@ -480,7 +481,7 @@ void Driver::Device_model::update(Xml_node const & node) for_each([&] (Device & device) { device._irq_list.for_each([&] (Device::Irq & irq) { - if (irq.type != Device::Irq::LEGACY) + if (irq.type != Irq_session::TYPE_LEGACY) return; if (shared_irqs.get(irq.number, 1)) diff --git a/repos/os/src/drivers/platform/device.h b/repos/os/src/drivers/platform/device.h index 62e30bc64e..c6aba3be73 100644 --- a/repos/os/src/drivers/platform/device.h +++ b/repos/os/src/drivers/platform/device.h @@ -97,10 +97,8 @@ class Driver::Device : private List_model::Element struct Irq : List_model::Element { - enum Type { LEGACY, MSI, MSIX }; - unsigned number; - Type type { LEGACY }; + Irq_session::Type type { Irq_session::TYPE_LEGACY }; Irq_session::Polarity polarity { Irq_session::POLARITY_UNCHANGED }; Irq_session::Trigger mode { Irq_session::TRIGGER_UNCHANGED }; bool shared { false }; diff --git a/repos/os/src/drivers/platform/device_component.cc b/repos/os/src/drivers/platform/device_component.cc index 81c76d7214..94690cdca3 100644 --- a/repos/os/src/drivers/platform/device_component.cc +++ b/repos/os/src/drivers/platform/device_component.cc @@ -104,15 +104,17 @@ Genode::Irq_session_capability Device_component::irq(unsigned idx) if (!irq.shared && !irq.irq.constructed()) { addr_t pci_cfg_addr = 0; - if (irq.type != Device::Irq::LEGACY) { + if (irq.type != Irq_session::TYPE_LEGACY) { if (_pci_config.constructed()) pci_cfg_addr = _pci_config->addr; else error("MSI(-x) detected for device without pci-config!"); - } - irq.irq.construct(_env, irq.number, irq.mode, irq.polarity, - pci_cfg_addr); + + irq.irq.construct(_env, irq.number, pci_cfg_addr, irq.type); + } else + irq.irq.construct(_env, irq.number, irq.mode, irq.polarity); + Irq_session::Info info = irq.irq->info(); - if (info.type == Irq_session::Info::MSI) + if (pci_cfg_addr && info.type == Irq_session::Info::MSI) pci_msi_enable(_env, *this, pci_cfg_addr, info, irq.type); } @@ -179,7 +181,7 @@ Device_component::Device_component(Registry & registry, try { device.for_each_irq([&] (unsigned idx, unsigned nr, - Device::Irq::Type type, + Irq_session::Type type, Irq_session::Polarity polarity, Irq_session::Trigger mode, bool shared) diff --git a/repos/os/src/drivers/platform/device_component.h b/repos/os/src/drivers/platform/device_component.h index a736437f49..5c2f5894ba 100644 --- a/repos/os/src/drivers/platform/device_component.h +++ b/repos/os/src/drivers/platform/device_component.h @@ -40,7 +40,7 @@ class Driver::Device_component : public Rpc_object & registry, unsigned idx, unsigned number, - Device::Irq::Type type, + Irq_session::Type type, Irq_session::Polarity polarity, Irq_session::Trigger mode, bool shared) diff --git a/repos/os/src/drivers/platform/pci.cc b/repos/os/src/drivers/platform/pci.cc index deabdb8d2e..572cc4fdfb 100644 --- a/repos/os/src/drivers/platform/pci.cc +++ b/repos/os/src/drivers/platform/pci.cc @@ -193,13 +193,13 @@ void Driver::pci_msi_enable(Env & env, Device_component & dc, addr_t cfg_space, Irq_session::Info const info, - Device::Irq::Type type) + Irq_session::Type type) { Attached_io_mem_dataspace io_mem { env, cfg_space, 0x1000 }; Config config { (addr_t)io_mem.local_addr() }; config.scan(); - if (type == Device::Irq::Type::MSIX && config.msi_x_cap.constructed()) { + if (type == Irq_session::TYPE_MSIX && config.msi_x_cap.constructed()) { try { /* find the MSI-x table from the device's memory bars */ Platform::Device_interface::Range range; @@ -231,7 +231,7 @@ void Driver::pci_msi_enable(Env & env, return; } - if (type == Device::Irq::Type::MSI && config.msi_cap.constructed()) { + if (type == Irq_session::TYPE_MSI && config.msi_cap.constructed()) { config.msi_cap->enable(info.address, (uint16_t)info.value); return; } diff --git a/repos/os/src/drivers/platform/pci.h b/repos/os/src/drivers/platform/pci.h index 2b8146c497..328804296f 100644 --- a/repos/os/src/drivers/platform/pci.h +++ b/repos/os/src/drivers/platform/pci.h @@ -36,7 +36,7 @@ namespace Driver { void pci_apply_quirks(Genode::Env & env, Device const & dev); void pci_msi_enable(Genode::Env & env, Device_component & dc, addr_t cfg_space, Genode::Irq_session::Info const info, - Device::Irq::Type type); + Irq_session::Type type); bool pci_device_matches(Genode::Session_policy const & policy, Device const & dev); void pci_device_specific_info(Device const & dev,