Re: [kvm-unit-tests PATCH v2 6/6] lib/x86/vm: enable malloc and friends

[Andrew Jones <drjones@redhat.com>]

On Thu, Nov 03, 2016 at 07:20:20PM +0100, Paolo Bonzini wrote:
> 
> 
> On 03/11/2016 19:12, Andrew Jones wrote:
> > On Thu, Nov 03, 2016 at 06:34:45PM +0100, Paolo Bonzini wrote:
> >> You're right, it needs alignment too.  So yeah, early_memalign's code
> >> can be repurposed as the early allocator's morecore callback.  For the
> >> current trivial malloc implementation, malloc would be implemented as
> >> ops->morecore(size, align_min).  I think we agree on the overall idea
> >> and on having only one function pointer.
> > 
> > Yup, already done. But 2, not 1, pointers. Still need 'free' too.
> 
> Didn't we say that the morecore allocator would only grow?

morecore[phys_alloc] only grows, free -> {}
morecore[vmemalign] can do both, free -> vfree

> 
> >> 3) make alloc_vpages go bottom to top (not strictly necessary but nice);
> > 
> > OK, from what base though? 0x8, i.e. just skip NULL?
> 
> Well, the right one would be "after the last thing that phys allocated".
>  For x86 however you'd have to init the phys allocator to edata.  But
> let's leave this for later.

Hmm, I see what's going on now in x86. First, setup_vm should be doing
setup_mmu *before* 'free_memory' (what I renamed to heap_init), because
it assumes it's using virtual address with page_alloc/free. Second, x86
creates an identity map up to end_of_memory, and that's the last address
the heap knows about too. So, if we want to allocate free virtual
addresses upwards, then we should start from end_of_memory.

> 
> >>
> >> 4) replaces vmalloc/vfree with memalign(align=4096)/free in x86/;
> > 
> > I thought about this already, but I wasn't sure we wanted to abandon
> > the explicit "I want virt mem" vmalloc naming. memalign/malloc will
> > be vmalloc for any unit test that does setup_vm, but otherwise (if
> > x86 adopted phys_alloc) it wouldn't. Maybe x86 should just never
> > adopt phys_alloc, leading to asserts when setup_vm hasn't been
> > called?
> 
> I don't think the tests want virtual memory specifically.  If they want,
> they use things like vmap.

OK

> 
> Paolo
> 
> >>
> >> 5) moves the virt allocator and the vmalloc ops to a new file
> >> lib/vmalloc.c (with install_page() remaining as the sole hook into
> >> arch-specific code), removing what's now dead from lib/vmalloc.c.
> > 
> > Yeah, this is a good next step. Will do.

This one looks like we'll need to cleanup the install_page parameter
expectations a bit in order to keep arch-specific stuff out of the callers,
but that should be doable.

drew