From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33506) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1byeqB-0005lk-U8 for qemu-devel@nongnu.org; Mon, 24 Oct 2016 08:59:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1byeqB-0008A2-1v for qemu-devel@nongnu.org; Mon, 24 Oct 2016 08:59:00 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60006) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1byeqA-00089w-Sa for qemu-devel@nongnu.org; Mon, 24 Oct 2016 08:58:58 -0400 References: <20161024124937.17239-1-haozhong.zhang@intel.com> From: Paolo Bonzini Message-ID: <93154c1b-84bf-1922-c495-867d78866730@redhat.com> Date: Mon, 24 Oct 2016 14:58:50 +0200 MIME-Version: 1.0 In-Reply-To: <20161024124937.17239-1-haozhong.zhang@intel.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] exec.c: workaround regression caused by alignment change in d2f39ad List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Haozhong Zhang , qemu-devel@nongnu.org Cc: Igor Mammedov , Peter Crosthwaite , Richard Henderson , Dominik Dingel , chao.p.peng@intel.com, anthony.xu@intel.com On 24/10/2016 14:49, Haozhong Zhang wrote: > Commit d2f39ad "exec.c: Ensure right alignment also for file backed ram" > added an additional alignment requirement on the size of backend file > besides the previous page size. On x86, the alignment is changed from > 4KB in QEMU 2.6 to 2MB in QEMU 2.7. > > This change breaks certain usages in QEMU 2.7 on x86, e.g. > -object memory-backend-file,id=mem1,mem-path=/tmp/,size=$SZ > -device pc-dimm,id=dimm1,memdev=mem1 > where $SZ is multiple of 4KB but not 2MB (e.g. 1023M). QEMU 2.7 > reports the following error message and aborts: > qemu-system-x86_64: -device pc-dimm,memdev=mem1,id=nv1: backend memory size must be multiple of 0x200000 > > The same regression may also happen in other platforms as indicated by > Igor Mammedov. This change is however necessary for s390 according to > the commit message of d2f39ad, so we workaround the regression by taking > the change only on s390. > > Signed-off-by: Haozhong Zhang > Reported-by: "Xu, Anthony" > --- > exec.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/exec.c b/exec.c > index e63c5a1..55915ea 100644 > --- a/exec.c > +++ b/exec.c > @@ -1254,7 +1254,11 @@ static void *file_ram_alloc(RAMBlock *block, > } > > block->page_size = qemu_fd_getpagesize(fd); > +#if defined(__s390x__) > block->mr->align = MAX(block->page_size, QEMU_VMALLOC_ALIGN); > +#else > + block->mr->align = block->page_size; > +#endif Better: block->mr->align = block->page_size; #ifdef __s390x__ if (kvm_enabled()) { block->mr->align = MAX(block->mr->align, QEMU_VMALLOC_ALIGN); } #endif Queued with this change. Paolo > > if (memory < block->page_size) { > error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to " >