Re: [PATCH v5 11/15] KVM: arm64: Reserve dirty ring space for the HDBSS buffer

[Tian Zheng <zhengtian10@huawei.com>]

On 11/10/2026 8:49 PM, Leonardo Bras wrote:
> On Tue, Nov 10, 2026 at 04:54:30PM +0800, Tian Zheng wrote:
>>
>>
>> On 11/10/2026 2:18 AM, Leonardo Bras wrote:
>>> On Mon, Nov 09, 2026 at 12:00:52PM +0800, Tian Zheng wrote:
>>>>
>>>>
>>>> On 11/6/2026 11:36 PM, Leonardo Bras wrote:
>>>>> On Fri, Nov 06, 2026 at 06:36:04PM +0800, Tian Zheng wrote:
>>>>>>
>>>>>>
>>>>>> On 11/2/2026 10:54 PM, Leonardo Bras wrote:
>>>>>>> On Tue, Sep 29, 2026 at 06:36:51PM +0800, Tian Zheng wrote:
>>>>>>>> The dirty ring reserves room for CPU-side dirty buffers via
>>>>>>>> kvm_cpu_dirty_log_size(). Report the HDBSS buffer entry count
>>>>>>>> through it, so a full buffer flush always fits in the ring.
>>>>>>>>
>>>>>>>> Pin the HDBSS buffer size to PAGE_SIZE when dirty-ring is enabled,
>>>>>>>> via kvm_arch_dirty_ring_size_updated(): a larger buffer only
>>>>>>>> shrinks the soft_limit and forces more frequent userspace drains.
>>>>>>>>
>>>>>>>> Signed-off-by: Tian Zheng <zhengtian10@huawei.com>
>>>>>>>> ---
>>>>>>>>      arch/arm64/kvm/arm.c | 8 ++++++++
>>>>>>>>      arch/arm64/kvm/mmu.c | 8 ++++++++
>>>>>>>>      2 files changed, 16 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
>>>>>>>> index 9d7bdece149a..5a7046acb62e 100644
>>>>>>>> --- a/arch/arm64/kvm/arm.c
>>>>>>>> +++ b/arch/arm64/kvm/arm.c
>>>>>>>> @@ -215,6 +215,14 @@ static int kvm_arm_default_max_vcpus(void)
>>>>>>>>      	return vgic_present ? kvm_vgic_get_max_vcpus() : KVM_MAX_VCPUS;
>>>>>>>>      }
>>>>>>>>
>>>>>>>> +void kvm_arch_dirty_ring_size_updated(struct kvm *kvm)
>>>>>>>> +{
>>>>>>>> +	if (!system_supports_hdbss())
>>>>>>>> +		return;
>>>>>>>> +
>>>>>>>> +	kvm->arch.hdbss_buffer_size = KVM_ARM_HDBSS_DEFAULT_SIZE;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>
>>>>>>> On the same subject as a previous patch:
>>>>>>> I don't think this is the best approach, on top of adding a new weak
>>>>>>> function to take care.
>>>>>>>
>>>>>>> Maybe during the dirty-ring allocation, on vcpu creation, we could take
>>>>>>> into account the requested dirty-ring size, and just allocate the HDBSS
>>>>>>> buffer taking that into account. That would make it simpler, don't you
>>>>>>> agree?
>>>>>>>
>>>>>>> Thanks!
>>>>>>> Leo
>>>>>>
>>>>>> Hi Leo,
>>>>>>
>>>>>> The "set once" guard we're discussing comes from the patch that adds the
>>>>>> ioctl, and it isn't really needed. We discussed this earlier [1]: we can
>>>>>> just initialize hdbss_buffer_size to PAGE_SIZE at VM creation and let
>>>>>> the ring reservation use it directly. **That also means
>>>>>> kvm_arch_dirty_ring_size_updated() can be removed.** Thanks for your
>>>>>> suggestion.
>>>>>
>>>>> :)
>>>>>
>>>>>>
>>>>>> As for why PAGE_SIZE is the right value in ring mode: the HDBSS buffer
>>>>>> is flushed on every VM exit, so a larger buffer doesn't reduce the flush
>>>>>> frequency. It only inflates the ring reservation (64 + size/
>>>>>> sizeof(u64)), which shrinks the soft_limit and forces more frequent
>>>>>> exits to userspace for the ring to be drained. Draining the dirty ring
>>>>>> from QEMU is more expensive than draining the HDBSS buffer from the
>>>>>> kernel, so the smallest buffer gives the best performance. In this mode,
>>>>>> a larger buffer provides no benefit to users.
>>>>>
>>>>> Humm, if properly dimensioned larger buffers would be benefitial for
>>>>> memory-intensive workloads, unless they are guest_exiting for other
>>>>> reasons in the meantime.
>>>>>
>>>>
>>>> Agreed, and the workload I tested is on the "unless" side. The guest
>>>> exits constantly for other reasons, so the larger buffer never got a
>>>> chance to pay off, and only cost ring capacity.
>>>>
>>>>>>
>>>>>> In my tests, I also saw performance degrade when more space is reserved.
>>>>>>
>>>>>
>>>>> Humm, what kind of workloads did you test?
>>>>> Did you have the chance to monitor what were the reasons for VM exiting?
>>>>>
>>>>
>>>> For the workload question, it's a Redis server in the guest under redis-
>>>> benchmark (SET-only, 2KB values, 1M keys, 100M requests). The traffic is
>>>> network-driven, so the guest exits constantly to service requests. The
>>>> HDBSS buffer only accumulates entries between two exits and is flushed
>>>> on every exit, so it rarely fills regardless of size. A larger buffer
>>>> only inflated the ring reservation, which shrinks the soft_limit and
>>>> makes the ring-full exit and userspace drain fire more often.
>>>>
>>>>> I am not saying this is mandatory for the HDBSS implementation, but the
>>>>> whole idea of letting the user choose their own HDBSS buffer size is
>>>>> because different workloads may benefit from different buffer sizes: if a
>>>>> VM writes a lot to new memory, and exits very little for other reasons,
>>>>> then a larger buffer will cause the VM to guest_exit much less often.
>>>>>
>>>>> Does that make sense?
>>>>>
>>>>> Thanks!
>>>>> Leo
>>>>
>>>> It does make sense. That's exactly the workload where a larger buffer
>>>> pays off, and the next version no longer gets in its way. The two ioctls
>>>> are no longer mutually exclusive.
>>>>
>>>> The only coupling left is the reservation. hdbss_buffer_size starts at
>>>> SZ_4K at VM creation, so a ring enabled right away reserves for the
>>>> default buffer (64 + 512 entries), and the minimum ring size accepted is
>>>> unchanged from v5. KVM_CAP_ARM_HDBSS_BUFFER_SIZE is the gate. When the
>>>> ring is already enabled, a size is accepted only if its reservation
>>>> still fits in the ring, otherwise -EINVAL. In the other order, ring-
>>>> enable validation rejects a ring too small for the configured size.
>>>
>>> I don't think this is needed. If using dirtyring, we don't need to allow
>>> user to set hdbss size: HDBSS size should be sized according to the
>>> dirty-ring dimensions, that are set by userspace before the first vcpu is
>>> created.
>>>
>>>
>>>> So SZ_4K is just the conservative default for guests that exit often.
>>>> Anything larger, within what the ring can reserve, is up to the user.
>>>>
>>>> Does this work for you?
>>>
>>> I still think we should do PAGE_SIZE for dirty-bitmap. Dirty ring should do
>>> based on the dirty-ring size, and maybe review the use of reserved
>>> entries, or it's accounting.
>>>
>>> What do you think?
>>>
>>> Thanks!
>>> Leo
>>
>> Hi Leo,
>>
>> Thanks for your review. It does make sense!
>>
>> I agree that dirty-bitmap mode will keep PAGE_SIZE as the default, and
>> in that mode the ioctl stays available for a larger buffer.
>>
>> In ring mode we will drop the ioctl, the two remain mutually exclusive,
>> and the buffer will be derived from the ring size userspace configured
>> before the first vCPU is created:
>>
>> ```
>> hdbss_buffer = clamp_t(unsigned long,
>>                         rounddown_pow_of_two(ring_size / 16),
>>                         SZ_4K, KVM_ARM_HDBSS_MAX_SIZE);
>> ```
>>
> 
> Are you allocating 1/16th of the ring size in bytes as an hdbss buffer?
> That's confusing. I recommend we rather have something like:
> 
> ring_entries = ring_size / sizeof(struct kvm_dirty_gfn);
> hdbss_entries = ring_entries / 2; // Or any number we decide to use here.
> hdbss_buf_size = hdbss_entries * sizeof(hdbss_buf_entry);
> 
> Then we do
> hdbss_buf_size = clamp_t(unsigned long,
>                           rounddown_pow_of_two(hdbss_buf_size),
>                           SZ_4K, KVM_ARM_HDBSS_MAX_SIZE);
> 
> So it's clear what is what here.
> Just dividing by 16 above can be confusing to whoever is reading the code.
> 
> 
>> A buffer entry is 8 bytes and a ring entry is 16 bytes, so:
>>
>> ```
>> buffer entries = buffer_size / 8
>> ring entries   = ring_size / 16
>> ```
> 
> Remember to use the sizeof instead of fixed numbers.
> 
>>
>> This means ring_size / 16 reserves at most one eighth of the ring
>> entries for the HDBSS buffer. That keeps the usable ring above 85% while
>> the buffer still grows with the configured ring size, and the result is
>> independent of the kernel page size, with SZ_4K as the only minimum.
> 
> Yeah, remember that if you are dealing with the ringbuffer like this, an
> user can request a 4k ringbuffer and we just won't be able to use hdbss, as
> it means 4k/16 entries, and hdbss has the minimum of 4k/8.
> 
> As I mentioned, maybe this approach of reserved entries is not the right
> way of dealing with it in arm64.
> 
>>
>> One eighth is the ratio that looks reasonable to me so far. What do you
>> think?
>>
> 
> I see no reason why it could not be higher than that. It all depends on how
> you want to deal with the full buffer in the future patchset. If hdbss
> buffer is full, it has to exit and drain to the dirtyring. The guarantee
> you seek with reserved entries is that all hdbss entries fit in the
> dirty-ring.
> 
> As long as the user is required to drain entries until the dirty-ring is
> not 'full' anymore, we could easily work with dirty-ring having twice as
> many entries as the hdbss buffer.
> 
> How does it sound?
> 
> Thanks!
> Leo

Hi Leo,

Thanks for the suggestion.

For the next version I'll still go with the reservation scheme. It's the
safest approach I can think of. The guarantee I want is that a full
buffer flush always fits in the ring by construction, without depending
on how quickly userspace drains it.

I'll also switch to sizeof(struct kvm_dirty_gfn) and
sizeof(hdbss_buf_entry) instead of the hardcoded 16 and 8, and size the
HDBSS buffer at half the ring entries as you suggested.

On the 4K ring case: HDBSS can't fit there. One option is to just not
enable HDBSS in that scenario, but I'll start with returning -EINVAL for
now. I'm happy to switch to the other option if you think that's better.

Thanks,
Tian