From 8b483849f4bc105bceb971462e269cfac8ed64f4 Mon Sep 17 00:00:00 2001 From: Ronald Caesar Date: Sat, 13 Sep 2025 22:47:01 -0400 Subject: [PATCH 1/4] arm64/mem: Refactor guest memory access and made it endian aware Refactors the core guest memory access subsystem (guest.h) to be safer and portable accross host systems with different endianness. The previous implementation used direct pointer casting, which is not endian safe. 1. All read and write functions have been converted from unsafe pointer casts to memcpy(). This resolves alignment warning -Wcast-align. 2. The access functions no longer rely on asserts for error checking. They now perform explicit boundary and alignment checking and returns a guest_mem_access_result_t status code. 3. A new header (endian.h) provides cross platform byte swapping macros. The memory access functions use these macros to ensure that the guest always sees memory in the correct endian format, regardless of the host's native byte order. The host endianness is now automatically detected via CMake. 3. Asserts are now explicitly enabled in release builds to catch critical errors. Signed-off-by: Ronald Caesar --- CMakeLists.txt | 22 ++ src/host/memory/arena.cpp | 4 +- src/host/memory/arena.h | 4 +- src/kvm/CMakeLists.txt | 1 + src/kvm/endian.h | 27 +++ src/kvm/guest.cpp | 35 +++ src/kvm/guest.h | 485 +++++++++++++++++++++++++++++--------- src/kvm/kvm.cpp | 104 +------- src/kvm/mmu.cpp | 3 +- 9 files changed, 467 insertions(+), 218 deletions(-) create mode 100644 src/kvm/endian.h create mode 100644 src/kvm/guest.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index d83307f..21831d8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -2,6 +2,7 @@ cmake_minimum_required(VERSION 3.22) + set(CMAKE_C_STANDARD 17) set(CMAKE_CXX_STANDARD 23) set(CMAKE_C_STANDARD_REQUIRED TRUE) @@ -12,6 +13,8 @@ if (NOT CMAKE_BUILD_TYPE) set(CMAKE_BUILD_TYPE Debug) endif() +# Enable asserts in release mode. +string( REPLACE "/DNDEBUG" "" CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE}") # Optimizations set(CMAKE_INTERPROCEDURAL_OPTIMIZATION_RELEASE ON) @@ -22,6 +25,7 @@ endif() project(Pound) + find_package(fmt 10.2.1 CONFIG) find_package(SDL3 3.2.10 CONFIG) add_subdirectory(3rd_Party) @@ -38,6 +42,24 @@ add_subdirectory(src/frontend) add_subdirectory(src/host) add_subdirectory(src/targets/switch1/hardware) +# Detect Host System Endianness. +include(TestBigEndian) +TEST_BIG_ENDIAN(WORDS_BIGENDIAN) +if(WORDS_BIGENDIAN) + message(STATUS "Host Endianness: Big Endian") +else() + message(STATUS "Host Endianness: Little Endian") +endif() + +# Configure Preprocessor Definitions based on Endianness. +if(WORDS_BIGENDIAN) + target_compile_definitions(Pound PRIVATE HOST_IS_BIG_ENDIAN=1) + target_compile_definitions(Pound PRIVATE HOST_IS_LITTLE_ENDIAN=0) +else() + target_compile_definitions(Pound PRIVATE HOST_IS_BIG_ENDIAN=0) + target_compile_definitions(Pound PRIVATE HOST_IS_LITTLE_ENDIAN=1) +endif() + target_compile_options(Pound PRIVATE -Wall -Wpedantic -Wshadow -Wpointer-arith diff --git a/src/host/memory/arena.cpp b/src/host/memory/arena.cpp index e2ded77..b359ff8 100644 --- a/src/host/memory/arena.cpp +++ b/src/host/memory/arena.cpp @@ -31,11 +31,11 @@ arena_t arena_init(size_t capacity) } // new more memsafe code (ownedbywuigi) (i give up on windows compatibility for now, will stick to the old unsafe code) -const void* arena_allocate(memory::arena_t* arena, const std::size_t size) +void* arena_allocate(memory::arena_t* arena, const std::size_t size) { assert(arena != nullptr); assert(arena->size + size <= arena->capacity); - const void* const data = static_cast(arena->data) + arena->size; + void* const data = static_cast(arena->data) + arena->size; arena->size += size; return data; } diff --git a/src/host/memory/arena.h b/src/host/memory/arena.h index 59722b6..f37801d 100644 --- a/src/host/memory/arena.h +++ b/src/host/memory/arena.h @@ -58,7 +58,7 @@ memory::arena_t arena_init(size_t capacity); * arena_allocate - Allocate memory from a pre-initialized arena. * * SYNOPSIS - * const void* arena_allocate(arena_t* arena, std::size_t size); + * void* arena_allocate(arena_t* arena, std::size_t size); * * DESCRIPTION * The function allocates size bytes from the specified arena. It assumes @@ -72,7 +72,7 @@ memory::arena_t arena_init(size_t capacity); * NOTES * Requires arena_t to be initialized with arena_init() or similar. */ -const void* arena_allocate(arena_t* arena, const std::size_t size); +void* arena_allocate(arena_t* arena, const std::size_t size); /* * NAME diff --git a/src/kvm/CMakeLists.txt b/src/kvm/CMakeLists.txt index 6bee731..468a5aa 100644 --- a/src/kvm/CMakeLists.txt +++ b/src/kvm/CMakeLists.txt @@ -3,6 +3,7 @@ set(KVM_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/mmu.cpp ${CMAKE_CURRENT_SOURCE_DIR}/mmio.cpp ${CMAKE_CURRENT_SOURCE_DIR}/kvm.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/guest.cpp ) target_sources(Pound PRIVATE ${KVM_SOURCES}) diff --git a/src/kvm/endian.h b/src/kvm/endian.h new file mode 100644 index 0000000..57fd2d5 --- /dev/null +++ b/src/kvm/endian.h @@ -0,0 +1,27 @@ +#ifndef POUND_KVM_ENDIAN_H +#define POUND_KVM_ENDIAN_H + +#define GUEST_IS_LITTLE_ENDIAN 1 + +#ifdef _WIN32 + +#include +#define bswap_16(x) _byteswap_ushort(x) +#define bswap_32(x) _byteswap_ulong(x) +#define bswap_64(x) _byteswap_uint64(x) + +#elif defined(__APPLE__) + +#include +#define bswap_16(x) OSSwaoInt16(x) +#define bswap_32(x) OSSwapInt32(x) +#define bswap_64(x) OSSwapInt64(x) + +#else + +#include + +#endif + + +#endif // POUND_KVM_ENDIAN_H diff --git a/src/kvm/guest.cpp b/src/kvm/guest.cpp new file mode 100644 index 0000000..9cd107f --- /dev/null +++ b/src/kvm/guest.cpp @@ -0,0 +1,35 @@ +#include "guest.h" +#include + +namespace pound::kvm::memory +{ +guest_memory_t* guest_memory_create(pound::host::memory::arena_t* arena) +{ + assert(nullptr != arena); + assert(nullptr != arena->data); + + guest_memory_t* memory = (guest_memory_t*)pound::host::memory::arena_allocate(arena, sizeof(guest_memory_t)); + size_t ram_size = arena->capacity - arena->size; + uint8_t* ram_block = (uint8_t*)pound::host::memory::arena_allocate(arena, ram_size); + + /* + * This requires casting away the 'const' qualifier, which is generally unsafe. + * However, it is safe in this specific context because: + * + * a) We are operating on a newly allocated heap object (`memory`). + * b) No other part of the system has a reference to this object yet. + * c) This is a one-time initialization; the const contract will be + * honored for the rest of the object's lifetime after this function + * returns. + * + * This allows us to create an immutable descriptor object on the + * heap. + */ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wcast-qual" + *(uint8_t**)&memory->base = ram_block; + *(uint64_t*)&memory->size = ram_size; +#pragma GCC diagnostic pop + return memory; +} +} // namespace pound::kvm::memory diff --git a/src/kvm/guest.h b/src/kvm/guest.h index 60012f0..0118b68 100644 --- a/src/kvm/guest.h +++ b/src/kvm/guest.h @@ -1,55 +1,109 @@ -#pragma once +#ifndef POUND_KVM_GUEST_H +#define POUND_KVM_GUEST_H -#include -#include +#include +#include +#include + +#include "endian.h" + +#include "host/memory/arena.h" namespace pound::kvm::memory { /* - * guest_memory_t - Describes a contiguous block of guest physical RAM. + * guest_memory_t - A non-owning descriptor for a block of guest physical RAM. * @base: Pointer to the start of the host-allocated memory block. * @size: The size of the memory block in bytes. + * + * + * This structure describes a contiguous block of guest physical memory. It acts + * as a handle or a "view" into a region of host memory, but it does not manage + * the lifetime of that memory itself. + * + * --- Ownership --- + * The guest_memory_t struct does NOT own the memory block pointed to by @base. + * Ownership of the underlying memory buffer is retained by the host memory + * arena from which it was allocated. The party responsible for creating the + * arena is also responsible for ultimately freeing it. This struct is merely a + * descriptor and can be safely passed by value or pointer without transferring + * ownership. + * + * --- Lifetime --- + * An instance of this struct should be considered valid only for as long as the + * backing memory arena is valid. Typically, this means it is created once +- * during virtual machine initialization and lives for the entire duration of + * the emulation session. Its lifetime is tied to the lifetime of the parent + * KVM instance. + * + * --- Invariants --- + * Both fields of this struct are declared `const`. This establishes the + * invariant that once a guest_memory_t descriptor is created and initialized + * by guest_memory_create(), its size and base address are immutable for the + * lifetime of the object. This prevents accidental resizing or repointing of + * the guest's physical RAM. */ typedef struct { - uint8_t* base; - uint64_t size; + uint8_t* const base; + const uint64_t size; } guest_memory_t; /* - * gpa_to_hva() - Translate a Guest Physical Address to a Host Virtual Address. - * @memory: The guest memory region to translate within. - * @gpa: The Guest Physical Address (offset) to translate. + * guest_memory_create() - Allocates and initializes a guest memory region from + * an arena. + * @arena: A pointer to a host memory arena that will be the source for all + * allocations. * - * This function provides a fast, direct translation for a flat guest memory - * model. It relies on the critical pre-condition that the guest's physical - * RAM is backed by a single, contiguous block of virtual memory in the host's - * userspace (typically allocated with mmap()). + * This function sets up the primary guest RAM block. It uses a provided host + * memory arena as the backing store for both the guest_memory_t descriptor + * struct and the guest RAM itself. * - * In this model, memory->base is the Host Virtual Address (HVA) of the start of - * the backing host memory. The provided Guest Physical Address (gpa) is not - * treated as a pointer, but as a simple byte offset from the start of the guest's - * physical address space (PAS). + * The function first allocates a small chunk from the arena for the guest_memory_t + * struct. It then dedicates the *entire remaining capacity* of the arena to be + * the main guest RAM block. * - * The translation is therefore a single pointer-offset calculation. This establishes - * a direct 1:1 mapping between the guest's PAS and the host's virtual memory block. + * Preconditions: + * - @arena must be a valid, non-NULL pointer to an initialized host arena. + * - @arena->data must point to a valid, host-allocated memory buffer. + * - The arena provided should be dedicated solely to this guest memory block; + * its entire remaining capacity will be consumed. * - * The function asserts that GPA is within bounds. The caller is responsible for - * ensuring the validity of the GPA prior to calling. - * - * Return: A valid host virtual address pointer corresponding to the GPA. + * Return: A pointer to a fully initialized guest_memory_t struct. The `base` + * pointer will point to the start of the guest RAM block within the arena, + * and `size` will reflect the size of that block. */ -static inline uint8_t* gpa_to_hva(guest_memory_t* memory, uint64_t gpa) -{ - assert(nullptr != memory); - assert(nullptr != memory->base); - assert(gpa < memory->size); - uint8_t* hva = memory->base + gpa; - return hva; -} +guest_memory_t* guest_memory_create(pound::host::memory::arena_t* arena); -// TODO(GloriousTacoo:aarch64) Implement big to little endian conversion for guest_mem read and write functions. +/* + * guest_mem_access_result_t - Defines the set of possible outcomes for a guest + * memory access operation. + * @GUEST_MEM_ACCESS_OK: The memory operation completed + * successfully. + * @GUEST_MEM_FAULT_UNALIGNED: The access was unaligned, and the + * emulated CPU requires an Alignment + * Fault to be raised. The operation was + * NOT completed. The host must inject a + * data abort into the guest. + * @GUEST_MEM_ACCESS_FAULT_BOUNDARY: An access fell outside the bounds of + * the defined memory region. The + * operation was NOT completed, The host + * must inject a Data Abort for a + * translation/permission fault into the + * guest. + * @GUEST_MEM_ACCESS_ERROR_INTERNAL: An unrecoverable internal error occured + * within the memory subsystem. This + * indicates a fatal host bug, not a guest + * induced fault. + */ +typedef enum +{ + GUEST_MEM_ACCESS_OK = 0, + GUEST_MEM_ACCESS_FAULT_UNALIGNED, + GUEST_MEM_ACCESS_FAULT_BOUNDARY, + GUEST_MEM_ACCESS_ERROR_INTERNAL, +} guest_mem_access_result_t; /* * ============================================================================ @@ -58,69 +112,174 @@ static inline uint8_t* gpa_to_hva(guest_memory_t* memory, uint64_t gpa) */ /* - * guest_mem_readb() - Read one byte from guest memory. - * @memory: The guest memory region. - * @gpa: The Guest Physical Address to read from. - * Returns the 8-bit value read from memory. + * guest_mem_readb() - Read one byte from guest physical memory. + * @memory: A pointer to the guest memory region. + * @gpa: The guest physical address to read from. + * @out_val: A pointer to a uint8_t where the result will be stored. + * + * This function safely reads a single 8-bit value from the guest's physical + * RAM. It performs a bounds check to ensure the access is within the allocated + * memory region. + * + * Preconditions: + * - @memory and @out_val must be valid, non-NULL pointers. + * - @memory->base must point to a valid, host-allocated memory buffer. + * + * Return: + * %GUEST_MEM_ACCESS_OK on success. + * %GUEST_MEM_ACCESS_FAULT_BOUNDARY if the @gpa is outside the valid memory + * range. */ -static inline uint8_t guest_mem_readb(guest_memory_t* memory, uint64_t gpa) +inline guest_mem_access_result_t guest_mem_readb(guest_memory_t* memory, uint64_t gpa, uint8_t* out_val) { assert(nullptr != memory); assert(nullptr != memory->base); - assert(gpa <= memory->size); - uint8_t* hva = gpa_to_hva(memory, gpa); - return *hva; + assert(nullptr != out_val); + + if (gpa >= memory->size) + { + return GUEST_MEM_ACCESS_FAULT_BOUNDARY; + } + + uint8_t* hva = memory->base + gpa; + *out_val = *hva; + + return GUEST_MEM_ACCESS_OK; } /* - * guest_mem_readw() - Read a 16-bit word from guest memory. - * @memory: The guest memory region. - * @gpa: The Guest Physical Address to read from (must be 2-byte aligned). - * Returns the 16-bit value, corrected for host endianness. + * guest_mem_readw() - Read a 16-bit word from guest physical memory. + * @memory: A pointer to the guest memory region. + * @gpa: The guest physical address to read from. + * @out_val: A pointer to a uint16_t where the result will be stored. + * + * This function safely reads a 16-bit little-endian value from guest RAM. + * It performs both boundary and alignment checks before the access. + * It will also perform a byte swap if the host system is not little-endian. + * + * Preconditions: + * - @memory and @out_val must be valid, non-NULL pointers. + * - @memory->base must point to a valid, host-allocated memory buffer. + * - The guest address @gpa must be 2-byte aligned. + * + * Return: + * %GUEST_MEM_ACCESS_OK on success. + * %GUEST_MEM_ACCESS_FAULT_BOUNDARY on an out-of-bounds access or + * %GUEST_MEM_ACCESS_FAULT_UNALIGNED if @gpa is not 2-byte aligned. */ -static inline uint16_t guest_mem_readw(guest_memory_t* memory, uint64_t gpa) +inline guest_mem_access_result_t guest_mem_readw(guest_memory_t* memory, uint64_t gpa, uint16_t* out_val) { assert(nullptr != memory); assert(nullptr != memory->base); - assert((gpa + sizeof(uint16_t)) <= memory->size); - // Check if gpa is aligned to 2 bytes. - assert((gpa & 1) == 0); - uint16_t* hva = (uint16_t*)gpa_to_hva(memory, gpa); - return *hva; + assert(nullptr != out_val); + + if (gpa > (memory->size - sizeof(uint16_t))) + { + return GUEST_MEM_ACCESS_FAULT_BOUNDARY; + } + + if ((gpa & 1) != 0) + { + return GUEST_MEM_ACCESS_FAULT_UNALIGNED; + } + + uint8_t* hva = memory->base + gpa; + memcpy(out_val, hva, sizeof(uint16_t)); + +#if HOST_IS_LITTLE_ENDIAN != GUEST_IS_LITTLE_ENDIAN + *out_val = bswap_16(*out_val); +#endif + return GUEST_MEM_ACCESS_OK; } /* - * guest_mem_readl() - Read a 32-bit long-word from guest memory. - * @memory: The guest memory region. - * @gpa: The Guest Physical Address to read from (must be 4-byte aligned). - * Returns the 32-bit value, corrected for host endianness. + * guest_mem_readl() - Read a 32-bit long-word from guest physical memory. + * @memory: A pointer to the guest memory region. + * @gpa: The guest physical address to read from. + * @out_val: A pointer to a uint32_t where the result will be stored. + * + * This function safely reads a 32-bit little-endian value from guest RAM. + * It performs both boundary and alignment checks before the access. + * It will also perform a byte swap if the host system is not little-endian. + * + * Preconditions: + * - @memory and @out_val must be valid, non-NULL pointers. + * - @memory->base must point to a valid, host-allocated memory buffer. + * - The guest address @gpa must be 4-byte aligned. + * + * Return: + * %GUEST_MEM_ACCESS_OK on success. + * %GUEST_MEM_ACCESS_FAULT_BOUNDARY on an out-of-bounds access or + * %GUEST_MEM_ACCESS_FAULT_UNALIGNED if @gpa is not 4-byte aligned. */ -static inline uint32_t guest_mem_readl(guest_memory_t* memory, uint64_t gpa) +inline guest_mem_access_result_t guest_mem_readl(guest_memory_t* memory, uint64_t gpa, uint32_t* out_val) { assert(nullptr != memory); assert(nullptr != memory->base); - assert((gpa + sizeof(uint32_t)) <= memory->size); - // Check if gpa is aligned to 4 bytes. - assert((gpa & 3) == 0); - uint32_t* hva = (uint32_t*)gpa_to_hva(memory, gpa); - return *hva; + assert(nullptr != out_val); + + if (gpa > (memory->size - sizeof(uint32_t))) + { + return GUEST_MEM_ACCESS_FAULT_BOUNDARY; + } + + if ((gpa & 3) != 0) + { + return GUEST_MEM_ACCESS_FAULT_UNALIGNED; + } + + uint8_t* hva = memory->base + gpa; + memcpy(out_val, hva, sizeof(uint32_t)); + +#if HOST_IS_LITTLE_ENDIAN != GUEST_IS_LITTLE_ENDIAN + *out_val = bswap_32(*out_val); +#endif + return GUEST_MEM_ACCESS_OK; } /* - * guest_mem_readq() - Read a 64-bit quad-word from guest memory. - * @memory: The guest memory region. - * @gpa: The Guest Physical Address to read from (must be 8-byte aligned). - * Returns the 64-bit value, corrected for host endianness. + * guest_mem_readq() - Read a 64-bit quad-word from guest physical memory. + * @memory: A pointer to the guest memory region. + * @gpa: The guest physical address to read from. + * @out_val: A pointer to a uint64_t where the result will be stored. + * + * This function safely reads a 64-bit little-endian value from guest RAM. + * It performs both boundary and alignment checks before the access. + * It will also perform a byte swap if the host system is not little-endian. + * + * Preconditions: + * - @memory and @out_val must be valid, non-NULL pointers. + * - @memory->base must point to a valid, host-allocated memory buffer. + * - The guest address @gpa must be 8-byte aligned. + * + * Return: + * %GUEST_MEM_ACCESS_OK on success. + * %GUEST_MEM_ACCESS_FAULT_BOUNDARY on an out-of-bounds access or + * %GUEST_MEM_ACCESS_FAULT_UNALIGNED if @gpa is not 8-byte aligned. */ -static inline uint64_t guest_mem_readq(guest_memory_t* memory, uint64_t gpa) +inline guest_mem_access_result_t guest_mem_readq(guest_memory_t* memory, uint64_t gpa, uint64_t* out_val) { assert(nullptr != memory); assert(nullptr != memory->base); - assert((gpa + sizeof(uint64_t)) <= memory->size); - // Check if gpa is aligned to 8 bytes. - assert((gpa & 7) == 0); - uint64_t* hva = (uint64_t*)gpa_to_hva(memory, gpa); - return *hva; + assert(nullptr != out_val); + + if (gpa > (memory->size - sizeof(uint64_t))) + { + return GUEST_MEM_ACCESS_FAULT_BOUNDARY; + } + + if ((gpa & 7) != 0) + { + return GUEST_MEM_ACCESS_FAULT_UNALIGNED; + } + + uint8_t* hva = memory->base + gpa; + memcpy(out_val, hva, sizeof(uint64_t)); + +#if HOST_IS_LITTLE_ENDIAN != GUEST_IS_LITTLE_ENDIAN + *out_val = bswap_64(*out_val); +#endif + return GUEST_MEM_ACCESS_OK; } /* @@ -130,67 +289,171 @@ static inline uint64_t guest_mem_readq(guest_memory_t* memory, uint64_t gpa) */ /* - * guest_mem_writeb() - Write one byte to guest memory. - * @memory: The guest memory region. - * @gpa: The Guest Physical Address to write to. + * guest_mem_writeb() - Write one byte to guest physical memory. + * @memory: A pointer to the guest memory region. + * @gpa: The guest physical address to write to. * @val: The 8-bit value to write. + * + * This function safely writes a single 8-bit value to the guest's physical + * RAM. It performs a bounds check to ensure the access is within the allocated + * memory region before performing the write. + * + * Preconditions: + * - @memory must be a valid, non-NULL pointer. + * - @memory->base must point to a valid, host-allocated memory buffer. + * + * Return: + * %GUEST_MEM_ACCESS_OK on success. + * %GUEST_MEM_ACCESS_FAULT_BOUNDARY if the @gpa is outside the valid memory + * range. */ -static inline void guest_mem_writeb(guest_memory_t* memory, uint64_t gpa, uint8_t val) +inline guest_mem_access_result_t guest_mem_writeb(guest_memory_t* memory, uint64_t gpa, uint8_t val) { assert(nullptr != memory); assert(nullptr != memory->base); - assert(gpa <= memory->size); - uint8_t* hva = gpa_to_hva(memory, gpa); + + if (gpa >= memory->size) + { + return GUEST_MEM_ACCESS_FAULT_BOUNDARY; + } + + uint8_t* hva = memory->base + gpa; *hva = val; + return GUEST_MEM_ACCESS_OK; } /* - * guest_mem_writew() - Write a 16-bit word to guest memory. - * @memory: The guest memory region. - * @gpa: The Guest Physical Address to write to (must be 2-byte aligned). - * @val: The 16-bit value to write (will be converted to guest endianness). + * guest_mem_writew() - Write a 16-bit word to guest physical memory. + * @memory: A pointer to the guest memory region. + * @gpa: The guest physical address to write to. + * @val: The 16-bit value to write. + * + * This function safely writes a 16-bit little-endian value to guest RAM. + * It performs both boundary and alignment checks before the access. + * It will also perform a byte swap if the host system is not little-endian. + * + * Preconditions: + * - @memory must be a valid, non-NULL pointer. + * - @memory->base must point to a valid, host-allocated memory buffer. + * - The guest address @gpa must be 2-byte aligned. + * + * Return: %GUEST_MEM_ACCESS_OK on success. Returns + * %GUEST_MEM_ACCESS_FAULT_BOUNDARY on an out-of-bounds access or + * %GUEST_MEM_ACCESS_FAULT_UNALIGNED if @gpa is not 2-byte aligned. */ -static inline void guest_mem_writew(guest_memory_t* memory, uint64_t gpa, uint16_t val) +inline guest_mem_access_result_t guest_mem_writew(guest_memory_t* memory, uint64_t gpa, uint16_t val) { assert(nullptr != memory); assert(nullptr != memory->base); - assert((gpa + sizeof(uint16_t)) <= memory->size); - // Check if gpa is aligned to 2 bytes. - assert((gpa & 1) == 0); - uint16_t* hva = (uint16_t*)gpa_to_hva(memory, gpa); - *hva = val; + + if (gpa > (memory->size - sizeof(uint16_t))) + { + return GUEST_MEM_ACCESS_FAULT_BOUNDARY; + } + + if ((gpa & 1) != 0) + { + return GUEST_MEM_ACCESS_FAULT_UNALIGNED; + } + + uint8_t* hva = memory->base + gpa; + +#if HOST_IS_LITTLE_ENDIAN != GUEST_IS_LITTLE_ENDIAN + val = bswap_16(val); +#endif + + memcpy(hva, &val, sizeof(uint16_t)); + return GUEST_MEM_ACCESS_OK; } /* - * guest_mem_writel() - Write a 32-bit long-word to guest memory. - * @memory: The guest memory region. - * @gpa: The Guest Physical Address to write to (must be 4-byte aligned). + * guest_mem_writel() - Write a 32-bit long-word to guest physical memory. + * @memory: A pointer to the guest memory region. + * @gpa: The guest physical address to write to. * @val: The 32-bit value to write. + * + * This function safely writes a 32-bit little-endian value to guest RAM. + * It performs both boundary and alignment checks before the access. + * It will also perform a byte swap if the host system is not little-endian. + * + * Preconditions: + * - @memory must be a valid, non-NULL pointer. + * - @memory->base must point to a valid, host-allocated memory buffer. + * - The guest address @gpa must be 4-byte aligned. + * + * Return: + * %GUEST_MEM_ACCESS_OK on success. + * %GUEST_MEM_ACCESS_FAULT_BOUNDARY on an out-of-bounds access or + * %GUEST_MEM_ACCESS_FAULT_UNALIGNED if @gpa is not 4-byte aligned. */ -static inline void guest_mem_writel(guest_memory_t* memory, uint64_t gpa, uint32_t val) -{ - assert(nullptr != memory->base); - assert((gpa + sizeof(uint32_t)) <= memory->size); - // Check if gpa is aligned to 4 bytes. - assert((gpa & 3) == 0); - uint32_t* hva = (uint32_t*)gpa_to_hva(memory, gpa); - *hva = val; -} - -/* - * guest_mem_writeq() - Write a 64-bit quad-word to guest memory. - * @memory: The guest memory region. - * @gpa: The Guest Physical Address to write to (must be 8-byte aligned). - * @val: The 64-bit value to write. - */ -static inline void guest_mem_writeq(guest_memory_t* memory, uint64_t gpa, uint64_t val) +inline guest_mem_access_result_t guest_mem_writel(guest_memory_t* memory, uint64_t gpa, uint32_t val) { assert(nullptr != memory); assert(nullptr != memory->base); - assert((gpa + sizeof(uint64_t)) <= memory->size); - // Check if gpa is aligned to 8 bytes. - assert((gpa & 7) == 0); - uint64_t* hva = (uint64_t*)gpa_to_hva(memory, gpa); - *hva = val; + + if (gpa > (memory->size - sizeof(uint32_t))) + { + return GUEST_MEM_ACCESS_FAULT_BOUNDARY; + } + + if ((gpa & 3) != 0) + { + return GUEST_MEM_ACCESS_FAULT_UNALIGNED; + } + + uint8_t* hva = memory->base + gpa; + +#if HOST_IS_LITTLE_ENDIAN != GUEST_IS_LITTLE_ENDIAN + val = bswap_32(val); +#endif + + memcpy(hva, &val, sizeof(uint32_t)); + return GUEST_MEM_ACCESS_OK; +} + +/* + * guest_mem_writeq() - Write a 64-bit quad-word to guest physical memory. + * @memory: A pointer to the guest memory region. + * @gpa: The guest physical address to write to. + * @val: The 64-bit value to write. + * + * This function safely writes a 64-bit little-endian value to guest RAM. + * It performs both boundary and alignment checks before the access. + * It will also perform a byte swap if the host system is not little-endian. + * + * Preconditions: + * - @memory must be a valid, non-NULL pointer. + * - @memory->base must point to a valid, host-allocated memory buffer. + * - The guest address @gpa must be 8-byte aligned. + * + * Return: + * %GUEST_MEM_ACCESS_OK on success. + * %GUEST_MEM_ACCESS_FAULT_BOUNDARY on an out-of-bounds access or + * %GUEST_MEM_ACCESS_FAULT_UNALIGNED if @gpa is not 8-byte aligned. + */ +inline guest_mem_access_result_t guest_mem_writeq(guest_memory_t* memory, uint64_t gpa, uint64_t val) +{ + assert(nullptr != memory); + assert(nullptr != memory->base); + + if (gpa > (memory->size - sizeof(uint64_t))) + { + return GUEST_MEM_ACCESS_FAULT_BOUNDARY; + } + + if ((gpa & 7) != 0) + { + return GUEST_MEM_ACCESS_FAULT_UNALIGNED; + } + + uint8_t* hva = memory->base + gpa; + +#if HOST_IS_LITTLE_ENDIAN != GUEST_IS_LITTLE_ENDIAN + val = bswap_64(val); +#endif + + memcpy(hva, &val, sizeof(uint64_t)); + return GUEST_MEM_ACCESS_OK; } } // namespace pound::kvm::memory +#endif // POUND_KVM_GUEST_H diff --git a/src/kvm/kvm.cpp b/src/kvm/kvm.cpp index 16ba9c1..36d966a 100644 --- a/src/kvm/kvm.cpp +++ b/src/kvm/kvm.cpp @@ -67,113 +67,13 @@ void take_synchronous_exception(kvm_vcpu_t* vcpu, uint8_t exception_class, uint3 * vcpu->pc = vcpu->vbar_el1 + offset; */ } -/** THIS FUNCTION WAS MADE WITH AI AND IS CALLED WHEN RUNNING THE CPU TEST FROM THE GUI! - * - * @brief Runs a comprehensive suite of tests on the guest memory access functions using the project's logging system. - * - * This function systematically tests the read and write capabilities of the memory - * subsystem for all standard data sizes (8, 16, 32, and 64 bits). It verifies - * that data written to memory can be correctly read back. - * - * It specifically checks: - * 1. A standard aligned address in the middle of RAM. - * - * @param memory A pointer to an initialized guest_memory_t struct. - * @return true if all tests pass, false otherwise. - */ -bool test_guest_ram_access(pound::kvm::memory::guest_memory_t* memory) -{ - LOG_INFO(Memory, "--- [ Starting Guest RAM Access Test ] ---"); - if (memory == nullptr || memory->base == nullptr || memory->size < 4096) - { - LOG_CRITICAL(Memory, "Invalid memory block provided. Cannot run tests."); - return false; - } - - bool all_tests_passed = true; - -#define RUN_TEST(description, condition) \ - do \ - { \ - char log_buffer[256]; \ - if (condition) \ - { \ - snprintf(log_buffer, sizeof(log_buffer), " [TEST] %-45s... [PASS]", description); \ - LOG_INFO(Memory, log_buffer); \ - } \ - else \ - { \ - snprintf(log_buffer, sizeof(log_buffer), " [TEST] %-45s... [FAIL]", description); \ - LOG_ERROR(Memory, log_buffer); \ - all_tests_passed = false; \ - } \ - } while (0) - -#define VERIFY_ACCESS(size, suffix, addr, write_val) \ - do \ - { \ - guest_mem_write##suffix(memory, addr, write_val); \ - uint##size##_t read_val = guest_mem_read##suffix(memory, addr); \ - bool success = (read_val == write_val); \ - RUN_TEST("Write/Read " #size "-bit", success); \ - if (!success) \ - { \ - char error_buffer[256]; \ - snprintf(error_buffer, sizeof(error_buffer), " -> At GPA 0x%016llx, Expected 0x%016llx, Got 0x%016llx", \ - (unsigned long long)addr, (unsigned long long)write_val, (unsigned long long)read_val); \ - LOG_ERROR(Memory, error_buffer); \ - } \ - } while (0) - - // --- 1. Test a typical, aligned address in the middle of memory --- - LOG_INFO(Memory, "[INFO] Testing standard access at a midrange address (GPA 0x1000)..."); - uint64_t test_addr = 0x1000; - VERIFY_ACCESS(8, b, test_addr + 0, 0xA5); - VERIFY_ACCESS(16, w, test_addr + 2, 0xBEEF); - VERIFY_ACCESS(32, l, test_addr + 4, 0xDEADBEEF); - VERIFY_ACCESS(64, q, test_addr + 8, 0xCAFEBABE01234567); - - // --- 2. Test the very beginning of the memory block --- - LOG_INFO(Memory, "[INFO] Testing boundary access at the start of RAM (GPA 0x0)..."); - VERIFY_ACCESS(64, q, 0x0, 0xFEEDFACEDEADBEEF); - - // --- 3. Test the very end of the memory block --- - LOG_INFO(Memory, "[INFO] Testing boundary access at the end of RAM..."); - - uint64_t end_addr_b = memory->size - 1; - uint64_t end_addr_w = (memory->size - 2) & ~1ULL; - uint64_t end_addr_l = (memory->size - 4) & ~3ULL; - uint64_t end_addr_q = (memory->size - 8) & ~7ULL; - - VERIFY_ACCESS(8, b, end_addr_b, 0xFE); - VERIFY_ACCESS(16, w, end_addr_w, 0xFEFE); - VERIFY_ACCESS(32, l, end_addr_l, 0xFEFEFEFE); - VERIFY_ACCESS(64, q, end_addr_q, 0xFEFEFEFEFEFEFEFE); - - // --- 4. Final Verdict --- - LOG_INFO(Memory, "--- [ Guest RAM Access Test Finished ] ---"); - if (all_tests_passed) - { - LOG_INFO(Memory, ">>> Result: ALL TESTS PASSED"); - } - else - { - LOG_ERROR(Memory, ">>> Result: SOME TESTS FAILED"); - } - LOG_INFO(Memory, "----------------------------------------------"); - - return all_tests_passed; -} - void cpuTest() { pound::host::memory::arena_t guest_memory_arena = pound::host::memory::arena_init(GUEST_RAM_SIZE); assert(nullptr != guest_memory_arena.data); - pound::kvm::memory::guest_memory_t guest_ram = {}; - guest_ram.base = static_cast(guest_memory_arena.data); - guest_ram.size = guest_memory_arena.capacity; + memory::guest_memory_t* guest_ram = memory::guest_memory_create(&guest_memory_arena); - (void)test_guest_ram_access(&guest_ram); + //(void)test_guest_ram_access(guest_ram); } } // namespace pound::kvm diff --git a/src/kvm/mmu.cpp b/src/kvm/mmu.cpp index 870d803..e4cdde1 100644 --- a/src/kvm/mmu.cpp +++ b/src/kvm/mmu.cpp @@ -340,7 +340,8 @@ int mmu_gva_to_gpa(pound::kvm::kvm_vcpu_t* vcpu, guest_memory_t* memory, uint64_ } const uint64_t level_entry_address = table_address + (level_index * page_table_entry_size); - const uint64_t descriptor = guest_mem_readq(memory, level_entry_address); + uint64_t descriptor = 0; + guest_mem_readq(memory, level_entry_address, &descriptor); uint64_t offset_mask = (1ULL << offset_bits) - 1; uint64_t page_offset = gva & offset_mask; uint64_t page_address_mask = ~offset_mask; From a3ed44003b6759a4a2077b3fa1bcf2c92d0cf748 Mon Sep 17 00:00:00 2001 From: Ronald Caesar Date: Sun, 14 Sep 2025 13:28:09 -0400 Subject: [PATCH 2/4] build: Refactor CMake build system This new architecture decomposes the project into several distict static libraries: common, host, kvm, and frontend. By using static libraries, changes within one module will only require that library to be re-linked, rather than recompiling and re-linking the entire executable. The third party library ImGui is now built as a static library target. Signed-off-by: Ronald Caesar --- 3rd_Party/CMakeLists.txt | 19 +++- CMakeLists.txt | 130 +++++++++++++-------------- src/common/Assert.cpp | 59 ------------ src/common/Assert.h | 121 ------------------------- src/common/CMakeLists.txt | 15 ++-- src/common/IoFile.cpp | 2 - src/common/Logging/Filter.cpp | 11 +-- src/common/Logging/TextFormatter.cpp | 4 +- src/frontend/CMakeLists.txt | 19 ++-- src/frontend/color.h | 2 +- src/frontend/gui.cpp | 10 +-- src/frontend/panels.cpp | 10 +-- src/host/CMakeLists.txt | 11 +-- src/kvm/CMakeLists.txt | 18 ++-- src/kvm/guest.cpp | 2 +- src/kvm/guest.h | 2 +- 16 files changed, 133 insertions(+), 302 deletions(-) delete mode 100644 src/common/Assert.cpp delete mode 100644 src/common/Assert.h diff --git a/3rd_Party/CMakeLists.txt b/3rd_Party/CMakeLists.txt index f4618ee..4800a6f 100644 --- a/3rd_Party/CMakeLists.txt +++ b/3rd_Party/CMakeLists.txt @@ -1,5 +1,3 @@ -# Copyright 2025 Xenon Emulator Project. All rights reserved. - set(BUILD_SHARED_LIBS OFF) set(BUILD_TESTING OFF) set_directory_properties(PROPERTIES EXCLUDE_FROM_ALL ON SYSTEM ON) @@ -19,3 +17,20 @@ if (NOT TARGET SDL3::SDL3) set(SDL_PIPEWIRE OFF) add_subdirectory(SDL3) endif() + +# ImGui +set(IMGUI_SRC + imgui/imgui.cpp + imgui/imgui_demo.cpp + imgui/imgui_draw.cpp + imgui/imgui_tables.cpp + imgui/imgui_widgets.cpp + imgui/backends/imgui_impl_sdl3.cpp + imgui/backends/imgui_impl_opengl3.cpp +) +add_library(imgui STATIC ${IMGUI_SRC}) +target_link_libraries(imgui PRIVATE SDL3::SDL3) +target_include_directories(imgui PUBLIC + imgui + imgui/backends +) diff --git a/CMakeLists.txt b/CMakeLists.txt index 21831d8..7ac622f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,103 +1,97 @@ -# Copyright 2025 Xenon Emulator Project. All rights reserved. - cmake_minimum_required(VERSION 3.22) +#------------------------ +# ---- Project Setup ---- +#------------------------ -set(CMAKE_C_STANDARD 17) -set(CMAKE_CXX_STANDARD 23) -set(CMAKE_C_STANDARD_REQUIRED TRUE) -set(CMAKE_CXX_STANDARD_REQUIRED TRUE) -set(IMGUI_DIR ${CMAKE_CURRENT_SOURCE_DIR}/3rd_Party/imgui) +project(Pound) if (NOT CMAKE_BUILD_TYPE) set(CMAKE_BUILD_TYPE Debug) endif() -# Enable asserts in release mode. -string( REPLACE "/DNDEBUG" "" CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE}") +# Enable asserts in release mode. Windows throws a metric fuck ton of compiler +# errors when asserts are disabled. +#if (UNIX) +# string(REPLACE "-DNDEBUG" "" CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE}") +#endif() -# Optimizations -set(CMAKE_INTERPROCEDURAL_OPTIMIZATION_RELEASE ON) -if (WIN32) - add_compile_options($<$:/Oi>) - add_compile_options($<$:/Ot>) -endif() - -project(Pound) +set(CMAKE_C_STANDARD 17) +set(CMAKE_CXX_STANDARD 23) +set(CMAKE_C_STANDARD_REQUIRED TRUE) +set(CMAKE_CXX_STANDARD_REQUIRED TRUE) +#------------------------------- +# ---- Dependency Discovery ---- +#------------------------------- +find_package(OpenGL REQUIRED) find_package(fmt 10.2.1 CONFIG) find_package(SDL3 3.2.10 CONFIG) -add_subdirectory(3rd_Party) +#----------------------------- +# ---- Target Definitions ---- +#----------------------------- + +if(WIN32) + add_compile_options(-DNOMINMAX -DWIN32_LEAN_AND_MEAN) +endif() add_executable(Pound src/main.cpp ) - -add_subdirectory(src/kvm) +add_subdirectory(3rd_Party) add_subdirectory(src/common) add_subdirectory(src/frontend) add_subdirectory(src/host) +add_subdirectory(src/kvm) add_subdirectory(src/targets/switch1/hardware) -# Detect Host System Endianness. +#-------------------------------- +# ---- Target Configurations ---- +#-------------------------------- + include(TestBigEndian) TEST_BIG_ENDIAN(WORDS_BIGENDIAN) -if(WORDS_BIGENDIAN) - message(STATUS "Host Endianness: Big Endian") -else() - message(STATUS "Host Endianness: Little Endian") -endif() -# Configure Preprocessor Definitions based on Endianness. -if(WORDS_BIGENDIAN) - target_compile_definitions(Pound PRIVATE HOST_IS_BIG_ENDIAN=1) - target_compile_definitions(Pound PRIVATE HOST_IS_LITTLE_ENDIAN=0) -else() - target_compile_definitions(Pound PRIVATE HOST_IS_BIG_ENDIAN=0) - target_compile_definitions(Pound PRIVATE HOST_IS_LITTLE_ENDIAN=1) -endif() +list(APPEND POUND_PROJECT_TARGETS common frontend host kvm) +foreach(TARGET ${POUND_PROJECT_TARGETS}) + # Apply Endianness definitions to all our targets. + if(WORDS_BIGENDIAN) + target_compile_definitions(${TARGET} PRIVATE HOST_IS_BIG_ENDIAN=1 HOST_IS_LITTLE_ENDIAN=0) + else() + target_compile_definitions(${TARGET} PRIVATE HOST_IS_BIG_ENDIAN=0 HOST_IS_LITTLE_ENDIAN=1) + endif() + target_compile_options(${TARGET} PRIVATE + $<$: + -Wall + -Wpedantic + -Wshadow + -Wpointer-arith + -Wcast-qual + -Wcast-align + -Wconversion> + ) +endforeach() -target_compile_options(Pound PRIVATE -Wall -Wpedantic --Wshadow --Wpointer-arith --Wcast-qual --Wcast-align --Wconversion -) -# Link libraries -target_link_libraries(Pound PRIVATE fmt::fmt SDL3::SDL3) +# Optimizations +set_property(TARGET Pound PROPERTY CMAKE_INTERPROCEDURAL_OPTIMIZATION_RELEASE TRUE) if (WIN32) - add_compile_definitions(NOMINMAX WIN32_LEAN_AND_MEAN) - - # Disables Warnings - add_compile_definitions(_CRT_SECURE_NO_WARNINGS) + target_compile_options(Pound PRIVATE $<$:/Oi>) + target_compile_options(Pound PRIVATE $<$:/Ot>) endif() +target_link_libraries(Pound PRIVATE + common + frontend + host + kvm -# ImGui -set(IMGUI_SRC - ${IMGUI_DIR}/imgui.cpp - ${IMGUI_DIR}/imgui_demo.cpp - ${IMGUI_DIR}/imgui_draw.cpp - ${IMGUI_DIR}/imgui_tables.cpp - ${IMGUI_DIR}/imgui_widgets.cpp - ${IMGUI_DIR}/backends/imgui_impl_sdl3.cpp - ${IMGUI_DIR}/backends/imgui_impl_opengl3.cpp + OpenGL::GL + fmt::fmt + SDL3::SDL3 + imgui ) - -target_sources(Pound PRIVATE ${IMGUI_SRC}) - -target_include_directories(Pound PRIVATE - ${IMGUI_DIR} - ${IMGUI_DIR}/backends -) - -find_package(OpenGL REQUIRED) -target_link_libraries(Pound PRIVATE OpenGL::GL) - -# add ./gui directory diff --git a/src/common/Assert.cpp b/src/common/Assert.cpp deleted file mode 100644 index 1d434a8..0000000 --- a/src/common/Assert.cpp +++ /dev/null @@ -1,59 +0,0 @@ -// Copyright 2025 Xenon Emulator Project. All rights reserved. - -#include - -#include "Arch.h" -#include "Assert.h" -#include "Logging/Backend.h" - -#ifdef _MSC_VER -#define Crash() __debugbreak() -#else -#if defined(ARCH_X86_64) -#define Crash() __asm__ __volatile__("int $3") -#elif defined(ARCH_X86) -#define Crash() __asm__ __volatile__("int $3") -#elif defined(ARCH_AARCH64) -#define Crash() __asm__ __volatile__("brk 0") -#elif defined(ARCH_PPC) -#include -#ifdef SIGTRAP -#define Crash() raise(SIGTRAP) -#else -#define Crash() raise(SIGABRT) -#endif -#else -#define Crash() __builtin_trap() -#endif -#endif // _MSVC_VER - -void throw_fail_impl() -{ - ::fflush(stdout); - Crash(); -} - -void assert_fail_impl() -{ - printf("Assertion Failed!\n"); - throw_fail_impl(); -} - -[[noreturn]] void unreachable_impl() -{ - ::fflush(stdout); - Crash(); - throw std::runtime_error("Unreachable code"); -} - -void assert_fail_debug_msg(const std::string& msg) -{ - printf("Assertion Failed! %s\n", msg.c_str()); - assert_fail_impl(); -} - -void throw_fail_debug_msg(const std::string& msg) -{ - printf("Assertion Failed! %s\n", msg.c_str()); - throw_fail_impl(); -} diff --git a/src/common/Assert.h b/src/common/Assert.h deleted file mode 100644 index 682cb04..0000000 --- a/src/common/Assert.h +++ /dev/null @@ -1,121 +0,0 @@ -// Copyright 2025 Xenon Emulator Project. All rights reserved. - -#pragma once - -#include "Logging/Log.h" - -// Sometimes we want to try to continue even after hitting an assert. -// However touching this file yields a global recompilation as this header is included almost -// everywhere. So let's just move the handling of the failed assert to a single cpp file. - -void assert_fail_impl(); -void throw_fail_impl(); -[[noreturn]] void unreachable_impl(); -void assert_fail_debug_msg(const std::string& msg); -void throw_fail_debug_msg(const std::string& msg); - -#ifdef _MSC_VER -#define POUND_NO_INLINE __declspec(noinline) -#else -#define POUND_NO_INLINE __attribute__((noinline)) -#endif - -#define THROW(_a_) \ - ( \ - [&]() POUND_NO_INLINE \ - { \ - if (!(_a_)) [[unlikely]] \ - { \ - LOG_CRITICAL(Debug, "Assertion Failed!"); \ - throw_fail_impl(); \ - } \ - }) - -#define ASSERT(_a_) \ - ( \ - [&]() POUND_NO_INLINE \ - { \ - if (!(_a_)) [[unlikely]] \ - { \ - LOG_CRITICAL(Debug, "Assertion Failed!"); \ - assert_fail_impl(); \ - } \ - }) - -#define THROW_MSG(_a_, ...) \ - ( \ - [&]() POUND_NO_INLINE \ - { \ - if (!(_a_)) [[unlikely]] \ - { \ - LOG_CRITICAL(Debug, "Assertion Failed!\n" __VA_ARGS__); \ - throw_fail_impl(); \ - } \ - }) - -#define ASSERT_MSG(_a_, ...) \ - ( \ - [&]() POUND_NO_INLINE \ - { \ - if (!(_a_)) [[unlikely]] \ - { \ - LOG_CRITICAL(Debug, "Assertion Failed!\n" __VA_ARGS__); \ - assert_fail_impl(); \ - } \ - }) - -#define UNREACHABLE() \ - do \ - { \ - LOG_CRITICAL(Debug, "Unreachable code!"); \ - unreachable_impl(); \ - } while (0) - -#define UNREACHABLE_MSG(...) \ - do \ - { \ - LOG_CRITICAL(Debug, "Unreachable code!\n" __VA_ARGS__); \ - unreachable_impl(); \ - } while (0) - -#ifdef _DEBUG -#define DEBUG_ASSERT(_a_) ASSERT(_a_) -#define DEBUG_ASSERT_MSG(_a_, ...) ASSERT_MSG(_a_, __VA_ARGS__) -#else // not debug -#define DEBUG_ASSERT(_a_) \ - do \ - { \ - } while (0) -#define DEBUG_ASSERT_MSG(_a_, _desc_, ...) \ - do \ - { \ - } while (0) -#endif - -#define UNIMPLEMENTED() THROW_MSG(false, "Unimplemented code!") -#define UNIMPLEMENTED_MSG(...) THROW_MSG(false, __VA_ARGS__) - -#define UNIMPLEMENTED_IF(cond) ASSERT_MSG(!(cond), "Unimplemented code!") -#define UNIMPLEMENTED_IF_MSG(cond, ...) ASSERT_MSG(!(cond), __VA_ARGS__) - -// If the assert is ignored, execute _b_ -#define ASSERT_OR_EXECUTE(_a_, _b_) \ - do \ - { \ - ASSERT(_a_); \ - if (!(_a_)) [[unlikely]] \ - { \ - _b_ \ - } \ - } while (0) - -// If the assert is ignored, execute _b_ -#define ASSERT_OR_EXECUTE_MSG(_a_, _b_, ...) \ - do \ - { \ - ASSERT_MSG(_a_, __VA_ARGS__); \ - if (!(_a_)) [[unlikely]] \ - { \ - _b_ \ - } \ - } while (0) diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt index 85bc7ec..963c14f 100644 --- a/src/common/CMakeLists.txt +++ b/src/common/CMakeLists.txt @@ -1,7 +1,6 @@ -#Copyright 2025 Pound Emulator Project.All rights reserved. +add_library(common STATIC) -set(COMMON_SOURCES - ${CMAKE_CURRENT_SOURCE_DIR}/Assert.cpp +target_sources(common PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/Config.cpp ${CMAKE_CURRENT_SOURCE_DIR}/IoFile.cpp ${CMAKE_CURRENT_SOURCE_DIR}/PathUtil.cpp @@ -9,8 +8,12 @@ set(COMMON_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/Thread.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Logging/Backend.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Logging/Filter.cpp - ${CMAKE_CURRENT_SOURCE_DIR}/Logging/TextFormatter.cpp) + ${CMAKE_CURRENT_SOURCE_DIR}/Logging/TextFormatter.cpp +) -target_sources(Pound PRIVATE ${COMMON_SOURCES}) +target_link_libraries(common PUBLIC fmt::fmt) -target_include_directories(Pound PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/..) +target_include_directories(common PUBLIC + ${CMAKE_CURRENT_SOURCE_DIR} + ${CMAKE_CURRENT_SOURCE_DIR}/.. +) diff --git a/src/common/IoFile.cpp b/src/common/IoFile.cpp index c12c805..4fb69c4 100644 --- a/src/common/IoFile.cpp +++ b/src/common/IoFile.cpp @@ -4,7 +4,6 @@ #include "IoFile.h" -#include "Assert.h" #include "Error.h" #include "Logging/Log.h" #include "PathUtil.h" @@ -287,7 +286,6 @@ uptr IOFile::GetFileMapping() mapping = hfile; fileMapping = std::bit_cast(mapping); - ASSERT_MSG(fileMapping, "{}", Base::GetLastErrorMsg()); return fileMapping; #else fileMapping = fileno(file); diff --git a/src/common/Logging/Filter.cpp b/src/common/Logging/Filter.cpp index 9b5feae..d032591 100644 --- a/src/common/Logging/Filter.cpp +++ b/src/common/Logging/Filter.cpp @@ -1,8 +1,7 @@ // Copyright 2025 Xenon Emulator Project. All rights reserved. -#include "common/Assert.h" - #include "Filter.h" +#include namespace Base { namespace Log { @@ -33,13 +32,13 @@ template bool ParseFilterRule(Filter &instance, Iterator begin, Iterator end) { const auto levelSeparator = std::find(begin, end, ':'); if (levelSeparator == end) { - LOG_ERROR(Log, "Invalid log filter. Must specify a log level after `:`: {}", std::string_view(begin, end)); + // LOG_ERROR(Log, "Invalid log filter. Must specify a log level after `:`: {}", std::string_view(begin, end)); return false; } const Level level = GetLevelByName(levelSeparator + 1, end); if (level == Level::Count) { - LOG_ERROR(Log, "Unknown log level in filter: {}", std::string_view(begin, end)); + //LOG_ERROR(Log, "Unknown log level in filter: {}", std::string_view(begin, end)); return false; } @@ -50,7 +49,7 @@ bool ParseFilterRule(Filter &instance, Iterator begin, Iterator end) { const Class logClass = GetClassByName(begin, levelSeparator); if (logClass == Class::Count) { - LOG_ERROR(Log, "Unknown log class in filter: {}", std::string(begin, end)); + //LOG_ERROR(Log, "Unknown log class in filter: {}", std::string(begin, end)); return false; } @@ -88,7 +87,6 @@ const char* GetLogClassName(Class logClass) { default: break; } - UNREACHABLE(); } const char* GetLevelName(Level logLevel) { @@ -108,7 +106,6 @@ const char* GetLevelName(Level logLevel) { break; } #undef LVL - UNREACHABLE(); } Filter::Filter(Level defaultLevel) { diff --git a/src/common/Logging/TextFormatter.cpp b/src/common/Logging/TextFormatter.cpp index 240bbc7..05b6ae5 100644 --- a/src/common/Logging/TextFormatter.cpp +++ b/src/common/Logging/TextFormatter.cpp @@ -1,12 +1,12 @@ // Copyright 2025 Xenon Emulator Project. All rights reserved. -#include "common/Assert.h" #include "common/Config.h" #include "TextFormatter.h" #include "Filter.h" #include "LogEntry.h" +#include namespace Base { namespace Log { @@ -57,7 +57,7 @@ void PrintColoredMessage(const Entry &entry) { color = ESC "[0;92m"; break; case Level::Count: - UNREACHABLE(); + break; } PrintMessage(color, entry); diff --git a/src/frontend/CMakeLists.txt b/src/frontend/CMakeLists.txt index 33dc67a..b7ad587 100644 --- a/src/frontend/CMakeLists.txt +++ b/src/frontend/CMakeLists.txt @@ -1,11 +1,14 @@ -#Copyright 2025 Pound Emulator Project.All rights reserved. +add_library(frontend STATIC) -#GUI sources -set(GUI_SOURCES ${CMAKE_CURRENT_SOURCE_DIR} / gui.cpp ${CMAKE_CURRENT_SOURCE_DIR} / - color.cpp ${CMAKE_CURRENT_SOURCE_DIR} / panels.cpp) +target_sources(frontend PRIVATE + ${CMAKE_CURRENT_SOURCE_DIR}/gui.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/color.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/panels.cpp +) -#Add all GUI sources to the main target - target_sources(Pound PRIVATE ${GUI_SOURCES}) +target_link_libraries(frontend PRIVATE common imgui SDL3::SDL3) -#Include directories for GUI - target_include_directories(Pound PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} /..) +target_include_directories(frontend PUBLIC + ${CMAKE_CURRENT_SOURCE_DIR} + ${CMAKE_CURRENT_SOURCE_DIR}/.. +) diff --git a/src/frontend/color.h b/src/frontend/color.h index 253c92e..abca286 100755 --- a/src/frontend/color.h +++ b/src/frontend/color.h @@ -1,7 +1,7 @@ #ifndef POUND_COLORS_H #define POUND_COLORS_H -#include +#include "imgui.h" #include #include diff --git a/src/frontend/gui.cpp b/src/frontend/gui.cpp index 2240d7e..c964187 100644 --- a/src/frontend/gui.cpp +++ b/src/frontend/gui.cpp @@ -1,6 +1,6 @@ #include "gui.h" #include "color.h" -#include "common/Assert.h" +#include #include "common/Logging/Log.h" #include "imgui_impl_opengl3_loader.h" @@ -16,8 +16,8 @@ static void apply_theme(); bool gui::window_init(window_t* window, const char* title, int64_t width, int64_t height) { - ASSERT(nullptr != window); - ASSERT(nullptr != title); + assert(nullptr != window); + assert(nullptr != title); bool ret = ::SDL_Init(SDL_INIT_VIDEO); if (false == ret) @@ -119,8 +119,8 @@ void gui::window_destroy(gui::window_t* window) bool gui::init_imgui(gui::window_t* main_window) { - ASSERT(nullptr != main_window->data); - ASSERT(nullptr != main_window->gl_context); + assert(nullptr != main_window->data); + assert(nullptr != main_window->gl_context); // Initialize ImGui IMGUI_CHECKVERSION(); diff --git a/src/frontend/panels.cpp b/src/frontend/panels.cpp index 4a6677b..ba37835 100644 --- a/src/frontend/panels.cpp +++ b/src/frontend/panels.cpp @@ -2,14 +2,14 @@ #include #include #include "kvm/kvm.h" -#include "common/Assert.h" +#include " int8_t gui::panel::render_performance_panel(gui::panel::performance_panel_t* panel, performance_data_t* data, std::chrono::steady_clock::time_point* last_render) { - ASSERT(nullptr != panel); - ASSERT(nullptr != data); - ASSERT(nullptr != last_render); + assert(nullptr != panel); + assert(nullptr != data); + assert(nullptr != last_render); bool is_visible = true; (void)::ImGui::Begin(PANEL_NAME_PERFORMANCE, &is_visible); @@ -82,7 +82,7 @@ int8_t gui::panel::render_performance_panel(gui::panel::performance_panel_t* pan int8_t gui::panel::render_cpu_panel(bool* show_cpu_result_popup) { - ASSERT(nullptr != show_cpu_result_popup); + assert(nullptr != show_cpu_result_popup); bool is_visible = true; (void)::ImGui::Begin(PANEL_NAME_CPU, &is_visible, ImGuiWindowFlags_NoCollapse); diff --git a/src/host/CMakeLists.txt b/src/host/CMakeLists.txt index bbd08df..ece9065 100644 --- a/src/host/CMakeLists.txt +++ b/src/host/CMakeLists.txt @@ -1,13 +1,10 @@ -# Copyright 2025 Pound Emulator Project. All rights reserved. +add_library(host STATIC) -set(HOST_SOURCES +target_sources(host PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/memory/arena.cpp ) -target_sources(Pound PRIVATE - ${HOST_SOURCES} -) - -target_include_directories(Pound PRIVATE +target_include_directories(host PUBLIC + ${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_CURRENT_SOURCE_DIR}/.. ) diff --git a/src/kvm/CMakeLists.txt b/src/kvm/CMakeLists.txt index 468a5aa..17a0077 100644 --- a/src/kvm/CMakeLists.txt +++ b/src/kvm/CMakeLists.txt @@ -1,11 +1,15 @@ -#Copyright 2025 Pound Emulator Project.All rights reserved. +add_library(kvm STATIC) -set(KVM_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/mmu.cpp - ${CMAKE_CURRENT_SOURCE_DIR}/mmio.cpp - ${CMAKE_CURRENT_SOURCE_DIR}/kvm.cpp - ${CMAKE_CURRENT_SOURCE_DIR}/guest.cpp +target_sources(kvm PRIVATE + mmu.cpp + mmio.cpp + kvm.cpp + guest.cpp ) -target_sources(Pound PRIVATE ${KVM_SOURCES}) +target_link_libraries(kvm PRIVATE common host) - target_include_directories(Pound PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} /..) +target_include_directories(kvm PUBLIC + ${CMAKE_CURRENT_SOURCE_DIR} + ${CMAKE_CURRENT_SOURCE_DIR}/.. +) diff --git a/src/kvm/guest.cpp b/src/kvm/guest.cpp index 9cd107f..5b47c12 100644 --- a/src/kvm/guest.cpp +++ b/src/kvm/guest.cpp @@ -1,5 +1,5 @@ #include "guest.h" -#include +#include namespace pound::kvm::memory { diff --git a/src/kvm/guest.h b/src/kvm/guest.h index 0118b68..0f0f344 100644 --- a/src/kvm/guest.h +++ b/src/kvm/guest.h @@ -1,7 +1,7 @@ #ifndef POUND_KVM_GUEST_H #define POUND_KVM_GUEST_H -#include +#include #include #include From 2e4567967563886ee5f47e915aab2cdd8574419f Mon Sep 17 00:00:00 2001 From: Ronald Caesar Date: Sun, 14 Sep 2025 20:36:03 -0400 Subject: [PATCH 3/4] common: Add design doc for log framework. Signed-off-by: Ronald Caesar --- src/common/DESIGN_DOC_LOGGING_FRAMEWORK.md | 124 +++++++++++++++++++++ 1 file changed, 124 insertions(+) create mode 100644 src/common/DESIGN_DOC_LOGGING_FRAMEWORK.md diff --git a/src/common/DESIGN_DOC_LOGGING_FRAMEWORK.md b/src/common/DESIGN_DOC_LOGGING_FRAMEWORK.md new file mode 100644 index 0000000..dcd2f36 --- /dev/null +++ b/src/common/DESIGN_DOC_LOGGING_FRAMEWORK.md @@ -0,0 +1,124 @@ +## **Design Document: Core Logging Subsystem** + +**Author:** GloriousTaco, Lead Developer +**Status:** FINAL +**Version:** 1.0 +**Date:** 2025-09-14 + +*Disclaimer: This was mostly written with AI. I'm not a good technical writer* + +### **1. Problem Statement** + +The Pound project's current logging system is full of object oriented abstractions with no documentation. The system currently risides in `src/common/Logging` with no one going anywhere near it. However, as we move on from prototyping to testing, we require a logging framework that provides a performant diagnostic output and is easy to maintain. + +### **2. Glossary** + +* **Log Level:** A classification of a log message's severity (e.g., TRACE, DEBUG, INFO, WARN, ERROR, FATAL). +* **Log Sink:** A destination for log messages. This can be a console, a file, a network socket, etc. +* **Structured Logging:** A practice of logging messages in a consistent, machine-readable format (e.g., JSON), with key-value pairs, rather than unstructured text strings. +* **Compile-Time Log Level:** The minimum log level that will be compiled into the binary. Messages below this level are completely removed by the preprocessor, incurring zero runtime cost. +* **Runtime Log Level:** The minimum log level that the system will process and output at runtime. This can be changed dynamically without recompiling. +* **PVM:** Pound Virtual Machine, the overall project. + +### **3. Breaking Changes** + +* This design will deprecate and forbid all usage of `printf`, `fprintf(stderr, ...)` and other direct-to-console I/O for diagnostic purposes within the PVM codebase (excluding `main.cpp` for initial setup/teardown). +* A pre-commit hook will be introduced to enforce this policy, which will cause existing pull requests to fail until they are updated to use the new logging API. +* All existing modules will require modification to adopt the new logging API. + +### **4. Success Criteria** + +* **Adoption:** 100% of diagnostic messages in the `kvm`, `common`, `host`, and `frontend` modules will use the new logging system. +* **Performance:** In a release build with the runtime log level set to `INFO`, the overhead of disabled `DEBUG` and `TRACE` log statements shall be statistically unmeasurable (<0.1% performance impact) compared to a binary compiled with logging completely disabled. +* **Usability:** A developer can successfully add a new namespaced log message and filter the system output to show logs only from their module within 15 minutes, using only the API header and a quick-start guide. + +### **5. Proposed Design** + +We will implement a lightweight, header-only, macro-based logging framework heavily inspired by systems like `spdlog` but simplified for our specific needs. The core design is built on the following tenets: + +* **Tenet 1: Performance is Paramount.** Logging is a diagnostic tool; it must never be the cause of a performance issue. The system will aggressively optimize away disabled log calls at compile time. +* **Tenet 2: Structure is Mandatory.** All log messages will be structured, capturing not just a message but also the severity level, timestamp, source location (file and line), and module. +* **Tenet 3: Control is Granular.** Developers must have fine-grained control over logging verbosity at both compile time and runtime, on a per-module basis. +* **Tenet 4: Simplicity in Use.** The API presented to developers must be simple and intuitive, encouraging adoption through macros like `LOG_WARN(...)`. + +The primary user interface will be a set of macros: +`LOG_TRACE(module, fmt, ...)` +`LOG_DEBUG(module, fmt, ...)` +`LOG_INFO(module, fmt, ...)` +`LOG_WARN(module, fmt, ...)` +`LOG_ERROR(module, fmt, ...)` +`LOG_FATAL(module, fmt, ...)` + +### **6. Technical Design** + +The system will be composed of three main parts: the frontend macros, the logging core, and the output sink. + +1. **Frontend Macros:** + * The `LOG_X` macros will be the only public-facing API. + * the `LOG_FATAL` macro will be terminal. After logging the message, it will immediately terminate the program via a call to `abort()`. + * The Log macros will first check against a `COMPILE_TIME_LOG_LEVEL`. If the message level is below this threshold, the macro will expand to nothing (`(void)0`), ensuring the code and its arguments are completely compiled out. + * If the level is sufficient, the macro will expand into a call to a logging core function, automatically passing `__FILE__`, `__LINE__`, the log level, and the module name. + * This will live in a `common/logging.h` header. + +2. **Logging Core:** + * A central `logger_log()` function will be the target of the macros. + * This function will check the message's log level against a globally configured `runtime_log_level` for the specified module. If the level is insufficient, the function returns immediately. + * If the level is sufficient, it will capture the current high-resolution timestamp, format the message string using the `fmt` library (which we already have as a dependency), and pass the formatted output string to the active sink. + * A small utility will manage the runtime log levels for each registered module (e.g., `logger_set_level("kvm", LEVEL_TRACE)`). + +3. **Output Sink:** + * The default sink will be a thread-safe, mutex-protected object that writes to `stderr`. + * The output format will be structured and non-negotiable: `[ISO8601 Timestamp] [LEVEL] [module] [file:line] Message` + * Example: `[2025-09-14T11:23:45.1234Z] [ERROR] [kvm] [mmu.cpp:412] Page table fault at GPA 0xDEADBEEF: Invalid permissions.` + * The design will allow for the possibility of replacing this sink in the future (e.g., to log to a file), but the initial implementation will be `stderr` only. + +### **7. Components** + +* **Application Modules (kvm, frontend, etc.):** These are the *producers* of log messages. They will include `common/logging.h` and use the `LOG_X` macros. +* **Logging Core (`common/logging`):** The central library responsible for filtering, formatting, and dispatching log messages. +* **Sink (`common/logging`):** The *consumer* of formatted log messages. Initially, this is the `stderr` writer. +* **Main Application (`main.cpp`):** The owner of the system. It is responsible for initializing the logging system, setting initial runtime log levels (e.g., from command-line arguments), and shutting it down cleanly. + +### **8. Dependencies** + +* **`fmt` library:** Will be used for high-performance string formatting. This is already a project dependency. +* **C++ Standard Library:** Specifically `` for timestamps and `` for thread safety in the sink. + +### **9. Major Risks & Mitigations** + +* **Risk 1: Performance Overhead.** Careless implementation could lead to significant overhead even for enabled logs (e.g., excessive string allocation, slow timestamping). + * **Mitigation:** The use of the `fmt` library is a known high-performance choice. We will benchmark the logging of 1 million messages in a tight loop to quantify the overhead and ensure it meets the success criteria. +* **Risk 2: Thread Safety Issues.** Improper locking in the sink could lead to garbled output or race conditions when multiple threads log simultaneously. + * **Mitigation:** A single `std::mutex` will protect all writes to the sink. Code will be peer-reviewed specifically for thread-safety concerns. +* **Risk 3: Slow Adoption / Incorrect Usage.** Developers may continue to use `printf` out of habit. + * **Mitigation:** The API will be designed for extreme ease of use. A short, clear guide will be written. Critically, a pre-commit hook and CI linting job will be added to the build system to automatically fail any code that uses `printf`/`fprintf`. This makes the correct path the only path. + +### **10. Out of Scope** + +* **File-based Logging:** The initial version will only log to `stderr`. A file sink is a desirable future feature but is not required for V1. +* **Network Logging:** Transmitting logs over a network is not a requirement. +* **Log Rotation:** Since we are not logging to files, rotation is not applicable. +* **Dynamic Sink Swapping:** The sink will be configured at startup and fixed for the application's lifetime. + +### **12. Alternatives Considered** + +* **Alternative #1: Use a full-featured third-party library (e.g., `spdlog`, `glog`).** + * **Pros:** Mature, feature-rich, well-tested. + * **Cons:** Adds another third-party dependency to maintain. May contain features (async logging, complex file sinks) that add unnecessary complexity and code size for our specific use case. Our needs are simple enough that a minimal, custom implementation is justified. + * **Reasons Discarded:** The primary reason is to minimize external dependencies and code footprint. We can achieve 95% of the value with 10% of the complexity by building a minimal system tailored to our exact needs, leveraging our existing `fmt` dependency. + +* **Alternative #2: "Do Nothing" (Continue using `printf`).** + * **Pros:** No development effort required. + * **Cons:** Unstructured, impossible to filter, no severity levels, no timestamps, not thread-safe. Fails to meet every requirement for a mission-safe diagnostic system. + * **Reasons Discarded:** This is a non-starter. It is fundamentally unsuitable for the project's goals. + +### **13. Appendix** +#### **Benchmarking Performance** +1. **Baseline Measurement (A):** A simple `for` loop will be created that iterates 1 million times, performing a trivial, non-optimizable operation (e.g., incrementing a `volatile` integer). The total execution time of this loop will be measured using a high-resolution clock. +2. **Disabled Log Measurement (B):** The same loop will be modified to include a `LOG_DEBUG` call. The test binary will be compiled with a `COMPILE_TIME_LOG_LEVEL` of `INFO`. This means the `LOG_DEBUG` macro will expand to `(void)0` and be completely compiled out. The execution time of this loop will be measured. +3. **Enabled Log Measurement (C):** The same loop will be run, but with the `runtime_log_level` set to allow the `LOG_DEBUG` messages to be processed and written to the sink. The sink's output will be redirected to `/dev/null` to measure the cost of formatting and dispatch, not the I/O cost of the terminal itself. The execution time will be measured. + +**Analysis:** +* The difference between **(B)** and **(A)** should be zero or statistically insignificant, proving the success criterion that disabled logs have no overhead. +* The difference between **(C)** and **(A)** represents the full overhead of an enabled log call. This allows us to calculate the average cost-per-message on our specific hardware. + From d0857d83f0232522f7744a1a0952e95e5789c8ec Mon Sep 17 00:00:00 2001 From: Ronald Caesar Date: Fri, 19 Sep 2025 20:18:19 -0400 Subject: [PATCH 4/4] common: Implement logging framework Signed-off-by: Ronald Caesar --- CMakeLists.txt | 9 +++ src/common/CMakeLists.txt | 1 + src/common/logging.cpp | 166 ++++++++++++++++++++++++++++++++++++++ src/common/logging.h | 136 +++++++++++++++++++++++++++++++ 4 files changed, 312 insertions(+) create mode 100644 src/common/logging.cpp create mode 100644 src/common/logging.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 7ac622f..367b784 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -73,6 +73,15 @@ foreach(TARGET ${POUND_PROJECT_TARGETS}) -Wcast-align -Wconversion> ) + + # Set Compile time log level for all targets. + # 1: Trace + # 2: Debug + # 3: Info + # 4: Warning + # 5: Error + # 6: Fatal + target_compile_definitions(${TARGET} PRIVATE COMPILE_TIME_LOG_LEVEL=1) endforeach() diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt index 963c14f..c5db7b3 100644 --- a/src/common/CMakeLists.txt +++ b/src/common/CMakeLists.txt @@ -6,6 +6,7 @@ target_sources(common PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/PathUtil.cpp ${CMAKE_CURRENT_SOURCE_DIR}/StringUtil.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Thread.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/logging.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Logging/Backend.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Logging/Filter.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Logging/TextFormatter.cpp diff --git a/src/common/logging.cpp b/src/common/logging.cpp new file mode 100644 index 0000000..aae9fa1 --- /dev/null +++ b/src/common/logging.cpp @@ -0,0 +1,166 @@ +#include "logging.h" +#include +#include +#include +#include + +#ifdef _WIN32 +/* + * Stupid shit like this is why I hate Windows. WHY CANT YOU JUST FOLLOW + * THE POSIX STAMDARD GOD DAMN. + */ +#define gmtime_r(src, dest) gmtime_s(dest, src) +#endif // _WIN32 + +/* + * TIMESTMP_BUFFER_LEN - The required buffer size for the timestamp format. + * + * Calculated for "%Y-%m-%dT%H:%M:%SZ": + * YYYY-mm-ddTHH:MM:SSZ + * 4+1+2+1+2+1+2+1+2+1+2+1 = 20 characters + * +1 for the null terminator = 21 bytes. + * + * We define the buffer as 32 bytes to provide a safe margin for future + * format changes, such as adding sub-second precision (e.g., ".123"). + */ +#define TIMESTMP_BUFFER_LEN 32 + +/* + * LOG_LINE_BUFFER_SIZE - A reasonable max size for a single log line. + * + * If it's too long, it will be truncated. + */ +#define LOG_LINE_BUFFER_SIZE 1024 + +/* + * Static strings to use when all else fails. + * Its unique format makes it easy to search for in logs. + */ +static const char* const FAILED_TIMESTAMP = "[TIMESTAMP_UNAVAILABLE]"; +static const char* const FAILED_LOG_LEVEL = "[LOG_LEVEL_UNAVAILABLE]"; + +log_level_t runtime_log_level = LOG_LEVEL_NONE; + +/* + * Pre-allocate a buffer for the timestamp string. + * Make it static so it's not constantly re-allocated on the stack. + */ +static char timestamp_buffer[TIMESTMP_BUFFER_LEN] = {}; + +const char* get_current_timestamp_str(void); + +void log_message(log_level_t level, const char* module_name, const char* file, int line, const char* message, ...) +{ + assert(nullptr != message); + + const char* timestamp_str = get_current_timestamp_str(); + const char* level_str = nullptr; + + if (level < runtime_log_level) + { + return; + } + else if (level <= LOG_LEVEL_NONE) + { + level_str = FAILED_LOG_LEVEL; + } + else + { + switch (level) + { + case LOG_LEVEL_TRACE: + level_str = "TRACE"; + break; + case LOG_LEVEL_DEBUG: + level_str = "DEBUG"; + break; + case LOG_LEVEL_INFO: + level_str = "INFO"; + break; + case LOG_LEVEL_WARNING: + level_str = "WARNING"; + break; + case LOG_LEVEL_ERROR: + level_str = "ERROR"; + break; + case LOG_LEVEL_FATAL: + level_str = "FATAL"; + break; + default: + level_str = FAILED_LOG_LEVEL; + break; + } + } + + char buffer[LOG_LINE_BUFFER_SIZE] = {}; + + /* Keep track of our position in the buffer */ + size_t offset = 0; + offset += (size_t)snprintf(buffer + offset, LOG_LINE_BUFFER_SIZE - offset, "[%s] [%s] [%s] [%s:%d] ", timestamp_str, + level_str, module_name, file, line); + + /* Handle the user's variadic format string. */ + va_list args; + va_start(args, message); + + if (offset < LOG_LINE_BUFFER_SIZE) + { + (void)vsnprintf(buffer + offset, LOG_LINE_BUFFER_SIZE - offset, message, args); + } + va_end(args); + + /* Print the entire, fully-formed buffer to stderr in a SINGLE, ATOMIC call. */ + fprintf(stderr, "%s\n", buffer); + + /* + * TODO(GloriousTaco:common): For guaranteed atomicity across + * threads, this entire function should be protected by a mutex. + * The fprintf call itself is usually thread-safe at the *call* level, but + * our logic requires atomicity at the *message* level. + * + * lock_logging_mutex(); + * fprintf(stderr, "%s\n", buffer); + * unlock_logging_mutex(); + */ +} + +const char* get_current_timestamp_str(void) +{ + time_t now; + + /* time() can fail, though it's rare. */ + if (time(&now) == (time_t)-1) + { + return FAILED_TIMESTAMP; + } + + struct tm time_since_epoch = {}; +#ifdef WIN32 + /* https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/gmtime-s-gmtime32-s-gmtime64-s?view=msvc-170 */ + (void) gmtime_r(&now, &time_since_epoch); + +#if 0 + TODO(GloriousTacoo:common): Someone fix this error handling. I have little + patience for anything Windows related. + if ((struct tm)-1 == &time_since_epoch) + { + return FAILED_TIMESTAMP; + } +#endif + +#else + if (nullptr == gmtime_r(&now, &time_since_epoch)) + { + return FAILED_TIMESTAMP; + } +#endif // WIN32 + + size_t bytes_written = strftime(timestamp_buffer, TIMESTMP_BUFFER_LEN, "%Y-%m-%dT%H:%M:%SZ", &time_since_epoch); + + if (0 == bytes_written) + { + return FAILED_TIMESTAMP; + } + + return timestamp_buffer; +} diff --git a/src/common/logging.h b/src/common/logging.h new file mode 100644 index 0000000..7c9540a --- /dev/null +++ b/src/common/logging.h @@ -0,0 +1,136 @@ +#ifndef POUND_COMMON_LOGGING_H +#define POUND_COMMON_LOGGING_H + +/* + * API USAGE CONTRACT: + * ------------------- + * + * Any source file (.c) that uses this logging framework MUST define + * the LOG_MODULE macro *before* including this header file. This + * provides a human-readable name for the component that is generating + * the log message. + * + * Example Usage in a .c file: + * + * #define LOG_MODULE "KVM_MMU" + * #include "common/logging.h" + * + * void mmu_translate_address(uint64_t gpa) { + * LOG_DEBUG("Translating GPA: 0x%llx", gpa); + * // ... + * if (error) { + * LOG_ERROR("Page table fault for GPA: 0x%llx", gpa); + * } + * } + */ + +/* + * log_level_t - Defines the severity for log messages. + */ +typedef enum log_level +{ + LOG_LEVEL_NONE = 0, + LOG_LEVEL_TRACE, + LOG_LEVEL_DEBUG, + LOG_LEVEL_INFO, + LOG_LEVEL_WARNING, + LOG_LEVEL_ERROR, + LOG_LEVEL_FATAL, +} log_level_t; + +extern log_level_t runtime_log_level; + +/* + * This function is apart of the initial implementation and should NEVER be + * called directly. Use LOG_DEBUG, etc. macros instead. + */ +void log_message(log_level_t level, const char* module_name, const char* file, int line, const char* message, ...); + +// ------------------------- +// ---- Internal Macros ---- +// ------------------------- + +#define __LOG_MODULE_NOT_DEFINED__ +#ifndef LOG_MODULE +#define LOG_MODULE __LOG_MODULE_NOT_DEFINED__ +#endif + +/* DO NOT USE THESE DIRECTLY. They are internal helpers. */ +#define _STRINGIFY_HELPER(x) #x +#define _STRINGIFY(x) _STRINGIFY_HELPER(x) + +#define CHECK_LOG_MODULE_DEFINED() \ + static_assert(__builtin_strcmp(_STRINGIFY(LOG_MODULE), _STRINGIFY(__LOG_MODULE_NOT_DEFINED__)) != 0, \ + "LOGGING ERROR: LOG_MODULE must be #defined before #including logging.h. Example: #define " \ + "LOG_MODULE \"MY_MODULE\"") + +#if COMPILE_TIME_LOG_LEVEL > LOG_LEVEL_TRACE +#define LOG_TRACE(format, ...) (void)0 +#else +#define LOG_TRACE(format, ...) \ + do \ + { \ + CHECK_LOG_MODULE_DEFINED(); \ + log_message(LOG_LEVEL_TRACE, LOG_MODULE, __FILE__, __LINE__, format, ##__VA_ARGS__); \ + } while (0) +#endif + +// ------------------------------- +// ---- Public Logging Macros ---- +// ------------------------------- + +#if COMPILE_TIME_LOG_LEVEL > LOG_LEVEL_DEBUG +#define LOG_DEBUG(format, ...) (void)0 +#else +#define LOG_DEBUG(format, ...) \ + do \ + { \ + CHECK_LOG_MODULE_DEFINED(); \ + log_message(LOG_LEVEL_DEBUG, LOG_MODULE, __FILE__, __LINE__, format, ##__VA_ARGS__); \ + } while (0) +#endif + +#if COMPILE_TIME_LOG_LEVEL > LOG_LEVEL_INFO +#define LOG_INFO(format, ...) (void)0 +#else +#define LOG_INFO(format, ...) \ + do \ + { \ + CHECK_LOG_MODULE_DEFINED(); \ + log_message(LOG_LEVEL_INFO, LOG_MODULE, __FILE__, __LINE__, format, ##__VA_ARGS__); \ + } while (0) +#endif + +#if COMPILE_TIME_LOG_LEVEL > LOG_LEVEL_WARNING +#define LOG_WARNING(format, ...) (void)0 +#else +#define LOG_WARNING(format, ...) \ + do \ + { \ + CHECK_LOG_MODULE_DEFINED(); \ + log_message(LOG_LEVEL_WARNING, LOG_MODULE, __FILE__, __LINE__, format, ##__VA_ARGS__); \ + } while (0) +#endif + +#if COMPILE_TIME_LOG_LEVEL > LOG_LEVEL_ERROR +#define LOG_ERROR(format, ...) (void)0 +#else +#define LOG_ERROR(format, ...) \ + do \ + { \ + CHECK_LOG_MODULE_DEFINED(); \ + log_message(LOG_LEVEL_ERROR, LOG_MODULE, __FILE__, __LINE__, format, ##__VA_ARGS__); \ + } while (0) +#endif + +#if COMPILE_TIME_LOG_LEVEL > LOG_LEVEL_FATAL +#define LOG_FATAL(format, ...) (void)0 +#else +#define LOG_FATAL(format, ...) \ + do \ + { \ + CHECK_LOG_MODULE_DEFINED(); \ + log_message(LOG_LEVEL_FATAL, LOG_MODULE, __FILE__, __LINE__, format, ##__VA_ARGS__); \ + } while (0) +#endif +#endif // POUND_COMMON_LOGGING_H