Kernel: Clean up and simplify MP table parsing

Use map_typed<T> to map physically addressed structs into kernel VM.
This is so much easier than doing address arithmetic everywhere. :^)

Kernel/ACPI/MultiProcessorParser.cpp

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2020, Liav A. <liavalb@hotmail.co.il>
+ * Copyright (c) 2020, Andreas Kling <kling@serenityos.org>
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -27,8 +28,9 @@
 #include <AK/StringView.h>
 #include <Kernel/ACPI/MultiProcessorParser.h>
 #include <Kernel/Interrupts/IOAPIC.h>
-#include <Kernel/VM/MemoryManager.h>
 #include <Kernel/StdLib.h>
+#include <Kernel/VM/MemoryManager.h>
+#include <Kernel/VM/TypedMapping.h>
 
 //#define MULTIPROCESSOR_DEBUG
 
@@ -49,9 +51,8 @@ void MultiProcessorParser::initialize()
 
 MultiProcessorParser::MultiProcessorParser()
     : m_floating_pointer(search_floating_pointer())
-    , m_operable((m_floating_pointer != (FlatPtr) nullptr))
 {
-    if (m_floating_pointer != (FlatPtr) nullptr) {
+    if (!m_floating_pointer.is_null()) {
         klog() << "MultiProcessor: Floating Pointer Structure @ " << PhysicalAddress(m_floating_pointer);
         parse_floating_pointer_data();
         parse_configuration_table();
@@ -62,29 +63,18 @@ MultiProcessorParser::MultiProcessorParser()
 
 void MultiProcessorParser::parse_floating_pointer_data()
 {
-    auto floating_pointer_region = MM.allocate_kernel_region(PhysicalAddress(page_base_of((u32)m_floating_pointer)), PAGE_SIZE * 2, "MultiProcessor Parser Parsing Floating Pointer Structure", Region::Access::Read, false, true);
-    auto* floating_pointer = (MultiProcessor::FloatingPointer*)floating_pointer_region->vaddr().offset(offset_in_page((u32)m_floating_pointer)).as_ptr();
-    m_configuration_table = floating_pointer->physical_address_ptr;
-    m_specification_revision = floating_pointer->specification_revision;
+    auto floating_pointer = map_typed<MultiProcessor::FloatingPointer>(m_floating_pointer);
+    m_configuration_table = PhysicalAddress(floating_pointer->physical_address_ptr);
     dbg() << "Features " << floating_pointer->feature_info[0] << ", IMCR? " << (floating_pointer->feature_info[0] & (1 << 7));
 }
 
-size_t MultiProcessorParser::get_configuration_table_length()
-{
-    auto config_table_region = MM.allocate_kernel_region(PhysicalAddress(page_base_of((u32)m_configuration_table)), PAGE_SIZE * 2, "MultiProcessor Parser Getting Configuration Table length", Region::Access::Read, false, true);
-    auto* config_table = (MultiProcessor::ConfigurationTableHeader*)config_table_region->vaddr().offset(offset_in_page((u32)m_configuration_table)).as_ptr();
-    return config_table->length;
-}
-
 void MultiProcessorParser::parse_configuration_table()
 {
-    m_configuration_table_length = get_configuration_table_length();
-    auto config_table_region = MM.allocate_kernel_region(PhysicalAddress(page_base_of((u32)m_configuration_table)), PAGE_ROUND_UP(m_configuration_table_length), "MultiProcessor Parser Parsing Configuration Table", Region::Access::Read, false, true);
-    auto* config_table = (MultiProcessor::ConfigurationTableHeader*)config_table_region->vaddr().offset(offset_in_page((u32)m_configuration_table)).as_ptr();
+    auto configuration_table_length = map_typed<MultiProcessor::ConfigurationTableHeader>(m_configuration_table)->length;
+    auto config_table = map_typed<MultiProcessor::ConfigurationTableHeader>(m_configuration_table, configuration_table_length);
 
     size_t entry_count = config_table->entry_count;
     auto* entry = config_table->entries;
-    auto* p_entry = reinterpret_cast<MultiProcessor::ConfigurationTableHeader*>(m_configuration_table)->entries;
     while (entry_count > 0) {
 #ifdef MULTIPROCESSOR_DEBUG
         dbg() << "MultiProcessor: Entry Type " << entry->entry_type << " detected.";
@@ -92,94 +82,85 @@ void MultiProcessorParser::parse_configuration_table()
         switch (entry->entry_type) {
         case ((u8)MultiProcessor::ConfigurationTableEntryType::Processor):
             entry = (MultiProcessor::EntryHeader*)(u32)entry + (u8)MultiProcessor::ConfigurationTableEntryLength::Processor;
-            p_entry = (MultiProcessor::EntryHeader*)(u32)p_entry + (u8)MultiProcessor::ConfigurationTableEntryLength::Processor;
             break;
         case ((u8)MultiProcessor::ConfigurationTableEntryType::Bus):
-            m_bus_entries.append((FlatPtr)p_entry);
+            m_bus_entries.append(*(const MultiProcessor::BusEntry*)entry);
             entry = (MultiProcessor::EntryHeader*)(u32)entry + (u8)MultiProcessor::ConfigurationTableEntryLength::Bus;
-            p_entry = (MultiProcessor::EntryHeader*)(u32)p_entry + (u8)MultiProcessor::ConfigurationTableEntryLength::Bus;
             break;
         case ((u8)MultiProcessor::ConfigurationTableEntryType::IOAPIC):
             entry = (MultiProcessor::EntryHeader*)(u32)entry + (u8)MultiProcessor::ConfigurationTableEntryLength::IOAPIC;
-            p_entry = (MultiProcessor::EntryHeader*)(u32)p_entry + (u8)MultiProcessor::ConfigurationTableEntryLength::IOAPIC;
             break;
         case ((u8)MultiProcessor::ConfigurationTableEntryType::IO_Interrupt_Assignment):
-            m_io_interrupt_redirection_entries.append((FlatPtr)p_entry);
+            m_io_interrupt_assignment_entries.append(*(const MultiProcessor::IOInterruptAssignmentEntry*)entry);
             entry = (MultiProcessor::EntryHeader*)(u32)entry + (u8)MultiProcessor::ConfigurationTableEntryLength::IO_Interrupt_Assignment;
-            p_entry = (MultiProcessor::EntryHeader*)(u32)p_entry + (u8)MultiProcessor::ConfigurationTableEntryLength::IO_Interrupt_Assignment;
             break;
         case ((u8)MultiProcessor::ConfigurationTableEntryType::Local_Interrupt_Assignment):
             entry = (MultiProcessor::EntryHeader*)(u32)entry + (u8)MultiProcessor::ConfigurationTableEntryLength::Local_Interrupt_Assignment;
-            p_entry = (MultiProcessor::EntryHeader*)(u32)p_entry + (u8)MultiProcessor::ConfigurationTableEntryLength::Local_Interrupt_Assignment;
             break;
         case ((u8)MultiProcessor::ConfigurationTableEntryType::SystemAddressSpaceMapping):
             entry = (MultiProcessor::EntryHeader*)(u32)entry + (u8)MultiProcessor::ConfigurationTableEntryLength::SystemAddressSpaceMapping;
-            p_entry = (MultiProcessor::EntryHeader*)(u32)p_entry + (u8)MultiProcessor::ConfigurationTableEntryLength::SystemAddressSpaceMapping;
             break;
         case ((u8)MultiProcessor::ConfigurationTableEntryType::BusHierarchyDescriptor):
             entry = (MultiProcessor::EntryHeader*)(u32)entry + (u8)MultiProcessor::ConfigurationTableEntryLength::BusHierarchyDescriptor;
-            p_entry = (MultiProcessor::EntryHeader*)(u32)p_entry + (u8)MultiProcessor::ConfigurationTableEntryLength::BusHierarchyDescriptor;
             break;
         case ((u8)MultiProcessor::ConfigurationTableEntryType::CompatibilityBusAddressSpaceModifier):
             entry = (MultiProcessor::EntryHeader*)(u32)entry + (u8)MultiProcessor::ConfigurationTableEntryLength::CompatibilityBusAddressSpaceModifier;
-            p_entry = (MultiProcessor::EntryHeader*)(u32)p_entry + (u8)MultiProcessor::ConfigurationTableEntryLength::CompatibilityBusAddressSpaceModifier;
             break;
+        default:
             ASSERT_NOT_REACHED();
         }
-        entry_count--;
+        --entry_count;
     }
 }
 
-FlatPtr MultiProcessorParser::search_floating_pointer()
+PhysicalAddress MultiProcessorParser::search_floating_pointer()
 {
-    FlatPtr mp_floating_pointer = (FlatPtr) nullptr;
+    PhysicalAddress mp_floating_pointer;
     auto region = MM.allocate_kernel_region(PhysicalAddress(0), PAGE_SIZE, "MultiProcessor Parser Floating Pointer Structure Finding", Region::Access::Read);
     u16 ebda_seg = (u16) * ((uint16_t*)((region->vaddr().get() & PAGE_MASK) + 0x40e));
     klog() << "MultiProcessor: Probing EBDA, Segment 0x" << String::format("%x", ebda_seg);
 
     mp_floating_pointer = search_floating_pointer_in_ebda(ebda_seg);
-    if (mp_floating_pointer != (FlatPtr) nullptr)
+    if (!mp_floating_pointer.is_null())
         return mp_floating_pointer;
     return search_floating_pointer_in_bios_area();
 }
 
-FlatPtr MultiProcessorParser::search_floating_pointer_in_ebda(u16 ebda_segment)
+PhysicalAddress MultiProcessorParser::search_floating_pointer_in_ebda(u16 ebda_segment)
 {
     auto floating_pointer_region = MM.allocate_kernel_region(PhysicalAddress(page_base_of((u32)(ebda_segment << 4))), PAGE_ROUND_UP(1024), "MultiProcessor Parser floating_pointer Finding #1", Region::Access::Read, false, true);
     char* p_floating_pointer_str = (char*)(PhysicalAddress(ebda_segment << 4).as_ptr());
     for (char* floating_pointer_str = (char*)floating_pointer_region->vaddr().offset(offset_in_page((u32)(ebda_segment << 4))).as_ptr(); floating_pointer_str < (char*)(floating_pointer_region->vaddr().offset(offset_in_page((u32)(ebda_segment << 4))).get() + 1024); floating_pointer_str += 16) {
 #ifdef MULTIPROCESSOR_DEBUG
-        dbg() << "MultiProcessor: Looking for floating pointer structure in EBDA @ V0x " << String::format("%x", floating_pointer_str) << ", P0x" << String::format("%x", p_floating_pointer_str);
+        //dbg() << "MultiProcessor: Looking for floating pointer structure in EBDA @ V0x " << String::format("%x", floating_pointer_str) << ", P0x" << String::format("%x", p_floating_pointer_str);
 #endif
         if (!strncmp("_MP_", floating_pointer_str, strlen("_MP_")))
-            return (FlatPtr)p_floating_pointer_str;
+            return PhysicalAddress((FlatPtr)p_floating_pointer_str);
         p_floating_pointer_str += 16;
     }
-    return (FlatPtr) nullptr;
+    return {};
 }
-FlatPtr MultiProcessorParser::search_floating_pointer_in_bios_area()
+PhysicalAddress MultiProcessorParser::search_floating_pointer_in_bios_area()
 {
     auto floating_pointer_region = MM.allocate_kernel_region(PhysicalAddress(page_base_of((u32)0xE0000)), PAGE_ROUND_UP(0xFFFFF - 0xE0000), "MultiProcessor Parser floating_pointer Finding #2", Region::Access::Read, false, true);
     char* p_floating_pointer_str = (char*)(PhysicalAddress(0xE0000).as_ptr());
     for (char* floating_pointer_str = (char*)floating_pointer_region->vaddr().offset(offset_in_page((u32)(0xE0000))).as_ptr(); floating_pointer_str < (char*)(floating_pointer_region->vaddr().offset(offset_in_page((u32)(0xE0000))).get() + (0xFFFFF - 0xE0000)); floating_pointer_str += 16) {
 #ifdef MULTIPROCESSOR_DEBUG
-        dbg() << "MultiProcessor: Looking for floating pointer structure in BIOS area @ V0x " << String::format("%x", floating_pointer_str) << ", P0x" << String::format("%x", p_floating_pointer_str);
+        //dbg() << "MultiProcessor: Looking for floating pointer structure in BIOS area @ V0x " << String::format("%x", floating_pointer_str) << ", P0x" << String::format("%x", p_floating_pointer_str);
 #endif
         if (!strncmp("_MP_", floating_pointer_str, strlen("_MP_")))
-            return (FlatPtr)p_floating_pointer_str;
+            return PhysicalAddress((FlatPtr)p_floating_pointer_str);
         p_floating_pointer_str += 16;
     }
-    return (FlatPtr) nullptr;
+    return {};
 }
 
-Vector<unsigned> MultiProcessorParser::get_pci_bus_ids()
+Vector<u8> MultiProcessorParser::get_pci_bus_ids() const
 {
-    Vector<unsigned> pci_bus_ids;
-    for (auto entry : m_bus_entries) {
-        auto entry_region = MM.allocate_kernel_region(PhysicalAddress(page_base_of((u32)entry)), PAGE_ROUND_UP(m_configuration_table_length), "MultiProcessor Parser Parsing Bus Entry", Region::Access::Read, false, true);
-        auto* v_entry_ptr = (MultiProcessor::BusEntry*)entry_region->vaddr().offset(offset_in_page((u32)entry)).as_ptr();
-        if (!strncmp("PCI   ", v_entry_ptr->bus_type, strlen("PCI   ")))
-            pci_bus_ids.append(v_entry_ptr->bus_id);
+    Vector<u8> pci_bus_ids;
+    for (auto& entry : m_bus_entries) {
+        if (!strncmp("PCI   ", entry.bus_type, strlen("PCI   ")))
+            pci_bus_ids.append(entry.bus_id);
     }
     return pci_bus_ids;
 }
@@ -194,24 +175,19 @@ Vector<PCIInterruptOverrideMetadata> MultiProcessorParser::get_pci_interrupt_red
 {
     dbg() << "MultiProcessor: Get PCI IOAPIC redirections";
     Vector<PCIInterruptOverrideMetadata> overrides;
-    Vector<unsigned> pci_bus_ids = get_pci_bus_ids();
-    for (auto entry : m_io_interrupt_redirection_entries) {
-        auto entry_region = MM.allocate_kernel_region(PhysicalAddress(page_base_of((u32)entry)), PAGE_ROUND_UP(m_configuration_table_length), "MultiProcessor Parser Parsing Bus Entry", Region::Access::Read, false, true);
-        auto* v_entry_ptr = (MultiProcessor::IOInterruptAssignmentEntry*)entry_region->vaddr().offset(offset_in_page((u32)entry)).as_ptr();
-#ifdef MULTIPROCESSOR_DEBUG
-        dbg() << "MultiProcessor: Parsing Entry P 0x" << String::format("%x", entry) << ", V " << v_entry_ptr;
-#endif
+    auto pci_bus_ids = get_pci_bus_ids();
+    for (auto& entry : m_io_interrupt_assignment_entries) {
         for (auto id : pci_bus_ids) {
-            if (id == v_entry_ptr->source_bus_id) {
+            if (id == entry.source_bus_id) {
 
-                klog() << "Interrupts: Bus " << v_entry_ptr->source_bus_id << ", Polarity " << v_entry_ptr->polarity << ", Trigger Mode " << v_entry_ptr->trigger_mode << ", INT " << v_entry_ptr->source_bus_irq << ", IOAPIC " << v_entry_ptr->destination_ioapic_id << ", IOAPIC INTIN " << v_entry_ptr->destination_ioapic_intin_pin;
+                klog() << "Interrupts: Bus " << entry.source_bus_id << ", Polarity " << entry.polarity << ", Trigger Mode " << entry.trigger_mode << ", INT " << entry.source_bus_irq << ", IOAPIC " << entry.destination_ioapic_id << ", IOAPIC INTIN " << entry.destination_ioapic_intin_pin;
                 overrides.empend(
-                    v_entry_ptr->source_bus_id,
-                    v_entry_ptr->polarity,
-                    v_entry_ptr->trigger_mode,
-                    v_entry_ptr->source_bus_irq,
-                    v_entry_ptr->destination_ioapic_id,
-                    v_entry_ptr->destination_ioapic_intin_pin);
+                    entry.source_bus_id,
+                    entry.polarity,
+                    entry.trigger_mode,
+                    entry.source_bus_irq,
+                    entry.destination_ioapic_id,
+                    entry.destination_ioapic_intin_pin);
             }
         }
     }

Kernel/ACPI/MultiProcessorParser.h

@@ -27,8 +27,8 @@
 #pragma once
 
 #include <AK/Types.h>
-#include <Kernel/VM/Region.h>
 #include <Kernel/PhysicalAddress.h>
+#include <Kernel/VM/Region.h>
 #include <Kernel/VirtualAddress.h>
 
 namespace Kernel {
@@ -201,22 +201,17 @@ protected:
     MultiProcessorParser();
 
     void parse_configuration_table();
-    size_t get_configuration_table_length();
     void parse_floating_pointer_data();
 
-    Vector<unsigned> get_pci_bus_ids();
-
-    FlatPtr search_floating_pointer();
-    FlatPtr search_floating_pointer_in_ebda(u16 ebda_segment);
-    FlatPtr search_floating_pointer_in_bios_area();
+    Vector<u8> get_pci_bus_ids() const;
 
-    FlatPtr m_floating_pointer;
-    FlatPtr m_configuration_table;
-    Vector<FlatPtr> m_io_interrupt_redirection_entries;
-    Vector<FlatPtr> m_bus_entries;
-    bool m_operable;
+    PhysicalAddress search_floating_pointer();
+    PhysicalAddress search_floating_pointer_in_ebda(u16 ebda_segment);
+    PhysicalAddress search_floating_pointer_in_bios_area();
 
-    size_t m_configuration_table_length;
-    u8 m_specification_revision;
+    PhysicalAddress m_floating_pointer;
+    PhysicalAddress m_configuration_table;
+    Vector<MultiProcessor::IOInterruptAssignmentEntry> m_io_interrupt_assignment_entries;
+    Vector<MultiProcessor::BusEntry> m_bus_entries;
 };
 }