* [Qemu-devel] Regression caused by d2f39ad "exec.c: Ensure right alignment also for file backed ram" @ 2016-10-21 8:28 Haozhong Zhang 2016-10-21 8:39 ` Haozhong Zhang ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: Haozhong Zhang @ 2016-10-21 8:28 UTC (permalink / raw) To: qemu-devel Cc: Paolo Bonzini, Peter Crosthwaite, Richard Henderson, Dominik Dingel, Eduardo Habkost, Igor Mammedov, anthony.xu, chao.p.peng Hi, Commit d2f39ad "exec.c: Ensure right alignment also for file backed ram" added an additional alignment requirement besides to the previous page size for the size of the backend file. On x86, the alignment requirement for the size of the backend file is changed from 4KB in QEMU 2.6 to 2MB in QEMU 2.7. This change immediately breaks the usage of "-object memory-backend-file,...,size=$SIZE" on x86, where $SIZE is multiple of 4KB but not 2MB. It works on QEMU 2.6. Could this be considered as a regression? The commit message shows it's for s390. I'm wondering whether the same regression happens on s390 and ARM. If not, could I fix the regression on x86 by keeping using the original alignment only on x86, e.g. modified exec.c @@ -1254,7 +1254,11 @@ static void *file_ram_alloc(RAMBlock *block, } block->page_size = qemu_fd_getpagesize(fd); +#if defined(__x86_64__) || defined(__i386__) + block->mr->align = block->page_size; +#else block->mr->align = MAX(block->page_size, QEMU_VMALLOC_ALIGN); +#endif if (memory < block->page_size) { error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to " Thanks, Haozhong ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] Regression caused by d2f39ad "exec.c: Ensure right alignment also for file backed ram" 2016-10-21 8:28 [Qemu-devel] Regression caused by d2f39ad "exec.c: Ensure right alignment also for file backed ram" Haozhong Zhang @ 2016-10-21 8:39 ` Haozhong Zhang 2016-10-21 12:44 ` Igor Mammedov 2016-10-21 14:12 ` Igor Mammedov 2016-10-24 11:44 ` Igor Mammedov 2 siblings, 1 reply; 8+ messages in thread From: Haozhong Zhang @ 2016-10-21 8:39 UTC (permalink / raw) To: qemu-devel Cc: chao.p.peng, Eduardo Habkost, Peter Crosthwaite, Dominik Dingel, anthony.xu, Igor Mammedov, Paolo Bonzini, Richard Henderson, Xiao Guangrong On 10/21/2016 04:28 PM, Haozhong Zhang wrote: > Hi, > > Commit d2f39ad "exec.c: Ensure right alignment also for file backed > ram" added an additional alignment requirement besides to the previous > page size for the size of the backend file. On x86, the alignment > requirement for the size of the backend file is changed from 4KB in > QEMU 2.6 to 2MB in QEMU 2.7. > > This change immediately breaks the usage of "-object > memory-backend-file,...,size=$SIZE" I forgot the frontend part, the broken usage should be -object memory-backend-file,id=mem1,mem-path=$FILE,size=$SIZE -device nvdimm,id=nv1,memdev=mem1 where $SIZE is multiple of 4KB but not 2MB. It works on QEMU 2.6, but fails on 2.7 when pc_dimm_memory_plug() tries to plug the nvdimm device. Haozhong > on x86, where $SIZE is multiple of 4KB but not 2MB. It works on QEMU 2.6. > Could this be considered as a regression? > > The commit message shows it's for s390. I'm wondering whether the same > regression happens on s390 and ARM. If not, could I fix the regression > on x86 by keeping using the original alignment only on x86, e.g. > > modified exec.c > @@ -1254,7 +1254,11 @@ static void *file_ram_alloc(RAMBlock *block, > } > > block->page_size = qemu_fd_getpagesize(fd); > +#if defined(__x86_64__) || defined(__i386__) > + block->mr->align = block->page_size; > +#else > block->mr->align = MAX(block->page_size, QEMU_VMALLOC_ALIGN); > +#endif > > if (memory < block->page_size) { > error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal > to " > > > Thanks, > Haozhong > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] Regression caused by d2f39ad "exec.c: Ensure right alignment also for file backed ram" 2016-10-21 8:39 ` Haozhong Zhang @ 2016-10-21 12:44 ` Igor Mammedov 0 siblings, 0 replies; 8+ messages in thread From: Igor Mammedov @ 2016-10-21 12:44 UTC (permalink / raw) To: Haozhong Zhang Cc: qemu-devel, chao.p.peng, Eduardo Habkost, Peter Crosthwaite, Dominik Dingel, anthony.xu, Paolo Bonzini, Richard Henderson, Xiao Guangrong On Fri, 21 Oct 2016 16:39:08 +0800 Haozhong Zhang <haozhong.zhang@intel.com> wrote: > On 10/21/2016 04:28 PM, Haozhong Zhang wrote: > > Hi, > > > > Commit d2f39ad "exec.c: Ensure right alignment also for file backed > > ram" added an additional alignment requirement besides to the previous > > page size for the size of the backend file. On x86, the alignment > > requirement for the size of the backend file is changed from 4KB in > > QEMU 2.6 to 2MB in QEMU 2.7. > > > > This change immediately breaks the usage of "-object > > memory-backend-file,...,size=$SIZE" > > I forgot the frontend part, the broken usage should be > -object memory-backend-file,id=mem1,mem-path=$FILE,size=$SIZE > -device nvdimm,id=nv1,memdev=mem1 > where $SIZE is multiple of 4KB but not 2MB. > > It works on QEMU 2.6, but fails on 2.7 when pc_dimm_memory_plug() tries > to plug the nvdimm device. it affect's not only nvdimm but also pc-dimm, qemu -m 1G,slots=10,maxmem=10G \ -object memory-backend-file,id=mem1,mem-path=/tmp,size=0x3e2000 -device pc-dimm,memdev=mem1 qemu: -device pc-dimm,memdev=mem1: backend memory size must be multiple of 0x200000 As for nvdimms d2f39ad commit changes auto-generated GPA when 'addr' property is not provided pre bug: mhp_pc_dimm_assigned_address 0x100000000 mhp_pc_dimm_assigned_slot 0x0 mhp_pc_dimm_assigned_address 0x1003e2000 mhp_pc_dimm_assigned_slot 0x1 vs post bug: mhp_pc_dimm_assigned_address 0x100000000 mhp_pc_dimm_assigned_slot 0 mhp_pc_dimm_assigned_address 0x100200000 mhp_pc_dimm_assigned_slot 1 or if 'addr' is provided, qemu will fail with "address must be aligned to 0x200000 bytes" so migration from qemu-2.6 and older builds to 2.7 and later isn't possible due to inability to start target with 2.6-page_based alignment. > > Haozhong > > > on x86, where $SIZE is multiple of 4KB but not 2MB. It works on QEMU 2.6. > > Could this be considered as a regression? > > > > The commit message shows it's for s390. I'm wondering whether the same > > regression happens on s390 and ARM. If not, could I fix the regression > > on x86 by keeping using the original alignment only on x86, e.g. > > > > modified exec.c > > @@ -1254,7 +1254,11 @@ static void *file_ram_alloc(RAMBlock *block, > > } > > > > block->page_size = qemu_fd_getpagesize(fd); > > +#if defined(__x86_64__) || defined(__i386__) > > + block->mr->align = block->page_size; > > +#else > > block->mr->align = MAX(block->page_size, QEMU_VMALLOC_ALIGN); > > +#endif > > > > if (memory < block->page_size) { > > error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal > > to " > > > > > > Thanks, > > Haozhong > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] Regression caused by d2f39ad "exec.c: Ensure right alignment also for file backed ram" 2016-10-21 8:28 [Qemu-devel] Regression caused by d2f39ad "exec.c: Ensure right alignment also for file backed ram" Haozhong Zhang 2016-10-21 8:39 ` Haozhong Zhang @ 2016-10-21 14:12 ` Igor Mammedov 2016-10-24 11:44 ` Igor Mammedov 2 siblings, 0 replies; 8+ messages in thread From: Igor Mammedov @ 2016-10-21 14:12 UTC (permalink / raw) To: Haozhong Zhang Cc: qemu-devel, chao.p.peng, Eduardo Habkost, Peter Crosthwaite, Dominik Dingel, anthony.xu, Paolo Bonzini, Richard Henderson On Fri, 21 Oct 2016 16:28:01 +0800 Haozhong Zhang <haozhong.zhang@intel.com> wrote: > Hi, > > Commit d2f39ad "exec.c: Ensure right alignment also for file backed > ram" added an additional alignment requirement besides to the previous > page size for the size of the backend file. On x86, the alignment > requirement for the size of the backend file is changed from 4KB in > QEMU 2.6 to 2MB in QEMU 2.7. > > This change immediately breaks the usage of "-object memory-backend-file,...,size=$SIZE" > on x86, where $SIZE is multiple of 4KB but not 2MB. It works on QEMU 2.6. > Could this be considered as a regression? > > The commit message shows it's for s390. I'm wondering whether the same > regression happens on s390 and ARM. If not, could I fix the regression > on x86 by keeping using the original alignment only on x86, e.g. > > modified exec.c > @@ -1254,7 +1254,11 @@ static void *file_ram_alloc(RAMBlock *block, > } > > block->page_size = qemu_fd_getpagesize(fd); > +#if defined(__x86_64__) || defined(__i386__) SPAPR supports mem hotplug so it's also affected > + block->mr->align = block->page_size; > +#else > block->mr->align = MAX(block->page_size, QEMU_VMALLOC_ALIGN); > +#endif > > if (memory < block->page_size) { > error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to " > > > Thanks, > Haozhong > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] Regression caused by d2f39ad "exec.c: Ensure right alignment also for file backed ram" 2016-10-21 8:28 [Qemu-devel] Regression caused by d2f39ad "exec.c: Ensure right alignment also for file backed ram" Haozhong Zhang 2016-10-21 8:39 ` Haozhong Zhang 2016-10-21 14:12 ` Igor Mammedov @ 2016-10-24 11:44 ` Igor Mammedov 2016-10-24 12:06 ` Haozhong Zhang 2016-10-24 12:33 ` Dominik Dingel 2 siblings, 2 replies; 8+ messages in thread From: Igor Mammedov @ 2016-10-24 11:44 UTC (permalink / raw) To: Haozhong Zhang Cc: qemu-devel, chao.p.peng, Eduardo Habkost, Peter Crosthwaite, Dominik Dingel, anthony.xu, Paolo Bonzini, Richard Henderson On Fri, 21 Oct 2016 16:28:01 +0800 Haozhong Zhang <haozhong.zhang@intel.com> wrote: > Hi, > > Commit d2f39ad "exec.c: Ensure right alignment also for file backed > ram" added an additional alignment requirement besides to the previous > page size for the size of the backend file. On x86, the alignment > requirement for the size of the backend file is changed from 4KB in > QEMU 2.6 to 2MB in QEMU 2.7. > > This change immediately breaks the usage of "-object memory-backend-file,...,size=$SIZE" > on x86, where $SIZE is multiple of 4KB but not 2MB. It works on QEMU 2.6. > Could this be considered as a regression? > > The commit message shows it's for s390. I'm wondering whether the same > regression happens on s390 and ARM. If not, could I fix the regression > on x86 by keeping using the original alignment only on x86, e.g. I don't see a nice way to work around this regression, so perhaps we should tree 2.7 as broken (wrt migration when *dimm devices with file backends with pagesize < 2Mb are used) and unbreak 2.8 and onwards. Could you send a proper patch to list with commit message explaining what is broken and how + CLI example. > > modified exec.c > @@ -1254,7 +1254,11 @@ static void *file_ram_alloc(RAMBlock *block, > } > > block->page_size = qemu_fd_getpagesize(fd); > +#if defined(__x86_64__) || defined(__i386__) > + block->mr->align = block->page_size; > +#else > block->mr->align = MAX(block->page_size, QEMU_VMALLOC_ALIGN); maybe it's worth to do other way around i.e. for special s390 case do above and for the rest leave page_size alignment? > +#endif > > if (memory < block->page_size) { > error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to " > > > Thanks, > Haozhong > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] Regression caused by d2f39ad "exec.c: Ensure right alignment also for file backed ram" 2016-10-24 11:44 ` Igor Mammedov @ 2016-10-24 12:06 ` Haozhong Zhang 2016-10-24 12:33 ` Dominik Dingel 1 sibling, 0 replies; 8+ messages in thread From: Haozhong Zhang @ 2016-10-24 12:06 UTC (permalink / raw) To: Igor Mammedov Cc: qemu-devel, chao.p.peng, Eduardo Habkost, Peter Crosthwaite, Dominik Dingel, anthony.xu, Paolo Bonzini, Richard Henderson On 10/24/16 13:44 +0200, Igor Mammedov wrote: >On Fri, 21 Oct 2016 16:28:01 +0800 >Haozhong Zhang <haozhong.zhang@intel.com> wrote: > >> Hi, >> >> Commit d2f39ad "exec.c: Ensure right alignment also for file backed >> ram" added an additional alignment requirement besides to the previous >> page size for the size of the backend file. On x86, the alignment >> requirement for the size of the backend file is changed from 4KB in >> QEMU 2.6 to 2MB in QEMU 2.7. >> >> This change immediately breaks the usage of "-object memory-backend-file,...,size=$SIZE" >> on x86, where $SIZE is multiple of 4KB but not 2MB. It works on QEMU 2.6. >> Could this be considered as a regression? >> >> The commit message shows it's for s390. I'm wondering whether the same >> regression happens on s390 and ARM. If not, could I fix the regression >> on x86 by keeping using the original alignment only on x86, e.g. >I don't see a nice way to work around this regression, so perhaps >we should tree 2.7 as broken (wrt migration when *dimm devices with file >backends with pagesize < 2Mb are used) and unbreak 2.8 and onwards. > >Could you send a proper patch to list with commit message >explaining what is broken and how + CLI example. > Sure, I'll send a patch later. >> >> modified exec.c >> @@ -1254,7 +1254,11 @@ static void *file_ram_alloc(RAMBlock *block, >> } >> >> block->page_size = qemu_fd_getpagesize(fd); >> +#if defined(__x86_64__) || defined(__i386__) >> + block->mr->align = block->page_size; >> +#else >> block->mr->align = MAX(block->page_size, QEMU_VMALLOC_ALIGN); >maybe it's worth to do other way around i.e. for special s390 case >do above and for the rest leave page_size alignment? > > >> +#endif >> >> if (memory < block->page_size) { >> error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to " >> >> >> Thanks, >> Haozhong >> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] Regression caused by d2f39ad "exec.c: Ensure right alignment also for file backed ram" 2016-10-24 11:44 ` Igor Mammedov 2016-10-24 12:06 ` Haozhong Zhang @ 2016-10-24 12:33 ` Dominik Dingel 2016-10-24 12:57 ` Paolo Bonzini 1 sibling, 1 reply; 8+ messages in thread From: Dominik Dingel @ 2016-10-24 12:33 UTC (permalink / raw) To: Igor Mammedov Cc: Haozhong Zhang, qemu-devel, chao.p.peng, Eduardo Habkost, Peter Crosthwaite, anthony.xu, Paolo Bonzini, Richard Henderson > Am 24.10.2016 um 13:44 schrieb Igor Mammedov <imammedo@redhat.com>: > >> >> modified exec.c >> @@ -1254,7 +1254,11 @@ static void *file_ram_alloc(RAMBlock *block, >> } >> >> block->page_size = qemu_fd_getpagesize(fd); >> +#if defined(__x86_64__) || defined(__i386__) >> + block->mr->align = block->page_size; >> +#else >> block->mr->align = MAX(block->page_size, QEMU_VMALLOC_ALIGN); > maybe it's worth to do other way around i.e. for special s390 case > do above and for the rest leave page_size alignment? > That should work, now the question would be why on x86 file backed memory allocations differ to anonymous allocations? On s390 all allocations used for KVM should be aligned on a segment basis. > SPAPR supports mem hotplug so it's also affected As power does not specialize QEMU_VMALLOC_ALIGN it shouldn’t be affected. Thanks, Dominik ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] Regression caused by d2f39ad "exec.c: Ensure right alignment also for file backed ram" 2016-10-24 12:33 ` Dominik Dingel @ 2016-10-24 12:57 ` Paolo Bonzini 0 siblings, 0 replies; 8+ messages in thread From: Paolo Bonzini @ 2016-10-24 12:57 UTC (permalink / raw) To: Dominik Dingel, Igor Mammedov Cc: Haozhong Zhang, qemu-devel, chao.p.peng, Eduardo Habkost, Peter Crosthwaite, anthony.xu, Richard Henderson On 24/10/2016 14:33, Dominik Dingel wrote: > That should work, now the question would be why on x86 file backed memory > allocations differ to anonymous allocations? > > On s390 all allocations used for KVM should be aligned on a segment basis. On x86, none of the allocations used for KVM need to be aligned. Reporting an error for hugetlbfs is just a convenience to ensure that people don't e.g. use a gigabyte-page hugetlbfs mount for small guests. Paolo ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-10-24 12:57 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-10-21 8:28 [Qemu-devel] Regression caused by d2f39ad "exec.c: Ensure right alignment also for file backed ram" Haozhong Zhang 2016-10-21 8:39 ` Haozhong Zhang 2016-10-21 12:44 ` Igor Mammedov 2016-10-21 14:12 ` Igor Mammedov 2016-10-24 11:44 ` Igor Mammedov 2016-10-24 12:06 ` Haozhong Zhang 2016-10-24 12:33 ` Dominik Dingel 2016-10-24 12:57 ` Paolo Bonzini
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.