RISC-V: KVM: Enhance the logging check for mmu mapping

[PATCH] RISC-V: KVM: Enhance the logging check for mmu mapping

Posted by Inochi Amaoto 1 week, 4 days ago

When enabling dirty ring, the dirty bitmap is disable, and the logging
check is always false as the RISC-V architecture does not select
"NEED_KVM_DIRTY_RING_WITH_BITMAP". Although the dirty log is recorded
since the write path already trying to add the dirty log, the logic for
logging check is broken and some side effect will occurs.

Enhance the logging check for mmu mapping so it can check both the dirty
ring and the dirty bitmap.

Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
---
 arch/riscv/kvm/mmu.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/riscv/kvm/mmu.c b/arch/riscv/kvm/mmu.c
index 2d3def024270..b5873bca9dce 100644
--- a/arch/riscv/kvm/mmu.c
+++ b/arch/riscv/kvm/mmu.c
@@ -142,9 +142,8 @@ void kvm_arch_commit_memory_region(struct kvm *kvm,
 				enum kvm_mr_change change)
 {
 	/*
-	 * At this point memslot has been committed and there is an
-	 * allocated dirty_bitmap[], dirty pages will be tracked while
-	 * the memory slot is write protected.
+	 * At this point memslot has been committed and dirty pages will be
+	 * tracked while the memory slot is write protected.
 	 */
 	if (change != KVM_MR_DELETE && new->flags & KVM_MEM_LOG_DIRTY_PAGES) {
 		if (kvm_dirty_log_manual_protect_and_init_set(kvm))
@@ -433,8 +432,8 @@ int kvm_riscv_mmu_map(struct kvm_vcpu *vcpu, struct kvm_memory_slot *memslot,
 	struct vm_area_struct *vma;
 	struct kvm *kvm = vcpu->kvm;
 	struct kvm_mmu_memory_cache *pcache = &vcpu->arch.mmu_page_cache;
-	bool logging = (memslot->dirty_bitmap &&
-			!(memslot->flags & KVM_MEM_READONLY)) ? true : false;
+	bool logging = kvm_slot_dirty_track_enabled(memslot) &&
+		       !(memslot->flags & KVM_MEM_READONLY);
 	unsigned long vma_pagesize, mmu_seq;
 	struct kvm_gstage gstage;
 	struct page *page;
-- 
2.54.0

Re: [PATCH] RISC-V: KVM: Enhance the logging check for mmu mapping

Posted by Anup Patel 2 days, 23 hours ago

On Thu, May 28, 2026 at 5:08 PM Inochi Amaoto <inochiama@gmail.com> wrote:
>
> When enabling dirty ring, the dirty bitmap is disable, and the logging
> check is always false as the RISC-V architecture does not select
> "NEED_KVM_DIRTY_RING_WITH_BITMAP". Although the dirty log is recorded
> since the write path already trying to add the dirty log, the logic for
> logging check is broken and some side effect will occurs.
>
> Enhance the logging check for mmu mapping so it can check both the dirty
> ring and the dirty bitmap.
>
> Signed-off-by: Inochi Amaoto <inochiama@gmail.com>

LGTM.

Reviewed-by: Anup Patel <anup@brainfault.org>

Queued this patch for Linux-7.2

Thanks,
Anup

> ---
>  arch/riscv/kvm/mmu.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/arch/riscv/kvm/mmu.c b/arch/riscv/kvm/mmu.c
> index 2d3def024270..b5873bca9dce 100644
> --- a/arch/riscv/kvm/mmu.c
> +++ b/arch/riscv/kvm/mmu.c
> @@ -142,9 +142,8 @@ void kvm_arch_commit_memory_region(struct kvm *kvm,
>                                 enum kvm_mr_change change)
>  {
>         /*
> -        * At this point memslot has been committed and there is an
> -        * allocated dirty_bitmap[], dirty pages will be tracked while
> -        * the memory slot is write protected.
> +        * At this point memslot has been committed and dirty pages will be
> +        * tracked while the memory slot is write protected.
>          */
>         if (change != KVM_MR_DELETE && new->flags & KVM_MEM_LOG_DIRTY_PAGES) {
>                 if (kvm_dirty_log_manual_protect_and_init_set(kvm))
> @@ -433,8 +432,8 @@ int kvm_riscv_mmu_map(struct kvm_vcpu *vcpu, struct kvm_memory_slot *memslot,
>         struct vm_area_struct *vma;
>         struct kvm *kvm = vcpu->kvm;
>         struct kvm_mmu_memory_cache *pcache = &vcpu->arch.mmu_page_cache;
> -       bool logging = (memslot->dirty_bitmap &&
> -                       !(memslot->flags & KVM_MEM_READONLY)) ? true : false;
> +       bool logging = kvm_slot_dirty_track_enabled(memslot) &&
> +                      !(memslot->flags & KVM_MEM_READONLY);
>         unsigned long vma_pagesize, mmu_seq;
>         struct kvm_gstage gstage;
>         struct page *page;
> --
> 2.54.0
>

Re: [PATCH] RISC-V: KVM: Enhance the logging check for mmu mapping

Posted by Anup Patel 3 days, 7 hours ago

On Thu, May 28, 2026 at 5:08 PM Inochi Amaoto <inochiama@gmail.com> wrote:
>
> When enabling dirty ring, the dirty bitmap is disable, and the logging
> check is always false as the RISC-V architecture does not select
> "NEED_KVM_DIRTY_RING_WITH_BITMAP". Although the dirty log is recorded
> since the write path already trying to add the dirty log, the logic for
> logging check is broken and some side effect will occurs.
>
> Enhance the logging check for mmu mapping so it can check both the dirty
> ring and the dirty bitmap.
>
> Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> ---
>  arch/riscv/kvm/mmu.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/arch/riscv/kvm/mmu.c b/arch/riscv/kvm/mmu.c
> index 2d3def024270..b5873bca9dce 100644
> --- a/arch/riscv/kvm/mmu.c
> +++ b/arch/riscv/kvm/mmu.c
> @@ -142,9 +142,8 @@ void kvm_arch_commit_memory_region(struct kvm *kvm,
>                                 enum kvm_mr_change change)
>  {
>         /*
> -        * At this point memslot has been committed and there is an
> -        * allocated dirty_bitmap[], dirty pages will be tracked while
> -        * the memory slot is write protected.
> +        * At this point memslot has been committed and dirty pages will be
> +        * tracked while the memory slot is write protected.
>          */
>         if (change != KVM_MR_DELETE && new->flags & KVM_MEM_LOG_DIRTY_PAGES) {
>                 if (kvm_dirty_log_manual_protect_and_init_set(kvm))
> @@ -433,8 +432,8 @@ int kvm_riscv_mmu_map(struct kvm_vcpu *vcpu, struct kvm_memory_slot *memslot,
>         struct vm_area_struct *vma;
>         struct kvm *kvm = vcpu->kvm;
>         struct kvm_mmu_memory_cache *pcache = &vcpu->arch.mmu_page_cache;
> -       bool logging = (memslot->dirty_bitmap &&
> -                       !(memslot->flags & KVM_MEM_READONLY)) ? true : false;

Why is this not correct approach to determine whether logging is enabled ?

KVM ARM64 does a very similar thing (Refer, memslot_is_logging() in
arch/arm64/kvm/mmu.c).

> +       bool logging = kvm_slot_dirty_track_enabled(memslot) &&
> +                      !(memslot->flags & KVM_MEM_READONLY);
>         unsigned long vma_pagesize, mmu_seq;
>         struct kvm_gstage gstage;
>         struct page *page;
> --
> 2.54.0
>

Regards,
Anup

Re: [PATCH] RISC-V: KVM: Enhance the logging check for mmu mapping

Posted by Inochi Amaoto 3 days, 6 hours ago

On Fri, Jun 05, 2026 at 12:19:03PM +0530, Anup Patel wrote:
> On Thu, May 28, 2026 at 5:08 PM Inochi Amaoto <inochiama@gmail.com> wrote:
> >
> > When enabling dirty ring, the dirty bitmap is disable, and the logging
> > check is always false as the RISC-V architecture does not select
> > "NEED_KVM_DIRTY_RING_WITH_BITMAP". Although the dirty log is recorded
> > since the write path already trying to add the dirty log, the logic for
> > logging check is broken and some side effect will occurs.
> >
> > Enhance the logging check for mmu mapping so it can check both the dirty
> > ring and the dirty bitmap.
> >
> > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > ---
> >  arch/riscv/kvm/mmu.c | 9 ++++-----
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/riscv/kvm/mmu.c b/arch/riscv/kvm/mmu.c
> > index 2d3def024270..b5873bca9dce 100644
> > --- a/arch/riscv/kvm/mmu.c
> > +++ b/arch/riscv/kvm/mmu.c
> > @@ -142,9 +142,8 @@ void kvm_arch_commit_memory_region(struct kvm *kvm,
> >                                 enum kvm_mr_change change)
> >  {
> >         /*
> > -        * At this point memslot has been committed and there is an
> > -        * allocated dirty_bitmap[], dirty pages will be tracked while
> > -        * the memory slot is write protected.
> > +        * At this point memslot has been committed and dirty pages will be
> > +        * tracked while the memory slot is write protected.
> >          */
> >         if (change != KVM_MR_DELETE && new->flags & KVM_MEM_LOG_DIRTY_PAGES) {
> >                 if (kvm_dirty_log_manual_protect_and_init_set(kvm))
> > @@ -433,8 +432,8 @@ int kvm_riscv_mmu_map(struct kvm_vcpu *vcpu, struct kvm_memory_slot *memslot,
> >         struct vm_area_struct *vma;
> >         struct kvm *kvm = vcpu->kvm;
> >         struct kvm_mmu_memory_cache *pcache = &vcpu->arch.mmu_page_cache;
> > -       bool logging = (memslot->dirty_bitmap &&
> > -                       !(memslot->flags & KVM_MEM_READONLY)) ? true : false;
> 
> Why is this not correct approach to determine whether logging is enabled ?
> 
> KVM ARM64 does a very similar thing (Refer, memslot_is_logging() in
> arch/arm64/kvm/mmu.c).
> 

ARM64 is correct as it select NEED_KVM_DIRTY_RING_WITH_BITMAP, which
will add a backup bitmap when enabling dirty ring. However, the RISCV
does not select this, which means the bitmap is not present when the
dirty ring is used. 

Although we still have dirty log with dirty ring now, it comes from
the check in the mark_page_dirty_in_slot(). And the check for variable
"logging" is always false when enabling dirty ring.

Regards,
Inochi

> > +       bool logging = kvm_slot_dirty_track_enabled(memslot) &&
> > +                      !(memslot->flags & KVM_MEM_READONLY);
> >         unsigned long vma_pagesize, mmu_seq;
> >         struct kvm_gstage gstage;
> >         struct page *page;
> > --
> > 2.54.0
> >
> 
> Regards,
> Anup
> 
> -- 
> kvm-riscv mailing list
> kvm-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kvm-riscv