ACPI general cleanup (#115)

* Cleanup the ACPI code a bit, in part by making more consistent with
the Multiprocessor Specification code.
* Add logging and a test.

.vscode/settings.json

@@ -10,7 +10,8 @@
         "unordered_map": "c",
         "vector": "c",
         "string_view": "c",
-        "initializer_list": "c"
+        "initializer_list": "c",
+        "iterator": "c"
     },
     "C_Cpp.default.intelliSenseMode": "linux-gcc-x86"
 }

include/kernel/infrastructure/acpi/acpi.h

@@ -40,8 +40,10 @@
 
 bool verify_acpi_checksum(const void *buffer, size_t buflen);
 
-bool verify_acpi_rsdp(const acpi_rsdp_t *rsdp);
+bool validate_acpi_rsdp(const acpi_rsdp_t *rsdp);
 
 void map_acpi_tables(kern_paddr_t rsdp_paddr, const acpi_table_def_t *table_defs);
 
+void report_acpi_tables(const acpi_table_def_t *table_defs);
+
 #endif

include/kernel/infrastructure/acpi/asm/tables.h

@@ -32,13 +32,18 @@
 #ifndef JINUE_KERNEL_INFRASTRUCTURE_ACPI_ASM_TABLES_H
 #define JINUE_KERNEL_INFRASTRUCTURE_ACPI_ASM_TABLES_H
 
-
 #define ACPI_FADT_SIGNATURE "FACP"
 
+#define ACPI_FADT_NAME      "FADT"
+
 #define ACPI_HPET_SIGNATURE "HPET"
 
+#define ACPI_HPET_NAME      ACPI_HPET_SIGNATURE
+
 #define ACPI_MADT_SIGNATURE "APIC"
 
+#define ACPI_MADT_NAME      "MADT"
+
 #define ACPI_RSDP_SIGNATURE "RSD PTR "
 
 #define ACPI_RSDT_SIGNATURE "RSDT"

include/kernel/infrastructure/acpi/types.h

@@ -43,6 +43,7 @@ typedef struct {
 } acpi_addr_range_t;
 
 typedef struct {
+    const char   *name;
     const char   *signature;
     const void  **ptr;
 } acpi_table_def_t;

include/kernel/infrastructure/i686/firmware/acpi.h

@@ -39,6 +39,8 @@ void find_acpi_rsdp(void);
 
 void init_acpi(void);
 
+void report_acpi(void);
+
 kern_paddr_t acpi_get_rsdp_paddr(void);
 
 #endif

kernel/infrastructure/acpi/acpi.c

@@ -30,6 +30,7 @@
  */
 
 #include <jinue/shared/asm/mman.h>
+#include <kernel/domain/services/logging.h>
 #include <kernel/domain/services/mman.h>
 #include <kernel/infrastructure/acpi/asm/acpi.h>
 #include <kernel/infrastructure/acpi/acpi.h>
