[PATCH v1 0/2] Get rid of legacy_s390_alloc() and phys_mem_set_alloc()

[PATCH v1 0/2] Get rid of legacy_s390_alloc() and phys_mem_set_alloc()

[David Hildenbrand]

Let's finally get rid of the alternative allocation function. Outcome of
a discussion in:
    https://lkml.kernel.org/r/20210303123517.04729c1e.cohuck@redhat.com

David Hildenbrand (2):
  s390x/kvm: Get rid of legacy_s390_alloc()
  exec: Get rid of phys_mem_set_alloc()

 include/sysemu/kvm.h |  4 ----
 softmmu/physmem.c    | 36 +++---------------------------------
 target/s390x/kvm.c   | 43 +++++--------------------------------------
 3 files changed, 8 insertions(+), 75 deletions(-)

-- 
2.29.2

[PATCH v1 0/2] Get rid of legacy_s390_alloc() and phys_mem_set_alloc()

[David Hildenbrand]

Let's finally get rid of the alternative allocation function. Outcome of
a discussion in:
    https://lkml.kernel.org/r/20210303123517.04729c1e.cohuck@redhat.com

David Hildenbrand (2):
  s390x/kvm: Get rid of legacy_s390_alloc()
  exec: Get rid of phys_mem_set_alloc()

 include/sysemu/kvm.h |  4 ----
 softmmu/physmem.c    | 36 +++---------------------------------
 target/s390x/kvm.c   | 43 +++++--------------------------------------
 3 files changed, 8 insertions(+), 75 deletions(-)

-- 
2.29.2

[PATCH v1 1/2] s390x/kvm: Get rid of legacy_s390_alloc()

[David Hildenbrand]

legacy_s390_alloc() was required for dealing with the absence of the ESOP
feature -- on old HW (< gen 10) and old z/VM versions (< 6.3).

As z/VM v6.2 (and even v6.3) is no longer supported since 2017 [1]
and we don't expect to have real users on such old hardware, let's drop
legacy_s390_alloc().

Still check+report an error just in case someone still runs on
such old z/VM environments, or someone runs under weird nested KVM
setups (where we can manually disable ESOP via the CPU model).

No need to check for KVM_CAP_GMAP - that should always be around on
kernels that also have KVM_CAP_DEVICE_CTRL (>= v3.15).

[1] https://www.ibm.com/support/lifecycle/search?q=z%2FVM

Suggested-by: Cornelia Huck <cohuck@redhat.com>
Suggested-by: Thomas Huth <thuth@redhat.com>
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>
---
 target/s390x/kvm.c | 43 +++++--------------------------------------
 1 file changed, 5 insertions(+), 38 deletions(-)

diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 7a892d663d..84b40572f2 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -161,8 +161,6 @@ static int cap_protected;
 
 static int active_cmma;
 
-static void *legacy_s390_alloc(size_t size, uint64_t *align, bool shared);
-
 static int kvm_s390_query_mem_limit(uint64_t *memory_limit)
 {
     struct kvm_device_attr attr = {
@@ -349,6 +347,11 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
                      "please use kernel 3.15 or newer");
         return -1;
     }
+    if (!kvm_check_extension(s, KVM_CAP_S390_COW)) {
+        error_report("KVM is missing capability KVM_CAP_S390_COW - "
+                     "unsupported environment");
+        return -1;
+    }
 
     cap_sync_regs = kvm_check_extension(s, KVM_CAP_SYNC_REGS);
     cap_async_pf = kvm_check_extension(s, KVM_CAP_ASYNC_PF);
@@ -357,11 +360,6 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
     cap_vcpu_resets = kvm_check_extension(s, KVM_CAP_S390_VCPU_RESETS);
     cap_protected = kvm_check_extension(s, KVM_CAP_S390_PROTECTED);
 
-    if (!kvm_check_extension(s, KVM_CAP_S390_GMAP)
-        || !kvm_check_extension(s, KVM_CAP_S390_COW)) {
-        phys_mem_set_alloc(legacy_s390_alloc);
-    }
-
     kvm_vm_enable_cap(s, KVM_CAP_S390_USER_SIGP, 0);
     kvm_vm_enable_cap(s, KVM_CAP_S390_VECTOR_REGISTERS, 0);
     kvm_vm_enable_cap(s, KVM_CAP_S390_USER_STSI, 0);
