Re: [Qemu-devel] [PATCH v2 2/4] util/oslib-win32: indicate alignment for qemu_anon_ram_alloc()

[Igor Mammedov <imammedo@redhat.com>]

On Thu, 28 Jun 2018 14:14:15 +0200
David Hildenbrand <david@redhat.com> wrote:

> Let's set the alignment just like for the posix variant. This will
> implicitly set the alignment of the underlying memory region and
> therefore make memory_region_get_alignment(mr) return something > 0 for
> all memory backends applicable to PCDIMM/NVDIMM.
> 
> This will allow us to drop special handling in pc.c for
> memory_region_get_alignment(mr) == 0, as we can then assume that it is
> always set (and AFAICS >= getpagesize()).
> 
> For pc in pc_memory_plug(), under Windows TARGET_PAGE_SIZE == getpagesize(),
> therefore alignment of DIMMs will not change, and therefore also not the
> guest physical memory layout.
why not use QEMU_VMALLOC_ALIGN for consistency (on win => getpagesize())
instead of TARGET_PAGE_SIZE like linux allocator does?

Also looking at FIXME comment it notes that VirtualAlloc might have 64K
alignment (though I haven't found it in VirtualAlloc manual).
If that's true then we might need set *align to it to avoid auto-picked
address overlap with previous allocation (not really sure about it).


> For spapr in spapr_memory_plug(), an alignment of 0 would have been used
note that align == 0 would lead to crash where QEMU_ALIGN_UP() is used,
so we don't care to keep it compatible (the same like in commit 92a37a04d)

> until now. Now getpagesize() is used. But as the size of DIMMs has to be
> multiple of 256MB on spapr, automatic address asignment will not change
> the layout (no memory holes will be created as the alignment change will
> not apply if DIMMs start/end on 256MB boundaries).
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  util/oslib-win32.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> index bb5ad28bd3..6e87721419 100644
> --- a/util/oslib-win32.c
> +++ b/util/oslib-win32.c
> @@ -76,6 +76,10 @@ void *qemu_anon_ram_alloc(size_t size, uint64_t *align, bool shared)
>         memory is page aligned. */
>      ptr = VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE);
>      trace_qemu_anon_ram_alloc(size, ptr);
> +
> +    if (ptr && align) {
> +        *align = getpagesize();
> +    }
>      return ptr;
>  }
>