| Message ID | 20230725150002.621-1-shameerali.kolothum.thodi@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] arm/kvm: Enable support for KVM_CAP_ARM_EAGER_SPLIT_CHUNK_SIZE | expand |
On Tue, 25 Jul 2023 at 16:01, Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote: > > Now that we have Eager Page Split support added for ARM in the kernel[0], > enable it in Qemu. This adds, > -eager-split-size to Qemu options to set the eager page split chunk size. > -enable KVM_CAP_ARM_EAGER_SPLIT_CHUNK_SIZE. It looks from the code like you've added a new sub-option to -accel, not a new global option. This is the right thing, but your commit message should document the actual option syntax to avoid confusion. > The chunk size specifies how many pages to break at a time, using a > single allocation. Bigger the chunk size, more pages need to be > allocated ahead of time. > > Notes: > - I am not sure whether we need to call kvm_vm_check_extension() for > KVM_CAP_ARM_EAGER_SPLIT_CHUNK_SIZE or not as kernel seems to disable > eager page size by default and it will return zero always. > > -ToDo: Update qemu-options.hx > > [0]: https://lore.kernel.org/all/168426111477.3193133.10748106199843780930.b4-ty@linux.dev/ Speaking of confusion, this message says "It's an optimization used in Google Cloud since 2016 on x86, and for the last couple of months on ARM." so I'm not sure why we've ended up with an Arm-specific KVM_CAP and code in target/arm/kvm.c rather than something more generic ? If this is going to arrive for other architectures in the future we should probably think about whether some of this code should be generic, not arm-specific. Also this seems to be an obscure tuning parameter -- it could use good documentation so users have some idea when it can help. As a more specific case of that: the kernel patchset says it makes Arm do the same thing that x86 already does, and split the huge pages automatically based on use of the dirty log. If the kernel can do this automatically and we never felt the need to provide a manual tuning knob for x86, do we even need to expose the Arm manual control via QEMU? Other than that, I have a few minor coding things below. > +static bool kvm_arm_eager_split_size_valid(uint64_t req_size, uint32_t sizes) > +{ > + int i; > + > + for (i = 0; i < sizeof(uint32_t) * BITS_PER_BYTE; i++) { > + if (!(sizes & (1 << i))) { > + continue; > + } > + > + if (req_size == (1 << i)) { > + return true; > + } > + } We know req_size is a power of 2 here, so if you also explicitly rule out 0 then you can do return sizes & (1 << ctz64(req_size)); rather than having to loop through. (Need to rule out 0 because otherwise ctz64() returns 64 and the shift is UB.) > + > + return false; > +} > + > int kvm_arch_init(MachineState *ms, KVMState *s) > { > int ret = 0; > @@ -280,6 +298,21 @@ int kvm_arch_init(MachineState *ms, KVMState *s) > } > } > > + if (s->kvm_eager_split_size) { > + uint32_t sizes; > + > + sizes = kvm_vm_check_extension(s, KVM_CAP_ARM_SUPPORTED_BLOCK_SIZES); > + if (!sizes) { > + error_report("Eager Page Split not supported on host"); > + } else if (!kvm_arm_eager_split_size_valid(s->kvm_eager_split_size, > + sizes)) { > + error_report("Eager Page Split requested chunk size not valid"); > + } else if (kvm_vm_enable_cap(s, KVM_CAP_ARM_EAGER_SPLIT_CHUNK_SIZE, 0, > + s->kvm_eager_split_size)) { > + error_report("Failed to set Eager Page Split chunk size"); > + } > + } > + > kvm_arm_init_debug(s); > > return ret; > @@ -1062,6 +1095,46 @@ bool kvm_arch_cpu_check_are_resettable(void) > return true; > } > > +static void kvm_arch_get_eager_split_size(Object *obj, Visitor *v, > + const char *name, void *opaque, > + Error **errp) > +{ > + KVMState *s = KVM_STATE(obj); > + uint64_t value = s->kvm_eager_split_size; > + > + visit_type_size(v, name, &value, errp); > +} > + > +static void kvm_arch_set_eager_split_size(Object *obj, Visitor *v, > + const char *name, void *opaque, > + Error **errp) > +{ > + KVMState *s = KVM_STATE(obj); > + uint64_t value; > + > + if (s->fd != -1) { > + error_setg(errp, "Cannot set properties after the accelerator has been initialized"); > + return; > + } > + > + if (!visit_type_size(v, name, &value, errp)) { > + return; > + } > + > + if (value & (value - 1)) { "if (!is_power_of_2(value))" is a clearer way to write this. > + error_setg(errp, "early-split-size must be a power of two."); > + return; > + } > + > + s->kvm_eager_split_size = value; > +} > + > void kvm_arch_accel_class_init(ObjectClass *oc) > { > + object_class_property_add(oc, "eager-split-size", "size", > + kvm_arch_get_eager_split_size, > + kvm_arch_set_eager_split_size, NULL, NULL); > + > + object_class_property_set_description(oc, "eager-split-size", > + "Configure Eager Page Split chunk size for hugepages. (default: 0, disabled)"); > } > -- thanks -- PMM
On Thu, 27 Jul 2023 at 16:43, Peter Maydell <peter.maydell@linaro.org> wrote: > > On Tue, 25 Jul 2023 at 16:01, Shameer Kolothum > <shameerali.kolothum.thodi@huawei.com> wrote: > > +static bool kvm_arm_eager_split_size_valid(uint64_t req_size, uint32_t sizes) > > +{ > > + int i; > > + > > + for (i = 0; i < sizeof(uint32_t) * BITS_PER_BYTE; i++) { > > + if (!(sizes & (1 << i))) { > > + continue; > > + } > > + > > + if (req_size == (1 << i)) { > > + return true; > > + } > > + } > > We know req_size is a power of 2 here, so if you also explicitly > rule out 0 then you can do > return sizes & (1 << ctz64(req_size)); Er, that's also over-complicated. Just return sizes & req_size; should do (and catches the 0 case correctly again). -- PMM
> -----Original Message----- > From: Peter Maydell [mailto:peter.maydell@linaro.org] > Sent: 27 July 2023 16:43 > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> > Cc: qemu-devel@nongnu.org; qemu-arm@nongnu.org; ricarkol@google.com; > kvm@vger.kernel.org; Jonathan Cameron <jonathan.cameron@huawei.com>; > Linuxarm <linuxarm@huawei.com> > Subject: Re: [RFC PATCH] arm/kvm: Enable support for > KVM_CAP_ARM_EAGER_SPLIT_CHUNK_SIZE > > On Tue, 25 Jul 2023 at 16:01, Shameer Kolothum > <shameerali.kolothum.thodi@huawei.com> wrote: > > > > Now that we have Eager Page Split support added for ARM in the kernel[0], > > enable it in Qemu. This adds, > > -eager-split-size to Qemu options to set the eager page split chunk size. > > -enable KVM_CAP_ARM_EAGER_SPLIT_CHUNK_SIZE. > > It looks from the code like you've added a new sub-option > to -accel, not a new global option. This is the right thing, > but your commit message should document the actual option syntax > to avoid confusion. Ok. Will update the commit message. > > The chunk size specifies how many pages to break at a time, using a > > single allocation. Bigger the chunk size, more pages need to be > > allocated ahead of time. > > > > Notes: > > - I am not sure whether we need to call kvm_vm_check_extension() for > > KVM_CAP_ARM_EAGER_SPLIT_CHUNK_SIZE or not as kernel seems to > disable > > eager page size by default and it will return zero always. > > > > -ToDo: Update qemu-options.hx > > > > [0]: > https://lore.kernel.org/all/168426111477.3193133.1074810619984378093 > 0.b4-ty@linux.dev/ > > Speaking of confusion, this message says "It's an optimization used > in Google Cloud since 2016 on x86, and for the last couple of months > on ARM." so I'm not sure why we've ended up with an Arm-specific > KVM_CAP and code in target/arm/kvm.c rather than something more > generic ? > > If this is going to arrive for other architectures in the future > we should probably think about whether some of this code should > be generic, not arm-specific. > > Also this seems to be an obscure tuning parameter -- it could > use good documentation so users have some idea when it can help. > > As a more specific case of that: the kernel patchset says it > makes Arm do the same thing that x86 already does, and split > the huge pages automatically based on use of the dirty log. > If the kernel can do this automatically and we never felt > the need to provide a manual tuning knob for x86, do we even > need to expose the Arm manual control via QEMU? From the history of the above series, it looks like, the main argument for making this a user adjustable knob for ARM is because of the upfront extra memory allocations required in kernel associated with splitting the block page. https://lore.kernel.org/kvmarm/86v8ktkqfx.wl-maz@kernel.org/ https://lore.kernel.org/kvmarm/86h6w70zhc.wl-maz@kernel.org/ And the knob for x86 case is a kvm module_param(eager_page_split). Not clear to me why x86 opted for a module_param per KVM but not per VM user space one. The discussion can be found here, https://lore.kernel.org/kvm/YaDrmNVsXSMXR72Z@xz-m1.local/#t > Other than that, I have a few minor coding things below. > > > +static bool kvm_arm_eager_split_size_valid(uint64_t req_size, uint32_t > sizes) > > +{ > > + int i; > > + > > + for (i = 0; i < sizeof(uint32_t) * BITS_PER_BYTE; i++) { > > + if (!(sizes & (1 << i))) { > > + continue; > > + } > > + > > + if (req_size == (1 << i)) { > > + return true; > > + } > > + } > > We know req_size is a power of 2 here, so if you also explicitly > rule out 0 then you can do > return sizes & (1 << ctz64(req_size)); > rather than having to loop through. (Need to rule out 0 > because otherwise ctz64() returns 64 and the shift is UB.) Yes, missed that we already handled the != power of 2 case. Will update as per your next comment on this patch. That is much simpler. Thanks. > > > + > > + return false; > > +} > > + > > int kvm_arch_init(MachineState *ms, KVMState *s) > > { > > int ret = 0; > > @@ -280,6 +298,21 @@ int kvm_arch_init(MachineState *ms, KVMState > *s) > > } > > } > > > > + if (s->kvm_eager_split_size) { > > + uint32_t sizes; > > + > > + sizes = kvm_vm_check_extension(s, > KVM_CAP_ARM_SUPPORTED_BLOCK_SIZES); > > + if (!sizes) { > > + error_report("Eager Page Split not supported on host"); > > + } else if > (!kvm_arm_eager_split_size_valid(s->kvm_eager_split_size, > > + sizes)) { > > + error_report("Eager Page Split requested chunk size not > valid"); > > + } else if (kvm_vm_enable_cap(s, > KVM_CAP_ARM_EAGER_SPLIT_CHUNK_SIZE, 0, > > + s->kvm_eager_split_size)) { > > + error_report("Failed to set Eager Page Split chunk size"); > > + } > > + } > > + > > kvm_arm_init_debug(s); > > > > return ret; > > @@ -1062,6 +1095,46 @@ bool > kvm_arch_cpu_check_are_resettable(void) > > return true; > > } > > > > +static void kvm_arch_get_eager_split_size(Object *obj, Visitor *v, > > + const char *name, void > *opaque, > > + Error **errp) > > +{ > > + KVMState *s = KVM_STATE(obj); > > + uint64_t value = s->kvm_eager_split_size; > > + > > + visit_type_size(v, name, &value, errp); > > +} > > + > > +static void kvm_arch_set_eager_split_size(Object *obj, Visitor *v, > > + const char *name, void > *opaque, > > + Error **errp) > > +{ > > + KVMState *s = KVM_STATE(obj); > > + uint64_t value; > > + > > + if (s->fd != -1) { > > + error_setg(errp, "Cannot set properties after the accelerator has > been initialized"); > > + return; > > + } > > + > > + if (!visit_type_size(v, name, &value, errp)) { > > + return; > > + } > > + > > + if (value & (value - 1)) { > > "if (!is_power_of_2(value))" is a clearer way to write this. Ok. Will update in next. Thanks for taking a look and sorry for late reply, was away. Shameer > > > + error_setg(errp, "early-split-size must be a power of two."); > > + return; > > + } > > + > > + s->kvm_eager_split_size = value; > > +} > > + > > void kvm_arch_accel_class_init(ObjectClass *oc) > > { > > + object_class_property_add(oc, "eager-split-size", "size", > > + kvm_arch_get_eager_split_size, > > + kvm_arch_set_eager_split_size, NULL, > NULL); > > + > > + object_class_property_set_description(oc, "eager-split-size", > > + "Configure Eager Page Split chunk size for hugepages. (default: 0, > disabled)"); > > } > > -- > > thanks > -- PMM
On 7/26/23 01:00, Shameer Kolothum wrote: > Now that we have Eager Page Split support added for ARM in the kernel[0], > enable it in Qemu. This adds, > -eager-split-size to Qemu options to set the eager page split chunk size. > -enable KVM_CAP_ARM_EAGER_SPLIT_CHUNK_SIZE. > > The chunk size specifies how many pages to break at a time, using a > single allocation. Bigger the chunk size, more pages need to be > allocated ahead of time. > > Notes: > - I am not sure whether we need to call kvm_vm_check_extension() for > KVM_CAP_ARM_EAGER_SPLIT_CHUNK_SIZE or not as kernel seems to disable > eager page size by default and it will return zero always. > > -ToDo: Update qemu-options.hx > > [0]: https://lore.kernel.org/all/168426111477.3193133.10748106199843780930.b4-ty@linux.dev/ > > Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> > --- > include/sysemu/kvm_int.h | 1 + > target/arm/kvm.c | 73 ++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 74 insertions(+) > > diff --git a/include/sysemu/kvm_int.h b/include/sysemu/kvm_int.h > index 511b42bde5..03a1660d40 100644 > --- a/include/sysemu/kvm_int.h > +++ b/include/sysemu/kvm_int.h > @@ -116,6 +116,7 @@ struct KVMState > uint64_t kvm_dirty_ring_bytes; /* Size of the per-vcpu dirty ring */ > uint32_t kvm_dirty_ring_size; /* Number of dirty GFNs per ring */ > bool kvm_dirty_ring_with_bitmap; > + uint64_t kvm_eager_split_size; /* Eager Page Splitting chunk size */ > struct KVMDirtyRingReaper reaper; > NotifyVmexitOption notify_vmexit; > uint32_t notify_window; > diff --git a/target/arm/kvm.c b/target/arm/kvm.c > index b4c7654f49..985d901062 100644 > --- a/target/arm/kvm.c > +++ b/target/arm/kvm.c > @@ -30,6 +30,7 @@ > #include "exec/address-spaces.h" > #include "hw/boards.h" > #include "hw/irq.h" > +#include "qapi/visitor.h" > #include "qemu/log.h" > > const KVMCapabilityInfo kvm_arch_required_capabilities[] = { > @@ -247,6 +248,23 @@ int kvm_arm_get_max_vm_ipa_size(MachineState *ms, bool *fixed_ipa) > return ret > 0 ? ret : 40; > } > > +static bool kvm_arm_eager_split_size_valid(uint64_t req_size, uint32_t sizes) > +{ > + int i; > + > + for (i = 0; i < sizeof(uint32_t) * BITS_PER_BYTE; i++) { > + if (!(sizes & (1 << i))) { > + continue; > + } > + > + if (req_size == (1 << i)) { > + return true; > + } > + } > + > + return false; > +} > + > int kvm_arch_init(MachineState *ms, KVMState *s) > { > int ret = 0; > @@ -280,6 +298,21 @@ int kvm_arch_init(MachineState *ms, KVMState *s) > } > } > > + if (s->kvm_eager_split_size) { > + uint32_t sizes; > + > + sizes = kvm_vm_check_extension(s, KVM_CAP_ARM_SUPPORTED_BLOCK_SIZES); > + if (!sizes) { > + error_report("Eager Page Split not supported on host"); > + } else if (!kvm_arm_eager_split_size_valid(s->kvm_eager_split_size, > + sizes)) { > + error_report("Eager Page Split requested chunk size not valid"); > + } else if (kvm_vm_enable_cap(s, KVM_CAP_ARM_EAGER_SPLIT_CHUNK_SIZE, 0, > + s->kvm_eager_split_size)) { > + error_report("Failed to set Eager Page Split chunk size"); > + } > + } > + > kvm_arm_init_debug(s); > > return ret; Do we really want to fail when KVM_CAP_ARM_SUPPORTED_BLOCK_SIZES isn't supported? I think the appropriate behavior is to warn and clear s->kvm_eager_split_size for this specific case, similar to what we are doing for s->kvm_dirty_ring_size in kvm_dirty_ring_init(). With this, the behavior is backwards compatible to the old host kernels. > @@ -1062,6 +1095,46 @@ bool kvm_arch_cpu_check_are_resettable(void) > return true; > } > > +static void kvm_arch_get_eager_split_size(Object *obj, Visitor *v, > + const char *name, void *opaque, > + Error **errp) > +{ > + KVMState *s = KVM_STATE(obj); > + uint64_t value = s->kvm_eager_split_size; > + > + visit_type_size(v, name, &value, errp); > +} > + > +static void kvm_arch_set_eager_split_size(Object *obj, Visitor *v, > + const char *name, void *opaque, > + Error **errp) > +{ > + KVMState *s = KVM_STATE(obj); > + uint64_t value; > + > + if (s->fd != -1) { > + error_setg(errp, "Cannot set properties after the accelerator has been initialized"); > + return; > + } > + > + if (!visit_type_size(v, name, &value, errp)) { > + return; > + } > + > + if (value & (value - 1)) { > + error_setg(errp, "early-split-size must be a power of two."); > + return; > + } > + > + s->kvm_eager_split_size = value; > +} > + > void kvm_arch_accel_class_init(ObjectClass *oc) > { > + object_class_property_add(oc, "eager-split-size", "size", > + kvm_arch_get_eager_split_size, > + kvm_arch_set_eager_split_size, NULL, NULL); > + > + object_class_property_set_description(oc, "eager-split-size", > + "Configure Eager Page Split chunk size for hugepages. (default: 0, disabled)"); > } Thanks, Gavin
> -----Original Message----- > From: Gavin Shan [mailto:gshan@redhat.com] > Sent: 07 August 2023 06:53 > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>; > qemu-devel@nongnu.org; qemu-arm@nongnu.org > Cc: peter.maydell@linaro.org; ricarkol@google.com; kvm@vger.kernel.org; > Jonathan Cameron <jonathan.cameron@huawei.com>; Linuxarm > <linuxarm@huawei.com> > Subject: Re: [RFC PATCH] arm/kvm: Enable support for > KVM_CAP_ARM_EAGER_SPLIT_CHUNK_SIZE > > > On 7/26/23 01:00, Shameer Kolothum wrote: > > Now that we have Eager Page Split support added for ARM in the kernel[0], > > enable it in Qemu. This adds, > > -eager-split-size to Qemu options to set the eager page split chunk size. > > -enable KVM_CAP_ARM_EAGER_SPLIT_CHUNK_SIZE. > > > > The chunk size specifies how many pages to break at a time, using a > > single allocation. Bigger the chunk size, more pages need to be > > allocated ahead of time. > > > > Notes: > > - I am not sure whether we need to call kvm_vm_check_extension() for > > KVM_CAP_ARM_EAGER_SPLIT_CHUNK_SIZE or not as kernel seems to > disable > > eager page size by default and it will return zero always. > > > > -ToDo: Update qemu-options.hx > > > > [0]: > https://lore.kernel.org/all/168426111477.3193133.1074810619984378093 > 0.b4-ty@linux.dev/ > > > > Signed-off-by: Shameer Kolothum > <shameerali.kolothum.thodi@huawei.com> > > --- > > include/sysemu/kvm_int.h | 1 + > > target/arm/kvm.c | 73 > ++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 74 insertions(+) > > > > diff --git a/include/sysemu/kvm_int.h b/include/sysemu/kvm_int.h > > index 511b42bde5..03a1660d40 100644 > > --- a/include/sysemu/kvm_int.h > > +++ b/include/sysemu/kvm_int.h > > @@ -116,6 +116,7 @@ struct KVMState > > uint64_t kvm_dirty_ring_bytes; /* Size of the per-vcpu dirty ring > */ > > uint32_t kvm_dirty_ring_size; /* Number of dirty GFNs per ring > */ > > bool kvm_dirty_ring_with_bitmap; > > + uint64_t kvm_eager_split_size; /* Eager Page Splitting chunk size */ > > struct KVMDirtyRingReaper reaper; > > NotifyVmexitOption notify_vmexit; > > uint32_t notify_window; > > diff --git a/target/arm/kvm.c b/target/arm/kvm.c > > index b4c7654f49..985d901062 100644 > > --- a/target/arm/kvm.c > > +++ b/target/arm/kvm.c > > @@ -30,6 +30,7 @@ > > #include "exec/address-spaces.h" > > #include "hw/boards.h" > > #include "hw/irq.h" > > +#include "qapi/visitor.h" > > #include "qemu/log.h" > > > > const KVMCapabilityInfo kvm_arch_required_capabilities[] = { > > @@ -247,6 +248,23 @@ int > kvm_arm_get_max_vm_ipa_size(MachineState *ms, bool *fixed_ipa) > > return ret > 0 ? ret : 40; > > } > > > > +static bool kvm_arm_eager_split_size_valid(uint64_t req_size, uint32_t > sizes) > > +{ > > + int i; > > + > > + for (i = 0; i < sizeof(uint32_t) * BITS_PER_BYTE; i++) { > > + if (!(sizes & (1 << i))) { > > + continue; > > + } > > + > > + if (req_size == (1 << i)) { > > + return true; > > + } > > + } > > + > > + return false; > > +} > > + > > int kvm_arch_init(MachineState *ms, KVMState *s) > > { > > int ret = 0; > > @@ -280,6 +298,21 @@ int kvm_arch_init(MachineState *ms, KVMState > *s) > > } > > } > > > > + if (s->kvm_eager_split_size) { > > + uint32_t sizes; > > + > > + sizes = kvm_vm_check_extension(s, > KVM_CAP_ARM_SUPPORTED_BLOCK_SIZES); > > + if (!sizes) { > > + error_report("Eager Page Split not supported on host"); > > + } else if > (!kvm_arm_eager_split_size_valid(s->kvm_eager_split_size, > > + sizes)) { > > + error_report("Eager Page Split requested chunk size not > valid"); > > + } else if (kvm_vm_enable_cap(s, > KVM_CAP_ARM_EAGER_SPLIT_CHUNK_SIZE, 0, > > + s->kvm_eager_split_size)) { > > + error_report("Failed to set Eager Page Split chunk size"); > > + } > > + } > > + > > kvm_arm_init_debug(s); > > > > return ret; > > Do we really want to fail when KVM_CAP_ARM_SUPPORTED_BLOCK_SIZES > isn't supported? > I think the appropriate behavior is to warn and clear s->kvm_eager_split_size > for this specific case, similar to what we are doing for s->kvm_dirty_ring_size > in kvm_dirty_ring_init(). With this, the behavior is backwards compatible to > the > old host kernels. Ok. That makes sense. Will update. Thanks, Shameer > > > @@ -1062,6 +1095,46 @@ bool > kvm_arch_cpu_check_are_resettable(void) > > return true; > > } > > > > +static void kvm_arch_get_eager_split_size(Object *obj, Visitor *v, > > + const char *name, void > *opaque, > > + Error **errp) > > +{ > > + KVMState *s = KVM_STATE(obj); > > + uint64_t value = s->kvm_eager_split_size; > > + > > + visit_type_size(v, name, &value, errp); > > +} > > + > > +static void kvm_arch_set_eager_split_size(Object *obj, Visitor *v, > > + const char *name, void > *opaque, > > + Error **errp) > > +{ > > + KVMState *s = KVM_STATE(obj); > > + uint64_t value; > > + > > + if (s->fd != -1) { > > + error_setg(errp, "Cannot set properties after the accelerator has > been initialized"); > > + return; > > + } > > + > > + if (!visit_type_size(v, name, &value, errp)) { > > + return; > > + } > > + > > + if (value & (value - 1)) { > > + error_setg(errp, "early-split-size must be a power of two."); > > + return; > > + } > > + > > + s->kvm_eager_split_size = value; > > +} > > + > > void kvm_arch_accel_class_init(ObjectClass *oc) > > { > > + object_class_property_add(oc, "eager-split-size", "size", > > + kvm_arch_get_eager_split_size, > > + kvm_arch_set_eager_split_size, NULL, > NULL); > > + > > + object_class_property_set_description(oc, "eager-split-size", > > + "Configure Eager Page Split chunk size for hugepages. (default: 0, > disabled)"); > > } > > Thanks, > Gavin
diff --git a/include/sysemu/kvm_int.h b/include/sysemu/kvm_int.h index 511b42bde5..03a1660d40 100644 --- a/include/sysemu/kvm_int.h +++ b/include/sysemu/kvm_int.h @@ -116,6 +116,7 @@ struct KVMState uint64_t kvm_dirty_ring_bytes; /* Size of the per-vcpu dirty ring */ uint32_t kvm_dirty_ring_size; /* Number of dirty GFNs per ring */ bool kvm_dirty_ring_with_bitmap; + uint64_t kvm_eager_split_size; /* Eager Page Splitting chunk size */ struct KVMDirtyRingReaper reaper; NotifyVmexitOption notify_vmexit; uint32_t notify_window; diff --git a/target/arm/kvm.c b/target/arm/kvm.c index b4c7654f49..985d901062 100644 --- a/target/arm/kvm.c +++ b/target/arm/kvm.c @@ -30,6 +30,7 @@ #include "exec/address-spaces.h" #include "hw/boards.h" #include "hw/irq.h" +#include "qapi/visitor.h" #include "qemu/log.h" const KVMCapabilityInfo kvm_arch_required_capabilities[] = { @@ -247,6 +248,23 @@ int kvm_arm_get_max_vm_ipa_size(MachineState *ms, bool *fixed_ipa) return ret > 0 ? ret : 40; } +static bool kvm_arm_eager_split_size_valid(uint64_t req_size, uint32_t sizes) +{ + int i; + + for (i = 0; i < sizeof(uint32_t) * BITS_PER_BYTE; i++) { + if (!(sizes & (1 << i))) { + continue; + } + + if (req_size == (1 << i)) { + return true; + } + } + + return false; +} + int kvm_arch_init(MachineState *ms, KVMState *s) { int ret = 0; @@ -280,6 +298,21 @@ int kvm_arch_init(MachineState *ms, KVMState *s) } } + if (s->kvm_eager_split_size) { + uint32_t sizes; + + sizes = kvm_vm_check_extension(s, KVM_CAP_ARM_SUPPORTED_BLOCK_SIZES); + if (!sizes) { + error_report("Eager Page Split not supported on host"); + } else if (!kvm_arm_eager_split_size_valid(s->kvm_eager_split_size, + sizes)) { + error_report("Eager Page Split requested chunk size not valid"); + } else if (kvm_vm_enable_cap(s, KVM_CAP_ARM_EAGER_SPLIT_CHUNK_SIZE, 0, + s->kvm_eager_split_size)) { + error_report("Failed to set Eager Page Split chunk size"); + } + } + kvm_arm_init_debug(s); return ret; @@ -1062,6 +1095,46 @@ bool kvm_arch_cpu_check_are_resettable(void) return true; } +static void kvm_arch_get_eager_split_size(Object *obj, Visitor *v, + const char *name, void *opaque, + Error **errp) +{ + KVMState *s = KVM_STATE(obj); + uint64_t value = s->kvm_eager_split_size; + + visit_type_size(v, name, &value, errp); +} + +static void kvm_arch_set_eager_split_size(Object *obj, Visitor *v, + const char *name, void *opaque, + Error **errp) +{ + KVMState *s = KVM_STATE(obj); + uint64_t value; + + if (s->fd != -1) { + error_setg(errp, "Cannot set properties after the accelerator has been initialized"); + return; + } + + if (!visit_type_size(v, name, &value, errp)) { + return; + } + + if (value & (value - 1)) { + error_setg(errp, "early-split-size must be a power of two."); + return; + } + + s->kvm_eager_split_size = value; +} + void kvm_arch_accel_class_init(ObjectClass *oc) { + object_class_property_add(oc, "eager-split-size", "size", + kvm_arch_get_eager_split_size, + kvm_arch_set_eager_split_size, NULL, NULL); + + object_class_property_set_description(oc, "eager-split-size", + "Configure Eager Page Split chunk size for hugepages. (default: 0, disabled)"); }
Now that we have Eager Page Split support added for ARM in the kernel[0], enable it in Qemu. This adds, -eager-split-size to Qemu options to set the eager page split chunk size. -enable KVM_CAP_ARM_EAGER_SPLIT_CHUNK_SIZE. The chunk size specifies how many pages to break at a time, using a single allocation. Bigger the chunk size, more pages need to be allocated ahead of time. Notes: - I am not sure whether we need to call kvm_vm_check_extension() for KVM_CAP_ARM_EAGER_SPLIT_CHUNK_SIZE or not as kernel seems to disable eager page size by default and it will return zero always. -ToDo: Update qemu-options.hx [0]: https://lore.kernel.org/all/168426111477.3193133.10748106199843780930.b4-ty@linux.dev/ Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> --- include/sysemu/kvm_int.h | 1 + target/arm/kvm.c | 73 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+)