Re: [PATCH v1 2/2] exec: Get rid of phys_mem_set_alloc()

From: Igor Mammedov <imammedo@redhat.com>
To: David Hildenbrand <david@redhat.com>
Cc: qemu-devel@nongnu.org, Thomas Huth <thuth@redhat.com>,
	kvm@vger.kernel.org, Cornelia Huck <cohuck@redhat.com>,
	Peter Xu <peterx@redhat.com>, Halil Pasic <pasic@linux.ibm.com>,
	Christian Borntraeger <borntraeger@de.ibm.com>,
	qemu-s390x@nongnu.org, Paolo Bonzini <pbonzini@redhat.com>,
	Richard Henderson <rth@twiddle.net>
Subject: Re: [PATCH v1 2/2] exec: Get rid of phys_mem_set_alloc()
Date: Wed, 3 Mar 2021 17:18:08 +0100	[thread overview]
Message-ID: <20210303171808.4822e880@MiWiFi-RA69-srv> (raw)
In-Reply-To: <20210303130916.22553-3-david@redhat.com>

On Wed,  3 Mar 2021 14:09:16 +0100
David Hildenbrand <david@redhat.com> wrote:

> As the last user is gone, we can get rid of phys_mem_set_alloc() and
> simplify.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Halil Pasic <pasic@linux.ibm.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Peter Xu <peterx@redhat.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  include/sysemu/kvm.h |  4 ----
>  softmmu/physmem.c    | 36 +++---------------------------------
>  2 files changed, 3 insertions(+), 37 deletions(-)
> 
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index 687c598be9..a1ab1ee12d 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -249,10 +249,6 @@ int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap);
>  int kvm_on_sigbus_vcpu(CPUState *cpu, int code, void *addr);
>  int kvm_on_sigbus(int code, void *addr);
>  
> -/* interface with exec.c */
> -
> -void phys_mem_set_alloc(void *(*alloc)(size_t, uint64_t *align, bool shared));
> -
>  /* internal API */
>  
>  int kvm_ioctl(KVMState *s, int type, ...);
> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
> index 19e0aa9836..141fce79e8 100644
> --- a/softmmu/physmem.c
> +++ b/softmmu/physmem.c
> @@ -1144,19 +1144,6 @@ static int subpage_register(subpage_t *mmio, uint32_t start, uint32_t end,
>                              uint16_t section);
>  static subpage_t *subpage_init(FlatView *fv, hwaddr base);
>  
> -static void *(*phys_mem_alloc)(size_t size, uint64_t *align, bool shared) =
> -                               qemu_anon_ram_alloc;
> -
> -/*
> - * Set a custom physical guest memory alloator.
> - * Accelerators with unusual needs may need this.  Hopefully, we can
> - * get rid of it eventually.
> - */
> -void phys_mem_set_alloc(void *(*alloc)(size_t, uint64_t *align, bool shared))
> -{
> -    phys_mem_alloc = alloc;
> -}
> -
>  static uint16_t phys_section_add(PhysPageMap *map,
>                                   MemoryRegionSection *section)
>  {
> @@ -1962,8 +1949,9 @@ static void ram_block_add(RAMBlock *new_block, Error **errp, bool shared)
>                  return;
>              }
>          } else {
> -            new_block->host = phys_mem_alloc(new_block->max_length,
> -                                             &new_block->mr->align, shared);
> +            new_block->host = qemu_anon_ram_alloc(new_block->max_length,
> +                                                  &new_block->mr->align,
> +                                                  shared);
>              if (!new_block->host) {
>                  error_setg_errno(errp, errno,
>                                   "cannot set up guest memory '%s'",
> @@ -2047,17 +2035,6 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr,
>          return NULL;
>      }
>  
> -    if (phys_mem_alloc != qemu_anon_ram_alloc) {
> -        /*
> -         * file_ram_alloc() needs to allocate just like
> -         * phys_mem_alloc, but we haven't bothered to provide
> -         * a hook there.
> -         */
> -        error_setg(errp,
> -                   "-mem-path not supported with this accelerator");
> -        return NULL;
> -    }
> -
>      size = HOST_PAGE_ALIGN(size);
>      file_size = get_file_size(fd);
>      if (file_size > 0 && file_size < size) {
> @@ -2247,13 +2224,6 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length)
>                      area = mmap(vaddr, length, PROT_READ | PROT_WRITE,
>                                  flags, block->fd, offset);
>                  } else {
> -                    /*
> -                     * Remap needs to match alloc.  Accelerators that
> -                     * set phys_mem_alloc never remap.  If they did,
> -                     * we'd need a remap hook here.
> -                     */
> -                    assert(phys_mem_alloc == qemu_anon_ram_alloc);
> -
>                      flags |= MAP_PRIVATE | MAP_ANONYMOUS;
>                      area = mmap(vaddr, length, PROT_READ | PROT_WRITE,
>                                  flags, -1, 0);