@@ -889,37 +887,6 @@ int kvm_s390_mem_op_pv(S390CPU *cpu, uint64_t offset, void *hostbuf,
     return ret;
 }
 
-/*
- * Legacy layout for s390:
- * Older S390 KVM requires the topmost vma of the RAM to be
- * smaller than an system defined value, which is at least 256GB.
- * Larger systems have larger values. We put the guest between
- * the end of data segment (system break) and this value. We
- * use 32GB as a base to have enough room for the system break
- * to grow. We also have to use MAP parameters that avoid
- * read-only mapping of guest pages.
- */
-static void *legacy_s390_alloc(size_t size, uint64_t *align, bool shared)
-{
-    static void *mem;
-
-    if (mem) {
-        /* we only support one allocation, which is enough for initial ram */
-        return NULL;
-    }
-
-    mem = mmap((void *) 0x800000000ULL, size,
-               PROT_EXEC|PROT_READ|PROT_WRITE,
-               MAP_SHARED | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
-    if (mem == MAP_FAILED) {
-        mem = NULL;
-    }
-    if (mem && align) {
-        *align = QEMU_VMALLOC_ALIGN;
-    }
-    return mem;
-}
-
 static uint8_t const *sw_bp_inst;
 static uint8_t sw_bp_ilen;
 
-- 
2.29.2

[PATCH v1 1/2] s390x/kvm: Get rid of legacy_s390_alloc()

[David Hildenbrand]

legacy_s390_alloc() was required for dealing with the absence of the ESOP
feature -- on old HW (< gen 10) and old z/VM versions (< 6.3).

As z/VM v6.2 (and even v6.3) is no longer supported since 2017 [1]
and we don't expect to have real users on such old hardware, let's drop
legacy_s390_alloc().

Still check+report an error just in case someone still runs on
such old z/VM environments, or someone runs under weird nested KVM
setups (where we can manually disable ESOP via the CPU model).

No need to check for KVM_CAP_GMAP - that should always be around on
kernels that also have KVM_CAP_DEVICE_CTRL (>= v3.15).

[1] https://www.ibm.com/support/lifecycle/search?q=z%2FVM

Suggested-by: Cornelia Huck <cohuck@redhat.com>
Suggested-by: Thomas Huth <thuth@redhat.com>
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>
---
 target/s390x/kvm.c | 43 +++++--------------------------------------
 1 file changed, 5 insertions(+), 38 deletions(-)

diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 7a892d663d..84b40572f2 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -161,8 +161,6 @@ static int cap_protected;
 
 static int active_cmma;
 
-static void *legacy_s390_alloc(size_t size, uint64_t *align, bool shared);
-
 static int kvm_s390_query_mem_limit(uint64_t *memory_limit)
 {
     struct kvm_device_attr attr = {
@@ -349,6 +347,11 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
                      "please use kernel 3.15 or newer");
         return -1;
     }
+    if (!kvm_check_extension(s, KVM_CAP_S390_COW)) {
+        error_report("KVM is missing capability KVM_CAP_S390_COW - "
+                     "unsupported environment");
+        return -1;
+    }
 
     cap_sync_regs = kvm_check_extension(s, KVM_CAP_SYNC_REGS);
     cap_async_pf = kvm_check_extension(s, KVM_CAP_ASYNC_PF);
@@ -357,11 +360,6 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
     cap_vcpu_resets = kvm_check_extension(s, KVM_CAP_S390_VCPU_RESETS);
     cap_protected = kvm_check_extension(s, KVM_CAP_S390_PROTECTED);
 
-    if (!kvm_check_extension(s, KVM_CAP_S390_GMAP)
-        || !kvm_check_extension(s, KVM_CAP_S390_COW)) {
-        phys_mem_set_alloc(legacy_s390_alloc);
-    }
-
     kvm_vm_enable_cap(s, KVM_CAP_S390_USER_SIGP, 0);
     kvm_vm_enable_cap(s, KVM_CAP_S390_VECTOR_REGISTERS, 0);
     kvm_vm_enable_cap(s, KVM_CAP_S390_USER_STSI, 0);
@@ -889,37 +887,6 @@ int kvm_s390_mem_op_pv(S390CPU *cpu, uint64_t offset, void *hostbuf,
     return ret;
 }
 
-/*
- * Legacy layout for s390:
- * Older S390 KVM requires the topmost vma of the RAM to be
- * smaller than an system defined value, which is at least 256GB.
- * Larger systems have larger values. We put the guest between
- * the end of data segment (system break) and this value. We
- * use 32GB as a base to have enough room for the system break
- * to grow. We also have to use MAP parameters that avoid
- * read-only mapping of guest pages.
- */
-static void *legacy_s390_alloc(size_t size, uint64_t *align, bool shared)
-{
-    static void *mem;
-
-    if (mem) {
-        /* we only support one allocation, which is enough for initial ram */
-        return NULL;
-    }
-
-    mem = mmap((void *) 0x800000000ULL, size,
-               PROT_EXEC|PROT_READ|PROT_WRITE,
-               MAP_SHARED | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
-    if (mem == MAP_FAILED) {
-        mem = NULL;
-    }
-    if (mem && align) {
-        *align = QEMU_VMALLOC_ALIGN;
-    }
-    return mem;
-}
-
 static uint8_t const *sw_bp_inst;
 static uint8_t sw_bp_ilen;
 
-- 
2.29.2

Re: [PATCH v1 1/2] s390x/kvm: Get rid of legacy_s390_alloc()

[Thomas Huth]

On 03/03/2021 14.09, David Hildenbrand wrote:
> legacy_s390_alloc() was required for dealing with the absence of the ESOP
> feature -- on old HW (< gen 10) and old z/VM versions (< 6.3).
> 
> As z/VM v6.2 (and even v6.3) is no longer supported since 2017 [1]
> and we don't expect to have real users on such old hardware, let's drop
> legacy_s390_alloc().
> 
> Still check+report an error just in case someone still runs on
> such old z/VM environments, or someone runs under weird nested KVM
> setups (where we can manually disable ESOP via the CPU model).
> 
> No need to check for KVM_CAP_GMAP - that should always be around on
> kernels that also have KVM_CAP_DEVICE_CTRL (>= v3.15).

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v1 1/2] s390x/kvm: Get rid of legacy_s390_alloc()

[Thomas Huth]

On 03/03/2021 14.09, David Hildenbrand wrote:
> legacy_s390_alloc() was required for dealing with the absence of the ESOP
> feature -- on old HW (< gen 10) and old z/VM versions (< 6.3).
> 
> As z/VM v6.2 (and even v6.3) is no longer supported since 2017 [1]
> and we don't expect to have real users on such old hardware, let's drop
> legacy_s390_alloc().
> 
> Still check+report an error just in case someone still runs on
> such old z/VM environments, or someone runs under weird nested KVM
> setups (where we can manually disable ESOP via the CPU model).
> 
> No need to check for KVM_CAP_GMAP - that should always be around on
> kernels that also have KVM_CAP_DEVICE_CTRL (>= v3.15).

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v1 1/2] s390x/kvm: Get rid of legacy_s390_alloc()

