From 0050358cd331cc750570c53e6d544ced7cd52e41 Mon Sep 17 00:00:00 2001 From: Liav A Date: Tue, 11 Apr 2023 00:47:52 +0300 Subject: [PATCH] Kernel/Storage: Modernize ATA IDE controller initialization code This is done by 2 ways which both fit very well together: - We stop use LockRefPtrs. We also don't allow expansion of the m_channels member, by setting it to be a fixed Array of 2 IDEChannels. - More error propagation through the code, in the construction point of IDEChannel(s). This means that in the future we could technically do something meaningful with OOM conditions when initializing an IDE controller. --- Kernel/Arch/x86_64/ISABus/IDEController.cpp | 8 ++++---- Kernel/Arch/x86_64/PCI/IDELegacyModeController.cpp | 12 ++++++------ Kernel/Storage/ATA/GenericIDE/Channel.cpp | 8 ++++---- Kernel/Storage/ATA/GenericIDE/Channel.h | 4 ++-- Kernel/Storage/ATA/GenericIDE/Controller.h | 2 +- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/Kernel/Arch/x86_64/ISABus/IDEController.cpp b/Kernel/Arch/x86_64/ISABus/IDEController.cpp index 26826febab..da19c4dc43 100644 --- a/Kernel/Arch/x86_64/ISABus/IDEController.cpp +++ b/Kernel/Arch/x86_64/ISABus/IDEController.cpp @@ -42,12 +42,12 @@ UNMAP_AFTER_INIT ErrorOr ISAIDEController::initialize_channels() auto primary_channel_io_window_group = IDEChannel::IOWindowGroup { move(primary_base_io_window), move(primary_control_io_window) }; auto secondary_channel_io_window_group = IDEChannel::IOWindowGroup { move(secondary_base_io_window), move(secondary_control_io_window) }; - TRY(m_channels.try_append(IDEChannel::create(*this, move(primary_channel_io_window_group), IDEChannel::ChannelType::Primary))); - TRY(initialize_and_enumerate(m_channels[0])); + m_channels[0] = TRY(IDEChannel::create(*this, move(primary_channel_io_window_group), IDEChannel::ChannelType::Primary)); + TRY(initialize_and_enumerate(*m_channels[0])); m_channels[0]->enable_irq(); - TRY(m_channels.try_append(IDEChannel::create(*this, move(secondary_channel_io_window_group), IDEChannel::ChannelType::Secondary))); - TRY(initialize_and_enumerate(m_channels[1])); + m_channels[1] = TRY(IDEChannel::create(*this, move(secondary_channel_io_window_group), IDEChannel::ChannelType::Secondary)); + TRY(initialize_and_enumerate(*m_channels[1])); m_channels[1]->enable_irq(); dbgln("ISA IDE controller detected and initialized"); return {}; diff --git a/Kernel/Arch/x86_64/PCI/IDELegacyModeController.cpp b/Kernel/Arch/x86_64/PCI/IDELegacyModeController.cpp index 66191cafd1..7760bcb8eb 100644 --- a/Kernel/Arch/x86_64/PCI/IDELegacyModeController.cpp +++ b/Kernel/Arch/x86_64/PCI/IDELegacyModeController.cpp @@ -147,19 +147,19 @@ UNMAP_AFTER_INIT ErrorOr PCIIDELegacyModeController::initialize_and_enumer auto secondary_channel_io_window_group = IDEChannel::IOWindowGroup { secondary_base_io_window.release_nonnull(), secondary_control_io_window.release_nonnull(), move(secondary_bus_master_io) }; if (is_pci_native_mode_enabled_on_primary_channel()) { - TRY(m_channels.try_append(IDEChannel::create(*this, irq_line, move(primary_channel_io_window_group), IDEChannel::ChannelType::Primary))); + m_channels[0] = TRY(IDEChannel::create(*this, irq_line, move(primary_channel_io_window_group), IDEChannel::ChannelType::Primary)); } else { - TRY(m_channels.try_append(IDEChannel::create(*this, move(primary_channel_io_window_group), IDEChannel::ChannelType::Primary))); + m_channels[0] = TRY(IDEChannel::create(*this, move(primary_channel_io_window_group), IDEChannel::ChannelType::Primary)); } - TRY(initialize_and_enumerate(m_channels[0])); + TRY(initialize_and_enumerate(*m_channels[0])); m_channels[0]->enable_irq(); if (is_pci_native_mode_enabled_on_secondary_channel()) { - TRY(m_channels.try_append(IDEChannel::create(*this, irq_line, move(secondary_channel_io_window_group), IDEChannel::ChannelType::Secondary))); + m_channels[1] = TRY(IDEChannel::create(*this, irq_line, move(secondary_channel_io_window_group), IDEChannel::ChannelType::Secondary)); } else { - TRY(m_channels.try_append(IDEChannel::create(*this, move(secondary_channel_io_window_group), IDEChannel::ChannelType::Secondary))); + m_channels[1] = TRY(IDEChannel::create(*this, move(secondary_channel_io_window_group), IDEChannel::ChannelType::Secondary)); } - TRY(initialize_and_enumerate(m_channels[1])); + TRY(initialize_and_enumerate(*m_channels[1])); m_channels[1]->enable_irq(); return {}; } diff --git a/Kernel/Storage/ATA/GenericIDE/Channel.cpp b/Kernel/Storage/ATA/GenericIDE/Channel.cpp index 35ff62cfda..9d41a2ba4c 100644 --- a/Kernel/Storage/ATA/GenericIDE/Channel.cpp +++ b/Kernel/Storage/ATA/GenericIDE/Channel.cpp @@ -24,16 +24,16 @@ namespace Kernel { #define PATA_PRIMARY_IRQ 14 #define PATA_SECONDARY_IRQ 15 -UNMAP_AFTER_INIT NonnullLockRefPtr IDEChannel::create(IDEController const& controller, IOWindowGroup io_window_group, ChannelType type) +UNMAP_AFTER_INIT ErrorOr> IDEChannel::create(IDEController const& controller, IOWindowGroup io_window_group, ChannelType type) { auto ata_identify_data_buffer = KBuffer::try_create_with_size("ATA Identify Page"sv, 4096, Memory::Region::Access::ReadWrite, AllocationStrategy::AllocateNow).release_value(); - return adopt_lock_ref(*new IDEChannel(controller, move(io_window_group), type, move(ata_identify_data_buffer))); + return adopt_nonnull_ref_or_enomem(new (nothrow) IDEChannel(controller, move(io_window_group), type, move(ata_identify_data_buffer))); } -UNMAP_AFTER_INIT NonnullLockRefPtr IDEChannel::create(IDEController const& controller, u8 irq, IOWindowGroup io_window_group, ChannelType type) +UNMAP_AFTER_INIT ErrorOr> IDEChannel::create(IDEController const& controller, u8 irq, IOWindowGroup io_window_group, ChannelType type) { auto ata_identify_data_buffer = KBuffer::try_create_with_size("ATA Identify Page"sv, 4096, Memory::Region::Access::ReadWrite, AllocationStrategy::AllocateNow).release_value(); - return adopt_lock_ref(*new IDEChannel(controller, irq, move(io_window_group), type, move(ata_identify_data_buffer))); + return adopt_nonnull_ref_or_enomem(new (nothrow) IDEChannel(controller, irq, move(io_window_group), type, move(ata_identify_data_buffer))); } StringView IDEChannel::channel_type_string() const diff --git a/Kernel/Storage/ATA/GenericIDE/Channel.h b/Kernel/Storage/ATA/GenericIDE/Channel.h index f6b3d5829f..6eaa369e87 100644 --- a/Kernel/Storage/ATA/GenericIDE/Channel.h +++ b/Kernel/Storage/ATA/GenericIDE/Channel.h @@ -88,8 +88,8 @@ public: }; public: - static NonnullLockRefPtr create(IDEController const&, IOWindowGroup, ChannelType type); - static NonnullLockRefPtr create(IDEController const&, u8 irq, IOWindowGroup, ChannelType type); + static ErrorOr> create(IDEController const&, IOWindowGroup, ChannelType type); + static ErrorOr> create(IDEController const&, u8 irq, IOWindowGroup, ChannelType type); virtual ~IDEChannel() override; diff --git a/Kernel/Storage/ATA/GenericIDE/Controller.h b/Kernel/Storage/ATA/GenericIDE/Controller.h index 59ce472de2..56f6b378a1 100644 --- a/Kernel/Storage/ATA/GenericIDE/Controller.h +++ b/Kernel/Storage/ATA/GenericIDE/Controller.h @@ -31,6 +31,6 @@ protected: IDEController(); LockRefPtr device_by_channel_and_position(u32 index) const; - Vector> m_channels; + Array, 2> m_channels; }; }