From d8efc6caf6babe278c48e8aa3277376fb68ff455 Mon Sep 17 00:00:00 2001 From: Alex Smith Date: Thu, 21 Jun 2012 18:02:23 +0100 Subject: [PATCH] Changes to kernel_args to make it identical for x86 and x86_64. * Added a FixedWidthPointer template class which uses 64-bit storage to hold a pointer. This is used in place of raw pointers in kernel_args. * Added __attribute__((packed)) to kernel_args and all structures contained within it. This is necessary due to different alignment behaviour for 32-bit and 64-bit compilation with GCC. * With these changes, kernel_args will now come out the same size for both the x86_64 kernel and the loader, excluding the preloaded_image structure which has not yet been changed. * Tested both an x86 GCC2 and GCC4 build, no problems caused by these changes. --- .../kernel/arch/x86/arch_kernel_args.h | 11 +- headers/private/kernel/boot/addr_range.h | 4 +- headers/private/kernel/boot/driver_settings.h | 3 +- headers/private/kernel/boot/elf.h | 5 +- headers/private/kernel/boot/kernel_args.h | 19 +-- .../platform/bios_ia32/platform_kernel_args.h | 6 +- .../private/kernel/util/FixedWidthPointer.h | 123 ++++++++++++++++++ src/system/boot/loader/main.cpp | 2 +- src/system/boot/platform/bios_ia32/debug.cpp | 2 +- src/system/boot/platform/bios_ia32/hpet.cpp | 2 +- src/system/boot/platform/bios_ia32/smp.cpp | 10 +- src/system/boot/platform/bios_ia32/video.cpp | 2 +- .../boot/platform/generic/video_splash.cpp | 8 +- .../boot/platform/openfirmware/video.cpp | 2 +- src/system/boot/platform/u-boot/serial.cpp | 2 +- src/system/kernel/arch/x86/apic.cpp | 2 +- src/system/kernel/arch/x86/ioapic.cpp | 2 +- .../kernel/arch/x86/timers/x86_hpet.cpp | 2 +- src/system/kernel/debug/debug.cpp | 4 +- .../kernel/debug/frame_buffer_console.cpp | 2 +- src/system/kernel/fs/vfs_boot.cpp | 2 +- 21 files changed, 174 insertions(+), 41 deletions(-) create mode 100644 headers/private/kernel/util/FixedWidthPointer.h diff --git a/headers/private/kernel/arch/x86/arch_kernel_args.h b/headers/private/kernel/arch/x86/arch_kernel_args.h index 0949d48726..de17c7ced6 100644 --- a/headers/private/kernel/arch/x86/arch_kernel_args.h +++ b/headers/private/kernel/arch/x86/arch_kernel_args.h @@ -9,6 +9,10 @@ # error This file is included from only #endif + +#include + + #define MAX_BOOT_PTABLES 4 #define _PACKED __attribute__((packed)) @@ -34,14 +38,13 @@ typedef struct { // smp stuff uint32 apic_time_cv_factor; // apic ticks per second uint32 apic_phys; - uint32 *apic; + FixedWidthPointer apic; uint32 ioapic_phys; - uint32 *ioapic; uint32 cpu_apic_id[MAX_BOOT_CPUS]; uint32 cpu_apic_version[MAX_BOOT_CPUS]; // hpet stuff uint32 hpet_phys; - uint32 *hpet; -} arch_kernel_args; + FixedWidthPointer hpet; +} _PACKED arch_kernel_args; #endif /* KERNEL_ARCH_x86_KERNEL_ARGS_H */ diff --git a/headers/private/kernel/boot/addr_range.h b/headers/private/kernel/boot/addr_range.h index 67129eb452..ae46b3cb53 100644 --- a/headers/private/kernel/boot/addr_range.h +++ b/headers/private/kernel/boot/addr_range.h @@ -13,13 +13,13 @@ typedef struct addr_range { uint64 start; uint64 size; -} addr_range; +} _PACKED addr_range; typedef struct phys_addr_range { phys_addr_t start; phys_size_t size; -} phys_addr_range; +} _PACKED phys_addr_range; #ifdef __cplusplus diff --git a/headers/private/kernel/boot/driver_settings.h b/headers/private/kernel/boot/driver_settings.h index 6a30b34277..bee57df92e 100644 --- a/headers/private/kernel/boot/driver_settings.h +++ b/headers/private/kernel/boot/driver_settings.h @@ -6,11 +6,12 @@ #define KERNEL_BOOT_DRIVER_SETTINGS_H +#include #include struct driver_settings_file { - struct driver_settings_file *next; + FixedWidthPointer next; char name[B_OS_NAME_LENGTH]; char *buffer; size_t size; diff --git a/headers/private/kernel/boot/elf.h b/headers/private/kernel/boot/elf.h index d825d440a7..3cc17049d4 100644 --- a/headers/private/kernel/boot/elf.h +++ b/headers/private/kernel/boot/elf.h @@ -9,10 +9,11 @@ #include #include #include +#include struct preloaded_image { - struct preloaded_image *next; + FixedWidthPointer next; char *name; elf_region text_region; elf_region data_region; @@ -37,7 +38,7 @@ struct preloaded_image { // the ID field will be filled out in the kernel bool is_module; // set by the module initialization code -}; +} _PACKED; #ifdef __cplusplus extern "C" { diff --git a/headers/private/kernel/boot/kernel_args.h b/headers/private/kernel/boot/kernel_args.h index 1922ae673d..bc206779ff 100644 --- a/headers/private/kernel/boot/kernel_args.h +++ b/headers/private/kernel/boot/kernel_args.h @@ -18,6 +18,9 @@ #include #include +#include + + #define CURRENT_KERNEL_ARGS_VERSION 1 #define MAX_KERNEL_ARGS_RANGE 20 @@ -42,7 +45,7 @@ typedef struct kernel_args { uint32 version; struct preloaded_image kernel_image; - struct preloaded_image *preloaded_images; + FixedWidthPointer preloaded_images; uint32 num_physical_memory_ranges; phys_addr_range physical_memory_range[MAX_PHYSICAL_MEMORY_RANGE]; @@ -58,10 +61,10 @@ typedef struct kernel_args { addr_range cpu_kstack[MAX_BOOT_CPUS]; // boot volume KMessage data - uint64 boot_volume; + FixedWidthPointer boot_volume; int32 boot_volume_size; - struct driver_settings_file *driver_settings; + FixedWidthPointer driver_settings; struct { phys_addr_range physical_buffer; @@ -72,12 +75,12 @@ typedef struct kernel_args { bool enabled; } frame_buffer; - void *vesa_modes; + FixedWidthPointer vesa_modes; uint16 vesa_modes_size; uint8 vesa_capabilities; - void *edid_info; + FixedWidthPointer edid_info; - void *debug_output; + FixedWidthPointer debug_output; uint32 debug_size; bool keep_debug_output_buffer; @@ -85,8 +88,8 @@ typedef struct kernel_args { arch_kernel_args arch_args; // bootsplash data - uint8 *boot_splash; + FixedWidthPointer boot_splash; -} kernel_args; +} _PACKED kernel_args; #endif /* KERNEL_BOOT_KERNEL_ARGS_H */ diff --git a/headers/private/kernel/boot/platform/bios_ia32/platform_kernel_args.h b/headers/private/kernel/boot/platform/bios_ia32/platform_kernel_args.h index 1b0ff82075..a191a0db14 100644 --- a/headers/private/kernel/boot/platform/bios_ia32/platform_kernel_args.h +++ b/headers/private/kernel/boot/platform/bios_ia32/platform_kernel_args.h @@ -12,6 +12,7 @@ #include #include +#include // must match SMP_MAX_CPUS in arch_smp.h @@ -25,9 +26,10 @@ typedef struct { uint16 serial_base_ports[MAX_SERIAL_PORTS]; - bios_drive *drives; // this does not contain the boot drive + FixedWidthPointer drives; + // this does not contain the boot drive apm_info apm; -} platform_kernel_args; +} _PACKED platform_kernel_args; #endif /* KERNEL_BOOT_PLATFORM_BIOS_IA32_KERNEL_ARGS_H */ diff --git a/headers/private/kernel/util/FixedWidthPointer.h b/headers/private/kernel/util/FixedWidthPointer.h new file mode 100644 index 0000000000..b8ceaaf0af --- /dev/null +++ b/headers/private/kernel/util/FixedWidthPointer.h @@ -0,0 +1,123 @@ +/* + * Copyright 2012, Alex Smith, alex@alex-smith.me.uk. + * Distributed under the terms of the MIT License. + */ +#ifndef KERNEL_UTIL_FIXED_WIDTH_POINTER_H +#define KERNEL_UTIL_FIXED_WIDTH_POINTER_H + + +#include + + +/*! + \class FixedWidthPointer + \brief Pointer class with fixed size (64-bit) storage. + + This class is a pointer-like class that uses a fixed size 64-bit storage. + This is used to make kernel_args compatible (i.e. the same size) for both + 32-bit and 64-bit kernels. +*/ +template +class FixedWidthPointer { +public: + operator Type*() const + { + return (Type *)(addr_t)fValue; + } + + operator addr_t() const + { + return (addr_t)fValue; + } + + Type &operator*() const + { + return *(Type *)*this; + } + + Type *operator->() const + { + return *this; + } + + FixedWidthPointer &operator=(const FixedWidthPointer &p) + { + fValue = p.fValue; + return *this; + } + + FixedWidthPointer &operator=(Type *p) + { + fValue = (addr_t)p; + return *this; + } + + /*! + Get the 64-bit pointer value. + \return Pointer address. + */ + uint64 Get() const + { + return fValue; + } + + /*! + Set the 64-bit pointer value. + \param addr New address for the pointer. + */ + void SetTo(uint64 addr) + { + fValue = addr; + } +private: + uint64 fValue; +} _PACKED; + + +// Specialization for void pointers, can be converted to another pointer type. +template<> +class FixedWidthPointer { +public: + operator void*() const + { + return (void *)(addr_t)fValue; + } + + template + operator OtherType*() const + { + return (OtherType *)(addr_t)fValue; + } + + operator addr_t() const + { + return (addr_t)fValue; + } + + FixedWidthPointer &operator=(const FixedWidthPointer &p) + { + fValue = p.fValue; + return *this; + } + + FixedWidthPointer &operator=(void *p) + { + fValue = (addr_t)p; + return *this; + } + + uint64 Get() const + { + return fValue; + } + + void SetTo(uint64 addr) + { + fValue = addr; + } +private: + uint64 fValue; +} _PACKED; + + +#endif /* KERNEL_UTIL_FIXED_WIDTH_POINTER_H */ diff --git a/src/system/boot/loader/main.cpp b/src/system/boot/loader/main.cpp index 2af8c86316..63a78a1e9a 100644 --- a/src/system/boot/loader/main.cpp +++ b/src/system/boot/loader/main.cpp @@ -128,7 +128,7 @@ main(stage2_args *args) buffer = (void*)(((addr_t)buffer + 3) & ~(addr_t)0x3); memcpy(buffer, gBootVolume.Buffer(), gBootVolume.ContentSize()); - gKernelArgs.boot_volume = (addr_t)buffer; + gKernelArgs.boot_volume = buffer; gKernelArgs.boot_volume_size = gBootVolume.ContentSize(); // ToDo: cleanup, heap_release() etc. diff --git a/src/system/boot/platform/bios_ia32/debug.cpp b/src/system/boot/platform/bios_ia32/debug.cpp index 73d2906415..deb389dce5 100644 --- a/src/system/boot/platform/bios_ia32/debug.cpp +++ b/src/system/boot/platform/bios_ia32/debug.cpp @@ -162,7 +162,7 @@ debug_cleanup(void) if (!gKernelArgs.keep_debug_output_buffer) { gKernelArgs.debug_output = kernel_args_malloc(sBufferPosition); - if (gKernelArgs.debug_output != NULL) { + if (gKernelArgs.debug_output) { memcpy(gKernelArgs.debug_output, sBuffer, sBufferPosition); gKernelArgs.debug_size = sBufferPosition; } diff --git a/src/system/boot/platform/bios_ia32/hpet.cpp b/src/system/boot/platform/bios_ia32/hpet.cpp index 42568d214b..de27385b33 100644 --- a/src/system/boot/platform/bios_ia32/hpet.cpp +++ b/src/system/boot/platform/bios_ia32/hpet.cpp @@ -50,6 +50,6 @@ hpet_init(void) TRACE(("hpet_init: found HPET at %x.\n", hpet->hpet_address.address)); gKernelArgs.arch_args.hpet_phys = hpet->hpet_address.address; - gKernelArgs.arch_args.hpet = (uint32 *)mmu_map_physical_memory( + gKernelArgs.arch_args.hpet = (void *)mmu_map_physical_memory( gKernelArgs.arch_args.hpet_phys, B_PAGE_SIZE, kDefaultPageFlags); } diff --git a/src/system/boot/platform/bios_ia32/smp.cpp b/src/system/boot/platform/bios_ia32/smp.cpp index fcc1bb640b..f31af15f2a 100644 --- a/src/system/boot/platform/bios_ia32/smp.cpp +++ b/src/system/boot/platform/bios_ia32/smp.cpp @@ -60,21 +60,21 @@ static int smp_get_current_cpu(void); static uint32 apic_read(uint32 offset) { - return *(volatile uint32 *)((uint32)gKernelArgs.arch_args.apic + offset); + return *(volatile uint32 *)((addr_t)gKernelArgs.arch_args.apic + offset); } static void apic_write(uint32 offset, uint32 data) { - *(volatile uint32 *)((uint32)gKernelArgs.arch_args.apic + offset) = data; + *(volatile uint32 *)((addr_t)gKernelArgs.arch_args.apic + offset) = data; } static int smp_get_current_cpu(void) { - if (gKernelArgs.arch_args.apic == NULL) + if (!gKernelArgs.arch_args.apic) return 0; uint8 apicID = apic_read(APIC_ID) >> 24; @@ -451,10 +451,10 @@ smp_init_other_cpus(void) (void *)gKernelArgs.arch_args.ioapic_phys)); // map in the apic - gKernelArgs.arch_args.apic = (uint32 *)mmu_map_physical_memory( + gKernelArgs.arch_args.apic = (void *)mmu_map_physical_memory( gKernelArgs.arch_args.apic_phys, B_PAGE_SIZE, kDefaultPageFlags); - TRACE(("smp: apic (mapped) = %p\n", gKernelArgs.arch_args.apic)); + TRACE(("smp: apic (mapped) = %p\n", (void *)gKernelArgs.arch_args.apic)); // calculate how fast the apic timer is calculate_apic_timer_conversion_factor(); diff --git a/src/system/boot/platform/bios_ia32/video.cpp b/src/system/boot/platform/bios_ia32/video.cpp index fa9b1fba8c..be634ff29a 100644 --- a/src/system/boot/platform/bios_ia32/video.cpp +++ b/src/system/boot/platform/bios_ia32/video.cpp @@ -957,7 +957,7 @@ platform_init_video(void) } gKernelArgs.edid_info = kernel_args_malloc(sizeof(edid1_info)); - if (gKernelArgs.edid_info != NULL) + if (gKernelArgs.edid_info) memcpy(gKernelArgs.edid_info, &info, sizeof(edid1_info)); } diff --git a/src/system/boot/platform/generic/video_splash.cpp b/src/system/boot/platform/generic/video_splash.cpp index 8d3cb49a8e..ceebb2eab1 100644 --- a/src/system/boot/platform/generic/video_splash.cpp +++ b/src/system/boot/platform/generic/video_splash.cpp @@ -146,12 +146,12 @@ video_display_splash(addr_t frameBuffer) // pointer into the lower half of the icons image data gKernelArgs.boot_splash = (uint8*)kernel_args_malloc(uncompressedSize); - if (gKernelArgs.boot_splash == NULL) + if (!gKernelArgs.boot_splash) return B_NO_MEMORY; uncompress(kSplashIcons8BitCompressedImage, sizeof(kSplashIcons8BitCompressedImage), gKernelArgs.boot_splash, uncompressedSize); - lowerHalfIconImage = gKernelArgs.boot_splash + lowerHalfIconImage = (uint8 *)gKernelArgs.boot_splash + (kSplashIconsWidth * iconsHalfHeight); break; default: // 24bits is assumed here @@ -159,12 +159,12 @@ video_display_splash(addr_t frameBuffer) // pointer into the lower half of the icons image data gKernelArgs.boot_splash = (uint8*)kernel_args_malloc(uncompressedSize); - if (gKernelArgs.boot_splash == NULL) + if (!gKernelArgs.boot_splash) return B_NO_MEMORY; uncompress(kSplashIcons24BitCompressedImage, sizeof(kSplashIcons24BitCompressedImage), gKernelArgs.boot_splash, uncompressedSize); - lowerHalfIconImage = gKernelArgs.boot_splash + lowerHalfIconImage = (uint8 *)gKernelArgs.boot_splash + (kSplashIconsWidth * iconsHalfHeight) * 3; break; } diff --git a/src/system/boot/platform/openfirmware/video.cpp b/src/system/boot/platform/openfirmware/video.cpp index 639723d09b..d996cd9035 100644 --- a/src/system/boot/platform/openfirmware/video.cpp +++ b/src/system/boot/platform/openfirmware/video.cpp @@ -126,7 +126,7 @@ platform_init_video(void) edid_dump(&info); #endif gKernelArgs.edid_info = kernel_args_malloc(sizeof(edid1_info)); - if (gKernelArgs.edid_info != NULL) + if (gKernelArgs.edid_info) memcpy(gKernelArgs.edid_info, &info, sizeof(edid1_info)); } diff --git a/src/system/boot/platform/u-boot/serial.cpp b/src/system/boot/platform/u-boot/serial.cpp index 12c6357d26..4ba65ef26a 100644 --- a/src/system/boot/platform/u-boot/serial.cpp +++ b/src/system/boot/platform/u-boot/serial.cpp @@ -83,7 +83,7 @@ serial_cleanup(void) return; gKernelArgs.debug_output = kernel_args_malloc(sBufferPosition); - if (gKernelArgs.debug_output != NULL) { + if (gKernelArgs.debug_output) { memcpy(gKernelArgs.debug_output, sBuffer, sBufferPosition); gKernelArgs.debug_size = sBufferPosition; } diff --git a/src/system/kernel/arch/x86/apic.cpp b/src/system/kernel/arch/x86/apic.cpp index 2c83935402..12492003ae 100644 --- a/src/system/kernel/arch/x86/apic.cpp +++ b/src/system/kernel/arch/x86/apic.cpp @@ -68,7 +68,7 @@ apic_disable_local_ints() status_t apic_init(kernel_args *args) { - if (args->arch_args.apic == NULL) + if (!args->arch_args.apic) return B_NO_INIT; sLocalAPIC = args->arch_args.apic; diff --git a/src/system/kernel/arch/x86/ioapic.cpp b/src/system/kernel/arch/x86/ioapic.cpp index 6d40b1645b..fa23b17571 100644 --- a/src/system/kernel/arch/x86/ioapic.cpp +++ b/src/system/kernel/arch/x86/ioapic.cpp @@ -650,7 +650,7 @@ ioapic_init(kernel_args* args) &ioapic_end_of_interrupt }; - if (args->arch_args.apic == NULL) + if (!args->arch_args.apic) return; if (args->arch_args.ioapic_phys == 0) { diff --git a/src/system/kernel/arch/x86/timers/x86_hpet.cpp b/src/system/kernel/arch/x86/timers/x86_hpet.cpp index d5dfa48693..e75a8795e0 100644 --- a/src/system/kernel/arch/x86/timers/x86_hpet.cpp +++ b/src/system/kernel/arch/x86/timers/x86_hpet.cpp @@ -217,7 +217,7 @@ hpet_init(struct kernel_args *args) /* hpet_acpi_probe() through a similar "scan spots" table to that of smp.cpp. Seems to be the most elegant solution right now. */ - if (args->arch_args.hpet == NULL) + if (!args->arch_args.hpet) return B_ERROR; if (sHPETRegs == NULL) { diff --git a/src/system/kernel/debug/debug.cpp b/src/system/kernel/debug/debug.cpp index 64810c204d..0a4d759dd1 100644 --- a/src/system/kernel/debug/debug.cpp +++ b/src/system/kernel/debug/debug.cpp @@ -1401,7 +1401,7 @@ syslog_init_post_vm(struct kernel_args* args) sSyslogMessage->ident[0] = '\0'; //strcpy(sSyslogMessage->ident, "KERNEL"); - if (args->debug_output != NULL) + if (args->debug_output) syslog_write((const char*)args->debug_output, args->debug_size, false); char revisionBuffer[64]; @@ -1431,7 +1431,7 @@ err1: static status_t syslog_init(struct kernel_args* args) { - if (!args->keep_debug_output_buffer || args->debug_output == NULL) + if (!args->keep_debug_output_buffer || !args->debug_output) return B_OK; sSyslogBuffer = create_ring_buffer_etc(args->debug_output, args->debug_size, diff --git a/src/system/kernel/debug/frame_buffer_console.cpp b/src/system/kernel/debug/frame_buffer_console.cpp index 5a1f67d865..205f50d9d5 100644 --- a/src/system/kernel/debug/frame_buffer_console.cpp +++ b/src/system/kernel/debug/frame_buffer_console.cpp @@ -442,7 +442,7 @@ frame_buffer_console_init(kernel_args* args) add_boot_item(VESA_MODES_BOOT_INFO, sVesaModes, args->vesa_modes_size); } - if (args->edid_info != NULL) { + if (args->edid_info) { edid1_info* info = (edid1_info*)malloc(sizeof(edid1_info)); if (info != NULL) { memcpy(info, args->edid_info, sizeof(edid1_info)); diff --git a/src/system/kernel/fs/vfs_boot.cpp b/src/system/kernel/fs/vfs_boot.cpp index ac039fb0be..b8240d71f2 100644 --- a/src/system/kernel/fs/vfs_boot.cpp +++ b/src/system/kernel/fs/vfs_boot.cpp @@ -325,7 +325,7 @@ static status_t get_boot_partitions(kernel_args* args, PartitionStack& partitions) { KMessage bootVolume; - bootVolume.SetTo((void *)(addr_t)args->boot_volume, args->boot_volume_size); + bootVolume.SetTo(args->boot_volume, args->boot_volume_size); dprintf("get_boot_partitions(): boot volume message:\n"); bootVolume.Dump(&dprintf);