[Qemu-devel] Regression caused by d2f39ad "exec.c: Ensure right alignment also for file backed ram"

[Qemu-devel] Regression caused by d2f39ad "exec.c: Ensure right alignment also for file backed ram"

[Haozhong Zhang]

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

Re: [Qemu-devel] Regression caused by d2f39ad "exec.c: Ensure right alignment also for file backed ram"

[Haozhong Zhang]

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
>

Re: [Qemu-devel] Regression caused by d2f39ad "exec.c: Ensure right alignment also for file backed ram"

[Igor Mammedov]

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
> >  
> 

Re: [Qemu-devel] Regression caused by d2f39ad "exec.c: Ensure right alignment also for file backed ram"

[Igor Mammedov]

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
> 

Re: [Qemu-devel] Regression caused by d2f39ad "exec.c: Ensure right alignment also for file backed ram"

[Igor Mammedov]

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
> 

Re: [Qemu-devel] Regression caused by d2f39ad "exec.c: Ensure right alignment also for file backed ram"

[Haozhong Zhang]

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
>>
>

Re: [Qemu-devel] Regression caused by d2f39ad "exec.c: Ensure right alignment also for file backed ram"

[Dominik Dingel]

> 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

Re: [Qemu-devel] Regression caused by d2f39ad "exec.c: Ensure right alignment also for file backed ram"

[Paolo Bonzini]

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