Hi Shameer,
On 8/30/23 21:48, Shameer Kolothum wrote:
Now that we have Eager Page Split support added for ARM in the kernel,
enable it in Qemu. This adds,
-eager-split-size to -accel sub-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.
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@xxxxxxxxxx>
---
v2: https://lore.kernel.org/qemu-devel/20230815092709.1290-1-shameerali.kolothum.thodi@xxxxxxxxxx/
-Addressed comments from Gavin(Thanks).
RFC v1: https://lore.kernel.org/qemu-devel/20230725150002.621-1-shameerali.kolothum.thodi@xxxxxxxxxx/
-Updated qemu-options.hx with description
-Addressed review comments from Peter and Gavin(Thanks).
---
accel/kvm/kvm-all.c | 1 +
include/sysemu/kvm_int.h | 1 +
qemu-options.hx | 15 +++++++++
target/arm/kvm.c | 68 ++++++++++++++++++++++++++++++++++++++++
4 files changed, 85 insertions(+)
One more question below. Please check if it's worthy to be addressed in v4, needed
to resolved other comments. Otherwise, it looks fine to me.
Reviewed-by: Gavin Shan <gshan@xxxxxxxxxx>
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 2ba7521695..ff1578bb32 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -3763,6 +3763,7 @@ static void kvm_accel_instance_init(Object *obj)
/* KVM dirty ring is by default off */
s->kvm_dirty_ring_size = 0;
s->kvm_dirty_ring_with_bitmap = false;
+ s->kvm_eager_split_size = 0;
s->notify_vmexit = NOTIFY_VMEXIT_OPTION_RUN;
s->notify_window = 0;
s->xen_version = 0;
diff --git a/include/sysemu/kvm_int.h b/include/sysemu/kvm_int.h
index 511b42bde5..a5b9122cb8 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/qemu-options.hx b/qemu-options.hx
index 29b98c3d4c..2e70704ee8 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -186,6 +186,7 @@ DEF("accel", HAS_ARG, QEMU_OPTION_accel,
" split-wx=on|off (enable TCG split w^x mapping)\n"
" tb-size=n (TCG translation block cache size)\n"
" dirty-ring-size=n (KVM dirty ring GFN count, default 0)\n"
+ " eager-split-size=n (KVM Eager Page Split chunk size, default 0, disabled. ARM only)\n"
" notify-vmexit=run|internal-error|disable,notify-window=n (enable notify VM exit and set notify window, x86 only)\n"
" thread=single|multi (enable multi-threaded TCG)\n", QEMU_ARCH_ALL)
SRST
@@ -244,6 +245,20 @@ SRST
is disabled (dirty-ring-size=0). When enabled, KVM will instead
record dirty pages in a bitmap.
+ ``eager-split-size=n``
+ KVM implements dirty page logging at the PAGE_SIZE granularity and
+ enabling dirty-logging on a huge-page requires breaking it into
+ PAGE_SIZE pages in the first place. KVM on ARM does this splitting
+ lazily by default. There are performance benefits in doing huge-page
+ split eagerly, especially in situations where TLBI costs associated
+ with break-before-make sequences are considerable and also if guest
+ workloads are read intensive. The size here specifies how many pages
+ to break at a time and needs to be a valid block size which is
+ 1GB/2MB/4KB, 32MB/16KB and 512MB/64KB for 4KB/16KB/64KB PAGE_SIZE
+ respectively. Be wary of specifying a higher size as it will have an
+ impact on the memory. By default, this feature is disabled
+ (eager-split-size=0).
+
``notify-vmexit=run|internal-error|disable,notify-window=n``
Enables or disables notify VM exit support on x86 host and specify
the corresponding notify window to trigger the VM exit if enabled.
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 23aeb09949..28d81ca790 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,12 @@ int kvm_arm_get_max_vm_ipa_size(MachineState *ms, bool *fixed_ipa)
return ret > 0 ? ret : 40;
}
+static inline bool kvm_arm_eager_split_size_valid(uint64_t req_size,
+ uint32_t sizes)
+{
+ return req_size & sizes;
+}
+
int kvm_arch_get_default_type(MachineState *ms)
{
bool fixed_ipa;
@@ -287,6 +294,27 @@ 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) {
+ s->kvm_eager_split_size = 0;
+ warn_report("Eager Page Split support not available");
+ } else if (!kvm_arm_eager_split_size_valid(s->kvm_eager_split_size,
+ sizes)) {
+ error_report("Eager Page Split requested chunk size not valid");
+ ret = -EINVAL;
+ } else {
+ ret = kvm_vm_enable_cap(s, KVM_CAP_ARM_EAGER_SPLIT_CHUNK_SIZE, 0,
+ s->kvm_eager_split_size);
+ if (ret < 0) {
+ error_report("Enabling of Eager Page Split failed: %s",
+ strerror(-ret));
+ }
+ }
+ }
+
kvm_arm_init_debug(s);
return ret;
The function kvm_arm_eager_split_size_valid() was suggested by Peter if I'm correct.
However, it seems we needn't it any more since it's called for once. Why not simply to
have something like below? The detailed error message can help to explain why we
need the condition of (s->kvm_eager_split_size & sizes) here.
} else if (s->kvm_eager_split_size & sizes) {
error_report("Unsupported Eager Page Split chunk size 0x%lx by 0x%x",
s->kvm_eager_split_size, sizes);
ret = -EINVAL;
}
@@ -1069,6 +1097,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, "Unable to set early-split-size after KVM has been initialized");
+ return;
+ }
+
+ if (!visit_type_size(v, name, &value, errp)) {
+ return;
+ }
+
+ if (value && !is_power_of_2(value)) {
+ 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",
+ "Eager Page Split chunk size for hugepages. (default: 0, disabled)");
}
Thanks,
Gavin
