Re: [PATCH v4 6/6] KVM: arm64: Add auto HDBSS enable/disable on dirty logging change
From: Tian Zheng
Date: Fri Jul 17 2026 - 03:24:52 EST
On 7/14/2026 7:16 PM, Leonardo Bras wrote:
On Tue, Jul 14, 2026 at 04:58:37PM +0800, Tian Zheng wrote:
On 7/13/2026 10:50 PM, Leonardo Bras wrote:
On Thu, Jul 09, 2026 at 06:40:26PM +0800, Tian Zheng wrote:
From: eillon <yezhenyu2@xxxxxxxxxx>chunk_size != PAGE_SIZE now, but that should change as well :)
HDBSS buffers store per-page dirty state after the stage-2 page tables
have been split down to page granularity (chunk_size == PAGE_SIZE).
Thanks, I'll clarify the comment in v5.
By the discussion we are having in the HACDBS patchset, I think we can't
assume the pages are split in the future. :\
Agreed.
When chunk_size == 0 the kernel may lazily skip splitting block mappings,(See cover letter reply)
leaving the page table coarser than what HDBSS expects. Therefore,
enabling HDBSS requires disabling lazy split so that all block mappings
are eagerly broken down before the buffer starts recording.
Add VM-level HDBSS enable/disable support. When dirty logging isIIUC you are counting the amount of pages the VM has, and based on that
enabled on any memslot, HDBSS is automatically enabled. When dirty
logging is disabled on all memslots, HDBSS is automatically disabled.
This includes:
- kvm_arm_enable_hdbss_global() to enable HDBSS for all vCPUs
- kvm_arm_disable_hdbss_global() to disable and free HDBSS buffers
- kvm_arm_hdbss_on_dirty_logging_change() for auto enable/disable
- kvm_arch_destroy_vm() cleanup path
- kvm_arch_commit_memory_region() integration
Signed-off-by: Eillon <yezhenyu2@xxxxxxxxxx>
Signed-off-by: Tian Zheng <zhengtian10@xxxxxxxxxx>
---
arch/arm64/include/asm/kvm_dirty_bit.h | 2 +
arch/arm64/kvm/arm.c | 8 ++
arch/arm64/kvm/dirty_bit.c | 105 +++++++++++++++++++++++++
arch/arm64/kvm/mmu.c | 3 +
4 files changed, 118 insertions(+)
diff --git a/arch/arm64/include/asm/kvm_dirty_bit.h b/arch/arm64/include/asm/kvm_dirty_bit.h
index 4b28000e972f..a4cda8cdab24 100644
--- a/arch/arm64/include/asm/kvm_dirty_bit.h
+++ b/arch/arm64/include/asm/kvm_dirty_bit.h
@@ -23,5 +23,7 @@ int kvm_arm_vcpu_alloc_hdbss(struct kvm_vcpu *vcpu, unsigned int order);
void kvm_arm_vcpu_free_hdbss(struct kvm_vcpu *vcpu);
void kvm_flush_hdbss_buffer(struct kvm_vcpu *vcpu);
int kvm_handle_hdbss_fault(struct kvm_vcpu *vcpu);
+void kvm_arm_hdbss_on_dirty_logging_change(struct kvm *kvm, int nr_memslots_logging);
+void kvm_arm_disable_hdbss_global(struct kvm *kvm);
#endif /* __ARM64_KVM_DIRTY_BIT_H__ */
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 566953a4e23a..536d94799ba8 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -317,6 +317,14 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
if (is_protected_kvm_enabled())
pkvm_destroy_hyp_vm(kvm);
+ /*
+ * Userspace may destroy the VM without disabling dirty logging,
+ * so the auto-disable path is never reached. Force disable HDBSS
+ * here to ensure vCPU buffers are freed and prevent memory leaks.
+ */
+ if (kvm->arch.enable_hdbss)
+ kvm_arm_disable_hdbss_global(kvm);
+
kvm_uninit_stage2_mmu(kvm);
kvm_destroy_mpidr_data(kvm);
diff --git a/arch/arm64/kvm/dirty_bit.c b/arch/arm64/kvm/dirty_bit.c
index 002366337637..c5bf866c23ef 100644
--- a/arch/arm64/kvm/dirty_bit.c
+++ b/arch/arm64/kvm/dirty_bit.c
@@ -112,3 +112,108 @@ int kvm_handle_hdbss_fault(struct kvm_vcpu *vcpu)
return -EFAULT;
}
}
+
+static unsigned int hdbss_auto_select_order(struct kvm *kvm)
+{
+ unsigned long npages = 0;
+ struct kvm_memory_slot *memslot;
+ int bkt;
+
+ kvm_for_each_memslot(memslot, bkt, kvm_memslots(kvm))
+ npages += memslot->npages;
+
+ if (npages <= 16384)
+ return 0;
+ else if (npages <= 262144)
+ return 3;
+ else if (npages <= 4194304)
+ return 6;
+ else
+ return 9;
+}
allocating a size for the HDBSS buffer.
A few notes here:
- It's not really nice to use magic numbers around like this. If you
actually want to use it, then use stuff like SZ_16K, SZ_256K, SZ_4M and
so on.
- You are returning magic numbers as well, why is it 0, 3, 6, or 9 here?
It only makes sense if the person is reading HDBSSBR_EL2 documentation,
which should not be necessary at this point. That's one reason I
recommended to using sizes. If that was really the best way to use it,
I would recommend using the defines that we get from sysreg, and you
actually used before to set the maximum order on a previous patch.
- Also, if you can return only valid values here, why do you check against
the maximum value in that previous patch?
- Also, are you using some undisclosed rule here? On 'order 0' the
meanining is 4KB, which translate to 512 HDBSS entries. Why are you using
it for any value under 16K? Same for 3-32KB-4kEntries you use for under
256K pages (and so on). If you are assuming a logical rule such as
'N pages would be ok with N/32 entries' it has to be described here at
least.
- Not sure VM size is the best way of doing that, since it will depend
more on the dirtying rate than the actual size, and most VMs would just
use the biggest size (4M x 4K pages is just 16GB). For instance with
dirty_ring we can use the dirty_ring.size as a better option.
(I know this is a hard one to estimate when using dirty-bitmap, though)
I'll replace the magic numbers and also add a comment in the next version
explaining
the mapping between the thresholds and the order values.
On auto-choosing the size: VM memory size is a simple starting point, but I
agree it's
not ideal. For dirty-ring mode we could use dirty_ring_size as a reference;
for dirty-bitmap
mode there's no equivalent, so I don't have a good answer yet. I'd really
appreciate any
suggestions from the community on a better idea for the dirty-bitmap case.
I am thinking that we could use a default value (say 1 PAGESIZE/vcpu) and
add an ioctl to optionally increase this value. This way we don't require
a new interface to benefit from HDBSS, but allow users to tune it.
That makes sense. I think we can use a default value (e.g., 1 page per
vCPU), but I'd prefer not to expose any ioctl for userspace to configure
it. Since the kernel auto-enables HDBSS, a userspace size knob would be
confusing — similar to what I mentioned in the discussion at [1].
[1] https://lore.kernel.org/linux-arm-kernel/3fb4b33e-4618-4523-b140-955e15fd9a8c@xxxxxxxxxx/
Also, since we automatically enable HDBSS in the kernel, the check against
HDBSS_MAX_ORDER is redundant. I'll remove it.
+Okay, say the user requested it to be disabled, you change the global vtcr,
+/*
+ * Enable HDBSS for all vCPUs in the VM.
+ *
+ * Called from kvm_arm_hdbss_on_dirty_logging_change() which is invoked
+ * by kvm_arch_commit_memory_region() under kvm->slots_lock.
+ *
+ * If buffer allocation fails, HDBSS remains disabled and dirty tracking
+ * falls back to the traditional software-based approach (PTE write-protect
+ * + software dirty marking). This does not affect correctness; dirty
+ * logging remains functional without HDBSS.
+ */
+static int kvm_arm_enable_hdbss_global(struct kvm *kvm)
+{
+ int err;
+ unsigned long i;
+ unsigned int order;
+ struct kvm_vcpu *vcpu;
+
+ if (!system_supports_hdbss())
+ return 0;
+
+ if (kvm->dirty_ring_size) /* Don't support HDBSS in dirty ring mode */
+ return 0;
+
+ if (kvm->arch.enable_hdbss) /* Already On */
+ return 0;
+
+ /* Turn it on */
+ order = hdbss_auto_select_order(kvm);
+ kvm_for_each_vcpu(i, vcpu, kvm) {
+ err = kvm_arm_vcpu_alloc_hdbss(vcpu, order);
+ if (err)
+ goto error_alloc;
+ }
+
+ kvm->arch.enable_hdbss = true;
+ kvm->arch.mmu.vtcr |= VTCR_EL2_HD | VTCR_EL2_HDBSS | VTCR_EL2_HA;
+
+ /*
+ * We should kick vcpus out of guest mode here to load new
+ * vtcr value to vtcr_el2 register when re-enter guest mode.
+ */
+ kvm_for_each_vcpu(i, vcpu, kvm)
+ kvm_vcpu_kick(vcpu);
+
+ return 0;
+
+error_alloc:
+ kvm_for_each_vcpu(i, vcpu, kvm)
+ if (vcpu->arch.hdbss.base_phys)
+ kvm_arm_vcpu_free_hdbss(vcpu);
+
+ pr_warn_once("kvm: failed to allocate HDBSS buffers (order=%u), "
+ "falling back to software dirty tracking\n", order);
+ return -ENOMEM;
+}
+
+void kvm_arm_disable_hdbss_global(struct kvm *kvm)
+{
+ unsigned long i;
+ struct kvm_vcpu *vcpu;
+
+ if (!kvm->arch.enable_hdbss) /* Already Off */
+ return;
+
+ /* Turn it off */
+ kvm->arch.mmu.vtcr &= ~(VTCR_EL2_HD | VTCR_EL2_HDBSS | VTCR_EL2_HA);
+
+ kvm_for_each_vcpu(i, vcpu, kvm)
+ kvm_arm_vcpu_free_hdbss(vcpu);
+
+ kvm->arch.enable_hdbss = false;
+}
+
then free the hdbss on every vcpu.
But the vcpus are still running, and since they will only disable this when
they go out of the guest, then in again, HDBSS will still be running,
right?
If some page gets dirty in the between, would not the HDBSS try to write to
the already loaded buffer adress, and write to memory that have already
been freed here?
You're right — this is a race condition. I'll fix this in v5 by clearing
VTCR_EL2_HDBSS from
kvm->arch.mmu.vtcr first, then kicking all vCPUs to force them to exit guest
mode and reload the config.
Once all vCPUs are out of guest mode, it will be safe to free the HDBSS
buffers.
That would be safer, indeed.
+void kvm_arm_hdbss_on_dirty_logging_change(struct kvm *kvm, int nr_memslots_logging)Okay, reading the above I remembered something really complicated:
+{
+ /*
+ * Called from kvm_arch_commit_memory_region() under kvm->slots_lock.
+ * All state transitions are serialized by slots_lock.
+ */
+ if (nr_memslots_logging > 0 && !kvm->arch.enable_hdbss)
+ kvm_arm_enable_hdbss_global(kvm);
+ else if (nr_memslots_logging == 0 && kvm->arch.enable_hdbss)
+ kvm_arm_disable_hdbss_global(kvm);
+}
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 949fb895add6..484f48dae000 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -2588,6 +2588,9 @@ void kvm_arch_commit_memory_region(struct kvm *kvm,
{
bool log_dirty_pages = new && new->flags & KVM_MEM_LOG_DIRTY_PAGES;
+ kvm_arm_hdbss_on_dirty_logging_change(kvm,
+ atomic_read(&kvm->nr_memslots_dirty_logging));
+
/*
* At this point memslot has been committed and there is an
* allocated dirty_bitmap[], dirty pages will be tracked while the
--
2.33.0
We can't really enable HDBSS partially if we start with DBM set for all
pages. Once we enable HDBSS wit will track changes for all memslots.
The only way to enable it partially would be to set DBM during the
dirty-bit tracking, which I recall being complicated for some reasons.
Well, we have to think about the overall strategy before a next version.
Thanks!
Leo
Yes, I did consider this when I switched to global DBM injection in v4.
There are
indeed some scenarios that are harder to control:
Firstly, for lazy split, if we add the DBM tag lazily during live migration
dirty tracking (like v3 did),
the first write to each page would trap. That trap serves two purposes: it
gives us a chance to split
hugepages on demand (lazy split), and it ensures the DBM addition happens at
a safer, more controlled point.
It also allows us to do the dirty-tracking by slot, which is not possible
with the v4 approach.
I think the per-memslot tracking capability is not a critical
requirement. As I mentioned in my earlier reply [1], we can follow the
same approach as PML on x86 — filtering at flush time works just fine.
[1] https://lore.kernel.org/linux-arm-kernel/a240e7a6-cd22-4fe4-a01d-472c95cc65e4@xxxxxxxxxx/
However, because v4 enables HDBSS and DBM upfront, we lose that initial
trap. That's exactly why we
now rely on your eager hugepage splitting patch as a mandatory dependency.
Correct.
Secondly, I'm also concerned about whether global DBM injection could
accidentally mark pages
that shouldn't be tracked — for example, pages with special mappings.
Well, if we want to not track those pages, we have just to make sure we can
detect them and not mark them with the DBM bit.
If
that's possible, then the
lazy approach (only adding DBM on the first write fault) would be safer
because it only touches pages
that are actually written to.
So I'd like to ask: is avoiding the first-trap overhead worth the potential
risks of global DBM injection?
Or do you think the lazy approach is actually safer overall? I'd appreciate
your thoughts on this trade-off.
Well, even though performance is important, the decision to set DBM bits
for all writtable pages at their mapping time was not driven by
performance, but instead by an issue with setting DBM while the VCPUs were
running. I have to rework what that was, and check if that is still an
issue, before we can even discuss what to do next :(
But the fact that the 'eager DBM setting' makes dirty-bit tracking start
global, instead of per-memslot, is something we have to consider as well.
Thanks!
Leo
That makes sense. I think global eager DBM is workable.
Thanks!
Tian