From bde9ff9319c38dcf9d25ede280971ecfe1cd1a15 Mon Sep 17 00:00:00 2001 From: Bananymous Date: Thu, 12 Oct 2023 21:35:25 +0300 Subject: [PATCH] Kernel: Generalize ATA device and cleanup code --- kernel/include/kernel/Storage/ATA/ATABus.h | 4 +- .../kernel/Storage/ATA/ATAController.h | 2 +- kernel/include/kernel/Storage/ATA/ATADevice.h | 85 +++++++++++------- .../kernel/Storage/StorageController.h | 4 +- kernel/include/kernel/Storage/StorageDevice.h | 8 +- kernel/kernel/Storage/ATA/ATABus.cpp | 34 ++++--- kernel/kernel/Storage/ATA/ATAController.cpp | 7 +- kernel/kernel/Storage/ATA/ATADevice.cpp | 90 +++++++++++-------- kernel/kernel/Storage/StorageDevice.cpp | 6 +- 9 files changed, 135 insertions(+), 105 deletions(-) diff --git a/kernel/include/kernel/Storage/ATA/ATABus.h b/kernel/include/kernel/Storage/ATA/ATABus.h index 06894e5f00..3c781d6b5f 100644 --- a/kernel/include/kernel/Storage/ATA/ATABus.h +++ b/kernel/include/kernel/Storage/ATA/ATABus.h @@ -27,8 +27,6 @@ namespace Kernel virtual void handle_irq() override; - void initialize_devfs(); - private: ATABus(uint16_t base, uint16_t ctrl) : m_base(base) @@ -54,7 +52,7 @@ namespace Kernel const uint16_t m_ctrl; SpinLock m_lock; - bool m_has_got_irq { false }; + volatile bool m_has_got_irq { false }; // Non-owning pointers BAN::Vector m_devices; diff --git a/kernel/include/kernel/Storage/ATA/ATAController.h b/kernel/include/kernel/Storage/ATA/ATAController.h index df6576c20b..822782876f 100644 --- a/kernel/include/kernel/Storage/ATA/ATAController.h +++ b/kernel/include/kernel/Storage/ATA/ATAController.h @@ -12,7 +12,7 @@ namespace Kernel class ATAController : public StorageController { public: - static BAN::ErrorOr> create(PCI::Device&); + static BAN::ErrorOr> create(PCI::Device&); virtual BAN::ErrorOr initialize() override; private: diff --git a/kernel/include/kernel/Storage/ATA/ATADevice.h b/kernel/include/kernel/Storage/ATA/ATADevice.h index 8851f2e1f7..f9f7ce1ed1 100644 --- a/kernel/include/kernel/Storage/ATA/ATADevice.h +++ b/kernel/include/kernel/Storage/ATA/ATADevice.h @@ -6,52 +6,69 @@ namespace Kernel { - class ATADevice final : public StorageDevice + namespace detail + { + + class ATABaseDevice : public StorageDevice + { + public: + enum class Command + { + Read, + Write + }; + + public: + virtual ~ATABaseDevice() {}; + + 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(); } + + 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; + + virtual dev_t rdev() const override { return m_rdev; } + + virtual BAN::ErrorOr read_impl(off_t, void*, size_t) override; + virtual BAN::ErrorOr write_impl(off_t, const void*, size_t) override; + + protected: + ATABaseDevice(); + BAN::ErrorOr initialize(BAN::Span identify_data); + + protected: + uint16_t m_signature; + uint16_t m_capabilities; + uint32_t m_command_set; + uint32_t m_sector_words; + uint64_t m_lba_count; + char m_model[41]; + + const dev_t m_rdev; + }; + + } + + class ATADevice final : public detail::ATABaseDevice { public: static BAN::ErrorOr> create(BAN::RefPtr, ATABus::DeviceType, bool is_secondary, BAN::Span identify_data); - 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 read_sectors_impl(uint64_t, uint8_t, uint8_t*) override; - virtual BAN::ErrorOr write_sectors_impl(uint64_t, uint8_t, const uint8_t*) override; private: ATADevice(BAN::RefPtr, ATABus::DeviceType, bool is_secodary); - BAN::ErrorOr initialize(BAN::Span identify_data); + + virtual BAN::ErrorOr read_sectors_impl(uint64_t, uint64_t, uint8_t*) override; + virtual BAN::ErrorOr write_sectors_impl(uint64_t, uint64_t, const uint8_t*) override; private: BAN::RefPtr m_bus; const ATABus::DeviceType m_type; const bool m_is_secondary; - - uint16_t m_signature; - uint16_t m_capabilities; - uint32_t m_command_set; - uint32_t m_sector_words; - uint64_t m_lba_count; - char m_model[41]; - - public: - virtual Mode mode() const override { return { Mode::IFBLK | Mode::IRUSR | Mode::IRGRP }; } - 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; } - - private: - virtual BAN::ErrorOr read_impl(off_t, void*, size_t) override; - - public: - const dev_t m_rdev; }; -} \ No newline at end of file +} diff --git a/kernel/include/kernel/Storage/StorageController.h b/kernel/include/kernel/Storage/StorageController.h index c619992083..86eec0b66e 100644 --- a/kernel/include/kernel/Storage/StorageController.h +++ b/kernel/include/kernel/Storage/StorageController.h @@ -1,9 +1,11 @@ #pragma once +#include + namespace Kernel { - class StorageController + class StorageController : public BAN::RefCounted { public: virtual ~StorageController() {} diff --git a/kernel/include/kernel/Storage/StorageDevice.h b/kernel/include/kernel/Storage/StorageDevice.h index c4cf11311c..15a32903e1 100644 --- a/kernel/include/kernel/Storage/StorageDevice.h +++ b/kernel/include/kernel/Storage/StorageDevice.h @@ -73,8 +73,8 @@ namespace Kernel BAN::ErrorOr initialize_partitions(); - BAN::ErrorOr read_sectors(uint64_t lba, uint8_t sector_count, uint8_t* buffer); - BAN::ErrorOr write_sectors(uint64_t lba, uint8_t sector_count, const uint8_t* buffer); + BAN::ErrorOr read_sectors(uint64_t lba, uint64_t sector_count, uint8_t* buffer); + BAN::ErrorOr write_sectors(uint64_t lba, uint64_t sector_count, const uint8_t* buffer); virtual uint32_t sector_size() const = 0; virtual uint64_t total_size() const = 0; @@ -86,8 +86,8 @@ namespace Kernel virtual bool is_storage_device() const override { return true; } protected: - virtual BAN::ErrorOr read_sectors_impl(uint64_t lba, uint8_t sector_count, uint8_t* buffer) = 0; - virtual BAN::ErrorOr write_sectors_impl(uint64_t lba, uint8_t sector_count, const uint8_t* buffer) = 0; + virtual BAN::ErrorOr read_sectors_impl(uint64_t lba, uint64_t sector_count, uint8_t* buffer) = 0; + virtual BAN::ErrorOr write_sectors_impl(uint64_t lba, uint64_t sector_count, const uint8_t* buffer) = 0; void add_disk_cache(); private: diff --git a/kernel/kernel/Storage/ATA/ATABus.cpp b/kernel/kernel/Storage/ATA/ATABus.cpp index abe125ed19..eb9d28a667 100644 --- a/kernel/kernel/Storage/ATA/ATABus.cpp +++ b/kernel/kernel/Storage/ATA/ATABus.cpp @@ -41,6 +41,10 @@ namespace Kernel else device_type = res.value(); + // Enable interrupts + select_device(is_secondary); + io_write(ATA_PORT_CONTROL, 0); + auto device_or_error = ATADevice::create(this, device_type, is_secondary, identify_buffer.span()); if (device_or_error.is_error()) @@ -50,35 +54,23 @@ namespace Kernel } auto device = device_or_error.release_value(); - device->ref(); TRY(m_devices.push_back(device.ptr())); } - // Enable disk interrupts - for (auto& device : m_devices) - { - select_device(device->is_secondary()); - io_write(ATA_PORT_CONTROL, 0); - } - return {}; } - void ATABus::initialize_devfs() + static void select_delay() { - for (auto& device : m_devices) - { - DevFileSystem::get().add_device(device); - if (auto res = device->initialize_partitions(); res.is_error()) - dprintln("{}", res.error()); - device->unref(); - } + auto time = SystemTimer::get().ns_since_boot(); + while (SystemTimer::get().ns_since_boot() < time + 400) + continue; } void ATABus::select_device(bool secondary) { io_write(ATA_PORT_DRIVE_SELECT, 0xA0 | ((uint8_t)secondary << 4)); - SystemTimer::get().sleep(1); + select_delay(); } BAN::ErrorOr ATABus::identify(bool secondary, BAN::Span buffer) @@ -236,6 +228,9 @@ namespace Kernel { // LBA28 io_write(ATA_PORT_DRIVE_SELECT, 0xE0 | ((uint8_t)device.is_secondary() << 4) | ((lba >> 24) & 0x0F)); + select_delay(); + io_write(ATA_PORT_CONTROL, 0); + 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)); @@ -268,14 +263,15 @@ namespace Kernel { // LBA28 io_write(ATA_PORT_DRIVE_SELECT, 0xE0 | ((uint8_t)device.is_secondary() << 4) | ((lba >> 24) & 0x0F)); + select_delay(); + io_write(ATA_PORT_CONTROL, 0); + 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)); io_write(ATA_PORT_LBA2, (uint8_t)(lba >> 16)); io_write(ATA_PORT_COMMAND, ATA_COMMAND_WRITE_SECTORS); - SystemTimer::get().sleep(1); - for (uint32_t sector = 0; sector < sector_count; sector++) { write_buffer(ATA_PORT_DATA, (uint16_t*)buffer + sector * device.words_per_sector(), device.words_per_sector()); diff --git a/kernel/kernel/Storage/ATA/ATAController.cpp b/kernel/kernel/Storage/ATA/ATAController.cpp index 312ff93381..5f9685664b 100644 --- a/kernel/kernel/Storage/ATA/ATAController.cpp +++ b/kernel/kernel/Storage/ATA/ATAController.cpp @@ -7,7 +7,7 @@ namespace Kernel { - BAN::ErrorOr> ATAController::create(PCI::Device& pci_device) + BAN::ErrorOr> ATAController::create(PCI::Device& pci_device) { StorageController* controller_ptr = nullptr; @@ -29,7 +29,7 @@ namespace Kernel if (controller_ptr == nullptr) return BAN::Error::from_errno(ENOMEM); - auto controller = BAN::UniqPtr::adopt(controller_ptr); + auto controller = BAN::RefPtr::adopt(controller_ptr); TRY(controller->initialize()); return controller; } @@ -67,9 +67,6 @@ namespace Kernel dprintln("unsupported IDE ATABus in native mode"); } - for (auto& bus : buses) - bus->initialize_devfs(); - return {}; } diff --git a/kernel/kernel/Storage/ATA/ATADevice.cpp b/kernel/kernel/Storage/ATA/ATADevice.cpp index e5dfb24844..357018fad0 100644 --- a/kernel/kernel/Storage/ATA/ATADevice.cpp +++ b/kernel/kernel/Storage/ATA/ATADevice.cpp @@ -21,24 +21,11 @@ namespace Kernel return minor++; } - BAN::ErrorOr> ATADevice::create(BAN::RefPtr bus, ATABus::DeviceType type, bool is_secondary, BAN::Span 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::adopt(device_ptr); - TRY(device->initialize(identify_data)); - return device; - } - - ATADevice::ATADevice(BAN::RefPtr 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())) + detail::ATABaseDevice::ATABaseDevice() + : m_rdev(makedev(get_ata_dev_major(), get_ata_dev_minor())) { } - BAN::ErrorOr ATADevice::initialize(BAN::Span identify_data) + BAN::ErrorOr detail::ATABaseDevice::initialize(BAN::Span identify_data) { ASSERT(identify_data.size() >= 256); @@ -77,41 +64,74 @@ namespace Kernel } m_model[40] = 0; - dprintln("ATA disk {} MB", total_size() / 1024 / 1024); + size_t model_len = 40; + while (model_len > 0 && m_model[model_len - 1] == ' ') + model_len--; + + dprintln("Initialized disk '{}' {} MB", BAN::StringView(m_model, model_len), total_size() / 1024 / 1024); add_disk_cache(); + DevFileSystem::get().add_device(this); + if (auto res = initialize_partitions(); res.is_error()) + dprintln("{}", res.error()); + return {}; } - BAN::ErrorOr ATADevice::read_sectors_impl(uint64_t lba, uint8_t sector_count, uint8_t* buffer) + BAN::ErrorOr detail::ATABaseDevice::read_impl(off_t offset, void* buffer, size_t bytes) { - TRY(m_bus->read(*this, lba, sector_count, buffer)); - return {}; - } - - BAN::ErrorOr 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 ATADevice::read_impl(off_t offset, void* buffer, size_t bytes) - { - ASSERT(offset >= 0); - if (offset % sector_size() || bytes % sector_size()) + if (offset % sector_size()) + return BAN::Error::from_errno(EINVAL); + if (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 + BAN::ErrorOr detail::ATABaseDevice::write_impl(off_t offset, const void* buffer, size_t bytes) + { + if (offset % sector_size()) + return BAN::Error::from_errno(EINVAL); + if (bytes % sector_size()) + return BAN::Error::from_errno(EINVAL); + TRY(write_sectors(offset / sector_size(), bytes / sector_size(), (const uint8_t*)buffer)); + return bytes; + } + + BAN::StringView detail::ATABaseDevice::name() const { static char device_name[] = "sda"; device_name[2] += minor(m_rdev); return device_name; } + BAN::ErrorOr> ATADevice::create(BAN::RefPtr bus, ATABus::DeviceType type, bool is_secondary, BAN::Span 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::adopt(device_ptr); + TRY(device->initialize(identify_data)); + return device; + } + + ATADevice::ATADevice(BAN::RefPtr bus, ATABus::DeviceType type, bool is_secondary) + : m_bus(bus) + , m_type(type) + , m_is_secondary(is_secondary) + { } + + BAN::ErrorOr ATADevice::read_sectors_impl(uint64_t lba, uint64_t sector_count, uint8_t* buffer) + { + TRY(m_bus->read(*this, lba, sector_count, buffer)); + return {}; + } + + BAN::ErrorOr ATADevice::write_sectors_impl(uint64_t lba, uint64_t sector_count, const uint8_t* buffer) + { + TRY(m_bus->write(*this, lba, sector_count, buffer)); + return {}; + } + } \ No newline at end of file diff --git a/kernel/kernel/Storage/StorageDevice.cpp b/kernel/kernel/Storage/StorageDevice.cpp index 7c517d739d..a64b656a20 100644 --- a/kernel/kernel/Storage/StorageDevice.cpp +++ b/kernel/kernel/Storage/StorageDevice.cpp @@ -283,9 +283,9 @@ namespace Kernel return {}; } - BAN::ErrorOr StorageDevice::read_sectors(uint64_t lba, uint8_t sector_count, uint8_t* buffer) + BAN::ErrorOr StorageDevice::read_sectors(uint64_t lba, uint64_t sector_count, uint8_t* buffer) { - for (uint8_t offset = 0; offset < sector_count; offset++) + for (uint64_t offset = 0; offset < sector_count; offset++) { LockGuard _(m_lock); Thread::TerminateBlocker blocker(Thread::current()); @@ -302,7 +302,7 @@ namespace Kernel return {}; } - BAN::ErrorOr StorageDevice::write_sectors(uint64_t lba, uint8_t sector_count, const uint8_t* buffer) + BAN::ErrorOr StorageDevice::write_sectors(uint64_t lba, uint64_t sector_count, const uint8_t* buffer) { // TODO: use disk cache for dirty pages. I don't wanna think about how to do it safely now for (uint8_t offset = 0; offset < sector_count; offset++)