| Message ID | 20200916004638.2444147-1-richard.henderson@linaro.org |
|---|---|
| Headers | show |
| Series | qom: Allow object to be aligned | expand |
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 > >
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~
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!
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(-)