Kernel: Rework whole ATA driver structure

Make ATA driver more compatible when we are adding SATA support

kernel/CMakeLists.txt

@@ -52,9 +52,9 @@ set(KERNEL_SOURCES
 	kernel/Semaphore.cpp
 	kernel/SpinLock.cpp
 	kernel/SSP.cpp
-	kernel/Storage/ATABus.cpp
+	kernel/Storage/ATA/ATABus.cpp
-	kernel/Storage/ATAController.cpp
+	kernel/Storage/ATA/ATAController.cpp
-	kernel/Storage/ATADevice.cpp
+	kernel/Storage/ATA/ATADevice.cpp
 	kernel/Storage/DiskCache.cpp
 	kernel/Storage/StorageDevice.cpp
 	kernel/Syscall.cpp

kernel/include/kernel/Device/Device.h

@@ -17,6 +17,8 @@ namespace Kernel
 
 		virtual dev_t rdev() const override = 0;
 
+		virtual BAN::StringView name() const = 0;
+
 	protected:
 		Device(mode_t, uid_t, gid_t);
 	};

kernel/include/kernel/Device/NullDevice.h

@@ -1,3 +1,5 @@
+#pragma once
+
 #include <kernel/Device/Device.h>
 
 namespace Kernel
@@ -10,6 +12,8 @@ namespace Kernel
 
 		virtual dev_t rdev() const override { return m_rdev; }
 
+		virtual BAN::StringView name() const override { return "null"sv; }
+
 	protected:
 		NullDevice(mode_t mode, uid_t uid, gid_t gid, dev_t rdev)
 			: CharacterDevice(mode, uid, gid)

kernel/include/kernel/Device/ZeroDevice.h

@@ -10,6 +10,8 @@ namespace Kernel
 
 		virtual dev_t rdev() const override { return m_rdev; }
 
+		virtual BAN::StringView name() const override { return "zero"sv; }
+
 	protected:
 		ZeroDevice(mode_t mode, uid_t uid, gid_t gid, dev_t rdev)
 			: CharacterDevice(mode, uid, gid)

kernel/include/kernel/FS/DevFS/FileSystem.h

@@ -15,10 +15,12 @@ namespace Kernel
 		
 		void initialize_device_updater();
 
-		void add_device(BAN::StringView path, BAN::RefPtr<RamInode>);
+		void add_device(BAN::RefPtr<Device>);
+		void add_inode(BAN::StringView path, BAN::RefPtr<RamInode>);
 		void for_each_device(const BAN::Function<BAN::Iteration(Device*)>& callback);
 
-		dev_t get_next_dev();
+		dev_t get_next_dev() const;
+		int get_next_input_device() const;
 
 		void initiate_sync(bool should_block);
 
@@ -28,7 +30,7 @@ namespace Kernel
 		{ }
 
 	private:
-		SpinLock m_device_lock;
+		mutable SpinLock m_device_lock;
 
 		Semaphore m_sync_done;
 		Semaphore m_sync_semaphore;

kernel/include/kernel/Input/PS2Controller.h

@@ -9,12 +9,13 @@ namespace Kernel::Input
 	class PS2Device : public CharacterDevice, public Interruptable
 	{
 	public:
+		PS2Device();
 		virtual ~PS2Device() {}
 
-	public:
+		virtual BAN::StringView name() const override { return m_name; }
-		PS2Device()
+
-			: CharacterDevice(Mode::IRUSR | Mode::IRGRP, 0, 0)
+	private:
-		{ }
+		const BAN::String m_name;
 	};
 
 	class PS2Controller

kernel/include/kernel/PCI.h

@@ -3,6 +3,9 @@
 #include <BAN/UniqPtr.h>
 #include <BAN/Vector.h>
 #include <kernel/Memory/Types.h>
+#include <kernel/Storage/StorageController.h>
+
+#include <sys/types.h>
 
 namespace Kernel::PCI
 {

kernel/include/kernel/Storage/ATA/ATABus.h

@@ -1,48 +1,46 @@
 #pragma once
 
-#include <BAN/Errors.h>
+#include <BAN/RefPtr.h>
+#include <BAN/Vector.h>
 #include <kernel/InterruptController.h>
 #include <kernel/SpinLock.h>
-#include <kernel/Storage/ATAController.h>
 
 namespace Kernel
 {
 
 	class ATADevice;
 
-	class ATABus : public Interruptable
+	class ATABus : public BAN::RefCounted<ATABus>, public Interruptable
 	{
 	public:
 		enum class DeviceType
 		{
-			None,
 			ATA,
 			ATAPI,
 		};
 
 	public:
-		static ATABus* create(ATAController&, uint16_t base, uint16_t ctrl, uint8_t irq);
+		static BAN::ErrorOr<BAN::RefPtr<ATABus>> create(uint16_t base, uint16_t ctrl, uint8_t irq);
 
 		BAN::ErrorOr<void> read(ATADevice&, uint64_t, uint8_t, uint8_t*);
 		BAN::ErrorOr<void> write(ATADevice&, uint64_t, uint8_t, const uint8_t*);
 
-		ATAController& controller() { return m_controller; }
-
 		virtual void handle_irq() override;
 
+		void initialize_devfs();
+
 	private:
-		ATABus(ATAController& controller, uint16_t base, uint16_t ctrl)
+		ATABus(uint16_t base, uint16_t ctrl)
-			: m_controller(controller)
+			: m_base(base)
-			, m_base(base)
 			, m_ctrl(ctrl)
 		{}
-		void initialize(uint8_t irq);
+		BAN::ErrorOr<void> initialize();
 
-		void select_device(const ATADevice&);
+		void select_device(bool secondary);
-		DeviceType identify(const ATADevice&, uint16_t*);
+		BAN::ErrorOr<DeviceType> identify(bool secondary, BAN::Span<uint16_t> buffer);
 
 		void block_until_irq();
-		uint8_t device_index(const ATADevice&) const;
+		//uint8_t device_index(const ATADevice&) const;
 
 		uint8_t io_read(uint16_t);
 		void io_write(uint16_t, uint8_t);
@@ -52,14 +50,14 @@ namespace Kernel
 		BAN::Error error();
 
 	private:
-		ATAController& m_controller;
 		const uint16_t m_base;
 		const uint16_t m_ctrl;
 		SpinLock m_lock;
 
 		bool m_has_got_irq { false };
 
-		BAN::RefPtr<ATADevice> m_devices[2] {};
+		// Non-owning pointers
+		BAN::Vector<ATADevice*> m_devices;
 
 		friend class ATAController;
 	};

kernel/include/kernel/Storage/ATA/ATAController.h

@@ -0,0 +1,27 @@
+#pragma once
+
+#include <BAN/UniqPtr.h>
+#include <kernel/PCI.h>
+#include <kernel/Storage/StorageController.h>
+#include <kernel/Storage/ATA/ATABus.h>
+#include <kernel/Storage/ATA/ATADevice.h>
+
+namespace Kernel
+{
+
+	class ATAController : public StorageController
+	{
+	public:
+		static BAN::ErrorOr<BAN::UniqPtr<StorageController>> create(PCI::Device&);
+		virtual BAN::ErrorOr<void> initialize() override;
+
+	private:
+		ATAController(PCI::Device& pci_device)
+			: m_pci_device(pci_device)
+		{ }
+
+	private:
+		PCI::Device& m_pci_device;
+	};
+
+}

kernel/include/kernel/Storage/ATA/ATADefinitions.h

@@ -1,5 +1,8 @@
 #pragma once
 
+#define ATA_PROGIF_PRIMARY_NATIVE	(1 << 0)
+#define ATA_PROGIF_SECONDARY_NATIVE	(1 << 2)
+
 #define ATA_PORT_DATA				0x00
 #define ATA_PORT_ERROR				0x00
 #define ATA_PORT_SECTOR_COUNT		0x02

kernel/include/kernel/Storage/ATA/ATADevice.h

@@ -1,6 +1,6 @@
 #pragma once
 
-#include <kernel/Storage/ATABus.h>
+#include <kernel/Storage/ATA/ATABus.h>
 #include <kernel/Storage/StorageDevice.h>
 
 namespace Kernel
@@ -9,23 +9,31 @@ namespace Kernel
 	class ATADevice final : public StorageDevice
 	{
 	public:
-		ATADevice(ATABus&);
+		static BAN::ErrorOr<BAN::RefPtr<ATADevice>> create(BAN::RefPtr<ATABus>, ATABus::DeviceType, bool is_secondary, BAN::Span<const uint16_t> identify_data);
-		BAN::ErrorOr<void> initialize(ATABus::DeviceType, const uint16_t*);
 
 		virtual uint32_t sector_size() const override { return m_sector_words * 2; }
 		virtual uint64_t total_size() const override { return m_lba_count * sector_size(); }
 
+		bool is_secondary() const { return m_is_secondary; }
+		uint32_t words_per_sector() const { return m_sector_words; }
+		uint64_t sector_count() const { return m_lba_count; }
+
 		BAN::StringView model() const { return m_model; }
+		BAN::StringView name() const;
 
 	protected:
 		virtual BAN::ErrorOr<void> read_sectors_impl(uint64_t, uint8_t, uint8_t*) override;
 		virtual BAN::ErrorOr<void> write_sectors_impl(uint64_t, uint8_t, const uint8_t*) override;
 
 	private:
-		ATABus& m_bus;
+		ATADevice(BAN::RefPtr<ATABus>, ATABus::DeviceType, bool is_secodary);
-		uint8_t m_index;
+		BAN::ErrorOr<void> initialize(BAN::Span<const uint16_t> identify_data);
+
+	private:
+		BAN::RefPtr<ATABus> m_bus;
+		const ATABus::DeviceType m_type;
+		const bool m_is_secondary;
 		
-		ATABus::DeviceType m_type;
 		uint16_t m_signature;
 		uint16_t m_capabilities;
 		uint32_t m_command_set;
@@ -33,8 +41,6 @@ namespace Kernel
 		uint64_t m_lba_count;
 		char m_model[41];
 
-		friend class ATABus;
-
 	public:
 		virtual Mode mode() const override { return { Mode::IFBLK | Mode::IRUSR | Mode::IRGRP }; }
 		virtual uid_t uid() const override { return 0; }

kernel/include/kernel/Storage/ATAController.h

@@ -1,38 +0,0 @@
-#pragma once
-
-#include <kernel/PCI.h>
-#include <kernel/Storage/StorageController.h>
-
-namespace Kernel
-{
-
-	class ATABus;
-
-	class ATAController final : public StorageController
-	{
-	public:
-		static BAN::ErrorOr<BAN::RefPtr<ATAController>> create(const PCI::Device&);
-
-		virtual BAN::Vector<BAN::RefPtr<StorageDevice>> devices() override;
-
-	private:
-		ATAController();
-		BAN::ErrorOr<void> initialize(const PCI::Device& device);
-
-	private:
-		ATABus* m_buses[2] { nullptr, nullptr };
-		friend class ATABus;
-
-	public:
-		virtual Mode mode() const override { return { Mode::IFCHR }; }
-		virtual uid_t uid() const override { return 0; }
-		virtual gid_t gid() const override { return 0; }
-		virtual dev_t rdev() const override { return m_rdev; }
-
-		virtual BAN::ErrorOr<size_t> read(size_t, void*, size_t) { return BAN::Error::from_errno(ENOTSUP); }
-
-	private:
-		const dev_t m_rdev;
-	};
-
-}

kernel/include/kernel/Storage/StorageController.h

@@ -1,17 +1,13 @@
 #pragma once
 
-#include <kernel/Storage/StorageDevice.h>
-
 namespace Kernel
 {
 
-	class StorageController : public CharacterDevice
+	class StorageController
 	{
 	public:
-		StorageController()
+		virtual ~StorageController() {}
-			: CharacterDevice(0660, 0, 0)
+		virtual BAN::ErrorOr<void> initialize() = 0;
-		{ }
-		virtual BAN::Vector<BAN::RefPtr<StorageDevice>> devices() = 0;
 	};
 
 }

kernel/include/kernel/Storage/StorageDevice.h

@@ -20,7 +20,7 @@ namespace Kernel
 	class Partition final : public BlockDevice
 	{
 	public:
-		Partition(StorageDevice&, const GUID&, const GUID&, uint64_t, uint64_t, uint64_t, const char*, uint32_t);
+		static BAN::ErrorOr<BAN::RefPtr<Partition>> create(StorageDevice&, const GUID& type, const GUID& guid, uint64_t start, uint64_t end, uint64_t attr, const char* label, uint32_t index);
 
 		const GUID& partition_type() const { return m_type; }
 		const GUID& partition_guid() const { return m_guid; }
@@ -33,6 +33,11 @@ namespace Kernel
 		BAN::ErrorOr<void> read_sectors(uint64_t lba, uint8_t sector_count, uint8_t* buffer);
 		BAN::ErrorOr<void> write_sectors(uint64_t lba, uint8_t sector_count, const uint8_t* buffer);
 		
+		virtual BAN::StringView name() const override { return m_name; }
+
+	private:
+		Partition(StorageDevice&, const GUID&, const GUID&, uint64_t, uint64_t, uint64_t, const char*, uint32_t);
+
 	private:
 		StorageDevice& m_device;
 		const GUID m_type;
@@ -41,6 +46,7 @@ namespace Kernel
 		const uint64_t m_lba_end;
 		const uint64_t m_attributes;
 		char m_label[36 * 4 + 1];
+		const BAN::String m_name;
 
 	public:
 		virtual bool is_partition() const override { return true; }
@@ -73,8 +79,8 @@ namespace Kernel
 		virtual uint32_t sector_size() const = 0;
 		virtual uint64_t total_size() const = 0;
 
-		BAN::Vector<Partition*>& partitions() { return m_partitions; }
+		BAN::Vector<BAN::RefPtr<Partition>>& partitions() { return m_partitions; }
-		const BAN::Vector<Partition*>& partitions() const { return m_partitions; }
+		const BAN::Vector<BAN::RefPtr<Partition>>& partitions() const { return m_partitions; }
 
 		BAN::ErrorOr<void> sync_disk_cache();
 		virtual bool is_storage_device() const override { return true; }
@@ -85,9 +91,9 @@ namespace Kernel
 		void add_disk_cache();
 
 	private:
-		SpinLock					m_lock;
+		SpinLock							m_lock;
-		BAN::Optional<DiskCache>	m_disk_cache;
+		BAN::Optional<DiskCache>			m_disk_cache;
-		BAN::Vector<Partition*>		m_partitions;
+		BAN::Vector<BAN::RefPtr<Partition>>	m_partitions;
 
 		friend class DiskCache;
 	};

kernel/include/kernel/Terminal/TTY.h

@@ -50,8 +50,6 @@ namespace Kernel
 		virtual BAN::ErrorOr<size_t> read_impl(off_t, void*, size_t) override;
 		virtual BAN::ErrorOr<size_t> write_impl(off_t, const void*, size_t) override;
 
-		virtual BAN::StringView name() const = 0;
-
 	private:
 		void do_backspace();
 

kernel/kernel/FS/DevFS/FileSystem.cpp

@@ -22,8 +22,8 @@ namespace Kernel
 		auto root_inode = MUST(RamDirectoryInode::create(*s_instance, 0, 0755, 0, 0));
 		MUST(s_instance->set_root_inode(root_inode));
 
-		s_instance->add_device("null", MUST(NullDevice::create(0666, 0, 0)));
+		s_instance->add_device(MUST(NullDevice::create(0666, 0, 0)));
-		s_instance->add_device("zero", MUST(ZeroDevice::create(0666, 0, 0)));
+		s_instance->add_device(MUST(ZeroDevice::create(0666, 0, 0)));
 	}
 
 	DevFileSystem& DevFileSystem::get()
@@ -117,10 +117,16 @@ namespace Kernel
 			m_sync_done.block();
 	}
 
-	void DevFileSystem::add_device(BAN::StringView path, BAN::RefPtr<RamInode> device)
+	void DevFileSystem::add_device(BAN::RefPtr<Device> device)
+	{
+		ASSERT(!device->name().contains('/'));
+		MUST(reinterpret_cast<RamDirectoryInode*>(root_inode().ptr())->add_inode(device->name(), device));
+	}
+
+	void DevFileSystem::add_inode(BAN::StringView path, BAN::RefPtr<RamInode> inode)
 	{
 		ASSERT(!path.contains('/'));
-		MUST(reinterpret_cast<RamDirectoryInode*>(root_inode().ptr())->add_inode(path, device));
+		MUST(reinterpret_cast<RamDirectoryInode*>(root_inode().ptr())->add_inode(path, inode));
 	}
 
 	void DevFileSystem::for_each_device(const BAN::Function<BAN::Iteration(Device*)>& callback)
@@ -136,11 +142,18 @@ namespace Kernel
 		);
 	}
 
