[haiku-commits] Re: BRANCH xyzzy-github.x86_64 - src/system/boot/loader src/system/boot/platform/bios_ia32 headers/private/kernel/boot src/system/kernel/arch/x86_64 src/system/boot/platform/openfirmware

  • From: "Ingo Weinhold" <ingo_weinhold@xxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Mon, 25 Jun 2012 03:34:32 +0200

xyzzy-github.x86_64 wrote:
> diff --git a/src/system/boot/platform/bios_ia32/mmu.cpp 
> b/src/system/boot/platform/bios_ia32/mmu.cpp
> index cfed3de..344abf7 100644
> --- a/src/system/boot/platform/bios_ia32/mmu.cpp
> +++ b/src/system/boot/platform/bios_ia32/mmu.cpp
> @@ -769,6 +769,43 @@ platform_free_region(void *address, size_t size)
>  }
>  
>  
> +status_t
> +platform_allocate_elf_region(uint32 *_address, uint32 size, uint8 protection,
> + void **_mappedAddress)
> +{
> + void *address = mmu_allocate((void *)*_address, size);
> + if (address == NULL)
> + return B_NO_MEMORY;
> +
> + *_address = (uint32)address;
> + *_mappedAddress = address;
> + return B_OK;
> +}
> +
> +
> +status_t
> +platform_allocate_elf_region(uint64 *_address, uint64 size, uint8 protection,
> + void **_mappedAddress)
> +{
> + // The 64-bit kernel is loaded to 0xFFFFFFFF80000000. You'll notice that
> + // the low 32 bits of this address are the same as the 32-bit KERNEL_BASE
> + // (0x80000000). Therefore, the way this function is implemented is to use
> + // mmu_allocate() and then set the upper 32 bits to all 1s. The long mode
> + // switch code will remap everything to the correct addresses.
> +
> + void *address = mmu_allocate((void *)(addr_t)(*_address & 0xFFFFFFFF), 
> size);
> + if (address == NULL)
> + return B_NO_MEMORY;
> +
> + // This is the address that the ELF loading code will access the mapping
> + // through.
> + *_mappedAddress = address;
> +
> + *_address = (uint64)(uint32)address | 0xFFFFFFFF00000000LL;
> + return B_OK;
> +}

Neither function looks particularly platform dependent. The 32 bit version not 
at all and the 64 bit version only in the respect that it assumes 32 bit 
pointer width. But that's something that can be determined at compile time 
(even at pre-processing time using the respective macro). Please move both to 
the generic ELF loader code in the boot loader, so we don't have unnecessary 
code duplication. Moreover the hardcoded addresses here (0x80000000 and 
0xffffffff80000000) should be KERNEL_BASE and something like 
KERNEL_BASE_64_BIT, resulting in:

 *_address = (uint64)(addr_t)address + KERNEL_BASE_64_BIT - KERNEL_BASE;

That also makes it obvious that the delta is a compile time constant (new macro 
for convenience -- ideally one that resolves to 0 in all other cases, so it can 
be added/subtracted unconditionally). Hence it doesn't need to be saved or 
passed anywhere, which should simplify things quite a bit.

A style remark: We generally use lower case letters in hex number, though the 
coding guidelines don't mention those.

CU, Ingo

Other related posts: