[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <36e0a4d0-b440-4aba-8dfd-0c0fcb5f4318@huawei.com>
Date: Fri, 9 Jan 2026 15:52:45 +0800
From: Tian Zheng <zhengtian10@...wei.com>
To: Robert Hoo <robert.hoo.linux@...il.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/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@...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.
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;
>>>
>
>
Powered by blists - more mailing lists