Ver Fonte

Kernel/Storage: Unify all ATA devices

There's basically no real difference in software between a SATA harddisk
and IDE harddisk. The difference in the implementation is for the host
bus adapter protocol and registers layout.
Therefore, there's no point in putting a distinction in software to
these devices.

This change also greatly simplifies and removes stale APIs and removes
unnecessary parameters in constructor calls, which tighten things
further everywhere.
Liav A há 3 anos atrás
pai
commit
741c871bc1

+ 3 - 2
Kernel/CMakeLists.txt

@@ -93,11 +93,12 @@ set(KERNEL_SOURCES
     Storage/AHCIController.cpp
     Storage/AHCIPort.cpp
     Storage/AHCIPortHandler.cpp
-    Storage/SATADiskDevice.cpp
+    Storage/ATADevice.cpp
+    Storage/ATADiskDevice.cpp
+    Storage/ATAPIDiscDevice.cpp
     Storage/BMIDEChannel.cpp
     Storage/IDEController.cpp
     Storage/IDEChannel.cpp
-    Storage/PATADiskDevice.cpp
     Storage/RamdiskController.cpp
     Storage/RamdiskDevice.cpp
     Storage/StorageManagement.cpp

+ 8 - 4
Kernel/Storage/AHCIController.cpp

@@ -11,7 +11,7 @@
 #include <Kernel/Bus/PCI/API.h>
 #include <Kernel/Memory/MemoryManager.h>
 #include <Kernel/Storage/AHCIController.h>
-#include <Kernel/Storage/SATADiskDevice.h>
+#include <Kernel/Storage/AHCIPortHandler.h>
 
 namespace Kernel {
 
@@ -58,9 +58,13 @@ size_t AHCIController::devices_count() const
     return count;
 }
 
-void AHCIController::start_request(const StorageDevice&, AsyncBlockDeviceRequest&)
+void AHCIController::start_request(const ATADevice& device, AsyncBlockDeviceRequest& request)
 {
-    VERIFY_NOT_REACHED();
+    // FIXME: For now we have one port handler, check all of them...
+    VERIFY(m_handlers.size() > 0);
+    auto port = m_handlers[0].port_at_index(device.ata_address().port);
+    VERIFY(port);
+    port->start_request(request);
 }
 
 void AHCIController::complete_current_request(AsyncDeviceRequest::RequestResult)
@@ -80,7 +84,7 @@ volatile AHCI::HBA& AHCIController::hba() const
 }
 
 AHCIController::AHCIController(PCI::DeviceIdentifier const& pci_device_identifier)
-    : StorageController()
+    : ATAController()
     , PCI::Device(pci_device_identifier.address())
     , m_hba_region(default_hba_region())
     , m_capabilities(capabilities())

+ 3 - 3
Kernel/Storage/AHCIController.h

@@ -11,7 +11,7 @@
 #include <AK/Types.h>
 #include <Kernel/Sections.h>
 #include <Kernel/Storage/AHCI.h>
