All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/6] qom: Allow object to be aligned
@ 2020-09-16  0:46 ` Richard Henderson
  0 siblings, 0 replies; 17+ messages in thread
From: Richard Henderson @ 2020-09-16  0:46 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Daniel P. Berrangé,
	Eduardo Habkost, qemu-riscv, David Hildenbrand, qemu-ppc,
	Cornelia Huck, qemu-s390x, qemu-arm, Alistair Francis,
	Paolo Bonzini, David Gibson

I've seen some failures on arm and s390x hosts after
enabling host vector support.  It turns out that the
malloc for these hosts does not provide 16-byte alignment.

We already have a function that can alloc with alignment,
but we need to pass this down from the structure.  We also
don't want to use this function unconditionally, because
the windows version does page allocation, which would be
overkill for the vast majority of the objects allocated.

Changes in v2:
  * Add _aligned_malloc patch for win32.  For what it's
    worth, this passes a gitlab cross-compile test.

  * Add and use qemu_max_align_t for choosing between
    g_malloc and qemu_memalign.

    I had been discussing extra checks for i386-linux with
    Eduardo, but then it occured to me that both linux libc
    posix_memalign is smart enough to not imply extra overhead.
    So qemu_memalign with alignment <= malloc alignment is
    handled easily by the system.


r~


Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: Alistair Francis <Alistair.Francis@wdc.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Cornelia Huck <cohuck@redhat.com>
Cc: qemu-arm@nongnu.org
Cc: qemu-ppc@nongnu.org
Cc: qemu-riscv@nongnu.org
Cc: qemu-s390x@nongnu.org


Richard Henderson (6):
  util/oslib-win32: Use _aligned_malloc for qemu_try_memalign
  qom: Allow objects to be allocated with increased alignment
  target/arm: Set instance_align on CPUARM TypeInfo
  target/ppc: Set instance_align on PowerPCCPU TypeInfo
  target/riscv: Set instance_align on RISCVCPU TypeInfo
  target/s390x: Set instance_align on S390CPU TypeInfo

 include/qom/object.h            |  5 +++++
 qom/object.c                    | 36 ++++++++++++++++++++++++++++++---
 target/arm/cpu.c                |  2 ++
 target/riscv/cpu.c              |  1 +
 target/s390x/cpu.c              |  1 +
 util/oslib-win32.c              | 10 +++------
 target/ppc/translate_init.c.inc |  1 +
 7 files changed, 46 insertions(+), 10 deletions(-)

-- 
2.25.1



^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v2 0/6] qom: Allow object to be aligned
@ 2020-09-16  0:46 ` Richard Henderson
  0 siblings, 0 replies; 17+ messages in thread
From: Richard Henderson @ 2020-09-16  0:46 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Daniel P. Berrangé,
	Eduardo Habkost, Peter Maydell, David Gibson, Alistair Francis,
	David Hildenbrand, Cornelia Huck, qemu-arm, qemu-ppc, qemu-riscv,
	qemu-s390x

I've seen some failures on arm and s390x hosts after
enabling host vector support.  It turns out that the
malloc for these hosts does not provide 16-byte alignment.

We already have a function that can alloc with alignment,
but we need to pass this down from the structure.  We also
don't want to use this function unconditionally, because
the windows version does page allocation, which would be
overkill for the vast majority of the objects allocated.

Changes in v2:
  * Add _aligned_malloc patch for win32.  For what it's
    worth, this passes a gitlab cross-compile test.

  * Add and use qemu_max_align_t for choosing between
    g_malloc and qemu_memalign.

    I had been discussing extra checks for i386-linux with
    Eduardo, but then it occured to me that both linux libc
    posix_memalign is smart enough to not imply extra overhead.
    So qemu_memalign with alignment <= malloc alignment is
    handled easily by the system.


r~


Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: Alistair Francis <Alistair.Francis@wdc.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Cornelia Huck <cohuck@redhat.com>
Cc: qemu-arm@nongnu.org
Cc: qemu-ppc@nongnu.org
Cc: qemu-riscv@nongnu.org
Cc: qemu-s390x@nongnu.org


