Re: [Xen-devel] [PATCH 17/18] x86/efi: create new early memory allocator

[Daniel Kiper <daniel.kiper@xxxxxxxxxx>]

On Mon, Mar 02, 2015 at 05:23:49PM +0000, Jan Beulich wrote:
> >>> On 30.01.15 at 18:54, <daniel.kiper@xxxxxxxxxx> wrote:
> > --- a/xen/arch/x86/efi/efi-boot.h
> > +++ b/xen/arch/x86/efi/efi-boot.h
> > @@ -103,9 +103,35 @@ static void __init relocate_trampoline(unsigned long 
> > phys)
> >          *(u16 *)(*trampoline_ptr + (long)trampoline_ptr) = phys >> 4;
> >  }
> >
> > +#define __MALLOC_SIZE      65536
> > +
> > +static char __malloc_mem[__MALLOC_SIZE];
> > +static char *__malloc_free = NULL;
> > +
> > +static void __init *__malloc(size_t size)
>
> Please do away with all these prefixing underscores.
>
> > +{
> > +    void *ptr;
> > +
> > +    /*
> > +     * Init __malloc_free on runtime. Static initialization
> > +     * will not work because it puts virtual address there.
> > +     */
> > +    if ( __malloc_free == NULL )
> > +        __malloc_free = __malloc_mem;
> > +
> > +    ptr = __malloc_free;
> > +
> > +    __malloc_free += size;
> > +
> > +    if ( __malloc_free - __malloc_mem > sizeof(__malloc_mem) )
> > +        blexit(L"Out of static memory\r\n");
> > +
> > +    return ptr;
> > +}
>
> You're ignoring alignment requirements here altogether.

I can understand why __malloc_mem should be let's say page aligned
but I am not sure why we should care here about alignment inside
of __malloc_mem array like...

> > @@ -192,12 +218,7 @@ static void __init
> > efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,
> >
> >  static void *__init efi_arch_allocate_mmap_buffer(UINTN *map_size)
> >  {
> > -    place_string(&mbi.mem_upper, NULL);
> > -    mbi.mem_upper -= *map_size;
> > -    mbi.mem_upper &= -__alignof__(EFI_MEMORY_DESCRIPTOR);

...here...

> > -    if ( mbi.mem_upper < xen_phys_start )
> > -        return NULL;
> > -    return (void *)(long)mbi.mem_upper;
> > +    return __malloc(*map_size);
> >  }
>
> Which then even suggests that _if_ we go this route, this could be
> shared with ARM (and hence become common code again).

So, go or no go this route? If not what do you suggest?

Daniel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel