| Message ID | 20240910175809.2135596-2-david@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | s390x: virtio-mem support | expand |
On 9/10/24 7:57 PM, David Hildenbrand wrote: > KVM is not happy when starting a VM with weird RAM sizes: > > # qemu-system-s390x --enable-kvm --nographic -m 1234K > qemu-system-s390x: kvm_set_user_memory_region: KVM_SET_USER_MEMORY_REGION > failed, slot=0, start=0x0, size=0x244000: Invalid argument > kvm_set_phys_mem: error registering slot: Invalid argument > Aborted (core dumped) > > Let's handle that in a better way by rejecting such weird RAM sizes > right from the start: > Huh, I always assumed that ram is handled in multiples of 1MB in QEMU. Acked-by: Janosch Frank <frankja@linux.ibm.com>
On 10/09/2024 19.57, David Hildenbrand wrote: > KVM is not happy when starting a VM with weird RAM sizes: > > # qemu-system-s390x --enable-kvm --nographic -m 1234K > qemu-system-s390x: kvm_set_user_memory_region: KVM_SET_USER_MEMORY_REGION > failed, slot=0, start=0x0, size=0x244000: Invalid argument > kvm_set_phys_mem: error registering slot: Invalid argument > Aborted (core dumped) > > Let's handle that in a better way by rejecting such weird RAM sizes > right from the start: > > # qemu-system-s390x --enable-kvm --nographic -m 1234K > qemu-system-s390x: ram size must be multiples of 1 MiB > > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > hw/s390x/s390-virtio-ccw.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) Reviewed-by: Thomas Huth <thuth@redhat.com>
On 11.09.24 13:28, Janosch Frank wrote: > On 9/10/24 7:57 PM, David Hildenbrand wrote: >> KVM is not happy when starting a VM with weird RAM sizes: >> >> # qemu-system-s390x --enable-kvm --nographic -m 1234K >> qemu-system-s390x: kvm_set_user_memory_region: KVM_SET_USER_MEMORY_REGION >> failed, slot=0, start=0x0, size=0x244000: Invalid argument >> kvm_set_phys_mem: error registering slot: Invalid argument >> Aborted (core dumped) >> >> Let's handle that in a better way by rejecting such weird RAM sizes >> right from the start: >> > > Huh, I always assumed that ram is handled in multiples of 1MB in QEMU. Me as well, I did not dig if that changed at some point ... or why such odd sizes would even be required :) Thanks!
On 11/09/2024 14.38, David Hildenbrand wrote: > On 11.09.24 13:28, Janosch Frank wrote: >> On 9/10/24 7:57 PM, David Hildenbrand wrote: >>> KVM is not happy when starting a VM with weird RAM sizes: >>> >>> # qemu-system-s390x --enable-kvm --nographic -m 1234K >>> qemu-system-s390x: kvm_set_user_memory_region: >>> KVM_SET_USER_MEMORY_REGION >>> failed, slot=0, start=0x0, size=0x244000: Invalid argument >>> kvm_set_phys_mem: error registering slot: Invalid argument >>> Aborted (core dumped) >>> >>> Let's handle that in a better way by rejecting such weird RAM sizes >>> right from the start: >>> >> >> Huh, I always assumed that ram is handled in multiples of 1MB in QEMU. > > Me as well, I did not dig if that changed at some point ... or why such odd > sizes would even be required :) I guess it's there for some old PC hardware ... Remember, 640K ought to be enough for anybody. Thomas
On 11.09.24 14:46, Thomas Huth wrote: > On 11/09/2024 14.38, David Hildenbrand wrote: >> On 11.09.24 13:28, Janosch Frank wrote: >>> On 9/10/24 7:57 PM, David Hildenbrand wrote: >>>> KVM is not happy when starting a VM with weird RAM sizes: >>>> >>>> # qemu-system-s390x --enable-kvm --nographic -m 1234K >>>> qemu-system-s390x: kvm_set_user_memory_region: >>>> KVM_SET_USER_MEMORY_REGION >>>> failed, slot=0, start=0x0, size=0x244000: Invalid argument >>>> kvm_set_phys_mem: error registering slot: Invalid argument >>>> Aborted (core dumped) >>>> >>>> Let's handle that in a better way by rejecting such weird RAM sizes >>>> right from the start: >>>> >>> >>> Huh, I always assumed that ram is handled in multiples of 1MB in QEMU. >> >> Me as well, I did not dig if that changed at some point ... or why such odd >> sizes would even be required :) > > I guess it's there for some old PC hardware ... Remember, 640K ought to be > enough for anybody. True, maybe the "default to MiB" made some of us believe that something smaller is impossible :) And in fact, I think suffix support was added in commit 6e1d3c1c855818a6d1399698572afae0d11b872b Author: Igor Mammedov <imammedo@redhat.com> Date: Wed Nov 27 01:27:35 2013 +0100 vl: convert -m to QemuOpts Adds option to -m "size" - startup memory amount For compatibility with legacy CLI if suffix-less number is passed, it assumes amount in Mb. Otherwise user is free to use suffixed number using suffixes b,k/K,M,G
On Tue, 2024-09-10 at 19:57 +0200, David Hildenbrand wrote: > KVM is not happy when starting a VM with weird RAM sizes: > > # qemu-system-s390x --enable-kvm --nographic -m 1234K > qemu-system-s390x: kvm_set_user_memory_region: KVM_SET_USER_MEMORY_REGION > failed, slot=0, start=0x0, size=0x244000: Invalid argument > kvm_set_phys_mem: error registering slot: Invalid argument > Aborted (core dumped) > > Let's handle that in a better way by rejecting such weird RAM sizes > right from the start: > > # qemu-system-s390x --enable-kvm --nographic -m 1234K > qemu-system-s390x: ram size must be multiples of 1 MiB > > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > hw/s390x/s390-virtio-ccw.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) TIL. Thanks David! Reviewed-by: Eric Farman <farman@linux.ibm.com>
On 10.09.24 19:57, David Hildenbrand wrote: > KVM is not happy when starting a VM with weird RAM sizes: > > # qemu-system-s390x --enable-kvm --nographic -m 1234K > qemu-system-s390x: kvm_set_user_memory_region: KVM_SET_USER_MEMORY_REGION > failed, slot=0, start=0x0, size=0x244000: Invalid argument > kvm_set_phys_mem: error registering slot: Invalid argument > Aborted (core dumped) > > Let's handle that in a better way by rejecting such weird RAM sizes > right from the start: > > # qemu-system-s390x --enable-kvm --nographic -m 1234K > qemu-system-s390x: ram size must be multiples of 1 MiB > > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > hw/s390x/s390-virtio-ccw.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > index 18240a0fd8..e30cf0a2a1 100644 > --- a/hw/s390x/s390-virtio-ccw.c > +++ b/hw/s390x/s390-virtio-ccw.c > @@ -180,6 +180,17 @@ static void s390_memory_init(MemoryRegion *ram) > { > MemoryRegion *sysmem = get_system_memory(); > > + if (!QEMU_IS_ALIGNED(memory_region_size(ram), 1 * MiB)) { > + /* > + * The SCLP cannot possibly expose smaller granularity right now and KVM > + * cannot handle smaller granularity. As we don't support NUMA, the > + * region size directly corresponds to machine->ram_size, and the region > + * is a single RAM memory region. > + */ > + error_report("ram size must be multiples of 1 MiB"); > + exit(EXIT_FAILURE); > + } I'll switch to error_setg(&error_fatal, "ram size must be multiples of 1 MiB"); here, to avoid the manual exit(). Please someone shout if I should keep it as is.
On 23/09/2024 11.19, David Hildenbrand wrote: > On 10.09.24 19:57, David Hildenbrand wrote: >> KVM is not happy when starting a VM with weird RAM sizes: >> >> # qemu-system-s390x --enable-kvm --nographic -m 1234K >> qemu-system-s390x: kvm_set_user_memory_region: KVM_SET_USER_MEMORY_REGION >> failed, slot=0, start=0x0, size=0x244000: Invalid argument >> kvm_set_phys_mem: error registering slot: Invalid argument >> Aborted (core dumped) >> >> Let's handle that in a better way by rejecting such weird RAM sizes >> right from the start: >> >> # qemu-system-s390x --enable-kvm --nographic -m 1234K >> qemu-system-s390x: ram size must be multiples of 1 MiB >> >> Signed-off-by: David Hildenbrand <david@redhat.com> >> --- >> hw/s390x/s390-virtio-ccw.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c >> index 18240a0fd8..e30cf0a2a1 100644 >> --- a/hw/s390x/s390-virtio-ccw.c >> +++ b/hw/s390x/s390-virtio-ccw.c >> @@ -180,6 +180,17 @@ static void s390_memory_init(MemoryRegion *ram) >> { >> MemoryRegion *sysmem = get_system_memory(); >> + if (!QEMU_IS_ALIGNED(memory_region_size(ram), 1 * MiB)) { >> + /* >> + * The SCLP cannot possibly expose smaller granularity right now >> and KVM >> + * cannot handle smaller granularity. As we don't support NUMA, the >> + * region size directly corresponds to machine->ram_size, and the >> region >> + * is a single RAM memory region. >> + */ >> + error_report("ram size must be multiples of 1 MiB"); >> + exit(EXIT_FAILURE); >> + } > > I'll switch to > > error_setg(&error_fatal, "ram size must be multiples of 1 MiB"); > > here, to avoid the manual exit(). > > Please someone shout if I should keep it as is. Please keep it, according to include/qapi/error.h: * Please don't error_setg(&error_fatal, ...), use error_report() and * exit(), because that's more obvious. Thanks, Thomas
On 23.09.24 17:36, Thomas Huth wrote: > On 23/09/2024 11.19, David Hildenbrand wrote: >> On 10.09.24 19:57, David Hildenbrand wrote: >>> KVM is not happy when starting a VM with weird RAM sizes: >>> >>> # qemu-system-s390x --enable-kvm --nographic -m 1234K >>> qemu-system-s390x: kvm_set_user_memory_region: KVM_SET_USER_MEMORY_REGION >>> failed, slot=0, start=0x0, size=0x244000: Invalid argument >>> kvm_set_phys_mem: error registering slot: Invalid argument >>> Aborted (core dumped) >>> >>> Let's handle that in a better way by rejecting such weird RAM sizes >>> right from the start: >>> >>> # qemu-system-s390x --enable-kvm --nographic -m 1234K >>> qemu-system-s390x: ram size must be multiples of 1 MiB >>> >>> Signed-off-by: David Hildenbrand <david@redhat.com> >>> --- >>> hw/s390x/s390-virtio-ccw.c | 11 +++++++++++ >>> 1 file changed, 11 insertions(+) >>> >>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c >>> index 18240a0fd8..e30cf0a2a1 100644 >>> --- a/hw/s390x/s390-virtio-ccw.c >>> +++ b/hw/s390x/s390-virtio-ccw.c >>> @@ -180,6 +180,17 @@ static void s390_memory_init(MemoryRegion *ram) >>> { >>> MemoryRegion *sysmem = get_system_memory(); >>> + if (!QEMU_IS_ALIGNED(memory_region_size(ram), 1 * MiB)) { >>> + /* >>> + * The SCLP cannot possibly expose smaller granularity right now >>> and KVM >>> + * cannot handle smaller granularity. As we don't support NUMA, the >>> + * region size directly corresponds to machine->ram_size, and the >>> region >>> + * is a single RAM memory region. >>> + */ >>> + error_report("ram size must be multiples of 1 MiB"); >>> + exit(EXIT_FAILURE); >>> + } >> >> I'll switch to >> >> error_setg(&error_fatal, "ram size must be multiples of 1 MiB"); >> >> here, to avoid the manual exit(). >> >> Please someone shout if I should keep it as is. > > Please keep it, according to include/qapi/error.h: > > * Please don't error_setg(&error_fatal, ...), use error_report() and > * exit(), because that's more obvious. > Controversial, but will do. Thanks!
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index 18240a0fd8..e30cf0a2a1 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -180,6 +180,17 @@ static void s390_memory_init(MemoryRegion *ram) { MemoryRegion *sysmem = get_system_memory(); + if (!QEMU_IS_ALIGNED(memory_region_size(ram), 1 * MiB)) { + /* + * The SCLP cannot possibly expose smaller granularity right now and KVM + * cannot handle smaller granularity. As we don't support NUMA, the + * region size directly corresponds to machine->ram_size, and the region + * is a single RAM memory region. + */ + error_report("ram size must be multiples of 1 MiB"); + exit(EXIT_FAILURE); + } + /* allocate RAM for core */ memory_region_add_subregion(sysmem, 0, ram);
KVM is not happy when starting a VM with weird RAM sizes: # qemu-system-s390x --enable-kvm --nographic -m 1234K qemu-system-s390x: kvm_set_user_memory_region: KVM_SET_USER_MEMORY_REGION failed, slot=0, start=0x0, size=0x244000: Invalid argument kvm_set_phys_mem: error registering slot: Invalid argument Aborted (core dumped) Let's handle that in a better way by rejecting such weird RAM sizes right from the start: # qemu-system-s390x --enable-kvm --nographic -m 1234K qemu-system-s390x: ram size must be multiples of 1 MiB Signed-off-by: David Hildenbrand <david@redhat.com> --- hw/s390x/s390-virtio-ccw.c | 11 +++++++++++ 1 file changed, 11 insertions(+)