Re: [PATCH v5 08/15] KVM: arm64: Flush the HDBSS buffer on VM exit

[Tian Zheng <zhengtian10@huawei.com>]

On 10/31/2026 12:52 AM, Leonardo Bras wrote:
> On Fri, Oct 30, 2026 at 12:57:06PM +0800, Tian Zheng wrote:
>>
>>
>> On 10/28/2026 2:13 AM, Leonardo Bras wrote:
>>> On Tue, Sep 29, 2026 at 06:36:48PM +0800, Tian Zheng wrote:
>>>> From: Eillon <yezhenyu2@huawei.com>
>>>>
>>>> HDBSS entries accumulate in the per-vCPU buffer while the guest runs,
>>>> and must be drained into the dirty bitmap or dirty ring before
>>>> userspace can observe them.
>>>>
>>>> Drain the buffer at a single point: kvm_arch_vcpu_ioctl_run() flushes
>>>> it on every VM exit, before the exit reason is handled. Flushing
>>>> inside the run loop keeps the dirty-ring feedback timely: when a flush
>>>> pushes the ring past its soft limit, kvm_dirty_ring_push() raises
>>>> KVM_REQ_DIRTY_RING_SOFT_FULL, which check_vcpu_requests() observes at
>>>> the top of the next loop iteration, so the vCPU exits to userspace for
>>>> the harvest before re-entering the guest.
>>>>
>>>> kvm_arch_sync_dirty_log() relies on the exit path to flush: it kicks
>>>> vCPUs out of guest mode, and the GET/CLEAR protocol tolerates
>>>> concurrent bitmap writes, so the snapshot is complete without extra
>>>> synchronization.
>>>>
>>>> Signed-off-by: Eillon <yezhenyu2@huawei.com>
>>>> Signed-off-by: Tian Zheng <zhengtian10@huawei.com>
>>>> ---
>>>>    arch/arm64/include/asm/kvm_dirty_bit.h | 15 +++++++++
>>>>    arch/arm64/kvm/arm.c                   | 20 ++++++++++++
>>>>    arch/arm64/kvm/dirty_bit.c             | 45 ++++++++++++++++++++++++++
>>>>    3 files changed, 80 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/include/asm/kvm_dirty_bit.h b/arch/arm64/include/asm/kvm_dirty_bit.h
>>>> index fe703f02626b..d828e6b43fe9 100644
>>>> --- a/arch/arm64/include/asm/kvm_dirty_bit.h
>>>> +++ b/arch/arm64/include/asm/kvm_dirty_bit.h
>>>> @@ -13,6 +13,9 @@
>>>>    #include <asm/sysreg.h>
>>>>    #include <linux/sizes.h>
>>>>
>>>> +#define HDBSS_ENTRY_VALID	BIT(0)
>>>> +#define HDBSS_ENTRY_IPA	GENMASK_ULL(55, 12)
>>>> +
>>>>    #define KVM_ARM_HDBSS_DEFAULT_SIZE  PAGE_SIZE
>>>>    #define KVM_ARM_HDBSS_MAX_SIZE      SZ_2M
>>>>
>>>> @@ -22,7 +25,19 @@ static inline u32 kvm_hdbss_buffer_size(struct kvm *kvm)
>>>>    	return kvm->arch.hdbss_buffer_size ?: KVM_ARM_HDBSS_DEFAULT_SIZE;
>>>>    }
>>>>
>>>> +static inline bool kvm_hdbss_enabled(struct kvm *kvm)
>>>> +{
>>>> +	return kvm->arch.mmu.vtcr & VTCR_EL2_HDBSS;
>>>> +}
>>>> +
>>>> +static inline bool vcpu_hdbss_enabled(struct kvm_vcpu *vcpu)
>>>> +{
>>>> +	return vcpu->arch.hw_mmu &&
>>>> +		(vcpu->arch.hw_mmu->vtcr & VTCR_EL2_HDBSS);
>>>> +}
>>>> +
>>>
>>> IIUC, arch.mmu is always the canonical s2_mmu, while hw_mmu can be the
>>> nested s2_mmu, if enabled.
>>>
>>> Was that your intend here?
>>> If not, we could use only kvm* version, which can be called with vcpu->kvm.
>>>
>>> If it was, then I would like to understand the logic here, as IIUC the
>>> buffer is accessible only for the non-nested guest, as it's
>>> allocated in a per-vcpu structure. (vcpu->arch)
>>>
>>
>> Hi leo,
>>
>> Yes, that was the intent. The shadow MMU never carries VTCR_EL2.HDBSS,
>> so the flush is naturally skipped when running nested.
> 
> Ah, right. If we always test the kvm one, we may enable it for the nested
> guest, which is undesired.
> 
> [...]
>>
>>>
>>>> @@ -2017,7 +2025,19 @@ long kvm_arch_vcpu_unlocked_ioctl(struct file *filp, unsigned int ioctl,
>>>>
>>>>    void kvm_arch_sync_dirty_log(struct kvm *kvm, struct kvm_memory_slot *memslot)
>>>>    {
>>>> +	unsigned long i;
>>>> +	struct kvm_vcpu *vcpu;
>>>>
>>>> +	if (!kvm_hdbss_enabled(kvm))
>>>> +		return;
>>>> +
> 
> And then here you check for the L1 VM because during migration the host
> could have dirtied memory, and you want to track that as well.
> 
> Makes sense
> 
>>>> +	/*
>>>> +	 * The buffer is drained on every VM exit, so kicking running
>>>> +	 * vCPUs is enough to flush them; the dirty-log GET/CLEAR
>>>> +	 * protocol tolerates bits set concurrently with the snapshot.
>>>> +	 */
>>>> +	kvm_for_each_vcpu(i, vcpu, kvm)
>>>> +		kvm_vcpu_kick(vcpu);
>>>>    }
>>>>
>>>>    static int kvm_vm_ioctl_set_device_addr(struct kvm *kvm,
>>>> diff --git a/arch/arm64/kvm/dirty_bit.c b/arch/arm64/kvm/dirty_bit.c
>>>> index f9aeb9f34ad0..be0d12555c84 100644
>>>> --- a/arch/arm64/kvm/dirty_bit.c
>>>> +++ b/arch/arm64/kvm/dirty_bit.c
>>>> @@ -13,6 +13,7 @@
>>>>    #include <linux/kconfig.h>
>>>>    #include <linux/log2.h>
>>>>    #include <linux/mm.h>
>>>> +#include <linux/srcu.h>
>>>>
>>>
>>> Out of curiosity, could you please explain you reasoning on selecting srcu
>>> as locking here?
>>>
>>> (Like, what are you protecting against, and why srcu is a good choice)
>>>
>>
>> kvm_vcpu_mark_page_dirty() resolves the memslot under SRCU
>> (kvm_vcpu_gfn_to_memslot), and the flush loop calls it for each entry,
>> so the whole loop needs the read lock held.
> 
> Oh, right, it's just about getting the memslot... now I see it :)
> 
> Thanks!
> Leo

Awesome!