Richard Henderson (6):
  util/oslib-win32: Use _aligned_malloc for qemu_try_memalign
  qom: Allow objects to be allocated with increased alignment
  target/arm: Set instance_align on CPUARM TypeInfo
  target/ppc: Set instance_align on PowerPCCPU TypeInfo
  target/riscv: Set instance_align on RISCVCPU TypeInfo
  target/s390x: Set instance_align on S390CPU TypeInfo

 include/qom/object.h            |  5 +++++
 qom/object.c                    | 36 ++++++++++++++++++++++++++++++---
 target/arm/cpu.c                |  2 ++
 target/riscv/cpu.c              |  1 +
 target/s390x/cpu.c              |  1 +
 util/oslib-win32.c              | 10 +++------
 target/ppc/translate_init.c.inc |  1 +
 7 files changed, 46 insertions(+), 10 deletions(-)

-- 
2.25.1



^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v2 1/6] util/oslib-win32: Use _aligned_malloc for qemu_try_memalign
  2020-09-16  0:46 ` Richard Henderson
  (?)
@ 2020-09-16  0:46 ` Richard Henderson
  2020-09-16  3:39   ` Stefan Weil
  2020-09-17 18:43   ` Eduardo Habkost
  -1 siblings, 2 replies; 17+ messages in thread
From: Richard Henderson @ 2020-09-16  0:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: Stefan Weil

We do not need or want to be allocating page sized quanta.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
Cc: Stefan Weil <sw@weilnetz.de>
---
 util/oslib-win32.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index c654dafd93..8d838bf342 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -56,10 +56,8 @@ void *qemu_try_memalign(size_t alignment, size_t size)
 {
     void *ptr;
 
-    if (!size) {
-        abort();
-    }
-    ptr = VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE);
+    assert(size != 0);
+    ptr = _aligned_malloc(alignment, size);
     trace_qemu_memalign(alignment, size, ptr);
     return ptr;
 }
@@ -93,9 +91,7 @@ void *qemu_anon_ram_alloc(size_t size, uint64_t *align, bool shared)
 void qemu_vfree(void *ptr)
 {
     trace_qemu_vfree(ptr);
-    if (ptr) {
-        VirtualFree(ptr, 0, MEM_RELEASE);
-    }
+    _aligned_free(ptr);
 }
 
 void qemu_anon_ram_free(void *ptr, size_t size)
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 2/6] qom: Allow objects to be allocated with increased alignment
  2020-09-16  0:46 ` Richard Henderson
  (?)
  (?)
@ 2020-09-16  0:46 ` Richard Henderson
  -1 siblings, 0 replies; 17+ messages in thread
From: Richard Henderson @ 2020-09-16  0:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Daniel P. Berrangé, Eduardo Habkost

It turns out that some hosts have a default malloc alignment less
than that required for vectors.

We assume that, with compiler annotation on CPUArchState, that we
can properly align the vector portion of the guest state.  Fix the
alignment of the allocation by using qemu_memalloc when required.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
---
 include/qom/object.h |  5 +++++
 qom/object.c         | 36 +++++++++++++++++++++++++++++++++---
 2 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index 056f67ab3b..5709c36859 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -691,6 +691,7 @@ struct Object
         .parent = TYPE_##PARENT_MODULE_OBJ_NAME, \
         .name = TYPE_##MODULE_OBJ_NAME, \
         .instance_size = sizeof(ModuleObjName), \
