Re: [PATCH v2 4/5] KVM: arm64: Enable HDBSS support and handle HDBSSF events

[Tian Zheng <zhengtian10@huawei.com>]

On 12/28/2025 9:21 PM, Robert Hoo wrote:
> On 12/24/2025 2:15 PM, Tian Zheng wrote:
>>
>>
>> On 12/17/2025 9:39 PM, Robert Hoo wrote:
>>> On 11/21/2025 5:23 PM, Tian Zheng wrote:
>>>> From: eillon <yezhenyu2@huawei.com>
>>>>
>>>> Implement the HDBSS enable/disable functionality using the
>>>> KVM_CAP_ARM_HW_DIRTY_STATE_TRACK ioctl.
>>>>
>>>> Userspace (e.g., QEMU) can enable HDBSS by invoking the ioctl
>>>> at the start of live migration, configuring the buffer size.
>>>> The feature is disabled by invoking the ioctl again with size
>>>> set to 0 once migration completes.
>>>>
>>>> Add support for updating the dirty bitmap based on the HDBSS
>>>> buffer. Similar to the x86 PML implementation, KVM flushes the
>>>> buffer on all VM-Exits, so running vCPUs only need to be kicked
>>>> to force a VM-Exit.
>>>>
>>>> Signed-off-by: eillon <yezhenyu2@huawei.com>
>>>> Signed-off-by: Tian Zheng <zhengtian10@huawei.com>
>>>> ---
>>>>   arch/arm64/include/asm/kvm_host.h |  10 +++
>>>>   arch/arm64/include/asm/kvm_mmu.h  |  17 +++++
>>>>   arch/arm64/kvm/arm.c              | 107 
>>>> ++++++++++++++++++++++++++++++
>>>>   arch/arm64/kvm/handle_exit.c      |  45 +++++++++++++
>>>>   arch/arm64/kvm/hyp/vhe/switch.c   |   1 +
>>>>   arch/arm64/kvm/mmu.c              |  10 +++
>>>>   arch/arm64/kvm/reset.c            |   3 +
>>>>   include/linux/kvm_host.h          |   1 +
>>>>   8 files changed, 194 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/include/asm/kvm_host.h 
>>>> b/arch/arm64/include/ asm/kvm_host.h
>>>> index d962932f0e5f..408e4c2b3d1a 100644
>>>> --- a/arch/arm64/include/asm/kvm_host.h
>>>> +++ b/arch/arm64/include/asm/kvm_host.h
>>>> @@ -87,6 +87,7 @@ int __init kvm_arm_init_sve(void);
>>>>   u32 __attribute_const__ kvm_target_cpu(void);
>>>>   void kvm_reset_vcpu(struct kvm_vcpu *vcpu);
>>>>   void kvm_arm_vcpu_destroy(struct kvm_vcpu *vcpu);
>>>> +void kvm_arm_vcpu_free_hdbss(struct kvm_vcpu *vcpu);
>>>>
>>>>   struct kvm_hyp_memcache {
>>>>       phys_addr_t head;
>>>> @@ -793,6 +794,12 @@ struct vcpu_reset_state {
>>>>       bool        reset;
>>>>   };
>>>>
>>>> +struct vcpu_hdbss_state {
>>>> +    phys_addr_t base_phys;
>>>> +    u32 size;
>>>> +    u32 next_index;
>>>> +};
>>>> +
>>>>   struct vncr_tlb;
>>>>
>>>>   struct kvm_vcpu_arch {
>>>> @@ -897,6 +904,9 @@ struct kvm_vcpu_arch {
>>>>
>>>>       /* Per-vcpu TLB for VNCR_EL2 -- NULL when !NV */
>>>>       struct vncr_tlb    *vncr_tlb;
>>>> +
>>>> +    /* HDBSS registers info */
>>>> +    struct vcpu_hdbss_state hdbss;
>>>>   };
>>>>
>>>>   /*
>>>> diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/ 
>>>> asm/kvm_mmu.h
>>>> index e4069f2ce642..6ace1080aed5 100644
>>>> --- a/arch/arm64/include/asm/kvm_mmu.h
>>>> +++ b/arch/arm64/include/asm/kvm_mmu.h
>>>> @@ -331,6 +331,23 @@ static __always_inline void 
>>>> __load_stage2(struct kvm_s2_mmu *mmu,
>>>>       asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_SPECULATIVE_AT));
>>>>   }
>>>>
>>>> +static __always_inline void __load_hdbss(struct kvm_vcpu *vcpu)
>>>> +{
>>>> +    struct kvm *kvm = vcpu->kvm;
>>>> +    u64 br_el2, prod_el2;
>>>> +
>>>> +    if (!kvm->enable_hdbss)
>>>> +        return;
>>>> +
>>>> +    br_el2 = HDBSSBR_EL2(vcpu->arch.hdbss.base_phys, vcpu- 
>>>> >arch.hdbss.size);
>>>> +    prod_el2 = vcpu->arch.hdbss.next_index;
>>>> +
>>>> +    write_sysreg_s(br_el2, SYS_HDBSSBR_EL2);
>>>> +    write_sysreg_s(prod_el2, SYS_HDBSSPROD_EL2);
>>>> +
>>>> +    isb();
>>>> +}
>>>> +
>>>>   static inline struct kvm *kvm_s2_mmu_to_kvm(struct kvm_s2_mmu *mmu)
>>>>   {
>>>>       return container_of(mmu->arch, struct kvm, arch);
>>>> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
>>>> index 870953b4a8a7..64f65e3c2a89 100644
>>>> --- a/arch/arm64/kvm/arm.c
>>>> +++ b/arch/arm64/kvm/arm.c
>>>> @@ -79,6 +79,92 @@ int kvm_arch_vcpu_should_kick(struct kvm_vcpu 
>>>> *vcpu)
>>>>       return kvm_vcpu_exiting_guest_mode(vcpu) == IN_GUEST_MODE;
>>>>   }
>>>>
>>>> +void kvm_arm_vcpu_free_hdbss(struct kvm_vcpu *vcpu)
>>>> +{
>>>> +    struct page *hdbss_pg = NULL;
>>>> +
>>>> +    hdbss_pg = phys_to_page(vcpu->arch.hdbss.base_phys);
>>>> +    if (hdbss_pg)
>>>> +        __free_pages(hdbss_pg, vcpu->arch.hdbss.size);
>>>> +
>>>> +    vcpu->arch.hdbss = (struct vcpu_hdbss_state) {
>>>> +        .base_phys = 0,
>>>> +        .size = 0,
>>>> +        .next_index = 0,
>>>> +    };
>>>> +}
>>>> +
>>>> +static int kvm_cap_arm_enable_hdbss(struct kvm *kvm,
>>>> +                    struct kvm_enable_cap *cap)
>>>> +{
>>>> +    unsigned long i;
>>>> +    struct kvm_vcpu *vcpu;
>>>> +    struct page *hdbss_pg = NULL;
>>>> +    int size = cap->args[0];
>>>> +    int ret = 0;
>>>> +
>>>> +    if (!system_supports_hdbss()) {
>>>> +        kvm_err("This system does not support HDBSS!\n");
>>>> +        return -EINVAL;
>>>> +    }
>>>> +
>>>> +    if (size < 0 || size > HDBSS_MAX_SIZE) {
>>>> +        kvm_err("Invalid HDBSS buffer size: %d!\n", size);
>>>> +        return -EINVAL;
>>>> +    }
>>>> +
>>>
>>> I think you should check if it's already enabled here. What if user 
>>> space calls this twice?
>>
>> Ok, I review the implement of qemu, when disable the hdbss feature in
>> ram_save_cleanup, size=0 will be set, so here can add a check, if (size
>> && kvm->arch.enable_hdbss), we will do nothing.
>>
>
> I mean you should check if ' kvm->enable_hdbss' is already set, if so, 
> return rather than alloc_pages() in below (you have allocated in 
> previous call with valid 'size').
>
> qemu is just one of the user space applications that would possibly 
> call this API, you cannot rely on your qemu patch's flow/sequence as 
> assumption to design a KVM API's implementation.


Yes, The latest v3 patch fixes this bug by checking if (size > 0 && 
kvm->arch.enable_hdbss).
When this condition is met, the function returns immediately rather than 
alloc_pages().


>
>>>
>>>> +    /* Enable the HDBSS feature if size > 0, otherwise disable it. */
>>>> +    if (size) {
>>>> +        kvm_for_each_vcpu(i, vcpu, kvm) {
>>>> +            hdbss_pg = alloc_pages(GFP_KERNEL_ACCOUNT, size);
>>>> +            if (!hdbss_pg) {
>>>> +                kvm_err("Alloc HDBSS buffer failed!\n");
>>>> +                ret = -ENOMEM;
>>>> +                goto error_alloc;
>>>> +            }
>>>> +
>>>> +            vcpu->arch.hdbss = (struct vcpu_hdbss_state) {
>>>> +                .base_phys = page_to_phys(hdbss_pg),
>>>> +                .size = size,
>>>> +                .next_index = 0,
>>>> +            };
>>>> +        }
>>>> +
>>>> +        kvm->enable_hdbss = true;
>>>> +        kvm->arch.mmu.vtcr |= VTCR_EL2_HD | VTCR_EL2_HDBSS;
>>>
>
>