Re: [Qemu-ppc] [PATCH v2 2/2] exec: Introduce qemu_getmaxrampagesize() and rename qemu_getrampagesize()

[David Gibson]

On Thu, Mar 28, 2019 at 12:34:58PM +0100, David Hildenbrand wrote:
> Rename qemu_getrampagesize() to qemu_getminrampagesize(). While at it,
> properly rename find_min_supported_pagesize() to find_max_pagesize().
> 
> s390x is actually interrested into the maximum ram pagesize, so

s/interrested/interested/

> introduce and use qemu_getmaxrampagesize().
> 
> Signed-off-by: David Hildenbrand <address@hidden>

Looks good in concept to me.  Comments below are just minor suggested
polish.

> ---
>  exec.c                     | 39 ++++++++++++++++++++++++++++++++++----
>  hw/ppc/spapr_caps.c        |  4 ++--
>  hw/s390x/s390-virtio-ccw.c |  2 +-
>  hw/vfio/spapr.c            |  2 +-
>  target/ppc/kvm.c           |  2 +-
>  5 files changed, 40 insertions(+), 9 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index 6ab62f4eee..cf74f5f284 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1687,7 +1687,7 @@ void ram_block_dump(Monitor *mon)
>   * when we actually open and map them.  Iterate over the file
>   * descriptors instead, and use qemu_fd_getpagesize().
>   */
> -static int find_max_supported_pagesize(Object *obj, void *opaque)
> +static int find_min_pagesize(Object *obj, void *opaque)

Maybe find_min_host_pagesize() to clarify exactly what we're finding.

>  {
>      long *hpsize_min = opaque;
>  
> @@ -1703,7 +1703,23 @@ static int find_max_supported_pagesize(Object *obj, 
> void *opaque)
>      return 0;
>  }
>  
> -long qemu_getrampagesize(void)
> +static int find_max_pagesize(Object *obj, void *opaque)
> +{
> +    long *hpsize_max = opaque;
> +
> +    if (object_dynamic_cast(obj, TYPE_MEMORY_BACKEND)) {
> +        HostMemoryBackend *backend = MEMORY_BACKEND(obj);
> +        long hpsize = host_memory_backend_pagesize(backend);
> +
> +        if (host_memory_backend_is_mapped(backend) && (hpsize > 
> *hpsize_max)) {
> +            *hpsize_max = hpsize;
> +        }
> +    }
> +
> +    return 0;
> +}
> +
> +long qemu_getminrampagesize(void)

s/getminrampagesize/minrampagesize/ for brevity.

The "get" doesn't really add anything useful.

>  {
>      long hpsize = LONG_MAX;
>      long mainrampagesize;
> @@ -1723,7 +1739,7 @@ long qemu_getrampagesize(void)
>       */
>      memdev_root = object_resolve_path("/objects", NULL);
>      if (memdev_root) {
> -        object_child_foreach(memdev_root, find_max_supported_pagesize, 
> &hpsize);
> +        object_child_foreach(memdev_root, find_min_pagesize, &hpsize);
>      }
>      if (hpsize == LONG_MAX) {
>          /* No additional memory regions found ==> Report main RAM page size 
> */
> @@ -1746,8 +1762,23 @@ long qemu_getrampagesize(void)
>  
>      return hpsize;
>  }
> +
> +long qemu_getmaxrampagesize(void)

Same here.

> +{
> +    long pagesize = qemu_mempath_getpagesize(mem_path);
> +    Object *memdev_root = object_resolve_path("/objects", NULL);
> +
> +    if (memdev_root) {
> +        object_child_foreach(memdev_root, find_max_pagesize, &pagesize);
> +    }
> +    return pagesize;
> +}
>  #else
> -long qemu_getrampagesize(void)
> +long qemu_getminrampagesize(void)
> +{
> +    return getpagesize();
> +}
> +long qemu_getmaxrampagesize(void)
>  {
>      return getpagesize();
>  }
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index edc5ed0e0c..3177dc2390 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -347,7 +347,7 @@ static void cap_hpt_maxpagesize_apply(SpaprMachineState 
> *spapr,
>          warn_report("Many guests require at least 64kiB hpt-max-page-size");
>      }
>  
> -    spapr_check_pagesize(spapr, qemu_getrampagesize(), errp);
> +    spapr_check_pagesize(spapr, qemu_getminrampagesize(), errp);
>  }
>  
>  static bool spapr_pagesize_cb(void *opaque, uint32_t seg_pshift,
> @@ -609,7 +609,7 @@ static SpaprCapabilities 
> default_caps_with_cpu(SpaprMachineState *spapr,
>          uint8_t mps;
>  
>          if (kvmppc_hpt_needs_host_contiguous_pages()) {
> -            mps = ctz64(qemu_getrampagesize());
> +            mps = ctz64(qemu_getminrampagesize());
>          } else {
>              mps = 34; /* allow everything up to 16GiB, i.e. everything */
>          }
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 3be5679657..143ac974ca 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -188,7 +188,7 @@ static void s390_memory_init(ram_addr_t mem_size)
>       * Configure the maximum page size. As no memory devices were created
>       * yet, this is the page size of initial memory only.
>       */
> -    s390_set_max_pagesize(qemu_getrampagesize(), &local_err);
> +    s390_set_max_pagesize(qemu_getmaxrampagesize(), &local_err);
>      if (local_err) {
>          error_report_err(local_err);
>          exit(EXIT_FAILURE);
> diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
> index 57fe758e54..30d409a46f 100644
> --- a/hw/vfio/spapr.c
> +++ b/hw/vfio/spapr.c
> @@ -148,7 +148,7 @@ int vfio_spapr_create_window(VFIOContainer *container,
>      uint64_t pagesize = memory_region_iommu_get_min_page_size(iommu_mr);
>      unsigned entries, bits_total, bits_per_level, max_levels;
>      struct vfio_iommu_spapr_tce_create create = { .argsz = sizeof(create) };
> -    long rampagesize = qemu_getrampagesize();
> +    long rampagesize = qemu_getminrampagesize();
>  
>      /*
>       * The host might not support the guest supported IOMMU page size,
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 2427c8ee13..90f65240d0 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -2136,7 +2136,7 @@ uint64_t kvmppc_rma_size(uint64_t current_size, 
> unsigned int hash_shift)
>      /* Find the largest hardware supported page size that's less than
>       * or equal to the (logical) backing page size of guest RAM */
>      kvm_get_smmu_info(&info, &error_fatal);
> -    rampagesize = qemu_getrampagesize();
> +    rampagesize = qemu_getminrampagesize();
>      best_page_shift = 0;
>  
>      for (i = 0; i < KVM_PPC_PAGE_SIZES_MAX_SZ; i++) {

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature