On Fri, 29 Jun 2018 17:39:10 +0200
David Hildenbrand <da...@redhat.com> wrote:
> On 29.06.2018 16:49, Igor Mammedov wrote:
> > On Thu, 28 Jun 2018 14:14:15 +0200
> > David Hildenbrand <da...@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?
>
> Sure we can do that, I wanted to match here exactly what has been
> written in the comment.
>
> >
> > 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).
>
> "To determine the size of a page and the allocation granularity on the
> host computer, use the GetSystemInfo" [1]
>
> "The size of the region, in bytes. If the lpAddress parameter is NULL,
> this value is rounded up to the next page boundary. " [1]
>
> Historically, this seems to be 64k. But it will always be at least 4k
> (page size). So what we could do is query the actual allocation granularity:
>
> int get_allocation_granularity(void) {
> SYSTEM_INFO system_info;
>
> GetSystemInfo(&system_info);
> return system_info.dwAllocationGranularity
> }
>
>
> "dwAllocationGranularity: The granularity for the starting address at
> which virtual memory can be allocated. For more information, see
> VirtualAlloc." [2]
>
> What do you think?
maybe do following:
*align = MAX(get_allocation_granularity(), getpagesize())
>
> >
> >
> >> 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)
>
> Good point!
>
> [1]
> https://msdn.microsoft.com/en-us/library/windows/desktop/aa366887(v=vs.85).aspx
> [2]
> https://msdn.microsoft.com/en-us/library/windows/desktop/ms724958(v=vs.85).aspx
>