+        .instance_align = __alignof__(ModuleObjName), \
         .instance_init = module_obj_name##_init, \
         .instance_finalize = module_obj_name##_finalize, \
         .class_size = sizeof(ModuleObjName##Class), \
@@ -770,6 +771,9 @@ struct Object
  * @instance_size: The size of the object (derivative of #Object).  If
  *   @instance_size is 0, then the size of the object will be the size of the
  *   parent object.
+ * @instance_align: The required alignment of the object.  If @instance_align
+ *   is 0, then normal malloc alignment is sufficient; if non-zero, then we
+ *   must use qemu_memalign for allocation.
  * @instance_init: This function is called to initialize an object.  The parent
  *   class will have already been initialized so the type is only responsible
  *   for initializing its own members.
@@ -807,6 +811,7 @@ struct TypeInfo
     const char *parent;
 
     size_t instance_size;
+    size_t instance_align;
     void (*instance_init)(Object *obj);
     void (*instance_post_init)(Object *obj);
     void (*instance_finalize)(Object *obj);
diff --git a/qom/object.c b/qom/object.c
index 387efb25eb..fcc15f6d88 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -50,6 +50,7 @@ struct TypeImpl
     size_t class_size;
 
     size_t instance_size;
+    size_t instance_align;
 
     void (*class_init)(ObjectClass *klass, void *data);
     void (*class_base_init)(ObjectClass *klass, void *data);
@@ -114,6 +115,7 @@ static TypeImpl *type_new(const TypeInfo *info)
 
     ti->class_size = info->class_size;
     ti->instance_size = info->instance_size;
+    ti->instance_align = info->instance_align;
 
     ti->class_init = info->class_init;
     ti->class_base_init = info->class_base_init;
@@ -688,16 +690,44 @@ static void object_finalize(void *data)
     }
 }
 
+/* Find the minimum alignment guaranteed by the system malloc. */
+#if __STDC_VERSION__ >= 201112L
+typddef max_align_t qemu_max_align_t;
+#else
+typedef union {
+    long l;
+    void *p;
+    double d;
+    long double ld;
+} qemu_max_align_t;
+#endif
+
 static Object *object_new_with_type(Type type)
 {
     Object *obj;
+    size_t size, align;
+    void (*obj_free)(void *);
 
     g_assert(type != NULL);
     type_initialize(type);
 
-    obj = g_malloc(type->instance_size);
-    object_initialize_with_type(obj, type->instance_size, type);
-    obj->free = g_free;
+    size = type->instance_size;
+    align = type->instance_align;
+
+    /*
+     * Do not use qemu_memalign unless required.  Depending on the
+     * implementation, extra alignment implies extra overhead.
+     */
+    if (likely(align <= __alignof__(qemu_max_align_t))) {
+        obj = g_malloc(size);
+        obj_free = g_free;
+    } else {
+        obj = qemu_memalign(align, size);
+        obj_free = qemu_vfree;
+    }
+
+    object_initialize_with_type(obj, size, type);
+    obj->free = obj_free;
 
     return obj;
 }
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 3/6] target/arm: Set instance_align on CPUARM TypeInfo
  2020-09-16  0:46 ` Richard Henderson
                   ` (2 preceding siblings ...)
  (?)
@ 2020-09-16  0:46 ` Richard Henderson
  -1 siblings, 0 replies; 17+ messages in thread
From: Richard Henderson @ 2020-09-16  0:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, qemu-arm

Fix alignment of CPUARMState.vfp.zregs.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org
---
 target/arm/cpu.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 7b5ea65fab..a7643deab4 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2290,6 +2290,7 @@ void arm_cpu_register(const ARMCPUInfo *info)
     TypeInfo type_info = {
         .parent = TYPE_ARM_CPU,
         .instance_size = sizeof(ARMCPU),
+        .instance_align = __alignof__(ARMCPU),
         .instance_init = arm_cpu_instance_init,
         .class_size = sizeof(ARMCPUClass),
         .class_init = info->class_init ?: cpu_register_class_init,
@@ -2305,6 +2306,7 @@ static const TypeInfo arm_cpu_type_info = {
     .name = TYPE_ARM_CPU,
     .parent = TYPE_CPU,
     .instance_size = sizeof(ARMCPU),
+    .instance_align = __alignof__(ARMCPU),
     .instance_init = arm_cpu_initfn,
     .instance_finalize = arm_cpu_finalizefn,
     .abstract = true,
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 4/6] target/ppc: Set instance_align on PowerPCCPU TypeInfo
  2020-09-16  0:46 ` Richard Henderson
                   ` (3 preceding siblings ...)
  (?)
@ 2020-09-16  0:46 ` Richard Henderson
  2020-09-16  2:18   ` David Gibson
  -1 siblings, 1 reply; 17+ messages in thread
From: Richard Henderson @ 2020-09-16  0:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-ppc, David Gibson

Fix alignment of CPUPPCState.vsr.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: qemu-ppc@nongnu.org
---
 target/ppc/translate_init.c.inc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
index 230a062d29..accb4f2fae 100644
--- a/target/ppc/translate_init.c.inc
+++ b/target/ppc/translate_init.c.inc
@@ -10960,6 +10960,7 @@ static const TypeInfo ppc_cpu_type_info = {
     .name = TYPE_POWERPC_CPU,
     .parent = TYPE_CPU,
     .instance_size = sizeof(PowerPCCPU),
+    .instance_align = __alignof__(PowerPCCPU),
     .instance_init = ppc_cpu_instance_init,
     .instance_finalize = ppc_cpu_instance_finalize,
     .abstract = true,
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 5/6] target/riscv: Set instance_align on RISCVCPU TypeInfo
  2020-09-16  0:46 ` Richard Henderson