-	dev_t DevFileSystem::get_next_dev()
+	dev_t DevFileSystem::get_next_dev() const
 	{
 		LockGuard _(m_device_lock);
 		static dev_t next_dev = 1;
 		return next_dev++;
 	}
 
+	int DevFileSystem::get_next_input_device() const
+	{
+		LockGuard _(m_device_lock);
+		static dev_t next_dev = 0;
+		return next_dev++;
+	}
+
 }

kernel/kernel/Input/PS2Controller.cpp

@@ -68,6 +68,11 @@ namespace Kernel::Input
 
 	static PS2Controller* s_instance = nullptr;
 
+	PS2Device::PS2Device()
+		: CharacterDevice(0440, 0, 0)
+		, m_name(BAN::String::formatted("input{}", DevFileSystem::get().get_next_input_device()))
+	{ }
+
 	BAN::ErrorOr<void> PS2Controller::initialize()
 	{
 		ASSERT(s_instance == nullptr);
@@ -175,14 +180,14 @@ namespace Kernel::Input
 			m_devices[0]->set_irq(PS2::IRQ::DEVICE0);
 			m_devices[0]->enable_interrupt();
 			config |= PS2::Config::INTERRUPT_FIRST_PORT;
-			DevFileSystem::get().add_device("input0", m_devices[0]);
+			DevFileSystem::get().add_device(m_devices[0]);
 		}
 		if (m_devices[1])
 		{
 			m_devices[1]->set_irq(PS2::IRQ::DEVICE1);
 			m_devices[1]->enable_interrupt();
 			config |= PS2::Config::INTERRUPT_SECOND_PORT;
-			DevFileSystem::get().add_device("input1", m_devices[1]);
+			DevFileSystem::get().add_device(m_devices[1]);
 		}
 
 		controller_send_command(PS2::Command::WRITE_CONFIG, config);