[Christian Borntraeger]

On 03.03.21 14:09, David Hildenbrand wrote:
> legacy_s390_alloc() was required for dealing with the absence of the ESOP
> feature -- on old HW (< gen 10) and old z/VM versions (< 6.3).
> 
> As z/VM v6.2 (and even v6.3) is no longer supported since 2017 [1]
> and we don't expect to have real users on such old hardware, let's drop
> legacy_s390_alloc().
> 
> Still check+report an error just in case someone still runs on
> such old z/VM environments, or someone runs under weird nested KVM
> setups (where we can manually disable ESOP via the CPU model).
> 
> No need to check for KVM_CAP_GMAP - that should always be around on
> kernels that also have KVM_CAP_DEVICE_CTRL (>= v3.15).
> 
> [1] https://www.ibm.com/support/lifecycle/search?q=z%2FVM
> 
> Suggested-by: Cornelia Huck <cohuck@redhat.com>
> Suggested-by: Thomas Huth <thuth@redhat.com>
> 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>

I agree, this should now be a corner case that we do not necessarily have to care about.

> ---
>   target/s390x/kvm.c | 43 +++++--------------------------------------
>   1 file changed, 5 insertions(+), 38 deletions(-)
> 
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 7a892d663d..84b40572f2 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -161,8 +161,6 @@ static int cap_protected;
>   
>   static int active_cmma;
>   
> -static void *legacy_s390_alloc(size_t size, uint64_t *align, bool shared);
> -
>   static int kvm_s390_query_mem_limit(uint64_t *memory_limit)
>   {
>       struct kvm_device_attr attr = {
> @@ -349,6 +347,11 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>                        "please use kernel 3.15 or newer");
>           return -1;
>       }
> +    if (!kvm_check_extension(s, KVM_CAP_S390_COW)) {
> +        error_report("KVM is missing capability KVM_CAP_S390_COW - "
> +                     "unsupported environment");
> +        return -1;
> +    }
>   
>       cap_sync_regs = kvm_check_extension(s, KVM_CAP_SYNC_REGS);
>       cap_async_pf = kvm_check_extension(s, KVM_CAP_ASYNC_PF);
> @@ -357,11 +360,6 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>       cap_vcpu_resets = kvm_check_extension(s, KVM_CAP_S390_VCPU_RESETS);
>       cap_protected = kvm_check_extension(s, KVM_CAP_S390_PROTECTED);
>   
> -    if (!kvm_check_extension(s, KVM_CAP_S390_GMAP)
> -        || !kvm_check_extension(s, KVM_CAP_S390_COW)) {
> -        phys_mem_set_alloc(legacy_s390_alloc);
> -    }
> -
>       kvm_vm_enable_cap(s, KVM_CAP_S390_USER_SIGP, 0);
>       kvm_vm_enable_cap(s, KVM_CAP_S390_VECTOR_REGISTERS, 0);
>       kvm_vm_enable_cap(s, KVM_CAP_S390_USER_STSI, 0);
> @@ -889,37 +887,6 @@ int kvm_s390_mem_op_pv(S390CPU *cpu, uint64_t offset, void *hostbuf,
>       return ret;
>   }
>   
> -/*
> - * Legacy layout for s390:
> - * Older S390 KVM requires the topmost vma of the RAM to be
> - * smaller than an system defined value, which is at least 256GB.
> - * Larger systems have larger values. We put the guest between
> - * the end of data segment (system break) and this value. We
> - * use 32GB as a base to have enough room for the system break
> - * to grow. We also have to use MAP parameters that avoid
> - * read-only mapping of guest pages.
> - */
> -static void *legacy_s390_alloc(size_t size, uint64_t *align, bool shared)
> -{
> -    static void *mem;
> -
> -    if (mem) {
> -        /* we only support one allocation, which is enough for initial ram */
> -        return NULL;
> -    }
> -
> -    mem = mmap((void *) 0x800000000ULL, size,
> -               PROT_EXEC|PROT_READ|PROT_WRITE,
> -               MAP_SHARED | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
> -    if (mem == MAP_FAILED) {
> -        mem = NULL;
> -    }
> -    if (mem && align) {
> -        *align = QEMU_VMALLOC_ALIGN;
> -    }
> -    return mem;
> -}
> -
>   static uint8_t const *sw_bp_inst;
>   static uint8_t sw_bp_ilen;
>   
> 

Re: [PATCH v1 1/2] s390x/kvm: Get rid of legacy_s390_alloc()

[Christian Borntraeger]

On 03.03.21 14:09, David Hildenbrand wrote:
> legacy_s390_alloc() was required for dealing with the absence of the ESOP
> feature -- on old HW (< gen 10) and old z/VM versions (< 6.3).
> 
> As z/VM v6.2 (and even v6.3) is no longer supported since 2017 [1]
> and we don't expect to have real users on such old hardware, let's drop
> legacy_s390_alloc().
> 
> Still check+report an error just in case someone still runs on
> such old z/VM environments, or someone runs under weird nested KVM
> setups (where we can manually disable ESOP via the CPU model).
> 
> No need to check for KVM_CAP_GMAP - that should always be around on
> kernels that also have KVM_CAP_DEVICE_CTRL (>= v3.15).
> 
> [1] https://www.ibm.com/support/lifecycle/search?q=z%2FVM
> 
> Suggested-by: Cornelia Huck <cohuck@redhat.com>
> Suggested-by: Thomas Huth <thuth@redhat.com>
> 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>

I agree, this should now be a corner case that we do not necessarily have to care about.

> ---
>   target/s390x/kvm.c | 43 +++++--------------------------------------
>   1 file changed, 5 insertions(+), 38 deletions(-)
> 
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 7a892d663d..84b40572f2 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -161,8 +161,6 @@ static int cap_protected;
>   
>   static int active_cmma;
>   
> -static void *legacy_s390_alloc(size_t size, uint64_t *align, bool shared);
> -
>   static int kvm_s390_query_mem_limit(uint64_t *memory_limit)
>   {
>       struct kvm_device_attr attr = {
> @@ -349,6 +347,11 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>                        "please use kernel 3.15 or newer");
>           return -1;
>       }
> +    if (!kvm_check_extension(s, KVM_CAP_S390_COW)) {
> +        error_report("KVM is missing capability KVM_CAP_S390_COW - "
> +                     "unsupported environment");
> +        return -1;
> +    }
>   
>       cap_sync_regs = kvm_check_extension(s, KVM_CAP_SYNC_REGS);
>       cap_async_pf = kvm_check_extension(s, KVM_CAP_ASYNC_PF);
> @@ -357,11 +360,6 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>       cap_vcpu_resets = kvm_check_extension(s, KVM_CAP_S390_VCPU_RESETS);
>       cap_protected = kvm_check_extension(s, KVM_CAP_S390_PROTECTED);
>   
> -    if (!kvm_check_extension(s, KVM_CAP_S390_GMAP)
> -        || !kvm_check_extension(s, KVM_CAP_S390_COW)) {
> -        phys_mem_set_alloc(legacy_s390_alloc);
> -    }
> -
>       kvm_vm_enable_cap(s, KVM_CAP_S390_USER_SIGP, 0);
>       kvm_vm_enable_cap(s, KVM_CAP_S390_VECTOR_REGISTERS, 0);
>       kvm_vm_enable_cap(s, KVM_CAP_S390_USER_STSI, 0);
> @@ -889,37 +887,6 @@ int kvm_s390_mem_op_pv(S390CPU *cpu, uint64_t offset, void *hostbuf,
>       return ret;
>   }
>   
> -/*
> - * Legacy layout for s390:
> - * Older S390 KVM requires the topmost vma of the RAM to be
> - * smaller than an system defined value, which is at least 256GB.
> - * Larger systems have larger values. We put the guest between
> - * the end of data segment (system break) and this value. We
> - * use 32GB as a base to have enough room for the system break
> - * to grow. We also have to use MAP parameters that avoid
> - * read-only mapping of guest pages.
> - */
> -static void *legacy_s390_alloc(size_t size, uint64_t *align, bool shared)
> -{
> -    static void *mem;
> -
> -    if (mem) {
> -        /* we only support one allocation, which is enough for initial ram */
> -        return NULL;
> -    }
> -
> -    mem = mmap((void *) 0x800000000ULL, size,
> -               PROT_EXEC|PROT_READ|PROT_WRITE,
> -               MAP_SHARED | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
> -    if (mem == MAP_FAILED) {
> -        mem = NULL;
> -    }
> -    if (mem && align) {
> -        *align = QEMU_VMALLOC_ALIGN;
> -    }
> -    return mem;
> -}
> -
>   static uint8_t const *sw_bp_inst;
>   static uint8_t sw_bp_ilen;
>   
> 

Re: [PATCH v1 1/2] s390x/kvm: Get rid of legacy_s390_alloc()

[Igor Mammedov]

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

> legacy_s390_alloc() was required for dealing with the absence of the ESOP
> feature -- on old HW (< gen 10) and old z/VM versions (< 6.3).
> 
> As z/VM v6.2 (and even v6.3) is no longer supported since 2017 [1]
> and we don't expect to have real users on such old hardware, let's drop
> legacy_s390_alloc().
> 
> Still check+report an error just in case someone still runs on
> such old z/VM environments, or someone runs under weird nested KVM
> setups (where we can manually disable ESOP via the CPU model).
> 
> No need to check for KVM_CAP_GMAP - that should always be around on
> kernels that also have KVM_CAP_DEVICE_CTRL (>= v3.15).
> 
> [1] https://www.ibm.com/support/lifecycle/search?q=z%2FVM
> 
> Suggested-by: Cornelia Huck <cohuck@redhat.com>
> Suggested-by: Thomas Huth <thuth@redhat.com>
> 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>

> ---
>  target/s390x/kvm.c | 43 +++++--------------------------------------
>  1 file changed, 5 insertions(+), 38 deletions(-)
> 
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 7a892d663d..84b40572f2 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -161,8 +161,6 @@ static int cap_protected;
>  
>  static int active_cmma;
>  
> -static void *legacy_s390_alloc(size_t size, uint64_t *align, bool shared);
> -
>  static int kvm_s390_query_mem_limit(uint64_t *memory_limit)
>  {
>      struct kvm_device_attr attr = {
> @@ -349,6 +347,11 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>                       "please use kernel 3.15 or newer");
>          return -1;
>      }
> +    if (!kvm_check_extension(s, KVM_CAP_S390_COW)) {
> +        error_report("KVM is missing capability KVM_CAP_S390_COW - "
> +                     "unsupported environment");
> +        return -1;
> +    }
>  
>      cap_sync_regs = kvm_check_extension(s, KVM_CAP_SYNC_REGS);
>      cap_async_pf = kvm_check_extension(s, KVM_CAP_ASYNC_PF);
> @@ -357,11 +360,6 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>      cap_vcpu_resets = kvm_check_extension(s, KVM_CAP_S390_VCPU_RESETS);
>      cap_protected = kvm_check_extension(s, KVM_CAP_S390_PROTECTED);
>  
> -    if (!kvm_check_extension(s, KVM_CAP_S390_GMAP)
> -        || !kvm_check_extension(s, KVM_CAP_S390_COW)) {
> -        phys_mem_set_alloc(legacy_s390_alloc);
> -    }
> -
>      kvm_vm_enable_cap(s, KVM_CAP_S390_USER_SIGP, 0);
>      kvm_vm_enable_cap(s, KVM_CAP_S390_VECTOR_REGISTERS, 0);
>      kvm_vm_enable_cap(s, KVM_CAP_S390_USER_STSI, 0);
> @@ -889,37 +887,6 @@ int kvm_s390_mem_op_pv(S390CPU *cpu, uint64_t offset, void *hostbuf,
>      return ret;
>  }
>  
> -/*
> - * Legacy layout for s390:
> - * Older S390 KVM requires the topmost vma of the RAM to be
> - * smaller than an system defined value, which is at least 256GB.
> - * Larger systems have larger values. We put the guest between
> - * the end of data segment (system break) and this value. We
> - * use 32GB as a base to have enough room for the system break
> - * to grow. We also have to use MAP parameters that avoid
> - * read-only mapping of guest pages.
> - */
> -static void *legacy_s390_alloc(size_t size, uint64_t *align, bool shared)
> -{
> -    static void *mem;
> -
> -    if (mem) {
> -        /* we only support one allocation, which is enough for initial ram */
> -        return NULL;
> -    }
> -
> -    mem = mmap((void *) 0x800000000ULL, size,
> -               PROT_EXEC|PROT_READ|PROT_WRITE,
> -               MAP_SHARED | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
> -    if (mem == MAP_FAILED) {
> -        mem = NULL;
> -    }
> -    if (mem && align) {
> -        *align = QEMU_VMALLOC_ALIGN;
> -    }
> -    return mem;
> -}
> -
>  static uint8_t const *sw_bp_inst;
>  static uint8_t sw_bp_ilen;
>  

Re: [PATCH v1 1/2] s390x/kvm: Get rid of legacy_s390_alloc()

[Igor Mammedov]

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

> legacy_s390_alloc() was required for dealing with the absence of the ESOP
> feature -- on old HW (< gen 10) and old z/VM versions (< 6.3).
> 
> As z/VM v6.2 (and even v6.3) is no longer supported since 2017 [1]
> and we don't expect to have real users on such old hardware, let's drop
> legacy_s390_alloc().
> 
> Still check+report an error just in case someone still runs on
> such old z/VM environments, or someone runs under weird nested KVM
> setups (where we can manually disable ESOP via the CPU model).
> 
> No need to check for KVM_CAP_GMAP - that should always be around on
> kernels that also have KVM_CAP_DEVICE_CTRL (>= v3.15).
> 
> [1] https://www.ibm.com/support/lifecycle/search?q=z%2FVM
> 
> Suggested-by: Cornelia Huck <cohuck@redhat.com>
> Suggested-by: Thomas Huth <thuth@redhat.com>
> 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>

> ---
>  target/s390x/kvm.c | 43 +++++--------------------------------------
>  1 file changed, 5 insertions(+), 38 deletions(-)
> 
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 7a892d663d..84b40572f2 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -161,8 +161,6 @@ static int cap_protected;
>  
>  static int active_cmma;
>  
> -static void *legacy_s390_alloc(size_t size, uint64_t *align, bool shared);
> -
>  static int kvm_s390_query_mem_limit(uint64_t *memory_limit)
>  {
>      struct kvm_device_attr attr = {
> @@ -349,6 +347,11 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>                       "please use kernel 3.15 or newer");
>          return -1;
>      }
> +    if (!kvm_check_extension(s, KVM_CAP_S390_COW)) {
> +        error_report("KVM is missing capability KVM_CAP_S390_COW - "
> +                     "unsupported environment");
> +        return -1;
> +    }
>  
>      cap_sync_regs = kvm_check_extension(s, KVM_CAP_SYNC_REGS);
>      cap_async_pf = kvm_check_extension(s, KVM_CAP_ASYNC_PF);
> @@ -357,11 +360,6 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>      cap_vcpu_resets = kvm_check_extension(s, KVM_CAP_S390_VCPU_RESETS);
>      cap_protected = kvm_check_extension(s, KVM_CAP_S390_PROTECTED);
>  
> -    if (!kvm_check_extension(s, KVM_CAP_S390_GMAP)
> -        || !kvm_check_extension(s, KVM_CAP_S390_COW)) {
> -        phys_mem_set_alloc(legacy_s390_alloc);
> -    }
> -
>      kvm_vm_enable_cap(s, KVM_CAP_S390_USER_SIGP, 0);
>      kvm_vm_enable_cap(s, KVM_CAP_S390_VECTOR_REGISTERS, 0);
>      kvm_vm_enable_cap(s, KVM_CAP_S390_USER_STSI, 0);
> @@ -889,37 +887,6 @@ int kvm_s390_mem_op_pv(S390CPU *cpu, uint64_t offset, void *hostbuf,
>      return ret;
>  }
>  
> -/*
> - * Legacy layout for s390:
> - * Older S390 KVM requires the topmost vma of the RAM to be
> - * smaller than an system defined value, which is at least 256GB.
> - * Larger systems have larger values. We put the guest between
> - * the end of data segment (system break) and this value. We
> - * use 32GB as a base to have enough room for the system break
> - * to grow. We also have to use MAP parameters that avoid
> - * read-only mapping of guest pages.
> - */
> -static void *legacy_s390_alloc(size_t size, uint64_t *align, bool shared)
> -{
> -    static void *mem;
> -
> -    if (mem) {
> -        /* we only support one allocation, which is enough for initial ram */
> -        return NULL;
> -    }
> -
> -    mem = mmap((void *) 0x800000000ULL, size,
> -               PROT_EXEC|PROT_READ|PROT_WRITE,
> -               MAP_SHARED | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
> -    if (mem == MAP_FAILED) {
> -        mem = NULL;
> -    }
> -    if (mem && align) {
> -        *align = QEMU_VMALLOC_ALIGN;
> -    }
> -    return mem;
> -}
> -
>  static uint8_t const *sw_bp_inst;
>  static uint8_t sw_bp_ilen;
>  

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

[David Hildenbrand]

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>
---
 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);
-- 
2.29.2

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

[David Hildenbrand]

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>
---
 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);
-- 
2.29.2

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

[Thomas Huth]

On 03/03/2021 14.09, David Hildenbrand 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>
> ---
>   include/sysemu/kvm.h |  4 ----
>   softmmu/physmem.c    | 36 +++---------------------------------
>   2 files changed, 3 insertions(+), 37 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

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

[Thomas Huth]

On 03/03/2021 14.09, David Hildenbrand 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>
> ---
>   include/sysemu/kvm.h |  4 ----
>   softmmu/physmem.c    | 36 +++---------------------------------
>   2 files changed, 3 insertions(+), 37 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

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

[Igor Mammedov]

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

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

[Igor Mammedov]

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

Re: [PATCH v1 0/2] Get rid of legacy_s390_alloc() and phys_mem_set_alloc()

[Cornelia Huck]

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

> Let's finally get rid of the alternative allocation function. Outcome of
> a discussion in:
>     https://lkml.kernel.org/r/20210303123517.04729c1e.cohuck@redhat.com
> 
> David Hildenbrand (2):
>   s390x/kvm: Get rid of legacy_s390_alloc()
>   exec: Get rid of phys_mem_set_alloc()
> 
>  include/sysemu/kvm.h |  4 ----
>  softmmu/physmem.c    | 36 +++---------------------------------
>  target/s390x/kvm.c   | 43 +++++--------------------------------------
>  3 files changed, 8 insertions(+), 75 deletions(-)
> 

Ok, who is going to queue this?

I'd be happy to take this through the s390x tree, if I get an ack from
Paolo :)

Re: [PATCH v1 0/2] Get rid of legacy_s390_alloc() and phys_mem_set_alloc()

[Cornelia Huck]

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

> Let's finally get rid of the alternative allocation function. Outcome of
> a discussion in:
>     https://lkml.kernel.org/r/20210303123517.04729c1e.cohuck@redhat.com
> 
> David Hildenbrand (2):
>   s390x/kvm: Get rid of legacy_s390_alloc()
>   exec: Get rid of phys_mem_set_alloc()
> 
>  include/sysemu/kvm.h |  4 ----
>  softmmu/physmem.c    | 36 +++---------------------------------
>  target/s390x/kvm.c   | 43 +++++--------------------------------------
>  3 files changed, 8 insertions(+), 75 deletions(-)
> 

Ok, who is going to queue this?

I'd be happy to take this through the s390x tree, if I get an ack from
Paolo :)