@ 2020-09-16  0:46   ` Richard Henderson
  -1 siblings, 0 replies; 17+ messages in thread
From: Richard Henderson @ 2020-09-16  0:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-riscv, Alistair Francis

Fix alignment of CPURISCVState.vreg.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
Cc: Alistair Francis <Alistair.Francis@wdc.com>
Cc: qemu-riscv@nongnu.org
---
 target/riscv/cpu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 57c006df5d..0bbfd7f457 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -628,6 +628,7 @@ static const TypeInfo riscv_cpu_type_infos[] = {
         .name = TYPE_RISCV_CPU,
         .parent = TYPE_CPU,
         .instance_size = sizeof(RISCVCPU),
+        .instance_align = __alignof__(RISCVCPU),
         .instance_init = riscv_cpu_init,
         .abstract = true,
         .class_size = sizeof(RISCVCPUClass),
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 5/6] target/riscv: Set instance_align on RISCVCPU TypeInfo
@ 2020-09-16  0:46   ` Richard Henderson
  0 siblings, 0 replies; 17+ messages in thread
From: Richard Henderson @ 2020-09-16  0:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alistair Francis, qemu-riscv

Fix alignment of CPURISCVState.vreg.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
Cc: Alistair Francis <Alistair.Francis@wdc.com>
Cc: qemu-riscv@nongnu.org
---
 target/riscv/cpu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 57c006df5d..0bbfd7f457 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -628,6 +628,7 @@ static const TypeInfo riscv_cpu_type_infos[] = {
         .name = TYPE_RISCV_CPU,
         .parent = TYPE_CPU,
         .instance_size = sizeof(RISCVCPU),
+        .instance_align = __alignof__(RISCVCPU),
         .instance_init = riscv_cpu_init,
         .abstract = true,
         .class_size = sizeof(RISCVCPUClass),
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 6/6] target/s390x: Set instance_align on S390CPU TypeInfo
  2020-09-16  0:46 ` Richard Henderson
                   ` (5 preceding siblings ...)
  (?)
@ 2020-09-16  0:46 ` Richard Henderson
  -1 siblings, 0 replies; 17+ messages in thread
From: Richard Henderson @ 2020-09-16  0:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-s390x, Cornelia Huck, David Hildenbrand

Fix alignment of CPUS390XState.vregs.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
Cc: David Hildenbrand <david@redhat.com>
Cc: Cornelia Huck <cohuck@redhat.com>
Cc: qemu-s390x@nongnu.org
---
 target/s390x/cpu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index 749cd548f0..e350edc9f5 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -517,6 +517,7 @@ static const TypeInfo s390_cpu_type_info = {
     .name = TYPE_S390_CPU,
     .parent = TYPE_CPU,
     .instance_size = sizeof(S390CPU),
+    .instance_align = __alignof__(S390CPU),
     .instance_init = s390_cpu_initfn,
     .instance_finalize = s390_cpu_finalize,
     .abstract = true,
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 4/6] target/ppc: Set instance_align on PowerPCCPU TypeInfo
  2020-09-16  0:46 ` [PATCH v2 4/6] target/ppc: Set instance_align on PowerPCCPU TypeInfo Richard Henderson
