Re: [Qemu-devel] [Qemu-ppc] [PATCH for-4.0?] exec: Only count mapped memory backends for qemu_getrampagesize()

[Laurent Vivier]

On 26/03/2019 04:50, David Gibson wrote:
> qemu_getrampagesize() works out the minimum host page size backing any of
> guest RAM.  This is required in a few places, such as for POWER8 PAPR KVM
> guests, because limitations of the hardware virtualization mean the guest
> can't use pagesizes larger than the host pages backing its memory.
> 
> However, it currently checks against *every* memory backend, whether or not
> it is actually mapped into guest memory at the moment.  This is incorrect.
> 
> This can cause a problem attempting to add memory to a POWER8 pseries KVM
> guest which is configured to allow hugepages in the guest (e.g.
> -machine cap-hpt-max-page-size=16m).  If you attempt to add non-hugepage,
> you can (correctly) create a memory backend, however it (correctly) will
> throw an error when you attempt to map that memory into the guest by
> 'device_add'ing a pc-dimm.
> 
> What's not correct is that if you then reset the guest a startup check
> against qemu_getrampagesize() will cause a fatal error because of the new
> memory object, even though it's not mapped into the guest.
> 
> This patch corrects the problem by adjusting find_max_supported_pagesize()
> (called from qemu_getrampagesize() via object_child_foreach) to exclude
> non-mapped memory backends.
> 
> Signed-off-by: David Gibson <address@hidden>
> ---
>  exec.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> This is definitely a bug, but it's not a regression.  I'm not sure if
> this is 4.0 material at this stage of the freeze or not.
> 
> diff --git a/exec.c b/exec.c
> index 86a38d3b3b..6ab62f4eee 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1692,9 +1692,10 @@ static int find_max_supported_pagesize(Object *obj, 
> void *opaque)
>      long *hpsize_min = opaque;
>  
>      if (object_dynamic_cast(obj, TYPE_MEMORY_BACKEND)) {
> -        long hpsize = host_memory_backend_pagesize(MEMORY_BACKEND(obj));
> +        HostMemoryBackend *backend = MEMORY_BACKEND(obj);
> +        long hpsize = host_memory_backend_pagesize(backend);
>  
> -        if (hpsize < *hpsize_min) {
> +        if (host_memory_backend_is_mapped(backend) && (hpsize < 
> *hpsize_min)) {
>              *hpsize_min = hpsize;
>          }
>      }
> 

Reviewed-by: Laurent Vivier <address@hidden>