[haiku-commits] Re: BRANCH xyzzy-github.x86_64 - src/system/boot/platform/bios_ia32 src/system/kernel/arch/x86_64 headers/private/kernel/arch/x86_64 src/system/kernel/arch/x86
- From: Ingo Weinhold <ingo_weinhold@xxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Wed, 27 Jun 2012 16:08:58 +0200
On 2012-06-26 at 12:48:58 [+0200], xyzzy-github.x86_64
<community@xxxxxxxxxxxx> wrote:
[...]
> diff --git a/headers/private/kernel/arch/x86_64/descriptors.h
> b/headers/private/kernel/arch/x86_64/descriptors.h
> new file mode 100644
> index 0000000..a1850f0
> --- /dev/null
> +++ b/headers/private/kernel/arch/x86_64/descriptors.h
[...]
> +static inline void
> +set_segment_descriptor(segment_descriptor* desc, uint8 type, uint8 dpl)
> +{
> + clear_segment_descriptor(desc);
> +
> + // In 64-bit mode the CPU ignores the base/limit of code/data segments,
> + // it always treats base as 0 and does no limit checks.
> + desc->base0 = 0;
> + desc->base1 = 0;
> + desc->limit0 = 0xFFFF;
> + desc->limit1 = 0xF;
Lower case, please.
> + desc->granularity = 1;
> +
> + desc->type = type;
> + desc->desc_type = DT_CODE_DATA_SEGMENT;
> + desc->dpl = dpl;
> + desc->present = 1;
> +
> + desc->long_mode = (type & DT_CODE_EXECUTE_ONLY) ? 1 : 0;
> + // Must be set to 1 for code segments only.
> +}
> +
> +
> +static inline void
> +set_tss_descriptor(segment_descriptor* _desc, uint64 base, uint32 limit)
> +{
> + clear_segment_descriptor(_desc);
> + clear_segment_descriptor(&_desc[1]);
> +
> + // The TSS descriptor is a special format in 64-bit mode, it is 16
> bytes
> + // instead of 8.
> + tss_descriptor* desc = (tss_descriptor*)_desc;
> +
> + desc->base0 = base & 0xffffff;
> + desc->base1 = ((base) >> 24) & 0xff;
> + desc->base2 = ((base) >> 32);
The parentheses around base are superfluous.
[...]
> diff --git a/src/system/boot/platform/bios_ia32/long.cpp
> b/src/system/boot/platform/bios_ia32/long.cpp
[...]
> +template<typename Type>
> +inline void
> +fix_address(FixedWidthPointer<Type>& p)
> +{
> + if(p != NULL)
Space after "if".
> + p.SetTo(fix_address(p.Get()));
> +}
> +
> +
> +static void
> +long_gdt_init()
> +{
> + // Allocate memory for the GDT.
> + segment_descriptor* gdt = (segment_descriptor*)
> + mmu_allocate_page(&gKernelArgs.arch_args.phys_gdt);
> + gKernelArgs.arch_args.vir_gdt = fix_address((addr_t)gdt);
> +
> + dprintf("GDT at phys 0x%lx, virt 0x%llx\n",
> gKernelArgs.arch_args.phys_gdt,
> + gKernelArgs.arch_args.vir_gdt);
You might have copied that from somewhere, but in new code please use "%#..."
to print the leading "0x" for hex numbers.
[...]
> +static void
> +convert_preloaded_image(preloaded_elf64_image* image)
> +{
> + fix_address(image->next);
> + fix_address(image->name);
> + fix_address(image->debug_string_table);
> + fix_address(image->syms);
> + fix_address(image->rel);
> + fix_address(image->rela);
> + fix_address(image->pltrel);
> + fix_address(image->debug_symbols);
> +}
> +
> +
> +/*! Convert all addresses in kernel_args to 64-bit addresses. */
> +static void
> +convert_kernel_args()
> +{
I wonder, if all FixedWidthPointers need to be adjusted, why not just let the
class do that automatically, i.e. in 64 bit mode just shift the pointer
address on return?
> + fix_address(gKernelArgs.boot_volume);
> + fix_address(gKernelArgs.vesa_modes);
> + fix_address(gKernelArgs.edid_info);
> + fix_address(gKernelArgs.debug_output);
> + fix_address(gKernelArgs.boot_splash);
> + fix_address(gKernelArgs.arch_args.apic);
> + fix_address(gKernelArgs.arch_args.hpet);
> +
> + convert_preloaded_image(static_cast<preloaded_elf64_image*>(
> + gKernelArgs.kernel_image.Pointer()));
> + fix_address(gKernelArgs.kernel_image);
> +
> + // Iterate over the preloaded images. Must save the next address before
> + // converting, as the next pointer will be converted.
> + preloaded_image* image = gKernelArgs.preloaded_images;
> + fix_address(gKernelArgs.preloaded_images);
> + while (image) {
"!= NULL", please.
> + preloaded_image* next = image->next;
> +
> convert_preloaded_image(static_cast<preloaded_elf64_image*>(image));
> + image = next;
> + }
> +
> + // Set correct kernel stack addresses.
> + for (uint32 i = 0; i < gKernelArgs.num_cpus; i++) {
> + gKernelArgs.cpu_kstack[i].start
> + = fix_address(gKernelArgs.cpu_kstack[i].start);
> + }
> +
> + // Fix driver settings files.
> + driver_settings_file* file = gKernelArgs.driver_settings;
> + fix_address(gKernelArgs.driver_settings);
> + while (file) {
Ditto.
> + driver_settings_file* next = file->next;
> + fix_address(file->next);
> + fix_address(file->buffer);
> + file = next;
> + }
> +}
> +
> +
> +void
> +long_start_kernel()
> +{
> + // Check whether long mode is supported.
> + cpuid_info info;
> + get_current_cpuid(&info, 0x80000001);
> + if ((info.regs.edx & (1<<29)) == 0)
Spaces around the << operator. Also in assembly code.
CU, Ingo
Other related posts:
