On 29.06.2018 17:56, Igor Mammedov wrote: > 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())
That sounds like the best alternative when having to guess what is actually going on behind the curtains. -- Thanks, David / dhildenb