[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <77111894-1b9f-4970-b41f-48e3a4c4b754@gmail.com>
Date: Sun, 28 Dec 2025 21:21:25 +0800
From: Robert Hoo <robert.hoo.linux@...il.com>
To: Tian Zheng <zhengtian10@...wei.com>, maz@...nel.org,
oliver.upton@...ux.dev, catalin.marinas@....com, corbet@....net,
pbonzini@...hat.com, will@...nel.org
Cc: linux-kernel@...r.kernel.org, yuzenghui@...wei.com,
wangzhou1@...ilicon.com, yezhenyu2@...wei.com, xiexiangyou@...wei.com,
zhengchuan@...wei.com, joey.gouly@....com, kvmarm@...ts.linux.dev,
kvm@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-doc@...r.kernel.org, suzuki.poulose@....com
Subject: Re: [PATCH v2 4/5] KVM: arm64: Enable HDBSS support and handle HDBSSF
events
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@...wei.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@...wei.com>
>>> Signed-off-by: Tian Zheng <zhengtian10@...wei.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.
>>
>>> + /* 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;
>>
Powered by blists - more mailing lists