| Message ID | 20180801131905.148778-3-frankja@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x: Enable KVM huge page backing support | expand |
On 01.08.2018 15:19, Janosch Frank wrote: > QEMU has had huge page support for a longer time already, but KVM > memory management under s390x needed some changes to work with huge > backings. > > Now that we have support, let's enable it if requested and > available. Otherwise we now properly tell the user if there is no > support and back out instead of failing to run the VM later on. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > target/s390x/kvm.c | 34 ++++++++++++++++++++++++++++++++-- > 1 file changed, 32 insertions(+), 2 deletions(-) > > diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c > index d923cf4240..d1c72975cf 100644 > --- a/target/s390x/kvm.c > +++ b/target/s390x/kvm.c > @@ -34,6 +34,8 @@ > #include "qapi/error.h" > #include "qemu/error-report.h" > #include "qemu/timer.h" > +#include "qemu/units.h" > +#include "qemu/mmap-alloc.h" > #include "sysemu/sysemu.h" > #include "sysemu/hw_accel.h" > #include "hw/hw.h" > @@ -139,6 +141,7 @@ static int cap_mem_op; > static int cap_s390_irq; > static int cap_ri; > static int cap_gs; > +static int cap_hpage_1m; > > static int active_cmma; > > @@ -220,9 +223,9 @@ static void kvm_s390_enable_cmma(void) > .attr = KVM_S390_VM_MEM_ENABLE_CMMA, > }; > > - if (mem_path) { > + if (cap_hpage_1m) { > warn_report("CMM will not be enabled because it is not " > - "compatible with hugetlbfs."); > + "compatible with huge memory backings."); This implies some change to certain setups (tmpfs), but as they are absolutely not common (and never really were supported/expected to work), I think this is fine. Reviewed-by: David Hildenbrand <david@redhat.com> > return; > } > rc = kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, &attr); > @@ -281,10 +284,37 @@ void kvm_s390_crypto_reset(void) > } > } > > +static int kvm_s390_configure_mempath_backing(KVMState *s) > +{ > + size_t path_psize = qemu_mempath_getpagesize(mem_path); > + > + if (path_psize == 4 * KiB) > + return 0; > + > + if (path_psize != 1 * MiB) { > + error_report("Memory backing with 2G pages was specified, " > + "but KVM does not support this memory backing"); > + return -EINVAL; > + } > + > + if (kvm_vm_enable_cap(s, KVM_CAP_S390_HPAGE_1M, 0)) { > + error_report("Memory backing with 1M pages was specified, " > + "but KVM does not support this memory backing"); > + return -EINVAL; > + } > + > + cap_hpage_1m = 1; > + return 0; > +} > + > int kvm_arch_init(MachineState *ms, KVMState *s) > { > MachineClass *mc = MACHINE_GET_CLASS(ms); > > + if (mem_path && kvm_s390_configure_mempath_backing(s)) { > + return -EINVAL; > + } > + > mc->default_cpu_type = S390_CPU_TYPE_NAME("host"); > cap_sync_regs = kvm_check_extension(s, KVM_CAP_SYNC_REGS); > cap_async_pf = kvm_check_extension(s, KVM_CAP_ASYNC_PF); >
On 08/01/2018 03:19 PM, Janosch Frank wrote: > QEMU has had huge page support for a longer time already, but KVM > memory management under s390x needed some changes to work with huge > backings. > > Now that we have support, let's enable it if requested and > available. Otherwise we now properly tell the user if there is no > support and back out instead of failing to run the VM later on. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > target/s390x/kvm.c | 34 ++++++++++++++++++++++++++++++++-- > 1 file changed, 32 insertions(+), 2 deletions(-) > > diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c > index d923cf4240..d1c72975cf 100644 > --- a/target/s390x/kvm.c > +++ b/target/s390x/kvm.c > @@ -34,6 +34,8 @@ > #include "qapi/error.h" > #include "qemu/error-report.h" > #include "qemu/timer.h" > +#include "qemu/units.h" > +#include "qemu/mmap-alloc.h" > #include "sysemu/sysemu.h" > #include "sysemu/hw_accel.h" > #include "hw/hw.h" > @@ -139,6 +141,7 @@ static int cap_mem_op; > static int cap_s390_irq; > static int cap_ri; > static int cap_gs; > +static int cap_hpage_1m; > > static int active_cmma; > > @@ -220,9 +223,9 @@ static void kvm_s390_enable_cmma(void) > .attr = KVM_S390_VM_MEM_ENABLE_CMMA, > }; > > - if (mem_path) { > + if (cap_hpage_1m) { > warn_report("CMM will not be enabled because it is not " > - "compatible with hugetlbfs."); > + "compatible with huge memory backings."); > return; > } > rc = kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, &attr); > @@ -281,10 +284,37 @@ void kvm_s390_crypto_reset(void) > } > } > > +static int kvm_s390_configure_mempath_backing(KVMState *s) > +{ > + size_t path_psize = qemu_mempath_getpagesize(mem_path); > + > + if (path_psize == 4 * KiB) > + return 0; Missing curly braces. > + if (path_psize != 1 * MiB) { > + error_report("Memory backing with 2G pages was specified, " > + "but KVM does not support this memory backing"); May I suggest to use a text that rather matches the condition of the if-statement (just in case there will be other sizes in the future...), e.g. "Memory backing is only supported for 1 MiB huge pages" or something similar. Thomas
On 01.08.2018 18:47, Thomas Huth wrote: > On 08/01/2018 03:19 PM, Janosch Frank wrote: >> +static int kvm_s390_configure_mempath_backing(KVMState *s) >> +{ >> + size_t path_psize = qemu_mempath_getpagesize(mem_path); >> + >> + if (path_psize == 4 * KiB) >> + return 0; > > Missing curly braces. urgh, right, this is not the kernel. > >> + if (path_psize != 1 * MiB) { >> + error_report("Memory backing with 2G pages was specified, " >> + "but KVM does not support this memory backing"); > > May I suggest to use a text that rather matches the condition of the > if-statement (just in case there will be other sizes in the future...), > e.g. "Memory backing is only supported for 1 MiB huge pages" or > something similar. > > Thomas > That's actually the gist of it. s390 supports 4k, 1M and 2G and looking at the POP I don't see a big chance of having any more sizes, as there are not a lot of software bits left and adding support in OSs is a pain. Also don't hold your breath for 2G, it would need a rewrite of gmap.c. Also this is more explicit.
On 08/02/2018 08:53 AM, Janosch Frank wrote: > On 01.08.2018 18:47, Thomas Huth wrote: >> On 08/01/2018 03:19 PM, Janosch Frank wrote: >>> +static int kvm_s390_configure_mempath_backing(KVMState *s) >>> +{ >>> + size_t path_psize = qemu_mempath_getpagesize(mem_path); >>> + >>> + if (path_psize == 4 * KiB) >>> + return 0; >> >> Missing curly braces. > > urgh, right, this is not the kernel. > >> >>> + if (path_psize != 1 * MiB) { >>> + error_report("Memory backing with 2G pages was specified, " >>> + "but KVM does not support this memory backing"); >> >> May I suggest to use a text that rather matches the condition of the >> if-statement (just in case there will be other sizes in the future...), >> e.g. "Memory backing is only supported for 1 MiB huge pages" or >> something similar. >> >> Thomas >> > > That's actually the gist of it. s390 supports 4k, 1M and 2G and looking > at the POP I don't see a big chance of having any more sizes, as there > are not a lot of software bits left and adding support in OSs is a pain. > Also don't hold your breath for 2G, it would need a rewrite of gmap.c. Well, never say never ... but if you want to keep the current text, that's fine for me, too. Thomas
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c index d923cf4240..d1c72975cf 100644 --- a/target/s390x/kvm.c +++ b/target/s390x/kvm.c @@ -34,6 +34,8 @@ #include "qapi/error.h" #include "qemu/error-report.h" #include "qemu/timer.h" +#include "qemu/units.h" +#include "qemu/mmap-alloc.h" #include "sysemu/sysemu.h" #include "sysemu/hw_accel.h" #include "hw/hw.h" @@ -139,6 +141,7 @@ static int cap_mem_op; static int cap_s390_irq; static int cap_ri; static int cap_gs; +static int cap_hpage_1m; static int active_cmma; @@ -220,9 +223,9 @@ static void kvm_s390_enable_cmma(void) .attr = KVM_S390_VM_MEM_ENABLE_CMMA, }; - if (mem_path) { + if (cap_hpage_1m) { warn_report("CMM will not be enabled because it is not " - "compatible with hugetlbfs."); + "compatible with huge memory backings."); return; } rc = kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, &attr); @@ -281,10 +284,37 @@ void kvm_s390_crypto_reset(void) } } +static int kvm_s390_configure_mempath_backing(KVMState *s) +{ + size_t path_psize = qemu_mempath_getpagesize(mem_path); + + if (path_psize == 4 * KiB) + return 0; + + if (path_psize != 1 * MiB) { + error_report("Memory backing with 2G pages was specified, " + "but KVM does not support this memory backing"); + return -EINVAL; + } + + if (kvm_vm_enable_cap(s, KVM_CAP_S390_HPAGE_1M, 0)) { + error_report("Memory backing with 1M pages was specified, " + "but KVM does not support this memory backing"); + return -EINVAL; + } + + cap_hpage_1m = 1; + return 0; +} + int kvm_arch_init(MachineState *ms, KVMState *s) { MachineClass *mc = MACHINE_GET_CLASS(ms); + if (mem_path && kvm_s390_configure_mempath_backing(s)) { + return -EINVAL; + } + mc->default_cpu_type = S390_CPU_TYPE_NAME("host"); cap_sync_regs = kvm_check_extension(s, KVM_CAP_SYNC_REGS); cap_async_pf = kvm_check_extension(s, KVM_CAP_ASYNC_PF);
QEMU has had huge page support for a longer time already, but KVM memory management under s390x needed some changes to work with huge backings. Now that we have support, let's enable it if requested and available. Otherwise we now properly tell the user if there is no support and back out instead of failing to run the VM later on. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- target/s390x/kvm.c | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-)