[PATCH 0/2] KVM: arm64: Small dirty logging fixes/cleanups

[PATCH 0/2] KVM: arm64: Small dirty logging fixes/cleanups

[Wei-Lin Chang]

Hi,

These are two minor fixes/cleanups I found when reading the code.
Although the diff are small the first patch has some implications.
Please view the patches for more detail.

Thanks,

Wei-Lin Chang (2):
  KVM: arm64: Replace memslot_is_logging() with
    kvm_slot_dirty_track_enabled()
  KVM: arm64: Remove superfluous aligning of gfn for dirty logging

 arch/arm64/kvm/mmu.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

-- 
2.43.0

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

[Wei-Lin Chang]

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.

Fix this by using kvm_slot_dirty_track_enabled() instead which checks
KVM_MEM_LOG_DIRTY_PAGES.

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.

Fixes: 9cb1096f8590 ("KVM: arm64: Enable ring-based dirty memory tracking")
Signed-off-by: Wei-Lin Chang <weilin.chang@arm.com>
---
It took me a long investigation to acquire the context needed to
understand this change, however the reason for this problem not being
detected is an educated guess. Please let me know if this is wrong or
if there are other issues, thanks!

 arch/arm64/kvm/mmu.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 4da9281312eb..06c46124d3e7 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -161,11 +161,6 @@ static int kvm_mmu_split_huge_pages(struct kvm *kvm, phys_addr_t addr,
 	return ret;
 }
 
-static bool memslot_is_logging(struct kvm_memory_slot *memslot)
-{
-	return memslot->dirty_bitmap && !(memslot->flags & KVM_MEM_READONLY);
-}
-
 /**
  * kvm_arch_flush_remote_tlbs() - flush all VM TLB entries for v7/8
  * @kvm:	pointer to kvm structure.
@@ -1748,7 +1743,7 @@ static short kvm_s2_resolve_vma_size(const struct kvm_s2_fault_desc *s2fd,
 {
 	short vma_shift;
 
-	if (memslot_is_logging(s2fd->memslot)) {
+	if (kvm_slot_dirty_track_enabled(s2fd->memslot)) {
 		s2vi->max_map_size = PAGE_SIZE;
 		vma_shift = PAGE_SHIFT;
 	} else {
@@ -1953,7 +1948,7 @@ static int kvm_s2_fault_compute_prot(const struct kvm_s2_fault_desc *s2fd,
 	*prot = KVM_PGTABLE_PROT_R;
 
 	if (s2vi->map_writable && (s2vi->device ||
-				   !memslot_is_logging(s2fd->memslot) ||
+				   !kvm_slot_dirty_track_enabled(s2fd->memslot) ||
 				   kvm_is_write_fault(s2fd->vcpu)))
 		*prot |= KVM_PGTABLE_PROT_W;
 
@@ -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))) {
 		ret = topup_mmu_memcache(s2fd->vcpu, memcache);
 		if (ret)
-- 
2.43.0

[PATCH 2/2] KVM: arm64: Remove superfluous aligning of gfn for dirty logging

[Wei-Lin Chang]

Stage-2 mapping size is forced to PAGE_SIZE when dirty logging is
enabled for a memslot, therefore we don't need to align it down to
a possibly larger vma size or THP adjusted size, they won't happen.

Signed-off-by: Wei-Lin Chang <weilin.chang@arm.com>
---
 arch/arm64/kvm/mmu.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 06c46124d3e7..d1f6ff7c2943 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -2050,13 +2050,12 @@ static int kvm_s2_fault_map(const struct kvm_s2_fault_desc *s2fd,
 
 	/*
 	 * Mark the page dirty only if the fault is handled successfully,
-	 * making sure we adjust the canonical IPA if the mapping size has
-	 * been updated (via a THP upgrade, for example).
+	 * mapping size is forced to PAGE_SIZE if dirty logging is enabled,
+	 * so we don't have to adjust the canonical IPA here.
 	 */
 	if (writable && !ret) {
-		phys_addr_t ipa = gfn_to_gpa(get_canonical_gfn(s2fd, s2vi));
-		ipa &= ~(mapping_size - 1);
-		mark_page_dirty_in_slot(kvm, s2fd->memslot, gpa_to_gfn(ipa));
+		gfn_t canonical_gfn = get_canonical_gfn(s2fd, s2vi);
+		mark_page_dirty_in_slot(kvm, s2fd->memslot, canonical_gfn);
 	}
 
 	if (ret != -EAGAIN)
-- 
2.43.0

Re: [PATCH 2/2] KVM: arm64: Remove superfluous aligning of gfn for dirty logging

[Leonardo Bras]

On Fri, Jun 05, 2026 at 04:32:48PM +0100, Wei-Lin Chang wrote:
> Stage-2 mapping size is forced to PAGE_SIZE when dirty logging is
> enabled for a memslot, therefore we don't need to align it down to
> a possibly larger vma size or THP adjusted size, they won't happen.
> 

IIRC there was some effort being made in terms of tracking in LEVEL2 block 
granularity instead of LEVEL3 (PAGE_SIZE). It makes sense as some platforms 
can have very fast networking so setup time is more relevant than actual 
sending time.

