Re: [PATCH kvm-unit-tests v2 07/11] s390x: initialize the physical allocator

From: Paolo Bonzini <pbonzini@redhat.com>
To: Andrew Jones <drjones@redhat.com>, David Hildenbrand <david@redhat.com>
Cc: kvm@vger.kernel.org, "Radim Krčmář" <rkrcmar@redhat.com>,
	"Thomas Huth" <thuth@redhat.com>,
	"Christian Borntraeger" <borntraeger@de.ibm.com>,
	"Cornelia Huck" <cohuck@redhat.com>
Subject: Re: [PATCH kvm-unit-tests v2 07/11] s390x: initialize the physical allocator
Date: Mon, 15 Jan 2018 11:38:30 +0100	[thread overview]
Message-ID: <804f02f7-61ee-e443-e750-31cb9af00c70@redhat.com> (raw)
In-Reply-To: <20180115101614.7zazuegmbmu3p6ku@kamzik.brq.redhat.com>

On 15/01/2018 11:16, Andrew Jones wrote:
> state 2: phys-alloc is set up by the phys_alloc_init() call
>
> state 3: alloc_page() is set up, need call that allocates chunk with the
>                                  phys-allocator for free_pages()
> 
> (Instead of a chunk, all memory could be given, but then either a
>  new set of early-ops should be provided that are based on
>  alloc_page(), or all early memory allocations must use alloc_page())
> 
> state 4: virtual memory is set up by the setup_vm() call
> 
> Or, simply remove state 2 and its code and alloc-ops, changing all early
> allocations to use alloc_page(). If each arch provides an mmu_enabled()
> call, then the alloc_ops->memalign call in memalign could be replaced
> with
> 
>  if (mmu_enabled())
>     vm_memalign(...)
>  else
>     /* fallback to alloc_pages() */
> 
> I probably should have written all this in a separate thread, because
> I wouldn't want to hold this series up on a rework of the general
> memory management framework. But, anyway, thoughts on that? Paolo?

State 3 should only exist in setup_mmu, so indeed the commit message
here is not accurate.  setup_vm does the transition from phys_alloc to
page_alloc before calling setup_mmu, exactly because setup_mmu already
needs to allocate pages:

        phys_alloc_get_unused(&base, &top);
        base = (base + PAGE_SIZE - 1) & -PAGE_SIZE;
        top = top & -PAGE_SIZE;
        free_pages(phys_to_virt(base), top - base);
        page_root = setup_mmu(top);
        alloc_ops = &vmalloc_ops;

The reason for state 2 to exist is that right now on x86 it is optional
to use the MMU.  You could use page-granularity allocation, but that
would be a waste of memory if we switch to a GTest-like API that needs
to dynamically allocate memory.

However, maybe it would make sense to make it internal just like state
3.  That would mean enabling the MMU directly where we now call
phys_alloc_init.  This is independent of course of this s390 series.

Paolo