kernel/kernel/PCI.cpp

@@ -3,7 +3,8 @@
 #include <kernel/MMIO.h>
 #include <kernel/Networking/E1000.h>
 #include <kernel/PCI.h>
-#include <kernel/Storage/ATAController.h>
+#include <kernel/Storage/ATA/AHCI/Controller.h>
+#include <kernel/Storage/ATA/ATAController.h>
 
 #define INVALID_VENDOR 0xFFFF
 #define MULTI_FUNCTION 0x80
@@ -143,6 +144,8 @@ namespace Kernel::PCI
 					switch (pci_device.subclass())
 					{
 						case 0x01:
+						case 0x05:
+						case 0x06:
 							if (auto res = ATAController::create(pci_device); res.is_error())
 								dprintln("ATA: {}", res.error());
 							break;

kernel/kernel/Storage/ATA/ATABus.cpp

@@ -1,27 +1,31 @@
-#include <BAN/ScopeGuard.h>
+#include <kernel/FS/DevFS/FileSystem.h>
 #include <kernel/IDT.h>
 #include <kernel/InterruptController.h>
 #include <kernel/IO.h>
 #include <kernel/LockGuard.h>
-#include <kernel/Storage/ATADevice.h>
+#include <kernel/Storage/ATA/ATABus.h>
-#include <kernel/Storage/ATABus.h>
+#include <kernel/Storage/ATA/ATADefinitions.h>
-#include <kernel/Storage/ATADefinitions.h>
+#include <kernel/Storage/ATA/ATADevice.h>
 #include <kernel/Timer/Timer.h>
 
 namespace Kernel
 {
 
-	ATABus* ATABus::create(ATAController& controller, uint16_t base, uint16_t ctrl, uint8_t irq)
+	BAN::ErrorOr<BAN::RefPtr<ATABus>> ATABus::create(uint16_t base, uint16_t ctrl, uint8_t irq)
 	{
-		ATABus* bus = new ATABus(controller, base, ctrl);
+		auto* bus_ptr = new ATABus(base, ctrl);
-		ASSERT(bus);
+		if (bus_ptr == nullptr)
-		bus->initialize(irq);
+			return BAN::Error::from_errno(ENOMEM);
+		auto bus = BAN::RefPtr<ATABus>::adopt(bus_ptr);
+		bus->set_irq(irq);
+		TRY(bus->initialize());
+		if (bus->m_devices.empty())
+			return BAN::Error::from_errno(ENODEV);
 		return bus;
 	}
 
-	void ATABus::initialize(uint8_t irq)
+	BAN::ErrorOr<void> ATABus::initialize()
 	{
-		set_irq(irq);
 		enable_interrupt();
 
 		BAN::Vector<uint16_t> identify_buffer;
@@ -29,49 +33,57 @@ namespace Kernel
 
 		for (uint8_t i = 0; i < 2; i++)
 		{
-			{
+			bool is_secondary = (i == 1);
-				auto* temp_ptr = new ATADevice(*this);
-				ASSERT(temp_ptr);
-				m_devices[i] = BAN::RefPtr<ATADevice>::adopt(temp_ptr);
-			}
-			ATADevice& device = *m_devices[i];
 
-			BAN::ScopeGuard guard([this, i] { m_devices[i] = nullptr; });
+			DeviceType device_type;
-
+			if (auto res = identify(is_secondary, identify_buffer.span()); res.is_error())
-			auto type = identify(device, identify_buffer.data());
-			if (type == DeviceType::None)
 				continue;
+			else
+				device_type = res.value();
 
-			auto res = device.initialize(type, identify_buffer.data());
+			auto device_or_error = ATADevice::create(this, device_type, is_secondary, identify_buffer.span());
-			if (res.is_error())
+
+			if (device_or_error.is_error())
 			{
-				dprintln("{}", res.error());
+				dprintln("{}", device_or_error.error());
 				continue;
 			}
 
-			guard.disable();
+			auto device = device_or_error.release_value();
+			device->ref();
+			TRY(m_devices.push_back(device.ptr()));
 		}
 
 		// Enable disk interrupts
-		for (int i = 0; i < 2; i++)
+		for (auto& device : m_devices)
 		{
-			if (!m_devices[i])
+			select_device(device->is_secondary());
-				continue;
-			select_device(*m_devices[i]);
 			io_write(ATA_PORT_CONTROL, 0);
 		}
+
+		return {};
+	}
+
+	void ATABus::initialize_devfs()
+	{
+		for (auto& device : m_devices)
+		{
+			DevFileSystem::get().add_device(device);
+			if (auto res = device->initialize_partitions(); res.is_error())
+				dprintln("{}", res.error());
+			device->unref();
+		}
 	}
 
-	void ATABus::select_device(const ATADevice& device)
+	void ATABus::select_device(bool secondary)
 	{
-		uint8_t device_index = this->device_index(device);
+		io_write(ATA_PORT_DRIVE_SELECT, 0xA0 | ((uint8_t)secondary << 4));
-		io_write(ATA_PORT_DRIVE_SELECT, 0xA0 | (device_index << 4));
 		SystemTimer::get().sleep(1);
 	}
 
-	ATABus::DeviceType ATABus::identify(const ATADevice& device, uint16_t* buffer)
+	BAN::ErrorOr<ATABus::DeviceType> ATABus::identify(bool secondary, BAN::Span<uint16_t> buffer)
 	{
-		select_device(device);
+		select_device(secondary);
 
 		// Disable interrupts
 		io_write(ATA_PORT_CONTROL, ATA_CONTROL_nIEN);
@@ -81,7 +93,7 @@ namespace Kernel
 
 		// No device on port
 		if (io_read(ATA_PORT_STATUS) == 0)
-			return DeviceType::None;
+			return BAN::Error::from_errno(EINVAL);
 
 		DeviceType type = DeviceType::ATA;
 
@@ -97,7 +109,7 @@ namespace Kernel
 			else
 			{
 				dprintln("Unsupported device type");
-				return DeviceType::None;
+				return BAN::Error::from_errno(EINVAL);
 			}
 
 			io_write(ATA_PORT_COMMAND, ATA_COMMAND_IDENTIFY_PACKET);
@@ -106,11 +118,12 @@ namespace Kernel
 			if (auto res = wait(true); res.is_error())
 			{
 				dprintln("Fatal error: {}", res.error());
-				return DeviceType::None;
+				return BAN::Error::from_errno(EINVAL);
 			}
 		}
 
-		read_buffer(ATA_PORT_DATA, buffer, 256);
+		ASSERT(buffer.size() >= 256);
+		read_buffer(ATA_PORT_DATA, buffer.data(), 256);
 
 		return type;
 	}
@@ -212,18 +225,9 @@ namespace Kernel
 		return BAN::Error::from_error_code(ErrorCode::None);
 	}
 
