diff mbox

exec.c: workaround regression caused by alignment change in d2f39ad

Message ID 20161024124937.17239-1-haozhong.zhang@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Haozhong Zhang Oct. 24, 2016, 12:49 p.m. UTC
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 <haozhong.zhang@intel.com>
Reported-by: "Xu, Anthony" <anthony.xu@intel.com>
---
 exec.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Paolo Bonzini Oct. 24, 2016, 12:58 p.m. UTC | #1
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 <haozhong.zhang@intel.com>
> Reported-by: "Xu, Anthony" <anthony.xu@intel.com>
> ---
>  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 "
>
Christian Borntraeger Oct. 26, 2016, 10:31 a.m. UTC | #2
On 10/24/2016 02:58 PM, Paolo Bonzini wrote:
> 
> 
> 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 <haozhong.zhang@intel.com>
>> Reported-by: "Xu, Anthony" <anthony.xu@intel.com>
>> ---
>>  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.

Thanks.
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>

> 
> Paolo
> 
>>  
>>      if (memory < block->page_size) {
>>          error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to "
>>
> 
>
diff mbox

Patch

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
 
     if (memory < block->page_size) {
         error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to "