From f9a967caf78e7968722e75424e1f5aef534ae6c4 Mon Sep 17 00:00:00 2001 From: "Justin C. Miller" Date: Fri, 19 Feb 2021 20:42:49 -0800 Subject: [PATCH] [kutil] Make enum bitfields usable in other scopes Changing the SFINAE/enable_if strategy from a type to a constexpr function means that it can be defined in other scopes than the functions themselves, because of function overloading. This lets us put everything into the kutil::bitfields namespace, and make bitfields out of enums in other namespaces. Also took the chance to clean up the implementation a bit. --- src/kernel/acpi_tables.h | 2 +- src/kernel/apic.h | 36 +++--- src/kernel/gdt.cpp | 44 +++---- src/kernel/gdt.h | 33 ++--- src/kernel/main.cpp | 4 +- src/kernel/memory_bootstrap.cpp | 9 +- src/kernel/objects/vm_area.h | 2 +- src/kernel/page_table.h | 2 +- src/kernel/vm_space.h | 2 +- .../kutil/include/kutil/enum_bitfields.h | 114 +++++------------- 10 files changed, 101 insertions(+), 147 deletions(-) diff --git a/src/kernel/acpi_tables.h b/src/kernel/acpi_tables.h index 806315f..b488ddd 100644 --- a/src/kernel/acpi_tables.h +++ b/src/kernel/acpi_tables.h @@ -84,7 +84,7 @@ enum class acpi_fadt_flags : uint32_t hw_reduced_acpi = 0x00100000, low_pwr_s0_idle = 0x00200000 }; -IS_BITFIELD(acpi_fadt_flags); +is_bitfield(acpi_fadt_flags); struct acpi_fadt { diff --git a/src/kernel/apic.h b/src/kernel/apic.h index f15982a..3c52113 100644 --- a/src/kernel/apic.h +++ b/src/kernel/apic.h @@ -19,23 +19,6 @@ protected: uint32_t *m_base; }; -enum class ipi : uint32_t -{ - // Delivery modes - fixed = 0x0000, - smi = 0x0200, - nmi = 0x0400, - init = 0x0500, - startup = 0x0600, - - // Flags - deassert = 0x0000, - assert = 0x4000, - edge = 0x0000, ///< edge-triggered - level = 0x8000, ///< level-triggered -}; -IS_BITFIELD(ipi); - /// Controller for processor-local APICs class lapic : public apic @@ -48,6 +31,22 @@ public: /// Get the local APIC's ID uint8_t get_id(); + enum class ipi : uint32_t + { + // Delivery modes + fixed = 0x0000, + smi = 0x0200, + nmi = 0x0400, + init = 0x0500, + startup = 0x0600, + + // Flags + deassert = 0x0000, + assert = 0x4000, + edge = 0x0000, ///< edge-triggered + level = 0x8000, ///< level-triggered + }; + /// Send an inter-processor interrupt. /// \arg mode The sending mode /// \arg vector The interrupt vector @@ -144,3 +143,6 @@ private: uint8_t m_id; uint8_t m_version; }; + +is_bitfield(lapic::ipi); + diff --git a/src/kernel/gdt.cpp b/src/kernel/gdt.cpp index 389f3e4..8f7b8a2 100644 --- a/src/kernel/gdt.cpp +++ b/src/kernel/gdt.cpp @@ -34,13 +34,13 @@ GDT::GDT(TSS *tss) : m_ptr.base = &m_entries[0]; // Kernel CS/SS - always 64bit - set(kern_cs_index, 0, 0xfffff, true, gdt_type::read_write | gdt_type::execute); - set(kern_ss_index, 0, 0xfffff, true, gdt_type::read_write); + set(kern_cs_index, 0, 0xfffff, true, type::read_write | type::execute); + set(kern_ss_index, 0, 0xfffff, true, type::read_write); // User CS32/SS/CS64 - layout expected by SYSRET - set(user_cs32_index, 0, 0xfffff, false, gdt_type::ring3 | gdt_type::read_write | gdt_type::execute); - set(user_ss_index, 0, 0xfffff, true, gdt_type::ring3 | gdt_type::read_write); - set(user_cs64_index, 0, 0xfffff, true, gdt_type::ring3 | gdt_type::read_write | gdt_type::execute); + set(user_cs32_index, 0, 0xfffff, false, type::ring3 | type::read_write | type::execute); + set(user_ss_index, 0, 0xfffff, true, type::ring3 | type::read_write); + set(user_cs64_index, 0, 0xfffff, true, type::ring3 | type::read_write | type::execute); set_tss(tss); } @@ -63,7 +63,7 @@ GDT::install() const } void -GDT::set(uint8_t i, uint32_t base, uint64_t limit, bool is64, gdt_type type) +GDT::set(uint8_t i, uint32_t base, uint64_t limit, bool is64, type t) { m_entries[i].limit_low = limit & 0xffff; m_entries[i].size = (limit >> 16) & 0xf; @@ -73,7 +73,7 @@ GDT::set(uint8_t i, uint32_t base, uint64_t limit, bool is64, gdt_type type) m_entries[i].base_mid = (base >> 16) & 0xff; m_entries[i].base_high = (base >> 24) & 0xff; - m_entries[i].type = type | gdt_type::system | gdt_type::present; + m_entries[i].type = t | type::system | type::present; } struct tss_descriptor @@ -81,7 +81,7 @@ struct tss_descriptor uint16_t limit_low; uint16_t base_00; uint8_t base_16; - gdt_type type; + GDT::type type; uint8_t size; uint8_t base_24; uint32_t base_32; @@ -105,10 +105,10 @@ GDT::set_tss(TSS *tss) tssd.reserved = 0; tssd.type = - gdt_type::accessed | - gdt_type::execute | - gdt_type::ring3 | - gdt_type::present; + type::accessed | + type::execute | + type::ring3 | + type::present; kutil::memcpy(&m_entries[tss_index], &tssd, sizeof(tss_descriptor)); } @@ -141,26 +141,26 @@ GDT::dump(unsigned index) const gdt[i].limit_low; cons->printf(" %02d:", i); - if (! bitfield_has(gdt[i].type, gdt_type::present)) { + if (! (gdt[i].type && type::present)) { cons->puts(" Not Present\n"); continue; } cons->printf(" Base %08x limit %05x ", base, limit); - switch (gdt[i].type & gdt_type::ring3) { - case gdt_type::ring3: cons->puts("ring3"); break; - case gdt_type::ring2: cons->puts("ring2"); break; - case gdt_type::ring1: cons->puts("ring1"); break; + switch (gdt[i].type & type::ring3) { + case type::ring3: cons->puts("ring3"); break; + case type::ring2: cons->puts("ring2"); break; + case type::ring1: cons->puts("ring1"); break; default: cons->puts("ring0"); break; } cons->printf(" %s %s %s %s %s %s %s\n", - bitfield_has(gdt[i].type, gdt_type::accessed) ? "A" : " ", - bitfield_has(gdt[i].type, gdt_type::read_write) ? "RW" : " ", - bitfield_has(gdt[i].type, gdt_type::conforming) ? "C" : " ", - bitfield_has(gdt[i].type, gdt_type::execute) ? "EX" : " ", - bitfield_has(gdt[i].type, gdt_type::system) ? "S" : " ", + (gdt[i].type && type::accessed) ? "A" : " ", + (gdt[i].type && type::read_write) ? "RW" : " ", + (gdt[i].type && type::conforming) ? "C" : " ", + (gdt[i].type && type::execute) ? "EX" : " ", + (gdt[i].type && type::system) ? "S" : " ", (gdt[i].size & 0x80) ? "KB" : " B", (gdt[i].size & 0x60) == 0x20 ? "64" : (gdt[i].size & 0x60) == 0x40 ? "32" : "16"); diff --git a/src/kernel/gdt.h b/src/kernel/gdt.h index 984636b..e78ce87 100644 --- a/src/kernel/gdt.h +++ b/src/kernel/gdt.h @@ -7,20 +7,6 @@ class TSS; -enum class gdt_type : uint8_t -{ - accessed = 0x01, - read_write = 0x02, - conforming = 0x04, - execute = 0x08, - system = 0x10, - ring1 = 0x20, - ring2 = 0x40, - ring3 = 0x60, - present = 0x80 -}; -IS_BITFIELD(gdt_type); - class GDT { public: @@ -39,8 +25,21 @@ public: /// \arg index Which entry to print, or -1 for all entries void dump(unsigned index = -1) const; + enum class type : uint8_t + { + accessed = 0x01, + read_write = 0x02, + conforming = 0x04, + execute = 0x08, + system = 0x10, + ring1 = 0x20, + ring2 = 0x40, + ring3 = 0x60, + present = 0x80 + }; + private: - void set(uint8_t i, uint32_t base, uint64_t limit, bool is64, gdt_type type); + void set(uint8_t i, uint32_t base, uint64_t limit, bool is64, type t); void set_tss(TSS *tss); struct descriptor @@ -48,7 +47,7 @@ private: uint16_t limit_low; uint16_t base_low; uint8_t base_mid; - gdt_type type; + type type; uint8_t size; uint8_t base_high; } __attribute__ ((packed, align(8))); @@ -64,3 +63,5 @@ private: ptr m_ptr; }; + +is_bitfield(GDT::type); diff --git a/src/kernel/main.cpp b/src/kernel/main.cpp index 577fcc2..626acd4 100644 --- a/src/kernel/main.cpp +++ b/src/kernel/main.cpp @@ -263,7 +263,7 @@ start_aps(lapic &apic, const kutil::vector &ids, void *kpml4) size_t free_stack_count = 0; uintptr_t stack_area_start = 0; - ipi mode = ipi::init | ipi::level | ipi::assert; + lapic::ipi mode = lapic::ipi::init | lapic::ipi::level | lapic::ipi::assert; apic.send_ipi_broadcast(mode, false, 0); for (uint8_t id : ids) { @@ -301,7 +301,7 @@ start_aps(lapic &apic, const kutil::vector &ids, void *kpml4) size_t current_count = ap_startup_count; log::debug(logs::boot, "Starting AP %d: stack %llx", cpu->index, stack_end); - ipi startup = ipi::startup | ipi::assert; + lapic::ipi startup = lapic::ipi::startup | lapic::ipi::assert; apic.send_ipi(startup, vector, id); for (unsigned i = 0; i < 20; ++i) { diff --git a/src/kernel/memory_bootstrap.cpp b/src/kernel/memory_bootstrap.cpp index 6568360..f2137d2 100644 --- a/src/kernel/memory_bootstrap.cpp +++ b/src/kernel/memory_bootstrap.cpp @@ -24,6 +24,11 @@ using memory::kernel_max_heap; using namespace kernel; +namespace kernel { +namespace args { + is_bitfield(section_flags); +}} + extern "C" void initialize_main_thread(); extern "C" uintptr_t initialize_main_user_stack(); @@ -203,8 +208,8 @@ load_simple_process(args::program &program) for (const auto § : program.sections) { vm_flags flags = - (bitfield_has(sect.type, section_flags::execute) ? vm_flags::exec : vm_flags::none) | - (bitfield_has(sect.type, section_flags::write) ? vm_flags::write : vm_flags::none); + ((sect.type && section_flags::execute) ? vm_flags::exec : vm_flags::none) | + ((sect.type && section_flags::write) ? vm_flags::write : vm_flags::none); vm_area *vma = new vm_area_fixed(sect.phys_addr, sect.size, flags); space.add(sect.virt_addr, vma); diff --git a/src/kernel/objects/vm_area.h b/src/kernel/objects/vm_area.h index eb96c25..0153f19 100644 --- a/src/kernel/objects/vm_area.h +++ b/src/kernel/objects/vm_area.h @@ -174,4 +174,4 @@ private: }; -IS_BITFIELD(vm_flags); +is_bitfield(vm_flags); diff --git a/src/kernel/page_table.h b/src/kernel/page_table.h index 8114f36..e780e8f 100644 --- a/src/kernel/page_table.h +++ b/src/kernel/page_table.h @@ -194,4 +194,4 @@ inline bool operator<(page_table::level a, page_table::level b) { inline page_table::level& operator++(page_table::level& l) { l = l + 1; return l; } inline page_table::level& operator--(page_table::level& l) { l = l - 1; return l; } -IS_BITFIELD(page_table::flag); +is_bitfield(page_table::flag); diff --git a/src/kernel/vm_space.h b/src/kernel/vm_space.h index 4437332..8dcd3ca 100644 --- a/src/kernel/vm_space.h +++ b/src/kernel/vm_space.h @@ -132,4 +132,4 @@ private: kutil::spinlock m_lock; }; -IS_BITFIELD(vm_space::fault_type); +is_bitfield(vm_space::fault_type); diff --git a/src/libraries/kutil/include/kutil/enum_bitfields.h b/src/libraries/kutil/include/kutil/enum_bitfields.h index 9d0b351..d4ad2b7 100644 --- a/src/libraries/kutil/include/kutil/enum_bitfields.h +++ b/src/libraries/kutil/include/kutil/enum_bitfields.h @@ -2,16 +2,16 @@ #include -template -struct is_enum_bitfield { static constexpr bool value = false; }; +namespace kutil { +namespace bitfields { -#define IS_BITFIELD(name) \ - template<> struct ::is_enum_bitfield {static constexpr bool value=true;} +template +constexpr bool is_enum_bitfield(E) { return false; } template struct enum_or_int { static constexpr bool value = - std::disjunction< is_enum_bitfield, std::is_integral >::value; + is_enum_bitfield(E{}) || std::is_integral::value; }; template @@ -34,116 +34,62 @@ template <> struct integral { using type = unsigned long; }; template <> struct integral { using type = long long; }; template <> struct integral { using type = unsigned long long; }; -template -constexpr typename std::enable_if::value,E>::type -operator & (E lhs, F rhs) -{ - return static_cast ( - static_cast::type>(lhs) & - static_cast::type>(rhs)); -} - -template -constexpr typename std::enable_if::value,E>::type -operator | (E lhs, F rhs) -{ - return static_cast ( - static_cast::type>(lhs) | - static_cast::type>(rhs)); -} - -template -constexpr typename std::enable_if::value,E>::type -operator ^ (E lhs, F rhs) -{ - return static_cast ( - static_cast::type>(lhs) ^ - static_cast::type>(rhs)); -} - -template -constexpr typename std::enable_if::value,E>::type -operator ~ (E rhs) -{ - return static_cast(~static_cast::type>(rhs)); -} - template constexpr typename std::enable_if::value,E>::type& operator |= (E &lhs, F rhs) { - lhs = static_cast( + return lhs = static_cast( static_cast::type>(lhs) | static_cast::type>(rhs)); - - return lhs; } template constexpr typename std::enable_if::value,E>::type& operator &= (E &lhs, F rhs) { - lhs = static_cast( + return lhs = static_cast( static_cast::type>(lhs) & static_cast::type>(rhs)); - - return lhs; } template constexpr typename std::enable_if::value,E>::type& operator ^= (E &lhs, F rhs) { - lhs = static_cast( + return lhs = static_cast( static_cast::type>(lhs) ^ static_cast::type>(rhs)); - - return lhs; } template -constexpr typename std::enable_if::value,E>::type& -operator -= (E &lhs, F rhs) -{ - lhs = static_cast( - static_cast::type>(lhs) & - ~static_cast::type>(rhs)); - - return lhs; -} +constexpr typename std::enable_if::value,E>::type +operator & (E lhs, F rhs) { return lhs &= rhs; } template -constexpr typename std::enable_if::value,E>::type& -operator += (E &lhs, F rhs) -{ - lhs = static_cast( - static_cast::type>(lhs) | - static_cast::type>(rhs)); +constexpr typename std::enable_if::value,E>::type +operator | (E lhs, F rhs) { return lhs |= rhs; } - return lhs; -} +template +constexpr typename std::enable_if::value,E>::type +operator ^ (E lhs, F rhs) { return lhs ^= rhs; } template -constexpr typename std::enable_if::value,bool>::type -operator ! (E rhs) -{ - return static_cast::type>(rhs) == 0; -} +constexpr typename std::enable_if::type +operator ~ (E rhs) { return static_cast(~static_cast::type>(rhs)); } template -constexpr bool -bitfield_has(E set, E flag) -{ - return - (static_cast::type>(set) & - static_cast::type>(flag)) == - static_cast::type>(flag); -} +constexpr typename std::enable_if::type +operator ! (E rhs) { return static_cast::type>(rhs) == 0; } -// Overload the logical-and operator to be 'bitwise-and, bool-cast' +/// Override logical-and to mean 'rhs contains all bits in lhs' template -constexpr typename std::enable_if::value,bool>::type -operator && (E set, E flag) -{ - return (set & flag) == flag; -} +constexpr typename std::enable_if::type +operator && (E rhs, E lhs) { return (rhs & lhs) == lhs; } + +} // namespace bitfields +} // namespace kutil + +#define is_bitfield(name) \ + constexpr bool is_enum_bitfield(name) { return true; } \ + using namespace kutil::bitfields; +