-#include <Kernel/Storage/StorageController.h>
+#include <Kernel/Storage/ATAController.h>
 #include <Kernel/Storage/StorageDevice.h>
 
 namespace Kernel {
@@ -19,7 +19,7 @@ namespace Kernel {
 class AsyncBlockDeviceRequest;
 class AHCIPortHandler;
 class AHCIPort;
-class AHCIController final : public StorageController
+class AHCIController final : public ATAController
     , public PCI::Device {
     friend class AHCIPortHandler;
     friend class AHCIPort;
@@ -32,7 +32,7 @@ public:
     virtual bool reset() override;
     virtual bool shutdown() override;
     virtual size_t devices_count() const override;
-    virtual void start_request(const StorageDevice&, AsyncBlockDeviceRequest&) override;
+    virtual void start_request(const ATADevice&, AsyncBlockDeviceRequest&) override;
     virtual void complete_current_request(AsyncDeviceRequest::RequestResult) override;
 
     const AHCI::HBADefinedCapabilities& hba_capabilities() const { return m_capabilities; };

+ 2 - 2
Kernel/Storage/AHCIPort.cpp

@@ -14,7 +14,7 @@
 #include <Kernel/Memory/TypedMapping.h>
 #include <Kernel/Storage/AHCIPort.h>
 #include <Kernel/Storage/ATA.h>
-#include <Kernel/Storage/SATADiskDevice.h>
+#include <Kernel/Storage/ATADiskDevice.h>
 #include <Kernel/Storage/StorageManagement.h>
 #include <Kernel/WorkQueue.h>
 
@@ -319,7 +319,7 @@ bool AHCIPort::initialize(SpinlockLocker<Spinlock>& main_lock)
 
         // FIXME: We don't support ATAPI devices yet, so for now we don't "create" them
         if (!is_atapi_attached()) {
-            m_connected_device = SATADiskDevice::create(m_parent_handler->hba_controller(), *this, logical_sector_size, max_addressable_sector);
+            m_connected_device = ATADiskDevice::create(m_parent_handler->hba_controller(), { m_port_index, 0 }, 0, logical_sector_size, max_addressable_sector);
         } else {
             dbgln("AHCI Port {}: Ignoring ATAPI devices for now as we don't currently support them.", representative_port_index());
         }

+ 5 - 5
Kernel/Storage/AHCIPort.h

@@ -22,7 +22,7 @@
 #include <Kernel/Sections.h>
 #include <Kernel/Storage/AHCI.h>
 #include <Kernel/Storage/AHCIPortHandler.h>
-#include <Kernel/Storage/StorageDevice.h>
+#include <Kernel/Storage/ATADevice.h>
 #include <Kernel/WaitQueue.h>
 
 namespace Kernel {
@@ -30,11 +30,11 @@ namespace Kernel {
 class AsyncBlockDeviceRequest;
 
 class AHCIPortHandler;
-class SATADiskDevice;
-class AHCIPort : public RefCounted<AHCIPort>
+class AHCIPort
+    : public RefCounted<AHCIPort>
     , public Weakable<AHCIPort> {
     friend class AHCIPortHandler;
-    friend class SATADiskDevice;
+    friend class AHCIController;
 
 public:
     UNMAP_AFTER_INIT static NonnullRefPtr<AHCIPort> create(const AHCIPortHandler&, volatile AHCI::PortRegisters&, u32 port_index);
@@ -114,7 +114,7 @@ private:
     RefPtr<Memory::PhysicalPage> m_command_list_page;
     OwnPtr<Memory::Region> m_command_list_region;
     RefPtr<Memory::PhysicalPage> m_fis_receive_page;
-    RefPtr<StorageDevice> m_connected_device;
+    RefPtr<ATADevice> m_connected_device;
 
     u32 m_port_index;
     volatile AHCI::PortRegisters& m_port_registers;

+ 0 - 1
Kernel/Storage/AHCIPortHandler.h

@@ -29,7 +29,6 @@ class AHCIPort;
 class AHCIPortHandler final : public RefCounted<AHCIPortHandler>
     , public IRQHandler {
     friend class AHCIController;
-    friend class SATADiskDevice;
 
 public:
     UNMAP_AFTER_INIT static NonnullRefPtr<AHCIPortHandler> create(AHCIController&, u8 irq, AHCI::MaskedBitField taken_ports);

+ 29 - 0
Kernel/Storage/ATAController.h

@@ -0,0 +1,29 @@
+/*
+ * Copyright (c) 2021, Liav A. <liavalb@hotmail.co.il>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#pragma once
+
+#include <AK/OwnPtr.h>
+#include <AK/RefPtr.h>
+#include <AK/Types.h>
+#include <Kernel/Devices/BlockDevice.h>
+#include <Kernel/Storage/StorageController.h>
+
+namespace Kernel {
+
+class AsyncBlockDeviceRequest;
+
+class ATADevice;
+class ATAController
+    : public StorageController
+    , public Weakable<ATAController> {
+public:
+    virtual void start_request(const ATADevice&, AsyncBlockDeviceRequest&) = 0;
+
+protected:
+    ATAController() = default;
+};
+}

+ 35 - 0
Kernel/Storage/ATADevice.cpp

@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) 2021, Liav A. <liavalb@hotmail.co.il>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include <AK/StringView.h>
+#include <Kernel/Sections.h>
+#include <Kernel/Storage/ATADevice.h>
+#include <Kernel/Storage/IDEChannel.h>
+#include <Kernel/Storage/IDEController.h>
+#include <Kernel/Storage/StorageManagement.h>
+
+namespace Kernel {
+
+ATADevice::ATADevice(const ATAController& controller, ATADevice::Address ata_address, unsigned minor_number, u16 capabilities, u16 logical_sector_size, u64 max_addressable_block, NonnullOwnPtr<KString> early_storage_name)
+    : StorageDevice(StorageManagement::major_number(), minor_number, logical_sector_size, max_addressable_block, move(early_storage_name))
+    , m_controller(controller)
+    , m_ata_address(ata_address)
+    , m_capabilities(capabilities)
+{
+}
+
+ATADevice::~ATADevice()
+{
+}
+
+void ATADevice::start_request(AsyncBlockDeviceRequest& request)
+{
+    auto controller = m_controller.strong_ref();
+    VERIFY(controller);
+    controller->start_request(*this, request);
+}
+
+}

+ 46 - 0
Kernel/Storage/ATADevice.h

@@ -0,0 +1,46 @@
+/*
+ * Copyright (c) 2021, Liav A. <liavalb@hotmail.co.il>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#pragma once
+
+#include <Kernel/Interrupts/IRQHandler.h>
+#include <Kernel/Locking/Mutex.h>
+#include <Kernel/Storage/ATAController.h>
+#include <Kernel/Storage/StorageDevice.h>
+
+namespace Kernel {
+
+class ATADevice : public StorageDevice {
+public:
+    // Note: For IDE drives, port means Primary or Secondary (0 or 1),
+    // and subport means Master or Slave (0 or 1).
+    // For SATA drives (AHCI driven HBAs), a port can be a number from 0 to 31,
+    // and subport can be a number from 0 to 14 (only 15 devices are allowed to
+    // be connected to one SATA port multiplier).
+    struct Address {
+        // FIXME: u32 for this value is wasteful, because even AHCI only support 32 ports
+        u32 port;
+        u8 subport;
+    };
+
+public:
+    virtual ~ATADevice() override;
+
+    // ^BlockDevice
+    virtual void start_request(AsyncBlockDeviceRequest&) override;
+
+    u16 ata_capabilites() const { return m_capabilities; }
+    const Address& ata_address() const { return m_ata_address; }
+
+protected:
+    ATADevice(const ATAController&, Address, unsigned, u16, u16, u64, NonnullOwnPtr<KString>);
+
+    WeakPtr<ATAController> m_controller;
+    const Address m_ata_address;
+    const u16 m_capabilities;
+};
+
+}

+ 45 - 0
Kernel/Storage/ATADiskDevice.cpp

@@ -0,0 +1,45 @@
+/*
+ * Copyright (c) 2021, Liav A. <liavalb@hotmail.co.il>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include <AK/StringView.h>
+#include <Kernel/Devices/DeviceManagement.h>
+#include <Kernel/Sections.h>
+#include <Kernel/Storage/ATADiskDevice.h>
+#include <Kernel/Storage/IDEChannel.h>
+#include <Kernel/Storage/IDEController.h>
+#include <Kernel/Storage/StorageManagement.h>
+
+namespace Kernel {
+
+NonnullRefPtr<ATADiskDevice> ATADiskDevice::create(const ATAController& controller, ATADevice::Address ata_address, u16 capabilities, u16 logical_sector_size, u64 max_addressable_block)
+{
+    auto minor_device_number = StorageManagement::minor_number();
+
+    // FIXME: We need a way of formatting strings with KString.
+    auto device_name = String::formatted("hd{:c}", 'a' + minor_device_number);
+    auto device_name_kstring = KString::must_create(device_name.view());
+
+    auto disk_device_or_error = DeviceManagement::try_create_device<ATADiskDevice>(controller, ata_address, minor_device_number, capabilities, logical_sector_size, max_addressable_block, move(device_name_kstring));
+    // FIXME: Find a way to propagate errors
+    VERIFY(!disk_device_or_error.is_error());
+    return disk_device_or_error.release_value();
+}
+
+ATADiskDevice::ATADiskDevice(const ATAController& controller, ATADevice::Address ata_address, unsigned minor_number, u16 capabilities, u16 logical_sector_size, u64 max_addressable_block, NonnullOwnPtr<KString> early_storage_name)
+    : ATADevice(controller, ata_address, minor_number, capabilities, logical_sector_size, max_addressable_block, move(early_storage_name))
+{
+}
+
+ATADiskDevice::~ATADiskDevice()
+{
+}
+
+StringView ATADiskDevice::class_name() const
+{
+    return "ATADiskDevice";
+}
+
+}

+ 35 - 0
Kernel/Storage/ATADiskDevice.h

@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) 2021, Liav A. <liavalb@hotmail.co.il>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#pragma once
+
+#include <Kernel/Interrupts/IRQHandler.h>
+#include <Kernel/Locking/Mutex.h>
+#include <Kernel/Storage/ATADevice.h>
+
+namespace Kernel {
+
+class IDEController;
+class IDEChannel;
+class ATADiskDevice final : public ATADevice {
+    friend class IDEController;
+    friend class DeviceManagement;
+
+public:
+    static NonnullRefPtr<ATADiskDevice> create(const ATAController&, ATADevice::Address, u16 capabilities, u16 logical_sector_size, u64 max_addressable_block);
+    virtual ~ATADiskDevice() override;
+
+    // ^StorageDevice
+    virtual CommandSet command_set() const override { return CommandSet::ATA; }
+
+private:
+    ATADiskDevice(const ATAController&, Address, unsigned, u16, u16, u64, NonnullOwnPtr<KString>);
+
+    // ^DiskDevice
+    virtual StringView class_name() const override;
+};
+
+}

+ 45 - 0
Kernel/Storage/ATAPIDiscDevice.cpp

@@ -0,0 +1,45 @@
+/*
+ * Copyright (c) 2021, Liav A. <liavalb@hotmail.co.il>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include <AK/StringView.h>
+#include <Kernel/Devices/DeviceManagement.h>
+#include <Kernel/Sections.h>
+#include <Kernel/Storage/ATAPIDiscDevice.h>
+#include <Kernel/Storage/IDEChannel.h>
+#include <Kernel/Storage/IDEController.h>
+#include <Kernel/Storage/StorageManagement.h>
+
+namespace Kernel {
+
+NonnullRefPtr<ATAPIDiscDevice> ATAPIDiscDevice::create(const ATAController& controller, ATADevice::Address ata_address, u16 capabilities, u64 max_addressable_block)
+{
+    auto minor_device_number = StorageManagement::minor_number();
+
+    // FIXME: We need a way of formatting strings with KString.
+    auto device_name = String::formatted("hd{:c}", 'a' + minor_device_number);
+    auto device_name_kstring = KString::must_create(device_name.view());
+
+    auto disc_device_or_error = DeviceManagement::try_create_device<ATAPIDiscDevice>(controller, ata_address, minor_device_number, capabilities, max_addressable_block, move(device_name_kstring));
+    // FIXME: Find a way to propagate errors
+    VERIFY(!disc_device_or_error.is_error());
+    return disc_device_or_error.release_value();
+}
+
+ATAPIDiscDevice::ATAPIDiscDevice(const ATAController& controller, ATADevice::Address ata_address, unsigned minor_number, u16 capabilities, u64 max_addressable_block, NonnullOwnPtr<KString> early_storage_name)
+    : ATADevice(controller, ata_address, minor_number, capabilities, 0, max_addressable_block, move(early_storage_name))
+{
+}
+
+ATAPIDiscDevice::~ATAPIDiscDevice()
+{
+}
+
+StringView ATAPIDiscDevice::class_name() const
+{
+    return "ATAPIDiscDevice";
+}
+
+}

+ 35 - 0
Kernel/Storage/ATAPIDiscDevice.h

@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) 2021, Liav A. <liavalb@hotmail.co.il>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#pragma once
+
+#include <Kernel/Interrupts/IRQHandler.h>
+#include <Kernel/Locking/Mutex.h>
+#include <Kernel/Storage/ATADevice.h>
+
+namespace Kernel {
+
+class IDEController;
+class IDEChannel;
+class ATAPIDiscDevice final : public ATADevice {
+    friend class IDEController;
+    friend class DeviceManagement;
+
+public:
+    static NonnullRefPtr<ATAPIDiscDevice> create(const ATAController&, ATADevice::Address, u16 capabilities, u64 max_addressable_block);
+    virtual ~ATAPIDiscDevice() override;
+
+    // ^StorageDevice
+    virtual CommandSet command_set() const override { return CommandSet::SCSI; }
+
+private:
+    ATAPIDiscDevice(const ATAController&, Address, unsigned, u16, u64, NonnullOwnPtr<KString>);
+
+    // ^DiskDevice
+    virtual StringView class_name() const override;
+};
+
+}

+ 8 - 7
Kernel/Storage/IDEChannel.cpp

@@ -12,9 +12,9 @@
 #include <Kernel/Process.h>
 #include <Kernel/Sections.h>
 #include <Kernel/Storage/ATA.h>
+#include <Kernel/Storage/ATADiskDevice.h>
 #include <Kernel/Storage/IDEChannel.h>
 #include <Kernel/Storage/IDEController.h>
-#include <Kernel/Storage/PATADiskDevice.h>
 #include <Kernel/WorkQueue.h>
 
 namespace Kernel {
@@ -328,7 +328,7 @@ UNMAP_AFTER_INIT void IDEChannel::detect_disks()
 
         bool check_for_atapi = false;
         bool device_presence = true;
-        PATADiskDevice::InterfaceType interface_type = PATADiskDevice::InterfaceType::ATA;
+        bool command_set_is_atapi = false;
 
         size_t milliseconds_elapsed = 0;
         for (;;) {
@@ -344,7 +344,6 @@ UNMAP_AFTER_INIT void IDEChannel::detect_disks()
 
             if (!(status & ATA_SR_BSY) && (status & ATA_SR_DRQ)) {
                 dbgln_if(PATA_DEBUG, "IDEChannel: {} {} device appears to be ATA.", channel_type_string(), channel_string(i));
-                interface_type = PATADiskDevice::InterfaceType::ATA;
                 break;
             }
 
@@ -370,7 +369,7 @@ UNMAP_AFTER_INIT void IDEChannel::detect_disks()
             u8 ch = m_io_group.io_base().offset(ATA_REG_LBA2).in<u8>();
 
             if ((cl == 0x14 && ch == 0xEB) || (cl == 0x69 && ch == 0x96)) {
-                interface_type = PATADiskDevice::InterfaceType::ATAPI;
+                command_set_is_atapi = true;
                 dbgln("IDEChannel: {} {} device appears to be ATAPI. We're going to ignore it for now as we don't support it.", channel_type_string(), channel_string(i));
                 continue;
             } else {
@@ -409,11 +408,13 @@ UNMAP_AFTER_INIT void IDEChannel::detect_disks()
         if (identify_block.commands_and_feature_sets_supported[1] & (1 << 10))
             max_addressable_block = identify_block.user_addressable_logical_sectors_count;
 
-        dbgln("IDEChannel: {} {} {} device found: Name={}, Capacity={}, Capabilities={:#04x}", channel_type_string(), channel_string(i), interface_type == PATADiskDevice::InterfaceType::ATA ? "ATA" : "ATAPI", ((char*)bbuf.data() + 54), max_addressable_block * 512, capabilities);
+        dbgln("IDEChannel: {} {} {} device found: Name={}, Capacity={}, Capabilities={:#04x}", channel_type_string(), channel_string(i), !command_set_is_atapi ? "ATA" : "ATAPI", ((char*)bbuf.data() + 54), max_addressable_block * 512, capabilities);
+        // FIXME: Don't assume all drives will have logical sector size of 512 bytes.
+        ATADevice::Address address = { m_channel_type == ChannelType::Primary ? static_cast<u8>(0) : static_cast<u8>(1), static_cast<u8>(i) };
         if (i == 0) {
-            m_master = PATADiskDevice::create(m_parent_controller, *this, PATADiskDevice::DriveType::Master, interface_type, capabilities, max_addressable_block);
+            m_master = ATADiskDevice::create(m_parent_controller, address, capabilities, 512, max_addressable_block);
         } else {
-            m_slave = PATADiskDevice::create(m_parent_controller, *this, PATADiskDevice::DriveType::Slave, interface_type, capabilities, max_addressable_block);
+            m_slave = ATADiskDevice::create(m_parent_controller, address, capabilities, 512, max_addressable_block);
         }
     }
 }

+ 3 - 3
Kernel/Storage/IDEChannel.h

@@ -25,6 +25,7 @@
 #include <Kernel/Memory/PhysicalPage.h>
 #include <Kernel/PhysicalAddress.h>
 #include <Kernel/Random.h>
+#include <Kernel/Storage/ATADevice.h>
 #include <Kernel/Storage/StorageDevice.h>
 #include <Kernel/WaitQueue.h>
 
@@ -36,7 +37,6 @@ class IDEController;
 class IDEChannel : public RefCounted<IDEChannel>
     , public IRQHandler {
     friend class IDEController;
-    friend class PATADiskDevice;
     AK_MAKE_ETERNAL
 public:
     enum class ChannelType : u8 {
@@ -151,8 +151,8 @@ protected:
     volatile u8 m_device_error { 0 };
     EntropySource m_entropy_source;
 
-    RefPtr<StorageDevice> m_master;
-    RefPtr<StorageDevice> m_slave;
+    RefPtr<ATADevice> m_master;
+    RefPtr<ATADevice> m_slave;
 
     RefPtr<AsyncBlockDeviceRequest> m_current_request;
     u64 m_current_request_block_index { 0 };

+ 13 - 3
Kernel/Storage/IDEController.cpp

@@ -10,9 +10,9 @@
 #include <Kernel/Bus/PCI/API.h>
 #include <Kernel/FileSystem/ProcFS.h>
 #include <Kernel/Sections.h>
+#include <Kernel/Storage/ATADiskDevice.h>
 #include <Kernel/Storage/BMIDEChannel.h>
 #include <Kernel/Storage/IDEController.h>
-#include <Kernel/Storage/PATADiskDevice.h>
 
 namespace Kernel {
 
@@ -41,8 +41,18 @@ size_t IDEController::devices_count() const
     return count;
 }
 
-void IDEController::start_request(const StorageDevice&, AsyncBlockDeviceRequest&)
+void IDEController::start_request(const ATADevice& device, AsyncBlockDeviceRequest& request)
 {
+    auto& address = device.ata_address();
+    VERIFY(address.subport < 2);
+    switch (address.port) {
+    case 0:
+        m_channels[0].start_request(request, address.subport == 0 ? false : true, device.ata_capabilites());
+        return;
+    case 1:
+        m_channels[1].start_request(request, address.subport == 0 ? false : true, device.ata_capabilites());
+        return;
+    }
     VERIFY_NOT_REACHED();
 }
 
@@ -52,7 +62,7 @@ void IDEController::complete_current_request(AsyncDeviceRequest::RequestResult)
 }
 
 UNMAP_AFTER_INIT IDEController::IDEController(PCI::DeviceIdentifier const& device_identifier, bool force_pio)
-    : StorageController()
+    : ATAController()
     , PCI::Device(device_identifier.address())
     , m_prog_if(device_identifier.prog_if())
     , m_interrupt_line(device_identifier.interrupt_line())

+ 3 - 3
Kernel/Storage/IDEController.h

@@ -9,15 +9,15 @@
 #include <AK/OwnPtr.h>
 #include <AK/RefPtr.h>
 #include <AK/Types.h>
+#include <Kernel/Storage/ATAController.h>
 #include <Kernel/Storage/IDEChannel.h>
-#include <Kernel/Storage/StorageController.h>
 #include <Kernel/Storage/StorageDevice.h>
 
 namespace Kernel {
 
 class AsyncBlockDeviceRequest;
 
-class IDEController final : public StorageController
+class IDEController final : public ATAController
     , public PCI::Device {
     AK_MAKE_ETERNAL
 public:
@@ -28,7 +28,7 @@ public:
     virtual bool reset() override;
     virtual bool shutdown() override;
     virtual size_t devices_count() const override;
-    virtual void start_request(const StorageDevice&, AsyncBlockDeviceRequest&) override;
+    virtual void start_request(const ATADevice&, AsyncBlockDeviceRequest&) override;
     virtual void complete_current_request(AsyncDeviceRequest::RequestResult) override;
 
     bool is_bus_master_capable() const;

+ 0 - 60
Kernel/Storage/PATADiskDevice.cpp

@@ -1,60 +0,0 @@
-/*
- * Copyright (c) 2018-2020, Andreas Kling <kling@serenityos.org>
- *
- * SPDX-License-Identifier: BSD-2-Clause
- */
-
-#include <AK/StringView.h>
-#include <Kernel/Devices/DeviceManagement.h>
-#include <Kernel/FileSystem/OpenFileDescription.h>
-#include <Kernel/Sections.h>
-#include <Kernel/Storage/IDEChannel.h>
-#include <Kernel/Storage/IDEController.h>
-#include <Kernel/Storage/PATADiskDevice.h>
-#include <Kernel/Storage/StorageManagement.h>
-
-namespace Kernel {
-
-UNMAP_AFTER_INIT NonnullRefPtr<PATADiskDevice> PATADiskDevice::create(const IDEController& controller, IDEChannel& channel, DriveType type, InterfaceType interface_type, u16 capabilities, u64 max_addressable_block)
-{
-    auto minor_device_number = StorageManagement::minor_number();
-
-    // FIXME: We need a way of formatting strings with KString.
-    auto device_name = String::formatted("hd{:c}", 'a' + minor_device_number);
-    auto device_name_kstring = KString::must_create(device_name.view());
-
-    auto device_or_error = DeviceManagement::try_create_device<PATADiskDevice>(controller, channel, type, interface_type, capabilities, max_addressable_block, minor_device_number, move(device_name_kstring));
-    // FIXME: Find a way to propagate errors
-    VERIFY(!device_or_error.is_error());
-    return device_or_error.release_value();
-}
-
-UNMAP_AFTER_INIT PATADiskDevice::PATADiskDevice(const IDEController& controller, IDEChannel& channel, DriveType type, InterfaceType interface_type, u16 capabilities, u64 max_addressable_block, int minor_device_number, NonnullOwnPtr<KString> device_name)
-    : StorageDevice(controller, StorageManagement::major_number(), minor_device_number, 512, max_addressable_block, move(device_name))
-    , m_capabilities(capabilities)
-    , m_channel(channel)
-    , m_drive_type(type)
-    , m_interface_type(interface_type)
-{
-}
-
-UNMAP_AFTER_INIT PATADiskDevice::~PATADiskDevice()
-{
-}
-
-StringView PATADiskDevice::class_name() const
-{
-    return "PATADiskDevice"sv;
-}
-
-void PATADiskDevice::start_request(AsyncBlockDeviceRequest& request)
-{
-    m_channel->start_request(request, is_slave(), m_capabilities);
-}
-
-bool PATADiskDevice::is_slave() const
-{
-    return m_drive_type == DriveType::Slave;
-}
-
-}

+ 0 - 61
Kernel/Storage/PATADiskDevice.h

@@ -1,61 +0,0 @@
-/*
- * Copyright (c) 2018-2020, Andreas Kling <kling@serenityos.org>
- *
- * SPDX-License-Identifier: BSD-2-Clause
- */
-
-//
-// A Disk Device Connected to a PATA Channel
-//
-
-#pragma once
-
-#include <Kernel/Interrupts/IRQHandler.h>
-#include <Kernel/Locking/Mutex.h>
-#include <Kernel/Storage/StorageDevice.h>
-
-namespace Kernel {
-
-class IDEController;
-class IDEChannel;
-class PATADiskDevice final : public StorageDevice {
-    friend class IDEController;
-    friend class DeviceManagement;
-    AK_MAKE_ETERNAL
-public:
-    // Type of drive this IDEDiskDevice is on the ATA channel.
-    //
-    // Each PATA channel can contain only two devices, which (I think) are
-    // jumper selectable on the drive itself by shorting two pins.
-    enum class DriveType : u8 {
-        Master,
-        Slave
-    };
-
-    enum class InterfaceType : u8 {
-        ATA,
-        ATAPI,
-    };
-
-public:
-    static NonnullRefPtr<PATADiskDevice> create(const IDEController&, IDEChannel&, DriveType, InterfaceType, u16, u64);
-    virtual ~PATADiskDevice() override;
-
-    // ^BlockDevice
-    virtual void start_request(AsyncBlockDeviceRequest&) override;
-
-private:
-    PATADiskDevice(const IDEController&, IDEChannel&, DriveType, InterfaceType, u16, u64, int, NonnullOwnPtr<KString>);
-
-    // ^DiskDevice
-    virtual StringView class_name() const override;
-
-    bool is_slave() const;
-
-    u16 m_capabilities { 0 };
-    NonnullRefPtr<IDEChannel> m_channel;
-    DriveType m_drive_type { DriveType::Master };
-    InterfaceType m_interface_type { InterfaceType::ATA };
-};
-
-}

+ 0 - 5
Kernel/Storage/RamdiskController.cpp

@@ -31,11 +31,6 @@ size_t RamdiskController::devices_count() const
     return m_devices.size();
 }
 
-void RamdiskController::start_request(const StorageDevice&, AsyncBlockDeviceRequest&)
-{
-    VERIFY_NOT_REACHED();
-}
-
 void RamdiskController::complete_current_request(AsyncDeviceRequest::RequestResult)
 {
     VERIFY_NOT_REACHED();

+ 0 - 1
Kernel/Storage/RamdiskController.h

@@ -28,7 +28,6 @@ public:
     virtual bool reset() override;
     virtual bool shutdown() override;
     virtual size_t devices_count() const override;
-    virtual void start_request(const StorageDevice&, AsyncBlockDeviceRequest&) override;
     virtual void complete_current_request(AsyncDeviceRequest::RequestResult) override;
 
 private:

+ 2 - 2
Kernel/Storage/RamdiskDevice.cpp

@@ -27,8 +27,8 @@ NonnullRefPtr<RamdiskDevice> RamdiskDevice::create(const RamdiskController& cont
     return device_or_error.release_value();
 }
 
-RamdiskDevice::RamdiskDevice(const RamdiskController& controller, NonnullOwnPtr<Memory::Region>&& region, int major, int minor, NonnullOwnPtr<KString> device_name)
-    : StorageDevice(controller, major, minor, 512, region->size() / 512, move(device_name))
+RamdiskDevice::RamdiskDevice(const RamdiskController&, NonnullOwnPtr<Memory::Region>&& region, int major, int minor, NonnullOwnPtr<KString> device_name)
+    : StorageDevice(major, minor, 512, region->size() / 512, move(device_name))
     , m_region(move(region))
 {
     dmesgln("Ramdisk: Device #{} @ {}, Capacity={}", minor, m_region->vaddr(), max_addressable_block() * 512);

+ 3 - 0
Kernel/Storage/RamdiskDevice.h

@@ -30,6 +30,9 @@ private:
     // ^BlockDevice
     virtual void start_request(AsyncBlockDeviceRequest&) override;
 
+    // ^StorageDevice
+    virtual CommandSet command_set() const override { return CommandSet::PlainMemory; }
+
     Mutex m_lock { "RamdiskDevice" };
 
     NonnullOwnPtr<Memory::Region> m_region;

+ 0 - 51
Kernel/Storage/SATADiskDevice.cpp

@@ -1,51 +0,0 @@
-/*
- * Copyright (c) 2021, Liav A. <liavalb@hotmail.co.il>
- *
- * SPDX-License-Identifier: BSD-2-Clause
- */
-
-#include <AK/StringView.h>
-#include <Kernel/Devices/DeviceManagement.h>
-#include <Kernel/FileSystem/OpenFileDescription.h>
-#include <Kernel/Storage/AHCIController.h>
-#include <Kernel/Storage/IDEChannel.h>
-#include <Kernel/Storage/SATADiskDevice.h>
-#include <Kernel/Storage/StorageManagement.h>
-
-namespace Kernel {
-
-NonnullRefPtr<SATADiskDevice> SATADiskDevice::create(const AHCIController& controller, const AHCIPort& port, size_t sector_size, u64 max_addressable_block)
-{
-    auto minor_device_number = StorageManagement::minor_number();
-
-    // FIXME: We need a way of formatting strings with KString.
-    auto device_name = String::formatted("hd{:c}", 'a' + minor_device_number);
-    auto device_name_kstring = KString::must_create(device_name.view());
-
-    auto device_or_error = DeviceManagement::try_create_device<SATADiskDevice>(controller, port, sector_size, max_addressable_block, minor_device_number, move(device_name_kstring));
-    // FIXME: Find a way to propagate errors
-    VERIFY(!device_or_error.is_error());
-    return device_or_error.release_value();
-}
-
-SATADiskDevice::SATADiskDevice(const AHCIController& controller, const AHCIPort& port, size_t sector_size, u64 max_addressable_block, int minor_number, NonnullOwnPtr<KString> device_name)
-    : StorageDevice(controller, StorageManagement::major_number(), minor_number, sector_size, max_addressable_block, move(device_name))
-    , m_port(port)
-{
-}
-
-SATADiskDevice::~SATADiskDevice()
-{
-}
-
-StringView SATADiskDevice::class_name() const
-{
-    return "SATADiskDevice"sv;
-}
-
-void SATADiskDevice::start_request(AsyncBlockDeviceRequest& request)
-{
-    m_port.strong_ref()->start_request(request);
-}
-
-}

+ 0 - 43
Kernel/Storage/SATADiskDevice.h

@@ -1,43 +0,0 @@
-/*
- * Copyright (c) 2021, Liav A. <liavalb@hotmail.co.il>
- *
- * SPDX-License-Identifier: BSD-2-Clause
- */
-
-#pragma once
-
-#include <Kernel/Interrupts/IRQHandler.h>
-#include <Kernel/Locking/Mutex.h>
-#include <Kernel/Storage/AHCIPort.h>
-#include <Kernel/Storage/StorageDevice.h>
-
-namespace Kernel {
-
-class AHCIController;
-class SATADiskDevice final : public StorageDevice {
-    friend class AHCIController;
-    friend class DeviceManagement;
-
-public:
-    enum class InterfaceType : u8 {
-        SATA,
-        SATAPI,
-    };
-
-public:
-    static NonnullRefPtr<SATADiskDevice> create(const AHCIController&, const AHCIPort&, size_t sector_size, u64 max_addressable_block);
-    virtual ~SATADiskDevice() override;
-
-    // ^StorageDevice
-    // ^BlockDevice
-    virtual void start_request(AsyncBlockDeviceRequest&) override;
-
-private:
-    SATADiskDevice(const AHCIController&, const AHCIPort&, size_t sector_size, u64 max_addressable_block, int minor_device_number, NonnullOwnPtr<KString> device_name);
-
-    // ^DiskDevice
-    virtual StringView class_name() const override;
-    WeakPtr<AHCIPort> m_port;
-};
-
-}

+ 0 - 3
Kernel/Storage/StorageController.h

@@ -30,9 +30,6 @@ public:
     virtual RefPtr<StorageDevice> device(u32 index) const = 0;
     virtual size_t devices_count() const = 0;
 
-protected:
-    virtual void start_request(const StorageDevice&, AsyncBlockDeviceRequest&) = 0;
-
 protected:
     virtual bool reset() = 0;
     virtual bool shutdown() = 0;

+ 4 - 10
Kernel/Storage/StorageDevice.cpp

@@ -13,10 +13,9 @@
 
 namespace Kernel {
 
-StorageDevice::StorageDevice(const StorageController& controller, int major, int minor, size_t sector_size, u64 max_addressable_block, NonnullOwnPtr<KString> device_name)
+StorageDevice::StorageDevice(int major, int minor, size_t sector_size, u64 max_addressable_block, NonnullOwnPtr<KString> device_name)
     : BlockDevice(major, minor, sector_size)
-    , m_storage_controller(controller)
-    , m_storage_device_name(move(device_name))
+    , m_early_storage_device_name(move(device_name))
     , m_max_addressable_block(max_addressable_block)
 {
 }
@@ -26,11 +25,6 @@ StringView StorageDevice::class_name() const
     return "StorageDevice"sv;
 }
 
-NonnullRefPtr<StorageController> StorageDevice::controller() const
-{
-    return m_storage_controller;
-}
-
 KResultOr<size_t> StorageDevice::read(OpenFileDescription&, u64 offset, UserOrKernelBuffer& outbuf, size_t len)
 {
     unsigned index = offset / block_size();
@@ -183,9 +177,9 @@ KResultOr<size_t> StorageDevice::write(OpenFileDescription&, u64 offset, const U
     return pos + remaining;
 }
 
-StringView StorageDevice::storage_name() const
+StringView StorageDevice::early_storage_name() const
 {
-    return m_storage_device_name->view();
+    return m_early_storage_device_name->view();
 }
 
 bool StorageDevice::can_write(const OpenFileDescription&, size_t offset) const

+ 24 - 6
Kernel/Storage/StorageDevice.h

@@ -19,9 +19,22 @@ class StorageDevice : public BlockDevice {
     friend class StorageManagement;
 
 public:
-    virtual u64 max_addressable_block() const { return m_max_addressable_block; }
+    // Note: this attribute describes the internal command set of a Storage device.
+    // For example, an ordinary harddrive utilizes the ATA command set, while
+    // an ATAPI device (e.g. Optical drive) that is connected to the ATA bus,
+    // is actually using SCSI commands (packets) encapsulated inside an ATA command.
+    // The IDE controller code being aware of the possibility of ATAPI devices attached
+    // to the ATA bus, will check whether the Command set is ATA or SCSI and will act
+    // accordingly.
+    enum class CommandSet {
+        PlainMemory,
+        SCSI,
+        ATA,
+        NVMe,
+    };
 
-    NonnullRefPtr<StorageController> controller() const;
+public:
+    virtual u64 max_addressable_block() const { return m_max_addressable_block; }
 
     // ^BlockDevice
     virtual KResultOr<size_t> read(OpenFileDescription&, u64, UserOrKernelBuffer&, size_t) override;
@@ -30,19 +43,24 @@ public:
     virtual bool can_write(const OpenFileDescription&, size_t) const override;
     virtual void prepare_for_unplug() { m_partitions.clear(); }
 
-    StringView storage_name() const;
+    // FIXME: Remove this method after figuring out another scheme for naming.
+    StringView early_storage_name() const;
+
     NonnullRefPtrVector<DiskPartition> partitions() const { return m_partitions; }
 
+    virtual CommandSet command_set() const = 0;
+
 protected:
-    StorageDevice(const StorageController&, int, int, size_t, u64, NonnullOwnPtr<KString>);
+    StorageDevice(int, int, size_t, u64, NonnullOwnPtr<KString>);
     // ^DiskDevice
     virtual StringView class_name() const override;
 
 private:
     mutable IntrusiveListNode<StorageDevice, RefPtr<StorageDevice>> m_list_node;
-    NonnullRefPtr<StorageController> m_storage_controller;
     NonnullRefPtrVector<DiskPartition> m_partitions;
-    NonnullOwnPtr<KString> m_storage_device_name;
+
+    // FIXME: Remove this method after figuring out another scheme for naming.
+    NonnullOwnPtr<KString> m_early_storage_device_name;
     u64 m_max_addressable_block;
 };
 

+ 1 - 1
Kernel/Storage/StorageManagement.cpp

@@ -126,7 +126,7 @@ UNMAP_AFTER_INIT void StorageManagement::determine_boot_device()
     if (m_boot_argument.starts_with("/dev/"sv)) {
         StringView storage_name = m_boot_argument.substring_view(5);
         for (auto& storage_device : m_storage_devices) {
-            if (storage_device.storage_name() == storage_name) {
+            if (storage_device.early_storage_name() == storage_name) {
                 m_boot_block_device = storage_device;
             }
         }