From 8beb8b758c33cf1ac5357b31296927e7df8cf971 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matteo=20Gm=C3=BCr?= Date: Mon, 14 Oct 2024 08:15:16 +0000 Subject: Fix typos, implementation in header and missing doxygen --- .../x86_64/include/arch/memory/frame_allocator.hpp | 122 ++++++++++++--------- arch/x86_64/src/kernel/main.cpp | 41 ++++--- arch/x86_64/src/memory/frame_allocator.cpp | 32 ++++++ 3 files changed, 125 insertions(+), 70 deletions(-) diff --git a/arch/x86_64/include/arch/memory/frame_allocator.hpp b/arch/x86_64/include/arch/memory/frame_allocator.hpp index fa22ce5..1fd1f08 100644 --- a/arch/x86_64/include/arch/memory/frame_allocator.hpp +++ b/arch/x86_64/include/arch/memory/frame_allocator.hpp @@ -10,7 +10,7 @@ namespace teachos::arch::memory { namespace { - const std::size_t PAGE_FRAME_SIZE = 4096U; + constexpr std::size_t PAGE_FRAME_SIZE = 4096U; ///< Default page size of x86_84 is always 4KiB } /** @@ -18,18 +18,12 @@ namespace teachos::arch::memory */ struct frame { - std::size_t frame_number; ///< Index number of the current frame, used to distinguish it from other frames - /** * @brief Constructor * * @param frame_number Index number that should be assigned to this frame */ - frame(std::size_t frame_number) - : frame_number(frame_number) - { - // Nothing to do - } + frame(std::size_t frame_number); /** * @brief Returns the frame the given address is contained in @@ -37,7 +31,7 @@ namespace teachos::arch::memory * @param address Address we want to get the corresponding frame for * @return Frame the given address is contained in */ - static auto containing_address(std::size_t address) -> frame { return frame{address / PAGE_FRAME_SIZE}; } + static auto containing_address(std::size_t address) -> frame; /** * @brief Defaulted equals operator @@ -48,6 +42,8 @@ namespace teachos::arch::memory * @brief Defaulted three-way comparsion operator */ constexpr auto operator<=>(const frame & other) const -> std::partial_ordering = default; + + std::size_t frame_number; ///< Index number of the current frame, used to distinguish it from other frames }; template @@ -57,36 +53,48 @@ namespace teachos::arch::memory }; /** - * @brief Iterator for memory areas + * @brief Iterator for memory areas. */ - class memory_area_iterator + struct memory_area_iterator { - memory_area * ptr; + /** + * @brief Constructor + * + * @param p Underlying address the iterator should point too, ensure to not pass an invalid pointer + */ + explicit memory_area_iterator(memory_area * p); - public: - std::size_t begin; - std::size_t end; + /** + * @brief Dereferences the initally given pointer to its value. + * + * @return Reference to the value + */ + memory_area & operator*() const; - explicit memory_area_iterator(memory_area * p) - : ptr(p) - { - } + /** + * @brief Post increment operator. Returns a copy of the value + * + * @return Copy of the incremented underlying address + */ + memory_area_iterator operator++(int); - memory_area & operator*() const { return *ptr; } - memory_area_iterator & operator++() - { - ++ptr; - return *this; - } + /** + * @brief Pre increment operator. Returns a reference to the changed value + * + * @return Reference to the incremented underlying address + */ + memory_area_iterator & operator++(); - memory_area_iterator operator++(int) - { - memory_area_iterator temp = *this; - ++(*this); - return temp; - } + /** + * @brief Defaulted comparsion operator. Simply compares the memory address of both iterators. + * + * @param other Other iterator to compare to + * @return Whether poith iterators point to the same underlying address in memory + */ + bool operator==(memory_area_iterator const & other) const = default; - bool operator==(const memory_area_iterator & other) const { return ptr == other.ptr; } + private: + memory_area * ptr; ///< Underlying address the iterator is currently pointing too }; /** @@ -94,18 +102,6 @@ namespace teachos::arch::memory */ struct area_frame_allocator { - frame next_free_frame{0}; //!< The frame after the last allocated one - std::optional current_area{std::nullopt}; //!< The current memory area - memory_area * areas; //!< Pointer to the first element of all memory areas - frame kernel_start; //!< The start address of the kernel code in memory - frame kernel_end; //!< The end address of the kernel code in memory - frame multiboot_start; //!< The start address of the multiboot code in memory - frame multiboot_end; //!< The end address of the multiboot code in memory - - private: - uint8_t N; - - public: /** * @brief Constructor * @@ -114,15 +110,16 @@ namespace teachos::arch::memory * @param multiboot_start Start address of the multiboot code in memory * @param multiboot_end End address of the multiboot code in memory * @param memory_areas Pointer to the first element of all memory areas + * @param area_count Amount of total entries in the memory_areas array */ area_frame_allocator(std::size_t kernel_start, std::size_t kernel_end, std::size_t multiboot_start, std::size_t multiboot_end, memory_area * memory_areas, uint8_t area_count) - : areas(memory_areas) - , kernel_start(frame{kernel_start}) - , kernel_end(frame{kernel_end}) - , multiboot_start(frame{multiboot_start}) - , multiboot_end(frame{multiboot_end}) - , N(area_count) + : area_begin(memory_areas) + , area_end(memory_areas + area_count) + , kernel_start(frame::containing_address(kernel_start)) + , kernel_end(frame::containing_address(kernel_end)) + , multiboot_start(frame::containing_address(multiboot_start)) + , multiboot_end(frame::containing_address(multiboot_end)) { choose_next_area(); } @@ -146,15 +143,36 @@ namespace teachos::arch::memory */ auto deallocate_frame(frame frame) -> void; - memory_area_iterator begin() { return memory_area_iterator(areas); } + /** + * @brief Returns the iterator pointing to the first element of the memory area. + * Allows using this class in the for each loop, because it follows the InputIterator template scheme + * + * @return Iterator pointing to first element of the memory area + */ + auto begin() -> memory_area_iterator; - memory_area_iterator end() { return memory_area_iterator(areas + N); } + /** + * @brief Returns the iterator pointing to one past the last element of the memory area. + * Allows using this class in the for each loop, because it follows the InputIterator template scheme + * + * @return Iterator pointing to one past the last element of the memory area + */ + auto end() -> memory_area_iterator; private: /** * @brief Find the next memory area and write it into current_area */ auto choose_next_area() -> void; + + frame next_free_frame{0}; ///< The frame after the last allocated one + std::optional current_area{std::nullopt}; ///< The current memory area + memory_area_iterator area_begin; ///< Pointer to the first element of all memory areas + memory_area_iterator area_end; ///< Pointer to one pas the last element of all memory areas + frame const kernel_start; ///< The start address of the kernel code in memory + frame const kernel_end; ///< The end address of the kernel code in memory + frame const multiboot_start; ///< The start address of the multiboot code in memory + frame const multiboot_end; ///< The end address of the multiboot code in memory }; } // namespace teachos::arch::memory diff --git a/arch/x86_64/src/kernel/main.cpp b/arch/x86_64/src/kernel/main.cpp index 3fa44d3..1289eb6 100644 --- a/arch/x86_64/src/kernel/main.cpp +++ b/arch/x86_64/src/kernel/main.cpp @@ -16,24 +16,28 @@ namespace teachos::arch::kernel // to prevent the while loop being optimized away // See // https://stackoverflow.com/questions/9495856/how-to-prevent-g-from-optimizing-out-a-loop-controlled-by-a-variable-that-can - // for mroe information. + // for more information. asm volatile("" : "+g"(condition)); } } - auto process_memory_map(arch::memory::memory_map * mminfo, arch::memory::memory_area ** memory_areas, - uint8_t * area_count) -> void + auto process_memory_map(arch::memory::memory_map * mminfo, arch::memory::memory_area *& memory_areas, + uint8_t & area_count) -> void { auto expected_entry_size = mminfo->entry_size; constexpr auto actual_entry_size = sizeof(arch::memory::memory_area); assert(expected_entry_size == actual_entry_size); - *memory_areas = &mminfo->entries; - *area_count = sizeof(mminfo->entries) / mminfo->entry_size; + auto total_size = mminfo->tag.size; + auto total_entries_size = total_size - sizeof(arch::memory::memory_map) + actual_entry_size; + auto number_of_entries = total_entries_size / actual_entry_size; + + memory_areas = &mminfo->entries; + area_count = number_of_entries; } - auto process_elf_sections(arch::memory::elf_symbols_section * symbol, uint64_t * kernel_start, - uint64_t * kernel_end) -> void + auto process_elf_sections(arch::memory::elf_symbols_section * symbol, uint64_t & kernel_start, + uint64_t & kernel_end) -> void { // Validate ELF sections auto expected_entry_size = symbol->entry_size; @@ -58,14 +62,14 @@ namespace teachos::arch::kernel switch (section->type) { case arch::memory::elf_section_type::PROGRAMM: - if (section->virtual_address < *kernel_start) + if (section->virtual_address < kernel_start) { - *kernel_start = section->virtual_address; + kernel_start = section->virtual_address; } - if (section->virtual_address + section->section_size > *kernel_end) + if (section->virtual_address + section->section_size > kernel_end) { - *kernel_end = section->virtual_address + section->section_size; + kernel_end = section->virtual_address + section->section_size; } break; case arch::memory::elf_section_type::DYNAMIC_SYMBOL_TABLE: @@ -73,7 +77,6 @@ namespace teachos::arch::kernel symbol_table_section_count++; break; case arch::memory::elf_section_type::DYNAMIC: - symbol_table_section_count++; dynamic_section_count++; break; default: @@ -94,8 +97,8 @@ namespace teachos::arch::kernel text::cursor(false); text::write("TeachOS is starting up...", text::common_attributes::green_on_black); - arch::memory::multi_boot_info * multiboot_information_pointer = - (arch::memory::multi_boot_info *)arch::boot::multiboot_information_pointer; + auto * multiboot_information_pointer = + reinterpret_cast(arch::boot::multiboot_information_pointer); auto multiboot_tag = &(multiboot_information_pointer->tags); uint64_t kernel_start = UINT64_MAX; @@ -113,15 +116,17 @@ namespace teachos::arch::kernel * The increment part aligns the size to an 8-byte address. */ for (auto tag = multiboot_tag; tag->type != arch::memory::multi_boot_tag_type::END; - tag = (arch::memory::multi_boot_tag *)(((uint8_t *)tag) + ((tag->size + 7) & ~7))) + tag = reinterpret_cast((reinterpret_cast(tag)) + + ((tag->size + 7) & ~7))) { switch (tag->type) { case arch::memory::multi_boot_tag_type::ELF_SECTIONS: - process_elf_sections((arch::memory::elf_symbols_section *)tag, &kernel_start, &kernel_end); + process_elf_sections(reinterpret_cast(tag), kernel_start, + kernel_end); break; case arch::memory::multi_boot_tag_type::MEMORY_MAP: - process_memory_map((arch::memory::memory_map *)tag, &memory_areas, &area_count); + process_memory_map(reinterpret_cast(tag), memory_areas, area_count); break; default: // All other cases are not important and can be ignored @@ -140,7 +145,7 @@ namespace teachos::arch::kernel // Address of Frame: 0x203F00 auto allocator = arch::memory::area_frame_allocator(kernel_start, kernel_end, multiboot_start, multiboot_end, memory_areas, area_count); - std::optional allocated = allocator.allocate_frame(); + auto allocated = allocator.allocate_frame(); // WATCH OUT: using optional::value() crashes the build... I think its because of missing exception handling if (allocated.has_value()) diff --git a/arch/x86_64/src/memory/frame_allocator.cpp b/arch/x86_64/src/memory/frame_allocator.cpp index 829cd6d..3ad2d96 100644 --- a/arch/x86_64/src/memory/frame_allocator.cpp +++ b/arch/x86_64/src/memory/frame_allocator.cpp @@ -2,6 +2,34 @@ namespace teachos::arch::memory { + frame::frame(std::size_t frame_number) + : frame_number(frame_number) + { + // Nothing to do + } + + auto frame::containing_address(std::size_t address) -> frame { return frame{address / PAGE_FRAME_SIZE}; } + + memory_area_iterator::memory_area_iterator(memory_area * p) + : ptr(p) + { + // Nothing to do + } + + memory_area & memory_area_iterator::operator*() const { return *ptr; } + + auto memory_area_iterator::operator++(int) -> memory_area_iterator + { + ++(*this); + return *this; + } + + auto memory_area_iterator::operator++() -> memory_area_iterator & + { + ++ptr; + return *this; + } + auto area_frame_allocator::choose_next_area() -> void { current_area = std::nullopt; @@ -77,4 +105,8 @@ namespace teachos::arch::memory { } } + + auto area_frame_allocator::begin() -> memory_area_iterator { return area_begin; } + + auto area_frame_allocator::end() -> memory_area_iterator { return area_end; } } // namespace teachos::arch::memory -- cgit v1.2.3