-	uint8_t ATABus::device_index(const ATADevice& device) const
-	{
-		if (m_devices[0] && m_devices[0].ptr() == &device)
-			return 0;
-		if (m_devices[1] && m_devices[1].ptr() == &device)
-			return 1;
-		ASSERT_NOT_REACHED();
-	}
-
 	BAN::ErrorOr<void> ATABus::read(ATADevice& device, uint64_t lba, uint8_t sector_count, uint8_t* buffer)
 	{
-		if (lba + sector_count > device.m_lba_count)
+		if (lba + sector_count > device.sector_count())
 			return BAN::Error::from_error_code(ErrorCode::Storage_Boundaries);
 
 		LockGuard _(m_lock);
@@ -231,7 +235,7 @@ namespace Kernel
 		if (lba < (1 << 28))
 		{
 			// LBA28
-			io_write(ATA_PORT_DRIVE_SELECT, 0xE0 | (device_index(device) << 4) | ((lba >> 24) & 0x0F));
+			io_write(ATA_PORT_DRIVE_SELECT, 0xE0 | ((uint8_t)device.is_secondary() << 4) | ((lba >> 24) & 0x0F));
 			io_write(ATA_PORT_SECTOR_COUNT, sector_count);
 			io_write(ATA_PORT_LBA0, (uint8_t)(lba >>  0));
 			io_write(ATA_PORT_LBA1, (uint8_t)(lba >>  8));
@@ -241,7 +245,7 @@ namespace Kernel
 			for (uint32_t sector = 0; sector < sector_count; sector++)
 			{
 				block_until_irq();
-				read_buffer(ATA_PORT_DATA, (uint16_t*)buffer + sector * device.m_sector_words, device.m_sector_words);
+				read_buffer(ATA_PORT_DATA, (uint16_t*)buffer + sector * device.words_per_sector(), device.words_per_sector());
 			}
 		}
 		else
@@ -255,7 +259,7 @@ namespace Kernel
 
 	BAN::ErrorOr<void> ATABus::write(ATADevice& device, uint64_t lba, uint8_t sector_count, const uint8_t* buffer)
 	{
-		if (lba + sector_count > device.m_lba_count)
+		if (lba + sector_count > device.sector_count())
 			return BAN::Error::from_error_code(ErrorCode::Storage_Boundaries);
 
 		LockGuard _(m_lock);
@@ -263,7 +267,7 @@ namespace Kernel
 		if (lba < (1 << 28))
 		{
 			// LBA28
-			io_write(ATA_PORT_DRIVE_SELECT, 0xE0 | (device_index(device) << 4) | ((lba >> 24) & 0x0F));
+			io_write(ATA_PORT_DRIVE_SELECT, 0xE0 | ((uint8_t)device.is_secondary() << 4) | ((lba >> 24) & 0x0F));
 			io_write(ATA_PORT_SECTOR_COUNT, sector_count);
 			io_write(ATA_PORT_LBA0, (uint8_t)(lba >>  0));
 			io_write(ATA_PORT_LBA1, (uint8_t)(lba >>  8));
@@ -274,7 +278,7 @@ namespace Kernel
 
 			for (uint32_t sector = 0; sector < sector_count; sector++)
 			{
-				write_buffer(ATA_PORT_DATA, (uint16_t*)buffer + sector * device.m_sector_words, device.m_sector_words);
+				write_buffer(ATA_PORT_DATA, (uint16_t*)buffer + sector * device.words_per_sector(), device.words_per_sector());
 				block_until_irq();
 			}
 		}

kernel/kernel/Storage/ATA/ATAController.cpp

@@ -0,0 +1,76 @@
+#include <kernel/FS/DevFS/FileSystem.h>
+#include <kernel/Storage/ATA/ATABus.h>
+#include <kernel/Storage/ATA/ATAController.h>
+#include <kernel/Storage/ATA/ATADefinitions.h>
+#include <kernel/Storage/ATA/ATADevice.h>
+
+namespace Kernel
+{
+
+	BAN::ErrorOr<BAN::UniqPtr<StorageController>> ATAController::create(PCI::Device& pci_device)
+	{
+		StorageController* controller_ptr = nullptr;
+
+		switch (pci_device.subclass())
+		{
+			case 0x01:
+				controller_ptr = new ATAController(pci_device);
+				break;
+			case 0x05:
+				dwarnln("unsupported DMA ATA Controller");
+				return BAN::Error::from_errno(ENOTSUP);
+			case 0x06:
+				dwarnln("unsupported SATA Controller");
+				return BAN::Error::from_errno(ENOTSUP);
+			default:
+				ASSERT_NOT_REACHED();
+		}
+
+		if (controller_ptr == nullptr)
+			return BAN::Error::from_errno(ENOMEM);
+
+		auto controller = BAN::UniqPtr<StorageController>::adopt(controller_ptr);
+		TRY(controller->initialize());
+		return controller;
+	}
+
+	BAN::ErrorOr<void> ATAController::initialize()
+	{
+		BAN::Vector<BAN::RefPtr<ATABus>> buses;
+
+		uint8_t prog_if = m_pci_device.read_byte(0x09);
+
+		if (!(prog_if & ATA_PROGIF_PRIMARY_NATIVE))
+		{
+			auto bus_or_error = ATABus::create(0x1F0, 0x3F6, 14);
+			if (bus_or_error.is_error())
+				dprintln("IDE ATABus: {}", bus_or_error.error());
+			else
+				TRY(buses.push_back(bus_or_error.release_value()));
+		}
+		else
+		{
+			dprintln("unsupported IDE ATABus in native mode");
+		}
+
+		// BUS 2
+		if (!(prog_if & ATA_PROGIF_SECONDARY_NATIVE))
+		{
+			auto bus_or_error = ATABus::create(0x170, 0x376, 15);
+			if (bus_or_error.is_error())
+				dprintln("IDE ATABus: {}", bus_or_error.error());
+			else
+				TRY(buses.push_back(bus_or_error.release_value()));
+		}
+		else
+		{
+			dprintln("unsupported IDE ATABus in native mode");
+		}
+
+		for (auto& bus : buses)
+			bus->initialize_devfs();
+
+		return {};
+	}
+
+}

kernel/kernel/Storage/ATA/ATADevice.cpp

@@ -0,0 +1,117 @@
+#include <kernel/FS/DevFS/FileSystem.h>
+#include <kernel/IO.h>
+#include <kernel/Storage/ATA/ATABus.h>
+#include <kernel/Storage/ATA/ATADefinitions.h>
+#include <kernel/Storage/ATA/ATADevice.h>
+
+#include <sys/sysmacros.h>
+
+namespace Kernel
+{
+
+	static dev_t get_ata_dev_major()
+	{
+		static dev_t major = DevFileSystem::get().get_next_dev();
+		return major;
+	}
+
+	static dev_t get_ata_dev_minor()
+	{
+		static dev_t minor = 0;
+		return minor++;
+	}
+
+	BAN::ErrorOr<BAN::RefPtr<ATADevice>> ATADevice::create(BAN::RefPtr<ATABus> bus, ATABus::DeviceType type, bool is_secondary, BAN::Span<const uint16_t> identify_data)
+	{
+		auto* device_ptr = new ATADevice(bus, type, is_secondary);
+		if (device_ptr == nullptr)
+			return BAN::Error::from_errno(ENOMEM);
+		auto device = BAN::RefPtr<ATADevice>::adopt(device_ptr);
+		TRY(device->initialize(identify_data));
+		return device;
+	}
+
+	ATADevice::ATADevice(BAN::RefPtr<ATABus> bus, ATABus::DeviceType type, bool is_secondary)
+		: m_bus(bus)
+		, m_type(type)
+		, m_is_secondary(is_secondary)
+		, m_rdev(makedev(get_ata_dev_major(), get_ata_dev_minor()))
+	{ }
+
+	BAN::ErrorOr<void> ATADevice::initialize(BAN::Span<const uint16_t> identify_data)
+	{
+		ASSERT(identify_data.size() >= 256);
+
+		m_signature = identify_data[ATA_IDENTIFY_SIGNATURE];
+		m_capabilities = identify_data[ATA_IDENTIFY_CAPABILITIES];
+
+		m_command_set = 0;
+		m_command_set |= (uint32_t)(identify_data[ATA_IDENTIFY_COMMAND_SET + 0] <<  0);
+		m_command_set |= (uint32_t)(identify_data[ATA_IDENTIFY_COMMAND_SET + 1] << 16);
+
+		if (!(m_capabilities & ATA_CAPABILITIES_LBA))
+			return BAN::Error::from_error_code(ErrorCode::ATA_NoLBA);
+		
+		if ((identify_data[ATA_IDENTIFY_SECTOR_INFO] & (1 << 15)) == 0 &&
+			(identify_data[ATA_IDENTIFY_SECTOR_INFO] & (1 << 14)) != 0 &&
+			(identify_data[ATA_IDENTIFY_SECTOR_INFO] & (1 << 12)) != 0)
+		{
+			m_sector_words = *(uint32_t*)(identify_data.data() + ATA_IDENTIFY_SECTOR_WORDS);
+		}
+		else
+		{
+			m_sector_words = 256;
+		}
+
+		m_lba_count = 0;
+		if (m_command_set & ATA_COMMANDSET_LBA48_SUPPORTED)
+			m_lba_count = *(uint64_t*)(identify_data.data() + ATA_IDENTIFY_LBA_COUNT_EXT);
+		if (m_lba_count < (1 << 28))
+			m_lba_count = *(uint32_t*)(identify_data.data() + ATA_IDENTIFY_LBA_COUNT);
+
+		for (int i = 0; i < 20; i++)
+		{
+			uint16_t word = identify_data[ATA_IDENTIFY_MODEL + i];
+			m_model[2 * i + 0] = word >> 8;
+			m_model[2 * i + 1] = word & 0xFF;
+		}
+		m_model[40] = 0;
+
+		dprintln("ATA disk {} MB", total_size() / 1024 / 1024);
+
+		add_disk_cache();
+
+		return {};
+	}
+
+	BAN::ErrorOr<void> ATADevice::read_sectors_impl(uint64_t lba, uint8_t sector_count, uint8_t* buffer)
+	{
+		TRY(m_bus->read(*this, lba, sector_count, buffer));
+		return {};
+	}
+
+	BAN::ErrorOr<void> ATADevice::write_sectors_impl(uint64_t lba, uint8_t sector_count, const uint8_t* buffer)
+	{
+		TRY(m_bus->write(*this, lba, sector_count, buffer));
+		return {};
+	}
+
+	BAN::ErrorOr<size_t> ATADevice::read_impl(off_t offset, void* buffer, size_t bytes)
+	{
+		ASSERT(offset >= 0);
+		if (offset % sector_size() || bytes % sector_size())
+			return BAN::Error::from_errno(EINVAL);
+		if ((size_t)offset == total_size())
+			return 0;
+		TRY(read_sectors(offset / sector_size(), bytes / sector_size(), (uint8_t*)buffer));
+		return bytes;
+	}
+
+	BAN::StringView ATADevice::name() const
+	{
+		static char device_name[] = "sda";
+		device_name[2] += minor(m_rdev);
+		return device_name;
+	}
+
+}

kernel/kernel/Storage/ATAController.cpp

@@ -1,102 +0,0 @@
-#include <BAN/ScopeGuard.h>
-#include <kernel/FS/DevFS/FileSystem.h>
-#include <kernel/LockGuard.h>
-#include <kernel/Storage/ATABus.h>
-#include <kernel/Storage/ATAController.h>
-#include <kernel/Storage/ATADefinitions.h>
-#include <kernel/Storage/ATADevice.h>
-
-#include <sys/sysmacros.h>
-
-namespace Kernel
-{
-
-	BAN::ErrorOr<BAN::RefPtr<ATAController>> ATAController::create(const PCI::Device& device)
-	{
-		ATAController* controller = new ATAController();
-		if (controller == nullptr)
-			return BAN::Error::from_errno(ENOMEM);
-		BAN::ScopeGuard guard([controller] { controller->unref(); });
-		TRY(controller->initialize(device));
-		guard.disable();
-
-		auto ref_ptr = BAN::RefPtr<ATAController>::adopt(controller);
-
-		DevFileSystem::get().add_device("hd"sv, ref_ptr);
-
-		auto devices = controller->devices();
-		for (size_t i = 0; i < devices.size(); i++)
-		{
-			char device_name[4] { 'h', 'd', (char)('a' + i), '\0' };
-			DevFileSystem::get().add_device(device_name, devices[i]);
-
-			if (auto res = devices[i]->initialize_partitions(); res.is_error())
-				dprintln("{}", res.error());
-			else
-			{
-				auto& partitions = devices[i]->partitions();
-				for (size_t j = 0; j < partitions.size(); j++)
-				{
-					char partition_name[5] { 'h', 'd', (char)('a' + i), (char)('1' + j), '\0' };
-					DevFileSystem::get().add_device(partition_name, partitions[j]);
-				}
-			}
-		}
-
-		return ref_ptr;
-	}
-
-	ATAController::ATAController()
-		: m_rdev(makedev(DevFileSystem::get().get_next_dev(), 0))
-	{ }
-
-	BAN::ErrorOr<void> ATAController::initialize(const PCI::Device& pci_device)
-	{
-		struct Bus
-		{
-			uint16_t base;
-			uint16_t ctrl;
-		};
-
-		Bus buses[2];
-		buses[0].base = 0x1F0;
-		buses[0].ctrl = 0x3F6;
-
-		buses[1].base = 0x170;
-		buses[1].ctrl = 0x376;
-
-		uint8_t prog_if = pci_device.read_byte(0x09);
-		if (prog_if & 0x01)
-		{
-			buses[0].base = pci_device.read_dword(0x10) & 0xFFFFFFFC;
-			buses[0].ctrl = pci_device.read_dword(0x14) & 0xFFFFFFFC;
-			return BAN::Error::from_error_code(ErrorCode::ATA_UnsupportedDevice);
-		}
-		if (prog_if & 0x04)
-		{
-			buses[1].base = pci_device.read_dword(0x18) & 0xFFFFFFFC;
-			buses[1].ctrl = pci_device.read_dword(0x1C) & 0xFFFFFFFC;
-			return BAN::Error::from_error_code(ErrorCode::ATA_UnsupportedDevice);
-		}
-
-		m_buses[0] = ATABus::create(*this, buses[0].base, buses[0].ctrl, 14);
-		m_buses[1] = ATABus::create(*this, buses[1].base, buses[1].ctrl, 15);
-
-		return {};
-	}
-
-	BAN::Vector<BAN::RefPtr<StorageDevice>> ATAController::devices()
-	{
-		BAN::Vector<BAN::RefPtr<StorageDevice>> devices;
-		if (m_buses[0]->m_devices[0])
-			MUST(devices.push_back(m_buses[0]->m_devices[0]));
-		if (m_buses[0]->m_devices[1])
-			MUST(devices.push_back(m_buses[0]->m_devices[1]));
-		if (m_buses[1]->m_devices[0])
-			MUST(devices.push_back(m_buses[1]->m_devices[0]));
-		if (m_buses[1]->m_devices[1])
-			MUST(devices.push_back(m_buses[1]->m_devices[1]));
-		return devices;
-	}
-
-}

kernel/kernel/Storage/ATADevice.cpp

@@ -1,86 +0,0 @@
-#include <kernel/FS/DevFS/FileSystem.h>
-#include <kernel/IO.h>
-#include <kernel/Storage/ATABus.h>
-#include <kernel/Storage/ATADefinitions.h>
-#include <kernel/Storage/ATADevice.h>
-
-#include <sys/sysmacros.h>
-
-namespace Kernel
-{
-
-	ATADevice::ATADevice(ATABus& bus)
-		: m_bus(bus)
-		, m_rdev(makedev(DevFileSystem::get().get_next_dev(), 0))
-	{ }
-
-	BAN::ErrorOr<void> ATADevice::initialize(ATABus::DeviceType type, const uint16_t* identify_buffer)
-	{
-		m_type = type;
-
-		m_signature = identify_buffer[ATA_IDENTIFY_SIGNATURE];
-		m_capabilities = identify_buffer[ATA_IDENTIFY_CAPABILITIES];
-
-		m_command_set = 0;
-		m_command_set |= (uint32_t)(identify_buffer[ATA_IDENTIFY_COMMAND_SET + 0] <<  0);
-		m_command_set |= (uint32_t)(identify_buffer[ATA_IDENTIFY_COMMAND_SET + 1] << 16);
-
-		if (!(m_capabilities & ATA_CAPABILITIES_LBA))
-			return BAN::Error::from_error_code(ErrorCode::ATA_NoLBA);
-		
-		if ((identify_buffer[ATA_IDENTIFY_SECTOR_INFO] & (1 << 15)) == 0 &&
-			(identify_buffer[ATA_IDENTIFY_SECTOR_INFO] & (1 << 14)) != 0 &&
-			(identify_buffer[ATA_IDENTIFY_SECTOR_INFO] & (1 << 12)) != 0)
-		{
-			m_sector_words = *(uint32_t*)(identify_buffer + ATA_IDENTIFY_SECTOR_WORDS);
-		}
-		else
-		{
-			m_sector_words = 256;
-		}
-
-		m_lba_count = 0;
-		if (m_command_set & ATA_COMMANDSET_LBA48_SUPPORTED)
-			m_lba_count = *(uint64_t*)(identify_buffer + ATA_IDENTIFY_LBA_COUNT_EXT);
-		if (m_lba_count < (1 << 28))
-			m_lba_count = *(uint32_t*)(identify_buffer + ATA_IDENTIFY_LBA_COUNT);
-
-		for (int i = 0; i < 20; i++)
-		{
-			uint16_t word = identify_buffer[ATA_IDENTIFY_MODEL + i];
-			m_model[2 * i + 0] = word >> 8;
-			m_model[2 * i + 1] = word & 0xFF;
-		}
-		m_model[40] = 0;
-
-		dprintln("ATA disk {} MB", total_size() / 1024 / 1024);
-
-		add_disk_cache();
-
-		return {};
-	}
-
-	BAN::ErrorOr<void> ATADevice::read_sectors_impl(uint64_t lba, uint8_t sector_count, uint8_t* buffer)
-	{
-		TRY(m_bus.read(*this, lba, sector_count, buffer));
-		return {};
-	}
-
-	BAN::ErrorOr<void> ATADevice::write_sectors_impl(uint64_t lba, uint8_t sector_count, const uint8_t* buffer)
-	{
-		TRY(m_bus.write(*this, lba, sector_count, buffer));
-		return {};
-	}
-
-	BAN::ErrorOr<size_t> ATADevice::read_impl(off_t offset, void* buffer, size_t bytes)
-	{
-		ASSERT(offset >= 0);
-		if (offset % sector_size() || bytes % sector_size())
-			return BAN::Error::from_errno(EINVAL);
-		if ((size_t)offset == total_size())
-			return 0;
-		TRY(read_sectors(offset / sector_size(), bytes / sector_size(), (uint8_t*)buffer));
-		return bytes;
-	}
-
-}

kernel/kernel/Storage/StorageDevice.cpp

@@ -2,6 +2,7 @@
 #include <BAN/ScopeGuard.h>
 #include <BAN/StringView.h>
 #include <BAN/UTF8.h>
+#include <kernel/FS/DevFS/FileSystem.h>
 #include <kernel/FS/VirtualFileSystem.h>
 #include <kernel/LockGuard.h>
 #include <kernel/PCI.h>
@@ -184,7 +185,7 @@ namespace Kernel
 			char utf8_name[36 * 4 + 1];
 			BAN::UTF8::from_codepoints(entry.partition_name, 36, utf8_name);
 
-			Partition* partition = new Partition(
+			auto partition = TRY(Partition::create(
 				*this, 
 				entry.partition_type_guid,
 				entry.unique_partition_guid,
@@ -193,14 +194,24 @@ namespace Kernel
 				entry.attributes,
 				utf8_name,
 				i + 1
-			);
+			));
-			ASSERT(partition != nullptr);
+			TRY(m_partitions.push_back(BAN::move(partition)));
-			MUST(m_partitions.push_back(partition));
 		}
 
+		for (auto partition : m_partitions)
+			DevFileSystem::get().add_device(partition);
+
 		return {};
 	}
 
