> -----Original Message----- > From: Gavin Shan [mailto:gshan@xxxxxxxxxx] > Sent: 07 August 2023 06:53 > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@xxxxxxxxxx>; > qemu-devel@xxxxxxxxxx; qemu-arm@xxxxxxxxxx > Cc: peter.maydell@xxxxxxxxxx; ricarkol@xxxxxxxxxx; kvm@xxxxxxxxxxxxxxx; > Jonathan Cameron <jonathan.cameron@xxxxxxxxxx>; Linuxarm > <linuxarm@xxxxxxxxxx> > 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@xxxxxxxxx/ > > > > Signed-off-by: Shameer Kolothum > <shameerali.kolothum.thodi@xxxxxxxxxx> > > --- > > 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