@ 2020-09-16  2:18   ` David Gibson
  0 siblings, 0 replies; 17+ messages in thread
From: David Gibson @ 2020-09-16  2:18 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-ppc, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 1172 bytes --]

On Tue, Sep 15, 2020 at 05:46:36PM -0700, Richard Henderson wrote:
> Fix alignment of CPUPPCState.vsr.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

LGTM

Acked-by: David Gibson <david@gibson.dropbear.id.au>

> ---
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: qemu-ppc@nongnu.org
> ---
>  target/ppc/translate_init.c.inc | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
> index 230a062d29..accb4f2fae 100644
> --- a/target/ppc/translate_init.c.inc
> +++ b/target/ppc/translate_init.c.inc
> @@ -10960,6 +10960,7 @@ static const TypeInfo ppc_cpu_type_info = {
>      .name = TYPE_POWERPC_CPU,
>      .parent = TYPE_CPU,
>      .instance_size = sizeof(PowerPCCPU),
> +    .instance_align = __alignof__(PowerPCCPU),
>      .instance_init = ppc_cpu_instance_init,
>      .instance_finalize = ppc_cpu_instance_finalize,
>      .abstract = true,

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 1/6] util/oslib-win32: Use _aligned_malloc for qemu_try_memalign
  2020-09-16  0:46 ` [PATCH v2 1/6] util/oslib-win32: Use _aligned_malloc for qemu_try_memalign Richard Henderson
@ 2020-09-16  3:39   ` Stefan Weil
  2020-09-17 18:43   ` Eduardo Habkost
  1 sibling, 0 replies; 17+ messages in thread
From: Stefan Weil @ 2020-09-16  3:39 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel

Am 16.09.20 um 02:46 schrieb Richard Henderson:

> We do not need or want to be allocating page sized quanta.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> Cc: Stefan Weil <sw@weilnetz.de>
> ---
>  util/oslib-win32.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> index c654dafd93..8d838bf342 100644
> --- a/util/oslib-win32.c
> +++ b/util/oslib-win32.c
> @@ -56,10 +56,8 @@ void *qemu_try_memalign(size_t alignment, size_t size)
>  {
>      void *ptr;
>  
> -    if (!size) {
> -        abort();
> -    }
> -    ptr = VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE);
> +    assert(size != 0);
> +    ptr = _aligned_malloc(alignment, size);
>      trace_qemu_memalign(alignment, size, ptr);
>      return ptr;
>  }
> @@ -93,9 +91,7 @@ void *qemu_anon_ram_alloc(size_t size, uint64_t *align, bool shared)
>  void qemu_vfree(void *ptr)
>  {
>      trace_qemu_vfree(ptr);
> -    if (ptr) {
> -        VirtualFree(ptr, 0, MEM_RELEASE);
> -    }
> +    _aligned_free(ptr);
>  }
>  
>  void qemu_anon_ram_free(void *ptr, size_t size)


According to the documentation, malloc.h should be included for
_aligned_malloc. See
https://docs.microsoft.com/de-de/cpp/c-runtime-library/reference/aligned-malloc?view=vs-2019

I'd also use g_assert instead of assert to make sure that it is not
removed by NDEBUG.

Thanks,

Stefan




^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 5/6] target/riscv: Set instance_align on RISCVCPU TypeInfo
  2020-09-16  0:46   ` Richard Henderson
@ 2020-09-16 14:58     ` Alistair Francis
  -1 siblings, 0 replies; 17+ messages in thread
From: Alistair Francis @ 2020-09-16 14:58 UTC (permalink / raw)
  To: Richard Henderson
  Cc: Alistair Francis, open list:RISC-V, qemu-devel@nongnu.org Developers