+	BAN::ErrorOr<BAN::RefPtr<Partition>> Partition::create(StorageDevice& device, const GUID& type, const GUID& guid, uint64_t start, uint64_t end, uint64_t attr, const char* label, uint32_t index)
+	{
+		auto partition_ptr = new Partition(device, type, guid, start, end, attr, label, index);
+		if (partition_ptr == nullptr)
+			return BAN::Error::from_errno(ENOMEM);
+		return BAN::RefPtr<Partition>::adopt(partition_ptr);
+	}	
+
 	Partition::Partition(StorageDevice& device, const GUID& type, const GUID& guid, uint64_t start, uint64_t end, uint64_t attr, const char* label, uint32_t index)
 		: BlockDevice(0660, 0, 0)
 		, m_device(device)
@@ -209,6 +220,7 @@ namespace Kernel
 		, m_lba_start(start)
 		, m_lba_end(end)
 		, m_attributes(attr)
+		, m_name(BAN::String::formatted("{}{}", device.name(), index))
 		, m_rdev(makedev(major(device.rdev()), index))
 	{
 		memcpy(m_label, label, sizeof(m_label));

kernel/kernel/Terminal/Serial.cpp

@@ -202,7 +202,7 @@ namespace Kernel
 		}
 
 		auto ref_ptr = BAN::RefPtr<SerialTTY>::adopt(tty);
