[PATCH 1/2] KVM: arm64: Replace memslot_is_logging() with kvm_slot_dirty_track_enabled()

[Fuad Tabba]

Hi Wei-Lin,

On Fri, 05 Jun 2026 16:32:47 +0100, Wei-Lin Chang <weilin.chang at arm.com> wrote:
> When checking whether a memslot has dirty logging enabled, the
> KVM_MEM_LOG_DIRTY_PAGES flag is the source of truth. Previously we were
> using memslot_is_logging() which only tests dirty bitmap and did not
> consider dirty ring. This was not detected because
> KVM_CAP_DIRTY_LOG_RING_WITH_BITMAP was introduced together with KVM
> arm64 dirty ring, and users need to enable it to ensure dirty
> information is not lost for the case of VGIC LPI/ITS table changes.

I ran into the same bug while testing the arm64 KVM_PRE_FAULT_MEMORY
series, and then found that you'd already fixed it.

It might be worth mentioning the impact in the commit message: with
the ring and no bitmap, a logged memslot still gets block mappings, so
guest writes to most of a block are never logged and a migration can
copy stale memory. That would also make the case for a Cc: stable.

> Note that memslot_is_logging() also treats a memslot as not logging if
> KVM_MEM_READONLY is set, hence a memslot with both dirty logging and
> read only would be seen as not logging for memslot_is_logging(), but
> logging for kvm_slot_dirty_track_enabled(). This allows a read only
> mapping of size > PAGE_SIZE to be built when memslot_is_logging() is
> used, leading to a better read performance compared to
> kvm_slot_dirty_track_enabled(). However memslots that have both
> KVM_MEM_LOG_DIRTY_PAGES and KVM_MEM_READONLY set do not really make
> sense as dirty logging is essentially nop for a read only memslot, so
> this shouldn't affect real workloads much.

FWIW, I'd go with Leo's suggestion of keeping the KVM_MEM_READONLY
check, so those memslots keep their block mappings.

[...]
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
[...]
> @@ -2084,7 +2079,7 @@ static int user_mem_abort(const struct kvm_s2_fault_desc *s2fd)
>          * and a write fault needs to collapse a block entry into a table.
>          */
>         memcache = get_mmu_memcache(s2fd->vcpu);
> -       if (!perm_fault || (memslot_is_logging(s2fd->memslot) &&
> +       if (!perm_fault || (kvm_slot_dirty_track_enabled(s2fd->memslot) &&
>                             kvm_is_write_fault(s2fd->vcpu))) {

FYI, this no longer applies to kvmarm/fixes or kvmarm/next, where the
condition now ends in memslot_is_logging() ||
is_protected_kvm_enabled().

I tested the series on v7.3-rc7 (with that conflict resolved) in QEMU,
with dirty_log_test changed to back the memslot with THP. In ring-only
mode it fails on the first iteration without the series and passes
with it. I'll send the test change later.

Reviewed-by: Fuad Tabba <fuad.tabba at linux.dev>
Tested-by: Fuad Tabba <fuad.tabba at linux.dev>

Cheers,
/fuad