On Tue, Sep 15, 2020 at 5:47 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Fix alignment of CPURISCVState.vreg.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
> Cc: Alistair Francis <Alistair.Francis@wdc.com>
> Cc: qemu-riscv@nongnu.org
> ---
>  target/riscv/cpu.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 57c006df5d..0bbfd7f457 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -628,6 +628,7 @@ static const TypeInfo riscv_cpu_type_infos[] = {
>          .name = TYPE_RISCV_CPU,
>          .parent = TYPE_CPU,
>          .instance_size = sizeof(RISCVCPU),
> +        .instance_align = __alignof__(RISCVCPU),
>          .instance_init = riscv_cpu_init,
>          .abstract = true,
>          .class_size = sizeof(RISCVCPUClass),
> --
> 2.25.1
>
>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 5/6] target/riscv: Set instance_align on RISCVCPU TypeInfo
@ 2020-09-16 14:58     ` Alistair Francis
  0 siblings, 0 replies; 17+ messages in thread
From: Alistair Francis @ 2020-09-16 14:58 UTC (permalink / raw)
  To: Richard Henderson
  Cc: qemu-devel@nongnu.org Developers, open list:RISC-V, Alistair Francis

On Tue, Sep 15, 2020 at 5:47 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Fix alignment of CPURISCVState.vreg.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
> Cc: Alistair Francis <Alistair.Francis@wdc.com>
> Cc: qemu-riscv@nongnu.org
> ---
>  target/riscv/cpu.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 57c006df5d..0bbfd7f457 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -628,6 +628,7 @@ static const TypeInfo riscv_cpu_type_infos[] = {
>          .name = TYPE_RISCV_CPU,
>          .parent = TYPE_CPU,
>          .instance_size = sizeof(RISCVCPU),
> +        .instance_align = __alignof__(RISCVCPU),
>          .instance_init = riscv_cpu_init,
>          .abstract = true,
>          .class_size = sizeof(RISCVCPUClass),
> --
> 2.25.1
>
>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 1/6] util/oslib-win32: Use _aligned_malloc for qemu_try_memalign
  2020-09-16  0:46 ` [PATCH v2 1/6] util/oslib-win32: Use _aligned_malloc for qemu_try_memalign Richard Henderson
  2020-09-16  3:39   ` Stefan Weil
@ 2020-09-17 18:43   ` Eduardo Habkost
  2020-09-17 20:34     ` Richard Henderson
  1 sibling, 1 reply; 17+ messages in thread
From: Eduardo Habkost @ 2020-09-17 18:43 UTC (permalink / raw)
  To: Richard Henderson; +Cc: Stefan Weil, qemu-devel

On Tue, Sep 15, 2020 at 05:46:33PM -0700, Richard Henderson wrote:
> We do not need or want to be allocating page sized quanta.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Would it be safe to keep patches 2-6 applied in case this one
gets dropped or reverted for any reason?

I consider patches 2-6 bug fixes (that I'd like to merge soon),
and this one as an optimization (that could be merged later).

> ---
> Cc: Stefan Weil <sw@weilnetz.de>
> ---
>  util/oslib-win32.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> index c654dafd93..8d838bf342 100644
> --- a/util/oslib-win32.c
> +++ b/util/oslib-win32.c
> @@ -56,10 +56,8 @@ void *qemu_try_memalign(size_t alignment, size_t size)
>  {
>      void *ptr;
>  
> -    if (!size) {
> -        abort();
> -    }
> -    ptr = VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE);
> +    assert(size != 0);
> +    ptr = _aligned_malloc(alignment, size);
>      trace_qemu_memalign(alignment, size, ptr);
>      return ptr;
>  }
> @@ -93,9 +91,7 @@ void *qemu_anon_ram_alloc(size_t size, uint64_t *align, bool shared)
>  void qemu_vfree(void *ptr)
>  {
>      trace_qemu_vfree(ptr);
> -    if (ptr) {
> -        VirtualFree(ptr, 0, MEM_RELEASE);
> -    }
> +    _aligned_free(ptr);
>  }
>  
>  void qemu_anon_ram_free(void *ptr, size_t size)
> -- 
> 2.25.1
> 
> 

-- 
Eduardo



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 1/6] util/oslib-win32: Use _aligned_malloc for qemu_try_memalign
  2020-09-17 18:43   ` Eduardo Habkost
@ 2020-09-17 20:34     ` Richard Henderson
  0 siblings, 0 replies; 17+ messages in thread
From: Richard Henderson @ 2020-09-17 20:34 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: Stefan Weil, qemu-devel

On 9/17/20 11:43 AM, Eduardo Habkost wrote:
> On Tue, Sep 15, 2020 at 05:46:33PM -0700, Richard Henderson wrote:
>> We do not need or want to be allocating page sized quanta.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> 
> Would it be safe to keep patches 2-6 applied in case this one
> gets dropped or reverted for any reason?

Yes.

Only objects using QEMU_ALIGNED() should have an __alignof__ larger than
qemu_max_align_t.

Therefore, I believe that the CPUArchState objects to be the only ones that
will go through qemu_memalign.  So we won't end up with all QOM objects being
page allocated on Windows.


r~


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 0/6] qom: Allow object to be aligned
  2020-09-16  0:46 ` Richard Henderson