@@ -63,7 +64,7 @@ bool verify_acpi_checksum(const void *buffer, size_t buflen) {
  * @param rsdp pointer to ACPI RSDP
  * @return true is RSDP is valid, false otherwise
  */
-bool verify_acpi_rsdp(const acpi_rsdp_t *rsdp) {
+bool validate_acpi_rsdp(const acpi_rsdp_t *rsdp) {
     if(strncmp(rsdp->signature, ACPI_RSDP_SIGNATURE, sizeof(rsdp->signature)) != 0) {
         return false;
     }
@@ -269,3 +270,20 @@ void map_acpi_tables(kern_paddr_t rsdp_paddr, const acpi_table_def_t *table_defs
 
     process_rsdt(rsdt, is_xsdt, table_defs);
 }
+
+/**
+ * Log information regarding ACPI tables that were found
+ * 
+ * @param table_defs table definitions array terminated by a NULL signature
+ */
+void report_acpi_tables(const acpi_table_def_t *table_defs) {
+    info("ACPI:");
+
+    for(int idx = 0; table_defs[idx].signature != NULL; ++idx) {
+        const acpi_table_def_t *def = &table_defs[idx];
+
+        if(*def->ptr != NULL) {
+            info("  Found %s table", def->name);
+        }
+    }
+}

kernel/infrastructure/i686/firmware/acpi.c

@@ -40,7 +40,9 @@
 #include <stdint.h>
 #include <string.h>
 
-static kern_paddr_t rsdp_paddr = 0;
+#define PADDR_NULL 0
+
+static uint32_t rsdp_paddr = 0;
 
 static struct {
     const acpi_fadt_t *fadt;
@@ -49,62 +51,89 @@ static struct {
 } acpi_tables;
 
 static const acpi_table_def_t table_defs[] = {
-    { .signature = ACPI_FADT_SIGNATURE, .ptr = (const void **)&acpi_tables.fadt },
-    { .signature = ACPI_MADT_SIGNATURE, .ptr = (const void **)&acpi_tables.madt },
-    { .signature = ACPI_HPET_SIGNATURE, .ptr = (const void **)&acpi_tables.hpet },
-    { .signature = NULL, .ptr = NULL },
+    {
+        .name       = ACPI_FADT_NAME,
+        .signature  = ACPI_FADT_SIGNATURE,
+        .ptr        = (const void **)&acpi_tables.fadt
+    },
+    {
+        .name       = ACPI_MADT_NAME,
+        .signature  = ACPI_MADT_SIGNATURE,
+        .ptr        = (const void **)&acpi_tables.madt
+    },
+    {
+        .name       = ACPI_HPET_NAME,
+        .signature  = ACPI_HPET_SIGNATURE,
+        .ptr        = (const void **)&acpi_tables.hpet
+    },
+    { .signature  = NULL },
 };
 
 /**
- * Find the RSDP in memory
+ * Scan a range of physical memory to find the RSDP
  * 
- * At the stage of the boot process where this function is called, the memory
- * where the RSDP is located is mapped 1:1 so a pointer to the RSDP has the
- * same value as its physical address.
+ * The start and end addresses must both be aligned on a 16-byte boundary.
  *
- * @return pointer to RSDP if found, NULL otherwise
+ * @param from start address of scan
+ * @param to end address of scan
+ * @return address of RSDP if found, PADDR_NULL otherwise
  */
-static const acpi_rsdp_t *find_rsdp(void) {
-    const char *const start = (const char *)0x0e0000;
-    const char *const end   = (const char *)0x100000;
-
-    for(const char *addr = start; addr < end; addr += 16) {
+static uint32_t scan_address_range(uint32_t from, uint32_t to) {
+    for(uintptr_t addr = from; addr < to; addr += 16) {
         /* At the stage of the boot process where this function is called, the
-         * memory where the RSDP is located is mapped 1:1 so a pointer to the
-         * RSDP has the same value as its physical address. */
+         * memory where the floating pointer structure can be located is mapped
+         * 1:1 so a pointer to the structure has the same value as its physical
+         * address.*/
         const acpi_rsdp_t *rsdp = (const acpi_rsdp_t *)addr;
 
-        if(verify_acpi_rsdp(rsdp)) {
-            return rsdp;
+        if(validate_acpi_rsdp(rsdp)) {
+            return addr;
         }
     }
 
-    const char *top     = (const char *)(0xa0000 - KB);
-    const char *ebda    = (const char *)get_bios_ebda_addr();
-
-    if(ebda == NULL || ebda > top) {
-        return NULL;
-    }
+    return PADDR_NULL;
+}
 
-    for(const char *addr = ebda; addr < ebda + KB; addr += 16) {
-        const acpi_rsdp_t *rsdp = (const acpi_rsdp_t *)addr;
+/**
+ * Scan memory for RSDP
+ * 
+ * The ranges where the RSDP can be located are defined in section 5.2.5.1 of
+ * the ACPI Specification:
+ * 
+ * " OSPM finds the Root System Description Pointer (RSDP) structure by
+ *   searching physical memory ranges on 16-byte boundaries for a valid Root
+ *   System Description Pointer structure signature and checksum match as
+ *   follows:
+ *     * The first 1 KB of the Extended BIOS Data Area (EBDA). For EISA or MCA
+ *       systems, the EBDA can be found in the two-byte location 40:0Eh on the
+ *       BIOS data area.
+ *     * The BIOS read-only memory space between 0E0000h and 0FFFFFh. "
+ *
+ * @return address of RSDP if found, PADDR_NULL otherwise
+ */
+static uint32_t scan_for_rsdp(void) {
+    uint32_t ebda = get_bios_ebda_addr();
 
-        if(verify_acpi_rsdp(rsdp)) {
+    if(ebda != 0 && ebda <= 0xa0000 - KB) {
+        uint32_t rsdp = scan_address_range(ebda, ebda + KB);
+
+        if(rsdp != PADDR_NULL) {
             return rsdp;
         }
     }
 
-    return NULL;
+    return scan_address_range(0x0e0000, 0x100000);
 }
 
 /**
  * Locate the ACPI RSDP in memory
+ * 
+ * At the stage of the boot process where this function is called, the memory
+ * where the ACPI can be located has to be mapped 1:1 so a pointer to the RSDP
+ * has the same value as its physical address.
  */
 void find_acpi_rsdp(void) {
-    /* At the stage of the boot process where this function is called, the memory
-     * where the RSDP is located is mapped 1:1 so a pointer to the RSDP has the
-     * same value as its physical address. */
-    rsdp_paddr = (kern_paddr_t)find_rsdp();
+    rsdp_paddr = scan_for_rsdp();
 }
 
 /**
@@ -113,13 +142,20 @@ void find_acpi_rsdp(void) {
 void init_acpi(void) {
     memset(&acpi_tables, 0, sizeof(acpi_tables));
 
-    if(rsdp_paddr == 0) {
+    if(rsdp_paddr == PADDR_NULL) {
         return;
     }
 
     map_acpi_tables(rsdp_paddr, table_defs);
 }
 
+/**
+ * Log information regarding ACPI
+ */
+void report_acpi(void) {
+    report_acpi_tables(table_defs);
+}
+
 /**
  * Get the physical address of the ACPI RSDP
  *

kernel/infrastructure/i686/firmware/mp.c

@@ -105,7 +105,7 @@ static bool validate_configuration_table_header(const mp_conf_table_t *table) {
  * @return physical address of structure, PADDR_NULL if not found
  */
 static uint32_t scan_address_range(uint32_t from, uint32_t to) {
-    for(uint32_t addr = from; addr < to; addr += 16) {
+    for(uintptr_t addr = from; addr < to; addr += 16) {
         /* At the stage of the boot process where this function is called, the
          * memory where the floating pointer structure can be located is mapped
          * 1:1 so a pointer to the structure has the same value as its physical

kernel/infrastructure/i686/init.c

@@ -361,6 +361,9 @@ void machine_init(const config_t *config) {
     initialize_page_allocator(&boot_alloc);
 
     init_acpi();
+
+    report_acpi();
+
     init_mp();
 
     /* create slab cache to allocate PDPTs

tests/Makefile

@@ -37,9 +37,10 @@ run_log             = run-test.log
 deps                = $(kernel_img) $(testapp_initrd)
 
 tests = \
-	test_detect_qemu \
+	test_acpi \
 	test_boot_nopae \
 	test_boot_pae \
+	test_detect_qemu \
 	test_mp \
 	test_ipc
 

tests/test_acpi.sh

@@ -0,0 +1,46 @@
+#!/bin/bash
+# Copyright (C) 2025 Philippe Aubertin.
+# All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions
+# are met:
+# 
+# 1. Redistributions of source code must retain the above copyright
+#    notice, this list of conditions and the following disclaimer.
+# 
+# 2. Redistributions in binary form must reproduce the above copyright
+#    notice, this list of conditions and the following disclaimer in the
+#    documentation and/or other materials provided with the distribution.
+# 
+# 3. Neither the name of the author nor the names of other contributors
+#    may be used to endorse or promote products derived from this software
+#    without specific prior written permission.
+# 
+# THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS "AS IS" AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+# WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+# DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE FOR ANY
+# DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+# (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+# SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+run
+
+check_kernel_start
+
+# If check_no_panic, check_no_error would also fail, but check_no_panic provides
+# more relevant context in the log.
+check_no_panic
+
+check_no_error
+
+for t in FADT HPET MADT; do
+    echo "* Check $t table was found"
+    grep -E "Found $t table" $LOG || fail
+done
+
+check_reboot

tests/test_mp.sh

@@ -39,9 +39,9 @@ check_no_panic
 check_no_error
 
 echo "* Check floating pointer structure was found"
-grep -E "Floating pointer structure found at address 0x[1-9a-f][0-9a-f]{4}" $LOG || fail
+grep -E "Floating pointer structure found at address 0x[1-9a-f][0-9a-f]{4}$" $LOG || fail
 
 echo "* Check configuration table was found and mapped"
-grep -E "Configuration table version 1.[14] at address 0x[1-9a-f][0-9a-f]{4}" $LOG || fail
+grep -E "Configuration table version 1.[14] at address 0x[1-9a-f][0-9a-f]{4}$" $LOG || fail
 
 check_reboot