Would not making this change affect such effort?

Thanks!
Leo


> Signed-off-by: Wei-Lin Chang <weilin.chang@arm.com>
> ---
>  arch/arm64/kvm/mmu.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index 06c46124d3e7..d1f6ff7c2943 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -2050,13 +2050,12 @@ static int kvm_s2_fault_map(const struct kvm_s2_fault_desc *s2fd,
>  
>  	/*
>  	 * Mark the page dirty only if the fault is handled successfully,
> -	 * making sure we adjust the canonical IPA if the mapping size has
> -	 * been updated (via a THP upgrade, for example).
> +	 * mapping size is forced to PAGE_SIZE if dirty logging is enabled,
> +	 * so we don't have to adjust the canonical IPA here.
>  	 */
>  	if (writable && !ret) {
> -		phys_addr_t ipa = gfn_to_gpa(get_canonical_gfn(s2fd, s2vi));
> -		ipa &= ~(mapping_size - 1);
> -		mark_page_dirty_in_slot(kvm, s2fd->memslot, gpa_to_gfn(ipa));
> +		gfn_t canonical_gfn = get_canonical_gfn(s2fd, s2vi);
> +		mark_page_dirty_in_slot(kvm, s2fd->memslot, canonical_gfn);
>  	}
>  
>  	if (ret != -EAGAIN)
> -- 
> 2.43.0
> 

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

[Leonardo Bras]

Hi Wei Lin,

On Fri, Jun 05, 2026 at 04:32:47PM +0100, Wei-Lin Chang 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.
> 
> Fix this by using kvm_slot_dirty_track_enabled() instead which checks
> KVM_MEM_LOG_DIRTY_PAGES.
> 
> 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.


It worries me a bit that we are ignoring the KVM_MEM_READONLY flag... 
I have not yet gone through the whole s2_mmu code but IIUC we can have 
scenarios on which a memslot can be read-only and have dirty-logging 
enabled. If a memslot is not faulted yet, IIUC it is marked as read-only 
(so it can be mapped on write fault), and we can have dirty-logging 
enabled for it as well (as the VMM has no idea). 

Would not that change impact this scenario?

I will take a better look in that part of the code as well, to properly 
understand it.

Thanks!
Leo




> 
> Fixes: 9cb1096f8590 ("KVM: arm64: Enable ring-based dirty memory tracking")
> Signed-off-by: Wei-Lin Chang <weilin.chang@arm.com>
> ---
> It took me a long investigation to acquire the context needed to
> understand this change, however the reason for this problem not being
> detected is an educated guess. Please let me know if this is wrong or
> if there are other issues, thanks!
> 
>  arch/arm64/kvm/mmu.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index 4da9281312eb..06c46124d3e7 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -161,11 +161,6 @@ static int kvm_mmu_split_huge_pages(struct kvm *kvm, phys_addr_t addr,
>  	return ret;
>  }
>  
> -static bool memslot_is_logging(struct kvm_memory_slot *memslot)
> -{
> -	return memslot->dirty_bitmap && !(memslot->flags & KVM_MEM_READONLY);
> -}
> -
>  /**
>   * kvm_arch_flush_remote_tlbs() - flush all VM TLB entries for v7/8
>   * @kvm:	pointer to kvm structure.
> @@ -1748,7 +1743,7 @@ static short kvm_s2_resolve_vma_size(const struct kvm_s2_fault_desc *s2fd,
>  {
>  	short vma_shift;
>  
> -	if (memslot_is_logging(s2fd->memslot)) {
> +	if (kvm_slot_dirty_track_enabled(s2fd->memslot)) {
>  		s2vi->max_map_size = PAGE_SIZE;
>  		vma_shift = PAGE_SHIFT;
>  	} else {
> @@ -1953,7 +1948,7 @@ static int kvm_s2_fault_compute_prot(const struct kvm_s2_fault_desc *s2fd,
>  	*prot = KVM_PGTABLE_PROT_R;
>  
>  	if (s2vi->map_writable && (s2vi->device ||
> -				   !memslot_is_logging(s2fd->memslot) ||
> +				   !kvm_slot_dirty_track_enabled(s2fd->memslot) ||
>  				   kvm_is_write_fault(s2fd->vcpu)))
>  		*prot |= KVM_PGTABLE_PROT_W;
>  
> @@ -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))) {
>  		ret = topup_mmu_memcache(s2fd->vcpu, memcache);
>  		if (ret)
> -- 
> 2.43.0
> 

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

[Leonardo Bras]

On Mon, Jun 08, 2026 at 04:55:45PM +0100, Leonardo Bras wrote:
> Hi Wei Lin,
> 
> On Fri, Jun 05, 2026 at 04:32:47PM +0100, Wei-Lin Chang 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.
> > 
> > Fix this by using kvm_slot_dirty_track_enabled() instead which checks
> > KVM_MEM_LOG_DIRTY_PAGES.
> > 
> > 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.
> 
> 
> It worries me a bit that we are ignoring the KVM_MEM_READONLY flag... 
> I have not yet gone through the whole s2_mmu code but IIUC we can have 
> scenarios on which a memslot can be read-only and have dirty-logging 
> enabled. 


> If a memslot is not faulted yet, IIUC it is marked as read-only 
> (so it can be mapped on write fault), and we can have dirty-logging 
> enabled for it as well (as the VMM has no idea). 
> 

Ignore above bit, I confused memslot with block/page entry.

Looking a bit more, my viewpoint is that:
- Due to dirty_ring, checking memslot.dirty_bitmap should be done only to 
  detect the existence of a dirty_bitmap, not the migration process.
- This changes how detection works, in regardas to read-only blocks:
  memslot_is_logging() -> Checks dirty-bitmap + read-only memslot
  kvm_slot_dirty_track_enabled()  -> Checks only memslot flag
- As a simpler change, we could have:

~~~
-   return memslot->dirty_bitmap && !(memslot->flags & KVM_MEM_READONLY);
+   return kvm_slot_dirty_track_enabled(memslot) && !(memslot->flags & KVM_MEM_READONLY);
~~~

Both are cheking memslot->flags, so it will be probably optimized by the 
compiler as:

~~~
return memslot->flags & 3 == 1
~~~

My main worry was that in the curent patch we are changing the behavior 
on skipping read-only memslots. So going through the users, we can see:

> > 
> > Fixes: 9cb1096f8590 ("KVM: arm64: Enable ring-based dirty memory tracking")
> > Signed-off-by: Wei-Lin Chang <weilin.chang@arm.com>
> > ---
> > It took me a long investigation to acquire the context needed to
> > understand this change, however the reason for this problem not being
> > detected is an educated guess. Please let me know if this is wrong or
> > if there are other issues, thanks!
> > 
> >  arch/arm64/kvm/mmu.c | 11 +++--------
> >  1 file changed, 3 insertions(+), 8 deletions(-)
> > 
> > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> > index 4da9281312eb..06c46124d3e7 100644
> > --- a/arch/arm64/kvm/mmu.c
> > +++ b/arch/arm64/kvm/mmu.c
> > @@ -161,11 +161,6 @@ static int kvm_mmu_split_huge_pages(struct kvm *kvm, phys_addr_t addr,
> >  	return ret;
> >  }
> >  
> > -static bool memslot_is_logging(struct kvm_memory_slot *memslot)
> > -{
> > -	return memslot->dirty_bitmap && !(memslot->flags & KVM_MEM_READONLY);
> > -}
> > -
> >  /**
> >   * kvm_arch_flush_remote_tlbs() - flush all VM TLB entries for v7/8
> >   * @kvm:	pointer to kvm structure.
> > @@ -1748,7 +1743,7 @@ static short kvm_s2_resolve_vma_size(const struct kvm_s2_fault_desc *s2fd,
> >  {
> >  	short vma_shift;
> >  
> > -	if (memslot_is_logging(s2fd->memslot)) {
> > +	if (kvm_slot_dirty_track_enabled(s2fd->memslot)) {
> >  		s2vi->max_map_size = PAGE_SIZE;
> >  		vma_shift = PAGE_SHIFT;
> >  	} else {

On the case dirty_track is enabled in a read-only slot, it will resolve to 
a smaller vma_size. The fault granule will be smaller here. This could be 
bad for performance, so maybe we could add a check for read-only block 
here:

~~~
-   if (memslot_is_logging(s2fd->memslot)) {
+   if (kvm_slot_dirty_track_enabled(s2fd->memslot) &&
+       !memslot_is_readonly(s2fd->memslot) {
~~~


> > @@ -1953,7 +1948,7 @@ static int kvm_s2_fault_compute_prot(const struct kvm_s2_fault_desc *s2fd,
> >  	*prot = KVM_PGTABLE_PROT_R;
> >  
> >  	if (s2vi->map_writable && (s2vi->device ||
> > -				   !memslot_is_logging(s2fd->memslot) ||
> > +				   !kvm_slot_dirty_track_enabled(s2fd->memslot) ||
> >  				   kvm_is_write_fault(s2fd->vcpu)))
> >  		*prot |= KVM_PGTABLE_PROT_W;
> >


On the same scenario (dirty_track enabled on readonly memslot):
This one should be safe, as kvm_is_write_fault() will check if the memslot 
is readonly and return false in this case. But then, it will have to 
actually call kvm_is_write_fault(), as the previous version would not even 
call it in that scenario.

Not sure how would that impact perforformance, though.

> > @@ -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))) {
> >  		ret = topup_mmu_memcache(s2fd->vcpu, memcache);
> >  		if (ret)

Same thing, if memslot is tracking and is readonly, topup_*() would be 
called with the new patch, but not with the old behavior. 

All of that depends on how the VMM uses dirty_tracking: does it enable for 
all memory, or only for memory that is writable?

I could not find anything that would prevent user from enabling 
dirty_tracking on read-only memslots, so we can either ignore this 
scenario, apply those patches and let those users carry the extra overhead, 
or do an extra test to make sure it's doing the same thing as before.

Thanks!
Leo