Re: [PATCH v1 0/2] Get rid of legacy_s390_alloc() and phys_mem_set_alloc()

[Cornelia Huck]

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

> Let's finally get rid of the alternative allocation function. Outcome of
> a discussion in:
>     https://lkml.kernel.org/r/20210303123517.04729c1e.cohuck@redhat.com
> 
> David Hildenbrand (2):
>   s390x/kvm: Get rid of legacy_s390_alloc()
>   exec: Get rid of phys_mem_set_alloc()
> 
>  include/sysemu/kvm.h |  4 ----
>  softmmu/physmem.c    | 36 +++---------------------------------
>  target/s390x/kvm.c   | 43 +++++--------------------------------------
>  3 files changed, 8 insertions(+), 75 deletions(-)
> 

Thanks, applied.

Re: [PATCH v1 0/2] Get rid of legacy_s390_alloc() and phys_mem_set_alloc()

[Cornelia Huck]

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

> Let's finally get rid of the alternative allocation function. Outcome of
> a discussion in:
>     https://lkml.kernel.org/r/20210303123517.04729c1e.cohuck@redhat.com
> 
> David Hildenbrand (2):
>   s390x/kvm: Get rid of legacy_s390_alloc()
>   exec: Get rid of phys_mem_set_alloc()
> 
>  include/sysemu/kvm.h |  4 ----
>  softmmu/physmem.c    | 36 +++---------------------------------
>  target/s390x/kvm.c   | 43 +++++--------------------------------------
>  3 files changed, 8 insertions(+), 75 deletions(-)
> 

Thanks, applied.