s390x/kvm: legacy_s390_alloc() fixes
cover s390x/kvm: legacy_s390_alloc() fixes
Two patches for legacy_s390_alloc(), only relevant under old z/VM versions.
Patch #1 is relevant when trying to allocate memory for more than one memory region. Patch #2 is only for completeness, to make it behave more similar to qemu_anon_ram_alloc().
David Hildenbrand (2):
s390x/kvm: legacy_s390_alloc() only supports one allocation s390x/kvm: indicate alignment in legacy_s390_alloc()
target/s390x/kvm.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)
Signature · 1 line
2.17.1
PATCH 1/2 s390x/kvm: legacy_s390_alloc() only supports one allocation
We always allocate at a fixed address, a second allocation can therefore of course never work. We would simply overwrite mappings.
This can e.g. happen in s390_memory_init(), if trying to allocate more than > 8TB. Let's just bail out, as there is no need for supporting it (legacy handling for z/VM).
--- target/s390x/kvm.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
target/s390x/kvm.c +10 -2
@@ -752,12 +752,20 @@ int kvm_s390_mem_op(S390CPU *cpu, vaddr addr, uint8_t ar, void *hostbuf,*/ static void *legacy_s390_alloc (size_t size,uint64_t *align,bool shared) { -void *mem; +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 ); -return mem ==MAP_FAILED ?NULL : mem; +if (mem ==MAP_FAILED ) { + mem =NULL ; + } +return mem; }static uint8_t const *sw_bp_inst;
Signature · 1 line
2.17.1
Re: [Qemu-devel] [qemu-s390x] [PATCH v1 1/2] s390x/kvm: legacy_s390_alloc() only supports one allocation
Quoted 8 lines · David Hildenbrand
We always allocate at a fixed address, a second allocation can therefore of course never work. We would simply overwrite mappings.
This can e.g. happen in
s390_memory_init(), if trying to allocate more than > 8TB. Let's just bail out, as there is no need for supporting it (legacy handling for z/VM).Signed-off-by David Hildenbrand <[email protected]>
Quoted 32 lines
--- target/s390x/kvm.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)target/s390x/kvm.c +10
-2@@ -752,12 +752,20 @@ int kvm_s390_mem_op(S390CPU *cpu, vaddr addr, uint8_t ar, void *hostbuf,*/ static void *legacy_s390_alloc (size_t size,uint64_t *align,bool shared) { -void *mem; +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 ); -return mem ==MAP_FAILED ?NULL : mem; +if (mem ==MAP_FAILED ) { + mem =NULL ; + } +return mem; }static uint8_t const *sw_bp_inst;
PATCH 2/2 s390x/kvm: indicate alignment in legacy_s390_alloc()
In reply to David HildenbrandLet's do this for completeness reason, although we don't support e.g. PCDIMM/NVDIMM, which would use the alignment for placing the memory region in guest physical memory. But maybe someday we would want to support something like this - then we don't forget about this if allowing multiple allocations in legacy_s390_alloc().
Use the same alignment as we would set in qemu_anon_ram_alloc(). Our fixed address satisfies this alignment (1MB). This implicitly sets the alignment of the underlying memory region.
--- target/s390x/kvm.c | 3 +++ 1 file changed, 3 insertions(+)
target/s390x/kvm.c +3 -0
@@ -765,6 +765,9 @@ static void *legacy_s390_alloc(size_t size, uint64_t *align, bool shared)if (mem ==MAP_FAILED ) { mem =NULL ; } +if (mem && align) { + *align =QEMU_VMALLOC_ALIGN ; + }return mem; }
Signature · 1 line
2.17.1
Cornelia Huck
AppliedQuoted 1 line · On Thu, 28 Jun 2018 13:38:15 +0200 David Hildenbrand <[email protected]> wrote:
Two patches for
legacy_s390_alloc(), only relevant under old z/VM versions.
Those teddy bears are really looking a bit scruffy... but the patches look sane.
Quoted 5 lines
Patch #1 is relevant when trying to allocate memory for more than one memory region. Patch #2 is only for completeness, to make it behave more similar to
qemu_anon_ram_alloc().
Quoted 7 lines
David Hildenbrand (2):
s390x/kvm:legacy_s390_alloc()only supports one allocation s390x/kvm: indicate alignment inlegacy_s390_alloc()target/s390x/kvm.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)
Thanks, applied.