-		DevFileSystem::get().add_device(ref_ptr->name(), ref_ptr);
+		DevFileSystem::get().add_device(ref_ptr);
 		if (serial.port() == COM1_PORT)
 			s_com1 = ref_ptr;
 		if (serial.port() == COM2_PORT)

kernel/kernel/Terminal/TTY.cpp

@@ -28,11 +28,11 @@ namespace Kernel
 	{
 		s_tty = this;
 
-		auto inode_or_error = DevFileSystem::get().root_inode()->find_inode("tty");
+		auto inode_or_error = DevFileSystem::get().root_inode()->find_inode("tty"sv);
 		if (inode_or_error.is_error())
 		{
 			if (inode_or_error.error().get_error_code() == ENOENT)
-				DevFileSystem::get().add_device("tty"sv, MUST(RamSymlinkInode::create(DevFileSystem::get(), s_tty->name(), 0666, 0, 0)));
+				DevFileSystem::get().add_inode("tty"sv, MUST(RamSymlinkInode::create(DevFileSystem::get(), s_tty->name(), 0666, 0, 0)));
 			else
 				dwarnln("{}", inode_or_error.error());
 			return;

kernel/kernel/Terminal/VirtualTTY.cpp

@@ -33,12 +33,12 @@ namespace Kernel
 
 	BAN::ErrorOr<BAN::RefPtr<VirtualTTY>> VirtualTTY::create(TerminalDriver* driver)
 	{
-		auto* tty = new VirtualTTY(driver);
+		auto* tty_ptr = new VirtualTTY(driver);
-		ASSERT(tty);
+		ASSERT(tty_ptr);
 
-		auto ref_ptr = BAN::RefPtr<VirtualTTY>::adopt(tty);
+		auto tty = BAN::RefPtr<VirtualTTY>::adopt(tty_ptr);
-		DevFileSystem::get().add_device(ref_ptr->name(), ref_ptr);
+		DevFileSystem::get().add_device(tty);
-		return ref_ptr;
+		return tty;
 	}
 
 	VirtualTTY::VirtualTTY(TerminalDriver* driver)

kernel/kernel/kernel.cpp

@@ -177,6 +177,7 @@ static void init2(void*)
 	dprintln("PCI initialized");
 
 	VirtualFileSystem::initialize(cmdline.root);
+	dprintln("VFS initialized");
 
 	if (auto res = PS2Controller::initialize(); res.is_error())
 		dprintln("{}", res.error());