@ 2020-09-18 18:00   ` Eduardo Habkost
  -1 siblings, 0 replies; 17+ messages in thread
From: Eduardo Habkost @ 2020-09-18 18:00 UTC (permalink / raw)
  To: Richard Henderson
  Cc: Peter Maydell, Daniel P. Berrangé,
	qemu-riscv, David Hildenbrand, qemu-ppc, Cornelia Huck,
	qemu-devel, qemu-s390x, qemu-arm, Alistair Francis,
	Paolo Bonzini, David Gibson

On Tue, Sep 15, 2020 at 05:46:32PM -0700, Richard Henderson wrote:
> I've seen some failures on arm and s390x hosts after
> enabling host vector support.  It turns out that the
> malloc for these hosts does not provide 16-byte alignment.
> 
> We already have a function that can alloc with alignment,
> but we need to pass this down from the structure.  We also
> don't want to use this function unconditionally, because
> the windows version does page allocation, which would be
> overkill for the vast majority of the objects allocated.

I'm queueing patches 2-6.  Thanks!

-- 
Eduardo



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 0/6] qom: Allow object to be aligned
@ 2020-09-18 18:00   ` Eduardo Habkost
  0 siblings, 0 replies; 17+ messages in thread
From: Eduardo Habkost @ 2020-09-18 18:00 UTC (permalink / raw)
  To: Richard Henderson
  Cc: qemu-devel, Paolo Bonzini, Daniel P. Berrangé,
	Peter Maydell, David Gibson, Alistair Francis, David Hildenbrand,
	Cornelia Huck, qemu-arm, qemu-ppc, qemu-riscv, qemu-s390x

On Tue, Sep 15, 2020 at 05:46:32PM -0700, Richard Henderson wrote:
> I've seen some failures on arm and s390x hosts after
> enabling host vector support.  It turns out that the
> malloc for these hosts does not provide 16-byte alignment.
> 
> We already have a function that can alloc with alignment,
> but we need to pass this down from the structure.  We also
> don't want to use this function unconditionally, because
> the windows version does page allocation, which would be
> overkill for the vast majority of the objects allocated.

I'm queueing patches 2-6.  Thanks!

-- 
Eduardo



^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2020-09-18 18:02 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-16  0:46 [PATCH v2 0/6] qom: Allow object to be aligned Richard Henderson
2020-09-16  0:46 ` Richard Henderson
2020-09-16  0:46 ` [PATCH v2 1/6] util/oslib-win32: Use _aligned_malloc for qemu_try_memalign Richard Henderson
2020-09-16  3:39   ` Stefan Weil
2020-09-17 18:43   ` Eduardo Habkost
2020-09-17 20:34     ` Richard Henderson
2020-09-16  0:46 ` [PATCH v2 2/6] qom: Allow objects to be allocated with increased alignment Richard Henderson
2020-09-16  0:46 ` [PATCH v2 3/6] target/arm: Set instance_align on CPUARM TypeInfo Richard Henderson
2020-09-16  0:46 ` [PATCH v2 4/6] target/ppc: Set instance_align on PowerPCCPU TypeInfo Richard Henderson
2020-09-16  2:18   ` David Gibson
2020-09-16  0:46 ` [PATCH v2 5/6] target/riscv: Set instance_align on RISCVCPU TypeInfo Richard Henderson
2020-09-16  0:46   ` Richard Henderson
2020-09-16 14:58   ` Alistair Francis
2020-09-16 14:58     ` Alistair Francis
2020-09-16  0:46 ` [PATCH v2 6/6] target/s390x: Set instance_align on S390CPU TypeInfo Richard Henderson
2020-09-18 18:00 ` [PATCH v2 0/6] qom: Allow object to be aligned Eduardo Habkost
2020-09-18 18:00   ` Eduardo Habkost

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.