diff --git a/Kernel/Bus/VirtIO/Console.cpp b/Kernel/Bus/VirtIO/Console.cpp index 10643b8bb4..2ea0a33689 100644 --- a/Kernel/Bus/VirtIO/Console.cpp +++ b/Kernel/Bus/VirtIO/Console.cpp @@ -25,16 +25,14 @@ UNMAP_AFTER_INIT ErrorOr Console::initialize_virtio_resources() { TRY(Device::initialize_virtio_resources()); auto const* cfg = TRY(transport_entity().get_config(VirtIO::ConfigurationType::Device)); - bool success = negotiate_features([&](u64 supported_features) { + TRY(negotiate_features([&](u64 supported_features) { u64 negotiated = 0; if (is_feature_set(supported_features, VIRTIO_CONSOLE_F_SIZE)) dbgln("VirtIO::Console: Console size is not yet supported!"); if (is_feature_set(supported_features, VIRTIO_CONSOLE_F_MULTIPORT)) negotiated |= VIRTIO_CONSOLE_F_MULTIPORT; return negotiated; - }); - if (!success) - return Error::from_errno(EIO); + })); u32 max_nr_ports = 0; u16 cols = 0, rows = 0; transport_entity().read_config_atomic([&]() { @@ -49,9 +47,7 @@ UNMAP_AFTER_INIT ErrorOr Console::initialize_virtio_resources() }); dbgln("VirtIO::Console: cols: {}, rows: {}, max nr ports {}", cols, rows, max_nr_ports); // Base receiveq/transmitq for port0 + optional control queues and 2 per every additional port - success = setup_queues(2 + max_nr_ports > 0 ? 2 + 2 * max_nr_ports : 0); - if (!success) - return Error::from_errno(EIO); + TRY(setup_queues(2 + max_nr_ports > 0 ? 2 + 2 * max_nr_ports : 0)); finish_init(); if (is_feature_accepted(VIRTIO_CONSOLE_F_MULTIPORT)) { @@ -70,10 +66,10 @@ UNMAP_AFTER_INIT Console::Console(NonnullOwnPtr transport_entit { } -bool Console::handle_device_config_change() +ErrorOr Console::handle_device_config_change() { dbgln("VirtIO::Console: Handle device config change"); - return true; + return {}; } void Console::handle_queue_update(u16 queue_index) diff --git a/Kernel/Bus/VirtIO/Console.h b/Kernel/Bus/VirtIO/Console.h index f98ee27297..3c7e598c14 100644 --- a/Kernel/Bus/VirtIO/Console.h +++ b/Kernel/Bus/VirtIO/Console.h @@ -59,7 +59,7 @@ private: constexpr static size_t CONTROL_MESSAGE_SIZE = sizeof(ControlMessage); constexpr static size_t CONTROL_BUFFER_SIZE = CONTROL_MESSAGE_SIZE * 32; - virtual bool handle_device_config_change() override; + virtual ErrorOr handle_device_config_change() override; virtual void handle_queue_update(u16 queue_index) override; Vector> m_ports; diff --git a/Kernel/Bus/VirtIO/Device.cpp b/Kernel/Bus/VirtIO/Device.cpp index e6c1b85310..4b75cce799 100644 --- a/Kernel/Bus/VirtIO/Device.cpp +++ b/Kernel/Bus/VirtIO/Device.cpp @@ -38,7 +38,7 @@ void Device::set_status_bit(u8 status_bit) m_transport_entity->set_status_bits({}, m_status); } -bool Device::accept_device_features(u64 device_features, u64 accepted_features) +ErrorOr Device::accept_device_features(u64 device_features, u64 accepted_features) { VERIFY(!m_did_accept_features); m_did_accept_features = true; @@ -70,28 +70,24 @@ bool Device::accept_device_features(u64 device_features, u64 accepted_features) if (!(m_status & DEVICE_STATUS_FEATURES_OK)) { set_status_bit(DEVICE_STATUS_FAILED); dbgln("{}: Features not accepted by host!", m_class_name); - return false; + return Error::from_errno(EIO); } m_accepted_features = accepted_features; dbgln_if(VIRTIO_DEBUG, "{}: Features accepted by host", m_class_name); - return true; + return {}; } -bool Device::setup_queue(u16 queue_index) +ErrorOr Device::setup_queue(u16 queue_index) { - auto queue_or_error = m_transport_entity->setup_queue({}, queue_index); - if (queue_or_error.is_error()) - return false; - - auto queue = queue_or_error.release_value(); + auto queue = TRY(m_transport_entity->setup_queue({}, queue_index)); dbgln_if(VIRTIO_DEBUG, "{}: Queue[{}] configured with size: {}", m_class_name, queue_index, queue->size()); - m_queues.append(move(queue)); - return true; + TRY(m_queues.try_append(move(queue))); + return {}; } -bool Device::setup_queues(u16 requested_queue_count) +ErrorOr Device::setup_queues(u16 requested_queue_count) { VERIFY(!m_did_setup_queues); m_did_setup_queues = true; @@ -103,7 +99,7 @@ bool Device::setup_queues(u16 requested_queue_count) m_queue_count = maximum_queue_count; } else if (requested_queue_count > maximum_queue_count) { dbgln("{}: {} queues requested but only {} available!", m_class_name, m_queue_count, maximum_queue_count); - return false; + return Error::from_errno(ENXIO); } else { m_queue_count = requested_queue_count; } @@ -113,15 +109,13 @@ bool Device::setup_queues(u16 requested_queue_count) } dbgln_if(VIRTIO_DEBUG, "{}: Setting up {} queues", m_class_name, m_queue_count); - for (u16 i = 0; i < m_queue_count; i++) { - if (!setup_queue(i)) - return false; - } - for (u16 i = 0; i < m_queue_count; i++) { // Queues can only be activated *after* all others queues were also configured - if (!m_transport_entity->activate_queue({}, i)) - return false; - } - return true; + for (u16 i = 0; i < m_queue_count; i++) + TRY(setup_queue(i)); + + // NOTE: Queues can only be activated *after* all others queues were also configured + for (u16 i = 0; i < m_queue_count; i++) + TRY(m_transport_entity->activate_queue({}, i)); + return {}; } void Device::finish_init() @@ -143,7 +137,7 @@ bool Device::handle_irq(Badge) } if (isr_type & DEVICE_CONFIG_INTERRUPT) { dbgln_if(VIRTIO_DEBUG, "{}: VirtIO Device config interrupt!", class_name()); - if (!handle_device_config_change()) { + if (handle_device_config_change().is_error()) { set_status_bit(DEVICE_STATUS_FAILED); dbgln("{}: Failed to handle device config change!", class_name()); } diff --git a/Kernel/Bus/VirtIO/Device.h b/Kernel/Bus/VirtIO/Device.h index 7d657f785c..b2138e56f8 100644 --- a/Kernel/Bus/VirtIO/Device.h +++ b/Kernel/Bus/VirtIO/Device.h @@ -35,7 +35,7 @@ protected: void mask_status_bits(u8 status_mask); void set_status_bit(u8); - bool setup_queues(u16 requested_queue_count = 0); + ErrorOr setup_queues(u16 requested_queue_count = 0); void finish_init(); Queue& get_queue(u16 queue_index) @@ -51,7 +51,7 @@ protected: } template - bool negotiate_features(F f) + ErrorOr negotiate_features(F f) { u64 device_features = m_transport_entity->get_device_features(); u64 accept_features = f(device_features); @@ -72,16 +72,16 @@ protected: void supply_chain_and_notify(u16 queue_index, QueueChain& chain); - virtual bool handle_device_config_change() = 0; + virtual ErrorOr handle_device_config_change() = 0; virtual void handle_queue_update(u16 queue_index) = 0; TransportEntity& transport_entity() { return *m_transport_entity; } private: - bool accept_device_features(u64 device_features, u64 accepted_features); + ErrorOr accept_device_features(u64 device_features, u64 accepted_features); - bool setup_queue(u16 queue_index); - bool activate_queue(u16 queue_index); + ErrorOr setup_queue(u16 queue_index); + ErrorOr activate_queue(u16 queue_index); void notify_queue(u16 queue_index); Vector> m_queues; diff --git a/Kernel/Bus/VirtIO/RNG.cpp b/Kernel/Bus/VirtIO/RNG.cpp index c2acf9dbb5..a028b0d278 100644 --- a/Kernel/Bus/VirtIO/RNG.cpp +++ b/Kernel/Bus/VirtIO/RNG.cpp @@ -19,14 +19,10 @@ UNMAP_AFTER_INIT NonnullLockRefPtr RNG::must_create_for_pci_instance(PCI::D UNMAP_AFTER_INIT ErrorOr RNG::initialize_virtio_resources() { TRY(Device::initialize_virtio_resources()); - bool success = negotiate_features([&](auto) { + TRY(negotiate_features([&](auto) { return 0; - }); - if (!success) - return Error::from_errno(EIO); - success = setup_queues(1); - if (!success) - return Error::from_errno(EIO); + })); + TRY(setup_queues(1)); finish_init(); m_entropy_buffer = TRY(MM.allocate_contiguous_kernel_region(PAGE_SIZE, "VirtIO::RNG"sv, Memory::Region::Access::ReadWrite)); memset(m_entropy_buffer->vaddr().as_ptr(), 0, m_entropy_buffer->size()); @@ -39,9 +35,9 @@ UNMAP_AFTER_INIT RNG::RNG(NonnullOwnPtr transport_entity) { } -bool RNG::handle_device_config_change() +ErrorOr RNG::handle_device_config_change() { - return false; // Device has no config + return Error::from_errno(EIO); // Device has no config } void RNG::handle_queue_update(u16 queue_index) diff --git a/Kernel/Bus/VirtIO/RNG.h b/Kernel/Bus/VirtIO/RNG.h index ccd2f75eea..55b26b1436 100644 --- a/Kernel/Bus/VirtIO/RNG.h +++ b/Kernel/Bus/VirtIO/RNG.h @@ -27,7 +27,7 @@ public: private: virtual StringView class_name() const override { return "VirtIORNG"sv; } explicit RNG(NonnullOwnPtr); - virtual bool handle_device_config_change() override; + virtual ErrorOr handle_device_config_change() override; virtual void handle_queue_update(u16 queue_index) override; void request_entropy_from_host(); diff --git a/Kernel/Bus/VirtIO/Transport/Entity.cpp b/Kernel/Bus/VirtIO/Transport/Entity.cpp index ae503c32fa..50097aa456 100644 --- a/Kernel/Bus/VirtIO/Transport/Entity.cpp +++ b/Kernel/Bus/VirtIO/Transport/Entity.cpp @@ -135,16 +135,16 @@ void TransportEntity::notify_queue(Badge, NotifyQueueDescriptor config_write16(*m_notify_cfg, descriptor.possible_notify_offset * m_notify_multiplier, descriptor.queue_index); } -bool TransportEntity::activate_queue(Badge, u16 queue_index) +ErrorOr TransportEntity::activate_queue(Badge, u16 queue_index) { if (!m_common_cfg) - return false; + return Error::from_errno(ENXIO); config_write16(*m_common_cfg, COMMON_CFG_QUEUE_SELECT, queue_index); config_write16(*m_common_cfg, COMMON_CFG_QUEUE_ENABLE, true); dbgln_if(VIRTIO_DEBUG, "Queue[{}] activated", queue_index); - return true; + return {}; } u64 TransportEntity::get_device_features() diff --git a/Kernel/Bus/VirtIO/Transport/Entity.h b/Kernel/Bus/VirtIO/Transport/Entity.h index 55a9d18da2..fb7fff7d04 100644 --- a/Kernel/Bus/VirtIO/Transport/Entity.h +++ b/Kernel/Bus/VirtIO/Transport/Entity.h @@ -30,7 +30,7 @@ public: u16 possible_notify_offset; }; void notify_queue(Badge, NotifyQueueDescriptor); - bool activate_queue(Badge, u16 queue_index); + ErrorOr activate_queue(Badge, u16 queue_index); ErrorOr> setup_queue(Badge, u16 queue_index); void set_status_bits(Badge, u8 status_bits); void reset_device(Badge); diff --git a/Kernel/Devices/GPU/VirtIO/GraphicsAdapter.cpp b/Kernel/Devices/GPU/VirtIO/GraphicsAdapter.cpp index 62cc40de8d..e9595aac2a 100644 --- a/Kernel/Devices/GPU/VirtIO/GraphicsAdapter.cpp +++ b/Kernel/Devices/GPU/VirtIO/GraphicsAdapter.cpp @@ -150,7 +150,7 @@ ErrorOr VirtIOGraphicsAdapter::initialize_virtio_resources() TRY(VirtIO::Device::initialize_virtio_resources()); auto* config = TRY(transport_entity().get_config(VirtIO::ConfigurationType::Device)); m_device_configuration = config; - bool success = negotiate_features([&](u64 supported_features) { + TRY(negotiate_features([&](u64 supported_features) { u64 negotiated = 0; if (is_feature_set(supported_features, VIRTIO_GPU_F_VIRGL)) { dbgln_if(VIRTIO_DEBUG, "VirtIO::GraphicsAdapter: VirGL is available, enabling"); @@ -160,21 +160,17 @@ ErrorOr VirtIOGraphicsAdapter::initialize_virtio_resources() if (is_feature_set(supported_features, VIRTIO_GPU_F_EDID)) negotiated |= VIRTIO_GPU_F_EDID; return negotiated; + })); + transport_entity().read_config_atomic([&]() { + m_num_scanouts = transport_entity().config_read32(*config, DEVICE_NUM_SCANOUTS); }); - if (success) { - transport_entity().read_config_atomic([&]() { - m_num_scanouts = transport_entity().config_read32(*config, DEVICE_NUM_SCANOUTS); - }); - dbgln_if(VIRTIO_DEBUG, "VirtIO::GraphicsAdapter: num_scanouts: {}", m_num_scanouts); - success = setup_queues(2); // CONTROLQ + CURSORQ - } - if (!success) - return Error::from_errno(EIO); + dbgln_if(VIRTIO_DEBUG, "VirtIO::GraphicsAdapter: num_scanouts: {}", m_num_scanouts); + TRY(setup_queues(2)); // CONTROLQ + CURSORQ finish_init(); return {}; } -bool VirtIOGraphicsAdapter::handle_device_config_change() +ErrorOr VirtIOGraphicsAdapter::handle_device_config_change() { auto events = get_pending_events(); if (events & VIRTIO_GPU_EVENT_DISPLAY) { @@ -184,9 +180,9 @@ bool VirtIOGraphicsAdapter::handle_device_config_change() } if (events & ~VIRTIO_GPU_EVENT_DISPLAY) { dbgln("VirtIO::GraphicsAdapter: Got unknown device config change event: {:#x}", events); - return false; + return Error::from_errno(EIO); } - return true; + return {}; } void VirtIOGraphicsAdapter::handle_queue_update(u16) diff --git a/Kernel/Devices/GPU/VirtIO/GraphicsAdapter.h b/Kernel/Devices/GPU/VirtIO/GraphicsAdapter.h index 2bb5d7bb83..bfcee9c56b 100644 --- a/Kernel/Devices/GPU/VirtIO/GraphicsAdapter.h +++ b/Kernel/Devices/GPU/VirtIO/GraphicsAdapter.h @@ -69,7 +69,7 @@ private: ErrorOr initialize_adapter(); - virtual bool handle_device_config_change() override; + virtual ErrorOr handle_device_config_change() override; virtual void handle_queue_update(u16 queue_index) override; u32 get_pending_events(); void clear_pending_events(u32 event_bitmask); diff --git a/Kernel/Net/VirtIO/VirtIONetworkAdapter.cpp b/Kernel/Net/VirtIO/VirtIONetworkAdapter.cpp index 326e6d7101..c42dd6d19d 100644 --- a/Kernel/Net/VirtIO/VirtIONetworkAdapter.cpp +++ b/Kernel/Net/VirtIO/VirtIONetworkAdapter.cpp @@ -127,7 +127,7 @@ UNMAP_AFTER_INIT ErrorOr VirtIONetworkAdapter::initialize_virtio_resources TRY(Device::initialize_virtio_resources()); m_device_config = TRY(transport_entity().get_config(VirtIO::ConfigurationType::Device)); - bool success = negotiate_features([&](u64 supported_features) { + TRY(negotiate_features([&](u64 supported_features) { u64 negotiated = 0; if (is_feature_set(supported_features, VIRTIO_NET_F_STATUS)) negotiated |= VIRTIO_NET_F_STATUS; @@ -138,17 +138,10 @@ UNMAP_AFTER_INIT ErrorOr VirtIONetworkAdapter::initialize_virtio_resources if (is_feature_set(supported_features, VIRTIO_NET_F_MTU)) negotiated |= VIRTIO_NET_F_MTU; return negotiated; - }); - if (!success) - return Error::from_errno(EIO); + })); - success = handle_device_config_change(); - if (!success) - return Error::from_errno(EIO); - - success = setup_queues(2); // receive & transmit - if (!success) - return Error::from_errno(EIO); + TRY(handle_device_config_change()); + TRY(setup_queues(2)); // receive & transmit finish_init(); @@ -168,7 +161,7 @@ UNMAP_AFTER_INIT ErrorOr VirtIONetworkAdapter::initialize_virtio_resources return {}; } -bool VirtIONetworkAdapter::handle_device_config_change() +ErrorOr VirtIONetworkAdapter::handle_device_config_change() { dbgln_if(VIRTIO_DEBUG, "VirtIONetworkAdapter: handle_device_config_change"); transport_entity().read_config_atomic([&]() { @@ -196,7 +189,7 @@ bool VirtIONetworkAdapter::handle_device_config_change() m_link_duplex = duplex == 0x01; } }); - return true; + return {}; } void VirtIONetworkAdapter::handle_queue_update(u16 queue_index) diff --git a/Kernel/Net/VirtIO/VirtIONetworkAdapter.h b/Kernel/Net/VirtIO/VirtIONetworkAdapter.h index bc56e134fd..e830c4c67b 100644 --- a/Kernel/Net/VirtIO/VirtIONetworkAdapter.h +++ b/Kernel/Net/VirtIO/VirtIONetworkAdapter.h @@ -37,7 +37,7 @@ private: explicit VirtIONetworkAdapter(StringView interface_name, NonnullOwnPtr); // VirtIO::Device - virtual bool handle_device_config_change() override; + virtual ErrorOr handle_device_config_change() override; virtual void handle_queue_update(u16 queue_index) override; // NetworkAdapter